From 80d07f6e6cd6defcd5aef3f68a355633bf8ce812 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Tue, 5 Jan 2021 09:27:01 -0600 Subject: [PATCH 1/2] consul/connect: avoid NPE from unset connect gateway proxy Submitting a job with an ingress gateway in host networking mode with an absent gateway.proxy block would cause the Nomad client to panic on NPE. The consul registration bits would assume the proxy stanza was not nil, but it could be if the user does not supply any manually configured envoy proxy settings. Check the proxy field is not nil before using it. Fixes #9669 --- command/agent/consul/connect.go | 38 +++++++++++++++------------- command/agent/consul/connect_test.go | 11 ++++++++ nomad/structs/services.go | 1 + 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/command/agent/consul/connect.go b/command/agent/consul/connect.go index 62049241b..1fcf3f568 100644 --- a/command/agent/consul/connect.go +++ b/command/agent/consul/connect.go @@ -45,29 +45,33 @@ func newConnectGateway(serviceName string, connect *structs.ConsulConnect) *api. return nil } - proxy := connect.Gateway.Proxy + var envoyConfig map[string]interface{} - envoyConfig := make(map[string]interface{}) + // Populate the envoy configuration from the gateway.proxy stanza, if + // such configuration is provided. + if proxy := connect.Gateway.Proxy; proxy != nil { + envoyConfig = make(map[string]interface{}) - if len(proxy.EnvoyGatewayBindAddresses) > 0 { - envoyConfig["envoy_gateway_bind_addresses"] = proxy.EnvoyGatewayBindAddresses - } + if len(proxy.EnvoyGatewayBindAddresses) > 0 { + envoyConfig["envoy_gateway_bind_addresses"] = proxy.EnvoyGatewayBindAddresses + } - if proxy.EnvoyGatewayNoDefaultBind { - envoyConfig["envoy_gateway_no_default_bind"] = true - } + if proxy.EnvoyGatewayNoDefaultBind { + envoyConfig["envoy_gateway_no_default_bind"] = true + } - if proxy.EnvoyGatewayBindTaggedAddresses { - envoyConfig["envoy_gateway_bind_tagged_addresses"] = true - } + if proxy.EnvoyGatewayBindTaggedAddresses { + envoyConfig["envoy_gateway_bind_tagged_addresses"] = true + } - if proxy.ConnectTimeout != nil { - envoyConfig["connect_timeout_ms"] = proxy.ConnectTimeout.Milliseconds() - } + if proxy.ConnectTimeout != nil { + envoyConfig["connect_timeout_ms"] = proxy.ConnectTimeout.Milliseconds() + } - if len(proxy.Config) > 0 { - for k, v := range proxy.Config { - envoyConfig[k] = v + if len(proxy.Config) > 0 { + for k, v := range proxy.Config { + envoyConfig[k] = v + } } } diff --git a/command/agent/consul/connect_test.go b/command/agent/consul/connect_test.go index ea1728bda..e3d08d3e9 100644 --- a/command/agent/consul/connect_test.go +++ b/command/agent/consul/connect_test.go @@ -429,6 +429,17 @@ func TestConnect_newConnectGateway(t *testing.T) { }, result) }) + t.Run("proxy undefined", func(t *testing.T) { + result := newConnectGateway("s1", &structs.ConsulConnect{ + Gateway: &structs.ConsulGateway{ + Proxy: nil, + }, + }) + require.Equal(t, &api.AgentServiceConnectProxyConfig{ + Config: nil, + }, result) + }) + t.Run("full", func(t *testing.T) { result := newConnectGateway("s1", &structs.ConsulConnect{ Gateway: &structs.ConsulGateway{ diff --git a/nomad/structs/services.go b/nomad/structs/services.go index 9d7d9152c..2ca1dd81f 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -733,6 +733,7 @@ func (c *ConsulConnect) IsNative() bool { return c != nil && c.Native } +// IsGateway checks if the service is a Connect gateway. func (c *ConsulConnect) IsGateway() bool { return c != nil && c.Gateway != nil } From d57b3904909a0cf305f3efabddba83a5f362174c Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Tue, 5 Jan 2021 09:31:22 -0600 Subject: [PATCH 2/2] docs: update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93de7d900..90e396c68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ IMPROVEMENTS: BUG FIXES: * consul: Fixed a bug where updating a task to include services would not work [[GH-9707](https://github.com/hashicorp/nomad/issues/9707)] + * consul/connect: Fixed a bug where absent ingress envoy proxy configuration could panic client [[GH-9669](https://github.com/hashicorp/nomad/issues/9669)] * template: Fixed multiple issues in template src/dest and artifact dest interpolation [[GH-9671](https://github.com/hashicorp/nomad/issues/9671)] * template: Fixed a bug where dynamic secrets did not trigger the template `change_mode` after a client restart. [[GH-9636](https://github.com/hashicorp/nomad/issues/9636)] * server: Fixed a bug where new servers may bootstrap prematurely when configured with `bootstrap_expect = 0`.