From 5ae8585e5db12e1e1083b0bd06cbb1035d08f49a Mon Sep 17 00:00:00 2001 From: Sakala Venkata Krishna Rohit Date: Mon, 7 Apr 2025 17:40:43 -0700 Subject: [PATCH] Fix namespace access control in steve (#568) * Fix adding namespace resource access * Add tests for addResourceAccess func --- pkg/accesscontrol/policy_rule_index.go | 25 +++- pkg/accesscontrol/policy_rule_index_test.go | 150 ++++++++++++++++++++ 2 files changed, 171 insertions(+), 4 deletions(-) diff --git a/pkg/accesscontrol/policy_rule_index.go b/pkg/accesscontrol/policy_rule_index.go index 0eae10ec..3444ea6e 100644 --- a/pkg/accesscontrol/policy_rule_index.go +++ b/pkg/accesscontrol/policy_rule_index.go @@ -98,14 +98,31 @@ func addResourceAccess(accessSet *AccessSet, namespace string, rule rbacv1.Polic } for _, resourceName := range names { for _, verb := range rule.Verbs { + access := Access{ + Namespace: namespace, + ResourceName: resourceName, + } + + // The first condition namespace != All is to determine if it is a RoleBinding. + // The second and third conditions are to check if the resource is for "namespaces" in core group. + // In kubernetes, rule are valid if they satisfy the following + // - Should be `namespaces` GR + // - From RoleBindings in `namespace` + // - From Rule with ResourceName `*`` or the `namespace` itself. + // Ref: https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/endpoints/request/requestinfo.go#L194 + // If the ResourceName is `All` || namespace itself then only the current namespace is considered as Resourcename + // In the case of Rolebinding for the resource "namespaces" in core group, access.Namespace + // is set to All since namespace on the resource "namespaces" is not valid. + if namespace != All && resource == "namespaces" && group == "" && (resourceName == All || resourceName == namespace) { + access.Namespace = All + access.ResourceName = namespace + } + accessSet.Add(verb, schema.GroupResource{ Group: group, Resource: resource, - }, Access{ - Namespace: namespace, - ResourceName: resourceName, - }) + }, access) } } } diff --git a/pkg/accesscontrol/policy_rule_index_test.go b/pkg/accesscontrol/policy_rule_index_test.go index 4eeec4df..1ac1ddf7 100644 --- a/pkg/accesscontrol/policy_rule_index_test.go +++ b/pkg/accesscontrol/policy_rule_index_test.go @@ -1,11 +1,13 @@ package accesscontrol import ( + "reflect" "slices" "testing" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" ) func Test_policyRuleIndex_roleBindingBySubject(t *testing.T) { @@ -236,3 +238,151 @@ func makeCRB(name string, roleRef rbacv1.RoleRef, subjects []rbacv1.Subject) *rb Subjects: subjects, } } + +func Test_addResourceAccess(t *testing.T) { + tests := []struct { + name string + namespace string + rule rbacv1.PolicyRule + want AccessSet + }{ + { + name: "RoleBinding namespaces resource with empty names", + namespace: "test-ns", + rule: rbacv1.PolicyRule{ + APIGroups: []string{""}, + Resources: []string{"namespaces", "deployments"}, + ResourceNames: []string{}, + Verbs: []string{"get"}, + }, + want: AccessSet{ + set: map[key]resourceAccessSet{ + { + verb: "get", gr: schema.GroupResource{Group: "", Resource: "namespaces"}}: { + Access{Namespace: "*", ResourceName: "test-ns"}: true, + }, + { + verb: "get", gr: schema.GroupResource{Group: "", Resource: "deployments"}}: { + Access{Namespace: "test-ns", ResourceName: "*"}: true, + }, + }, + }, + }, + { + name: "ClusterRoleBinding namespaces resource with empty names", + namespace: "*", + rule: rbacv1.PolicyRule{ + APIGroups: []string{""}, + Resources: []string{"namespaces", "deployments"}, + ResourceNames: []string{}, + Verbs: []string{"get"}, + }, + want: AccessSet{ + set: map[key]resourceAccessSet{ + {verb: "get", gr: schema.GroupResource{Group: "", Resource: "namespaces"}}: { + Access{Namespace: "*", ResourceName: "*"}: true, + }, + { + verb: "get", gr: schema.GroupResource{Group: "", Resource: "deployments"}}: { + Access{Namespace: "*", ResourceName: "*"}: true, + }, + }, + }, + }, + { + name: "RoleBinding namespaces resource with specific names", + namespace: "test-ns", + rule: rbacv1.PolicyRule{ + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + ResourceNames: []string{"specific-ns"}, + Verbs: []string{"get"}, + }, + want: AccessSet{ + set: map[key]resourceAccessSet{ + {verb: "get", gr: schema.GroupResource{Group: "", Resource: "namespaces"}}: { + Access{Namespace: "test-ns", ResourceName: "specific-ns"}: true, + }, + }, + }, + }, + { + name: "RoleBinding namespaces resource with its own namespace", + namespace: "test-ns", + rule: rbacv1.PolicyRule{ + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + ResourceNames: []string{"test-ns"}, + Verbs: []string{"get"}, + }, + want: AccessSet{ + set: map[key]resourceAccessSet{ + {verb: "get", gr: schema.GroupResource{Group: "", Resource: "namespaces"}}: { + Access{Namespace: "*", ResourceName: "test-ns"}: true, + }, + }, + }, + }, + { + name: "RoleBinding other resource with empty names", + namespace: "test-ns", + rule: rbacv1.PolicyRule{ + APIGroups: []string{"apps"}, + Resources: []string{"deployments"}, + ResourceNames: []string{}, + Verbs: []string{"get"}, + }, + want: AccessSet{ + set: map[key]resourceAccessSet{ + {verb: "get", gr: schema.GroupResource{Group: "apps", Resource: "deployments"}}: { + Access{Namespace: "test-ns", ResourceName: "*"}: true, + }, + }, + }, + }, + { + name: "ClusterRoleBinding other resource with empty names", + namespace: "*", + rule: rbacv1.PolicyRule{ + APIGroups: []string{"apps"}, + Resources: []string{"deployments"}, + ResourceNames: []string{}, + Verbs: []string{"get"}, + }, + want: AccessSet{ + set: map[key]resourceAccessSet{ + {verb: "get", gr: schema.GroupResource{Group: "apps", Resource: "deployments"}}: { + Access{Namespace: "*", ResourceName: "*"}: true, + }, + }, + }, + }, + { + name: "RoleBinding other resource with specific names", + namespace: "test-ns", + rule: rbacv1.PolicyRule{ + APIGroups: []string{"apps"}, + Resources: []string{"deployments"}, + ResourceNames: []string{"my-deploy"}, + Verbs: []string{"get"}, + }, + want: AccessSet{ + set: map[key]resourceAccessSet{ + {verb: "get", gr: schema.GroupResource{Group: "apps", Resource: "deployments"}}: { + Access{Namespace: "test-ns", ResourceName: "my-deploy"}: true, + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + accessSet := &AccessSet{} + addResourceAccess(accessSet, tt.namespace, tt.rule) + if !reflect.DeepEqual(*accessSet, tt.want) { + t.Errorf("addResourceAccess() got = %v, want %v", *accessSet, tt.want) + } + }) + } +}