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

Jump to bottom

peering: update unit tests to not reuse peering token #1401

Merged
merged 3 commits into from
Aug 9, 2022
Merged

peering: update unit tests to not reuse peering token #1401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ce2a2f7
update unit tests to not reuse peering token
ndhanushkodi Aug 9, 2022
61d9c1e
fix lint
ndhanushkodi Aug 9, 2022
4fafa90
use released versions
ndhanushkodi Aug 9, 2022
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
14 changes: 8 additions & 6 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ on:

env:
TEST_RESULTS: /tmp/test-results # path to where test results are saved
CONSUL_VERSION: 1.13.0-alpha2 # Consul's OSS version to use in tests
CONSUL_ENT_VERSION: 1.13.0-alpha2+ent # Consul's enterprise version to use in tests
CONSUL_VERSION: 1.13.0 # Consul's OSS version to use in tests
CONSUL_ENT_VERSION: 1.13.0+ent # Consul's enterprise version to use in tests
GOTESTSUM_VERSION: 1.8.1 # You cannot use environment variables with workflows. The gotestsum version is hardcoded in the reusable workflows too.

jobs:
Expand Down Expand Up @@ -146,8 +146,9 @@ jobs:
working-directory: control-plane
run: |
mkdir -p $HOME/bin
wget https://github.com/ndhanushkodi/binaries/releases/download/v4.1oss/consul -O consulbin && \
mv consulbin $HOME/bin/consul &&
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
chmod +x $HOME/bin/consul

- name: Run go tests
Expand Down Expand Up @@ -194,8 +195,9 @@ jobs:
working-directory: control-plane
run: |
mkdir -p $HOME/bin
wget https://github.com/ndhanushkodi/binaries/releases/download/v4.1ent/consul -O consulbin && \
mv consulbin $HOME/bin/consul &&
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

- name: Run go tests
Expand Down
4 changes: 2 additions & 2 deletions control-plane/connect-inject/peering_dialer_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func (r *PeeringDialerController) Reconcile(ctx context.Context, req ctrl.Reques
// Or, if the peering in Consul does exist, compare it to the spec's secret. If there's any
// differences, initiate peering.
if r.specStatusSecretsDifferent(dialer, specSecret) {
r.Log.Info("the version annotation was incremented; re-establishing peering with spec.peer.secret", "secret-name", dialer.Secret().Name, "secret-namespace", dialer.Namespace)
r.Log.Info("the spec.peer.secret is different from the status secret, re-establishing peering", "secret-name", dialer.Secret().Name, "secret-namespace", dialer.Namespace)
peeringToken := specSecret.Data[dialer.Secret().Key]
if err := r.establishPeering(ctx, dialer.Name, string(peeringToken)); err != nil {
r.updateStatusError(ctx, dialer, ConsulAgentError, err)
Expand All @@ -163,7 +163,7 @@ func (r *PeeringDialerController) Reconcile(ctx context.Context, req ctrl.Reques
}

if updated, err := r.versionAnnotationUpdated(dialer); err == nil && updated {
r.Log.Info("status.secret exists, but the peering doesn't exist in Consul; establishing peering with the existing spec.peer.secret", "secret-name", dialer.Secret().Name, "secret-namespace", dialer.Namespace)
r.Log.Info("the version annotation was incremented; re-establishing peering with spec.peer.secret", "secret-name", dialer.Secret().Name, "secret-namespace", dialer.Namespace)
peeringToken := specSecret.Data[dialer.Secret().Key]
if err := r.establishPeering(ctx, dialer.Name, string(peeringToken)); err != nil {
r.updateStatusError(ctx, dialer, ConsulAgentError, err)
Expand Down
85 changes: 32 additions & 53 deletions control-plane/connect-inject/peering_dialer_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@ package connectinject

import (
"context"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -270,34 +266,22 @@ func TestReconcile_CreateUpdatePeeringDialer(t *testing.T) {
// Create fake k8s client
k8sObjects := append(tt.k8sObjects(), &ns)

// This is responsible for updating the token generated by the acceptor side with the IP
// of the Consul server as the generated token currently does not have that set on it.
// Generate a token.
var encodedPeeringToken string
if tt.peeringName != "" {
var token struct {
CA string
ServerAddresses []string
ServerName string
PeerID string
}
// Create the initial token.
baseToken, _, err := acceptorClient.Peerings().GenerateToken(context.Background(), api.PeeringGenerateTokenRequest{PeerName: tt.peeringName}, nil)
require.NoError(t, err)
// Decode the token to extract the ServerName and PeerID from the token. CA is always NULL.
decodeBytes, err := base64.StdEncoding.DecodeString(baseToken.PeeringToken)
require.NoError(t, err)
err = json.Unmarshal(decodeBytes, &token)
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:%s"],"ServerName":"%s","PeerID":"%s"}`, addr, port, token.ServerName, token.PeerID)
// Create peering initiate secret in Kubernetes.
encodedPeeringToken = base64.StdEncoding.EncodeToString([]byte(tokenString))
encodedPeeringToken = baseToken.PeeringToken
}

// If the peering is not supposed to already exist in Consul, then create a secret with the generated token.
if !tt.peeringExists {
secret := tt.peeringSecret(encodedPeeringToken)
secret.SetResourceVersion("latest-version")
k8sObjects = append(k8sObjects, secret)
if secret != nil {
secret.SetResourceVersion("latest-version")
k8sObjects = append(k8sObjects, secret)
}
}

// Create test consul server.
Expand All @@ -314,12 +298,19 @@ func TestReconcile_CreateUpdatePeeringDialer(t *testing.T) {
dialerClient, err := api.NewClient(cfg)
require.NoError(t, err)

// If the peering is supposed to already exist in Consul, then establish a peering with the existing token, so the peering will exist on the dialing side.
if tt.peeringExists {
_, _, err := dialerClient.Peerings().Establish(context.Background(), api.PeeringEstablishRequest{PeerName: tt.peeringName, PeeringToken: encodedPeeringToken}, nil)
require.NoError(t, err)
k8sObjects = append(k8sObjects, createSecret("dialer-token-old", "default", "token", "old-token"))
// Create a new token to be used by Reconcile(). The original token has already been
// used once to simulate establishing an existing peering.
baseToken, _, err := acceptorClient.Peerings().GenerateToken(context.Background(), api.PeeringGenerateTokenRequest{PeerName: tt.peeringName}, nil)
require.NoError(t, err)
secret := tt.peeringSecret(baseToken.PeeringToken)
secret.SetResourceVersion("latest-version")
k8sObjects = append(k8sObjects, secret)
}

s := scheme.Scheme
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringDialer{}, &v1alpha1.PeeringDialerList{})
fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(k8sObjects...).Build()
Expand Down Expand Up @@ -481,32 +472,11 @@ func TestReconcile_VersionAnnotationPeeringDialer(t *testing.T) {
// Create fake k8s client
k8sObjects := []runtime.Object{dialer, ns}

// This is responsible for updating the token generated by the acceptor side with the IP
// of the Consul server as the generated token currently does not have that set on it.
var encodedPeeringToken string
var token struct {
CA string
ServerAddresses []string
ServerName string
PeerID string
}
// Create the initial token.
baseToken, _, err := acceptorClient.Peerings().GenerateToken(context.Background(), api.PeeringGenerateTokenRequest{PeerName: "peering"}, nil)
// Create a peering connection in Consul by generating and establishing a peering connection before calling
// Reconcile().
// Generate a token.
generatedToken, _, err := acceptorClient.Peerings().GenerateToken(context.Background(), api.PeeringGenerateTokenRequest{PeerName: "peering"}, nil)
require.NoError(t, err)
// Decode the token to extract the ServerName and PeerID from the token. CA is always NULL.
decodeBytes, err := base64.StdEncoding.DecodeString(baseToken.PeeringToken)
require.NoError(t, err)
err = json.Unmarshal(decodeBytes, &token)
require.NoError(t, err)
// Get the IP of the Consul server.
addr := strings.Split(acceptorPeerServer.HTTPAddr, ":")[0]
// Generate expected token for Peering Initiate.
tokenString := fmt.Sprintf(`{"CA":null,"ServerAddresses":["%s:8300"],"ServerName":"%s","PeerID":"%s"}`, addr, token.ServerName, token.PeerID)
// Create peering initiate secret in Kubernetes.
encodedPeeringToken = base64.StdEncoding.EncodeToString([]byte(tokenString))
secret := createSecret("dialer-token", "default", "token", encodedPeeringToken)
secret.SetResourceVersion("latest-version")
k8sObjects = append(k8sObjects, secret)

// Create test consul server.
dialerPeerServer, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) {
Expand All @@ -522,10 +492,19 @@ func TestReconcile_VersionAnnotationPeeringDialer(t *testing.T) {
dialerClient, err := api.NewClient(cfg)
require.NoError(t, err)

_, _, err = dialerClient.Peerings().Establish(context.Background(), api.PeeringEstablishRequest{PeerName: "peering", PeeringToken: encodedPeeringToken}, nil)
// Establish a peering with the generated token.
_, _, err = dialerClient.Peerings().Establish(context.Background(), api.PeeringEstablishRequest{PeerName: "peering", PeeringToken: generatedToken.PeeringToken}, nil)
require.NoError(t, err)
k8sObjects = append(k8sObjects, createSecret("dialer-token-old", "default", "token", "old-token"))

// Create a new token to be potentially used by Reconcile(). The original token has already been
// used once to simulate establishing an existing peering.
token, _, err := acceptorClient.Peerings().GenerateToken(context.Background(), api.PeeringGenerateTokenRequest{PeerName: "peering"}, nil)
require.NoError(t, err)
secret := createSecret("dialer-token", "default", "token", token.PeeringToken)
secret.SetResourceVersion("latest-version")
k8sObjects = append(k8sObjects, secret)

s := scheme.Scheme
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringDialer{}, &v1alpha1.PeeringDialerList{})
fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(k8sObjects...).Build()
Expand Down