From 3ff5201420f00205c6cbe06fc16fbf1c3bba020c Mon Sep 17 00:00:00 2001 From: Charlie Voiselle <464492+angrycub@users.noreply.github.com> Date: Mon, 1 Nov 2021 19:05:46 -0400 Subject: [PATCH 1/3] Parse `job > group > consul` block in HCL1 Moved the validate function into structs/consul_oss.go so that we can have alternative enterprise implementation. Prevents silent ignore. Added parse test for jobspec and error tests on the struct --- jobspec/parse_group.go | 40 ++++++++++++++++++++++ jobspec/parse_test.go | 16 +++++++++ jobspec/test-fixtures/consul-namespace.hcl | 7 ++++ nomad/structs/consul.go | 6 ---- nomad/structs/consul_oss.go | 9 +++++ nomad/structs/consul_test.go | 2 +- 6 files changed, 73 insertions(+), 7 deletions(-) create mode 100644 jobspec/test-fixtures/consul-namespace.hcl diff --git a/jobspec/parse_group.go b/jobspec/parse_group.go index 8977afd5140..060af851f55 100644 --- a/jobspec/parse_group.go +++ b/jobspec/parse_group.go @@ -40,6 +40,7 @@ func parseGroups(result *api.Job, list *ast.ObjectList) error { valid := []string{ "count", "constraint", + "consul", "affinity", "restart", "meta", @@ -67,6 +68,7 @@ func parseGroups(result *api.Job, list *ast.ObjectList) error { } delete(m, "constraint") + delete(m, "consul") delete(m, "affinity") delete(m, "meta") delete(m, "task") @@ -104,6 +106,13 @@ func parseGroups(result *api.Job, list *ast.ObjectList) error { } } + // Parse consul + if o := listVal.Filter("consul"); len(o.Items) > 0 { + if err := parseConsul(&g.Consul, o); err != nil { + return multierror.Prefix(err, fmt.Sprintf("'%s', consul ->", n)) + } + } + // Parse affinities if o := listVal.Filter("affinity"); len(o.Items) > 0 { if err := parseAffinities(&g.Affinities, o); err != nil { @@ -228,6 +237,37 @@ func parseGroups(result *api.Job, list *ast.ObjectList) error { return nil } +func parseConsul(result **api.Consul, list *ast.ObjectList) error { + list = list.Elem() + if len(list.Items) > 1 { + return fmt.Errorf("only one 'consul' block allowed") + } + + // Get our consul object + obj := list.Items[0] + + // Check for invalid keys + valid := []string{ + "namespace", + } + 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 + } + + var consul api.Consul + if err := mapstructure.WeakDecode(m, &consul); err != nil { + return err + } + *result = &consul + + return nil +} + func parseEphemeralDisk(result **api.EphemeralDisk, list *ast.ObjectList) error { list = list.Elem() if len(list.Items) > 1 { diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 0f563045cfd..63cac85dfd7 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -1671,6 +1671,22 @@ func TestParse(t *testing.T) { nil, true, }, + { + "consul-namespace.hcl", + &api.Job{ + ID: stringToPtr("consul-namespace"), + Name: stringToPtr("consul-namespace"), + TaskGroups: []*api.TaskGroup{ + { + Name: stringToPtr("group"), + Consul: &api.Consul{ + Namespace: "foo", + }, + }, + }, + }, + false, + }, { "multiregion.hcl", &api.Job{ diff --git a/jobspec/test-fixtures/consul-namespace.hcl b/jobspec/test-fixtures/consul-namespace.hcl new file mode 100644 index 00000000000..7e808527c19 --- /dev/null +++ b/jobspec/test-fixtures/consul-namespace.hcl @@ -0,0 +1,7 @@ +job "consul-namespace" { + group "group" { + consul { + namespace = "foo" + } + } +} diff --git a/nomad/structs/consul.go b/nomad/structs/consul.go index aa934a1a2a5..2275af09da3 100644 --- a/nomad/structs/consul.go +++ b/nomad/structs/consul.go @@ -24,12 +24,6 @@ func (c *Consul) Equals(o *Consul) bool { return c.Namespace == o.Namespace } -// Validate returns whether c is valid. -func (c *Consul) Validate() error { - // nothing to do here - return nil -} - // ConsulUsage is provides meta information about how Consul is used by a job, // noting which connect services and normal services will be registered, and // whether the keystore will be read via template. diff --git a/nomad/structs/consul_oss.go b/nomad/structs/consul_oss.go index fa1c36c70f6..575a5e41767 100644 --- a/nomad/structs/consul_oss.go +++ b/nomad/structs/consul_oss.go @@ -3,6 +3,15 @@ package structs +import "errors" + func (c *Consul) GetNamespace() string { return "" } + +func (c *Consul) Validate() error { + if c.Namespace != "" { + return errors.New("Setting Consul namespaces in a job requires Nomad Enterprise.") + } + return nil +} diff --git a/nomad/structs/consul_test.go b/nomad/structs/consul_test.go index 43801c93370..f9224599dd6 100644 --- a/nomad/structs/consul_test.go +++ b/nomad/structs/consul_test.go @@ -50,6 +50,6 @@ func TestConsul_Validate(t *testing.T) { t.Run("with ns", func(t *testing.T) { result := (&Consul{Namespace: "one"}).Validate() - require.Nil(t, result) + require.Error(t, result) }) } From 62f52001574c618f937f9cd43821f679ee49135f Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Wed, 3 Nov 2021 13:12:27 -0400 Subject: [PATCH 2/3] revert validation on Consul namespace jobspec field --- nomad/structs/consul.go | 6 ++++++ nomad/structs/consul_oss.go | 9 --------- nomad/structs/consul_test.go | 2 +- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/nomad/structs/consul.go b/nomad/structs/consul.go index 2275af09da3..aa934a1a2a5 100644 --- a/nomad/structs/consul.go +++ b/nomad/structs/consul.go @@ -24,6 +24,12 @@ func (c *Consul) Equals(o *Consul) bool { return c.Namespace == o.Namespace } +// Validate returns whether c is valid. +func (c *Consul) Validate() error { + // nothing to do here + return nil +} + // ConsulUsage is provides meta information about how Consul is used by a job, // noting which connect services and normal services will be registered, and // whether the keystore will be read via template. diff --git a/nomad/structs/consul_oss.go b/nomad/structs/consul_oss.go index 575a5e41767..fa1c36c70f6 100644 --- a/nomad/structs/consul_oss.go +++ b/nomad/structs/consul_oss.go @@ -3,15 +3,6 @@ package structs -import "errors" - func (c *Consul) GetNamespace() string { return "" } - -func (c *Consul) Validate() error { - if c.Namespace != "" { - return errors.New("Setting Consul namespaces in a job requires Nomad Enterprise.") - } - return nil -} diff --git a/nomad/structs/consul_test.go b/nomad/structs/consul_test.go index f9224599dd6..43801c93370 100644 --- a/nomad/structs/consul_test.go +++ b/nomad/structs/consul_test.go @@ -50,6 +50,6 @@ func TestConsul_Validate(t *testing.T) { t.Run("with ns", func(t *testing.T) { result := (&Consul{Namespace: "one"}).Validate() - require.Error(t, result) + require.Nil(t, result) }) } From 47ed06038e8af0eb09d35cd3aa6647f187e8e557 Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Wed, 3 Nov 2021 13:18:58 -0400 Subject: [PATCH 3/3] changelog: add entry for #11423 --- .changelog/11423.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/11423.txt diff --git a/.changelog/11423.txt b/.changelog/11423.txt new file mode 100644 index 00000000000..2f94018c673 --- /dev/null +++ b/.changelog/11423.txt @@ -0,0 +1,3 @@ +```release-note:bug +cli: Fix support for `group.consul` field in the HCLv1 parser +```