Merge pull request #22 from patoarvizu/append_roles

Append roles
patoarvizu · Mar 15, 2020 · 95af3b6 · 95af3b6
2 parents 32b16a8 + a8f373c
commit 95af3b6
Show file tree

Hide file tree

Showing 5 changed files with 79 additions and 26 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -68,6 +68,7 @@ jobs:
             k3d import-images patoarvizu/vault-dynamic-configuration-operator:latest
             kubectl apply -f test/manifests/vault/vault-crd.yaml
             kubectl apply -f test/manifests/vault/vault-cluster.yaml
+            kubectl apply -f test/manifests/namespaces/test.yaml
             kubectl apply -f deploy/
             operator-sdk test local ./test/e2e/ --namespace "vault" --go-test-flags '-v -run TestSingleNamespace'
             kubectl -n vault patch deployment vault-dynamic-configuration-operator --type='json' -p='[{"op":"add", "path":"/spec/template/spec/containers/0/command/-", "value":"--bound-roles-to-all-namespaces"}]'

diff --git a/pkg/controller/vdc/serviceaccount_controller.go b/pkg/controller/vdc/serviceaccount_controller.go
@@ -147,18 +147,18 @@ type DBRole struct {
 }
 
 type Role struct {
-	BoundServiceAccountNames      string   `json:"bound_service_account_names"`
-	BoundServiceAccountNamespaces string   `json:"bound_service_account_namespaces"`
-	Name                          string   `json:"name"`
-	TokenPolicies                 []string `json:"token_policies"`
-	TokenTtl                      string   `json:"token_ttl,omitempty"`
-	TokenMaxTtl                   string   `json:"token_max_ttl,omitempty"`
-	TokenBoundCidrs               []string `json:"token_bound_cidrs,omitempty"`
-	TokenExplicitMaxTtl           string   `json:"token_explicit_max_ttl,omitempty"`
-	TokenNoDefaultPolicy          bool     `json:"token_no_default_policy,omitempty"`
-	TokenNumUses                  int      `json:"token_num_uses,omitempty"`
-	TokenPeriod                   string   `json:"token_period,omitempty"`
-	TokenType                     string   `json:"token_type,omitempty"`
+	BoundServiceAccountNames      string      `json:"bound_service_account_names"`
+	BoundServiceAccountNamespaces interface{} `json:"bound_service_account_namespaces"`
+	Name                          string      `json:"name"`
+	TokenPolicies                 []string    `json:"token_policies"`
+	TokenTtl                      string      `json:"token_ttl,omitempty"`
+	TokenMaxTtl                   string      `json:"token_max_ttl,omitempty"`
+	TokenBoundCidrs               []string    `json:"token_bound_cidrs,omitempty"`
+	TokenExplicitMaxTtl           string      `json:"token_explicit_max_ttl,omitempty"`
+	TokenNoDefaultPolicy          bool        `json:"token_no_default_policy,omitempty"`
+	TokenNumUses                  int         `json:"token_num_uses,omitempty"`
+	TokenPeriod                   string      `json:"token_period,omitempty"`
+	TokenType                     string      `json:"token_type,omitempty"`
 }
 
 type policyTemplateInput struct {
@@ -311,18 +311,33 @@ func addOrUpdatePolicy(bvConfig *BankVaultsConfig, metadata metav1.ObjectMeta, c
 }
 
 func addOrUpdateKubernetesRole(kubernetesAuth *Auth, metadata metav1.ObjectMeta) {
-	for _, r := range kubernetesAuth.Roles {
+	for i, r := range kubernetesAuth.Roles {
 		if r.Name == metadata.Name {
+			if BoundRolesToAllNamespaces {
+				kubernetesAuth.Roles[i].BoundServiceAccountNamespaces = []string{"*"}
+			} else {
+				switch r.BoundServiceAccountNamespaces.(type) {
+				case string:
+					kubernetesAuth.Roles[i].BoundServiceAccountNamespaces = []string{metadata.Name}
+				case []interface{}:
+					for _, n := range r.BoundServiceAccountNamespaces.([]interface{}) {
+						if n.(string) == metadata.Namespace {
+							return
+						}
+					}
+					kubernetesAuth.Roles[i].BoundServiceAccountNamespaces = append(kubernetesAuth.Roles[i].BoundServiceAccountNamespaces.([]interface{}), metadata.Namespace)
+				}
+			}
 			return
 		}
 	}
 	newRole := &Role{
 		BoundServiceAccountNames: metadata.Name,
-		BoundServiceAccountNamespaces: func(namespace string) string {
+		BoundServiceAccountNamespaces: func(namespace string) []string {
 			if BoundRolesToAllNamespaces {
-				return "*"
+				return []string{"*"}
 			} else {
-				return namespace
+				return []string{namespace}
 			}
 		}(metadata.Namespace),
 		Name:          metadata.Name,

diff --git a/test/e2e/operator_test.go b/test/e2e/operator_test.go
@@ -10,23 +10,32 @@ func TestSingleNamespaceServiceAccountAnnotation(t *testing.T) {
 	ctx := framework.NewTestCtx(t)
 	defer ctx.Cleanup()
 	setup(t, ctx)
-	createServiceAccount("operator-test", map[string]string{}, ctx)
-	testVaultRole("operator-test", "default", t)
+	createServiceAccount("operator-test", "default", map[string]string{}, ctx)
+	testVaultRole("operator-test", []string{"default"}, t)
 }
 
 func TestSingleNamespaceServiceAccountDBAnnotation(t *testing.T) {
 	ctx := framework.NewTestCtx(t)
 	defer ctx.Cleanup()
 	setup(t, ctx)
-	createServiceAccount("operator-test-db", map[string]string{"vault.patoarvizu.dev/db-dynamic-creds": "mysql"}, ctx)
-	testVaultRole("operator-test-db", "default", t)
+	createServiceAccount("operator-test-db", "default", map[string]string{"vault.patoarvizu.dev/db-dynamic-creds": "mysql"}, ctx)
+	testVaultRole("operator-test-db", []string{"default"}, t)
 	testVaultDBRole("operator-test-db", t)
 }
 
+func TestSingleNamespaceMultipleAccounts(t *testing.T) {
+	ctx := framework.NewTestCtx(t)
+	defer ctx.Cleanup()
+	setup(t, ctx)
+	createServiceAccount("operator-test-multi-ns", "test-vdc1", map[string]string{}, ctx)
+	createServiceAccount("operator-test-multi-ns", "test-vdc2", map[string]string{}, ctx)
+	testVaultRole("operator-test-multi-ns", []string{"test-vdc1", "test-vdc2"}, t)
+}
+
 func TestAllNamespacesServiceAccountAnnotation(t *testing.T) {
 	ctx := framework.NewTestCtx(t)
 	defer ctx.Cleanup()
 	setup(t, ctx)
-	createServiceAccount("operator-test-all", map[string]string{}, ctx)
-	testVaultRole("operator-test-all", "*", t)
+	createServiceAccount("operator-test-all", "default", map[string]string{}, ctx)
+	testVaultRole("operator-test-all", []string{"*"}, t)
 }
diff --git a/test/e2e/util.go b/test/e2e/util.go
@@ -30,22 +30,31 @@ func setup(t *testing.T, ctx *test.TestCtx) {
 	}
 }
 
-func createServiceAccount(name string, extraAnnotations map[string]string, ctx *test.TestCtx) error {
+func createServiceAccount(name string, namespace string, extraAnnotations map[string]string, ctx *test.TestCtx) error {
 	annotations := map[string]string{"vault.patoarvizu.dev/auto-configure": "true"}
 	for k, v := range extraAnnotations {
 		annotations[k] = v
 	}
 	var opertatorTestServiceAccount = &apiv1.ServiceAccount{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        name,
-			Namespace:   "default",
+			Namespace:   namespace,
 			Annotations: annotations,
 		},
 	}
 	return framework.Global.Client.Create(context.TODO(), opertatorTestServiceAccount, &framework.CleanupOptions{TestContext: ctx, Timeout: time.Second * 60, RetryInterval: time.Second * 1})
 }
 
-func testVaultRole(name string, namespace string, t *testing.T) {
+func namespaceIsInAllowedList(namespace string, allowedNamespaces interface{}) bool {
+	for _, ns := range allowedNamespaces.([]interface{}) {
+		if ns.(string) == namespace {
+			return true
+		}
+	}
+	return false
+}
+
+func testVaultRole(name string, namespaces []string, t *testing.T) {
 	vaultCR := &bankvaultsv1alpha1.Vault{}
 	bvConfig := vdc.BankVaultsConfig{}
 	err := wait.Poll(time.Second*2, time.Second*20, func() (done bool, err error) {
@@ -62,9 +71,17 @@ func testVaultRole(name string, namespace string, t *testing.T) {
 		if wErr != nil {
 			return false, nil
 		}
-		if role.BoundServiceAccountNames != name || role.BoundServiceAccountNamespaces != namespace || role.TokenTtl != "5m" {
+		if role.BoundServiceAccountNames != name || role.TokenTtl != "5m" {
 			t.Errorf("Test role '%s' is not configured correctly", name)
 		}
+		if len(role.BoundServiceAccountNamespaces.([]interface{})) < len(namespaces) {
+			return false, nil
+		}
+		for _, ns := range namespaces {
+			if !namespaceIsInAllowedList(ns, role.BoundServiceAccountNamespaces) {
+				t.Errorf("Namespace '%s' is not in list of role bound namespaces", ns)
+			}
+		}
 		if role.TokenPolicies[0] != name {
 			t.Errorf("Test role '%s' policies are not configured correctly", name)
 		}

diff --git a/test/manifests/namespaces/test.yaml b/test/manifests/namespaces/test.yaml
@@ -0,0 +1,11 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: test-vdc1
+
+---
+
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: test-vdc2