From 729931aced36defd83495e4ea1d130d79218f33c Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Thu, 9 Apr 2020 21:01:16 -0600 Subject: [PATCH] connect: enable configuring sidecar_task.name Before, the submitted jobspec for sidecar_task would pass through 2 key validation steps - once for the subset specific to connect sidecar task definitions, and once again for the set of normal task definition where the task would actually get unmarshalled. The valid keys for the normal task definition did not include "name", which is supposed to be configurable for the sidecar task. To fix this, just eliminate the double validation step, and instead pass-in the correct set of keys to validate against to the one generic task parser. Fixes #7680 --- jobspec/parse_service.go | 27 +---- jobspec/parse_task.go | 101 ++++++++++-------- jobspec/parse_test.go | 22 ++++ .../service-connect-sidecar_task-name.hcl | 15 +++ 4 files changed, 97 insertions(+), 68 deletions(-) create mode 100644 jobspec/test-fixtures/service-connect-sidecar_task-name.hcl diff --git a/jobspec/parse_service.go b/jobspec/parse_service.go index 00963b327..24c35176d 100644 --- a/jobspec/parse_service.go +++ b/jobspec/parse_service.go @@ -263,32 +263,7 @@ func parseSidecarService(o *ast.ObjectItem) (*api.ConsulSidecarService, error) { } func parseSidecarTask(item *ast.ObjectItem) (*api.SidecarTask, error) { - // We need this later - var listVal *ast.ObjectList - if ot, ok := item.Val.(*ast.ObjectType); ok { - listVal = ot.List - } else { - return nil, fmt.Errorf("should be an object") - } - - // Check for invalid keys - valid := []string{ - "config", - "driver", - "env", - "kill_timeout", - "logs", - "meta", - "resources", - "shutdown_delay", - "user", - "kill_signal", - } - if err := helper.CheckHCLKeys(listVal, valid); err != nil { - return nil, err - } - - task, err := parseTask(item) + task, err := parseTask(item, sidecarTaskKeys) if err != nil { return nil, err } diff --git a/jobspec/parse_task.go b/jobspec/parse_task.go index 230ef32a8..1e6ace7ca 100644 --- a/jobspec/parse_task.go +++ b/jobspec/parse_task.go @@ -12,47 +12,8 @@ import ( "github.com/mitchellh/mapstructure" ) -func parseTasks(result *[]*api.Task, list *ast.ObjectList) error { - list = list.Children() - if len(list.Items) == 0 { - return nil - } - - // Go through each object and turn it into an actual result. - seen := make(map[string]struct{}) - for _, item := range list.Items { - n := item.Keys[0].Token.Value().(string) - - // Make sure we haven't already found this - if _, ok := seen[n]; ok { - return fmt.Errorf("task '%s' defined more than once", n) - } - seen[n] = struct{}{} - - t, err := parseTask(item) - if err != nil { - return multierror.Prefix(err, fmt.Sprintf("'%s',", n)) - } - - t.Name = n - - *result = append(*result, t) - } - - return nil -} - -func parseTask(item *ast.ObjectItem) (*api.Task, error) { - // We need this later - var listVal *ast.ObjectList - if ot, ok := item.Val.(*ast.ObjectType); ok { - listVal = ot.List - } else { - return nil, fmt.Errorf("should be an object") - } - - // Check for invalid keys - valid := []string{ +var ( + normalTaskKeys = []string{ "artifact", "config", "constraint", @@ -77,7 +38,63 @@ func parseTask(item *ast.ObjectItem) (*api.Task, error) { "volume_mount", "csi_plugin", } - if err := helper.CheckHCLKeys(listVal, valid); err != nil { + + sidecarTaskKeys = []string{ + "name", + "driver", + "user", + "config", + "env", + "resources", + "meta", + "logs", + "kill_timeout", + "shutdown_delay", + "kill_signal", + } +) + +func parseTasks(result *[]*api.Task, list *ast.ObjectList) error { + list = list.Children() + if len(list.Items) == 0 { + return nil + } + + // Go through each object and turn it into an actual result. + seen := make(map[string]struct{}) + for _, item := range list.Items { + n := item.Keys[0].Token.Value().(string) + + // Make sure we haven't already found this + if _, ok := seen[n]; ok { + return fmt.Errorf("task '%s' defined more than once", n) + } + seen[n] = struct{}{} + + t, err := parseTask(item, normalTaskKeys) + if err != nil { + return multierror.Prefix(err, fmt.Sprintf("'%s',", n)) + } + + t.Name = n + + *result = append(*result, t) + } + + return nil +} + +func parseTask(item *ast.ObjectItem, keys []string) (*api.Task, error) { + // We need this later + var listVal *ast.ObjectList + if ot, ok := item.Val.(*ast.ObjectType); ok { + listVal = ot.List + } else { + return nil, fmt.Errorf("should be an object") + } + + // Check for invalid keys + if err := helper.CheckHCLKeys(listVal, keys); err != nil { return nil, err } diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index c445a6818..c171f9acf 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -894,6 +894,28 @@ func TestParse(t *testing.T) { }, false, }, + { + "service-connect-sidecar_task-name.hcl", + &api.Job{ + ID: helper.StringToPtr("sidecar_task_name"), + Name: helper.StringToPtr("sidecar_task_name"), + Type: helper.StringToPtr("service"), + TaskGroups: []*api.TaskGroup{{ + Name: helper.StringToPtr("group"), + Services: []*api.Service{{ + Name: "example", + Connect: &api.ConsulConnect{ + Native: false, + SidecarService: &api.ConsulSidecarService{}, + SidecarTask: &api.SidecarTask{ + Name: "my-sidecar", + }, + }, + }}, + }}, + }, + false, + }, { "reschedule-job.hcl", &api.Job{ diff --git a/jobspec/test-fixtures/service-connect-sidecar_task-name.hcl b/jobspec/test-fixtures/service-connect-sidecar_task-name.hcl new file mode 100644 index 000000000..e46d8ce3d --- /dev/null +++ b/jobspec/test-fixtures/service-connect-sidecar_task-name.hcl @@ -0,0 +1,15 @@ +job "sidecar_task_name" { + type = "service" + + group "group" { + service { + name = "example" + connect { + sidecar_service {} + sidecar_task { + name = "my-sidecar" + } + } + } + } +} \ No newline at end of file