From 340ad2db58392d6a37e30a0a37f48655e2bf2a9e Mon Sep 17 00:00:00 2001 From: Jorge Marey <6938602+jorgemarey@users.noreply.github.com> Date: Mon, 30 Jan 2023 16:31:16 +0100 Subject: [PATCH] Rename fields on proxyConfig (#15541) * Change api Fields for expose and paths * Add changelog entry * changelog: add deprecation notes about connect fields * api: minor style tweaks --------- Co-authored-by: Seth Hoenig --- .changelog/15541.txt | 11 +++++++++++ api/consul.go | 12 +++++++++--- api/consul_test.go | 6 +++--- command/agent/job_endpoint.go | 18 ++++++++++++++++-- command/agent/job_endpoint_test.go | 6 +++--- jobspec/parse_service.go | 6 +++--- jobspec/parse_test.go | 8 ++++---- nomad/structs/services.go | 7 ++----- 8 files changed, 51 insertions(+), 23 deletions(-) create mode 100644 .changelog/15541.txt diff --git a/.changelog/15541.txt b/.changelog/15541.txt new file mode 100644 index 00000000000..ed1e9a7c950 --- /dev/null +++ b/.changelog/15541.txt @@ -0,0 +1,11 @@ +```release-note:bug +api: Fixed a bug where exposeConfig field was not provided correctly when getting the jobs via the API +``` + +```release-note:deprecation +api: The connect `ConsulProxy.ExposeConfig` field is deprecated in favor of `ConsulProxy.Expose` +``` + +```release-note:deprecation +api: The connect `ConsulExposeConfig.Path` field is deprecated in favor of `ConsulExposeConfig.Paths` +``` diff --git a/api/consul.go b/api/consul.go index aa2b9d8ed33..a70a8098484 100644 --- a/api/consul.go +++ b/api/consul.go @@ -137,7 +137,8 @@ func (st *SidecarTask) Canonicalize() { type ConsulProxy struct { LocalServiceAddress string `mapstructure:"local_service_address" hcl:"local_service_address,optional"` LocalServicePort int `mapstructure:"local_service_port" hcl:"local_service_port,optional"` - ExposeConfig *ConsulExposeConfig `mapstructure:"expose" hcl:"expose,block"` + Expose *ConsulExposeConfig `mapstructure:"expose" hcl:"expose,block"` + ExposeConfig *ConsulExposeConfig // Deprecated: only to maintain backwards compatibility. Use Expose instead. Upstreams []*ConsulUpstream `hcl:"upstreams,block"` Config map[string]interface{} `hcl:"config,block"` } @@ -147,7 +148,7 @@ func (cp *ConsulProxy) Canonicalize() { return } - cp.ExposeConfig.Canonicalize() + cp.Expose.Canonicalize() if len(cp.Upstreams) == 0 { cp.Upstreams = nil @@ -234,7 +235,8 @@ func (cu *ConsulUpstream) Canonicalize() { } type ConsulExposeConfig struct { - Path []*ConsulExposePath `mapstructure:"path" hcl:"path,block"` + Paths []*ConsulExposePath `mapstructure:"path" hcl:"path,block"` + Path []*ConsulExposePath // Deprecated: only to maintain backwards compatibility. Use Paths instead. } func (cec *ConsulExposeConfig) Canonicalize() { @@ -242,6 +244,10 @@ func (cec *ConsulExposeConfig) Canonicalize() { return } + if len(cec.Paths) == 0 { + cec.Paths = nil + } + if len(cec.Path) == 0 { cec.Path = nil } diff --git a/api/consul_test.go b/api/consul_test.go index f44e47a1ca9..bc714b65d27 100644 --- a/api/consul_test.go +++ b/api/consul_test.go @@ -133,7 +133,7 @@ func TestConsulProxy_Canonicalize(t *testing.T) { cp.Canonicalize() must.Eq(t, "", cp.LocalServiceAddress) must.Zero(t, cp.LocalServicePort) - must.Nil(t, cp.ExposeConfig) + must.Nil(t, cp.Expose) must.Nil(t, cp.Upstreams) must.MapEmpty(t, cp.Config) }) @@ -142,14 +142,14 @@ func TestConsulProxy_Canonicalize(t *testing.T) { cp := &ConsulProxy{ LocalServiceAddress: "127.0.0.1", LocalServicePort: 80, - ExposeConfig: new(ConsulExposeConfig), + Expose: new(ConsulExposeConfig), Upstreams: make([]*ConsulUpstream, 0), Config: make(map[string]interface{}), } cp.Canonicalize() must.Eq(t, "127.0.0.1", cp.LocalServiceAddress) must.Eq(t, 80, cp.LocalServicePort) - must.Eq(t, &ConsulExposeConfig{}, cp.ExposeConfig) + must.Eq(t, &ConsulExposeConfig{}, cp.Expose) must.Nil(t, cp.Upstreams) must.Nil(t, cp.Config) }) diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 9ece74fea43..3c0be480230 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1646,11 +1646,18 @@ func apiConnectSidecarServiceProxyToStructs(in *api.ConsulProxy) *structs.Consul if in == nil { return nil } + + // TODO: to maintain backwards compatibility + expose := in.Expose + if in.ExposeConfig != nil { + expose = in.ExposeConfig + } + return &structs.ConsulProxy{ LocalServiceAddress: in.LocalServiceAddress, LocalServicePort: in.LocalServicePort, Upstreams: apiUpstreamsToStructs(in.Upstreams), - Expose: apiConsulExposeConfigToStructs(in.ExposeConfig), + Expose: apiConsulExposeConfigToStructs(expose), Config: maps.Clone(in.Config), } } @@ -1686,8 +1693,15 @@ func apiConsulExposeConfigToStructs(in *api.ConsulExposeConfig) *structs.ConsulE if in == nil { return nil } + + // TODO: to maintain backwards compatibility + paths := in.Paths + if in.Path != nil { + paths = in.Path + } + return &structs.ConsulExposeConfig{ - Paths: apiConsulExposePathsToStructs(in.Path), + Paths: apiConsulExposePathsToStructs(paths), } } diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 33ce182c6dc..fa2a19a3ec6 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -3694,7 +3694,7 @@ func TestConversion_apiConsulExposeConfigToStructs(t *testing.T) { require.Equal(t, &structs.ConsulExposeConfig{ Paths: []structs.ConsulExposePath{{Path: "/health"}}, }, apiConsulExposeConfigToStructs(&api.ConsulExposeConfig{ - Path: []*api.ConsulExposePath{{Path: "/health"}}, + Paths: []*api.ConsulExposePath{{Path: "/health"}}, })) } @@ -3747,8 +3747,8 @@ func TestConversion_apiConnectSidecarServiceProxyToStructs(t *testing.T) { Upstreams: []*api.ConsulUpstream{{ DestinationName: "upstream", }}, - ExposeConfig: &api.ConsulExposeConfig{ - Path: []*api.ConsulExposePath{{ + Expose: &api.ConsulExposeConfig{ + Paths: []*api.ConsulExposePath{{ Path: "/health", }}, }, diff --git a/jobspec/parse_service.go b/jobspec/parse_service.go index 5d6858bb2f7..70e715af25b 100644 --- a/jobspec/parse_service.go +++ b/jobspec/parse_service.go @@ -819,7 +819,7 @@ func parseProxy(o *ast.ObjectItem) (*api.ConsulProxy, error) { if e, err := parseExpose(eo.Items[0]); err != nil { return nil, err } else { - proxy.ExposeConfig = e + proxy.Expose = e } } @@ -870,13 +870,13 @@ func parseExpose(eo *ast.ObjectItem) (*api.ConsulExposeConfig, error) { po := listVal.Filter("path") // array if len(po.Items) > 0 { - expose.Path = make([]*api.ConsulExposePath, len(po.Items)) + expose.Paths = make([]*api.ConsulExposePath, len(po.Items)) for i := range po.Items { p, err := parseExposePath(po.Items[i]) if err != nil { return nil, err } - expose.Path[i] = p + expose.Paths[i] = p } } diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 1e5cd8bb2b3..f23c1f2eb48 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -1282,8 +1282,8 @@ func TestParse(t *testing.T) { Connect: &api.ConsulConnect{ SidecarService: &api.ConsulSidecarService{ Proxy: &api.ConsulProxy{ - ExposeConfig: &api.ConsulExposeConfig{ - Path: []*api.ConsulExposePath{{ + Expose: &api.ConsulExposeConfig{ + Paths: []*api.ConsulExposePath{{ Path: "/health", Protocol: "http", LocalPathPort: 2222, @@ -1386,8 +1386,8 @@ func TestParse(t *testing.T) { Proxy: &api.ConsulProxy{ LocalServiceAddress: "10.0.1.2", LocalServicePort: 8080, - ExposeConfig: &api.ConsulExposeConfig{ - Path: []*api.ConsulExposePath{{ + Expose: &api.ConsulExposeConfig{ + Paths: []*api.ConsulExposePath{{ Path: "/metrics", Protocol: "http", LocalPathPort: 9001, diff --git a/nomad/structs/services.go b/nomad/structs/services.go index a2b8a4493f1..4b15a350d49 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -1356,9 +1356,7 @@ type ConsulProxy struct { // Expose configures the consul proxy.expose block to "open up" endpoints // used by task-group level service checks using HTTP or gRPC protocols. - // - // Use json tag to match with field name in api/ - Expose *ConsulExposeConfig `json:"ExposeConfig"` + Expose *ConsulExposeConfig // Config is a proxy configuration. It is opaque to Nomad and passed // directly to Consul. @@ -1526,8 +1524,7 @@ func upstreamsEquals(a, b []ConsulUpstream) bool { // ConsulExposeConfig represents a Consul Connect expose jobspec block. type ConsulExposeConfig struct { - // Use json tag to match with field name in api/ - Paths []ConsulExposePath `json:"Path"` + Paths []ConsulExposePath } type ConsulExposePath struct {