From 1b13defef78ff9c53dbdc6c43a9e14659f868991 Mon Sep 17 00:00:00 2001 From: Noor Malik Date: Fri, 22 Nov 2024 18:51:39 +0100 Subject: [PATCH 01/15] use global slices for verbs and resources --- test/e2e/authorization.go | 118 ++++++++++++++++---------------------- 1 file changed, 49 insertions(+), 69 deletions(-) diff --git a/test/e2e/authorization.go b/test/e2e/authorization.go index f3d33c9ffe..0c067c8e7b 100644 --- a/test/e2e/authorization.go +++ b/test/e2e/authorization.go @@ -23,6 +23,29 @@ var ( {"CollaboratorPowerUser"}, {"Administrator"}, } + + // "secrets" are not included as they have their own set of test cases. + namespacedResources = []string{ + "pods", + "apps/deployments", + "apps/statefulsets", + "apps/deployments/scale", + "apps/statefulsets/scale", + "services", + "persistentvolumes", + "persistentvolumeclaims", + "configmaps", + } + // "nodes" are not included as they have their own set of test cases. + globalResources = []string{ + "namespaces", + "rbac.authorization.k8s.io/clusterroles", + "storage.k8s.io/storageclasses", + "apiextensions.k8s.io/customresourcedefinitions", + } + readOperations = []string{"get", "list", "watch"} + writeOperations = []string{"create", "update", "delete", "patch"} + allOperations = append(readOperations, writeOperations...) ) var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { @@ -92,7 +115,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { tc.data.resources = []string{"secrets"} }) g.It("should deny access in all namespaces", func() { - tc.data.verbs = []string{"get", "list", "watch", "create", "update", "delete", "patch"} + tc.data.verbs = allOperations tc.data.namespaces = []string{"", "teapot", "kube-system"} tc.run(context.TODO(), cs, false) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) @@ -100,47 +123,30 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { }) g.When("the resource is not a Secret resource", func() { g.BeforeEach(func() { - tc.data.resources = []string{ - "pods", - "apps/deployments", - "apps/daemonsets", - "apps/statefulsets", - "apps/deployments/scale", - "apps/statefulsets/scale", - "services", - "persistentvolumes", - "persistentvolumeclaims", - "configmaps", - } + tc.data.resources = namespacedResources tc.data.namespaces = []string{"", "teapot", "kube-system"} }) g.It("should allow read access in all namespaces", func() { - tc.data.verbs = []string{"get", "list", "watch"} + tc.data.verbs = readOperations tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) g.It("should deny write access in all namespaces", func() { - tc.data.verbs = []string{"create", "update", "delete", "patch"} + tc.data.verbs = writeOperations tc.run(context.TODO(), cs, false) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) }) g.When("the resource is a global resource", func() { g.BeforeEach(func() { - tc.data.resources = []string{ - "namespaces", - "nodes", - "rbac.authorization.k8s.io/clusterroles", - "storage.k8s.io/storageclasses", - "apiextensions.k8s.io/customresourcedefinitions", - } + tc.data.resources = append(globalResources, "nodes") g.It("should allow read access", func() { - tc.data.verbs = []string{"get", "list", "watch"} + tc.data.verbs = readOperations tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) g.It("should deny write access", func() { - tc.data.verbs = []string{"create", "update", "delete", "patch"} + tc.data.verbs = writeOperations tc.run(context.TODO(), cs, false) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) @@ -162,21 +168,21 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { g.It("should deny read access to Secrets in kube-system and visibility namespaces", func() { tc.data.resources = []string{"secrets"} tc.data.namespaces = []string{"kube-system", "visibility"} - tc.data.verbs = []string{"get", "list", "watch"} + tc.data.verbs = readOperations tc.run(context.TODO(), cs, false) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) g.It("should deny write access to Nodes", func() { tc.data.resources = []string{"nodes"} - tc.data.verbs = []string{"create", "update", "delete", "patch"} + tc.data.verbs = writeOperations tc.run(context.TODO(), cs, false) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) g.It("should deny write access to DaemonSets", func() { tc.data.resources = []string{"apps/daemonsets"} - tc.data.verbs = []string{"create", "update", "delete", "patch"} + tc.data.verbs = writeOperations tc.run(context.TODO(), cs, false) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) @@ -198,18 +204,8 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { g.When("the resource is a namespaced resource", func() { g.BeforeEach(func() { - tc.data.resources = []string{ - "pods", - "apps/deployments", - "apps/statefulsets", - "apps/deployments/scale", - "apps/statefulsets/scale", - "services", - "persistentvolumes", - "persistentvolumeclaims", - "configmaps", - } - tc.data.verbs = []string{"create", "update", "delete", "patch"} + tc.data.resources = namespacedResources + tc.data.verbs = writeOperations }) // These should be covered by the admission-controller tests. // They're written here for completeness. @@ -218,7 +214,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { }) g.When("the resource is a global resource", func() { g.BeforeEach(func() { - tc.data.verbs = []string{"create", "update", "delete", "patch"} + tc.data.verbs = writeOperations }) g.It("should deny write access to Nodes", func() { tc.data.resources = []string{"nodes"} @@ -226,11 +222,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) g.It("should allow write access to resources other than Nodes", func() { - tc.data.resources = []string{ - "namespaces", - "storage.k8s.io/storageclasses", - "apiextensions.k8s.io/customresourcedefinitions", - } + tc.data.resources = globalResources tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) @@ -253,7 +245,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { g.When("the resource is a Secret", func() { g.BeforeEach(func() { tc.data.resources = []string{"secrets"} - tc.data.verbs = []string{"get", "list", "watch"} + tc.data.verbs = readOperations }) g.It("should allow read access to visibility namespace", func() { @@ -270,13 +262,13 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { g.It("should deny write access to Nodes", func() { tc.data.resources = []string{"nodes"} - tc.data.verbs = []string{"create", "update", "delete", "patch"} + tc.data.verbs = writeOperations tc.run(context.TODO(), cs, false) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) g.It("should allow write access to DaemonSets", func() { tc.data.resources = []string{"apps/daemonsets"} - tc.data.verbs = []string{"create", "update", "delete", "patch"} + tc.data.verbs = writeOperations tc.data.namespaces = []string{"visibility"} tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) @@ -297,16 +289,8 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { g.When("the resource is a namespaced resource", func() { g.BeforeEach(func() { - tc.data.resources = []string{ - "pods", - "apps/deployments", - "apps/statefulsets", - "services", - "persistentvolumes", - "persistentvolumeclaims", - "configmaps", - } - tc.data.verbs = []string{"create", "update", "delete", "patch"} + tc.data.resources = namespacedResources + tc.data.verbs = writeOperations }) // This should be covered by the admission-controller tests. // It's written here for completeness. @@ -320,7 +304,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { g.When("the resource is a global resource", func() { g.BeforeEach(func() { - tc.data.verbs = []string{"create", "update", "delete", "patch"} + tc.data.verbs = writeOperations }) g.It("should deny access to Nodes", func() { tc.data.resources = []string{"nodes"} @@ -328,11 +312,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) g.It("should allow access to resources other than Nodes", func() { - tc.data.resources = []string{ - "namespaces", - "storage.k8s.io/storageclasses", - "apiextensions.k8s.io/customresourcedefinitions", - } + tc.data.resources = globalResources tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) @@ -512,7 +492,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { tc.data.resources = []string{"secrets"} }) g.It("should allow read access", func() { - tc.data.verbs = []string{"get", "list", "watch"} + tc.data.verbs = readOperations tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) @@ -523,12 +503,12 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { tc.data.resources = []string{"pods"} }) g.It("should allow read access", func() { - tc.data.verbs = []string{"get", "list", "watch"} + tc.data.verbs = readOperations tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) g.It("should allow write access", func() { - tc.data.verbs = []string{"create", "update", "delete", "patch"} + tc.data.verbs = writeOperations tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) @@ -552,7 +532,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { tc.data.resources = []string{"secrets"} }) g.It("should allow read access", func() { - tc.data.verbs = []string{"get", "list", "watch"} + tc.data.verbs = readOperations tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) @@ -562,7 +542,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { tc.data.resources = []string{"pods, apps/daemonsets"} }) g.It("should allow write access", func() { - tc.data.verbs = []string{"create", "update", "delete", "patch"} + tc.data.verbs = writeOperations tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) From 6f7f8c96c5ef8339c41832b8ffdc86a8a9b46b97 Mon Sep 17 00:00:00 2001 From: Noor Malik Date: Fri, 22 Nov 2024 19:18:18 +0100 Subject: [PATCH 02/15] re-introduce write protection test cases and add skip rules for them --- test/e2e/authorization.go | 28 +++++++++++++++++++++------- test/e2e/run_e2e.sh | 1 + 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/test/e2e/authorization.go b/test/e2e/authorization.go index 0c067c8e7b..3de9a64663 100644 --- a/test/e2e/authorization.go +++ b/test/e2e/authorization.go @@ -207,10 +207,18 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { tc.data.resources = namespacedResources tc.data.verbs = writeOperations }) - // These should be covered by the admission-controller tests. - // They're written here for completeness. - g.It("should deny write access in kube-system and visibility namespaces", func() {}) - g.It("should allow write access in namespaces other than kube-system and visibility", func() {}) + // These should be covered by the admission-controller tests. They will + // be skipped here. Later when we cover everything with RBAC, we can run them again. + g.It("should deny write access in kube-system and visibility namespaces", func() { + tc.data.namespaces = []string{"kube-system", "visibility"} + tc.run(context.TODO(), cs, false) + gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) + }) + g.It("should allow write access in namespaces other than kube-system and visibility", func() { + tc.data.namespaces = []string{"", "teapot"} + tc.run(context.TODO(), cs, true) + gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) + }) }) g.When("the resource is a global resource", func() { g.BeforeEach(func() { @@ -279,6 +287,8 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) + // This should be covered by the admission-controller tests. It will + // be skipped here. Later when we cover everything with RBAC, we can run it again. g.It("should deny deletion of kube-system or visibility namespaces", func() { tc.data.resources = []string{"namespaces"} tc.data.names = []string{"kube-system", "visibility"} @@ -292,9 +302,13 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { tc.data.resources = namespacedResources tc.data.verbs = writeOperations }) - // This should be covered by the admission-controller tests. - // It's written here for completeness. - g.It("should deny write access in kube-system namespace", func() {}) + // This should be covered by the admission-controller tests. It will + // be skipped here. Later when we cover everything with RBAC, we can run it again. + g.It("should deny write access in kube-system namespace", func() { + tc.data.namespaces = []string{"kube-system"} + tc.run(context.TODO(), cs, false) + gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) + }) g.It("should allow write access in namespaces other than kube-system", func() { tc.data.namespaces = []string{"", "teapot"} tc.run(context.TODO(), cs, true) diff --git a/test/e2e/run_e2e.sh b/test/e2e/run_e2e.sh index eb8b2f5538..6a98bf2cf6 100755 --- a/test/e2e/run_e2e.sh +++ b/test/e2e/run_e2e.sh @@ -185,6 +185,7 @@ if [ "$e2e" = true ]; then ginkgo -procs=25 -flake-attempts=2 \ -focus="(\[Conformance\]|\[StatefulSetBasic\]|\[Feature:StatefulSet\]\s\[Slow\].*mysql|\[Zalando\])" \ -skip="(\[Serial\]|validates.that.there.is.no.conflict.between.pods.with.same.hostPort.but.different.hostIP.and.protocol|Should.create.gradual.traffic.routes)" \ + -skip="should.deny.write.access.in.kube-system.and.visibility.namespaces|should.allow.write.access.in.namespaces.other.than.kube-system.and.visibility|should.deny.write.access.in.kube-system.namespace|should.deny.deletion.of.kube-system.or.visibility.namespaces" \ "e2e.test" -- \ -delete-namespace-on-failure=false \ -non-blocking-taints=node.kubernetes.io/role,nvidia.com/gpu,dedicated \ From 4eba59b8cb63f1d56b7ef05fad86022bfe76ebae Mon Sep 17 00:00:00 2001 From: Noor Malik Date: Fri, 22 Nov 2024 19:31:36 +0100 Subject: [PATCH 03/15] combine default service-account test cases --- test/e2e/authorization.go | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/test/e2e/authorization.go b/test/e2e/authorization.go index 3de9a64663..60e5507e7b 100644 --- a/test/e2e/authorization.go +++ b/test/e2e/authorization.go @@ -372,21 +372,9 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { // g.It("should allow to create Pods", func() {}) }) - g.When("the default service account is in default namespace", func() { + g.When("the service account is the default service account", func() { g.BeforeEach(func() { - tc.data.users = []string{"system:serviceaccount:default:default"} - }) - g.It("should deny to list StatefulSets", func() { - tc.data.resources = []string{"apps/statefulsets"} - tc.data.verbs = []string{"list"} - tc.run(context.TODO(), cs, false) - gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) - }) - }) - - g.When("the default service account is in non-default namespace", func() { - g.BeforeEach(func() { - tc.data.users = []string{"system:serviceaccount:non-default:default"} + tc.data.users = []string{"system:serviceaccount:default:default", "system:serviceaccount:non-default:default"} }) g.It("should deny to list StatefulSets", func() { tc.data.resources = []string{"apps/statefulsets"} From 39e6e18d57eb00c4da9ef0a7d4a1fa4c044c91ee Mon Sep 17 00:00:00 2001 From: Noor Malik Date: Fri, 22 Nov 2024 20:11:26 +0100 Subject: [PATCH 04/15] add new tests for credentials-provider user --- test/e2e/authorization.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/e2e/authorization.go b/test/e2e/authorization.go index 60e5507e7b..ff426b5f37 100644 --- a/test/e2e/authorization.go +++ b/test/e2e/authorization.go @@ -449,11 +449,16 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { g.When("the user is k8sapi_credentials-provider", func() { g.BeforeEach(func() { tc.data.users = []string{"zalando-iam:zalando:service:k8sapi_credentials-provider"} - }) - g.It("should allow to get Secrets in kube-system namespace", func() { tc.data.resources = []string{"secrets"} tc.data.namespaces = []string{"kube-system"} - tc.data.verbs = []string{"get"} + }) + g.It("should not allow to delete secrets in kube-system namespace", func() { + tc.data.verbs = []string{"delete"} + tc.run(context.TODO(), cs, false) + gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) + }) + g.It("should allow all non-delete operations on secrets in kube-system namespace", func() { + tc.data.verbs = []string{"get", "list", "watch", "create", "update", "patch"} tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) From 04827d63f832859c141450ad58b07440dd42b616 Mon Sep 17 00:00:00 2001 From: Noor Malik Date: Fri, 22 Nov 2024 20:15:48 +0100 Subject: [PATCH 05/15] add test case for secret write access for administrators --- test/e2e/authorization.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/e2e/authorization.go b/test/e2e/authorization.go index ff426b5f37..df3e265ca9 100644 --- a/test/e2e/authorization.go +++ b/test/e2e/authorization.go @@ -503,6 +503,11 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) + g.It("should allow write access", func() { + tc.data.verbs = writeOperations + tc.run(context.TODO(), cs, true) + gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) + }) }) g.When("the resource is not a Secret", func() { From 1646c0671183e2aa08fea97d3fb23933872c86d5 Mon Sep 17 00:00:00 2001 From: Noor Malik Date: Fri, 22 Nov 2024 20:42:20 +0100 Subject: [PATCH 06/15] Unify read and write test cases into a single and other nits Refactor allNamespaces as a global slice Add clarification comments on the global slices Replace all instances of "resource not a secret" with a wider list of namespacedResources --- test/e2e/authorization.go | 50 +++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/test/e2e/authorization.go b/test/e2e/authorization.go index df3e265ca9..140179c0a3 100644 --- a/test/e2e/authorization.go +++ b/test/e2e/authorization.go @@ -36,6 +36,7 @@ var ( "persistentvolumeclaims", "configmaps", } + // "nodes" are not included as they have their own set of test cases. globalResources = []string{ "namespaces", @@ -43,9 +44,21 @@ var ( "storage.k8s.io/storageclasses", "apiextensions.k8s.io/customresourcedefinitions", } - readOperations = []string{"get", "list", "watch"} + // a slice of "get", "list", "watch" verbs + readOperations = []string{"get", "list", "watch"} + + // a slice of "create", "update", "delete", "patch" verbs writeOperations = []string{"create", "update", "delete", "patch"} - allOperations = append(readOperations, writeOperations...) + + // a slice of all operations + allOperations = append(readOperations, writeOperations...) + + // a slice representing all namespaces with respect to the test cases + // "" represents the default namespace + // "teapot" is a random namespace + // "visibility" is a namespace where collaborators will have access + // "kube-system" is a namespace where only administrators will have access + allNamespaces = []string{"", "teapot", "visibility", "kube-system"} ) var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { @@ -53,12 +66,10 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { f := framework.NewDefaultFramework("authorization") - // Initialise the clientset before each test g.BeforeEach(func() { cs = f.ClientSet }) - // Test cases for all groups of users g.Context("For all groups", func() { var tc testCase g.BeforeEach(func() { @@ -80,7 +91,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { }) g.It("should deny access for service accounts", func() { tc.data.resources = []string{"serviceaccounts"} - tc.data.namespaces = []string{"", "teapot", "kube-system"} + tc.data.namespaces = allNamespaces tc.run(context.TODO(), cs, false) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) @@ -97,7 +108,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { }) g.It("should deny access for roles in all namespaces", func() { tc.data.resources = []string{"rbac.authorization.k8s.io/role"} - tc.data.namespaces = []string{"", "teapot", "kube-system"} + tc.data.namespaces = allNamespaces tc.run(context.TODO(), cs, false) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) @@ -116,7 +127,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { }) g.It("should deny access in all namespaces", func() { tc.data.verbs = allOperations - tc.data.namespaces = []string{"", "teapot", "kube-system"} + tc.data.namespaces = allNamespaces tc.run(context.TODO(), cs, false) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) @@ -124,7 +135,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { g.When("the resource is not a Secret resource", func() { g.BeforeEach(func() { tc.data.resources = namespacedResources - tc.data.namespaces = []string{"", "teapot", "kube-system"} + tc.data.namespaces = allNamespaces }) g.It("should allow read access in all namespaces", func() { tc.data.verbs = readOperations @@ -498,13 +509,8 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { g.BeforeEach(func() { tc.data.resources = []string{"secrets"} }) - g.It("should allow read access", func() { - tc.data.verbs = readOperations - tc.run(context.TODO(), cs, true) - gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) - }) - g.It("should allow write access", func() { - tc.data.verbs = writeOperations + g.It("should allow read and write access", func() { + tc.data.verbs = allOperations tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) @@ -512,20 +518,14 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { g.When("the resource is not a Secret", func() { g.BeforeEach(func() { - tc.data.resources = []string{"pods"} + tc.data.resources = namespacedResources }) - g.It("should allow read access", func() { - tc.data.verbs = readOperations - tc.run(context.TODO(), cs, true) - gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) - }) - g.It("should allow write access", func() { - tc.data.verbs = writeOperations + g.It("should allow read and write access", func() { + tc.data.verbs = allOperations tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) }) - }) g.When("namespace is not kube-system", func() { @@ -551,7 +551,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { }) g.When("the resource is not a Secret", func() { g.BeforeEach(func() { - tc.data.resources = []string{"pods, apps/daemonsets"} + tc.data.resources = namespacedResources }) g.It("should allow write access", func() { tc.data.verbs = writeOperations From cba3a81411c9908f325a5e38dfbaff4cad3cc41a Mon Sep 17 00:00:00 2001 From: Noor Malik Date: Sun, 24 Nov 2024 20:49:06 +0100 Subject: [PATCH 07/15] drop secret read permission from poweruser ClusterRole --- cluster/manifests/roles/poweruser-role.yaml | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/cluster/manifests/roles/poweruser-role.yaml b/cluster/manifests/roles/poweruser-role.yaml index 9d6ce88005..33619c9d95 100644 --- a/cluster/manifests/roles/poweruser-role.yaml +++ b/cluster/manifests/roles/poweruser-role.yaml @@ -58,19 +58,6 @@ rules: - services/proxy verbs: - get -- apiGroups: - - '' - resources: - - secrets - verbs: - - create - - delete - - deletecollection - - get - - list - - patch - - update - - watch - apiGroups: - '' - extensions From c8b3679ac1c65d3e304592dda40cb9e499c20c0b Mon Sep 17 00:00:00 2001 From: Noor Malik Date: Mon, 25 Nov 2024 10:14:50 +0100 Subject: [PATCH 08/15] add secret reading permisisons to collaborator Roles --- cluster/manifests/roles/collaborator-roles.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cluster/manifests/roles/collaborator-roles.yaml b/cluster/manifests/roles/collaborator-roles.yaml index 8355cc9f09..2afe883e31 100644 --- a/cluster/manifests/roles/collaborator-roles.yaml +++ b/cluster/manifests/roles/collaborator-roles.yaml @@ -14,6 +14,14 @@ rules: - update - patch - delete +- apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch --- kind: RoleBinding apiVersion: rbac.authorization.k8s.io/v1 From 4962df79493713ef47d26c635f86b95a5caac622 Mon Sep 17 00:00:00 2001 From: Noor Malik Date: Mon, 25 Nov 2024 14:06:29 +0100 Subject: [PATCH 09/15] fix default namespace and pv as global resource --- test/e2e/authorization.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/e2e/authorization.go b/test/e2e/authorization.go index 140179c0a3..fff82a076f 100644 --- a/test/e2e/authorization.go +++ b/test/e2e/authorization.go @@ -32,7 +32,6 @@ var ( "apps/deployments/scale", "apps/statefulsets/scale", "services", - "persistentvolumes", "persistentvolumeclaims", "configmaps", } @@ -42,6 +41,7 @@ var ( "namespaces", "rbac.authorization.k8s.io/clusterroles", "storage.k8s.io/storageclasses", + "storage.k8s.io/persistentvolumes", "apiextensions.k8s.io/customresourcedefinitions", } // a slice of "get", "list", "watch" verbs @@ -54,11 +54,11 @@ var ( allOperations = append(readOperations, writeOperations...) // a slice representing all namespaces with respect to the test cases - // "" represents the default namespace + // "default" is the default namespace // "teapot" is a random namespace // "visibility" is a namespace where collaborators will have access // "kube-system" is a namespace where only administrators will have access - allNamespaces = []string{"", "teapot", "visibility", "kube-system"} + allNamespaces = []string{"default", "teapot", "visibility", "kube-system"} ) var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { @@ -226,7 +226,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) g.It("should allow write access in namespaces other than kube-system and visibility", func() { - tc.data.namespaces = []string{"", "teapot"} + tc.data.namespaces = []string{"default", "teapot"} tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) @@ -321,7 +321,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) g.It("should allow write access in namespaces other than kube-system", func() { - tc.data.namespaces = []string{"", "teapot"} + tc.data.namespaces = []string{"default", "teapot"} tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) From 19b2dafd2a81217d23e6abe107e221ffcd695433 Mon Sep 17 00:00:00 2001 From: Noor Malik Date: Mon, 25 Nov 2024 14:12:26 +0100 Subject: [PATCH 10/15] test that poweruser can also read secrets in non-system namespaces --- test/e2e/authorization.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/e2e/authorization.go b/test/e2e/authorization.go index fff82a076f..a63b91003a 100644 --- a/test/e2e/authorization.go +++ b/test/e2e/authorization.go @@ -184,6 +184,14 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) + g.It("should allow read access to Secrets in namespaces other than kube-system and visibility", func() { + tc.data.resources = []string{"secrets"} + tc.data.namespaces = []string{"default", "teapot"} + tc.data.verbs = readOperations + tc.run(context.TODO(), cs, true) + gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) + }) + g.It("should deny write access to Nodes", func() { tc.data.resources = []string{"nodes"} tc.data.verbs = writeOperations From 6bda2f6155c60591e21c685c0880587dc30cbda9 Mon Sep 17 00:00:00 2001 From: Noor Malik Date: Mon, 25 Nov 2024 14:32:09 +0100 Subject: [PATCH 11/15] make RBAC changes only when role-sync-controller is enabled --- cluster/manifests/roles/collaborator-roles.yaml | 2 ++ cluster/manifests/roles/poweruser-role.yaml | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/cluster/manifests/roles/collaborator-roles.yaml b/cluster/manifests/roles/collaborator-roles.yaml index 2afe883e31..f23f446041 100644 --- a/cluster/manifests/roles/collaborator-roles.yaml +++ b/cluster/manifests/roles/collaborator-roles.yaml @@ -14,6 +14,7 @@ rules: - update - patch - delete +{{ if eq .Cluster.ConfigItems.role_sync_controller_enabled "true" }} - apiGroups: - "" resources: @@ -22,6 +23,7 @@ rules: - get - list - watch +{{ end }} --- kind: RoleBinding apiVersion: rbac.authorization.k8s.io/v1 diff --git a/cluster/manifests/roles/poweruser-role.yaml b/cluster/manifests/roles/poweruser-role.yaml index 33619c9d95..af1666d3c5 100644 --- a/cluster/manifests/roles/poweruser-role.yaml +++ b/cluster/manifests/roles/poweruser-role.yaml @@ -58,6 +58,21 @@ rules: - services/proxy verbs: - get +{{ if ne .Cluster.ConfigItems.role_sync_controller_enabled "true" }} +- apiGroups: + - '' + resources: + - secrets + verbs: + - create + - delete + - deletecollection + - get + - list + - patch + - update + - watch +{{ end }} - apiGroups: - '' - extensions From cbce361778adc4af9b686cc8da116a9719f6c13a Mon Sep 17 00:00:00 2001 From: Noor Malik Date: Mon, 25 Nov 2024 18:54:01 +0100 Subject: [PATCH 12/15] remove persistentvolumes from global resource slice --- test/e2e/authorization.go | 1 - 1 file changed, 1 deletion(-) diff --git a/test/e2e/authorization.go b/test/e2e/authorization.go index a63b91003a..a8803895c5 100644 --- a/test/e2e/authorization.go +++ b/test/e2e/authorization.go @@ -41,7 +41,6 @@ var ( "namespaces", "rbac.authorization.k8s.io/clusterroles", "storage.k8s.io/storageclasses", - "storage.k8s.io/persistentvolumes", "apiextensions.k8s.io/customresourcedefinitions", } // a slice of "get", "list", "watch" verbs From acd2bdf1e2c87310e34869a6bd0da7c20ffbdfe9 Mon Sep 17 00:00:00 2001 From: Noor Malik Date: Tue, 26 Nov 2024 11:26:40 +0100 Subject: [PATCH 13/15] handle skipped tests better --- test/e2e/authorization.go | 24 ++++-------------------- test/e2e/run_e2e.sh | 1 - 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/test/e2e/authorization.go b/test/e2e/authorization.go index a8803895c5..8833224138 100644 --- a/test/e2e/authorization.go +++ b/test/e2e/authorization.go @@ -225,17 +225,11 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { tc.data.resources = namespacedResources tc.data.verbs = writeOperations }) - // These should be covered by the admission-controller tests. They will - // be skipped here. Later when we cover everything with RBAC, we can run them again. g.It("should deny write access in kube-system and visibility namespaces", func() { - tc.data.namespaces = []string{"kube-system", "visibility"} - tc.run(context.TODO(), cs, false) - gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) + g.Skip("handled by admission-controller") }) g.It("should allow write access in namespaces other than kube-system and visibility", func() { - tc.data.namespaces = []string{"default", "teapot"} - tc.run(context.TODO(), cs, true) - gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) + g.Skip("handled by admission-controller") }) }) g.When("the resource is a global resource", func() { @@ -305,14 +299,8 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { tc.run(context.TODO(), cs, true) gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) }) - // This should be covered by the admission-controller tests. It will - // be skipped here. Later when we cover everything with RBAC, we can run it again. g.It("should deny deletion of kube-system or visibility namespaces", func() { - tc.data.resources = []string{"namespaces"} - tc.data.names = []string{"kube-system", "visibility"} - tc.data.verbs = []string{"delete"} - tc.run(context.TODO(), cs, false) - gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) + g.Skip("handled by admission-controller") }) g.When("the resource is a namespaced resource", func() { @@ -320,12 +308,8 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { tc.data.resources = namespacedResources tc.data.verbs = writeOperations }) - // This should be covered by the admission-controller tests. It will - // be skipped here. Later when we cover everything with RBAC, we can run it again. g.It("should deny write access in kube-system namespace", func() { - tc.data.namespaces = []string{"kube-system"} - tc.run(context.TODO(), cs, false) - gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) + g.Skip("handled by admission-controller") }) g.It("should allow write access in namespaces other than kube-system", func() { tc.data.namespaces = []string{"default", "teapot"} diff --git a/test/e2e/run_e2e.sh b/test/e2e/run_e2e.sh index 6a98bf2cf6..eb8b2f5538 100755 --- a/test/e2e/run_e2e.sh +++ b/test/e2e/run_e2e.sh @@ -185,7 +185,6 @@ if [ "$e2e" = true ]; then ginkgo -procs=25 -flake-attempts=2 \ -focus="(\[Conformance\]|\[StatefulSetBasic\]|\[Feature:StatefulSet\]\s\[Slow\].*mysql|\[Zalando\])" \ -skip="(\[Serial\]|validates.that.there.is.no.conflict.between.pods.with.same.hostPort.but.different.hostIP.and.protocol|Should.create.gradual.traffic.routes)" \ - -skip="should.deny.write.access.in.kube-system.and.visibility.namespaces|should.allow.write.access.in.namespaces.other.than.kube-system.and.visibility|should.deny.write.access.in.kube-system.namespace|should.deny.deletion.of.kube-system.or.visibility.namespaces" \ "e2e.test" -- \ -delete-namespace-on-failure=false \ -non-blocking-taints=node.kubernetes.io/role,nvidia.com/gpu,dedicated \ From 3931435b2de747eb4f5b8d25079246c76dd40868 Mon Sep 17 00:00:00 2001 From: Noor Malik Date: Tue, 26 Nov 2024 15:44:32 +0100 Subject: [PATCH 14/15] skip test for delete deny for system namespace for poweruser,manual,emergency groups --- test/e2e/authorization.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/e2e/authorization.go b/test/e2e/authorization.go index 8833224138..49f59e5357 100644 --- a/test/e2e/authorization.go +++ b/test/e2e/authorization.go @@ -213,11 +213,7 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() { }) g.It("should deny deleting kube-system or visibility namespaces", func() { - tc.data.resources = []string{"namespaces"} - tc.data.namespaces = []string{"kube-system", "visibility"} - tc.data.verbs = []string{"delete"} - tc.run(context.TODO(), cs, false) - gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String()) + g.Skip("handled by admission-controller") }) g.When("the resource is a namespaced resource", func() { From 27dcc32153d26a8b6c7c25c18f1d1b1d117db5d1 Mon Sep 17 00:00:00 2001 From: Mikkel Oscar Lyderik Larsen Date: Wed, 27 Nov 2024 13:57:20 +0100 Subject: [PATCH 15/15] Update custom metrics image to work with ipv6 Signed-off-by: Mikkel Oscar Lyderik Larsen --- test/e2e/kube_metrics_adapter_test.go | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/test/e2e/kube_metrics_adapter_test.go b/test/e2e/kube_metrics_adapter_test.go index e8374cd6dd..5975c12ff2 100644 --- a/test/e2e/kube_metrics_adapter_test.go +++ b/test/e2e/kube_metrics_adapter_test.go @@ -330,7 +330,7 @@ func podMetricDeployment(name string, replicas int32, containers []CustomMetricC func podContainerSpec(name string) corev1.Container { return corev1.Container{ Name: name, - Image: "container-registry.zalando.net/teapot/sample-custom-metrics-autoscaling:master-15", + Image: "container-registry.zalando.net/teapot/sample-custom-metrics-autoscaling:main-5", Ports: []corev1.ContainerPort{{ContainerPort: 8000, Protocol: "TCP"}}, Resources: corev1.ResourceRequirements{ Limits: map[corev1.ResourceName]resource.Quantity{ @@ -347,7 +347,7 @@ func podContainerSpec(name string) corev1.Container { func podMetricContainerSpec(container CustomMetricContainerSpec) corev1.Container { return corev1.Container{ Name: container.Name, - Image: "container-registry.zalando.net/teapot/sample-custom-metrics-autoscaling:master-15", + Image: "container-registry.zalando.net/teapot/sample-custom-metrics-autoscaling:main-5", Ports: []corev1.ContainerPort{{ContainerPort: 8000, Protocol: "TCP"}}, Resources: corev1.ResourceRequirements{ Limits: map[corev1.ResourceName]resource.Quantity{ @@ -358,22 +358,12 @@ func podMetricContainerSpec(container CustomMetricContainerSpec) corev1.Containe corev1.ResourceMemory: resource.MustParse("300Mi"), }, }, - Env: []corev1.EnvVar{ - { - Name: metricNameToEnv(container.MetricName), - Value: strconv.FormatInt(container.MetricValue, 10), - }, + Args: []string{ + "-fake-queue-length", strconv.FormatInt(container.MetricValue, 10), }, } } -// metricNameToEnv converts a metric name like "queue-count" to an env var like "QUEUE_COUNT" -func metricNameToEnv(metric string) string { - nodashes := strings.Replace(metric, "-", "_", 1) - allcaps := strings.ToUpper(nodashes) - return allcaps -} - func simplePodMetricHPA(deploymentName string, metricName string, metricTarget int64) *autoscaling.HorizontalPodAutoscaler { return podMetricHPA(deploymentName, map[string]int64{metricName: metricTarget}) } @@ -401,7 +391,7 @@ func podMetricHPA(deploymentName string, metricTargets map[string]int64) *autosc ObjectMeta: metav1.ObjectMeta{ Name: "custom-metrics-pods-hpa", Annotations: map[string]string{ - strings.Join([]string{"metric-config.pods", metricName, "json-path/json-key"}, "."): "$.queue_count", + strings.Join([]string{"metric-config.pods", metricName, "json-path/json-key"}, "."): "$.queue.length", strings.Join([]string{"metric-config.pods", metricName, "json-path/path"}, "."): "/metrics", strings.Join([]string{"metric-config.pods", metricName, "json-path/port"}, "."): "8000", },