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 <shoenig@duck.com>
This commit is contained in:
Jorge Marey 2023-01-30 16:31:16 +01:00 committed by GitHub
parent bb79824a20
commit d1c9aad762
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 51 additions and 23 deletions

11
.changelog/15541.txt Normal file
View File

@ -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`
```

View File

@ -137,7 +137,8 @@ func (st *SidecarTask) Canonicalize() {
type ConsulProxy struct { type ConsulProxy struct {
LocalServiceAddress string `mapstructure:"local_service_address" hcl:"local_service_address,optional"` LocalServiceAddress string `mapstructure:"local_service_address" hcl:"local_service_address,optional"`
LocalServicePort int `mapstructure:"local_service_port" hcl:"local_service_port,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"` Upstreams []*ConsulUpstream `hcl:"upstreams,block"`
Config map[string]interface{} `hcl:"config,block"` Config map[string]interface{} `hcl:"config,block"`
} }
@ -147,7 +148,7 @@ func (cp *ConsulProxy) Canonicalize() {
return return
} }
cp.ExposeConfig.Canonicalize() cp.Expose.Canonicalize()
if len(cp.Upstreams) == 0 { if len(cp.Upstreams) == 0 {
cp.Upstreams = nil cp.Upstreams = nil
@ -234,7 +235,8 @@ func (cu *ConsulUpstream) Canonicalize() {
} }
type ConsulExposeConfig struct { 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() { func (cec *ConsulExposeConfig) Canonicalize() {
@ -242,6 +244,10 @@ func (cec *ConsulExposeConfig) Canonicalize() {
return return
} }
if len(cec.Paths) == 0 {
cec.Paths = nil
}
if len(cec.Path) == 0 { if len(cec.Path) == 0 {
cec.Path = nil cec.Path = nil
} }

View File

@ -133,7 +133,7 @@ func TestConsulProxy_Canonicalize(t *testing.T) {
cp.Canonicalize() cp.Canonicalize()
must.Eq(t, "", cp.LocalServiceAddress) must.Eq(t, "", cp.LocalServiceAddress)
must.Zero(t, cp.LocalServicePort) must.Zero(t, cp.LocalServicePort)
must.Nil(t, cp.ExposeConfig) must.Nil(t, cp.Expose)
must.Nil(t, cp.Upstreams) must.Nil(t, cp.Upstreams)
must.MapEmpty(t, cp.Config) must.MapEmpty(t, cp.Config)
}) })
@ -142,14 +142,14 @@ func TestConsulProxy_Canonicalize(t *testing.T) {
cp := &ConsulProxy{ cp := &ConsulProxy{
LocalServiceAddress: "127.0.0.1", LocalServiceAddress: "127.0.0.1",
LocalServicePort: 80, LocalServicePort: 80,
ExposeConfig: new(ConsulExposeConfig), Expose: new(ConsulExposeConfig),
Upstreams: make([]*ConsulUpstream, 0), Upstreams: make([]*ConsulUpstream, 0),
Config: make(map[string]interface{}), Config: make(map[string]interface{}),
} }
cp.Canonicalize() cp.Canonicalize()
must.Eq(t, "127.0.0.1", cp.LocalServiceAddress) must.Eq(t, "127.0.0.1", cp.LocalServiceAddress)
must.Eq(t, 80, cp.LocalServicePort) 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.Upstreams)
must.Nil(t, cp.Config) must.Nil(t, cp.Config)
}) })

View File

@ -1646,11 +1646,18 @@ func apiConnectSidecarServiceProxyToStructs(in *api.ConsulProxy) *structs.Consul
if in == nil { if in == nil {
return nil return nil
} }
// TODO: to maintain backwards compatibility
expose := in.Expose
if in.ExposeConfig != nil {
expose = in.ExposeConfig
}
return &structs.ConsulProxy{ return &structs.ConsulProxy{
LocalServiceAddress: in.LocalServiceAddress, LocalServiceAddress: in.LocalServiceAddress,
LocalServicePort: in.LocalServicePort, LocalServicePort: in.LocalServicePort,
Upstreams: apiUpstreamsToStructs(in.Upstreams), Upstreams: apiUpstreamsToStructs(in.Upstreams),
Expose: apiConsulExposeConfigToStructs(in.ExposeConfig), Expose: apiConsulExposeConfigToStructs(expose),
Config: maps.Clone(in.Config), Config: maps.Clone(in.Config),
} }
} }
@ -1686,8 +1693,15 @@ func apiConsulExposeConfigToStructs(in *api.ConsulExposeConfig) *structs.ConsulE
if in == nil { if in == nil {
return nil return nil
} }
// TODO: to maintain backwards compatibility
paths := in.Paths
if in.Path != nil {
paths = in.Path
}
return &structs.ConsulExposeConfig{ return &structs.ConsulExposeConfig{
Paths: apiConsulExposePathsToStructs(in.Path), Paths: apiConsulExposePathsToStructs(paths),
} }
} }

View File

@ -3694,7 +3694,7 @@ func TestConversion_apiConsulExposeConfigToStructs(t *testing.T) {
require.Equal(t, &structs.ConsulExposeConfig{ require.Equal(t, &structs.ConsulExposeConfig{
Paths: []structs.ConsulExposePath{{Path: "/health"}}, Paths: []structs.ConsulExposePath{{Path: "/health"}},
}, apiConsulExposeConfigToStructs(&api.ConsulExposeConfig{ }, 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{{ Upstreams: []*api.ConsulUpstream{{
DestinationName: "upstream", DestinationName: "upstream",
}}, }},
ExposeConfig: &api.ConsulExposeConfig{ Expose: &api.ConsulExposeConfig{
Path: []*api.ConsulExposePath{{ Paths: []*api.ConsulExposePath{{
Path: "/health", Path: "/health",
}}, }},
}, },

View File

@ -819,7 +819,7 @@ func parseProxy(o *ast.ObjectItem) (*api.ConsulProxy, error) {
if e, err := parseExpose(eo.Items[0]); err != nil { if e, err := parseExpose(eo.Items[0]); err != nil {
return nil, err return nil, err
} else { } else {
proxy.ExposeConfig = e proxy.Expose = e
} }
} }
@ -870,13 +870,13 @@ func parseExpose(eo *ast.ObjectItem) (*api.ConsulExposeConfig, error) {
po := listVal.Filter("path") // array po := listVal.Filter("path") // array
if len(po.Items) > 0 { 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 { for i := range po.Items {
p, err := parseExposePath(po.Items[i]) p, err := parseExposePath(po.Items[i])
if err != nil { if err != nil {
return nil, err return nil, err
} }
expose.Path[i] = p expose.Paths[i] = p
} }
} }

View File

@ -1282,8 +1282,8 @@ func TestParse(t *testing.T) {
Connect: &api.ConsulConnect{ Connect: &api.ConsulConnect{
SidecarService: &api.ConsulSidecarService{ SidecarService: &api.ConsulSidecarService{
Proxy: &api.ConsulProxy{ Proxy: &api.ConsulProxy{
ExposeConfig: &api.ConsulExposeConfig{ Expose: &api.ConsulExposeConfig{
Path: []*api.ConsulExposePath{{ Paths: []*api.ConsulExposePath{{
Path: "/health", Path: "/health",
Protocol: "http", Protocol: "http",
LocalPathPort: 2222, LocalPathPort: 2222,
@ -1386,8 +1386,8 @@ func TestParse(t *testing.T) {
Proxy: &api.ConsulProxy{ Proxy: &api.ConsulProxy{
LocalServiceAddress: "10.0.1.2", LocalServiceAddress: "10.0.1.2",
LocalServicePort: 8080, LocalServicePort: 8080,
ExposeConfig: &api.ConsulExposeConfig{ Expose: &api.ConsulExposeConfig{
Path: []*api.ConsulExposePath{{ Paths: []*api.ConsulExposePath{{
Path: "/metrics", Path: "/metrics",
Protocol: "http", Protocol: "http",
LocalPathPort: 9001, LocalPathPort: 9001,

View File

@ -1356,9 +1356,7 @@ type ConsulProxy struct {
// Expose configures the consul proxy.expose block to "open up" endpoints // Expose configures the consul proxy.expose block to "open up" endpoints
// used by task-group level service checks using HTTP or gRPC protocols. // used by task-group level service checks using HTTP or gRPC protocols.
// Expose *ConsulExposeConfig
// Use json tag to match with field name in api/
Expose *ConsulExposeConfig `json:"ExposeConfig"`
// Config is a proxy configuration. It is opaque to Nomad and passed // Config is a proxy configuration. It is opaque to Nomad and passed
// directly to Consul. // directly to Consul.
@ -1526,8 +1524,7 @@ func upstreamsEquals(a, b []ConsulUpstream) bool {
// ConsulExposeConfig represents a Consul Connect expose jobspec block. // ConsulExposeConfig represents a Consul Connect expose jobspec block.
type ConsulExposeConfig struct { type ConsulExposeConfig struct {
// Use json tag to match with field name in api/ Paths []ConsulExposePath
Paths []ConsulExposePath `json:"Path"`
} }
type ConsulExposePath struct { type ConsulExposePath struct {