From c0de455e830df36f2e099793a96c440464a2b1bb Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Tue, 16 Aug 2022 12:52:54 -0400 Subject: [PATCH] Rename flag for peering token server addresses. (#1426) - The flag was initially named `server-address` but the `server-address` flag has been used across the project to imply ONLY the address of the external server when enabled. This meaning is overloaded in the context of the server address used for generating the peering token. This leads to errors (specifically in the agentless context), where the deployment must explicitly know the list of external servers. --- .../templates/connect-inject-deployment.yaml | 4 +-- .../test/unit/connect-inject-deployment.bats | 26 +++++++++---------- .../subcommand/inject-connect/command.go | 8 +++--- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/charts/consul/templates/connect-inject-deployment.yaml b/charts/consul/templates/connect-inject-deployment.yaml index c924b95acd..779dceb572 100644 --- a/charts/consul/templates/connect-inject-deployment.yaml +++ b/charts/consul/templates/connect-inject-deployment.yaml @@ -149,14 +149,14 @@ spec: {{- if .Values.externalServers.enabled }} {{- $port := .Values.externalServers.grpcPort }} {{- range $h := .Values.externalServers.hosts }} - -server-address="{{ $h }}:{{ $port }}" \ + -token-server-address="{{ $h }}:{{ $port }}" \ {{- end }} {{- end }} {{- end }} {{- end }} {{- if (eq .Values.global.peering.tokenGeneration.serverAddresses.source "static") }} {{- range $addr := .Values.global.peering.tokenGeneration.serverAddresses.static }} - -server-address="{{ $addr }}" \ + -token-server-address="{{ $addr }}" \ {{- end }} {{- end }} {{- end }} diff --git a/charts/consul/test/unit/connect-inject-deployment.bats b/charts/consul/test/unit/connect-inject-deployment.bats index 95d0958564..8ab6c3855f 100755 --- a/charts/consul/test/unit/connect-inject-deployment.bats +++ b/charts/consul/test/unit/connect-inject-deployment.bats @@ -1937,7 +1937,7 @@ EOF [[ "$output" =~ "global.peering.tokenGeneration.serverAddresses.source must be one of empty string, 'consul' or 'static'" ]] } -@test "connectInject/Deployment: -read-server-expose-service and -server-address is not set when global.peering.tokenGeneration.serverAddresses.source is consul" { +@test "connectInject/Deployment: -read-server-expose-service and -token-server-address is not set when global.peering.tokenGeneration.serverAddresses.source is consul" { cd `chart_dir` local command=$(helm template \ -s templates/connect-inject-deployment.yaml \ @@ -1950,11 +1950,11 @@ EOF local actual=$(echo $command | jq -r ' . | any(contains("-read-server-expose-service=true"))' | tee /dev/stderr) [ "${actual}" = "false" ] - local actual=$(echo $command | jq -r ' . | any(contains("-server-address"))' | tee /dev/stderr) + local actual=$(echo $command | jq -r ' . | any(contains("-token-server-address"))' | tee /dev/stderr) [ "${actual}" = "false" ] } -@test "connectInject/Deployment: when servers are not enabled and externalServers.enabled=true, passes in -server-address flags with hosts" { +@test "connectInject/Deployment: when servers are not enabled and externalServers.enabled=true, passes in -token-server-address flags with hosts" { cd `chart_dir` local command=$(helm template \ -s templates/connect-inject-deployment.yaml \ @@ -1967,10 +1967,10 @@ EOF . | tee /dev/stderr | yq '.spec.template.spec.containers[0].command') - local actual=$(echo $command | jq -r ' . | any(contains("-server-address=\"1.2.3.4:8503\""))' | tee /dev/stderr) + local actual=$(echo $command | jq -r ' . | any(contains("-token-server-address=\"1.2.3.4:8503\""))' | tee /dev/stderr) [ "${actual}" = "true" ] - local actual=$(echo $command | jq -r ' . | any(contains("-server-address=\"2.2.3.4:8503\""))' | tee /dev/stderr) + local actual=$(echo $command | jq -r ' . | any(contains("-token-server-address=\"2.2.3.4:8503\""))' | tee /dev/stderr) [ "${actual}" = "true" ] } @@ -1988,14 +1988,14 @@ EOF . | tee /dev/stderr | yq '.spec.template.spec.containers[0].command') - local actual=$(echo $command | jq -r ' . | any(contains("-server-address=\"1.2.3.4:1234\""))' | tee /dev/stderr) + local actual=$(echo $command | jq -r ' . | any(contains("-token-server-address=\"1.2.3.4:1234\""))' | tee /dev/stderr) [ "${actual}" = "true" ] - local actual=$(echo $command | jq -r ' . | any(contains("-server-address=\"2.2.3.4:1234\""))' | tee /dev/stderr) + local actual=$(echo $command | jq -r ' . | any(contains("-token-server-address=\"2.2.3.4:1234\""))' | tee /dev/stderr) [ "${actual}" = "true" ] } -@test "connectInject/Deployment: when peering token generation source is static passes in -server-address flags with static addresses" { +@test "connectInject/Deployment: when peering token generation source is static passes in -token-server-address flags with static addresses" { cd `chart_dir` local command=$(helm template \ -s templates/connect-inject-deployment.yaml \ @@ -2007,14 +2007,14 @@ EOF . | tee /dev/stderr | yq '.spec.template.spec.containers[0].command') - local actual=$(echo $command | jq -r ' . | any(contains("-server-address=\"1.2.3.4:1234\""))' | tee /dev/stderr) + local actual=$(echo $command | jq -r ' . | any(contains("-token-server-address=\"1.2.3.4:1234\""))' | tee /dev/stderr) [ "${actual}" = "true" ] - local actual=$(echo $command | jq -r ' . | any(contains("-server-address=\"2.2.3.4:2234\""))' | tee /dev/stderr) + local actual=$(echo $command | jq -r ' . | any(contains("-token-server-address=\"2.2.3.4:2234\""))' | tee /dev/stderr) [ "${actual}" = "true" ] } -@test "connectInject/Deployment: when peering token generation source is static and externalHosts are set, passes in -server-address flags with static addresses, not externalServers.hosts" { +@test "connectInject/Deployment: when peering token generation source is static and externalHosts are set, passes in -token-server-address flags with static addresses, not externalServers.hosts" { cd `chart_dir` local command=$(helm template \ -s templates/connect-inject-deployment.yaml \ @@ -2030,10 +2030,10 @@ EOF . | tee /dev/stderr | yq '.spec.template.spec.containers[0].command') - local actual=$(echo $command | jq -r ' . | any(contains("-server-address=\"1.2.3.4:1234\""))' | tee /dev/stderr) + local actual=$(echo $command | jq -r ' . | any(contains("-token-server-address=\"1.2.3.4:1234\""))' | tee /dev/stderr) [ "${actual}" = "true" ] - local actual=$(echo $command | jq -r ' . | any(contains("-server-address=\"2.2.3.4:2234\""))' | tee /dev/stderr) + local actual=$(echo $command | jq -r ' . | any(contains("-token-server-address=\"2.2.3.4:2234\""))' | tee /dev/stderr) [ "${actual}" = "true" ] } diff --git a/control-plane/subcommand/inject-connect/command.go b/control-plane/subcommand/inject-connect/command.go index 37731da881..cfc8b4f9c1 100644 --- a/control-plane/subcommand/inject-connect/command.go +++ b/control-plane/subcommand/inject-connect/command.go @@ -96,7 +96,7 @@ type Command struct { // Server address flags. flagReadServerExposeService bool - flagServerAddresses []string + flagTokenServerAddresses []string // Transparent proxy flags. flagDefaultEnableTransparentProxy bool @@ -200,8 +200,8 @@ func (c *Command) init() { "Enable or disable JSON output format for logging.") c.flagSet.BoolVar(&c.flagReadServerExposeService, "read-server-expose-service", false, "Enables polling the Consul servers' external service for its IP(s).") - c.flagSet.Var((*flags.AppendSliceValue)(&c.flagServerAddresses), "server-address", - "An address of the Consul server(s), formatted host:port, where host may be an IP or DNS name and port must be a gRPC port. May be specified multiple times for multiple addresses.") + c.flagSet.Var((*flags.AppendSliceValue)(&c.flagTokenServerAddresses), "token-server-address", + "An address of the Consul server(s) as saved in the peering token, formatted host:port, where host may be an IP or DNS name and port must be a gRPC port. May be specified multiple times for multiple addresses.") // Proxy sidecar resource setting flags. c.flagSet.StringVar(&c.flagDefaultSidecarProxyCPURequest, "default-sidecar-proxy-cpu-request", "", "Default sidecar proxy CPU request.") @@ -457,7 +457,7 @@ func (c *Command) Run(args []string) int { ConsulClient: c.consulClient, ExposeServersServiceName: c.flagResourcePrefix + "-expose-servers", ReadServerExternalService: c.flagReadServerExposeService, - TokenServerAddresses: c.flagServerAddresses, + TokenServerAddresses: c.flagTokenServerAddresses, ReleaseNamespace: c.flagReleaseNamespace, Log: ctrl.Log.WithName("controller").WithName("peering-acceptor"), Scheme: mgr.GetScheme(),