Skip to content

Commit

Permalink
Validate Consul secrets while respecting federation (#1126)
Browse files Browse the repository at this point in the history 
* Add validation package with Kubernetes checks

* Add values struct

* Add Release and a way to check if a fed secret should be expected

* Add comment to CloseWithError

* Correct ReplicationToken values to strings

* Look for secrets across all namespaces

* Check for secrets, respecting the federation secret

* Modify secrets test to take into account federation

* Remove validations that aren't ready to be used yet

* Remove previous helmValues struct

* Simplify collecting namespaced secrets

* Add changelog and fix comment

* Capitalize an "e"

* Use command context

* Use string template in place of concatenation
  • Loading branch information
Thomas Eckert authored Mar 30, 2022
1 parent 07b191b commit 7ae8173
Show file tree
Hide file tree
Showing 9 changed files with 949 additions and 56 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
87 changes: 53 additions & 34 deletions cli/cmd/install/install.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
115 changes: 93 additions & 22 deletions cli/cmd/install/install_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions cli/common/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
Loading

0 comments on commit 7ae8173

Please sign in to comment.