Validate Consul secrets while respecting federation

CHANGELOG.md

@@ -9,6 +9,8 @@ IMPROVEMENTS:
   * Upgrade Docker image Alpine version from 3.14 to 3.15. [[GH-1058](https://github.com/hashicorp/consul-k8s/pull/1058)]
 * Helm
   * API Gateway: Allow controller to read Kubernetes namespaces in order to determine if route is allowed for gateway. [[GH-1092](https://github.com/hashicorp/consul-k8s/pull/1092)]
+* CLI
+  * Enable users to set up secondary clusters with existing federation secrets. [[GH-1126](https://github.com/hashicorp/consul-k8s/pull/1126)] 
 
 BUG FIXES:
 * Helm

cli/cmd/install/install.go

@@ -14,6 +14,8 @@ import (
 	"github.com/hashicorp/consul-k8s/cli/common/terminal"
 	"github.com/hashicorp/consul-k8s/cli/config"
 	"github.com/hashicorp/consul-k8s/cli/helm"
+	"github.com/hashicorp/consul-k8s/cli/release"
+	"github.com/hashicorp/consul-k8s/cli/validation"
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/chart/loader"
 	helmCLI "helm.sh/helm/v3/pkg/cli"
@@ -176,19 +178,6 @@ func (c *Command) init() {
 	c.Init()
 }
 
-type helmValues struct {
-	Global globalValues `yaml:"global"`
-}
-
-type globalValues struct {
-	EnterpriseLicense enterpriseLicense `yaml:"enterpriseLicense"`
-}
-
-type enterpriseLicense struct {
-	SecretName string `yaml:"secretName"`
-	SecretKey  string `yaml:"secretKey"`
-}
-
 // Run installs Consul into a Kubernetes cluster.
 func (c *Command) Run(args []string) int {
 	c.once.Do(c.init)
@@ -268,13 +257,6 @@ func (c *Command) Run(args []string) int {
 	}
 	c.UI.Output("No existing Consul persistent volume claims found", terminal.WithSuccessStyle())
 
-	// Ensure there's no previous bootstrap secret lying around.
-	if err := c.checkForPreviousSecrets(); err != nil {
-		c.UI.Output(err.Error(), terminal.WithErrorStyle())
-		return 1
-	}
-	c.UI.Output("No existing Consul secrets found", terminal.WithSuccessStyle())
-
 	// Handle preset, value files, and set values logic.
 	vals, err := c.mergeValuesFlagsWithPrecedence(settings)
 	if err != nil {
@@ -287,16 +269,29 @@ func (c *Command) Run(args []string) int {
 		return 1
 	}
 
-	var v helmValues
-	err = yaml.Unmarshal(valuesYaml, &v)
+	var values helm.Values
+	err = yaml.Unmarshal(valuesYaml, &values)
 	if err != nil {
 		c.UI.Output(err.Error(), terminal.WithErrorStyle())
 		return 1
 	}
 
+	release := release.Release{
+		Name:          "consul",
+		Namespace:     c.flagNamespace,
+		Configuration: values,
+	}
+
+	msg, err := c.checkForPreviousSecrets(release)
+	if err != nil {
+		c.UI.Output(err.Error(), terminal.WithErrorStyle())
+		return 1
+	}
+	c.UI.Output(msg, terminal.WithSuccessStyle())
+
 	// If an enterprise license secret was provided, check that the secret exists and that the enterprise Consul image is set.
-	if v.Global.EnterpriseLicense.SecretName != "" {
-		if err := c.checkValidEnterprise(v.Global.EnterpriseLicense.SecretName); err != nil {
+	if values.Global.EnterpriseLicense.SecretName != "" {
+		if err := c.checkValidEnterprise(release.Configuration.Global.EnterpriseLicense.SecretName); err != nil {
 			c.UI.Output(err.Error(), terminal.WithErrorStyle())
 			return 1
 		}
@@ -420,21 +415,45 @@ func (c *Command) checkForPreviousPVCs() error {
 	return nil
 }
 
-// checkForPreviousSecrets checks for the bootstrap token and returns an error if found.
-func (c *Command) checkForPreviousSecrets() error {
-	secrets, err := c.kubernetes.CoreV1().Secrets("").List(c.Ctx, metav1.ListOptions{LabelSelector: common.CLILabelKey + "=" + common.CLILabelValue})
+// checkForPreviousSecrets checks for Consul secrets that exist in the cluster
+// and returns a message if the secret configuration is ok or an error if
+// the secret configuration could cause a conflict.
+func (c *Command) checkForPreviousSecrets(release release.Release) (string, error) {
+	secrets, err := validation.ListConsulSecrets(c.Ctx, c.kubernetes)
 	if err != nil {
-		return fmt.Errorf("error listing secrets: %s", err)
+		return "", fmt.Errorf("Error listing Consul secrets: %s", err)
+	}
+
+	// If the Consul configuration is a secondary DC, only one secret should
+	// exist, the Consul federation secret.
+	fedSecret := release.Configuration.Global.Acls.ReplicationToken.SecretName
+	if release.ShouldExpectFederationSecret() {
+		if len(secrets.Items) == 1 && secrets.Items[0].Name == fedSecret {
+			return fmt.Sprintf("Found secret %s for Consul federation.", fedSecret), nil
+		} else if len(secrets.Items) == 0 {
+			return "", fmt.Errorf("Missing secret %s for Consul federation.\n"+
+				"Please refer to the Consul Secondary Cluster configuration docs:\nhttps://www.consul.io/docs/k8s/installation/multi-cluster/kubernetes#secondary-cluster-s", fedSecret)
+		}
 	}
-	for _, secret := range secrets.Items {
-		// future TODO: also check for federation secret
-		if secret.ObjectMeta.Labels[common.CLILabelKey] == common.CLILabelValue {
-			return fmt.Errorf("found Consul secret from previous installation: %q in namespace %q. Use the command `kubectl delete secret %s --namespace %s` to delete",
-				secret.Name, secret.Namespace, secret.Name, secret.Namespace)
+
+	// If not a secondary DC for federation, no Consul secrets should exist.
+	if len(secrets.Items) > 0 {
+		// Nicely format the delete commands for existing Consul secrets.
+		namespacedSecrets := make(map[string][]string)
+		for _, secret := range secrets.Items {
+			namespacedSecrets[secret.Namespace] = append(namespacedSecrets[secret.Namespace], secret.Name)
+		}
+
+		var deleteCmds string
+		for namespace, secretNames := range namespacedSecrets {
+			deleteCmds += fmt.Sprintf("kubectl delete secret %s --namespace %s\n", strings.Join(secretNames, " "), namespace)
 		}
+
+		return "", fmt.Errorf("Found Consul secrets, possibly from a previous installation.\n"+
+			"Delete existing Consul secrets from Kubernetes:\n\n%s", deleteCmds)
 	}
 
-	return nil
+	return "No existing Consul secrets found.", nil
 }
 
 // mergeValuesFlagsWithPrecedence is responsible for merging all the values to determine the values file for the

cli/cmd/install/install_test.go

@@ -6,6 +6,8 @@ import (
 	"testing"
 
 	"github.com/hashicorp/consul-k8s/cli/common"
+	"github.com/hashicorp/consul-k8s/cli/helm"
+	"github.com/hashicorp/consul-k8s/cli/release"
 	"github.com/hashicorp/go-hclog"
 	"github.com/stretchr/testify/require"
 	v1 "k8s.io/api/core/v1"
@@ -49,33 +51,102 @@ func TestCheckForPreviousPVCs(t *testing.T) {
 }
 
 func TestCheckForPreviousSecrets(t *testing.T) {
-	c := getInitializedCommand(t)
-	c.kubernetes = fake.NewSimpleClientset()
-	secret := &v1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:   "test-consul-bootstrap-acl-token",
-			Labels: map[string]string{common.CLILabelKey: common.CLILabelValue},
+	t.Parallel()
+
+	cases := map[string]struct {
+		helmValues helm.Values
+		secret     *v1.Secret
+		expectMsg  bool
+		expectErr  bool
+	}{
+		"No secrets, none expected": {
+			helmValues: helm.Values{},
+			secret:     nil,
+			expectMsg:  true,
+			expectErr:  false,
+		},
+		"Non-Consul secrets, none expected": {
+			helmValues: helm.Values{},
+			secret: &v1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "non-consul-secret",
+				},
+			},
+			expectMsg: true,
+			expectErr: false,
+		},
+		"Consul secrets, none expected": {
+			helmValues: helm.Values{},
+			secret: &v1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:   "consul-secret",
+					Labels: map[string]string{common.CLILabelKey: common.CLILabelValue},
+				},
+			},
+			expectMsg: false,
+			expectErr: true,
+		},
+		"Federation secret, expected": {
+			helmValues: helm.Values{
+				Global: helm.Global{
+					Datacenter: "dc2",
+					Federation: helm.Federation{
+						Enabled:                true,
+						PrimaryDatacenter:      "dc1",
+						CreateFederationSecret: false,
+					},
+					Acls: helm.Acls{
+						ReplicationToken: helm.ReplicationToken{
+							SecretName: "consul-federation",
+						},
+					},
+				},
+			},
+			secret: &v1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:   "consul-federation",
+					Labels: map[string]string{common.CLILabelKey: common.CLILabelValue},
+				},
+			},
+			expectMsg: true,
+			expectErr: false,
+		},
+		"No federation secret, but expected": {
+			helmValues: helm.Values{
+				Global: helm.Global{
+					Datacenter: "dc2",
+					Federation: helm.Federation{
+						Enabled:                true,
+						PrimaryDatacenter:      "dc1",
+						CreateFederationSecret: false,
+					},
+					Acls: helm.Acls{
+						ReplicationToken: helm.ReplicationToken{
+							SecretName: "consul-federation",
+						},
+					},
+				},
+			},
+			secret:    nil,
+			expectMsg: false,
+			expectErr: true,
 		},
 	}
-	c.kubernetes.CoreV1().Secrets("default").Create(context.Background(), secret, metav1.CreateOptions{})
-	err := c.checkForPreviousSecrets()
-	require.Error(t, err)
-	require.Contains(t, err.Error(), "found Consul secret from previous installation")
 
-	// Clear out the client and make sure the check now passes.
-	c.kubernetes = fake.NewSimpleClientset()
-	err = c.checkForPreviousSecrets()
-	require.NoError(t, err)
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			c := getInitializedCommand(t)
+			c.kubernetes = fake.NewSimpleClientset()
 
-	// Add a new irrelevant secret and make sure the check continues to pass.
-	secret = &v1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "irrelevant-secret",
-		},
+			c.kubernetes.CoreV1().Secrets("consul").Create(context.Background(), tc.secret, metav1.CreateOptions{})
+
+			release := release.Release{Configuration: tc.helmValues}
+			msg, err := c.checkForPreviousSecrets(release)
+
+			require.Equal(t, tc.expectMsg, msg != "")
+			require.Equal(t, tc.expectErr, err != nil)
+		})
 	}
-	c.kubernetes.CoreV1().Secrets("default").Create(context.Background(), secret, metav1.CreateOptions{})
-	err = c.checkForPreviousSecrets()
-	require.NoError(t, err)
 }
 
 // TestValidateFlags tests the validate flags function.

cli/common/utils.go

@@ -74,6 +74,9 @@ func MergeMaps(a, b map[string]interface{}) map[string]interface{} {
 	return out
 }
 
+// CloseWithError terminates a command and cleans up its resources.
+// If termination fails, the error is logged and the process exits with an
+// exit code of 1.
 func CloseWithError(c *BaseCommand) {
 	if err := c.Close(); err != nil {
 		c.Log.Error(err.Error())