From 36ad15160991c334b782a9c46ba79517683aeb70 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 9 Nov 2020 14:35:02 -0500 Subject: [PATCH 1/9] copy variables files from Packer --- jobspec2/addrs/README.md | 1 + jobspec2/addrs/doc.go | 11 + jobspec2/addrs/input_variable.go | 11 + jobspec2/addrs/parse_ref.go | 93 +++++ jobspec2/addrs/referenceable.go | 22 + jobspec2/types.variables.go | 691 +++++++++++++++++++++++++++++++ 6 files changed, 829 insertions(+) create mode 100644 jobspec2/addrs/README.md create mode 100644 jobspec2/addrs/doc.go create mode 100644 jobspec2/addrs/input_variable.go create mode 100644 jobspec2/addrs/parse_ref.go create mode 100644 jobspec2/addrs/referenceable.go create mode 100644 jobspec2/types.variables.go diff --git a/jobspec2/addrs/README.md b/jobspec2/addrs/README.md new file mode 100644 index 000000000..9f952d5da --- /dev/null +++ b/jobspec2/addrs/README.md @@ -0,0 +1 @@ +This package is copied from Packer: https://github.com/hashicorp/packer/tree/2bf912bddf297c907deef286b1d63dcd07e2c6c2/hcl2template/addrs diff --git a/jobspec2/addrs/doc.go b/jobspec2/addrs/doc.go new file mode 100644 index 000000000..13c300030 --- /dev/null +++ b/jobspec2/addrs/doc.go @@ -0,0 +1,11 @@ +// Package addrs contains types that represent "addresses", which are +// references to specific objects within a Packer configuration. +// +// All addresses have string representations based on HCL traversal syntax +// which should be used in the user-interface, and also in-memory +// representations that can be used internally. +// +// All types within this package should be treated as immutable, even if this +// is not enforced by the Go compiler. It is always an implementation error +// to modify an address object in-place after it is initially constructed. +package addrs diff --git a/jobspec2/addrs/input_variable.go b/jobspec2/addrs/input_variable.go new file mode 100644 index 000000000..ad4370bb8 --- /dev/null +++ b/jobspec2/addrs/input_variable.go @@ -0,0 +1,11 @@ +package addrs + +// InputVariable is the address of an input variable. +type InputVariable struct { + referenceable + Name string +} + +func (v InputVariable) String() string { + return "var." + v.Name +} diff --git a/jobspec2/addrs/parse_ref.go b/jobspec2/addrs/parse_ref.go new file mode 100644 index 000000000..ee3f238ef --- /dev/null +++ b/jobspec2/addrs/parse_ref.go @@ -0,0 +1,93 @@ +package addrs + +import ( + "fmt" + + "github.com/hashicorp/hcl/v2" +) + +// Reference describes a reference to an address with source location +// information. +type Reference struct { + Subject Referenceable + SourceRange hcl.Range + Remaining hcl.Traversal +} + +// ParseRef attempts to extract a referencable address from the prefix of the +// given traversal, which must be an absolute traversal or this function +// will panic. +// +// If no error diagnostics are returned, the returned reference includes the +// address that was extracted, the source range it was extracted from, and any +// remaining relative traversal that was not consumed as part of the +// reference. +// +// If error diagnostics are returned then the Reference value is invalid and +// must not be used. +func ParseRef(traversal hcl.Traversal) (*Reference, hcl.Diagnostics) { + ref, diags := parseRef(traversal) + + // Normalize a little to make life easier for callers. + if ref != nil { + if len(ref.Remaining) == 0 { + ref.Remaining = nil + } + } + + return ref, diags +} + +func parseRef(traversal hcl.Traversal) (*Reference, hcl.Diagnostics) { + var diags hcl.Diagnostics + + root := traversal.RootName() + rootRange := traversal[0].SourceRange() + + switch root { + + case "var": + name, rng, remain, diags := parseSingleAttrRef(traversal) + return &Reference{ + Subject: InputVariable{Name: name}, + SourceRange: rng, + Remaining: remain, + }, diags + + default: + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unhandled reference type", + Detail: `Currently parseRef can only parse "var" references.`, + Subject: &rootRange, + }) + } + return nil, diags +} + +func parseSingleAttrRef(traversal hcl.Traversal) (string, hcl.Range, hcl.Traversal, hcl.Diagnostics) { + var diags hcl.Diagnostics + + root := traversal.RootName() + rootRange := traversal[0].SourceRange() + + if len(traversal) < 2 { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid reference", + Detail: fmt.Sprintf("The %q object cannot be accessed directly. Instead, access one of its attributes.", root), + Subject: &rootRange, + }) + return "", hcl.Range{}, nil, diags + } + if attrTrav, ok := traversal[1].(hcl.TraverseAttr); ok { + return attrTrav.Name, hcl.RangeBetween(rootRange, attrTrav.SrcRange), traversal[2:], diags + } + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid reference", + Detail: fmt.Sprintf("The %q object does not support this operation.", root), + Subject: traversal[1].SourceRange().Ptr(), + }) + return "", hcl.Range{}, nil, diags +} diff --git a/jobspec2/addrs/referenceable.go b/jobspec2/addrs/referenceable.go new file mode 100644 index 000000000..8caec1241 --- /dev/null +++ b/jobspec2/addrs/referenceable.go @@ -0,0 +1,22 @@ +package addrs + +// Referenceable is an interface implemented by all address types that can +// appear as references in configuration language expressions. +type Referenceable interface { + // referenceableSigil is private to ensure that all Referenceables are + // implentented in this current package. For now this does nothing. + referenceableSigil() + + // String produces a string representation of the address that could be + // parsed as a HCL traversal and passed to ParseRef to produce an identical + // result. + String() string +} + +// referenceable is an empty struct that implements Referenceable, add it to +// your Referenceable struct so that it can be recognized as such. +type referenceable struct { +} + +func (r referenceable) referenceableSigil() { +} diff --git a/jobspec2/types.variables.go b/jobspec2/types.variables.go new file mode 100644 index 000000000..f0b5a4449 --- /dev/null +++ b/jobspec2/types.variables.go @@ -0,0 +1,691 @@ +package jobspec2 + +// This file is copied verbatim from Packer: https://github.com/hashicorp/packer/blob/7a1680df97e028c4a75622effe08f6610d0ee5b4/hcl2template/types.variables.go +// with few changes. Packer references in comments are preserved to reduce the diff between files. + +import ( + "fmt" + "strings" + "unicode" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/ext/typeexpr" + "github.com/hashicorp/hcl/v2/gohcl" + "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/hashicorp/nomad/jobspec2/addrs" + "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/convert" +) + +// A consistent detail message for all "not a valid identifier" diagnostics. +const badIdentifierDetail = "A name must start with a letter or underscore and may contain only letters, digits, underscores, and dashes." + +// Local represents a single entry from a "locals" block in a file. +// The "locals" block itself is not represented, because it serves only to +// provide context for us to interpret its contents. +type LocalBlock struct { + Name string + Expr hcl.Expression +} + +// VariableAssignment represents a way a variable was set: the expression +// setting it and the value of that expression. It helps pinpoint were +// something was set in diagnostics. +type VariableAssignment struct { + // From tells were it was taken from, command/varfile/env/default + From string + Value cty.Value + Expr hcl.Expression +} + +type Variable struct { + // Values contains possible values for the variable; The last value set + // from these will be the one used. If none is set; an error will be + // returned by Value(). + Values []VariableAssignment + + // Validations contains all variables validation rules to be applied to the + // used value. Only the used value - the last value from Values - is + // validated. + Validations []*VariableValidation + + // Cty Type of the variable. If the default value or a collected value is + // not of this type nor can be converted to this type an error diagnostic + // will show up. This allows us to assume that values are valid later in + // code. + // + // When a default value - and no type - is passed in the variable + // declaration, the type of the default variable will be used. This will + // allow to ensure that users set this variable correctly. + Type cty.Type + // Common name of the variable + Name string + // Description of the variable + Description string + + Range hcl.Range +} + +func (v *Variable) GoString() string { + b := &strings.Builder{} + fmt.Fprintf(b, "{type:%s", v.Type.GoString()) + for _, vv := range v.Values { + fmt.Fprintf(b, ",%s:%s", vv.From, vv.Value) + } + fmt.Fprintf(b, "}") + return b.String() +} + +// validateValue ensures that all of the configured custom validations for a +// variable value are passing. +// +func (v *Variable) validateValue(val VariableAssignment) (diags hcl.Diagnostics) { + if len(v.Validations) == 0 { + return nil + } + + hclCtx := &hcl.EvalContext{ + Variables: map[string]cty.Value{ + "var": cty.ObjectVal(map[string]cty.Value{ + v.Name: val.Value, + }), + }, + Functions: Functions("", false), + } + + for _, validation := range v.Validations { + const errInvalidCondition = "Invalid variable validation result" + + result, moreDiags := validation.Condition.Value(hclCtx) + diags = append(diags, moreDiags...) + if moreDiags.HasErrors() { + } + if !result.IsKnown() { + continue // We'll wait until we've learned more, then. + } + if result.IsNull() { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: errInvalidCondition, + Detail: "Validation condition expression must return either true or false, not null.", + Subject: validation.Condition.Range().Ptr(), + Expression: validation.Condition, + EvalContext: hclCtx, + }) + continue + } + var err error + result, err = convert.Convert(result, cty.Bool) + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: errInvalidCondition, + Detail: fmt.Sprintf("Invalid validation condition result value: %s.", err), + Subject: validation.Condition.Range().Ptr(), + Expression: validation.Condition, + EvalContext: hclCtx, + }) + continue + } + + if result.False() { + subj := validation.DeclRange.Ptr() + if val.Expr != nil { + subj = val.Expr.Range().Ptr() + } + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Invalid value for %s variable", val.From), + Detail: fmt.Sprintf("%s\n\nThis was checked by the validation rule at %s.", validation.ErrorMessage, validation.DeclRange.String()), + Subject: subj, + }) + } + } + + return diags +} + +// Value returns the last found value from the list of variable settings. +func (v *Variable) Value() (cty.Value, hcl.Diagnostics) { + if len(v.Values) == 0 { + return cty.UnknownVal(v.Type), hcl.Diagnostics{&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Unset variable %q", v.Name), + Detail: "A used variable must be set or have a default value; see " + + "https://packer.io/docs/configuration/from-1.5/syntax for " + + "details.", + Context: v.Range.Ptr(), + }} + } + val := v.Values[len(v.Values)-1] + return val.Value, v.validateValue(v.Values[len(v.Values)-1]) +} + +type Variables map[string]*Variable + +func (variables Variables) Keys() []string { + keys := make([]string, 0, len(variables)) + for key := range variables { + keys = append(keys, key) + } + return keys +} + +func (variables Variables) Values() (map[string]cty.Value, hcl.Diagnostics) { + res := map[string]cty.Value{} + var diags hcl.Diagnostics + for k, v := range variables { + value, moreDiags := v.Value() + diags = append(diags, moreDiags...) + res[k] = value + } + return res, diags +} + +// decodeVariable decodes a variable key and value into Variables +func (variables *Variables) decodeVariable(key string, attr *hcl.Attribute, ectx *hcl.EvalContext) hcl.Diagnostics { + var diags hcl.Diagnostics + + if (*variables) == nil { + (*variables) = Variables{} + } + + if _, found := (*variables)[key]; found { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Duplicate variable", + Detail: "Duplicate " + key + " variable definition found.", + Subject: attr.NameRange.Ptr(), + }) + return diags + } + + value, moreDiags := attr.Expr.Value(ectx) + diags = append(diags, moreDiags...) + if moreDiags.HasErrors() { + return diags + } + + (*variables)[key] = &Variable{ + Name: key, + Values: []VariableAssignment{{ + From: "default", + Value: value, + Expr: attr.Expr, + }}, + Type: value.Type(), + Range: attr.Range, + } + + return diags +} + +var variableBlockSchema = &hcl.BodySchema{ + Attributes: []hcl.AttributeSchema{ + { + Name: "description", + }, + { + Name: "default", + }, + { + Name: "type", + }, + }, + Blocks: []hcl.BlockHeaderSchema{ + { + Type: "validation", + }, + }, +} + +// decodeVariableBlock decodes a "variables" section the way packer 1 used to +func (variables *Variables) decodeVariableBlock(block *hcl.Block, ectx *hcl.EvalContext) hcl.Diagnostics { + if (*variables) == nil { + (*variables) = Variables{} + } + + if _, found := (*variables)[block.Labels[0]]; found { + + return []*hcl.Diagnostic{{ + Severity: hcl.DiagError, + Summary: "Duplicate variable", + Detail: "Duplicate " + block.Labels[0] + " variable definition found.", + Context: block.DefRange.Ptr(), + }} + } + + name := block.Labels[0] + + content, diags := block.Body.Content(variableBlockSchema) + if !hclsyntax.ValidIdentifier(name) { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid variable name", + Detail: badIdentifierDetail, + Subject: &block.LabelRanges[0], + }) + } + + v := &Variable{ + Name: name, + Range: block.DefRange, + } + + if attr, exists := content.Attributes["description"]; exists { + valDiags := gohcl.DecodeExpression(attr.Expr, nil, &v.Description) + diags = append(diags, valDiags...) + } + + if t, ok := content.Attributes["type"]; ok { + tp, moreDiags := typeexpr.Type(t.Expr) + diags = append(diags, moreDiags...) + if moreDiags.HasErrors() { + return diags + } + + v.Type = tp + } + + if def, ok := content.Attributes["default"]; ok { + defaultValue, moreDiags := def.Expr.Value(ectx) + diags = append(diags, moreDiags...) + if moreDiags.HasErrors() { + return diags + } + + if v.Type != cty.NilType { + var err error + defaultValue, err = convert.Convert(defaultValue, v.Type) + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid default value for variable", + Detail: fmt.Sprintf("This default value is not compatible with the variable's type constraint: %s.", err), + Subject: def.Expr.Range().Ptr(), + }) + defaultValue = cty.DynamicVal + } + } + + v.Values = append(v.Values, VariableAssignment{ + From: "default", + Value: defaultValue, + Expr: def.Expr, + }) + + // It's possible no type attribute was assigned so lets make sure we + // have a valid type otherwise there could be issues parsing the value. + if v.Type == cty.NilType { + v.Type = defaultValue.Type() + } + } + + for _, block := range content.Blocks { + switch block.Type { + case "validation": + vv, moreDiags := decodeVariableValidationBlock(v.Name, block) + diags = append(diags, moreDiags...) + v.Validations = append(v.Validations, vv) + } + } + + (*variables)[name] = v + + return diags +} + +var variableValidationBlockSchema = &hcl.BodySchema{ + Attributes: []hcl.AttributeSchema{ + { + Name: "condition", + Required: true, + }, + { + Name: "error_message", + Required: true, + }, + }, +} + +// VariableValidation represents a configuration-defined validation rule +// for a particular input variable, given as a "validation" block inside +// a "variable" block. +type VariableValidation struct { + // Condition is an expression that refers to the variable being tested and + // contains no other references. The expression must return true to + // indicate that the value is valid or false to indicate that it is + // invalid. If the expression produces an error, that's considered a bug in + // the block defining the validation rule, not an error in the caller. + Condition hcl.Expression + + // ErrorMessage is one or more full sentences, which _should_ be in English + // for consistency with the rest of the error message output but can in + // practice be in any language as long as it ends with a period. The + // message should describe what is required for the condition to return + // true in a way that would make sense to a caller of the module. + ErrorMessage string + + DeclRange hcl.Range +} + +func decodeVariableValidationBlock(varName string, block *hcl.Block) (*VariableValidation, hcl.Diagnostics) { + var diags hcl.Diagnostics + vv := &VariableValidation{ + DeclRange: block.DefRange, + } + + content, moreDiags := block.Body.Content(variableValidationBlockSchema) + diags = append(diags, moreDiags...) + + if attr, exists := content.Attributes["condition"]; exists { + vv.Condition = attr.Expr + + // The validation condition must refer to the variable itself and + // nothing else; to ensure that the variable declaration can't create + // additional edges in the dependency graph. + goodRefs := 0 + for _, traversal := range vv.Condition.Variables() { + + ref, moreDiags := addrs.ParseRef(traversal) + if !moreDiags.HasErrors() { + if addr, ok := ref.Subject.(addrs.InputVariable); ok { + if addr.Name == varName { + goodRefs++ + continue // Reference is valid + } + } + } + + // If we fall out here then the reference is invalid. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid reference in variable validation", + Detail: fmt.Sprintf("The condition for variable %q can only refer to the variable itself, using var.%s.", varName, varName), + Subject: traversal.SourceRange().Ptr(), + }) + } + if goodRefs < 1 { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid variable validation condition", + Detail: fmt.Sprintf("The condition for variable %q must refer to var.%s in order to test incoming values.", varName, varName), + Subject: attr.Expr.Range().Ptr(), + }) + } + } + + if attr, exists := content.Attributes["error_message"]; exists { + moreDiags := gohcl.DecodeExpression(attr.Expr, nil, &vv.ErrorMessage) + diags = append(diags, moreDiags...) + if !moreDiags.HasErrors() { + const errSummary = "Invalid validation error message" + switch { + case vv.ErrorMessage == "": + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: errSummary, + Detail: "An empty string is not a valid nor useful error message.", + Subject: attr.Expr.Range().Ptr(), + }) + case !looksLikeSentences(vv.ErrorMessage): + // Because we're going to include this string verbatim as part + // of a bigger error message written in our usual style, we'll + // require the given error message to conform to that. We might + // relax this in future if e.g. we start presenting these error + // messages in a different way, or if Packer starts supporting + // producing error messages in other human languages, etc. For + // pragmatism we also allow sentences ending with exclamation + // points, but we don't mention it explicitly here because + // that's not really consistent with the Packer UI writing + // style. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: errSummary, + Detail: "Validation error message must be at least one full sentence starting with an uppercase letter ( if the alphabet permits it ) and ending with a period or question mark.", + Subject: attr.Expr.Range().Ptr(), + }) + } + } + } + + return vv, diags +} + +// looksLikeSentence is a simple heuristic that encourages writing error +// messages that will be presentable when included as part of a larger error +// diagnostic whose other text is written in the UI writing style. +// +// This is intentionally not a very strong validation since we're assuming that +// authors want to write good messages and might just need a nudge about +// Packer's specific style, rather than that they are going to try to work +// around these rules to write a lower-quality message. +func looksLikeSentences(s string) bool { + if len(s) < 1 { + return false + } + runes := []rune(s) // HCL guarantees that all strings are valid UTF-8 + first := runes[0] + last := runes[len(runes)-1] + + // If the first rune is a letter then it must be an uppercase letter. To + // sorts of nudge people into writing sentences. For alphabets that don't + // have the notion of 'upper', this does nothing. + if unicode.IsLetter(first) && !unicode.IsUpper(first) { + return false + } + + // The string must be at least one full sentence, which implies having + // sentence-ending punctuation. + return last == '.' || last == '?' || last == '!' +} + +// Prefix your environment variables with VarEnvPrefix so that Packer can see +// them. +const VarEnvPrefix = "NOMAD_VAR_" + +func (c *jobConfig) collectInputVariableValues(env []string, files []*hcl.File, argv map[string]string) hcl.Diagnostics { + var diags hcl.Diagnostics + variables := c.InputVariables + + for _, raw := range env { + if !strings.HasPrefix(raw, VarEnvPrefix) { + continue + } + raw = raw[len(VarEnvPrefix):] // trim the prefix + + eq := strings.Index(raw, "=") + if eq == -1 { + // Seems invalid, so we'll ignore it. + continue + } + + name := raw[:eq] + value := raw[eq+1:] + + variable, found := variables[name] + if !found { + // this variable was not defined in the hcl files, let's skip it ! + continue + } + + fakeFilename := fmt.Sprintf("", name) + expr, moreDiags := expressionFromVariableDefinition(fakeFilename, value, variable.Type) + diags = append(diags, moreDiags...) + if moreDiags.HasErrors() { + continue + } + + val, valDiags := expr.Value(nil) + diags = append(diags, valDiags...) + if variable.Type != cty.NilType { + var err error + val, err = convert.Convert(val, variable.Type) + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid value for variable", + Detail: fmt.Sprintf("The value for %s is not compatible with the variable's type constraint: %s.", name, err), + Subject: expr.Range().Ptr(), + }) + val = cty.DynamicVal + } + } + variable.Values = append(variable.Values, VariableAssignment{ + From: "env", + Value: val, + Expr: expr, + }) + } + + // files will contain files found in the folder then files passed as + // arguments. + for _, file := range files { + // Before we do our real decode, we'll probe to see if there are any + // blocks of type "variable" in this body, since it's a common mistake + // for new users to put variable declarations in pkrvars rather than + // variable value definitions, and otherwise our error message for that + // case is not so helpful. + { + content, _, _ := file.Body.PartialContent(&hcl.BodySchema{ + Blocks: []hcl.BlockHeaderSchema{ + { + Type: "variable", + LabelNames: []string{"name"}, + }, + }, + }) + for _, block := range content.Blocks { + name := block.Labels[0] + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Variable declaration in a .var file", + Detail: fmt.Sprintf("A .var file is used to assign "+ + "values to variables that have already been declared "+ + "in job files, not to declare new variables. To "+ + "declare variable %q, place this block in one of your"+ + " job files\n\nTo set a "+ + "value for this variable in %s, use the definition "+ + "syntax instead:\n %s = ", + name, block.TypeRange.Filename, name), + Subject: &block.TypeRange, + }) + } + if diags.HasErrors() { + // If we already found problems then JustAttributes below will find + // the same problems with less-helpful messages, so we'll bail for + // now to let the user focus on the immediate problem. + return diags + } + } + + attrs, moreDiags := file.Body.JustAttributes() + diags = append(diags, moreDiags...) + + for name, attr := range attrs { + variable, found := variables[name] + if !found { + sev := hcl.DiagWarning + if c.ParseConfig.Strict { + sev = hcl.DiagError + } + diags = append(diags, &hcl.Diagnostic{ + Severity: sev, + Summary: "Undefined variable", + Detail: fmt.Sprintf("A %q variable was set but was "+ + "not found in known variables. To declare "+ + "variable %q, place this block in your "+ + "job files", + name, name), + Context: attr.Range.Ptr(), + }) + continue + } + + val, moreDiags := attr.Expr.Value(nil) + diags = append(diags, moreDiags...) + + if variable.Type != cty.NilType { + var err error + val, err = convert.Convert(val, variable.Type) + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid value for variable", + Detail: fmt.Sprintf("The value for %s is not compatible with the variable's type constraint: %s.", name, err), + Subject: attr.Expr.Range().Ptr(), + }) + val = cty.DynamicVal + } + } + + variable.Values = append(variable.Values, VariableAssignment{ + From: "varfile", + Value: val, + Expr: attr.Expr, + }) + } + } + + // Finally we process values given explicitly on the command line. + for name, value := range argv { + variable, found := variables[name] + if !found { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Undefined -var variable", + Detail: fmt.Sprintf("A %q variable was passed in the command "+ + "line but was not found in known variables. "+ + "To declare variable %q, place this block in your"+ + " job file", + name, name), + }) + continue + } + + fakeFilename := fmt.Sprintf("", name) + expr, moreDiags := expressionFromVariableDefinition(fakeFilename, value, variable.Type) + diags = append(diags, moreDiags...) + if moreDiags.HasErrors() { + continue + } + + val, valDiags := expr.Value(nil) + diags = append(diags, valDiags...) + + if variable.Type != cty.NilType { + var err error + val, err = convert.Convert(val, variable.Type) + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid argument value for -var variable", + Detail: fmt.Sprintf("The received arg value for %s is not compatible with the variable's type constraint: %s.", name, err), + Subject: expr.Range().Ptr(), + }) + val = cty.DynamicVal + } + } + + variable.Values = append(variable.Values, VariableAssignment{ + From: "cmd", + Value: val, + Expr: expr, + }) + } + + return diags +} + +// expressionFromVariableDefinition creates an hclsyntax.Expression that is capable of evaluating the specified value for a given cty.Type. +// The specified filename is to identify the source of where value originated from in the diagnostics report, if there is an error. +func expressionFromVariableDefinition(filename string, value string, variableType cty.Type) (hclsyntax.Expression, hcl.Diagnostics) { + switch variableType { + case cty.String, cty.Number, cty.NilType: + // when the type is nil (not set in a variable block) we default to + // interpreting everything as a string literal. + return &hclsyntax.LiteralValueExpr{Val: cty.StringVal(value)}, nil + default: + return hclsyntax.ParseExpression([]byte(value), filename, hcl.Pos{Line: 1, Column: 1}) + } +} From 878b3f82fea1bff3d7e6f2bad87f00edee7a86a7 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 9 Nov 2020 14:58:57 -0500 Subject: [PATCH 2/9] Restructure Variables and Locals This reimplements the handling of Variables and Locals in HCL2 config. This change supports declaring variables and locals, with defaults. --- jobspec2/hcl_conversions.go | 1 - jobspec2/parse.go | 107 +++++++------ jobspec2/parse_job.go | 51 +----- jobspec2/types.config.go | 299 ++++++++++++++++++++++++++++++++++++ 4 files changed, 369 insertions(+), 89 deletions(-) create mode 100644 jobspec2/types.config.go diff --git a/jobspec2/hcl_conversions.go b/jobspec2/hcl_conversions.go index 928981b3d..fa70600ee 100644 --- a/jobspec2/hcl_conversions.go +++ b/jobspec2/hcl_conversions.go @@ -33,7 +33,6 @@ func newHCLDecoder() *gohcl.Decoder { // custom nomad types decoder.RegisterBlockDecoder(reflect.TypeOf(api.Affinity{}), decodeAffinity) decoder.RegisterBlockDecoder(reflect.TypeOf(api.Constraint{}), decodeConstraint) - decoder.RegisterBlockDecoder(reflect.TypeOf(jobWrapper{}), decodeJob) return decoder } diff --git a/jobspec2/parse.go b/jobspec2/parse.go index cc04d0b55..1f1282cd4 100644 --- a/jobspec2/parse.go +++ b/jobspec2/parse.go @@ -3,7 +3,9 @@ package jobspec2 import ( "bytes" "errors" + "fmt" "io" + "io/ioutil" "os" "path/filepath" "strings" @@ -14,66 +16,85 @@ import ( hcljson "github.com/hashicorp/hcl/v2/json" "github.com/hashicorp/nomad/api" "github.com/hashicorp/nomad/jobspec2/hclutil" - "github.com/zclconf/go-cty/cty" ) func Parse(path string, r io.Reader) (*api.Job, error) { - return ParseWithArgs(path, r, nil, false) -} - -func toVars(vars map[string]string) cty.Value { - attrs := make(map[string]cty.Value, len(vars)) - for k, v := range vars { - attrs[k] = cty.StringVal(v) - } - - return cty.ObjectVal(attrs) -} - -func ParseWithArgs(path string, r io.Reader, vars map[string]string, allowFS bool) (*api.Job, error) { if path == "" { if f, ok := r.(*os.File); ok { path = f.Name() } } - basedir := filepath.Dir(path) - // Copy the reader into an in-memory buffer first since HCL requires it. var buf bytes.Buffer - if _, err := io.Copy(&buf, r); err != nil { - return nil, err - } - - evalContext := &hcl.EvalContext{ - Functions: Functions(basedir, allowFS), - Variables: map[string]cty.Value{ - "vars": toVars(vars), - }, - UnknownVariable: func(expr string) (cty.Value, error) { - v := "${" + expr + "}" - return cty.StringVal(v), nil - }, - } - var result struct { - Job jobWrapper `hcl:"job,block"` - } - err := decode(path, buf.Bytes(), evalContext, &result) + _, err := io.Copy(&buf, r) if err != nil { return nil, err } - normalizeJob(&result.Job) - return result.Job.Job, nil + return ParseWithConfig(&ParseConfig{ + Path: path, + Body: buf.Bytes(), + AllowFS: false, + Strict: true, + }) } -func decode(filename string, src []byte, ctx *hcl.EvalContext, target interface{}) error { +func ParseWithConfig(args *ParseConfig) (*api.Job, error) { + args.normalize() + + c := &jobConfig{ + ParseConfig: args, + } + err := decode(c) + if err != nil { + return nil, err + } + + normalizeJob(c) + return c.Job, nil +} + +type ParseConfig struct { + Path string + BaseDir string + + // Body is the HCL body + Body []byte + + // AllowFS enables HCL functions that require file system accecss + AllowFS bool + + // ArgVars is the CLI -var arguments + ArgVars []string + + // VarFiles is the paths of variable data files + VarFiles []string + + // Envs represent process environment variable + Envs []string + + Strict bool + + // parsedVarFiles represent parsed HCL AST of the passed EnvVars + parsedVarFiles []*hcl.File +} + +func (c *ParseConfig) normalize() { + if c.BaseDir == "" { + c.BaseDir = filepath.Dir(c.Path) + } +} + +func decode(c *jobConfig) error { var file *hcl.File var diags hcl.Diagnostics - if !isJSON(src) { - file, diags = hclsyntax.ParseConfig(src, filename, hcl.Pos{Line: 1, Column: 1}) + pc := c.ParseConfig + + if !isJSON(pc.Body) { + file, diags = hclsyntax.ParseConfig(pc.Body, pc.Path, hcl.Pos{Line: 1, Column: 1}) } else { - file, diags = hcljson.Parse(src, filename) + file, diags = hcljson.Parse(pc.Body, pc.Path) } if diags.HasErrors() { @@ -81,8 +102,8 @@ func decode(filename string, src []byte, ctx *hcl.EvalContext, target interface{ } body := hclutil.BlocksAsAttrs(file.Body) - body = dynblock.Expand(body, ctx) - diags = hclDecoder.DecodeBody(body, ctx, target) + body = dynblock.Expand(body, c.EvalContext()) + diags = c.decodeBody(body) if diags.HasErrors() { var str strings.Builder for i, diag := range diags { @@ -93,7 +114,7 @@ func decode(filename string, src []byte, ctx *hcl.EvalContext, target interface{ } return errors.New(str.String()) } - diags = append(diags, decodeMapInterfaceType(target, ctx)...) + diags = append(diags, decodeMapInterfaceType(&c, c.EvalContext())...) return nil } diff --git a/jobspec2/parse_job.go b/jobspec2/parse_job.go index ec50fe40f..cf834ce49 100644 --- a/jobspec2/parse_job.go +++ b/jobspec2/parse_job.go @@ -3,49 +3,10 @@ package jobspec2 import ( "time" - "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/hcl/v2/gohcl" "github.com/hashicorp/nomad/api" ) -type jobWrapper struct { - JobID string `hcl:",label"` - Job *api.Job - - Extra struct { - Vault *api.Vault `hcl:"vault,block"` - Tasks []*api.Task `hcl:"task,block"` - } -} - -func decodeJob(body hcl.Body, ctx *hcl.EvalContext, val interface{}) hcl.Diagnostics { - m := val.(*jobWrapper) - extra, _ := gohcl.ImpliedBodySchema(m.Extra) - content, job, diags := body.PartialContent(extra) - if len(diags) != 0 { - return diags - } - - for _, b := range content.Blocks { - if b.Type == "vault" { - v := &api.Vault{} - diags = append(diags, hclDecoder.DecodeBody(b.Body, ctx, v)...) - m.Extra.Vault = v - } else if b.Type == "task" { - t := &api.Task{} - diags = append(diags, hclDecoder.DecodeBody(b.Body, ctx, t)...) - if len(b.Labels) == 1 { - t.Name = b.Labels[0] - m.Extra.Tasks = append(m.Extra.Tasks, t) - } - } - } - - m.Job = &api.Job{} - return hclDecoder.DecodeBody(job, ctx, m.Job) -} - -func normalizeJob(jw *jobWrapper) { +func normalizeJob(jw *jobConfig) { j := jw.Job if j.Name == nil { j.Name = &jw.JobID @@ -59,11 +20,11 @@ func normalizeJob(jw *jobWrapper) { j.Periodic.SpecType = &v } - normalizeVault(jw.Extra.Vault) + normalizeVault(jw.Vault) - if len(jw.Extra.Tasks) != 0 { - alone := make([]*api.TaskGroup, 0, len(jw.Extra.Tasks)) - for _, t := range jw.Extra.Tasks { + if len(jw.Tasks) != 0 { + alone := make([]*api.TaskGroup, 0, len(jw.Tasks)) + for _, t := range jw.Tasks { alone = append(alone, &api.TaskGroup{ Name: &t.Name, Tasks: []*api.Task{t}, @@ -86,7 +47,7 @@ func normalizeJob(jw *jobWrapper) { normalizeVault(t.Vault) if t.Vault == nil { - t.Vault = jw.Extra.Vault + t.Vault = jw.Vault } } } diff --git a/jobspec2/types.config.go b/jobspec2/types.config.go new file mode 100644 index 000000000..8d7a7fc40 --- /dev/null +++ b/jobspec2/types.config.go @@ -0,0 +1,299 @@ +package jobspec2 + +import ( + "fmt" + "strings" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/nomad/api" + "github.com/zclconf/go-cty/cty" +) + +const ( + variablesLabel = "variables" + variableLabel = "variable" + localsLabel = "locals" + vaultLabel = "vault" + taskLabel = "task" + + inputVariablesAccessor = "var" + localsAccessor = "local" +) + +type jobConfig struct { + JobID string `hcl:",label"` + Job *api.Job + + ParseConfig *ParseConfig + + Vault *api.Vault `hcl:"vault,block"` + Tasks []*api.Task `hcl:"task,block"` + + InputVariables Variables + LocalVariables Variables + + LocalBlocks []*LocalBlock +} + +var jobConfigSchema = &hcl.BodySchema{ + Blocks: []hcl.BlockHeaderSchema{ + {Type: variablesLabel}, + {Type: variableLabel, LabelNames: []string{"name"}}, + {Type: localsLabel}, + {Type: "job", LabelNames: []string{"name"}}, + }, +} + +func (c *jobConfig) decodeBody(body hcl.Body) hcl.Diagnostics { + content, diags := body.Content(jobConfigSchema) + if len(diags) != 0 { + return diags + } + + diags = append(diags, c.decodeInputVariables(content)...) + diags = append(diags, c.parseLocalVariables(content)...) + diags = append(diags, c.collectInputVariableValues(c.ParseConfig.Envs, c.ParseConfig.parsedVarFiles, toVars(c.ParseConfig.ArgVars))...) + + _, moreDiags := c.InputVariables.Values() + diags = append(diags, moreDiags...) + _, moreDiags = c.LocalVariables.Values() + diags = append(diags, moreDiags...) + diags = append(diags, c.evaluateLocalVariables(c.LocalBlocks)...) + + nctx := c.EvalContext() + + diags = append(diags, c.decodeJob(content, nctx)...) + return diags +} + +// decodeInputVariables looks in the found blocks for 'variables' and +// 'variable' blocks. It should be called firsthand so that other blocks can +// use the variables. +func (j *jobConfig) decodeInputVariables(content *hcl.BodyContent) hcl.Diagnostics { + var diags hcl.Diagnostics + + for _, block := range content.Blocks { + switch block.Type { + case variableLabel: + moreDiags := j.InputVariables.decodeVariableBlock(block, nil) + diags = append(diags, moreDiags...) + case variablesLabel: + attrs, moreDiags := block.Body.JustAttributes() + diags = append(diags, moreDiags...) + for key, attr := range attrs { + moreDiags = j.InputVariables.decodeVariable(key, attr, nil) + diags = append(diags, moreDiags...) + } + } + } + return diags +} + +// parseLocalVariables looks in the found blocks for 'locals' blocks. It +// should be called after parsing input variables so that they can be +// referenced. +func (c *jobConfig) parseLocalVariables(content *hcl.BodyContent) hcl.Diagnostics { + var diags hcl.Diagnostics + + for _, block := range content.Blocks { + switch block.Type { + case localsLabel: + attrs, moreDiags := block.Body.JustAttributes() + diags = append(diags, moreDiags...) + for name, attr := range attrs { + if _, found := c.LocalVariables[name]; found { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Duplicate value in " + localsLabel, + Detail: "Duplicate " + name + " definition found.", + Subject: attr.NameRange.Ptr(), + Context: block.DefRange.Ptr(), + }) + return diags + } + c.LocalBlocks = append(c.LocalBlocks, &LocalBlock{ + Name: name, + Expr: attr.Expr, + }) + } + } + } + + return diags +} + +func (c *jobConfig) decodeTopLevelExtras(content *hcl.BodyContent, ctx *hcl.EvalContext) hcl.Diagnostics { + var diags hcl.Diagnostics + + var foundVault *hcl.Block + for _, b := range content.Blocks { + if b.Type == vaultLabel { + if foundVault != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Duplicate %s block", b.Type), + Detail: fmt.Sprintf( + "Only one block of type %q is allowed. Previous definition was at %s.", + b.Type, foundVault.DefRange.String(), + ), + Subject: &b.DefRange, + }) + continue + } + foundVault = b + + v := &api.Vault{} + diags = append(diags, hclDecoder.DecodeBody(b.Body, ctx, v)...) + c.Vault = v + + } else if b.Type == taskLabel { + t := &api.Task{} + diags = append(diags, hclDecoder.DecodeBody(b.Body, ctx, t)...) + if len(b.Labels) == 1 { + t.Name = b.Labels[0] + c.Tasks = append(c.Tasks, t) + } + } + } + + return diags +} + +func (c *jobConfig) evaluateLocalVariables(locals []*LocalBlock) hcl.Diagnostics { + var diags hcl.Diagnostics + + if len(locals) > 0 && c.LocalVariables == nil { + c.LocalVariables = Variables{} + } + + var retry, previousL int + for len(locals) > 0 { + local := locals[0] + moreDiags := c.evaluateLocalVariable(local) + if moreDiags.HasErrors() { + if len(locals) == 1 { + // If this is the only local left there's no need + // to try evaluating again + return append(diags, moreDiags...) + } + if previousL == len(locals) { + if retry == 100 { + // To get to this point, locals must have a circle dependency + return append(diags, moreDiags...) + } + retry++ + } + previousL = len(locals) + + // If local uses another local that has not been evaluated yet this could be the reason of errors + // Push local to the end of slice to be evaluated later + locals = append(locals, local) + } else { + retry = 0 + diags = append(diags, moreDiags...) + } + // Remove local from slice + locals = append(locals[:0], locals[1:]...) + } + + return diags +} + +func (c *jobConfig) evaluateLocalVariable(local *LocalBlock) hcl.Diagnostics { + var diags hcl.Diagnostics + + value, moreDiags := local.Expr.Value(c.EvalContext()) + diags = append(diags, moreDiags...) + if moreDiags.HasErrors() { + return diags + } + c.LocalVariables[local.Name] = &Variable{ + Name: local.Name, + Values: []VariableAssignment{{ + Value: value, + Expr: local.Expr, + From: "default", + }}, + Type: value.Type(), + } + + return diags +} + +func (c *jobConfig) decodeJob(content *hcl.BodyContent, ctx *hcl.EvalContext) hcl.Diagnostics { + var diags hcl.Diagnostics + + c.Job = &api.Job{} + + var found *hcl.Block + for _, b := range content.Blocks { + if b.Type != "job" { + continue + } + + if found != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Duplicate %s block", b.Type), + Detail: fmt.Sprintf( + "Only one block of type %q is allowed. Previous definition was at %s.", + b.Type, found.DefRange.String(), + ), + Subject: &b.DefRange, + }) + continue + } + found = b + + c.JobID = b.Labels[0] + + extra, remain, mdiags := b.Body.PartialContent(&hcl.BodySchema{ + Blocks: []hcl.BlockHeaderSchema{ + {Type: "vault"}, + {Type: "task", LabelNames: []string{"name"}}, + }, + }) + diags = append(diags, mdiags...) + diags = append(diags, c.decodeTopLevelExtras(extra, ctx)...) + diags = append(diags, hclDecoder.DecodeBody(remain, ctx, c.Job)...) + } + + if found == nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Missing job block", + Detail: "A job block is required", + }) + } + + return diags + +} + +func (c *jobConfig) EvalContext() *hcl.EvalContext { + vars, _ := c.InputVariables.Values() + locals, _ := c.LocalVariables.Values() + return &hcl.EvalContext{ + Functions: Functions(c.ParseConfig.BaseDir, c.ParseConfig.AllowFS), + Variables: map[string]cty.Value{ + inputVariablesAccessor: cty.ObjectVal(vars), + localsAccessor: cty.ObjectVal(locals), + }, + UnknownVariable: func(expr string) (cty.Value, error) { + v := "${" + expr + "}" + return cty.StringVal(v), nil + }, + } +} + +func toVars(vars []string) map[string]string { + attrs := make(map[string]string, len(vars)) + for _, arg := range vars { + parts := strings.SplitN(arg, "=", 2) + if len(parts) == 2 { + attrs[parts[0]] = parts[1] + } + } + + return attrs +} From 5a784f43a391cb601e9db80cbf0387ad88766ecc Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 9 Nov 2020 15:01:31 -0500 Subject: [PATCH 3/9] Start using the new jobspec2 API --- command/agent/job_endpoint.go | 10 +++++---- command/helpers.go | 38 ++++++++++++----------------------- command/helpers_test.go | 11 ---------- 3 files changed, 19 insertions(+), 40 deletions(-) diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index c4e6d01fd..a37ddb981 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -675,14 +675,16 @@ func (s *HTTPServer) JobsParseRequest(resp http.ResponseWriter, req *http.Reques return nil, CodedError(400, "Job spec is empty") } - jobfile := strings.NewReader(args.JobHCL) - var jobStruct *api.Job var err error if args.HCLv1 { - jobStruct, err = jobspec.Parse(jobfile) + jobStruct, err = jobspec.Parse(strings.NewReader(args.JobHCL)) } else { - jobStruct, err = jobspec2.ParseWithArgs("input.hcl", jobfile, nil, false) + jobStruct, err = jobspec2.ParseWithConfig(&jobspec2.ParseConfig{ + Path: "input.hcl", + Body: []byte(args.JobHCL), + AllowFS: false, + }) } if err != nil { return nil, CodedError(400, err.Error()) diff --git a/command/helpers.go b/command/helpers.go index abd93fbf5..0f87cf140 100644 --- a/command/helpers.go +++ b/command/helpers.go @@ -388,10 +388,10 @@ type JobGetter struct { // StructJob returns the Job struct from jobfile. func (j *JobGetter) ApiJob(jpath string) (*api.Job, error) { - return j.ApiJobWithArgs(jpath, nil) + return j.ApiJobWithArgs(jpath, nil, nil) } -func (j *JobGetter) ApiJobWithArgs(jpath string, vars map[string]string) (*api.Job, error) { +func (j *JobGetter) ApiJobWithArgs(jpath string, vars []string, varfiles []string) (*api.Job, error) { var jobfile io.Reader pathName := filepath.Base(jpath) switch jpath { @@ -447,7 +447,17 @@ func (j *JobGetter) ApiJobWithArgs(jpath string, vars map[string]string) (*api.J if j.hcl1 { jobStruct, err = jobspec.Parse(jobfile) } else { - jobStruct, err = jobspec2.ParseWithArgs(pathName, jobfile, vars, true) + var buf bytes.Buffer + _, err = io.Copy(&buf, jobfile) + if err != nil { + return nil, fmt.Errorf("Error reading job file from %s: %v", jpath, err) + } + jobStruct, err = jobspec2.ParseWithConfig(&jobspec2.ParseConfig{ + Path: pathName, + Body: buf.Bytes(), + ArgVars: vars, + AllowFS: true, + }) } if err != nil { return nil, fmt.Errorf("Error parsing job file from %s:\n%v", jpath, err) @@ -523,25 +533,3 @@ func (w *uiErrorWriter) Close() error { } return nil } - -// parseVars decodes a slice of `=` or `` strings into a golang map. -// -// `` without corresponding value, is mapped to the `` environment variable. -func parseVars(vars []string) map[string]string { - if len(vars) == 0 { - return nil - } - - result := make(map[string]string, len(vars)) - for _, v := range vars { - parts := strings.SplitN(v, "=", 2) - k := parts[0] - if len(parts) == 2 { - result[k] = parts[1] - } else { - result[k] = os.Getenv(k) - } - } - - return result -} diff --git a/command/helpers_test.go b/command/helpers_test.go index dd2f5e113..dbead5b62 100644 --- a/command/helpers_test.go +++ b/command/helpers_test.go @@ -392,14 +392,3 @@ func TestUiErrorWriter(t *testing.T) { expectedErr += "and thensome more\n" require.Equal(t, expectedErr, errBuf.String()) } - -func TestParseVars(t *testing.T) { - input := []string{"key1=val1", "HOME", "key2=321"} - expected := map[string]string{ - "key1": "val1", - "HOME": os.Getenv("HOME"), - "key2": "321", - } - - require.Equal(t, expected, parseVars(input)) -} From a2dff8ed1e73ca793d60016ce80c39d3d6b4f0e2 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 9 Nov 2020 15:02:21 -0500 Subject: [PATCH 4/9] Parse variable files --- jobspec2/parse.go | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/jobspec2/parse.go b/jobspec2/parse.go index 1f1282cd4..669d56bc4 100644 --- a/jobspec2/parse.go +++ b/jobspec2/parse.go @@ -97,6 +97,11 @@ func decode(c *jobConfig) error { file, diags = hcljson.Parse(pc.Body, pc.Path) } + + parsedVarFiles, mdiags := parseVarFiles(pc.VarFiles) + pc.parsedVarFiles = parsedVarFiles + diags = append(diags, mdiags...) + if diags.HasErrors() { return diags } @@ -118,6 +123,41 @@ func decode(c *jobConfig) error { return nil } +func parseVarFiles(paths []string) ([]*hcl.File, hcl.Diagnostics) { + if len(paths) == 0 { + return nil, nil + } + + files := make([]*hcl.File, 0, len(paths)) + var diags hcl.Diagnostics + + for _, p := range paths { + body, err := ioutil.ReadFile(p) + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Failed to read file", + Detail: fmt.Sprintf("failed to read %q: %v", p, err), + }) + continue + } + + var file *hcl.File + var mdiags hcl.Diagnostics + if !isJSON(body) { + file, mdiags = hclsyntax.ParseConfig(body, p, hcl.Pos{Line: 1, Column: 1}) + } else { + file, mdiags = hcljson.Parse(body, p) + + } + + files = append(files, file) + diags = append(diags, mdiags...) + } + + return files, diags +} + func isJSON(src []byte) bool { for _, c := range src { if c == ' ' { From dda76b2e0372c6c3ac9addfb829bf07d409715cb Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 9 Nov 2020 15:02:58 -0500 Subject: [PATCH 5/9] Start accepting input variable files as CLI arguments --- command/job_plan.go | 12 ++++++++++-- command/job_run.go | 12 ++++++++++-- command/job_validate.go | 17 +++++++++++++---- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/command/job_plan.go b/command/job_plan.go index ff0fe9aeb..1872d3dfd 100644 --- a/command/job_plan.go +++ b/command/job_plan.go @@ -79,6 +79,12 @@ Plan Options: -policy-override Sets the flag to force override any soft mandatory Sentinel policies. + -var 'key=value' + Variable for template, can be used multiple times. + + -var-file=path + Path to HCL2 file containing user variables. + -verbose Increase diff verbosity. ` @@ -97,6 +103,7 @@ func (c *JobPlanCommand) AutocompleteFlags() complete.Flags { "-verbose": complete.PredictNothing, "-hcl1": complete.PredictNothing, "-var": complete.PredictAnything, + "-var-file": complete.PredictFiles("*.var"), }) } @@ -107,7 +114,7 @@ func (c *JobPlanCommand) AutocompleteArgs() complete.Predictor { func (c *JobPlanCommand) Name() string { return "job plan" } func (c *JobPlanCommand) Run(args []string) int { var diff, policyOverride, verbose bool - var varArgs cflags.AppendSliceValue + var varArgs, varFiles cflags.AppendSliceValue flags := c.Meta.FlagSet(c.Name(), FlagSetClient) flags.Usage = func() { c.Ui.Output(c.Help()) } @@ -116,6 +123,7 @@ func (c *JobPlanCommand) Run(args []string) int { flags.BoolVar(&verbose, "verbose", false, "") flags.BoolVar(&c.JobGetter.hcl1, "hcl1", false, "") flags.Var(&varArgs, "var", "") + flags.Var(&varFiles, "var-file", "") if err := flags.Parse(args); err != nil { return 255 @@ -131,7 +139,7 @@ func (c *JobPlanCommand) Run(args []string) int { path := args[0] // Get Job struct from Jobfile - job, err := c.JobGetter.ApiJobWithArgs(args[0], parseVars(varArgs)) + job, err := c.JobGetter.ApiJobWithArgs(args[0], varArgs, varFiles) if err != nil { c.Ui.Error(fmt.Sprintf("Error getting job struct: %s", err)) return 255 diff --git a/command/job_run.go b/command/job_run.go index f97912445..b10d5d04c 100644 --- a/command/job_run.go +++ b/command/job_run.go @@ -109,6 +109,12 @@ Run Options: If set, the passed Vault namespace is stored in the job before sending to the Nomad servers. + -var 'key=value' + Variable for template, can be used multiple times. + + -var-file=path + Path to HCL2 file containing user variables. + -verbose Display full information. ` @@ -133,6 +139,7 @@ func (c *JobRunCommand) AutocompleteFlags() complete.Flags { "-preserve-counts": complete.PredictNothing, "-hcl1": complete.PredictNothing, "-var": complete.PredictAnything, + "-var-file": complete.PredictFiles("*.var"), }) } @@ -145,7 +152,7 @@ func (c *JobRunCommand) Name() string { return "job run" } func (c *JobRunCommand) Run(args []string) int { var detach, verbose, output, override, preserveCounts bool var checkIndexStr, consulToken, vaultToken, vaultNamespace string - var varArgs cflags.AppendSliceValue + var varArgs, varFiles cflags.AppendSliceValue flags := c.Meta.FlagSet(c.Name(), FlagSetClient) flags.Usage = func() { c.Ui.Output(c.Help()) } @@ -160,6 +167,7 @@ func (c *JobRunCommand) Run(args []string) int { flags.StringVar(&vaultToken, "vault-token", "", "") flags.StringVar(&vaultNamespace, "vault-namespace", "", "") flags.Var(&varArgs, "var", "") + flags.Var(&varFiles, "var-file", "") if err := flags.Parse(args); err != nil { return 1 @@ -180,7 +188,7 @@ func (c *JobRunCommand) Run(args []string) int { } // Get Job struct from Jobfile - job, err := c.JobGetter.ApiJobWithArgs(args[0], parseVars(varArgs)) + job, err := c.JobGetter.ApiJobWithArgs(args[0], varArgs, varFiles) if err != nil { c.Ui.Error(fmt.Sprintf("Error getting job struct: %s", err)) return 1 diff --git a/command/job_validate.go b/command/job_validate.go index a52c24cc5..658549967 100644 --- a/command/job_validate.go +++ b/command/job_validate.go @@ -33,6 +33,13 @@ Validate Options: -hcl1 Parses the job file as HCLv1. + + -var 'key=value' + Variable for template, can be used multiple times. + + -var-file=path + Path to HCL2 file containing user variables. + ` return strings.TrimSpace(helpText) } @@ -43,8 +50,9 @@ func (c *JobValidateCommand) Synopsis() string { func (c *JobValidateCommand) AutocompleteFlags() complete.Flags { return complete.Flags{ - "-hcl1": complete.PredictNothing, - "-var": complete.PredictAnything, + "-hcl1": complete.PredictNothing, + "-var": complete.PredictAnything, + "-var-file": complete.PredictFiles("*.var"), } } @@ -55,12 +63,13 @@ func (c *JobValidateCommand) AutocompleteArgs() complete.Predictor { func (c *JobValidateCommand) Name() string { return "job validate" } func (c *JobValidateCommand) Run(args []string) int { - var varArgs cflags.AppendSliceValue + var varArgs, varFiles cflags.AppendSliceValue flags := c.Meta.FlagSet(c.Name(), FlagSetNone) flags.Usage = func() { c.Ui.Output(c.Help()) } flags.BoolVar(&c.JobGetter.hcl1, "hcl1", false, "") flags.Var(&varArgs, "var", "") + flags.Var(&varFiles, "var-file", "") if err := flags.Parse(args); err != nil { return 1 @@ -75,7 +84,7 @@ func (c *JobValidateCommand) Run(args []string) int { } // Get Job struct from Jobfile - job, err := c.JobGetter.ApiJobWithArgs(args[0], parseVars(varArgs)) + job, err := c.JobGetter.ApiJobWithArgs(args[0], varArgs, varFiles) if err != nil { c.Ui.Error(fmt.Sprintf("Error getting job struct: %s", err)) return 1 From 62ee73316e62361c80ed6bc80d1d9d982e0c12e9 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 9 Nov 2020 15:03:17 -0500 Subject: [PATCH 6/9] Update variable interpolation tests --- jobspec2/parse_test.go | 184 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 174 insertions(+), 10 deletions(-) diff --git a/jobspec2/parse_test.go b/jobspec2/parse_test.go index d774aa616..71d26d8ef 100644 --- a/jobspec2/parse_test.go +++ b/jobspec2/parse_test.go @@ -3,7 +3,6 @@ package jobspec2 import ( "io/ioutil" "os" - "strings" "testing" "github.com/hashicorp/nomad/jobspec" @@ -57,13 +56,21 @@ func TestEquivalentToHCL1_ComplexConfig(t *testing.T) { func TestParse_VarsAndFunctions(t *testing.T) { hcl := ` +variables { + region_var = "default" +} job "example" { datacenters = [for s in ["dc1", "dc2"] : upper(s)] - region = vars.region_var + region = var.region_var } ` - out, err := ParseWithArgs("input.hcl", strings.NewReader(hcl), map[string]string{"region_var": "aug"}, true) + out, err := ParseWithConfig(&ParseConfig{ + Path: "input.hcl", + Body: []byte(hcl), + ArgVars: []string{"region_var=aug"}, + AllowFS: true, + }) require.NoError(t, err) require.Equal(t, []string{"DC1", "DC2"}, out.Datacenters) @@ -71,20 +78,112 @@ job "example" { require.Equal(t, "aug", *out.Region) } +func TestParse_VariablesDefaultsAndSet(t *testing.T) { + hcl := ` +variables { + region_var = "default_region" +} + +variable "dc_var" { + default = "default_dc" +} + +job "example" { + datacenters = [var.dc_var] + region = var.region_var +} +` + + t.Run("defaults", func(t *testing.T) { + out, err := ParseWithConfig(&ParseConfig{ + Path: "input.hcl", + Body: []byte(hcl), + AllowFS: true, + }) + require.NoError(t, err) + + require.Equal(t, []string{"default_dc"}, out.Datacenters) + require.NotNil(t, out.Region) + require.Equal(t, "default_region", *out.Region) + }) + + t.Run("set via -var args", func(t *testing.T) { + out, err := ParseWithConfig(&ParseConfig{ + Path: "input.hcl", + Body: []byte(hcl), + ArgVars: []string{"dc_var=set_dc", "region_var=set_region"}, + AllowFS: true, + }) + require.NoError(t, err) + + require.Equal(t, []string{"set_dc"}, out.Datacenters) + require.NotNil(t, out.Region) + require.Equal(t, "set_region", *out.Region) + }) + + t.Run("set via envvars", func(t *testing.T) { + out, err := ParseWithConfig(&ParseConfig{ + Path: "input.hcl", + Body: []byte(hcl), + Envs: []string{ + "NOMAD_VAR_dc_var=set_dc", + "NOMAD_VAR_region_var=set_region", + }, + AllowFS: true, + }) + require.NoError(t, err) + + require.Equal(t, []string{"set_dc"}, out.Datacenters) + require.NotNil(t, out.Region) + require.Equal(t, "set_region", *out.Region) + }) + + t.Run("set via var-files", func(t *testing.T) { + varFile, err := ioutil.TempFile("", "") + require.NoError(t, err) + defer os.Remove(varFile.Name()) + + content := `dc_var = "set_dc" +region_var = "set_region"` + _, err = varFile.WriteString(content) + require.NoError(t, err) + + out, err := ParseWithConfig(&ParseConfig{ + Path: "input.hcl", + Body: []byte(hcl), + VarFiles: []string{varFile.Name()}, + AllowFS: true, + }) + require.NoError(t, err) + + require.Equal(t, []string{"set_dc"}, out.Datacenters) + require.NotNil(t, out.Region) + require.Equal(t, "set_region", *out.Region) + }) +} + // TestParse_UnknownVariables asserts that unknown variables are left intact for further processing func TestParse_UnknownVariables(t *testing.T) { hcl := ` +variables { + region_var = "default" +} job "example" { datacenters = [for s in ["dc1", "dc2"] : upper(s)] - region = vars.region_var + region = var.region_var meta { - known_var = "${vars.region_var}" + known_var = "${var.region_var}" unknown_var = "${UNKNOWN}" } } ` - out, err := ParseWithArgs("input.hcl", strings.NewReader(hcl), map[string]string{"region_var": "aug"}, true) + out, err := ParseWithConfig(&ParseConfig{ + Path: "input.hcl", + Body: []byte(hcl), + ArgVars: []string{"region_var=aug"}, + AllowFS: true, + }) require.NoError(t, err) meta := map[string]string{ @@ -94,6 +193,52 @@ job "example" { require.Equal(t, meta, out.Meta) } +func TestParse_Locals(t *testing.T) { + hcl := ` +variables { + region_var = "default_region" +} + +locals { + # literal local + dc = "local_dc" + # local that depends on a variable + region = "${var.region_var}.local" +} + +job "example" { + datacenters = [local.dc] + region = local.region +} +` + + t.Run("defaults", func(t *testing.T) { + out, err := ParseWithConfig(&ParseConfig{ + Path: "input.hcl", + Body: []byte(hcl), + AllowFS: true, + }) + require.NoError(t, err) + + require.Equal(t, []string{"local_dc"}, out.Datacenters) + require.NotNil(t, out.Region) + require.Equal(t, "default_region.local", *out.Region) + }) + + t.Run("set via -var argments", func(t *testing.T) { + out, err := ParseWithConfig(&ParseConfig{ + Path: "input.hcl", + Body: []byte(hcl), + ArgVars: []string{"region_var=set_region"}, + AllowFS: true, + }) + require.NoError(t, err) + + require.Equal(t, []string{"local_dc"}, out.Datacenters) + require.NotNil(t, out.Region) + require.Equal(t, "set_region.local", *out.Region) + }) +} func TestParse_FileOperators(t *testing.T) { hcl := ` @@ -103,7 +248,12 @@ job "example" { ` t.Run("enabled", func(t *testing.T) { - out, err := ParseWithArgs("input.hcl", strings.NewReader(hcl), nil, true) + out, err := ParseWithConfig(&ParseConfig{ + Path: "input.hcl", + Body: []byte(hcl), + ArgVars: nil, + AllowFS: true, + }) require.NoError(t, err) expected, err := ioutil.ReadFile("parse_test.go") @@ -114,7 +264,12 @@ job "example" { }) t.Run("disabled", func(t *testing.T) { - _, err := ParseWithArgs("input.hcl", strings.NewReader(hcl), nil, false) + _, err := ParseWithConfig(&ParseConfig{ + Path: "input.hcl", + Body: []byte(hcl), + ArgVars: nil, + AllowFS: false, + }) require.Error(t, err) require.Contains(t, err.Error(), "filesystem function disabled") }) @@ -137,7 +292,12 @@ dynamic "group" { } } ` - out, err := ParseWithArgs("input.hcl", strings.NewReader(hcl), nil, true) + out, err := ParseWithConfig(&ParseConfig{ + Path: "input.hcl", + Body: []byte(hcl), + ArgVars: nil, + AllowFS: false, + }) require.NoError(t, err) require.Len(t, out.TaskGroups, 3) @@ -295,7 +455,11 @@ job "example" { for _, c := range cases { t.Run(c.name, func(t *testing.T) { - _, err := ParseWithArgs(c.name+".hcl", strings.NewReader(c.hcl), nil, true) + _, err := ParseWithConfig(&ParseConfig{ + Path: c.name + ".hcl", + Body: []byte(c.hcl), + AllowFS: false, + }) if c.expectedErr == "" { require.NoError(t, err) } else { From 147c8e12c2858f5aad1b8a0e6dc7baed1654562b Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 9 Nov 2020 16:23:09 -0500 Subject: [PATCH 7/9] clarify variable references --- jobspec2/parse_job.go | 18 +++++++++--------- jobspec2/types.config.go | 6 +++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/jobspec2/parse_job.go b/jobspec2/parse_job.go index cf834ce49..9b533874f 100644 --- a/jobspec2/parse_job.go +++ b/jobspec2/parse_job.go @@ -6,13 +6,13 @@ import ( "github.com/hashicorp/nomad/api" ) -func normalizeJob(jw *jobConfig) { - j := jw.Job +func normalizeJob(jc *jobConfig) { + j := jc.Job if j.Name == nil { - j.Name = &jw.JobID + j.Name = &jc.JobID } if j.ID == nil { - j.ID = &jw.JobID + j.ID = &jc.JobID } if j.Periodic != nil && j.Periodic.Spec != nil { @@ -20,11 +20,11 @@ func normalizeJob(jw *jobConfig) { j.Periodic.SpecType = &v } - normalizeVault(jw.Vault) + normalizeVault(jc.Vault) - if len(jw.Tasks) != 0 { - alone := make([]*api.TaskGroup, 0, len(jw.Tasks)) - for _, t := range jw.Tasks { + if len(jc.Tasks) != 0 { + alone := make([]*api.TaskGroup, 0, len(jc.Tasks)) + for _, t := range jc.Tasks { alone = append(alone, &api.TaskGroup{ Name: &t.Name, Tasks: []*api.Task{t}, @@ -47,7 +47,7 @@ func normalizeJob(jw *jobConfig) { normalizeVault(t.Vault) if t.Vault == nil { - t.Vault = jw.Vault + t.Vault = jc.Vault } } } diff --git a/jobspec2/types.config.go b/jobspec2/types.config.go index 8d7a7fc40..9191d20be 100644 --- a/jobspec2/types.config.go +++ b/jobspec2/types.config.go @@ -69,19 +69,19 @@ func (c *jobConfig) decodeBody(body hcl.Body) hcl.Diagnostics { // decodeInputVariables looks in the found blocks for 'variables' and // 'variable' blocks. It should be called firsthand so that other blocks can // use the variables. -func (j *jobConfig) decodeInputVariables(content *hcl.BodyContent) hcl.Diagnostics { +func (c *jobConfig) decodeInputVariables(content *hcl.BodyContent) hcl.Diagnostics { var diags hcl.Diagnostics for _, block := range content.Blocks { switch block.Type { case variableLabel: - moreDiags := j.InputVariables.decodeVariableBlock(block, nil) + moreDiags := c.InputVariables.decodeVariableBlock(block, nil) diags = append(diags, moreDiags...) case variablesLabel: attrs, moreDiags := block.Body.JustAttributes() diags = append(diags, moreDiags...) for key, attr := range attrs { - moreDiags = j.InputVariables.decodeVariable(key, attr, nil) + moreDiags = c.InputVariables.decodeVariable(key, attr, nil) diags = append(diags, moreDiags...) } } From 21be89025652edace696f50073c8a4dbb9a1674e Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 9 Nov 2020 16:24:22 -0500 Subject: [PATCH 8/9] clarify test --- jobspec2/parse_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/jobspec2/parse_test.go b/jobspec2/parse_test.go index 71d26d8ef..d3d331450 100644 --- a/jobspec2/parse_test.go +++ b/jobspec2/parse_test.go @@ -203,7 +203,7 @@ locals { # literal local dc = "local_dc" # local that depends on a variable - region = "${var.region_var}.local" + region = "${var.region_var}.example" } job "example" { @@ -222,7 +222,7 @@ job "example" { require.Equal(t, []string{"local_dc"}, out.Datacenters) require.NotNil(t, out.Region) - require.Equal(t, "default_region.local", *out.Region) + require.Equal(t, "default_region.example", *out.Region) }) t.Run("set via -var argments", func(t *testing.T) { @@ -236,7 +236,7 @@ job "example" { require.Equal(t, []string{"local_dc"}, out.Datacenters) require.NotNil(t, out.Region) - require.Equal(t, "set_region.local", *out.Region) + require.Equal(t, "set_region.example", *out.Region) }) } From c2a6a6da8063919dcfcc8eac1e82549791604b22 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 9 Nov 2020 16:27:06 -0500 Subject: [PATCH 9/9] use a constructor to initialize job config --- jobspec2/parse.go | 4 +--- jobspec2/types.config.go | 9 +++++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/jobspec2/parse.go b/jobspec2/parse.go index 669d56bc4..c19ca3aba 100644 --- a/jobspec2/parse.go +++ b/jobspec2/parse.go @@ -42,9 +42,7 @@ func Parse(path string, r io.Reader) (*api.Job, error) { func ParseWithConfig(args *ParseConfig) (*api.Job, error) { args.normalize() - c := &jobConfig{ - ParseConfig: args, - } + c := newJobConfig(args) err := decode(c) if err != nil { return nil, err diff --git a/jobspec2/types.config.go b/jobspec2/types.config.go index 9191d20be..a8fa5fd28 100644 --- a/jobspec2/types.config.go +++ b/jobspec2/types.config.go @@ -35,6 +35,15 @@ type jobConfig struct { LocalBlocks []*LocalBlock } +func newJobConfig(parseConfig *ParseConfig) *jobConfig { + return &jobConfig{ + ParseConfig: parseConfig, + + InputVariables: Variables{}, + LocalVariables: Variables{}, + } +} + var jobConfigSchema = &hcl.BodySchema{ Blocks: []hcl.BlockHeaderSchema{ {Type: variablesLabel},