diff --git a/command/run_test.go b/command/run_test.go index 408feec9f..efd92817e 100644 --- a/command/run_test.go +++ b/command/run_test.go @@ -86,7 +86,7 @@ job "job1" { resources = { cpu = 1000 disk = 150 - mem = 512 + memory = 512 } } } diff --git a/command/validate_test.go b/command/validate_test.go index 649251eb3..069e0af65 100644 --- a/command/validate_test.go +++ b/command/validate_test.go @@ -33,7 +33,7 @@ job "job1" { resources = { cpu = 1000 disk = 150 - mem = 512 + memory = 512 } } } diff --git a/jobspec/parse.go b/jobspec/parse.go index ffef6bfef..58ee18924 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -10,13 +10,14 @@ import ( "strconv" "strings" + "github.com/hashicorp/go-multierror" "github.com/hashicorp/hcl" "github.com/hashicorp/hcl/hcl/ast" "github.com/hashicorp/nomad/nomad/structs" "github.com/mitchellh/mapstructure" ) -var reDynamicPorts *regexp.Regexp = regexp.MustCompile("^[a-zA-Z0-9_]+$") +var reDynamicPorts = regexp.MustCompile("^[a-zA-Z0-9_]+$") var errPortLabel = fmt.Errorf("Port label does not conform to naming requirements %s", reDynamicPorts.String()) // Parse parses the job spec from the given io.Reader. @@ -43,6 +44,14 @@ func Parse(r io.Reader) (*structs.Job, error) { return nil, fmt.Errorf("error parsing: root should be an object") } + // Check for invalid keys + valid := []string{ + "job", + } + if err := checkHCLKeys(list, valid); err != nil { + return nil, err + } + var job structs.Job // Parse the job out @@ -114,24 +123,44 @@ func parseJob(result *structs.Job, list *ast.ObjectList) error { return fmt.Errorf("job '%s' value: should be an object", result.ID) } + // Check for invalid keys + valid := []string{ + "id", + "name", + "region", + "all_at_once", + "type", + "priority", + "datacenters", + "constraint", + "update", + "periodic", + "meta", + "task", + "group", + } + if err := checkHCLKeys(listVal, valid); err != nil { + return multierror.Prefix(err, "job:") + } + // Parse constraints if o := listVal.Filter("constraint"); len(o.Items) > 0 { if err := parseConstraints(&result.Constraints, o); err != nil { - return err + return multierror.Prefix(err, "constraint ->") } } // If we have an update strategy, then parse that if o := listVal.Filter("update"); len(o.Items) > 0 { if err := parseUpdate(&result.Update, o); err != nil { - return err + return multierror.Prefix(err, "update ->") } } // If we have a periodic definition, then parse that if o := listVal.Filter("periodic"); len(o.Items) > 0 { if err := parsePeriodic(&result.Periodic, o); err != nil { - return err + return multierror.Prefix(err, "periodic ->") } } @@ -153,7 +182,7 @@ func parseJob(result *structs.Job, list *ast.ObjectList) error { if o := listVal.Filter("task"); len(o.Items) > 0 { var tasks []*structs.Task if err := parseTasks(result.Name, "", &tasks, o); err != nil { - return err + return multierror.Prefix(err, "task:") } result.TaskGroups = make([]*structs.TaskGroup, len(tasks), len(tasks)*2) @@ -169,7 +198,7 @@ func parseJob(result *structs.Job, list *ast.ObjectList) error { // Parse the task groups if o := listVal.Filter("group"); len(o.Items) > 0 { if err := parseGroups(result, o); err != nil { - return fmt.Errorf("error parsing 'group': %s", err) + return multierror.Prefix(err, "group:") } } @@ -202,6 +231,18 @@ func parseGroups(result *structs.Job, list *ast.ObjectList) error { return fmt.Errorf("group '%s': should be an object", n) } + // Check for invalid keys + valid := []string{ + "count", + "constraint", + "restart", + "meta", + "task", + } + if err := checkHCLKeys(listVal, valid); err != nil { + return multierror.Prefix(err, fmt.Sprintf("'%s' ->", n)) + } + var m map[string]interface{} if err := hcl.DecodeObject(&m, item.Val); err != nil { return err @@ -226,14 +267,14 @@ func parseGroups(result *structs.Job, list *ast.ObjectList) error { // Parse constraints if o := listVal.Filter("constraint"); len(o.Items) > 0 { if err := parseConstraints(&g.Constraints, o); err != nil { - return err + return multierror.Prefix(err, fmt.Sprintf("'%s', constraint ->", n)) } } // Parse restart policy if o := listVal.Filter("restart"); len(o.Items) > 0 { if err := parseRestartPolicy(&g.RestartPolicy, o); err != nil { - return err + return multierror.Prefix(err, fmt.Sprintf("'%s', restart ->", n)) } } @@ -254,7 +295,7 @@ func parseGroups(result *structs.Job, list *ast.ObjectList) error { // Parse tasks if o := listVal.Filter("task"); len(o.Items) > 0 { if err := parseTasks(result.Name, g.Name, &g.Tasks, o); err != nil { - return err + return multierror.Prefix(err, fmt.Sprintf("'%s', task:", n)) } } @@ -274,6 +315,17 @@ func parseRestartPolicy(final **structs.RestartPolicy, list *ast.ObjectList) err // Get our job object obj := list.Items[0] + // Check for invalid keys + valid := []string{ + "attempts", + "interval", + "delay", + "mode", + } + if err := checkHCLKeys(obj.Val, valid); err != nil { + return err + } + var m map[string]interface{} if err := hcl.DecodeObject(&m, obj.Val); err != nil { return err @@ -298,10 +350,24 @@ func parseRestartPolicy(final **structs.RestartPolicy, list *ast.ObjectList) err func parseConstraints(result *[]*structs.Constraint, list *ast.ObjectList) error { for _, o := range list.Elem().Items { + // Check for invalid keys + valid := []string{ + "attribute", + "operator", + "value", + "version", + "regexp", + "distinct_hosts", + } + if err := checkHCLKeys(o.Val, valid); err != nil { + return err + } + var m map[string]interface{} if err := hcl.DecodeObject(&m, o.Val); err != nil { return err } + m["LTarget"] = m["attribute"] m["RTarget"] = m["value"] m["Operand"] = m["operator"] @@ -391,6 +457,22 @@ func parseTasks(jobName string, taskGroupName string, result *[]*structs.Task, l return fmt.Errorf("group '%s': should be an object", n) } + // Check for invalid keys + valid := []string{ + "driver", + "env", + "service", + "config", + "constraint", + "meta", + "resources", + "logs", + "kill_timeout", + } + if err := checkHCLKeys(listVal, valid); err != nil { + return multierror.Prefix(err, fmt.Sprintf("'%s' ->", n)) + } + var m map[string]interface{} if err := hcl.DecodeObject(&m, item.Val); err != nil { return err @@ -436,7 +518,7 @@ func parseTasks(jobName string, taskGroupName string, result *[]*structs.Task, l if o := listVal.Filter("service"); len(o.Items) > 0 { if err := parseServices(jobName, taskGroupName, &t, o); err != nil { - return err + return multierror.Prefix(err, fmt.Sprintf("'%s',", n)) } } @@ -457,7 +539,8 @@ func parseTasks(jobName string, taskGroupName string, result *[]*structs.Task, l // Parse constraints if o := listVal.Filter("constraint"); len(o.Items) > 0 { if err := parseConstraints(&t.Constraints, o); err != nil { - return err + return multierror.Prefix(err, fmt.Sprintf( + "'%s', constraint ->", n)) } } @@ -479,7 +562,7 @@ func parseTasks(jobName string, taskGroupName string, result *[]*structs.Task, l if o := listVal.Filter("resources"); len(o.Items) > 0 { var r structs.Resources if err := parseResources(&r, o); err != nil { - return fmt.Errorf("task '%s': %s", t.Name, err) + return multierror.Prefix(err, fmt.Sprintf("'%s',", n)) } t.Resources = &r @@ -493,6 +576,16 @@ func parseTasks(jobName string, taskGroupName string, result *[]*structs.Task, l } var m map[string]interface{} logsBlock := o.Items[0] + + // Check for invalid keys + valid := []string{ + "max_files", + "max_file_size", + } + if err := checkHCLKeys(logsBlock.Val, valid); err != nil { + return multierror.Prefix(err, fmt.Sprintf("'%s', logs ->", n)) + } + if err := hcl.DecodeObject(&m, logsBlock.Val); err != nil { return err } @@ -513,6 +606,17 @@ func parseServices(jobName string, taskGroupName string, task *structs.Task, ser task.Services = make([]*structs.Service, len(serviceObjs.Items)) var defaultServiceName bool for idx, o := range serviceObjs.Items { + // Check for invalid keys + valid := []string{ + "name", + "tags", + "port", + "check", + } + if err := checkHCLKeys(o.Val, valid); err != nil { + return multierror.Prefix(err, fmt.Sprintf("service (%d) ->", idx)) + } + var service structs.Service var m map[string]interface{} if err := hcl.DecodeObject(&m, o.Val); err != nil { @@ -534,7 +638,7 @@ func parseServices(jobName string, taskGroupName string, task *structs.Task, ser service.Name = fmt.Sprintf("%s-%s-%s", jobName, taskGroupName, task.Name) } - // Fileter checks + // Filter checks var checkList *ast.ObjectList if ot, ok := o.Val.(*ast.ObjectType); ok { checkList = ot.List @@ -544,7 +648,7 @@ func parseServices(jobName string, taskGroupName string, task *structs.Task, ser if co := checkList.Filter("check"); len(co.Items) > 0 { if err := parseChecks(&service, co); err != nil { - return err + return multierror.Prefix(err, fmt.Sprintf("service: '%s',", service.Name)) } } @@ -557,6 +661,19 @@ func parseServices(jobName string, taskGroupName string, task *structs.Task, ser func parseChecks(service *structs.Service, checkObjs *ast.ObjectList) error { service.Checks = make([]*structs.ServiceCheck, len(checkObjs.Items)) for idx, co := range checkObjs.Items { + // Check for invalid keys + valid := []string{ + "name", + "type", + "interval", + "timeout", + "path", + "protocol", + } + if err := checkHCLKeys(co.Val, valid); err != nil { + return multierror.Prefix(err, "check ->") + } + var check structs.ServiceCheck var cm map[string]interface{} if err := hcl.DecodeObject(&cm, co.Val); err != nil { @@ -600,6 +717,18 @@ func parseResources(result *structs.Resources, list *ast.ObjectList) error { return fmt.Errorf("resource: should be an object") } + // Check for invalid keys + valid := []string{ + "cpu", + "disk", + "iops", + "memory", + "network", + } + if err := checkHCLKeys(listVal, valid); err != nil { + return multierror.Prefix(err, "resources ->") + } + var m map[string]interface{} if err := hcl.DecodeObject(&m, o.Val); err != nil { return err @@ -616,6 +745,15 @@ func parseResources(result *structs.Resources, list *ast.ObjectList) error { return fmt.Errorf("only one 'network' resource allowed") } + // Check for invalid keys + valid := []string{ + "mbits", + "port", + } + if err := checkHCLKeys(o.Items[0].Val, valid); err != nil { + return multierror.Prefix(err, "resources, network ->") + } + var r structs.NetworkResource var m map[string]interface{} if err := hcl.DecodeObject(&m, o.Items[0].Val); err != nil { @@ -632,7 +770,7 @@ func parseResources(result *structs.Resources, list *ast.ObjectList) error { return fmt.Errorf("resource: should be an object") } if err := parsePorts(networkObj, &r); err != nil { - return err + return multierror.Prefix(err, "resources, network, ports ->") } result.Networks = []*structs.NetworkResource{&r} @@ -646,6 +784,15 @@ func parseResources(result *structs.Resources, list *ast.ObjectList) error { } func parsePorts(networkObj *ast.ObjectList, nw *structs.NetworkResource) error { + // Check for invalid keys + valid := []string{ + "mbits", + "port", + } + if err := checkHCLKeys(networkObj, valid); err != nil { + return err + } + portsObjList := networkObj.Filter("port") knownPortLabels := make(map[string]bool) for _, port := range portsObjList.Items { @@ -693,6 +840,15 @@ func parseUpdate(result *structs.UpdateStrategy, list *ast.ObjectList) error { return err } + // Check for invalid keys + valid := []string{ + "stagger", + "max_parallel", + } + if err := checkHCLKeys(o.Val, valid); err != nil { + return err + } + dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ DecodeHook: mapstructure.StringToTimeDurationHookFunc(), WeaklyTypedInput: true, @@ -718,6 +874,16 @@ func parsePeriodic(result **structs.PeriodicConfig, list *ast.ObjectList) error return err } + // Check for invalid keys + valid := []string{ + "enabled", + "cron", + "prohibit_overlap", + } + if err := checkHCLKeys(o.Val, valid); err != nil { + return err + } + // Enabled by default if the periodic block exists. if value, ok := m["enabled"]; !ok { m["Enabled"] = true @@ -743,3 +909,31 @@ func parsePeriodic(result **structs.PeriodicConfig, list *ast.ObjectList) error *result = &p return nil } + +func checkHCLKeys(node ast.Node, valid []string) error { + var list *ast.ObjectList + switch n := node.(type) { + case *ast.ObjectList: + list = n + case *ast.ObjectType: + list = n.List + default: + return fmt.Errorf("cannot check HCL keys of type %T", n) + } + + validMap := make(map[string]struct{}, len(valid)) + for _, v := range valid { + validMap[v] = struct{}{} + } + + var result error + for _, item := range list.Items { + key := item.Keys[0].Token.Value().(string) + if _, ok := validMap[key]; !ok { + result = multierror.Append(result, fmt.Errorf( + "invalid key: %s", key)) + } + } + + return result +} diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 3ffb24fac..f6a7351b9 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -370,3 +370,20 @@ func TestIncompleteServiceDefn(t *testing.T) { t.Fatalf("Expected collision error; got %v", err) } } + +func TestIncorrectKey(t *testing.T) { + path, err := filepath.Abs(filepath.Join("./test-fixtures", "basic_wrong_key.hcl")) + if err != nil { + t.Fatalf("Can't get absolute path for file: %s", err) + } + + _, err = ParseFile(path) + + if err == nil { + t.Fatalf("Expected an error") + } + + if !strings.Contains(err.Error(), "* group: 'binsl', task: 'binstore', service: 'binstore-storagelocker-binsl-binstore', check -> invalid key: nterval") { + t.Fatalf("Expected collision error; got %v", err) + } +} diff --git a/jobspec/test-fixtures/basic.hcl b/jobspec/test-fixtures/basic.hcl index 84413e3c1..efd4fc4dc 100644 --- a/jobspec/test-fixtures/basic.hcl +++ b/jobspec/test-fixtures/basic.hcl @@ -92,7 +92,7 @@ job "binstore-storagelocker" { resources { cpu = 500 memory = 128 - IOPS = 30 + iops = 30 } constraint { attribute = "kernel.arch" diff --git a/jobspec/test-fixtures/basic_wrong_key.hcl b/jobspec/test-fixtures/basic_wrong_key.hcl new file mode 100644 index 000000000..cf357aec4 --- /dev/null +++ b/jobspec/test-fixtures/basic_wrong_key.hcl @@ -0,0 +1,114 @@ +job "binstore-storagelocker" { + region = "global" + type = "service" + priority = 50 + all_at_once = true + datacenters = ["us2", "eu1"] + + meta { + foo = "bar" + } + + constraint { + attribute = "kernel.os" + value = "windows" + } + + update { + stagger = "60s" + max_parallel = 2 + } + + task "outside" { + driver = "java" + config { + jar = "s3://my-cool-store/foo.jar" + } + meta { + my-cool-key = "foobar" + } + } + + group "binsl" { + count = 5 + restart { + attempts = 5 + interval = "10m" + delay = "15s" + mode = "delay" + } + task "binstore" { + driver = "docker" + config { + image = "hashicorp/binstore" + } + logs { + max_files = 10 + max_file_size = 100 + } + env { + HELLO = "world" + LOREM = "ipsum" + } + service { + tags = ["foo", "bar"] + port = "http" + check { + name = "check-name" + type = "tcp" + nterval = "10s" + timeout = "2s" + } + } + resources { + cpu = 500 + memory = 128 + + network { + mbits = "100" + port "one" { + static = 1 + } + port "two" { + static = 2 + } + port "three" { + static = 3 + } + port "http" {} + port "https" {} + port "admin" {} + } + } + + kill_timeout = "22s" + } + + task "storagelocker" { + driver = "java" + config { + image = "hashicorp/storagelocker" + } + resources { + cpu = 500 + memory = 128 + iops = 30 + } + constraint { + attribute = "kernel.arch" + value = "amd64" + } + } + + constraint { + attribute = "kernel.os" + value = "linux" + } + + meta { + elb_mode = "tcp" + elb_interval = 10 + elb_checks = 3 + } + } +}