Backport of Rename fields on proxyConfig into release/1.2.x (#15954)

* Rename fields on proxyConfig (manual backport of #15541) * fixup backport mistake --------- Co-authored-by: Seth Hoenig <shoenig@duck.com>
hashicorp · Jan 30, 2023 · ada9163 · ada9163
1 parent c0ce214
commit ada9163
Show file tree

Hide file tree

Showing 8 changed files with 51 additions and 22 deletions.
diff --git a/.changelog/15541.txt b/.changelog/15541.txt
@@ -0,0 +1,12 @@
+```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/services.go b/api/services.go
@@ -265,7 +265,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"`
 }
@@ -275,7 +276,7 @@ func (cp *ConsulProxy) Canonicalize() {
 		return
 	}
 
-	cp.ExposeConfig.Canonicalize()
+	cp.Expose.Canonicalize()
 
 	if len(cp.Upstreams) == 0 {
 		cp.Upstreams = nil
@@ -357,14 +358,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/services_test.go b/api/services_test.go
@@ -194,7 +194,7 @@ func TestService_Connect_ConsulProxy_Canonicalize(t *testing.T) {
 		cp.Canonicalize()
 		require.Empty(t, cp.LocalServiceAddress)
 		require.Zero(t, cp.LocalServicePort)
-		require.Nil(t, cp.ExposeConfig)
+		require.Nil(t, cp.Expose)
 		require.Nil(t, cp.Upstreams)
 		require.Empty(t, cp.Config)
 	})
@@ -203,14 +203,14 @@ func TestService_Connect_ConsulProxy_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()
 		require.Equal(t, "127.0.0.1", cp.LocalServiceAddress)
 		require.Equal(t, 80, cp.LocalServicePort)
-		require.Equal(t, &ConsulExposeConfig{}, cp.ExposeConfig)
+		require.Equal(t, &ConsulExposeConfig{}, cp.Expose)
 		require.Nil(t, cp.Upstreams)
 		require.Nil(t, cp.Config)
 	})

diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go
@@ -1568,11 +1568,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),
 	}
 }
@@ -1608,8 +1615,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
@@ -3610,8 +3610,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
@@ -799,7 +799,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
 		}
 	}
 
@@ -850,13 +850,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
@@ -1232,8 +1232,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,
@@ -1336,8 +1336,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
@@ -1135,9 +1135,7 @@ type ConsulProxy struct {
 
 	// Expose configures the consul proxy.expose stanza 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.
@@ -1348,8 +1346,7 @@ func (u *ConsulUpstream) Equals(o *ConsulUpstream) bool {
 
 // ConsulExposeConfig represents a Consul Connect expose jobspec stanza.
 type ConsulExposeConfig struct {
-	// Use json tag to match with field name in api/
-	Paths []ConsulExposePath `json:"Path"`
+	Paths []ConsulExposePath
 }
 
 type ConsulExposePath struct {