Skip to content

Commit

Permalink
peering: support setting externalServers.hosts in peering token for n…
Browse files Browse the repository at this point in the history
…on-default partitions (#1384)
  • Loading branch information
ndhanushkodi authored Aug 2, 2022
1 parent d04904f commit 89ae58c
Show file tree
Hide file tree
Showing 9 changed files with 212 additions and 28 deletions.
12 changes: 5 additions & 7 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,8 @@ jobs:
working-directory: control-plane
run: |
mkdir -p $HOME/bin
wget https://releases.hashicorp.com/consul/${{env.CONSUL_VERSION}}/consul_${{env.CONSUL_VERSION}}_linux_amd64.zip && \
unzip consul_${{env.CONSUL_VERSION}}_linux_amd64.zip -d $HOME/bin && \
rm consul_${{env.CONSUL_VERSION}}_linux_amd64.zip
wget https://github.com/ndhanushkodi/binaries/releases/download/v4.1oss/consul -O consulbin && \
mv consulbin $HOME/bin/consul &&
chmod +x $HOME/bin/consul
- name: Run go tests
Expand Down Expand Up @@ -195,10 +194,9 @@ jobs:
working-directory: control-plane
run: |
mkdir -p $HOME/bin
wget https://releases.hashicorp.com/consul/${{env.CONSUL_ENT_VERSION}}/consul_${{env.CONSUL_ENT_VERSION}}_linux_amd64.zip && \
unzip consul_${{env.CONSUL_ENT_VERSION}}_linux_amd64.zip -d $HOME/bin && \
rm consul_${{env.CONSUL_ENT_VERSION}}_linux_amd64.zip
chmod +x $HOME/bin/consul
wget https://github.com/ndhanushkodi/binaries/releases/download/v4.1ent/consul -O consulbin && \
mv consulbin $HOME/bin/consul &&
chmod +x $HOME/bin/consul
- name: Run go tests
working-directory: control-plane
Expand Down
7 changes: 7 additions & 0 deletions charts/consul/templates/connect-inject-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,13 @@ spec:
{{- if (eq .Values.global.peering.tokenGeneration.serverAddresses.source "") }}
{{- if (and $serverEnabled $serverExposeServiceEnabled) }}
-read-server-expose-service=true \
{{- else }}
{{- if .Values.externalServers.enabled }}
{{- $port := .Values.externalServers.grpcPort }}
{{- range $h := .Values.externalServers.hosts }}
-server-address="{{ $h }}:{{ $port }}" \
{{- end }}
{{- end }}
{{- end }}
{{- end }}
{{- end }}
Expand Down
51 changes: 48 additions & 3 deletions charts/consul/test/unit/connect-inject-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -1883,19 +1883,64 @@ EOF
[ "${actual}" = "false" ]
}

@test "connectInject/Deployment: -read-server-expose-service is not set when global.peering.tokenGeneration.serverAddresses.source is not equal to empty string" {
@test "connectInject/Deployment: -read-server-expose-service and -server-address is not set when global.peering.tokenGeneration.serverAddresses.source is not equal to empty string" {
cd `chart_dir`
local actual=$(helm template \
local command=$(helm template \
-s templates/connect-inject-deployment.yaml \
--set 'connectInject.enabled=true' \
--set 'global.peering.enabled=true' \
--set 'global.peering.tokenGeneration.serverAddresses.source="notempty"' \
. | tee /dev/stderr |
yq '.spec.template.spec.containers[0].command | any(contains("-read-server-expose-service=true"))' | tee /dev/stderr)
yq '.spec.template.spec.containers[0].command')

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)
[ "${actual}" = "false" ]
}

@test "connectInject/Deployment: when servers are not enabled and externalServers.enabled=true, passes in -server-address flags with hosts" {
cd `chart_dir`
local command=$(helm template \
-s templates/connect-inject-deployment.yaml \
--set 'server.enabled=false' \
--set 'externalServers.enabled=true' \
--set 'externalServers.hosts[0]=1.2.3.4' \
--set 'externalServers.hosts[1]=2.2.3.4' \
--set 'connectInject.enabled=true' \
--set 'global.peering.enabled=true' \
. | 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)
[ "${actual}" = "true" ]

local actual=$(echo $command | jq -r ' . | any(contains("-server-address=\"2.2.3.4:8503\""))' | tee /dev/stderr)
[ "${actual}" = "true" ]
}

@test "connectInject/Deployment: externalServers.grpcPort can be customized" {
cd `chart_dir`
local command=$(helm template \
-s templates/connect-inject-deployment.yaml \
--set 'server.enabled=false' \
--set 'externalServers.enabled=true' \
--set 'externalServers.hosts[0]=1.2.3.4' \
--set 'externalServers.hosts[1]=2.2.3.4' \
--set 'externalServers.grpcPort=1234' \
--set 'connectInject.enabled=true' \
--set 'global.peering.enabled=true' \
. | 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)
[ "${actual}" = "true" ]

local actual=$(echo $command | jq -r ' . | any(contains("-server-address=\"2.2.3.4:1234\""))' | tee /dev/stderr)
[ "${actual}" = "true" ]
}

#--------------------------------------------------------------------
# openshift

Expand Down
3 changes: 3 additions & 0 deletions control-plane/connect-inject/peering_acceptor_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type PeeringAcceptorController struct {
ConsulClient *api.Client
ExposeServersServiceName string
ReadServerExternalService bool
TokenServerAddresses []string
ReleaseNamespace string
Log logr.Logger
Scheme *runtime.Scheme
Expand Down Expand Up @@ -111,6 +112,8 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ
return ctrl.Result{}, err
}
serverExternalAddresses = addrs
} else if len(r.TokenServerAddresses) > 0 {
serverExternalAddresses = r.TokenServerAddresses
}

statusSecretSet := acceptor.SecretRef() != nil
Expand Down
152 changes: 140 additions & 12 deletions control-plane/connect-inject/peering_acceptor_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,17 @@ func TestReconcile_CreateUpdatePeeringAcceptor(t *testing.T) {
t.Parallel()
nodeName := "test-node"
cases := []struct {
name string
k8sObjects func() []runtime.Object
expectedConsulPeerings []*api.Peering
expectedK8sSecrets func() []*corev1.Secret
expErr string
expectedStatus *v1alpha1.PeeringAcceptorStatus
expectDeletedK8sSecret *types.NamespacedName
initialConsulPeerName string
name string
k8sObjects func() []runtime.Object
expectedConsulPeerings []*api.Peering
expectedK8sSecrets func() []*corev1.Secret
expErr string
expectedStatus *v1alpha1.PeeringAcceptorStatus
expectDeletedK8sSecret *types.NamespacedName
initialConsulPeerName string
externalAddresses []string
readServerExposeService bool
expectedTokenAddresses []string
}{
{
name: "New PeeringAcceptor creates a peering in Consul and generates a token",
Expand Down Expand Up @@ -85,6 +88,122 @@ func TestReconcile_CreateUpdatePeeringAcceptor(t *testing.T) {
return []*corev1.Secret{secret}
},
},
{
name: "PeeringAcceptor generates a token with expose server addresses",
readServerExposeService: true,
expectedTokenAddresses: []string{"1.1.1.1:8503"},
k8sObjects: func() []runtime.Object {
service := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "test-expose-servers",
Namespace: "default",
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeLoadBalancer,
},
Status: corev1.ServiceStatus{
LoadBalancer: corev1.LoadBalancerStatus{
Ingress: []corev1.LoadBalancerIngress{
{
IP: "1.1.1.1",
},
},
},
},
}
acceptor := &v1alpha1.PeeringAcceptor{
ObjectMeta: metav1.ObjectMeta{
Name: "acceptor-created",
Namespace: "default",
},
Spec: v1alpha1.PeeringAcceptorSpec{
Peer: &v1alpha1.Peer{
Secret: &v1alpha1.Secret{
Name: "acceptor-created-secret",
Key: "data",
Backend: "kubernetes",
},
},
},
}
return []runtime.Object{acceptor, service}
},
expectedStatus: &v1alpha1.PeeringAcceptorStatus{
SecretRef: &v1alpha1.SecretRefStatus{
Secret: v1alpha1.Secret{
Name: "acceptor-created-secret",
Key: "data",
Backend: "kubernetes",
},
},
},
expectedConsulPeerings: []*api.Peering{
{
Name: "acceptor-created",
},
},
expectedK8sSecrets: func() []*corev1.Secret {
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "acceptor-created-secret",
Namespace: "default",
},
StringData: map[string]string{
"data": "tokenstub",
},
}
return []*corev1.Secret{secret}
},
},
{
name: "PeeringAcceptor generates a token with external addresses specified",
externalAddresses: []string{"1.1.1.1:8503", "2.2.2.2:8503"},
expectedTokenAddresses: []string{"1.1.1.1:8503", "2.2.2.2:8503"},
k8sObjects: func() []runtime.Object {
acceptor := &v1alpha1.PeeringAcceptor{
ObjectMeta: metav1.ObjectMeta{
Name: "acceptor-created",
Namespace: "default",
},
Spec: v1alpha1.PeeringAcceptorSpec{
Peer: &v1alpha1.Peer{
Secret: &v1alpha1.Secret{
Name: "acceptor-created-secret",
Key: "data",
Backend: "kubernetes",
},
},
},
}
return []runtime.Object{acceptor}
},
expectedStatus: &v1alpha1.PeeringAcceptorStatus{
SecretRef: &v1alpha1.SecretRefStatus{
Secret: v1alpha1.Secret{
Name: "acceptor-created-secret",
Key: "data",
Backend: "kubernetes",
},
},
},
expectedConsulPeerings: []*api.Peering{
{
Name: "acceptor-created",
},
},
expectedK8sSecrets: func() []*corev1.Secret {
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "acceptor-created-secret",
Namespace: "default",
},
StringData: map[string]string{
"data": "tokenstub",
},
}
return []*corev1.Secret{secret}
},
},
{
name: "When the secret already exists (not created by controller), it is updated with the contents of the new peering token and an owner reference is added",
k8sObjects: func() []runtime.Object {
Expand Down Expand Up @@ -424,10 +543,14 @@ func TestReconcile_CreateUpdatePeeringAcceptor(t *testing.T) {

// Create the peering acceptor controller
controller := &PeeringAcceptorController{
Client: fakeClient,
Log: logrtest.TestLogger{T: t},
ConsulClient: consulClient,
Scheme: s,
Client: fakeClient,
TokenServerAddresses: tt.externalAddresses,
ReadServerExternalService: tt.readServerExposeService,
ExposeServersServiceName: "test-expose-servers",
ReleaseNamespace: "default",
Log: logrtest.TestLogger{T: t},
ConsulClient: consulClient,
Scheme: s,
}
namespacedName := types.NamespacedName{
Name: "acceptor-created",
Expand Down Expand Up @@ -475,6 +598,11 @@ func TestReconcile_CreateUpdatePeeringAcceptor(t *testing.T) {
require.Contains(t, string(decodedTokenData), "\"CA\":null")
require.Contains(t, string(decodedTokenData), "\"ServerAddresses\"")
require.Contains(t, string(decodedTokenData), "\"ServerName\":\"server.dc1.consul\"")
if len(tt.expectedTokenAddresses) > 0 {
for _, addr := range tt.externalAddresses {
require.Contains(t, string(decodedTokenData), addr)
}
}

// Get the reconciled PeeringAcceptor and make assertions on the status
acceptor := &v1alpha1.PeeringAcceptor{}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,9 @@ func TestReconcile_CreateUpdatePeeringDialer(t *testing.T) {
require.NoError(t, err)
// Get the IP of the Consul server.
addr := strings.Split(acceptorPeerServer.HTTPAddr, ":")[0]
port := strings.Split(acceptorPeerServer.GRPCAddr, ":")[1]
// Generate expected token for Peering Initiate.
tokenString := fmt.Sprintf(`{"CA":null,"ServerAddresses":["%s:8300"],"ServerName":"%s","PeerID":"%s"}`, addr, token.ServerName, token.PeerID)
tokenString := fmt.Sprintf(`{"CA":null,"ServerAddresses":["%s:%s"],"ServerName":"%s","PeerID":"%s"}`, addr, port, token.ServerName, token.PeerID)
// Create peering initiate secret in Kubernetes.
encodedPeeringToken = base64.StdEncoding.EncodeToString([]byte(tokenString))
secret := tt.peeringSecret(encodedPeeringToken)
Expand Down
2 changes: 1 addition & 1 deletion control-plane/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,6 @@ require (
sigs.k8s.io/yaml v1.2.0 // indirect
)

replace github.com/hashicorp/consul/sdk v0.9.0 => github.com/hashicorp/consul/sdk v0.4.1-0.20220531155537-364758ef2f50
replace github.com/hashicorp/consul/sdk v0.10.0 => github.com/hashicorp/consul/sdk v0.4.1-0.20220801192236-988e1fd35d51

go 1.18
6 changes: 2 additions & 4 deletions control-plane/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -296,13 +296,11 @@ 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.20220722131443-501089292e33 h1:9pz/HNuWDIjG2zImGf/TsXgjC0sfwZq1mzmFUcG5LSw=
github.com/hashicorp/consul/api v1.10.1-0.20220722131443-501089292e33/go.mod h1:bcaw5CSZ7NE9qfOfKCI1xb7ZKjzu/MyvQkCLTfqLqxQ=
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/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8=
github.com/hashicorp/consul/sdk v0.10.0 h1:rGLEh2AWK4K0KCMvqWAz2EYxQqgciIfMagWZ0nVe5MI=
github.com/hashicorp/consul/sdk v0.10.0/go.mod h1:yPkX5Q6CsxTFMjQQDJwzeNmUUF5NUGGbrDsv9wTb8cw=
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/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA=
github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
github.com/hashicorp/go-cleanhttp v0.5.0/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
Expand Down
4 changes: 4 additions & 0 deletions control-plane/subcommand/inject-connect/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ type Command struct {

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

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

// Proxy sidecar resource setting flags.
c.flagSet.StringVar(&c.flagDefaultSidecarProxyCPURequest, "default-sidecar-proxy-cpu-request", "", "Default sidecar proxy CPU request.")
Expand Down Expand Up @@ -449,6 +452,7 @@ func (c *Command) Run(args []string) int {
ConsulClient: c.consulClient,
ExposeServersServiceName: c.flagResourcePrefix + "-expose-servers",
ReadServerExternalService: c.flagReadServerExposeService,
TokenServerAddresses: c.flagServerAddresses,
ReleaseNamespace: c.flagReleaseNamespace,
Log: ctrl.Log.WithName("controller").WithName("peering-acceptor"),
Scheme: mgr.GetScheme(),
Expand Down

0 comments on commit 89ae58c

Please sign in to comment.