From e3900a4b3b23074e3ce8bb3fbceb9a96fbe5fec4 Mon Sep 17 00:00:00 2001 From: Denis Biondic Date: Tue, 12 Nov 2019 15:36:43 +0100 Subject: [PATCH 1/8] fix: remove elasticsearch log format --- Program.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Program.cs b/Program.cs index 27a4e2f..01b5d55 100644 --- a/Program.cs +++ b/Program.cs @@ -1,7 +1,7 @@ using Microsoft.AspNetCore; using Microsoft.AspNetCore.Hosting; using Serilog; -using Serilog.Formatting.Elasticsearch; +using Serilog.Formatting.Json; namespace ConplementAG.CopsController { @@ -18,7 +18,7 @@ public static IWebHostBuilder CreateWebHostBuilder(string[] args) => { config.Enrich.FromLogContext(); config.MinimumLevel.Debug(); - config.WriteTo.Console(new ElasticsearchJsonFormatter()); + config.WriteTo.Console(new JsonFormatter()); }) .UseStartup(); } From 46ad394a336268972859fdb86678ab4fc7171e65 Mon Sep 17 00:00:00 2001 From: Denis Biondic Date: Tue, 12 Nov 2019 15:57:27 +0100 Subject: [PATCH 2/8] test: failing test for service account functionality --- examples/invalid-namespace-definition.yaml | 6 -- run_tests.sh | 62 +++++++++++++++ tests/0-test-rights.yaml | 26 +++++++ tests/1-valid-cns.yaml | 10 +++ tests/tests.sh | 90 ++++++++++++++++++++++ 5 files changed, 188 insertions(+), 6 deletions(-) delete mode 100644 examples/invalid-namespace-definition.yaml create mode 100755 run_tests.sh create mode 100644 tests/0-test-rights.yaml create mode 100644 tests/1-valid-cns.yaml create mode 100644 tests/tests.sh diff --git a/examples/invalid-namespace-definition.yaml b/examples/invalid-namespace-definition.yaml deleted file mode 100644 index 4f4a82f..0000000 --- a/examples/invalid-namespace-definition.yaml +++ /dev/null @@ -1,6 +0,0 @@ -apiVersion: coreops.conplement.cloud/v1 -kind: CopsNamespace -metadata: - name: invalid-specified-namespace -spec: - this: that \ No newline at end of file diff --git a/run_tests.sh b/run_tests.sh new file mode 100755 index 0000000..0d6476b --- /dev/null +++ b/run_tests.sh @@ -0,0 +1,62 @@ +#!/bin/bash + +set -eo pipefail +programname=$0 + +function usage { + echo "usage: $programname [-r repository] [-t tag]" + echo " MAKE SURE YOU SPECIFY THE ARGUMENTS IN THE EXACT ORDER AS BELOW, THIS SCRIPT DOES NOT SUPPORT OUT OF ORDER ARGUMENTS!" + echo " -r repository (MANDATORY) cops controller image repository. Should be accessible from the cluster (e.g. remote registry or local one shared with the cluster)." + echo " -t tag (MANDATORY) cops controller image tag." + echo " --install-helm (optional) use to install global helm in the cluster. Make sure you have Helm > 2.16 if you use this option and you are running on k8s > 1.16" + echo " " + echo "Prerequisites: " + echo " To run the tests, you need a running k8s cluster and docker engine" + echo " " + echo "Test cluster setup:" + echo " 1. For example, you can install Docker for Windows or Minikube, or use a remote cluster" + echo " Ubuntu instructions:" + echo " - install VirtualBox" + echo " - curl -Lo minikube https://storage.googleapis.com/minikube/releases/latest/minikube-linux-amd64 && chmod +x minikube" + echo " - sudo install minikube /usr/local/bin/" + echo " 2. The cluster has to have access to the docker registry with the pushed image. Either you push to a remote registry," + echo " or you can use something like minikube docker-env to share the images on your machine." + echo " Ubuntu with minikube instructions:" + echo " - minikube start" + echo " - eval $(minikube docker-env)" + echo " - docker build . -t cops-controller:latest" + echo " - ./tests.sh -r cops-controller -t latest --install-helm" + exit 1 +} + +exit_code=0 +initialContext=$(kubectl config current-context) + +# the cleanup function will be the exit point +cleanup () { + echo "Setting back the initial context $initialContext" + kubectl config use-context $initialContext + + exit $exit_code +} + +# mandatory params check +if [ $1 != "-r" ] || [ -z "$2" ] || [ $3 != "-t" ] || [ -z "$4" ]; then + usage +else + repository=$2 + tag=$4 + + # optional params check + if [ -n "$5" ] && [ $5 != "--install-helm" ]; then + usage + else + # register the cleanup function for all signal types (emulating finally block) + trap cleanup EXIT ERR INT TERM + + . ./tests/tests.sh $repository $tag $5 + + # set the exit_code with the real result, used when cleanup is called + exit_code=$? + fi +fi \ No newline at end of file diff --git a/tests/0-test-rights.yaml b/tests/0-test-rights.yaml new file mode 100644 index 0000000..050115a --- /dev/null +++ b/tests/0-test-rights.yaml @@ -0,0 +1,26 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: copsnamespace-create-role +rules: +- apiGroups: + - coreops.conplement.cloud + resources: + - copsnamespaces + verbs: + - list + - get + - create # no delete intentionally! +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: copsnamespace-create-binding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: copsnamespace-create-role +subjects: + - kind: ServiceAccount + name: {{SERVICE_ACCOUNT}} + namespace: {{SERVICE_ACCOUNT_NAMESPACE}} \ No newline at end of file diff --git a/tests/1-valid-cns.yaml b/tests/1-valid-cns.yaml new file mode 100644 index 0000000..6cf1d07 --- /dev/null +++ b/tests/1-valid-cns.yaml @@ -0,0 +1,10 @@ +apiVersion: coreops.conplement.cloud/v1 +kind: CopsNamespace +metadata: + name: test-one +spec: + namespaceAdminUsers: + - Test.User@conplement.de + - Second.User@conplement.de + namespaceAdminServiceAccounts: + - {{SERVICE_ACCOUNT}}.{{SERVICE_ACCOUNT_NAMESPACE}} \ No newline at end of file diff --git a/tests/tests.sh b/tests/tests.sh new file mode 100644 index 0000000..36793f4 --- /dev/null +++ b/tests/tests.sh @@ -0,0 +1,90 @@ +#!/bin/bash + +function fail { + echo $1 + exit 1 +} + +function setupCluster { + set -eo pipefail + + repository=$1 + tag=$2 + installHelm=$3 + + if [ -n "$3" ]; then + kubectl -n kube-system create serviceaccount tiller --dry-run=true -o yaml | kubectl apply -f - + kubectl create clusterrolebinding tiller --clusterrole=cluster-admin --serviceaccount=kube-system:tiller --dry-run=true -o yaml | kubectl apply -f - + helm init --service-account tiller --wait --upgrade + fi + + # ensure metacontroller dependency in the cluster + metacontrollerVersion="v0.4.0" + kubectl apply -f "https://raw.githubusercontent.com/GoogleCloudPlatform/metacontroller/${metacontrollerVersion}/manifests/metacontroller-namespace.yaml" + kubectl apply -f "https://raw.githubusercontent.com/GoogleCloudPlatform/metacontroller/${metacontrollerVersion}/manifests/metacontroller-rbac.yaml" + kubectl apply -f "https://raw.githubusercontent.com/GoogleCloudPlatform/metacontroller/${metacontrollerVersion}/manifests/metacontroller.yaml" + + # install cops controller via the local chart + copsControllerNamespace="coreops-cops-controller-component-test" + kubectl apply -f deployment/crds + + helm upgrade --install --wait --timeout 60 --namespace $copsControllerNamespace \ + --set image.repository=$1 \ + --set image.tag=$2 \ + cops-controller deployment/cops-controller +} + +function setupTestServiceAccount { + testAccount=$1 + testAccountNamespace=$2 + + # create account + kubectl create serviceaccount $testAccount --dry-run=true -o yaml | kubectl apply -f - + + # extract the secret, set into kubeconfig + serviceAccountSecret=$(kubectl get serviceaccount $testAccount -n $testAccountNamespace -o jsonpath={.secrets[0].name}) + token=$(kubectl get secret $serviceAccountSecret -n $testAccountNamespace -o jsonpath={.data.token} | base64 --decode) + kubectl config set-credentials $testAccount --token=$token + + # create the new kubeconfig context + kubectl config set-context $testAccount --user=$testAccount --cluster=$(kubectl config view --minify -o jsonpath={.clusters[0].name}) +} + +function should_deploy_valid_cns { + # Arrange + testAccount=$1 + testAccountNamespace=$2 + + # Act + cat "tests/1-valid-cns.yaml" | sed "s/{{SERVICE_ACCOUNT}}/$testAccount/g" \ + | sed "s/{{SERVICE_ACCOUNT_NAMESPACE}}/$testAccountNamespace/g" \ + | kubectl apply -f - + + # Assert + kubectl get ns test-one +} + +function should_update_valid_cns { + # Arrange & Act + kubectl apply -f tests/2-updated-cns.yaml + + # Assert +} + +setupCluster $1 $2 $3 + +testServiceAccount="cops-controller-test-user" +testServiceAccountNamespace="default" +setupTestServiceAccount $testServiceAccount $testServiceAccountNamespace + +# bind the test service account to the only right required for the cops-controller, which is currently the right +# to manage cops namespaces +cat "tests/0-test-rights.yaml" | sed "s/{{SERVICE_ACCOUNT}}/$testServiceAccount/g" \ + | sed "s/{{SERVICE_ACCOUNT_NAMESPACE}}/$testServiceAccountNamespace/g" \ + | kubectl apply -f - + +# switch to the test user +kubectl config use-context $testServiceAccount + +should_deploy_valid_cns $testServiceAccount $testServiceAccountNamespace +# should_update_valid_cns $testServiceAccount $testServiceAccountNamespace \ No newline at end of file From d2127f54af577120770161b028bac21bb3dba9e4 Mon Sep 17 00:00:00 2001 From: Denis Biondic Date: Tue, 12 Nov 2019 18:07:44 +0100 Subject: [PATCH 3/8] feat: service account support, rename of roles and bindings for clarity --- Models/CopsNamespace.cs | 3 + Models/K8sClusterRole.cs | 5 +- Models/K8sClusterRoleBinding.cs | 19 +++++-- Models/K8sRoleBinding.cs | 57 ++++++------------- Models/K8sSubjects.cs | 41 +++++++++++++ Services/K8sResourceFactory.cs | 6 +- ... 00-copsnamespace-create-clusterrole.yaml} | 4 +- ...=> 00-copsnamespace-user-clusterrole.yaml} | 2 +- deployment/crds/copsnamespace.crd.yaml | 11 +++- run_tests.sh | 2 +- 10 files changed, 95 insertions(+), 55 deletions(-) create mode 100644 Models/K8sSubjects.cs rename deployment/cops-controller/templates/{00-copsresource-inital-create-role.yaml => 00-copsnamespace-create-clusterrole.yaml} (68%) rename deployment/cops-controller/templates/{00-copsnamespace-full-access-role.yaml => 00-copsnamespace-user-clusterrole.yaml} (91%) diff --git a/Models/CopsNamespace.cs b/Models/CopsNamespace.cs index 0bbfbf6..1aa51b5 100644 --- a/Models/CopsNamespace.cs +++ b/Models/CopsNamespace.cs @@ -26,5 +26,8 @@ public partial class CopsSpec { [JsonProperty("namespaceAdminUsers")] public string[] NamespaceAdminUsers { get; set; } + + [JsonProperty("namespaceAdminServiceAccounts")] + public string[] NamespaceAdminServiceAccounts { get; set; } } } \ No newline at end of file diff --git a/Models/K8sClusterRole.cs b/Models/K8sClusterRole.cs index 485162e..c83f56b 100644 --- a/Models/K8sClusterRole.cs +++ b/Models/K8sClusterRole.cs @@ -18,11 +18,14 @@ public class K8sClusterRole public static K8sClusterRole CopsNamespaceEdit(string namespacename) { + // this is the cluster role to ensure copsnamespace edit rights are given to the admin users after the namespace is created + // We must do this with cluster role because we need to restrict this right only to the namespace that the user created, + // and since cops namespace is a cluster scoped resource, we must do it with a cluster role with a resource filter return new K8sClusterRole { Kind = "ClusterRole", ApiVersion = "rbac.authorization.k8s.io/v1", - Metadata = new K8sMetadata { Name = $"{namespacename}-copsnamespace-edit-role" }, + Metadata = new K8sMetadata { Name = $"copsnamespace-editor-{namespacename}" }, Rules = new K8sRule[] { new K8sRule(new[]{ "coreops.conplement.cloud" }, new[]{ "copsnamespaces" }, new[] { namespacename }, new[]{ "get", "list", "update", "patch", "delete" }) diff --git a/Models/K8sClusterRoleBinding.cs b/Models/K8sClusterRoleBinding.cs index 3250024..66e582d 100644 --- a/Models/K8sClusterRoleBinding.cs +++ b/Models/K8sClusterRoleBinding.cs @@ -20,15 +20,26 @@ public class K8sClusterRoleBinding [JsonProperty("roleRef")] public K8sRoleRef RoleRef { get; set; } - public static K8sClusterRoleBinding CopsNamespaceEditBinding(string namespacename, string[] users) + public static K8sClusterRoleBinding CopsNamespaceEditBinding(string namespacename, string[] users, string[] serviceAccounts) { + var subjects = users.ToList() + .Select(user => { return new K8sUserSubjectItem(user, "rbac.authorization.k8s.io"); }).ToList() + .Concat(serviceAccounts.ToList() + .Select(sa => + { + // TODO error handling + return new K8sServiceAccountSubjectItem(sa.Split(".")[0], sa.Split(".")[1]); + }).ToList() + ); + + // this is the concrete binding of admin users to the cops namespace edit role (which allows for the edit / delete of own namespaces) return new K8sClusterRoleBinding { Kind = "ClusterRoleBinding", ApiVersion = "rbac.authorization.k8s.io/v1", - Metadata = new K8sMetadata { Name = $"{namespacename}-copsnamespace-edit-clusterrolebinding" }, - RoleRef = new K8sRoleRef("ClusterRole", $"{namespacename}-copsnamespace-edit-role", "rbac.authorization.k8s.io"), - Subjects = users.ToList().Select(user => { return new K8sUserSubjectItem(user, "rbac.authorization.k8s.io"); }).ToList().ToArray() + Metadata = new K8sMetadata { Name = $"copsnamespace-editor-{namespacename}" }, + RoleRef = new K8sRoleRef("ClusterRole", $"copsnamespace-editor-{namespacename}", "rbac.authorization.k8s.io"), + Subjects = subjects.ToArray() }; } } diff --git a/Models/K8sRoleBinding.cs b/Models/K8sRoleBinding.cs index 16304d1..5f05fcc 100644 --- a/Models/K8sRoleBinding.cs +++ b/Models/K8sRoleBinding.cs @@ -20,7 +20,7 @@ public class K8sRoleBinding [JsonProperty("roleRef")] public K8sRoleRef RoleRef { get; set; } - public static K8sRoleBinding NamespaceFullAccess(string namespacename, string[] users) + public static K8sRoleBinding NamespaceFullAccess(string namespacename, string[] users, string[] serviceAccounts) { if (string.IsNullOrEmpty(namespacename)) { @@ -32,15 +32,27 @@ public static K8sRoleBinding NamespaceFullAccess(string namespacename, string[] throw new System.ArgumentNullException(nameof(users)); } + // this is the binding for admin users to get full access inside a cops namespace. The cluster role itself is + // not managed by the controller since it is a static resource deployed with rest of the static resources + // (check the Helm chart) var roleBinding = new K8sRoleBinding { Kind = "RoleBinding", ApiVersion = "rbac.authorization.k8s.io/v1", - Metadata = new K8sMetadata { Name = $"copsnamespace-full-access-rolebinding", Namespace = namespacename }, - RoleRef = new K8sRoleRef("ClusterRole", "copsnamespace-full-access-role", "rbac.authorization.k8s.io") + Metadata = new K8sMetadata { Name = $"copsnamespace-user", Namespace = namespacename }, + RoleRef = new K8sRoleRef("ClusterRole", "copsnamespace-user", "rbac.authorization.k8s.io") }; - var subjects = users.ToList().Select(user => { return new K8sUserSubjectItem(user, "rbac.authorization.k8s.io"); }).ToList(); + var subjects = users.ToList() + .Select(user => { return new K8sUserSubjectItem(user, "rbac.authorization.k8s.io"); }).ToList() + .Concat(serviceAccounts.ToList() + .Select(sa => + { + // TODO error handling + return new K8sServiceAccountSubjectItem(sa.Split(".")[0], sa.Split(".")[1]); + }).ToList() + ); + roleBinding.Subjects = subjects.ToArray(); return roleBinding; @@ -65,41 +77,4 @@ public K8sRoleRef(string kind, string name, string apigroup) ApiGroup = apigroup; } } - - public abstract class K8sSubjectBaseItem - { - [JsonProperty("kind")] - public string Kind { get; set; } - - [JsonProperty("name")] - public string Name { get; set; } - - public K8sSubjectBaseItem(string kind, string name) - { - Kind = kind; - Name = name; - } - } - - public class K8sUserSubjectItem : K8sSubjectBaseItem - { - [JsonProperty("apiGroup")] - public string ApiGroup { get; set; } - - public K8sUserSubjectItem(string name, string apigroup) : base("User", name) - { - ApiGroup = apigroup; - } - } - - public class K8sServiceAccountSubjectItem : K8sSubjectBaseItem - { - [JsonProperty("namespace")] - public string Namespace { get; set; } - - public K8sServiceAccountSubjectItem(string name, string @namespace) : base("ServiceAccount", name) - { - Namespace = @namespace; - } - } } diff --git a/Models/K8sSubjects.cs b/Models/K8sSubjects.cs new file mode 100644 index 0000000..bce10e0 --- /dev/null +++ b/Models/K8sSubjects.cs @@ -0,0 +1,41 @@ +using Newtonsoft.Json; + +namespace ConplementAG.CopsController.Models +{ + public abstract class K8sSubjectBaseItem + { + [JsonProperty("kind")] + public string Kind { get; set; } + + [JsonProperty("name")] + public string Name { get; set; } + + public K8sSubjectBaseItem(string kind, string name) + { + Kind = kind; + Name = name; + } + } + + public class K8sUserSubjectItem : K8sSubjectBaseItem + { + [JsonProperty("apiGroup")] + public string ApiGroup { get; set; } + + public K8sUserSubjectItem(string name, string apigroup) : base("User", name) + { + ApiGroup = apigroup; + } + } + + public class K8sServiceAccountSubjectItem : K8sSubjectBaseItem + { + [JsonProperty("namespace")] + public string Namespace { get; set; } + + public K8sServiceAccountSubjectItem(string name, string @namespace) : base("ServiceAccount", name) + { + Namespace = @namespace; + } + } +} diff --git a/Services/K8sResourceFactory.cs b/Services/K8sResourceFactory.cs index ec4ebff..112ffd6 100644 --- a/Services/K8sResourceFactory.cs +++ b/Services/K8sResourceFactory.cs @@ -15,14 +15,14 @@ public static IList Create(CopsResource resource) return (IList)method.Invoke(null, new[] { source }); } - // Methode used by reflection call + // Method used by reflection call private static IList Create(CopsNamespace copsNamespace) { return new List { new K8sNamespace(copsNamespace.Metadata.Name), - K8sRoleBinding.NamespaceFullAccess(copsNamespace.Metadata.Name, copsNamespace.Spec.NamespaceAdminUsers), - K8sClusterRoleBinding.CopsNamespaceEditBinding(copsNamespace.Metadata.Name, copsNamespace.Spec.NamespaceAdminUsers), + K8sRoleBinding.NamespaceFullAccess(copsNamespace.Metadata.Name, copsNamespace.Spec.NamespaceAdminUsers, copsNamespace.Spec.NamespaceAdminServiceAccounts), + K8sClusterRoleBinding.CopsNamespaceEditBinding(copsNamespace.Metadata.Name, copsNamespace.Spec.NamespaceAdminUsers, copsNamespace.Spec.NamespaceAdminServiceAccounts), K8sClusterRole.CopsNamespaceEdit(copsNamespace.Metadata.Name) }; } diff --git a/deployment/cops-controller/templates/00-copsresource-inital-create-role.yaml b/deployment/cops-controller/templates/00-copsnamespace-create-clusterrole.yaml similarity index 68% rename from deployment/cops-controller/templates/00-copsresource-inital-create-role.yaml rename to deployment/cops-controller/templates/00-copsnamespace-create-clusterrole.yaml index 3d6141c..24592d2 100644 --- a/deployment/cops-controller/templates/00-copsresource-inital-create-role.yaml +++ b/deployment/cops-controller/templates/00-copsnamespace-create-clusterrole.yaml @@ -1,8 +1,8 @@ kind: ClusterRole apiVersion: rbac.authorization.k8s.io/v1beta1 metadata: - name: copsnamespace-inital-create-role + name: copsnamespace-creator rules: - apiGroups: ["coreops.conplement.cloud"] - resources: ["CopsNamespace"] + resources: ["copsnamespaces"] verbs: ["get", "list", "create"] \ No newline at end of file diff --git a/deployment/cops-controller/templates/00-copsnamespace-full-access-role.yaml b/deployment/cops-controller/templates/00-copsnamespace-user-clusterrole.yaml similarity index 91% rename from deployment/cops-controller/templates/00-copsnamespace-full-access-role.yaml rename to deployment/cops-controller/templates/00-copsnamespace-user-clusterrole.yaml index 9376023..0fb9ea9 100644 --- a/deployment/cops-controller/templates/00-copsnamespace-full-access-role.yaml +++ b/deployment/cops-controller/templates/00-copsnamespace-user-clusterrole.yaml @@ -1,7 +1,7 @@ kind: ClusterRole apiVersion: rbac.authorization.k8s.io/v1beta1 metadata: - name: copsnamespace-full-access-role + name: copsnamespace-user rules: - apiGroups: ["rbac.authorization.k8s.io"] resources: [ "rolebindings", "roles" ] diff --git a/deployment/crds/copsnamespace.crd.yaml b/deployment/crds/copsnamespace.crd.yaml index be7a1ca..b2cc485 100644 --- a/deployment/crds/copsnamespace.crd.yaml +++ b/deployment/crds/copsnamespace.crd.yaml @@ -23,9 +23,16 @@ spec: properties: namespaceAdminUsers: description: |- - Array with all the namespace administrators. You need to reference the administrators by specifying + Array with all the namespace administrator users. You need to reference the administrators by specifying their Conplement Azure AD User IDs, which is usually in the form of - Firstname.Lastname@conplement.de (notice the capital letters!)" + Firstname.Lastname@conplement.de (notice the capital letters!) + type: array + items: + type: string + namespaceAdminServiceAccounts: + description: |- + Array with all the namespace administrator service accounts. Service accounts need to be specified in the + accountname.namespace string format! (for example, myaccount.default) type: array items: type: string \ No newline at end of file diff --git a/run_tests.sh b/run_tests.sh index 0d6476b..798c0b6 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -25,7 +25,7 @@ function usage { echo " - minikube start" echo " - eval $(minikube docker-env)" echo " - docker build . -t cops-controller:latest" - echo " - ./tests.sh -r cops-controller -t latest --install-helm" + echo " - ./run_tests.sh -r cops-controller -t latest --install-helm" exit 1 } From 686712bc664435b77991c1a819ae1b1de4279864 Mon Sep 17 00:00:00 2001 From: Denis Biondic Date: Wed, 13 Nov 2019 15:08:05 +0100 Subject: [PATCH 4/8] test: testing valid and invalid crd definitions, testing namespace rbac with updating --- examples/valid-namespace-definition.yaml | 8 - run_tests.sh | 49 +++++- tests/0-test-rights.yaml | 20 +-- tests/{1-valid-cns.yaml => 1-empire-cns.yaml} | 4 +- tests/2-updated-cns.yaml | 14 ++ tests/invalid-definitions/1.yaml | 6 + tests/invalid-definitions/2.yaml | 8 + tests/invalid-definitions/3.yaml | 10 ++ tests/tests.sh | 150 ++++++++++++++---- tests/valid-definitions/1.yaml | 13 ++ tests/valid-definitions/2.yaml | 10 ++ tests/valid-definitions/3.yaml | 31 ++++ 12 files changed, 265 insertions(+), 58 deletions(-) delete mode 100644 examples/valid-namespace-definition.yaml rename tests/{1-valid-cns.yaml => 1-empire-cns.yaml} (76%) create mode 100644 tests/2-updated-cns.yaml create mode 100644 tests/invalid-definitions/1.yaml create mode 100644 tests/invalid-definitions/2.yaml create mode 100644 tests/invalid-definitions/3.yaml create mode 100644 tests/valid-definitions/1.yaml create mode 100644 tests/valid-definitions/2.yaml create mode 100644 tests/valid-definitions/3.yaml diff --git a/examples/valid-namespace-definition.yaml b/examples/valid-namespace-definition.yaml deleted file mode 100644 index 7fa6ef3..0000000 --- a/examples/valid-namespace-definition.yaml +++ /dev/null @@ -1,8 +0,0 @@ -apiVersion: coreops.conplement.cloud/v1 -kind: CopsNamespace -metadata: - name: my-party-namespace -spec: - namespaceAdminUsers: - - Test.User@conplement.de - - Second.User@conplement.de \ No newline at end of file diff --git a/run_tests.sh b/run_tests.sh index 798c0b6..750fc55 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -3,6 +3,8 @@ set -eo pipefail programname=$0 +UNIVERSAL_TEST_IDENTIFIED="cops-controller-component-tests" + function usage { echo "usage: $programname [-r repository] [-t tag]" echo " MAKE SURE YOU SPECIFY THE ARGUMENTS IN THE EXACT ORDER AS BELOW, THIS SCRIPT DOES NOT SUPPORT OUT OF ORDER ARGUMENTS!" @@ -29,13 +31,51 @@ function usage { exit 1 } -exit_code=0 -initialContext=$(kubectl config current-context) +function colorecho { + RED="\033[0;31m" + GREEN="\033[0;32m" + YELLOW="\033[1;33m" + # ... ADD MORE COLORS + NC="\033[0m" # No Color + + printf "${!1}${2} ${NC}\n" +} # the cleanup function will be the exit point -cleanup () { +initialContext=$(kubectl config current-context) + +function cleanup { + exit_code=$? echo "Setting back the initial context $initialContext" + kubectl config use-context $initialContext + + echo "Deleting all cops namespaces for cleanup..." + + namespaces=$(kubectl get cns -l tests=$UNIVERSAL_TEST_IDENTIFIED -o name) + + if [ -n "$namespaces" ]; then + kubectl delete $namespaces --ignore-not-found + fi + + echo "Deleting all remaining RBAC rules..." + clusterRoles=$(kubectl get clusterroles -o name -l tests=$UNIVERSAL_TEST_IDENTIFIED) + clusterRoleBindings=$(kubectl get clusterrolebindings -o name -l tests=$UNIVERSAL_TEST_IDENTIFIED) + + if [ -n "$clusterRoles" ]; then + kubectl delete $clusterRoles --ignore-not-found + fi + + if [ -n "$clusterRoleBindings" ]; then + kubectl delete $clusterRoleBindings --ignore-not-found + fi + + if [ $exit_code != 0 ]; then + colorecho "RED" "Tests failed, check for the last error occured." + else + colorecho "GREEN" "All tests succeeded." + colorecho "GREEN" "Stdout could contain some failure statements, but this is because we pipe everything to stdout, even the failure checks where we expect to fail." + fi exit $exit_code } @@ -55,8 +95,5 @@ else trap cleanup EXIT ERR INT TERM . ./tests/tests.sh $repository $tag $5 - - # set the exit_code with the real result, used when cleanup is called - exit_code=$? fi fi \ No newline at end of file diff --git a/tests/0-test-rights.yaml b/tests/0-test-rights.yaml index 050115a..3f66561 100644 --- a/tests/0-test-rights.yaml +++ b/tests/0-test-rights.yaml @@ -1,25 +1,13 @@ apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: copsnamespace-create-role -rules: -- apiGroups: - - coreops.conplement.cloud - resources: - - copsnamespaces - verbs: - - list - - get - - create # no delete intentionally! ---- -apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding metadata: - name: copsnamespace-create-binding + name: {{BINDING_NAME}} + labels: + tests: cops-controller-component-tests roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole - name: copsnamespace-create-role + name: copsnamespace-creator subjects: - kind: ServiceAccount name: {{SERVICE_ACCOUNT}} diff --git a/tests/1-valid-cns.yaml b/tests/1-empire-cns.yaml similarity index 76% rename from tests/1-valid-cns.yaml rename to tests/1-empire-cns.yaml index 6cf1d07..d45e6d4 100644 --- a/tests/1-valid-cns.yaml +++ b/tests/1-empire-cns.yaml @@ -1,7 +1,9 @@ apiVersion: coreops.conplement.cloud/v1 kind: CopsNamespace metadata: - name: test-one + name: {{NAMESPACE}} + labels: + tests: cops-controller-component-tests spec: namespaceAdminUsers: - Test.User@conplement.de diff --git a/tests/2-updated-cns.yaml b/tests/2-updated-cns.yaml new file mode 100644 index 0000000..cc71895 --- /dev/null +++ b/tests/2-updated-cns.yaml @@ -0,0 +1,14 @@ +apiVersion: coreops.conplement.cloud/v1 +kind: CopsNamespace +metadata: + name: {{NAMESPACE}} + labels: + tests: cops-controller-component-tests +spec: + namespaceAdminUsers: + - Test.User@conplement.de + - Third.User@conplement.de # remove one user, add another two + - Fourth.User@conplement.de + namespaceAdminServiceAccounts: + - {{SERVICE_ACCOUNT}}.{{SERVICE_ACCOUNT_NAMESPACE}} + - {{ADDITIONAL_SERVICE_ACCOUNT}}.{{ADDITIONAL_SERVICE_ACCOUNT_NAMESPACE}} \ No newline at end of file diff --git a/tests/invalid-definitions/1.yaml b/tests/invalid-definitions/1.yaml new file mode 100644 index 0000000..2ddf23f --- /dev/null +++ b/tests/invalid-definitions/1.yaml @@ -0,0 +1,6 @@ +apiVersion: coreops.conplement.cloud/v1 +kind: CopsNamespace +metadata: + name: test-invalid-ns-1-without-spec + labels: + tests: cops-controller-component-tests \ No newline at end of file diff --git a/tests/invalid-definitions/2.yaml b/tests/invalid-definitions/2.yaml new file mode 100644 index 0000000..aa61d9a --- /dev/null +++ b/tests/invalid-definitions/2.yaml @@ -0,0 +1,8 @@ +apiVersion: coreops.conplement.cloud/v1 +kind: CopsNamespace +metadata: + name: test-invalid-ns-2-without-users + labels: + tests: cops-controller-component-tests +spec: + namespaceAdminUsers: \ No newline at end of file diff --git a/tests/invalid-definitions/3.yaml b/tests/invalid-definitions/3.yaml new file mode 100644 index 0000000..7a94d68 --- /dev/null +++ b/tests/invalid-definitions/3.yaml @@ -0,0 +1,10 @@ +apiVersion: coreops.conplement.cloud/v1 +kind: CopsNamespace +metadata: + name: test-invalid-ns-3-only-with-sas + labels: + tests: cops-controller-component-tests +spec: + namespaceAdminServiceAccounts: + - account1.default + - account2.default \ No newline at end of file diff --git a/tests/tests.sh b/tests/tests.sh index 36793f4..c166b64 100644 --- a/tests/tests.sh +++ b/tests/tests.sh @@ -1,13 +1,27 @@ #!/bin/bash +set -eo pipefail function fail { - echo $1 + colorecho "RED" "$1" exit 1 } -function setupCluster { - set -eo pipefail +function success { + colorecho "GREEN" "$1" +} + +######################################################################### +# Globals # +######################################################################### + +serviceAccountsNamespace="default" +darthVaderAccount="cops-controller-darth-vader" +kyloRenAccount="cops-controller-kylo-ren" +######################################################################### +# Arrange helpers # +######################################################################### +function setupCluster { repository=$1 tag=$2 installHelm=$3 @@ -34,12 +48,17 @@ function setupCluster { cops-controller deployment/cops-controller } -function setupTestServiceAccount { +function setupTheEmpireServiceAccounts { + setupServiceAccount $darthVaderAccount $serviceAccountsNamespace + setupServiceAccount $kyloRenAccount $serviceAccountsNamespace +} + +function setupServiceAccount { testAccount=$1 testAccountNamespace=$2 # create account - kubectl create serviceaccount $testAccount --dry-run=true -o yaml | kubectl apply -f - + kubectl create serviceaccount $testAccount -n $testAccountNamespace --dry-run=true -o yaml | kubectl apply -f - # extract the secret, set into kubeconfig serviceAccountSecret=$(kubectl get serviceaccount $testAccount -n $testAccountNamespace -o jsonpath={.secrets[0].name}) @@ -48,43 +67,120 @@ function setupTestServiceAccount { # create the new kubeconfig context kubectl config set-context $testAccount --user=$testAccount --cluster=$(kubectl config view --minify -o jsonpath={.clusters[0].name}) + + # bind the test service account to initial rights as the users have them + cat "tests/0-test-rights.yaml" | sed "s/{{SERVICE_ACCOUNT}}/$testAccount/g" \ + | sed "s/{{SERVICE_ACCOUNT_NAMESPACE}}/$testAccountNamespace/g" \ + | sed "s/{{BINDING_NAME}}/copsnamespace-creator-binding-$testAccount/g" \ + | kubectl apply -f - +} + +######################################################################### +# Assert helpers # +######################################################################### +function ensureAccessToNamespace { + namespaceName=$1 + + # we have to retry here because it might take a second or two to create the namespace + n=1 + until [ $n -ge 15 ] + do + echo "Attempting to list basic namespace resources... Attempt $n out of 15." + kubectl get pods,svc,deploy -n $namespaceName && break + n=$[$n+1] + sleep 2 + done + + # last check after all attempts so that we can pipe the failure correctly + kubectl get pods,svc,deploy -n $namespaceName || fail "It was expected that the namespace setup is completed at this point." +} + +######################################################################### +# The Tests # +######################################################################### + +function test_validDefinitions { + kubectl apply -f tests/valid-definitions +} + +function test_invalidDefinitions { + hasFailed="no" + + kubectl apply -f tests/invalid-definitions || hasFailed="yes" + + if [ $hasFailed == "no" ]; then + fail "Some of the invalid definition succeeded, which was unexpected!" + fi } -function should_deploy_valid_cns { +# Tests following business cases: +# - user can create a cops namespace and gain rights inside it +# - all other users are denied access +function test_shouldDeployEmpireCnsWithValidRbac { # Arrange - testAccount=$1 - testAccountNamespace=$2 + kubectl config use-context $darthVaderAccount + namespaceName="empire" # Act - cat "tests/1-valid-cns.yaml" | sed "s/{{SERVICE_ACCOUNT}}/$testAccount/g" \ - | sed "s/{{SERVICE_ACCOUNT_NAMESPACE}}/$testAccountNamespace/g" \ + cat "tests/1-empire-cns.yaml" | sed "s/{{SERVICE_ACCOUNT}}/$darthVaderAccount/g" \ + | sed "s/{{SERVICE_ACCOUNT_NAMESPACE}}/$serviceAccountsNamespace/g" \ + | sed "s/{{NAMESPACE}}/$namespaceName/g" \ | kubectl apply -f - # Assert - kubectl get ns test-one + ensureAccessToNamespace $namespaceName + + # no access for other accounts + kubectl config use-context $kyloRenAccount + + authOutput=$(kubectl auth can-i get pods -n $namespaceName) || success "Kylo ren didn't have access, which was expected :)" + + if [ $authOutput != "no" ]; then + fail "Listing pods as kylo ren should have failed!" + fi } -function should_update_valid_cns { - # Arrange & Act - kubectl apply -f tests/2-updated-cns.yaml +# Tests following business cases: +# - namespace admin user edit the namespace and extend it with additional rbac +function test_shouldUpdateEmpireCnsWithAdditionalRbac { + # Arrange + kubectl config use-context $darthVaderAccount + namespaceName="empire" + + # Act + cat "tests/2-updated-cns.yaml" | sed "s/{{SERVICE_ACCOUNT}}/$darthVaderAccount/g" \ + | sed "s/{{SERVICE_ACCOUNT_NAMESPACE}}/$testAccountNamespace/g" \ + | sed "s/{{ADDITIONAL_SERVICE_ACCOUNT_NAMESPACE}}/$testAccountNamespace/g" \ + | sed "s/{{ADDITIONAL_SERVICE_ACCOUNT}}/$kyloRenAccount/g" \ + | sed "s/{{NAMESPACE}}/$namespaceName/g" \ + | kubectl apply -f - # Assert + ensureAccessToNamespace $namespaceName # darth should still have access + + # but kylo too + kubectl config use-context $kyloRenAccount + ensureAccessToNamespace $namespaceName + + # to check the changes for user accounts, we can only ensure changes were done in RBAC. Real + # RBAC testing we cannot do because we cannot impersonate user accounts + } -setupCluster $1 $2 $3 +######################################################################### +# Test runner # +######################################################################### -testServiceAccount="cops-controller-test-user" -testServiceAccountNamespace="default" -setupTestServiceAccount $testServiceAccount $testServiceAccountNamespace +setupCluster $1 $2 $3 -# bind the test service account to the only right required for the cops-controller, which is currently the right -# to manage cops namespaces -cat "tests/0-test-rights.yaml" | sed "s/{{SERVICE_ACCOUNT}}/$testServiceAccount/g" \ - | sed "s/{{SERVICE_ACCOUNT_NAMESPACE}}/$testServiceAccountNamespace/g" \ - | kubectl apply -f - +setupTheEmpireServiceAccounts -# switch to the test user -kubectl config use-context $testServiceAccount +# now we can run all the tests +test_validDefinitions +test_invalidDefinitions +test_shouldDeployEmpireCnsWithValidRbac +test_shouldUpdateEmpireCnsWithAdditionalRbac -should_deploy_valid_cns $testServiceAccount $testServiceAccountNamespace -# should_update_valid_cns $testServiceAccount $testServiceAccountNamespace \ No newline at end of file +# error handling, invalid values -> check TODOs in code +# test default peremissions, no access to default or kube-system +# integrate into ci/cd diff --git a/tests/valid-definitions/1.yaml b/tests/valid-definitions/1.yaml new file mode 100644 index 0000000..2d3d0c5 --- /dev/null +++ b/tests/valid-definitions/1.yaml @@ -0,0 +1,13 @@ +apiVersion: coreops.conplement.cloud/v1 +kind: CopsNamespace +metadata: + name: test-valid-ns-1-typical-with-non-existing-sa + labels: + tests: cops-controller-component-tests +spec: + namespaceAdminUsers: + - Test.User@conplement.de + - Second.User@conplement.de + namespaceAdminServiceAccounts: + - default.default # should work with existing sa + - notyetthere.default # but also with non-existing \ No newline at end of file diff --git a/tests/valid-definitions/2.yaml b/tests/valid-definitions/2.yaml new file mode 100644 index 0000000..4530551 --- /dev/null +++ b/tests/valid-definitions/2.yaml @@ -0,0 +1,10 @@ +apiVersion: coreops.conplement.cloud/v1 +kind: CopsNamespace +metadata: + name: test-valid-ns-2-only-users + labels: + tests: cops-controller-component-tests +spec: + namespaceAdminUsers: # should work without service account which are optional + - Test.User@conplement.de + - Second.User@conplement.de \ No newline at end of file diff --git a/tests/valid-definitions/3.yaml b/tests/valid-definitions/3.yaml new file mode 100644 index 0000000..2cbd37a --- /dev/null +++ b/tests/valid-definitions/3.yaml @@ -0,0 +1,31 @@ +apiVersion: coreops.conplement.cloud/v1 +kind: CopsNamespace +metadata: + name: test-valid-ns-3-lot-of-users-and-sa + labels: + tests: cops-controller-component-tests +spec: + namespaceAdminUsers: # should work with a lot of users and service accounts + - Test.User@conplement.de + - Second.User@conplement.de + - Third.User@conplement.de + - Fourth.User@conplement.de + - Fifth.User@conplement.de + - Sixth.User@conplement.de + - Seventh.User@conplement.de + - Eight.User@conplement.de + - Ninth.User@conplement.de + - Tenth.User@conplement.de + - Eleventh.User@conplement.de + namespaceAdminServiceAccounts: + - account1.default + - account2.default + - account3.default + - account4.default + - account5.default + - account6.default + - account1.custom + - account2.custom + - account3.custom + - account4.custom + - account5.custom \ No newline at end of file From e4efdfa5df3aafcab85a12d85daa6a692b4413ad Mon Sep 17 00:00:00 2001 From: Denis Biondic Date: Wed, 13 Nov 2019 15:37:01 +0100 Subject: [PATCH 5/8] test: helm and cops controller setup now optional to support running tests in prod --- run_tests.sh | 42 ++++++++++++++++++++++-------------------- tests/tests.sh | 36 ++++++++++++++++++------------------ 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/run_tests.sh b/run_tests.sh index 750fc55..9ed3e84 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -6,11 +6,11 @@ programname=$0 UNIVERSAL_TEST_IDENTIFIED="cops-controller-component-tests" function usage { - echo "usage: $programname [-r repository] [-t tag]" + echo "usage: $programname [--install-helm-and-cops-controller] [-r repository] [-t tag]" echo " MAKE SURE YOU SPECIFY THE ARGUMENTS IN THE EXACT ORDER AS BELOW, THIS SCRIPT DOES NOT SUPPORT OUT OF ORDER ARGUMENTS!" - echo " -r repository (MANDATORY) cops controller image repository. Should be accessible from the cluster (e.g. remote registry or local one shared with the cluster)." - echo " -t tag (MANDATORY) cops controller image tag." - echo " --install-helm (optional) use to install global helm in the cluster. Make sure you have Helm > 2.16 if you use this option and you are running on k8s > 1.16" + echo " --install-helm-and-cops-controller (optional) use to install global helm in the cluster and to deploy the cops controller specified by the -r and -t arguments. Make sure you have Helm > 2.16 if you use this option and you are running on k8s > 1.16" + echo " -r repository (optional) cops controller image repository. Should be accessible from the cluster (e.g. remote registry or local one shared with the cluster)." + echo " -t tag (optional) cops controller image tag." echo " " echo "Prerequisites: " echo " To run the tests, you need a running k8s cluster and docker engine" @@ -25,9 +25,9 @@ function usage { echo " or you can use something like minikube docker-env to share the images on your machine." echo " Ubuntu with minikube instructions:" echo " - minikube start" - echo " - eval $(minikube docker-env)" + echo " - import variables with eval from: minikube docker-env" echo " - docker build . -t cops-controller:latest" - echo " - ./run_tests.sh -r cops-controller -t latest --install-helm" + echo " - ./run_tests.sh --install-helm-and-cops-controller -r cops-controller -t latest" exit 1 } @@ -80,20 +80,22 @@ function cleanup { exit $exit_code } -# mandatory params check -if [ $1 != "-r" ] || [ -z "$2" ] || [ $3 != "-t" ] || [ -z "$4" ]; then - usage -else - repository=$2 - tag=$4 - - # optional params check - if [ -n "$5" ] && [ $5 != "--install-helm" ]; then +installController="no" +repository="" +tag="" + +if [ -n "$1" ]; then # if any argument specified + # all parameters mandatory now, in correct order + if [ $1 != "--install-helm-and-cops-controller" ] || [ $2 != "-r" ] || [ -z "$3" ] || [ $4 != "-t" ] || [ -z "$5" ]; then usage else - # register the cleanup function for all signal types (emulating finally block) - trap cleanup EXIT ERR INT TERM + installController="yes" + repository=$3 + tag=$5 + fi +fi + +# register the cleanup function for all signal types (emulating finally block) +trap cleanup EXIT ERR INT TERM - . ./tests/tests.sh $repository $tag $5 - fi -fi \ No newline at end of file +. ./tests/tests.sh "$installController" "$repository" "$tag" \ No newline at end of file diff --git a/tests/tests.sh b/tests/tests.sh index c166b64..2cb2e3e 100644 --- a/tests/tests.sh +++ b/tests/tests.sh @@ -22,30 +22,30 @@ kyloRenAccount="cops-controller-kylo-ren" # Arrange helpers # ######################################################################### function setupCluster { - repository=$1 - tag=$2 - installHelm=$3 + installController=$1 + repository=$2 + tag=$3 - if [ -n "$3" ]; then + if [ $installController == "yes" ]; then kubectl -n kube-system create serviceaccount tiller --dry-run=true -o yaml | kubectl apply -f - kubectl create clusterrolebinding tiller --clusterrole=cluster-admin --serviceaccount=kube-system:tiller --dry-run=true -o yaml | kubectl apply -f - helm init --service-account tiller --wait --upgrade - fi - # ensure metacontroller dependency in the cluster - metacontrollerVersion="v0.4.0" - kubectl apply -f "https://raw.githubusercontent.com/GoogleCloudPlatform/metacontroller/${metacontrollerVersion}/manifests/metacontroller-namespace.yaml" - kubectl apply -f "https://raw.githubusercontent.com/GoogleCloudPlatform/metacontroller/${metacontrollerVersion}/manifests/metacontroller-rbac.yaml" - kubectl apply -f "https://raw.githubusercontent.com/GoogleCloudPlatform/metacontroller/${metacontrollerVersion}/manifests/metacontroller.yaml" + # ensure metacontroller dependency in the cluster + metacontrollerVersion="v0.4.0" + kubectl apply -f "https://raw.githubusercontent.com/GoogleCloudPlatform/metacontroller/${metacontrollerVersion}/manifests/metacontroller-namespace.yaml" + kubectl apply -f "https://raw.githubusercontent.com/GoogleCloudPlatform/metacontroller/${metacontrollerVersion}/manifests/metacontroller-rbac.yaml" + kubectl apply -f "https://raw.githubusercontent.com/GoogleCloudPlatform/metacontroller/${metacontrollerVersion}/manifests/metacontroller.yaml" - # install cops controller via the local chart - copsControllerNamespace="coreops-cops-controller-component-test" - kubectl apply -f deployment/crds + # install cops controller via the local chart + copsControllerNamespace="coreops-cops-controller-component-test" + kubectl apply -f deployment/crds - helm upgrade --install --wait --timeout 60 --namespace $copsControllerNamespace \ - --set image.repository=$1 \ - --set image.tag=$2 \ - cops-controller deployment/cops-controller + helm upgrade --install --wait --timeout 60 --namespace $copsControllerNamespace \ + --set image.repository=$repository \ + --set image.tag=$tag \ + cops-controller deployment/cops-controller + fi } function setupTheEmpireServiceAccounts { @@ -171,7 +171,7 @@ function test_shouldUpdateEmpireCnsWithAdditionalRbac { # Test runner # ######################################################################### -setupCluster $1 $2 $3 +setupCluster "$1" "$2" "$3" setupTheEmpireServiceAccounts From 7cc55dc6d277cda14f0b0243fb0bb66f55634877 Mon Sep 17 00:00:00 2001 From: Denis Biondic Date: Thu, 14 Nov 2019 09:22:18 +0100 Subject: [PATCH 6/8] test: additional rbac tests --- ...00-copsnamespace-creator-clusterrole.yaml} | 0 run_tests.sh | 6 +-- tests/1-empire-cns.yaml | 2 +- tests/2-updated-cns.yaml | 2 +- tests/tests.sh | 46 +++++++++++++++++-- 5 files changed, 46 insertions(+), 10 deletions(-) rename deployment/cops-controller/templates/{00-copsnamespace-create-clusterrole.yaml => 00-copsnamespace-creator-clusterrole.yaml} (100%) diff --git a/deployment/cops-controller/templates/00-copsnamespace-create-clusterrole.yaml b/deployment/cops-controller/templates/00-copsnamespace-creator-clusterrole.yaml similarity index 100% rename from deployment/cops-controller/templates/00-copsnamespace-create-clusterrole.yaml rename to deployment/cops-controller/templates/00-copsnamespace-creator-clusterrole.yaml diff --git a/run_tests.sh b/run_tests.sh index 9ed3e84..011d39c 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -42,13 +42,13 @@ function colorecho { } # the cleanup function will be the exit point -initialContext=$(kubectl config current-context) +INITIAL_CONTEXT=$(kubectl config current-context) function cleanup { exit_code=$? - echo "Setting back the initial context $initialContext" + echo "Setting back the initial context $INITIAL_CONTEXT" - kubectl config use-context $initialContext + kubectl config use-context $INITIAL_CONTEXT echo "Deleting all cops namespaces for cleanup..." diff --git a/tests/1-empire-cns.yaml b/tests/1-empire-cns.yaml index d45e6d4..dd8d179 100644 --- a/tests/1-empire-cns.yaml +++ b/tests/1-empire-cns.yaml @@ -6,7 +6,7 @@ metadata: tests: cops-controller-component-tests spec: namespaceAdminUsers: - - Test.User@conplement.de + - First.User@conplement.de - Second.User@conplement.de namespaceAdminServiceAccounts: - {{SERVICE_ACCOUNT}}.{{SERVICE_ACCOUNT_NAMESPACE}} \ No newline at end of file diff --git a/tests/2-updated-cns.yaml b/tests/2-updated-cns.yaml index cc71895..c023e37 100644 --- a/tests/2-updated-cns.yaml +++ b/tests/2-updated-cns.yaml @@ -6,7 +6,7 @@ metadata: tests: cops-controller-component-tests spec: namespaceAdminUsers: - - Test.User@conplement.de + - First.User@conplement.de - Third.User@conplement.de # remove one user, add another two - Fourth.User@conplement.de namespaceAdminServiceAccounts: diff --git a/tests/tests.sh b/tests/tests.sh index 2cb2e3e..19e6f6d 100644 --- a/tests/tests.sh +++ b/tests/tests.sh @@ -10,6 +10,10 @@ function success { colorecho "GREEN" "$1" } +function logTestStarted { + colorecho "YELLOW" "$1" +} + ######################################################################### # Globals # ######################################################################### @@ -99,11 +103,33 @@ function ensureAccessToNamespace { # The Tests # ######################################################################### +function test_noAccessExceptCnsGetListCreate { + logTestStarted ${FUNCNAME[0]} + + kubectl config use-context $darthVaderAccount + + authOutput=$(kubectl auth can-i get pods -n default) || success "No access in default, good" + + if [ $authOutput != "no" ]; then + fail "Listing pods in default as darth vader should have failed!" + fi + + authOutput=$(kubectl auth can-i get pods -n kube-system) || success "No access in kube-system, good" + + if [ $authOutput != "no" ]; then + fail "Listing pods in kube-system as darth vader should have failed!" + fi +} + function test_validDefinitions { + logTestStarted ${FUNCNAME[0]} + kubectl apply -f tests/valid-definitions } function test_invalidDefinitions { + logTestStarted ${FUNCNAME[0]} + hasFailed="no" kubectl apply -f tests/invalid-definitions || hasFailed="yes" @@ -117,6 +143,8 @@ function test_invalidDefinitions { # - user can create a cops namespace and gain rights inside it # - all other users are denied access function test_shouldDeployEmpireCnsWithValidRbac { + logTestStarted ${FUNCNAME[0]} + # Arrange kubectl config use-context $darthVaderAccount namespaceName="empire" @@ -143,6 +171,8 @@ function test_shouldDeployEmpireCnsWithValidRbac { # Tests following business cases: # - namespace admin user edit the namespace and extend it with additional rbac function test_shouldUpdateEmpireCnsWithAdditionalRbac { + logTestStarted ${FUNCNAME[0]} + # Arrange kubectl config use-context $darthVaderAccount namespaceName="empire" @@ -164,7 +194,14 @@ function test_shouldUpdateEmpireCnsWithAdditionalRbac { # to check the changes for user accounts, we can only ensure changes were done in RBAC. Real # RBAC testing we cannot do because we cannot impersonate user accounts + kubectl config use-context $INITIAL_CONTEXT + users=$(kubectl get clusterrolebinding copsnamespace-editor-empire -o jsonpath={.subjects[*].name} --ignore-not-found) + echo "Found users: $users" + + if [[ $users != *"First.User"* ]] || [[ $users == *"Second.User"* ]] || [[ $users != *"Fourth.User"* ]] || [[ $users != *"Third.User"* ]]; then + fail "Users were not updated correctly." + fi } ######################################################################### @@ -176,11 +213,10 @@ setupCluster "$1" "$2" "$3" setupTheEmpireServiceAccounts # now we can run all the tests +test_noAccessExceptCnsGetListCreate # this test has to run first + +# following tests should run in any order test_validDefinitions test_invalidDefinitions test_shouldDeployEmpireCnsWithValidRbac -test_shouldUpdateEmpireCnsWithAdditionalRbac - -# error handling, invalid values -> check TODOs in code -# test default peremissions, no access to default or kube-system -# integrate into ci/cd +test_shouldUpdateEmpireCnsWithAdditionalRbac \ No newline at end of file From 0f5cc8fb8a412cc6f2e91f547c0031621bb0a53c Mon Sep 17 00:00:00 2001 From: Denis Biondic Date: Thu, 14 Nov 2019 14:10:11 +0100 Subject: [PATCH 7/8] feat: strong validation of service account schema using crd validation --- Models/CopsNamespace.cs | 11 ++++++++- Models/K8sClusterRoleBinding.cs | 5 ++--- Models/K8sRoleBinding.cs | 7 +++--- deployment/crds/copsnamespace.crd.yaml | 15 ++++++++++--- tests/1-empire-cns.yaml | 3 ++- tests/2-updated-cns.yaml | 6 +++-- tests/invalid-definitions/3.yaml | 6 +++-- tests/invalid-definitions/4.yaml | 11 +++++++++ tests/invalid-definitions/5.yaml | 11 +++++++++ tests/invalid-definitions/6.yaml | 11 +++++++++ tests/invalid-definitions/7.yaml | 11 +++++++++ tests/tests.sh | 22 +++++++++++++----- tests/valid-definitions/1.yaml | 6 +++-- tests/valid-definitions/3.yaml | 31 +++++++++++++++++--------- 14 files changed, 122 insertions(+), 34 deletions(-) create mode 100644 tests/invalid-definitions/4.yaml create mode 100644 tests/invalid-definitions/5.yaml create mode 100644 tests/invalid-definitions/6.yaml create mode 100644 tests/invalid-definitions/7.yaml diff --git a/Models/CopsNamespace.cs b/Models/CopsNamespace.cs index 1aa51b5..41170c3 100644 --- a/Models/CopsNamespace.cs +++ b/Models/CopsNamespace.cs @@ -28,6 +28,15 @@ public partial class CopsSpec public string[] NamespaceAdminUsers { get; set; } [JsonProperty("namespaceAdminServiceAccounts")] - public string[] NamespaceAdminServiceAccounts { get; set; } + public CopsAdminServiceAccountSpec[] NamespaceAdminServiceAccounts { get; set; } + } + + public class CopsAdminServiceAccountSpec + { + [JsonProperty("serviceAccount")] + public string ServiceAccount { get; set; } + + [JsonProperty("namespace")] + public string Namespace { get; set; } } } \ No newline at end of file diff --git a/Models/K8sClusterRoleBinding.cs b/Models/K8sClusterRoleBinding.cs index 66e582d..037d51c 100644 --- a/Models/K8sClusterRoleBinding.cs +++ b/Models/K8sClusterRoleBinding.cs @@ -20,15 +20,14 @@ public class K8sClusterRoleBinding [JsonProperty("roleRef")] public K8sRoleRef RoleRef { get; set; } - public static K8sClusterRoleBinding CopsNamespaceEditBinding(string namespacename, string[] users, string[] serviceAccounts) + public static K8sClusterRoleBinding CopsNamespaceEditBinding(string namespacename, string[] users, CopsAdminServiceAccountSpec[] serviceAccounts) { var subjects = users.ToList() .Select(user => { return new K8sUserSubjectItem(user, "rbac.authorization.k8s.io"); }).ToList() .Concat(serviceAccounts.ToList() .Select(sa => { - // TODO error handling - return new K8sServiceAccountSubjectItem(sa.Split(".")[0], sa.Split(".")[1]); + return new K8sServiceAccountSubjectItem(sa.ServiceAccount, sa.Namespace); }).ToList() ); diff --git a/Models/K8sRoleBinding.cs b/Models/K8sRoleBinding.cs index 5f05fcc..5abd896 100644 --- a/Models/K8sRoleBinding.cs +++ b/Models/K8sRoleBinding.cs @@ -20,7 +20,7 @@ public class K8sRoleBinding [JsonProperty("roleRef")] public K8sRoleRef RoleRef { get; set; } - public static K8sRoleBinding NamespaceFullAccess(string namespacename, string[] users, string[] serviceAccounts) + public static K8sRoleBinding NamespaceFullAccess(string namespacename, string[] users, CopsAdminServiceAccountSpec[] serviceAccounts) { if (string.IsNullOrEmpty(namespacename)) { @@ -47,9 +47,8 @@ public static K8sRoleBinding NamespaceFullAccess(string namespacename, string[] .Select(user => { return new K8sUserSubjectItem(user, "rbac.authorization.k8s.io"); }).ToList() .Concat(serviceAccounts.ToList() .Select(sa => - { - // TODO error handling - return new K8sServiceAccountSubjectItem(sa.Split(".")[0], sa.Split(".")[1]); + { + return new K8sServiceAccountSubjectItem(sa.ServiceAccount, sa.Namespace); }).ToList() ); diff --git a/deployment/crds/copsnamespace.crd.yaml b/deployment/crds/copsnamespace.crd.yaml index b2cc485..b4c9162 100644 --- a/deployment/crds/copsnamespace.crd.yaml +++ b/deployment/crds/copsnamespace.crd.yaml @@ -31,8 +31,17 @@ spec: type: string namespaceAdminServiceAccounts: description: |- - Array with all the namespace administrator service accounts. Service accounts need to be specified in the - accountname.namespace string format! (for example, myaccount.default) + Array with all the namespace administrator service accounts. type: array items: - type: string \ No newline at end of file + type: object + required: + - serviceAccount + - namespace + properties: + serviceAccount: + description: |- + Service account name + namespace: + description: |- + Namespace where the service account is located \ No newline at end of file diff --git a/tests/1-empire-cns.yaml b/tests/1-empire-cns.yaml index dd8d179..4ad482e 100644 --- a/tests/1-empire-cns.yaml +++ b/tests/1-empire-cns.yaml @@ -9,4 +9,5 @@ spec: - First.User@conplement.de - Second.User@conplement.de namespaceAdminServiceAccounts: - - {{SERVICE_ACCOUNT}}.{{SERVICE_ACCOUNT_NAMESPACE}} \ No newline at end of file + - serviceAccount: {{SERVICE_ACCOUNT}} + namespace: {{SERVICE_ACCOUNT_NAMESPACE}} \ No newline at end of file diff --git a/tests/2-updated-cns.yaml b/tests/2-updated-cns.yaml index c023e37..2825b66 100644 --- a/tests/2-updated-cns.yaml +++ b/tests/2-updated-cns.yaml @@ -10,5 +10,7 @@ spec: - Third.User@conplement.de # remove one user, add another two - Fourth.User@conplement.de namespaceAdminServiceAccounts: - - {{SERVICE_ACCOUNT}}.{{SERVICE_ACCOUNT_NAMESPACE}} - - {{ADDITIONAL_SERVICE_ACCOUNT}}.{{ADDITIONAL_SERVICE_ACCOUNT_NAMESPACE}} \ No newline at end of file + - serviceAccount: {{SERVICE_ACCOUNT}} + namespace: {{SERVICE_ACCOUNT_NAMESPACE}} + - serviceAccount: {{ADDITIONAL_SERVICE_ACCOUNT}} + namespace: {{ADDITIONAL_SERVICE_ACCOUNT_NAMESPACE}} \ No newline at end of file diff --git a/tests/invalid-definitions/3.yaml b/tests/invalid-definitions/3.yaml index 7a94d68..adcfbfe 100644 --- a/tests/invalid-definitions/3.yaml +++ b/tests/invalid-definitions/3.yaml @@ -6,5 +6,7 @@ metadata: tests: cops-controller-component-tests spec: namespaceAdminServiceAccounts: - - account1.default - - account2.default \ No newline at end of file + - serviceAccount: account1 + namespace: custom + - serviceAccount: account2 + namespace: custom \ No newline at end of file diff --git a/tests/invalid-definitions/4.yaml b/tests/invalid-definitions/4.yaml new file mode 100644 index 0000000..088eddc --- /dev/null +++ b/tests/invalid-definitions/4.yaml @@ -0,0 +1,11 @@ +apiVersion: coreops.conplement.cloud/v1 +kind: CopsNamespace +metadata: + name: test-invalid-ns-4-with-invalid-sa-formats-1 + labels: + tests: cops-controller-component-tests +spec: + namespaceAdminUsers: + - User@conplement.de + namespaceAdminServiceAccounts: + - account1 \ No newline at end of file diff --git a/tests/invalid-definitions/5.yaml b/tests/invalid-definitions/5.yaml new file mode 100644 index 0000000..36fa761 --- /dev/null +++ b/tests/invalid-definitions/5.yaml @@ -0,0 +1,11 @@ +apiVersion: coreops.conplement.cloud/v1 +kind: CopsNamespace +metadata: + name: test-invalid-ns-5-with-invalid-sa-formats-2 + labels: + tests: cops-controller-component-tests +spec: + namespaceAdminUsers: + - User@conplement.de + namespaceAdminServiceAccounts: + - account1.namespace \ No newline at end of file diff --git a/tests/invalid-definitions/6.yaml b/tests/invalid-definitions/6.yaml new file mode 100644 index 0000000..26cb1b8 --- /dev/null +++ b/tests/invalid-definitions/6.yaml @@ -0,0 +1,11 @@ +apiVersion: coreops.conplement.cloud/v1 +kind: CopsNamespace +metadata: + name: test-invalid-ns-6-with-invalid-sa-formats-3 + labels: + tests: cops-controller-component-tests +spec: + namespaceAdminUsers: + - User@conplement.de + namespaceAdminServiceAccounts: + - serviceAccount: test \ No newline at end of file diff --git a/tests/invalid-definitions/7.yaml b/tests/invalid-definitions/7.yaml new file mode 100644 index 0000000..04c29c4 --- /dev/null +++ b/tests/invalid-definitions/7.yaml @@ -0,0 +1,11 @@ +apiVersion: coreops.conplement.cloud/v1 +kind: CopsNamespace +metadata: + name: test-invalid-ns-7-with-invalid-sa-formats-4 + labels: + tests: cops-controller-component-tests +spec: + namespaceAdminUsers: + - User@conplement.de + namespaceAdminServiceAccounts: + - namespace: test \ No newline at end of file diff --git a/tests/tests.sh b/tests/tests.sh index 19e6f6d..99be386 100644 --- a/tests/tests.sh +++ b/tests/tests.sh @@ -99,6 +99,16 @@ function ensureAccessToNamespace { kubectl get pods,svc,deploy -n $namespaceName || fail "It was expected that the namespace setup is completed at this point." } +function expectApplyToFail { + hasFailed="no" + + kubectl apply -f $1 || hasFailed="yes" + + if [ $hasFailed == "no" ]; then + fail "$1 succeeded, which was unexpected!" + fi +} + ######################################################################### # The Tests # ######################################################################### @@ -132,11 +142,13 @@ function test_invalidDefinitions { hasFailed="no" - kubectl apply -f tests/invalid-definitions || hasFailed="yes" - - if [ $hasFailed == "no" ]; then - fail "Some of the invalid definition succeeded, which was unexpected!" - fi + expectApplyToFail "tests/invalid-definitions/1.yaml" + expectApplyToFail "tests/invalid-definitions/2.yaml" + expectApplyToFail "tests/invalid-definitions/3.yaml" + expectApplyToFail "tests/invalid-definitions/4.yaml" + expectApplyToFail "tests/invalid-definitions/5.yaml" + expectApplyToFail "tests/invalid-definitions/6.yaml" + expectApplyToFail "tests/invalid-definitions/7.yaml" } # Tests following business cases: diff --git a/tests/valid-definitions/1.yaml b/tests/valid-definitions/1.yaml index 2d3d0c5..79ed76a 100644 --- a/tests/valid-definitions/1.yaml +++ b/tests/valid-definitions/1.yaml @@ -9,5 +9,7 @@ spec: - Test.User@conplement.de - Second.User@conplement.de namespaceAdminServiceAccounts: - - default.default # should work with existing sa - - notyetthere.default # but also with non-existing \ No newline at end of file + - serviceAccount: default # should work with existing sa + namespace: default + - serviceAccount: notyetthere # but also with non-existing + namespace: default \ No newline at end of file diff --git a/tests/valid-definitions/3.yaml b/tests/valid-definitions/3.yaml index 2cbd37a..92658be 100644 --- a/tests/valid-definitions/3.yaml +++ b/tests/valid-definitions/3.yaml @@ -18,14 +18,23 @@ spec: - Tenth.User@conplement.de - Eleventh.User@conplement.de namespaceAdminServiceAccounts: - - account1.default - - account2.default - - account3.default - - account4.default - - account5.default - - account6.default - - account1.custom - - account2.custom - - account3.custom - - account4.custom - - account5.custom \ No newline at end of file + - serviceAccount: account1 + namespace: default + - serviceAccount: account2 + namespace: default + - serviceAccount: account3 + namespace: default + - serviceAccount: account4 + namespace: default + - serviceAccount: account5 + namespace: default + - serviceAccount: account1 + namespace: custom + - serviceAccount: account2 + namespace: custom + - serviceAccount: account3 + namespace: custom + - serviceAccount: account4 + namespace: custom + - serviceAccount: account5 + namespace: custom \ No newline at end of file From 2cd0bd3bce14b5d3b46ee44861aa6d5e2d714b6b Mon Sep 17 00:00:00 2001 From: Denis Biondic Date: Thu, 14 Nov 2019 14:50:44 +0100 Subject: [PATCH 8/8] test: name simplification --- tests/tests.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/tests.sh b/tests/tests.sh index 99be386..2099246 100644 --- a/tests/tests.sh +++ b/tests/tests.sh @@ -113,7 +113,7 @@ function expectApplyToFail { # The Tests # ######################################################################### -function test_noAccessExceptCnsGetListCreate { +function test_noAccessPerDefault { logTestStarted ${FUNCNAME[0]} kubectl config use-context $darthVaderAccount @@ -225,9 +225,8 @@ setupCluster "$1" "$2" "$3" setupTheEmpireServiceAccounts # now we can run all the tests -test_noAccessExceptCnsGetListCreate # this test has to run first +test_noAccessPerDefault # this test has to run first -# following tests should run in any order test_validDefinitions test_invalidDefinitions test_shouldDeployEmpireCnsWithValidRbac