From 9ee5468f47f82f7e43f38fb491d191256ae2cc94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20B=C3=A4hler?= <26610571+oliverbaehler@users.noreply.github.com> Date: Wed, 15 Jul 2026 10:35:36 +0200 Subject: [PATCH] feat: add rolebindings to rules api (#2032) * feat: add rolebindings to rules api Signed-off-by: Oliver Baehler * feat: add rolebindings to rules api Signed-off-by: Oliver Baehler * feat: add rolebindings to rules api Signed-off-by: Oliver Baehler * feat: add rolebindings to rules api Signed-off-by: Oliver Baehler * feat: add rolebindings to rules api Signed-off-by: Oliver Baehler * feat: implement review Signed-off-by: Oliver Baehler --------- Signed-off-by: Oliver Baehler --- Makefile | 4 +- api/v1beta2/additional_role_bindings.go | 2 +- .../crds/capsule.clastix.io_tenants.yaml | 60 +++++++- e2e/additional_role_bindings_test.go | 137 ++++++++++++++++++ hack/distro/capsule/example-setup/rbac.yaml | 9 ++ .../distro/capsule/example-setup/tenants.yaml | 58 +++++--- internal/controllers/tenant/rolebindings.go | 70 ++++++++- .../controllers/tenant/rolebindings_test.go | 69 +++++++++ .../tenant/validation/rolebindings_regex.go | 13 +- pkg/api/rbac/additional_role_bindings.go | 2 +- pkg/api/rules/permission_types.go | 5 + pkg/api/rules/zz_generated.deepcopy.go | 8 + pkg/utils/hashes.go | 15 +- pkg/utils/hashes_test.go | 76 +++++++++- 14 files changed, 487 insertions(+), 41 deletions(-) create mode 100644 internal/controllers/tenant/rolebindings_test.go diff --git a/Makefile b/Makefile index 6b5f1829..4934c74c 100644 --- a/Makefile +++ b/Makefile @@ -242,7 +242,7 @@ dev-setup-argocd: dev-setup-fluxcd @printf " \033[1mkubectl get secret -n argocd argocd-initial-admin-secret -o jsonpath='{.data.password}' | base64 -d\033[0m\n\n" @printf " \033[1mkubectl port-forward svc/argocd-server 9091:80 -n argocd\033[0m\n\n" -dev-setup-cert-manager: +dev-setup-cert-manager: dev-setup-fluxcd @$(KUBECTL) kustomize --load-restrictor='LoadRestrictionsNone' hack/distro/cert-manager | envsubst | kubectl apply -f - dev-setup-fluxcd: @@ -261,7 +261,7 @@ dev-setup-capsule: dev-setup-fluxcd @$(MAKE) wait-for-helmreleases @$(MAKE) dev-setup-capsule-example -dev-setup-capsule-example: dev-setup-fluxcd +dev-setup-capsule-example: @$(KUBECTL) kustomize --load-restrictor='LoadRestrictionsNone' hack/distro/capsule/example-setup | envsubst | kubectl apply -f - @$(KUBECTL) create ns wind-uat --as joe --as-group projectcapsule.dev || true @$(KUBECTL) label ns wind-uat env=test diff --git a/api/v1beta2/additional_role_bindings.go b/api/v1beta2/additional_role_bindings.go index 4b55e419..3e053ce1 100644 --- a/api/v1beta2/additional_role_bindings.go +++ b/api/v1beta2/additional_role_bindings.go @@ -7,6 +7,6 @@ import rbacv1 "k8s.io/api/rbac/v1" type AdditionalRoleBindingsSpec struct { ClusterRoleName string `json:"clusterRoleName"` - // kubebuilder:validation:Minimum=1 + // +kubebuilder:validation:MinItems=1 Subjects []rbacv1.Subject `json:"subjects"` } diff --git a/charts/capsule/crds/capsule.clastix.io_tenants.yaml b/charts/capsule/crds/capsule.clastix.io_tenants.yaml index 75c696e3..5691d78d 100644 --- a/charts/capsule/crds/capsule.clastix.io_tenants.yaml +++ b/charts/capsule/crds/capsule.clastix.io_tenants.yaml @@ -81,7 +81,6 @@ spec: description: Additional Labels for the synchronized rolebindings type: object subjects: - description: kubebuilder:validation:Minimum=1 items: description: |- Subject contains a reference to the object or user identities a role binding applies to. This can either hold a direct API object reference, @@ -111,6 +110,7 @@ spec: - name type: object x-kubernetes-map-type: atomic + minItems: 1 type: array required: - clusterRoleName @@ -1154,7 +1154,6 @@ spec: description: Additional Labels for the synchronized rolebindings type: object subjects: - description: kubebuilder:validation:Minimum=1 items: description: |- Subject contains a reference to the object or user identities a role binding applies to. This can either hold a direct API object reference, @@ -1184,6 +1183,7 @@ spec: - name type: object x-kubernetes-map-type: atomic + minItems: 1 type: array required: - clusterRoleName @@ -2884,6 +2884,62 @@ spec: permissions: description: Permissions for given rule properties: + bindings: + description: Bindings defines additional RoleBindings for + namespaces selected by this rule. + items: + properties: + annotations: + additionalProperties: + type: string + description: Additional Annotations for the synchronized + rolebindings + type: object + clusterRoleName: + type: string + labels: + additionalProperties: + type: string + description: Additional Labels for the synchronized + rolebindings + type: object + subjects: + items: + description: |- + Subject contains a reference to the object or user identities a role binding applies to. This can either hold a direct API object reference, + or a value for non-objects such as user and group names. + properties: + apiGroup: + description: |- + APIGroup holds the API group of the referenced subject. + Defaults to "" for ServiceAccount subjects. + Defaults to "rbac.authorization.k8s.io" for User and Group subjects. + type: string + kind: + description: |- + Kind of object being referenced. Values defined by this API group are "User", "Group", and "ServiceAccount". + If the Authorizer does not recognized the kind value, the Authorizer should report an error. + type: string + name: + description: Name of the object being referenced. + type: string + namespace: + description: |- + Namespace of the referenced object. If the object kind is non-namespace, such as "User" or "Group", and this value is not empty + the Authorizer should report an error. + type: string + required: + - kind + - name + type: object + x-kubernetes-map-type: atomic + minItems: 1 + type: array + required: + - clusterRoleName + - subjects + type: object + type: array promotions: description: |- Define Promotion Rules which distributed additional ClusterRoles across the Tenant diff --git a/e2e/additional_role_bindings_test.go b/e2e/additional_role_bindings_test.go index 008cec84..e43526aa 100644 --- a/e2e/additional_role_bindings_test.go +++ b/e2e/additional_role_bindings_test.go @@ -5,16 +5,21 @@ package e2e import ( "context" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" "github.com/projectcapsule/capsule/pkg/api/meta" "github.com/projectcapsule/capsule/pkg/api/rbac" + "github.com/projectcapsule/capsule/pkg/api/rules" + "github.com/projectcapsule/capsule/pkg/utils" ) var _ = Describe("creating a Namespace with an additional Role Binding", Ordered, Label("tenant", "permissions", "rolebindings"), func() { @@ -84,3 +89,135 @@ var _ = Describe("creating a Namespace with an additional Role Binding", Ordered VerifyTenantRoleBindings(t) }) }) + +var _ = Describe("creating additional RoleBindings from namespace rules", Ordered, Label("tenant", "rules", "permissions", "rolebindings"), func() { + const ( + customLabel = "reflection.proxy.projectcapsule.dev/enabled" + customAnnotation = "projectcapsule.dev/e2e-rule-binding" + selectorLabel = "projectcapsule.dev/e2e-role-binding-environment" + ) + + globalBinding := rbac.AdditionalRoleBindingsSpec{ + ClusterRoleName: "view", + Subjects: []rbacv1.Subject{ + { + Kind: rbacv1.GroupKind, + APIGroup: rbacv1.GroupName, + Name: "system:authenticated", + }, + }, + Labels: map[string]string{customLabel: "true"}, + Annotations: map[string]string{customAnnotation: "global"}, + } + selectedBinding := rbac.AdditionalRoleBindingsSpec{ + ClusterRoleName: "edit", + Subjects: []rbacv1.Subject{ + { + Kind: rbacv1.UserKind, + APIGroup: rbacv1.GroupName, + Name: "e2e-rule-role-binding-user", + }, + }, + Labels: map[string]string{customLabel: "true"}, + Annotations: map[string]string{customAnnotation: "selected"}, + } + + tnt := &capsulev1beta2.Tenant{ + ObjectMeta: metav1.ObjectMeta{Name: "e2e-rule-role-bindings"}, + Spec: capsulev1beta2.TenantSpec{ + Owners: rbac.OwnerListSpec{ + { + CoreOwnerSpec: rbac.CoreOwnerSpec{ + UserSpec: rbac.UserSpec{ + Name: "e2e-rule-role-bindings", + Kind: rbac.UserOwner, + }, + }, + }, + }, + Rules: []*rules.NamespaceRuleBodyTenant{ + { + Permissions: rules.NamespaceRulePermissionBody{ + Bindings: []rbac.AdditionalRoleBindingsSpec{globalBinding}, + }, + }, + { + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{selectorLabel: "prod"}, + }, + Permissions: rules.NamespaceRulePermissionBody{ + Bindings: []rbac.AdditionalRoleBindingsSpec{selectedBinding}, + }, + }, + }, + }, + } + + JustBeforeEach(func() { + EventuallyCreation(func() error { + tnt.ResourceVersion = "" + return k8sClient.Create(context.TODO(), tnt) + }).Should(Succeed()) + + TenantReady(tnt, metav1.ConditionTrue, defaultTimeoutInterval) + }) + + JustAfterEach(func() { + EventuallyDeletion(tnt) + }) + + It("applies bindings according to each rule's namespace selector without mutating their metadata", func() { + prod := NewNamespace("", map[string]string{ + meta.TenantLabel: tnt.GetName(), + selectorLabel: "prod", + }) + NamespaceCreation(prod, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + + dev := NewNamespace("", map[string]string{ + meta.TenantLabel: tnt.GetName(), + selectorLabel: "dev", + }) + NamespaceCreation(dev, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + + NamespaceIsPartOfTenant(tnt, prod).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, dev).Should(Succeed()) + + assertBinding := func(namespace string, binding rbac.AdditionalRoleBindingsSpec) { + Eventually(func(g Gomega) { + roleBinding := &rbacv1.RoleBinding{} + err := k8sClient.Get(context.Background(), client.ObjectKey{ + Namespace: namespace, + Name: meta.NameForManagedRoleBindings(utils.RoleBindingHashFunc(binding)), + }, roleBinding) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(roleBinding.RoleRef.Name).To(Equal(binding.ClusterRoleName)) + g.Expect(roleBinding.Subjects).To(ConsistOf(binding.Subjects)) + g.Expect(roleBinding.Labels).To(HaveKeyWithValue(customLabel, "true")) + g.Expect(roleBinding.Labels).To(HaveKeyWithValue(meta.NewTenantLabel, tnt.Name)) + g.Expect(roleBinding.Labels).To(HaveKeyWithValue(meta.NewManagedByCapsuleLabel, meta.ValueController)) + g.Expect(roleBinding.Annotations).To(HaveKeyWithValue(customAnnotation, binding.Annotations[customAnnotation])) + }).WithTimeout(defaultTimeoutInterval).WithPolling(defaultPollInterval).Should(Succeed()) + } + + assertBinding(prod.Name, globalBinding) + assertBinding(dev.Name, globalBinding) + assertBinding(prod.Name, selectedBinding) + + Consistently(func() bool { + roleBinding := &rbacv1.RoleBinding{} + err := k8sClient.Get(context.Background(), client.ObjectKey{ + Namespace: dev.Name, + Name: meta.NameForManagedRoleBindings(utils.RoleBindingHashFunc(selectedBinding)), + }, roleBinding) + + return apierrors.IsNotFound(err) + }, 2*time.Second, defaultPollInterval).Should(BeTrue()) + + current := &capsulev1beta2.Tenant{} + Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: tnt.Name}, current)).To(Succeed()) + Expect(current.Spec.Rules[0].Permissions.Bindings[0].Labels).To(Equal(globalBinding.Labels)) + Expect(current.Spec.Rules[0].Permissions.Bindings[0].Annotations).To(Equal(globalBinding.Annotations)) + Expect(current.Spec.Rules[1].Permissions.Bindings[0].Labels).To(Equal(selectedBinding.Labels)) + Expect(current.Spec.Rules[1].Permissions.Bindings[0].Annotations).To(Equal(selectedBinding.Annotations)) + }) +}) diff --git a/hack/distro/capsule/example-setup/rbac.yaml b/hack/distro/capsule/example-setup/rbac.yaml index c645da71..4e5a2660 100644 --- a/hack/distro/capsule/example-setup/rbac.yaml +++ b/hack/distro/capsule/example-setup/rbac.yaml @@ -20,3 +20,12 @@ rules: - apiGroups: [""] resources: ["secrets"] verbs: ["get", "create", "patch", "watch", "list", "delete"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: custom:proxy-viewer +rules: +- apiGroups: [""] + resources: ["*"] + verbs: ["list"] diff --git a/hack/distro/capsule/example-setup/tenants.yaml b/hack/distro/capsule/example-setup/tenants.yaml index 52dc0c19..20c22470 100644 --- a/hack/distro/capsule/example-setup/tenants.yaml +++ b/hack/distro/capsule/example-setup/tenants.yaml @@ -11,9 +11,25 @@ spec: kind: User rules: - permissions: - promotions: - - clusterRoles: - - "configmap-replicator" + bindings: + - clusterRoleName: 'custom:proxy-viewer' + subjects: + - apiGroup: rbac.authorization.k8s.io + kind: User + name: joe + labels: + reflection.proxy.projectcapsule.dev/enabled: "true" + enforce: + action: deny + metadata: + - apiGroups: + - "rbac.authorization.k8s.io/v1" + kinds: + - "RoleBinding" + labels: + reflection.proxy.projectcapsule.dev/enabled: + values: + - exp: ".*" - namespaceSelector: matchExpressions: - key: env @@ -49,12 +65,6 @@ spec: additionalMetadataList: - labels: customer: a - additionalRoleBindings: - - clusterRoleName: 'view' - subjects: - - apiGroup: rbac.authorization.k8s.io - kind: User - name: joe resourceQuotas: scope: Tenant items: @@ -82,13 +92,17 @@ spec: owners: - name: bob kind: User - additionalRoleBindings: - - clusterRoleName: 'view' - subjects: - - apiGroup: rbac.authorization.k8s.io - kind: User - name: alice rules: + - permissions: + bindings: + - clusterRoleName: 'custom:proxy-viewer' + subjects: + - apiGroup: rbac.authorization.k8s.io + kind: User + name: alice + labels: + reflection.proxy.projectcapsule.dev/enabled: "true" + - enforce: action: "allow" services: @@ -149,9 +163,11 @@ spec: owners: - name: joe kind: User - additionalRoleBindings: - - clusterRoleName: 'view' - subjects: - - apiGroup: rbac.authorization.k8s.io - kind: Group - name: wind-users + rules: + - permissions: + bindings: + - clusterRoleName: 'view' + subjects: + - apiGroup: rbac.authorization.k8s.io + kind: Group + name: wind-users diff --git a/internal/controllers/tenant/rolebindings.go b/internal/controllers/tenant/rolebindings.go index 3a11a9f8..e055ec91 100644 --- a/internal/controllers/tenant/rolebindings.go +++ b/internal/controllers/tenant/rolebindings.go @@ -6,12 +6,14 @@ package tenant import ( "context" "fmt" + "maps" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" @@ -41,6 +43,66 @@ func (r *Manager) syncRoleBindings(ctx context.Context, log logr.Logger, tenant } } + nsCache := make(map[string]*corev1.Namespace, len(namespaceBindings)) + + for i, rule := range tenant.Spec.Rules { + if rule == nil || len(rule.Permissions.Bindings) == 0 { + continue + } + + // A rule without a selector applies to every tenant namespace and does not + // require resolving Namespace objects. + if rule.NamespaceSelector == nil { + for namespace := range namespaceBindings { + for _, binding := range rule.Permissions.Bindings { + hash := utils.RoleBindingHashFunc(binding) + + namespaceBindings[namespace][hash] = binding + } + } + + continue + } + + for namespace := range namespaceBindings { + ns, ok := nsCache[namespace] + if !ok { + ns = &corev1.Namespace{} + if err := r.Get(ctx, client.ObjectKey{Name: namespace}, ns); err != nil { + if apierrors.IsNotFound(err) { + // Cache missing namespaces as well to avoid repeating the GET for + // subsequent selector-based rules in this reconciliation. + nsCache[namespace] = nil + + continue + } + + return fmt.Errorf("get namespace %q for rules[%d]: %w", namespace, i, err) + } + + nsCache[namespace] = ns + } + + if ns == nil { + continue + } + + matches, err := utils.IsNamespaceSelectedBySelector(ns, rule.NamespaceSelector) + if err != nil { + return fmt.Errorf("invalid namespaceSelector in rules[%d]: %w", i, err) + } + + if !matches { + continue + } + + for _, binding := range rule.Permissions.Bindings { + hash := utils.RoleBindingHashFunc(binding) + namespaceBindings[namespace][hash] = binding + } + } + } + // Does not target all namespaces for _, promotion := range tenant.GetPromotionRoleBindings() { namespace := string(promotion.Namespace) @@ -88,9 +150,7 @@ func (r *Manager) syncAdditionalRoleBinding( target.Labels = map[string]string{} target.Annotations = map[string]string{} - if roleBinding.Labels != nil { - target.Labels = roleBinding.Labels - } + maps.Copy(target.Labels, roleBinding.Labels) target.Labels[meta.NewTenantLabel] = tenant.Name target.Labels[meta.RolebindingLabel] = hash @@ -99,9 +159,7 @@ func (r *Manager) syncAdditionalRoleBinding( // Remove Legacy labels delete(target.Labels, meta.TenantLabel) - if roleBinding.Annotations != nil { - target.Annotations = roleBinding.Annotations - } + maps.Copy(target.Annotations, roleBinding.Annotations) target.RoleRef = rbacv1.RoleRef{ APIGroup: rbacv1.GroupName, diff --git a/internal/controllers/tenant/rolebindings_test.go b/internal/controllers/tenant/rolebindings_test.go new file mode 100644 index 00000000..da403784 --- /dev/null +++ b/internal/controllers/tenant/rolebindings_test.go @@ -0,0 +1,69 @@ +// Copyright 2020-2026 Project Capsule Authors +// SPDX-License-Identifier: Apache-2.0 + +package tenant + +import ( + "context" + "maps" + "testing" + + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" + "github.com/projectcapsule/capsule/pkg/api/rbac" +) + +func TestSyncAdditionalRoleBindingDoesNotMutateSpecMetadata(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + + if err := rbacv1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + + if err := capsulev1beta2.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + + labels := map[string]string{"example.com/label": "value"} + annotations := map[string]string{"example.com/annotation": "value"} + originalLabels := maps.Clone(labels) + originalAnnotations := maps.Clone(annotations) + binding := rbac.AdditionalRoleBindingsSpec{ + ClusterRoleName: "custom:pod-viewer", + Subjects: []rbacv1.Subject{{Kind: rbacv1.UserKind, Name: "alice"}}, + Labels: labels, + Annotations: annotations, + } + + manager := &Manager{Client: fake.NewClientBuilder().WithScheme(scheme).Build()} + tenant := &capsulev1beta2.Tenant{ObjectMeta: metav1.ObjectMeta{Name: "green"}} + + if err := manager.syncAdditionalRoleBinding( + context.Background(), + logr.Discard(), + tenant, + "green-app", + map[string]rbac.AdditionalRoleBindingsSpec{"hash": binding}, + ); err != nil { + t.Fatal(err) + } + + if !maps.Equal(labels, originalLabels) { + t.Fatalf("binding labels were mutated: got %v, want %v", labels, originalLabels) + } + + if !maps.Equal(annotations, originalAnnotations) { + t.Fatalf("binding annotations were mutated: got %v, want %v", annotations, originalAnnotations) + } +} diff --git a/internal/webhook/tenant/validation/rolebindings_regex.go b/internal/webhook/tenant/validation/rolebindings_regex.go index ce4aecfd..0665d878 100644 --- a/internal/webhook/tenant/validation/rolebindings_regex.go +++ b/internal/webhook/tenant/validation/rolebindings_regex.go @@ -13,6 +13,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" + "github.com/projectcapsule/capsule/pkg/api/rbac" ad "github.com/projectcapsule/capsule/pkg/runtime/admission" "github.com/projectcapsule/capsule/pkg/runtime/events" "github.com/projectcapsule/capsule/pkg/runtime/handlers" @@ -62,8 +63,16 @@ func (h *rbRegexHandler) OnUpdate( } func (h *rbRegexHandler) validate(tnt *capsulev1beta2.Tenant, decoder admission.Decoder) *admission.Response { - if len(tnt.Spec.AdditionalRoleBindings) > 0 { - for _, binding := range tnt.Spec.AdditionalRoleBindings { + bindings := append([]rbac.AdditionalRoleBindingsSpec(nil), tnt.Spec.AdditionalRoleBindings...) + + for _, rule := range tnt.Spec.Rules { + if rule != nil { + bindings = append(bindings, rule.Permissions.Bindings...) + } + } + + if len(bindings) > 0 { + for _, binding := range bindings { for _, subject := range binding.Subjects { if subject.Kind == rbacv1.ServiceAccountKind { err := validation.IsDNS1123Subdomain(subject.Name) diff --git a/pkg/api/rbac/additional_role_bindings.go b/pkg/api/rbac/additional_role_bindings.go index 77dc9fef..0c80444d 100644 --- a/pkg/api/rbac/additional_role_bindings.go +++ b/pkg/api/rbac/additional_role_bindings.go @@ -13,7 +13,7 @@ import ( type AdditionalRoleBindingsSpec struct { ClusterRoleName string `json:"clusterRoleName"` - // kubebuilder:validation:Minimum=1 + // +kubebuilder:validation:MinItems=1 Subjects []rbacv1.Subject `json:"subjects"` // Additional Labels for the synchronized rolebindings Labels map[string]string `json:"labels,omitempty"` diff --git a/pkg/api/rules/permission_types.go b/pkg/api/rules/permission_types.go index f42097b5..78fb5185 100644 --- a/pkg/api/rules/permission_types.go +++ b/pkg/api/rules/permission_types.go @@ -5,10 +5,15 @@ package rules import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/projectcapsule/capsule/pkg/api/rbac" ) // +kubebuilder:object:generate=true type NamespaceRulePermissionBody struct { + // Bindings defines additional RoleBindings for namespaces selected by this rule. + Bindings []rbac.AdditionalRoleBindingsSpec `json:"bindings,omitempty"` + // Define Promotion Rules which distributed additional ClusterRoles across the Tenant // for promoted ServiceAccounts. Promotions []*NamespaceRulePromotionRule `json:"promotions,omitempty"` diff --git a/pkg/api/rules/zz_generated.deepcopy.go b/pkg/api/rules/zz_generated.deepcopy.go index 03146b9a..a51b36df 100644 --- a/pkg/api/rules/zz_generated.deepcopy.go +++ b/pkg/api/rules/zz_generated.deepcopy.go @@ -8,6 +8,7 @@ package rules import ( + "github.com/projectcapsule/capsule/pkg/api/rbac" "github.com/projectcapsule/capsule/pkg/api/runtime" "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -212,6 +213,13 @@ func (in *NamespaceRuleEnforceWorkloadsBody) DeepCopy() *NamespaceRuleEnforceWor // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NamespaceRulePermissionBody) DeepCopyInto(out *NamespaceRulePermissionBody) { *out = *in + if in.Bindings != nil { + in, out := &in.Bindings, &out.Bindings + *out = make([]rbac.AdditionalRoleBindingsSpec, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.Promotions != nil { in, out := &in.Promotions, &out.Promotions *out = make([]*NamespaceRulePromotionRule, len(*in)) diff --git a/pkg/utils/hashes.go b/pkg/utils/hashes.go index 2860598b..6b347694 100644 --- a/pkg/utils/hashes.go +++ b/pkg/utils/hashes.go @@ -4,6 +4,7 @@ package utils import ( + "encoding/binary" "fmt" "hash/fnv" @@ -12,11 +13,21 @@ import ( func RoleBindingHashFunc(binding rbac.AdditionalRoleBindingsSpec) string { h := fnv.New64a() + writeField := func(value string) { + var length [8]byte - _, _ = h.Write([]byte(binding.ClusterRoleName)) + binary.LittleEndian.PutUint64(length[:], uint64(len(value))) + _, _ = h.Write(length[:]) + _, _ = h.Write([]byte(value)) + } + + writeField(binding.ClusterRoleName) for _, sub := range binding.Subjects { - _, _ = h.Write([]byte(sub.Kind + sub.Name)) + writeField(sub.APIGroup) + writeField(sub.Kind) + writeField(sub.Namespace) + writeField(sub.Name) } return fmt.Sprintf("%x", h.Sum64()) diff --git a/pkg/utils/hashes_test.go b/pkg/utils/hashes_test.go index 0a4df6e6..824a0984 100644 --- a/pkg/utils/hashes_test.go +++ b/pkg/utils/hashes_test.go @@ -86,6 +86,76 @@ func TestRoleBindingHashFunc_ChangesWhenSubjectNameChanges(t *testing.T) { } } +func TestRoleBindingHashFunc_ChangesWhenSubjectNamespaceChanges(t *testing.T) { + b1 := rbac.AdditionalRoleBindingsSpec{ + ClusterRoleName: "admin", + Subjects: []rbacv1.Subject{{ + Kind: rbacv1.ServiceAccountKind, + Namespace: "team-a", + Name: "deployer", + }}, + } + b2 := rbac.AdditionalRoleBindingsSpec{ + ClusterRoleName: "admin", + Subjects: []rbacv1.Subject{{ + Kind: rbacv1.ServiceAccountKind, + Namespace: "team-b", + Name: "deployer", + }}, + } + + h1 := utils.RoleBindingHashFunc(b1) + h2 := utils.RoleBindingHashFunc(b2) + + if h1 == h2 { + t.Fatalf("expected different hashes when subject Namespace changes, got %q", h1) + } +} + +func TestRoleBindingHashFunc_ChangesWhenSubjectAPIGroupChanges(t *testing.T) { + b1 := rbac.AdditionalRoleBindingsSpec{ + ClusterRoleName: "admin", + Subjects: []rbacv1.Subject{{ + APIGroup: rbacv1.GroupName, + Kind: rbacv1.UserKind, + Name: "alice", + }}, + } + b2 := rbac.AdditionalRoleBindingsSpec{ + ClusterRoleName: "admin", + Subjects: []rbacv1.Subject{{ + APIGroup: "example.com", + Kind: rbacv1.UserKind, + Name: "alice", + }}, + } + + h1 := utils.RoleBindingHashFunc(b1) + h2 := utils.RoleBindingHashFunc(b2) + + if h1 == h2 { + t.Fatalf("expected different hashes when subject APIGroup changes, got %q", h1) + } +} + +func TestRoleBindingHashFunc_UsesUnambiguousFieldBoundaries(t *testing.T) { + b1 := rbac.AdditionalRoleBindingsSpec{ + ClusterRoleName: "ab", + Subjects: []rbacv1.Subject{{Kind: "c", Name: "d"}}, + } + b2 := rbac.AdditionalRoleBindingsSpec{ + ClusterRoleName: "a", + Subjects: []rbacv1.Subject{{Kind: "bc", Name: "d"}}, + } + + h1 := utils.RoleBindingHashFunc(b1) + h2 := utils.RoleBindingHashFunc(b2) + + if h1 == h2 { + t.Fatalf("expected different hashes for differently bounded fields, got %q", h1) + } +} + func TestRoleBindingHashFunc_EmptyInputsStillProduceHash(t *testing.T) { b := rbac.AdditionalRoleBindingsSpec{ ClusterRoleName: "", @@ -98,9 +168,7 @@ func TestRoleBindingHashFunc_EmptyInputsStillProduceHash(t *testing.T) { } } -func TestRoleBindingHashFunc_SubjectOrderMatters_CurrentBehavior(t *testing.T) { - // This test documents the CURRENT behavior: - // the hash is order-dependent because subjects are written in slice order. +func TestRoleBindingHashFunc_SubjectOrderMatters(t *testing.T) { b1 := rbac.AdditionalRoleBindingsSpec{ ClusterRoleName: "admin", Subjects: []rbacv1.Subject{ @@ -120,6 +188,6 @@ func TestRoleBindingHashFunc_SubjectOrderMatters_CurrentBehavior(t *testing.T) { h2 := utils.RoleBindingHashFunc(b2) if h1 == h2 { - t.Fatalf("expected different hashes when subject order changes (current behavior), got %q", h1) + t.Fatalf("expected different hashes when subject order changes, got %q", h1) } }