Skip to content

Commit

Permalink
connect: enable configuring sidecar_task.name
Browse files Browse the repository at this point in the history
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
  • Loading branch information
shoenig committed Apr 10, 2020
1 parent 5b65b95 commit 729931a
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 54 deletions.
27 changes: 1 addition & 26 deletions jobspec/parse_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
73 changes: 45 additions & 28 deletions jobspec/parse_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,48 @@ import (
"github.com/mitchellh/mapstructure"
)

var (
normalTaskKeys = []string{
"artifact",
"config",
"constraint",
"affinity",
"dispatch_payload",
"lifecycle",
"driver",
"env",
"kill_timeout",
"leader",
"logs",
"meta",
"resources",
"restart",
"service",
"shutdown_delay",
"template",
"user",
"vault",
"kill_signal",
"kind",
"volume_mount",
"csi_plugin",
}

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 {
Expand All @@ -29,7 +71,7 @@ func parseTasks(result *[]*api.Task, list *ast.ObjectList) error {
}
seen[n] = struct{}{}

t, err := parseTask(item)
t, err := parseTask(item, normalTaskKeys)
if err != nil {
return multierror.Prefix(err, fmt.Sprintf("'%s',", n))
}
Expand All @@ -42,7 +84,7 @@ func parseTasks(result *[]*api.Task, list *ast.ObjectList) error {
return nil
}

func parseTask(item *ast.ObjectItem) (*api.Task, error) {
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 {
Expand All @@ -52,32 +94,7 @@ func parseTask(item *ast.ObjectItem) (*api.Task, error) {
}

// Check for invalid keys
valid := []string{
"artifact",
"config",
"constraint",
"affinity",
"dispatch_payload",
"lifecycle",
"driver",
"env",
"kill_timeout",
"leader",
"logs",
"meta",
"resources",
"restart",
"service",
"shutdown_delay",
"template",
"user",
"vault",
"kill_signal",
"kind",
"volume_mount",
"csi_plugin",
}
if err := helper.CheckHCLKeys(listVal, valid); err != nil {
if err := helper.CheckHCLKeys(listVal, keys); err != nil {
return nil, err
}

Expand Down
22 changes: 22 additions & 0 deletions jobspec/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
15 changes: 15 additions & 0 deletions jobspec/test-fixtures/service-connect-sidecar_task-name.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
job "sidecar_task_name" {
type = "service"

group "group" {
service {
name = "example"
connect {
sidecar_service {}
sidecar_task {
name = "my-sidecar"
}
}
}
}
}

0 comments on commit 729931a

Please sign in to comment.