Rename fields on proxyConfig

[jorgemarey]

This renames two fields in the proxyConfig.

• Expose to ExposeConfig
• Paths to Path

I don't like the use of "path" in singular for a list of items, but it's already like that in the public API, so changing that would break compatibility.

Fixes https://github.com/hashicorp/nomad/issues/11304, https://github.com/hashicorp/nomad/issues/12174 and https://github.com/hashicorp/nomad/issues/9379.

[vercel[bot]]

@jorgemarey is attempting to deploy a commit to the HashiCorp Team on Vercel.

A member of the Team first needs to authorize it.

[jorgemarey]

Hi @jrasell. I thought of that, but wasn't sure about it. I don't know if it's a problem with the msgpack library or other thing, but when returning the job via the API, those fields are not serialized correctly and so the input Job differs from the output job, which is a real issue get doing thinks like get a job, modify a parameter and run it again.

Do you have any ideas of how to best solve this?

[jorgemarey]

Hi again, If this aproach doesn't work. How about chaning the API but making it compatible with the current format. It would be something like the following (we'll need to add the changes to the Path field also):

diff --git a/api/consul.go b/api/consul.go
index 9a76bfb329..7457ec91f4 100644
--- a/api/consul.go
+++ b/api/consul.go
@@ -133,11 +133,13 @@ func (st *SidecarTask) Canonicalize() {
 
 // ConsulProxy represents a Consul Connect sidecar proxy jobspec stanza.
 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"`
-	Upstreams           []*ConsulUpstream      `hcl:"upstreams,block"`
-	Config              map[string]interface{} `hcl:"config,block"`
+	LocalServiceAddress string              `mapstructure:"local_service_address" hcl:"local_service_address,optional"`
+	LocalServicePort    int                 `mapstructure:"local_service_port" hcl:"local_service_port,optional"`
+	Expose              *ConsulExposeConfig `mapstructure:"expose" hcl:"expose,block"`
+	// Deprecated, only to maintain backwards compatibility
+	ExposeConfig *ConsulExposeConfig
+	Upstreams    []*ConsulUpstream      `hcl:"upstreams,block"`
+	Config       map[string]interface{} `hcl:"config,block"`
 }
 
 func (cp *ConsulProxy) Canonicalize() {
@@ -145,7 +147,7 @@ func (cp *ConsulProxy) Canonicalize() {
 		return
 	}
 
-	cp.ExposeConfig.Canonicalize()
+	cp.Expose.Canonicalize()
 
 	if len(cp.Upstreams) == 0 {
 		cp.Upstreams = nil
diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go
index a6440b924c..45d96f1966 100644
--- a/command/agent/job_endpoint.go
+++ b/command/agent/job_endpoint.go
@@ -1646,11 +1646,17 @@ 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),
 	}
 }
diff --git a/jobspec/parse_service.go b/jobspec/parse_service.go
index 7dc10214b3..8dcc9a1ed1 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
 		}
 	}

Basically, change the api field to be Expose (like in the struct field but checking if in the json the value ExposeConfig has content, and in that case use that.

[dpewsey]

Hi @jrasell can you check the above? Seems to be a fix and this is quite a significant issue that is broken.
Limits us from being able to move to nomad right now.

[shoenig]

Hi @jorgemarey sorry for letting this issue slip by - but yeah I think introducing the new field name while deprecating the old name but keeping it for compatibility sounds reasonable. We're expecting to ship 1.5-beta early next week so if we can get those changes in, that would be really great.

[jorgemarey]

Hi @shoenig , thanks for looking into this. I'll try to make the changes this weekend.

[jorgemarey]

Hi @shoenig, I think I made all the needed changes for this to work. Let me know if you think I should change anything else (feel free to make the changes yourself if you want to)