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 <[email protected]>
hashicorp · Jan 30, 2023 · 340ad2d · 340ad2d
1 parent 631fd47
commit 340ad2d
Show file tree

Hide file tree

Showing 8 changed files with 51 additions and 23 deletions.
diff --git a/.changelog/15541.txt 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
@@ -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,14 +235,19 @@ 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() {
 	if cec == nil {
 		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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 {