Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename flag for peering token server addresses. #1426

Merged
merged 1 commit into from
Aug 16, 2022
Merged

Rename flag for peering token server addresses. #1426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions charts/consul/templates/connect-inject-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
26 changes: 13 additions & 13 deletions charts/consul/test/unit/connect-inject-deployment.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand All @@ -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 \
Expand All @@ -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" ]
}

Expand All @@ -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 \
Expand All @@ -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 \
Expand All @@ -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" ]
}

Expand Down
2 changes: 1 addition & 1 deletion control-plane/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems related to changes not made in the PR but caused automatically by a go mod tidy.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm, this was a change I made yesterday for the pointer changes.. I wonder how that worked..

Copy link
Contributor Author

@thisisnotashwin thisisnotashwin Aug 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah..not quite sure myself

sigs.k8s.io/controller-runtime v0.10.2
)

Expand Down Expand Up @@ -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
)
Expand Down
4 changes: 0 additions & 4 deletions control-plane/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
8 changes: 4 additions & 4 deletions control-plane/subcommand/inject-connect/command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ type Command struct {

// Server address flags.
flagReadServerExposeService bool
flagServerAddresses []string
flagTokenServerAddresses []string

// Transparent proxy flags.
flagDefaultEnableTransparentProxy bool
Expand Down Expand Up @@ -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.")
Expand Down Expand Up @@ -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(),
Expand Down