Get federation secret from any of the places it is set. (hashicorp#1154)

* Get federation secret from any of the places it is set. * Resolve Changelog * FedSecret is just the release + "-federation" * Add release name to install test * Only expect fed secret if Vault is not the secrets backend
linydquantil · Apr 11, 2022 · edbf3ff · edbf3ff
1 parent 857a6b5
commit edbf3ff
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@ BUG FIXES:
 * CLI
   * Fix issue where clusters not in the same namespace as their deployment name could not be upgraded. [[GH-1115](https://github.com/hashicorp/consul-k8s/pull/1115)]
   * Fix issue where the CLI was looking for secrets in namespaces other than the namespace targeted by the release. [[GH-1156](https://github.com/hashicorp/consul-k8s/pull/1156)]
+  * Fix issue where the federation secret was not being found in certain configurations. [[GH-1154](https://github.com/hashicorp/consul-k8s/issue/1154)]
 
 IMPROVEMENTS:
 * Helm

diff --git a/cli/cmd/install/install.go b/cli/cmd/install/install.go
@@ -418,7 +418,7 @@ func (c *Command) checkForPreviousSecrets(release release.Release) (string, erro
 
 	// If the Consul configuration is a secondary DC, only one secret should
 	// exist, the Consul federation secret.
-	fedSecret := release.Configuration.Global.Acls.ReplicationToken.SecretName
+	fedSecret := release.FedSecret()
 	if release.ShouldExpectFederationSecret() {
 		if len(secrets.Items) == 1 && secrets.Items[0].Name == fedSecret {
 			return fmt.Sprintf("Found secret %s for Consul federation.", fedSecret), nil

diff --git a/cli/cmd/install/install_test.go b/cli/cmd/install/install_test.go
@@ -54,19 +54,22 @@ func TestCheckForPreviousSecrets(t *testing.T) {
 	t.Parallel()
 
 	cases := map[string]struct {
-		helmValues helm.Values
-		secret     *v1.Secret
-		expectMsg  bool
-		expectErr  bool
+		releaseName string
+		helmValues  helm.Values
+		secret      *v1.Secret
+		expectMsg   bool
+		expectErr   bool
 	}{
 		"No secrets, none expected": {
-			helmValues: helm.Values{},
-			secret:     nil,
-			expectMsg:  true,
-			expectErr:  false,
+			releaseName: "consul",
+			helmValues:  helm.Values{},
+			secret:      nil,
+			expectMsg:   true,
+			expectErr:   false,
 		},
 		"Non-Consul secrets, none expected": {
-			helmValues: helm.Values{},
+			releaseName: "consul",
+			helmValues:  helm.Values{},
 			secret: &v1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "non-consul-secret",
@@ -76,7 +79,8 @@ func TestCheckForPreviousSecrets(t *testing.T) {
 			expectErr: false,
 		},
 		"Consul secrets, none expected": {
-			helmValues: helm.Values{},
+			releaseName: "consul",
+			helmValues:  helm.Values{},
 			secret: &v1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:   "consul-secret",
@@ -87,6 +91,7 @@ func TestCheckForPreviousSecrets(t *testing.T) {
 			expectErr: true,
 		},
 		"Federation secret, expected": {
+			releaseName: "consul",
 			helmValues: helm.Values{
 				Global: helm.Global{
 					Datacenter: "dc2",
@@ -112,6 +117,7 @@ func TestCheckForPreviousSecrets(t *testing.T) {
 			expectErr: false,
 		},
 		"No federation secret, but expected": {
+			releaseName: "consul",
 			helmValues: helm.Values{
 				Global: helm.Global{
 					Datacenter: "dc2",
@@ -140,7 +146,7 @@ func TestCheckForPreviousSecrets(t *testing.T) {
 
 			c.kubernetes.CoreV1().Secrets("consul").Create(context.Background(), tc.secret, metav1.CreateOptions{})
 
-			release := release.Release{Configuration: tc.helmValues}
+			release := release.Release{Name: tc.releaseName, Configuration: tc.helmValues}
 			msg, err := c.checkForPreviousSecrets(release)
 
 			require.Equal(t, tc.expectMsg, msg != "")

diff --git a/cli/helm/values.go b/cli/helm/values.go
@@ -75,13 +75,13 @@ type GossipEncryption struct {
 }
 
 type CaCert struct {
-	SecretName interface{} `yaml:"secretName"`
-	SecretKey  interface{} `yaml:"secretKey"`
+	SecretName string `yaml:"secretName"`
+	SecretKey  string `yaml:"secretKey"`
 }
 
 type CaKey struct {
-	SecretName interface{} `yaml:"secretName"`
-	SecretKey  interface{} `yaml:"secretKey"`
+	SecretName string `yaml:"secretName"`
+	SecretKey  string `yaml:"secretKey"`
 }
 
 type TLS struct {

diff --git a/cli/release/release.go b/cli/release/release.go
@@ -21,5 +21,12 @@ type Release struct {
 func (r *Release) ShouldExpectFederationSecret() bool {
 	return r.Configuration.Global.Federation.Enabled &&
 		r.Configuration.Global.Datacenter != r.Configuration.Global.Federation.PrimaryDatacenter &&
-		!r.Configuration.Global.Federation.CreateFederationSecret
+		!r.Configuration.Global.Federation.CreateFederationSecret &&
+		!r.Configuration.Global.SecretsBackend.Vault.Enabled
+}
+
+// FedSecret returns the name of the federation secret which should be created
+// by the operator.
+func (r *Release) FedSecret() string {
+	return r.Name + "-federation"
 }
diff --git a/cli/release/release_test.go b/cli/release/release_test.go
@@ -48,6 +48,24 @@ func TestShouldExpectFederationSecret(t *testing.T) {
 			},
 			expected: true,
 		},
+		"Non-primary DC, federation enabled, Vault secrets backend": {
+			configuration: helm.Values{
+				Global: helm.Global{
+					Datacenter: "dc2",
+					Federation: helm.Federation{
+						Enabled:                true,
+						PrimaryDatacenter:      "dc1",
+						CreateFederationSecret: false,
+					},
+					SecretsBackend: helm.SecretsBackend{
+						Vault: helm.Vault{
+							Enabled: true,
+						},
+					},
+				},
+			},
+			expected: false,
+		},
 	}
 
 	for name, tc := range cases {
@@ -61,3 +79,14 @@ func TestShouldExpectFederationSecret(t *testing.T) {
 		})
 	}
 }
+
+func TestFedSecret(t *testing.T) {
+	release := Release{
+		Name: "test",
+	}
+	expected := "test-federation"
+
+	actual := release.FedSecret()
+
+	require.Equal(t, expected, actual)
+}