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 2 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
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ jobs:
working-directory: control-plane
run: |
mkdir -p $HOME/bin
wget https://github.com/ndhanushkodi/binaries/releases/download/v4.1oss/consul -O consulbin && \
wget https://github.com/ndhanushkodi/binaries/releases/download/v6oss/consul -O consulbin && \
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 is based of of 1.13.x branch of consul

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we wait for core release before merging?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ya that works or I can make an explicit revert change to switch back to releases.hashicorp.com right after core release-- i think if it takes them past the early afternoon, then I'll merge this for today and make the explicit revert change! that way we can have our unit tests running against 1.13 on main as we're merging other k8s PRs too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

theyve released! So i'll update this PR

mv consulbin $HOME/bin/consul &&
chmod +x $HOME/bin/consul

Expand Down Expand Up @@ -194,7 +194,7 @@ jobs:
working-directory: control-plane
run: |
mkdir -p $HOME/bin
wget https://github.com/ndhanushkodi/binaries/releases/download/v4.1ent/consul -O consulbin && \
wget https://github.com/ndhanushkodi/binaries/releases/download/v6ent/consul -O consulbin && \
mv consulbin $HOME/bin/consul &&
chmod +x $HOME/bin/consul

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