From 74148f7b5581f1a397a526ead1f8e8ef84a459b4 Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Tue, 16 Aug 2022 11:01:06 -0400 Subject: [PATCH] Rename flag for peering token server addresses. - The flag was initally named `server-address` but the `server-address` flag has been used across the project to imply ONLY the address of the external server when they are 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 needs to explicitly know the list of external servers. --- .../templates/connect-inject-deployment.yaml | 4 +-- .../test/unit/connect-inject-deployment.bats | 26 +++++++++---------- control-plane/go.mod | 2 +- control-plane/go.sum | 4 --- .../subcommand/inject-connect/command.go | 8 +++--- 5 files changed, 20 insertions(+), 24 deletions(-) diff --git a/charts/consul/templates/connect-inject-deployment.yaml b/charts/consul/templates/connect-inject-deployment.yaml index 266eeb4994..a5cae8edc5 100644 --- a/charts/consul/templates/connect-inject-deployment.yaml +++ b/charts/consul/templates/connect-inject-deployment.yaml @@ -148,14 +148,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 52552c6721..22464ae1b4 100755 --- a/charts/consul/test/unit/connect-inject-deployment.bats +++ b/charts/consul/test/unit/connect-inject-deployment.bats @@ -1907,7 +1907,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 \ @@ -1920,11 +1920,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 \ @@ -1937,10 +1937,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" ] } @@ -1958,14 +1958,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 \ @@ -1977,14 +1977,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 \ @@ -2000,10 +2000,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/go.mod b/control-plane/go.mod index 742f7df71d..1848c8f435 100644 --- a/control-plane/go.mod +++ b/control-plane/go.mod @@ -26,6 +26,7 @@ require ( k8s.io/apimachinery v0.22.2 k8s.io/client-go v0.22.2 k8s.io/klog/v2 v2.9.0 + k8s.io/utils v0.0.0-20220812165043-ad590609e2e5 sigs.k8s.io/controller-runtime v0.10.2 ) @@ -122,7 +123,6 @@ require ( k8s.io/apiextensions-apiserver v0.22.2 // indirect k8s.io/component-base v0.22.2 // indirect k8s.io/kube-openapi v0.0.0-20210421082810-95288971da7e // indirect - k8s.io/utils v0.0.0-20220812165043-ad590609e2e5 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.1.2 // indirect sigs.k8s.io/yaml v1.2.0 // indirect ) diff --git a/control-plane/go.sum b/control-plane/go.sum index 4bcd31929e..03ca166926 100644 --- a/control-plane/go.sum +++ b/control-plane/go.sum @@ -296,12 +296,9 @@ github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgf github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw= github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q= -github.com/hashicorp/consul/api v1.10.1-0.20220725163158-2da8949d788c h1:lEMAoMGwTncIh9opSHvvGxM0WrNT5YuCbKipwtU7UNs= -github.com/hashicorp/consul/api v1.10.1-0.20220725163158-2da8949d788c/go.mod h1:bcaw5CSZ7NE9qfOfKCI1xb7ZKjzu/MyvQkCLTfqLqxQ= github.com/hashicorp/consul/api v1.14.0 h1:Y64GIJ8hYTu+tuGekwO4G4ardXoiCivX9wv1iP/kihk= github.com/hashicorp/consul/api v1.14.0/go.mod h1:bcaw5CSZ7NE9qfOfKCI1xb7ZKjzu/MyvQkCLTfqLqxQ= github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8= -github.com/hashicorp/consul/sdk v0.4.1-0.20220801192236-988e1fd35d51 h1:jLjJ0p6QaJFg0PJjWcx+qWTTMujanlJRIRJl15jvG8I= github.com/hashicorp/consul/sdk v0.4.1-0.20220801192236-988e1fd35d51/go.mod h1:yPkX5Q6CsxTFMjQQDJwzeNmUUF5NUGGbrDsv9wTb8cw= github.com/hashicorp/consul/sdk v0.11.0 h1:HRzj8YSCln2yGgCumN5CL8lYlD3gBurnervJRJAZyC4= github.com/hashicorp/consul/sdk v0.11.0/go.mod h1:yPkX5Q6CsxTFMjQQDJwzeNmUUF5NUGGbrDsv9wTb8cw= @@ -1015,7 +1012,6 @@ k8s.io/kube-openapi v0.0.0-20200121204235-bf4fb3bd569c/go.mod h1:GRQhZsXIAJ1xR0C k8s.io/kube-openapi v0.0.0-20210421082810-95288971da7e h1:KLHHjkdQFomZy8+06csTWZ0m1343QqxZhR2LJ1OxCYM= k8s.io/kube-openapi v0.0.0-20210421082810-95288971da7e/go.mod h1:vHXdDvt9+2spS2Rx9ql3I8tycm3H9FDfdUoIuKCefvw= k8s.io/utils v0.0.0-20200324210504-a9aa75ae1b89/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew= -k8s.io/utils v0.0.0-20210819203725-bdf08cb9a70a h1:8dYfu/Fc9Gz2rNJKB9IQRGgQOh2clmRzNIPPY1xLY5g= k8s.io/utils v0.0.0-20210819203725-bdf08cb9a70a/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= k8s.io/utils v0.0.0-20220812165043-ad590609e2e5 h1:XmRqFcQlCy/lKRZ39j+RVpokYNroHPqV3mcBRfnhT5o= k8s.io/utils v0.0.0-20220812165043-ad590609e2e5/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= diff --git a/control-plane/subcommand/inject-connect/command.go b/control-plane/subcommand/inject-connect/command.go index 1b4de11afb..008cb8b400 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 @@ -195,8 +195,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.") @@ -452,7 +452,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(),