diff --git a/charts/capsule/README.md b/charts/capsule/README.md index 6d00ef6f..018e745f 100644 --- a/charts/capsule/README.md +++ b/charts/capsule/README.md @@ -299,6 +299,14 @@ The following Values have changed key or Value: | webhooks.hooks.nodes.namespaceSelector | object | `{}` | [NamespaceSelector](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-namespaceselector) | | webhooks.hooks.nodes.objectSelector | object | `{}` | [ObjectSelector](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-objectselector) | | webhooks.hooks.nodes.opts | object | `{}` | Capsule Hook Options | +| webhooks.hooks.owners.enabled | bool | `true` | Enable the Hook | +| webhooks.hooks.owners.failurePolicy | string | `"Fail"` | [FailurePolicy](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#failure-policy) | +| webhooks.hooks.owners.matchConditions | list | `[]` | [MatchConditions](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-matchpolicy) | +| webhooks.hooks.owners.matchPolicy | string | `"Exact"` | [MatchPolicy](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-matchpolicy) | +| webhooks.hooks.owners.namespaceSelector | object | `{}` | [NamespaceSelector](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-namespaceselector) | +| webhooks.hooks.owners.objectSelector | object | `{}` | [ObjectSelector](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-objectselector) | +| webhooks.hooks.owners.opts | object | `{}` | Capsule Hook Options | +| webhooks.hooks.owners.reinvocationPolicy | string | `"Never"` | [ReinvocationPolicy](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#reinvocation-policy) | | webhooks.hooks.persistentvolumeclaims.enabled | bool | `true` | Enable the Hook | | webhooks.hooks.persistentvolumeclaims.failurePolicy | string | `"Fail"` | [FailurePolicy](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#failure-policy) | | webhooks.hooks.persistentvolumeclaims.matchConditions | list | `[]` | [MatchConditions](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-matchpolicy) | diff --git a/charts/capsule/templates/configuration.yaml b/charts/capsule/templates/configuration.yaml index 930f00cd..da5597bb 100644 --- a/charts/capsule/templates/configuration.yaml +++ b/charts/capsule/templates/configuration.yaml @@ -102,7 +102,7 @@ spec: resources: - namespaces - namespaces/status - - namespace/finalize + - namespaces/finalize scope: '*' sideEffects: None timeoutSeconds: {{ $.Values.webhooks.validatingWebhooksTimeoutSeconds }} @@ -577,6 +577,48 @@ spec: timeoutSeconds: {{ $.Values.webhooks.validatingWebhooksTimeoutSeconds }} {{- end }} {{- end }} + {{- with .Values.webhooks.hooks.owners }} + {{- if .enabled }} + {{- $any = true }} + - name: owners.validating.projectcapsule.dev + {{- with .opts }} + opts: + {{- toYaml . | nindent 10 }} + {{- end }} + admissionReviewVersions: + - v1 + - v1beta1 + path: "/tenantowners/validating" + failurePolicy: {{ .failurePolicy }} + matchPolicy: {{ .matchPolicy }} + {{- with .namespaceSelector }} + namespaceSelector: + {{- toYaml . | nindent 10 }} + {{- end }} + {{- with .objectSelector }} + objectSelector: + {{- toYaml . | nindent 10 }} + {{- end }} + {{- with .matchConditions }} + matchConditions: + {{- toYaml . | nindent 10 }} + {{- end }} + rules: + - apiGroups: + - capsule.clastix.io + apiVersions: + - v1beta2 + operations: + - CREATE + - UPDATE + - DELETE + resources: + - tenantowners + scope: 'Cluster' + sideEffects: None + timeoutSeconds: {{ $.Values.webhooks.validatingWebhooksTimeoutSeconds }} + {{- end }} + {{- end }} {{- with .Values.webhooks.hooks.resourcepools.pools }} {{- if .enabled }} {{- $any = true }} diff --git a/charts/capsule/templates/rbac.yaml b/charts/capsule/templates/rbac.yaml index 71927a98..7d0845f9 100644 --- a/charts/capsule/templates/rbac.yaml +++ b/charts/capsule/templates/rbac.yaml @@ -232,6 +232,8 @@ rules: - "" resources: - "namespaces" + - "namespaces/status" + - "namespaces/finalize" verbs: - "get" - "list" diff --git a/charts/capsule/values.schema.json b/charts/capsule/values.schema.json index 3a3cff6a..a7c79880 100644 --- a/charts/capsule/values.schema.json +++ b/charts/capsule/values.schema.json @@ -1739,6 +1739,43 @@ } } }, + "owners": { + "type": "object", + "properties": { + "enabled": { + "description": "Enable the Hook", + "type": "boolean" + }, + "failurePolicy": { + "description": "[FailurePolicy](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#failure-policy)", + "type": "string" + }, + "matchConditions": { + "description": "[MatchConditions](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-matchpolicy)", + "type": "array" + }, + "matchPolicy": { + "description": "[MatchPolicy](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-matchpolicy)", + "type": "string" + }, + "namespaceSelector": { + "description": "[NamespaceSelector](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-namespaceselector)", + "type": "object" + }, + "objectSelector": { + "description": "[ObjectSelector](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-objectselector)", + "type": "object" + }, + "opts": { + "description": "Capsule Hook Options", + "type": "object" + }, + "reinvocationPolicy": { + "description": "[ReinvocationPolicy](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#reinvocation-policy)", + "type": "string" + } + } + }, "persistentvolumeclaims": { "type": "object", "properties": { diff --git a/charts/capsule/values.yaml b/charts/capsule/values.yaml index b3d8176b..fb47320e 100644 --- a/charts/capsule/values.yaml +++ b/charts/capsule/values.yaml @@ -869,6 +869,26 @@ webhooks: # -- [ReinvocationPolicy](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#reinvocation-policy) reinvocationPolicy: Never + + owners: + # -- Enable the Hook + enabled: true + # -- Capsule Hook Options + opts: {} + # -- [FailurePolicy](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#failure-policy) + failurePolicy: Fail + # -- [MatchPolicy](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-matchpolicy) + matchPolicy: Exact + # -- [ObjectSelector](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-objectselector) + objectSelector: {} + # -- [NamespaceSelector](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-namespaceselector) + namespaceSelector: {} + # -- [MatchConditions](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-matchpolicy) + matchConditions: [] + # -- [ReinvocationPolicy](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#reinvocation-policy) + reinvocationPolicy: Never + + config: # -- Enable the Hook enabled: true diff --git a/cmd/controller/main.go b/cmd/controller/main.go index 94681d8e..a926cdec 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -69,6 +69,7 @@ import ( namespacemutation "github.com/projectcapsule/capsule/internal/webhook/namespace/mutation" namespacevalidation "github.com/projectcapsule/capsule/internal/webhook/namespace/validation" "github.com/projectcapsule/capsule/internal/webhook/node" + "github.com/projectcapsule/capsule/internal/webhook/owners" "github.com/projectcapsule/capsule/internal/webhook/pod" "github.com/projectcapsule/capsule/internal/webhook/pvc" "github.com/projectcapsule/capsule/internal/webhook/resourcepool" @@ -611,13 +612,16 @@ func main() { tenantvalidation.RuleHandler(), tenantvalidation.HostnameRegexHandler(), tenantvalidation.FreezedEmitter(), - tenantvalidation.ServiceAccountNameHandler(), + tenantvalidation.OwnersHandler(), tenantvalidation.ForbiddenAnnotationsRegexHandler(), tenantvalidation.ProtectedHandler(), tenantvalidation.RequiredMetadataHandler(), tenantvalidation.WarningHandler(cfg), ), ), + route.TenantOwnersValidation( + owners.UserMetadataHandler(), + ), route.NamespaceValidation( namespacevalidation.NamespaceHandler( cfg, @@ -661,6 +665,7 @@ func main() { cfgvalidation.Handler(cfg, cfgvalidation.WarningHandler(), cfgvalidation.ServiceAccountHandler(), + cfgvalidation.OwnerHandler(), ), ), ) diff --git a/e2e/namespace_hijacking_test.go b/e2e/namespace_hijacking_test.go index e278ac55..1c1cfa6c 100644 --- a/e2e/namespace_hijacking_test.go +++ b/e2e/namespace_hijacking_test.go @@ -104,6 +104,95 @@ var _ = Describe("creating several Namespaces for a Tenant", Ordered, Label("nam }, } + grantNamespaceSubresourceUpdate := func(name string, subject rbacv1.Subject) { + clusterRole := &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{ + "namespaces/status", + "namespaces/finalize", + }, + Verbs: []string{ + "get", + "update", + "patch", + }, + }, + { + APIGroups: []string{""}, + Resources: []string{ + "namespaces", + }, + Verbs: []string{ + "get", + "update", + "patch", + }, + }, + }, + } + + clusterRoleBinding := &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Subjects: []rbacv1.Subject{ + subject, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: name, + }, + } + + EventuallyCreation(func() error { + return k8sClient.Create(context.TODO(), clusterRole) + }).Should(Succeed()) + + EventuallyCreation(func() error { + return k8sClient.Create(context.TODO(), clusterRoleBinding) + }).Should(Succeed()) + } + + cleanupNamespaceSubresourceGrant := func(name string) { + Eventually(func() error { + return k8sClient.Delete(context.TODO(), &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + }) + }).Should(SatisfyAny(Succeed(), WithTransform(apierrors.IsNotFound, BeTrue()))) + + Eventually(func() error { + return k8sClient.Delete(context.TODO(), &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + }) + }).Should(SatisfyAny(Succeed(), WithTransform(apierrors.IsNotFound, BeTrue()))) + } + + expectNoTenantHijackPersisted := func(nsName string, originalTenant, attackerTenant *capsulev1beta2.Tenant) { + Eventually(func(g Gomega) { + current := &corev1.Namespace{} + Expect(k8sClient.Get(context.TODO(), types.NamespacedName{Name: nsName}, current)).Should(Succeed()) + + g.Expect(current.Labels).To(HaveKeyWithValue(meta.TenantLabel, originalTenant.GetName())) + g.Expect(current.Labels).NotTo(HaveKeyWithValue(meta.TenantLabel, attackerTenant.GetName())) + + g.Expect(hasTenantOwnerReferenceByNameAndUID(current, originalTenant.GetName(), originalTenant.GetUID())). + To(BeTrue(), "namespace should keep original tenant ownerReference") + + g.Expect(hasTenantOwnerReferenceByNameAndUID(current, attackerTenant.GetName(), attackerTenant.GetUID())). + To(BeFalse(), "namespace must not gain attacker tenant ownerReference") + }).Should(Succeed()) + } + kubeSystem := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: "kube-system", @@ -198,6 +287,177 @@ var _ = Describe("creating several Namespaces for a Tenant", Ordered, Label("nam EventuallyDeletion(t3) }) + It("Owners can not hijack Tenant ownership through namespaces/status", func() { + tenantA := getTenant(t1.Name) + tenantB := getTenant(t2.Name) + + owner := t1.Spec.Owners[0].UserSpec + cs := ownerClient(owner) + + grantName := "e2e-ns-status-hijack" + grantNamespaceSubresourceUpdate(grantName, rbacv1.Subject{ + APIGroup: rbacv1.GroupName, + Kind: rbacv1.UserKind, + Name: owner.Name, + }) + DeferCleanup(func() { + cleanupNamespaceSubresourceGrant(grantName) + }) + + ns := NewNamespace("", map[string]string{ + meta.TenantLabel: tenantA.GetName(), + }) + + NamespaceCreation(ns, owner, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(t1, ns).Should(Succeed()) + + current, err := cs.CoreV1().Namespaces().Get( + context.TODO(), + ns.Name, + metav1.GetOptions{}, + ) + Expect(err).ToNot(HaveOccurred()) + + hijacked := current.DeepCopy() + + if hijacked.Labels == nil { + hijacked.Labels = map[string]string{} + } + + hijacked.Labels[meta.TenantLabel] = tenantB.GetName() + + replaced := false + for i := range hijacked.OwnerReferences { + ref := &hijacked.OwnerReferences[i] + + if ref.APIVersion != capsulev1beta2.GroupVersion.String() { + continue + } + + if ref.Kind != "Tenant" { + continue + } + + if ref.Name != tenantA.GetName() { + continue + } + + if ref.UID != tenantA.GetUID() { + continue + } + + ref.Name = tenantB.GetName() + ref.UID = tenantB.GetUID() + replaced = true + + break + } + + Expect(replaced).To(BeTrue(), "expected test namespace to contain Tenant ownerReference for %q", tenantA.GetName()) + + Expect(hasTenantOwnerReferenceByNameAndUID(hijacked, tenantA.GetName(), tenantA.GetUID())). + To(BeFalse(), "hijack payload should no longer contain original Tenant ownerReference") + Expect(hasTenantOwnerReferenceByNameAndUID(hijacked, tenantB.GetName(), tenantB.GetUID())). + To(BeTrue(), "hijack payload should contain attacker Tenant ownerReference") + + _, err = cs.CoreV1().Namespaces().UpdateStatus( + context.TODO(), + hijacked, + metav1.UpdateOptions{}, + ) + + if err != nil { + By(fmt.Sprintf("namespaces/status hijack attempt was rejected: %v", err)) + } + + expectNoTenantHijackPersisted(ns.Name, tenantA, tenantB) + }) + It("Owners can not hijack Tenant ownership through namespaces/finalize", func() { + tenantA := getTenant(t1.Name) + tenantB := getTenant(t2.Name) + + owner := t1.Spec.Owners[0].UserSpec + cs := ownerClient(owner) + + grantName := "e2e-ns-finalize-hijack" + grantNamespaceSubresourceUpdate(grantName, rbacv1.Subject{ + APIGroup: rbacv1.GroupName, + Kind: rbacv1.UserKind, + Name: owner.Name, + }) + DeferCleanup(func() { + cleanupNamespaceSubresourceGrant(grantName) + }) + + ns := NewNamespace("", map[string]string{ + meta.TenantLabel: tenantA.GetName(), + }) + + NamespaceCreation(ns, owner, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(t1, ns).Should(Succeed()) + + current, err := cs.CoreV1().Namespaces().Get( + context.TODO(), + ns.Name, + metav1.GetOptions{}, + ) + Expect(err).ToNot(HaveOccurred()) + + hijacked := current.DeepCopy() + + if hijacked.Labels == nil { + hijacked.Labels = map[string]string{} + } + + hijacked.Labels[meta.TenantLabel] = tenantB.GetName() + + replaced := false + for i := range hijacked.OwnerReferences { + ref := &hijacked.OwnerReferences[i] + + if ref.APIVersion != capsulev1beta2.GroupVersion.String() { + continue + } + + if ref.Kind != "Tenant" { + continue + } + + if ref.Name != tenantA.GetName() { + continue + } + + if ref.UID != tenantA.GetUID() { + continue + } + + ref.Name = tenantB.GetName() + ref.UID = tenantB.GetUID() + replaced = true + + break + } + + Expect(replaced).To(BeTrue(), "expected test namespace to contain Tenant ownerReference for %q", tenantA.GetName()) + + Expect(hasTenantOwnerReferenceByNameAndUID(hijacked, tenantA.GetName(), tenantA.GetUID())). + To(BeFalse(), "hijack payload should no longer contain original Tenant ownerReference") + Expect(hasTenantOwnerReferenceByNameAndUID(hijacked, tenantB.GetName(), tenantB.GetUID())). + To(BeTrue(), "hijack payload should contain attacker Tenant ownerReference") + + _, err = cs.CoreV1().Namespaces().Finalize( + context.TODO(), + hijacked, + metav1.UpdateOptions{}, + ) + + if err != nil { + By(fmt.Sprintf("namespaces/finalize hijack attempt was rejected: %v", err)) + } + + expectNoTenantHijackPersisted(ns.Name, tenantA, tenantB) + }) + It("Owners can not add a second Tenant ownerReference to a managed namespace", func() { tenantA := getTenant(t1.Name) tenantB := getTenant(t2.Name) diff --git a/e2e/utils_test.go b/e2e/utils_test.go index 5a6e8bff..2fa4551a 100644 --- a/e2e/utils_test.go +++ b/e2e/utils_test.go @@ -1202,3 +1202,35 @@ func holdNamespaceTerminating(ctx context.Context, name string) func() { }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) } } + +func hasTenantOwnerReferenceByNameAndUID( + obj metav1.Object, + tenantName string, + tenantUID types.UID, +) bool { + if obj == nil { + return false + } + + for _, ref := range obj.GetOwnerReferences() { + if ref.APIVersion != capsulev1beta2.GroupVersion.String() { + continue + } + + if ref.Kind != "Tenant" { + continue + } + + if ref.Name != tenantName { + continue + } + + if ref.UID != tenantUID { + continue + } + + return true + } + + return false +} diff --git a/hack/distro/capsule/example-setup/owners.yaml b/hack/distro/capsule/example-setup/owners.yaml index 79c4b68f..cf472fb6 100644 --- a/hack/distro/capsule/example-setup/owners.yaml +++ b/hack/distro/capsule/example-setup/owners.yaml @@ -1,6 +1,16 @@ --- apiVersion: capsule.clastix.io/v1beta2 kind: TenantOwner +metadata: + labels: + team: devops + name: devops-user +spec: + kind: User + name: "some@user.com" +--- +apiVersion: capsule.clastix.io/v1beta2 +kind: TenantOwner metadata: labels: team: devops diff --git a/internal/controllers/rbac/manager.go b/internal/controllers/rbac/manager.go index a28a62ba..d8f1b29c 100644 --- a/internal/controllers/rbac/manager.go +++ b/internal/controllers/rbac/manager.go @@ -20,6 +20,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" @@ -107,6 +108,8 @@ func (r *Manager) Reconcile(ctx context.Context, request reconcile.Request) (res } func (r *Manager) EnsureClusterRoleBindingsProvisioner(ctx context.Context) error { + log := log.FromContext(ctx) + cfg := r.Configuration.RBAC() crb := &rbacv1.ClusterRoleBinding{ @@ -152,7 +155,14 @@ func (r *Manager) EnsureClusterRoleBindingsProvisioner(ctx context.Context) erro case rbac.ServiceAccountOwner: namespace, name, err := serviceaccount.SplitUsername(entity.Name) if err != nil { - return err + log.Error( + err, + "can not parse serviceaccount reference", + "subject", entity.Name, + "kind", entity.Kind, + ) + + continue } crb.Subjects = append(crb.Subjects, rbacv1.Subject{ diff --git a/internal/controllers/resources/global.go b/internal/controllers/resources/global.go index fdf0c934..d5e63125 100644 --- a/internal/controllers/resources/global.go +++ b/internal/controllers/resources/global.go @@ -627,7 +627,18 @@ func (r *globalResourceController) updateReconcilingStatus(ctx context.Context, latest.Status.Conditions.UpdateConditionByType(meta.NewReadyConditionReconcilingReason(instance)) - return r.client.Status().Update(ctx, latest) + if err := r.client.Status().Update(ctx, latest); err != nil { + if apierrors.IsNotFound(err) { + return nil + } + + return err + } + + // Keep the in-memory object aligned with what we just wrote. + instance.Status = latest.Status + + return nil }) } diff --git a/internal/controllers/resources/namespaced.go b/internal/controllers/resources/namespaced.go index 9a7a0a55..b9ea1260 100644 --- a/internal/controllers/resources/namespaced.go +++ b/internal/controllers/resources/namespaced.go @@ -635,7 +635,18 @@ func (r *namespacedResourceController) updateReconcilingStatus(ctx context.Conte latest.Status.Conditions.UpdateConditionByType(meta.NewReadyConditionReconcilingReason(instance)) - return r.client.Status().Update(ctx, latest) + if err := r.client.Status().Update(ctx, latest); err != nil { + if apierrors.IsNotFound(err) { + return nil + } + + return err + } + + // Keep the in-memory object aligned with what we just wrote. + instance.Status = latest.Status + + return nil }) } diff --git a/internal/controllers/tenant/namespaces.go b/internal/controllers/tenant/namespaces.go index 49000b31..0ce98507 100644 --- a/internal/controllers/tenant/namespaces.go +++ b/internal/controllers/tenant/namespaces.go @@ -25,28 +25,136 @@ import ( ) // Ensuring all annotations are applied to each Namespace handled by the Tenant. -func (r *Manager) reconcileNamespaces(ctx context.Context, log logr.Logger, tnt *capsulev1beta2.Tenant) (err error) { +func (r *Manager) reconcileNamespaces( + ctx context.Context, + log logr.Logger, + tnt *capsulev1beta2.Tenant, +) error { if tnt.DeletionTimestamp != nil { - for _, ns := range tnt.Status.Spaces { - ns := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: ns.Name, - }, - } - - if err := r.Delete(ctx, ns, &client.DeleteOptions{ - PropagationPolicy: ptr.To(metav1.DeletePropagationBackground), - }); err != nil && !apierrors.IsNotFound(err) { - log.Error(err, "unable to delete tenant namespace", - "tenant", tnt.GetName(), - "namespace", ns.Name, - ) - - return err - } - } + return r.reconcileDeletingTenantNamespaces(ctx, log, tnt) } + return r.reconcileActiveTenantNamespaces(ctx, log, tnt) +} + +func (r *Manager) reconcileDeletingTenantNamespaces( + ctx context.Context, + log logr.Logger, + tnt *capsulev1beta2.Tenant, +) error { + group, ctx := errgroup.WithContext(ctx) + group.SetLimit(8) + + results := make(chan *capsulev1beta2.TenantStatusNamespaceItem, len(tnt.Status.Spaces)) + removed := make(chan string, len(tnt.Status.Spaces)) + errs := make(chan error, len(tnt.Status.Spaces)) + + for i := range tnt.Status.Spaces { + statusNamespace := tnt.Status.Spaces[i] + + group.Go(func() error { + namespace := &corev1.Namespace{} + + err := r.Get(ctx, client.ObjectKey{Name: statusNamespace.Name}, namespace) + if apierrors.IsNotFound(err) { + removed <- statusNamespace.Name + + return nil + } + + if err != nil { + errs <- fmt.Errorf("get namespace %q: %w", statusNamespace.Name, err) + + return nil + } + + if namespace.DeletionTimestamp == nil { + if err := r.Delete(ctx, namespace, &client.DeleteOptions{ + PropagationPolicy: ptr.To(metav1.DeletePropagationBackground), + }); err != nil && !apierrors.IsNotFound(err) { + log.Error(err, "unable to delete tenant namespace", + "tenant", tnt.GetName(), + "namespace", namespace.GetName(), + ) + + errs <- fmt.Errorf("delete namespace %q: %w", namespace.Name, err) + + return nil + } + + latest := &corev1.Namespace{} + + err := r.Get(ctx, client.ObjectKey{Name: namespace.Name}, latest) + if apierrors.IsNotFound(err) { + removed <- statusNamespace.Name + + return nil + } + + if err != nil { + errs <- fmt.Errorf("get namespace %q after delete: %w", namespace.Name, err) + + return nil + } + + namespace = latest + } + + stat, err := r.reconcileNamespace(ctx, log, namespace, tnt) + if stat != nil { + results <- stat + } + + if err != nil { + log.Error(err, "failed to reconcile deleting namespace", + "tenant", tnt.GetName(), + "namespace", namespace.GetName(), + ) + + errs <- fmt.Errorf("namespace %q: %w", namespace.Name, err) + } + + return nil + }) + } + + _ = group.Wait() + + close(results) + close(removed) + close(errs) + + for name := range removed { + r.Metrics.DeleteAllMetricsForNamespace(name) + + tnt.Status.RemoveInstance(&capsulev1beta2.TenantStatusNamespaceItem{ + Name: name, + }) + } + + for stat := range results { + if stat == nil { + continue + } + + tnt.Status.UpdateInstance(stat) + } + + var joined []error + for itemErr := range errs { + joined = append(joined, itemErr) + } + + tnt.Status.Size = uint(len(tnt.Status.Spaces)) + + return errors.Join(joined...) +} + +func (r *Manager) reconcileActiveTenantNamespaces( + ctx context.Context, + log logr.Logger, + tnt *capsulev1beta2.Tenant, +) error { list := &corev1.NamespaceList{} if err := r.List(ctx, list, client.MatchingFields{".metadata.ownerReferences[*].capsule": tnt.GetName()}); err != nil { return err @@ -95,7 +203,7 @@ func (r *Manager) reconcileNamespaces(ctx context.Context, log logr.Logger, tnt joined = append(joined, itemErr) } - err = errors.Join(joined...) + err := errors.Join(joined...) desiredStatus := make(map[string]struct{}, len(list.Items)) @@ -126,7 +234,12 @@ func (r *Manager) reconcileNamespaces(ctx context.Context, log logr.Logger, tnt return err } -func (r *Manager) reconcileNamespace(ctx context.Context, log logr.Logger, namespace *corev1.Namespace, tnt *capsulev1beta2.Tenant) ( +func (r *Manager) reconcileNamespace( + ctx context.Context, + log logr.Logger, + namespace *corev1.Namespace, + tnt *capsulev1beta2.Tenant, +) ( stat *capsulev1beta2.TenantStatusNamespaceItem, err error, ) { @@ -144,18 +257,8 @@ func (r *Manager) reconcileNamespace(ctx context.Context, log logr.Logger, names stat = instance } - dropFromStatus := false - - // Always update tenant status condition after reconciliation + // Always update tenant status condition after reconciliation. defer func() { - if dropFromStatus { - stat = nil - - r.Metrics.DeleteAllMetricsForNamespace(namespace.GetName()) - - return - } - readCondition := meta.NewReadyCondition(namespace) switch { @@ -192,7 +295,7 @@ func (r *Manager) reconcileNamespace(ctx context.Context, log logr.Logger, names r.syncNamespaceStatusMetrics(tnt, namespace) }() - // Verify if namespace is still active or terminating + // Verify if namespace is still active or terminating. if namespace.DeletionTimestamp != nil { terminating = true @@ -236,17 +339,15 @@ func (r *Manager) reconcileNamespace(ctx context.Context, log logr.Logger, names return stat, nil } - terminatingState.Message = "removed managed resources" + terminatingState.Reason = meta.TerminatingReason + terminatingState.Status = metav1.ConditionFalse + terminatingState.Message = "waiting for namespace finalization" stat.Conditions.UpdateConditionByType(terminatingState) - r.Metrics.DeleteAllMetricsForNamespace(namespace.GetName()) - - dropFromStatus = true - - return nil, nil + return stat, nil } - // Collect Rules for namespace + // Collect Rules for namespace. err = r.reconcileRuleStatus(ctx, log, tnt, namespace) if err != nil { return stat, err @@ -292,7 +393,7 @@ func (r *Manager) reconcileNamespaceMetadata( managedMetadataOnly := tnt.Spec.NamespaceOptions != nil && tnt.Spec.NamespaceOptions.ManagedMetadataOnly - // Handle User-Defined Metadata, if allowed + // Handle User-Defined Metadata, if allowed. if !managedMetadataOnly { if originLabels != nil { maps.Copy(originLabels, managedLabels) @@ -302,7 +403,7 @@ func (r *Manager) reconcileNamespaceMetadata( maps.Copy(originAnnotations, managedAnnotations) } - // Cleanup old Metadata + // Cleanup old Metadata. instance := tnt.Status.GetInstance(stat) if instance != nil && instance.Metadata != nil { for label := range instance.Metadata.Labels { diff --git a/internal/webhook/tenant/validation/serviceaccount_format.go b/internal/webhook/cfg/owners.go similarity index 53% rename from internal/webhook/tenant/validation/serviceaccount_format.go rename to internal/webhook/cfg/owners.go index 67db9551..cadbb800 100644 --- a/internal/webhook/tenant/validation/serviceaccount_format.go +++ b/internal/webhook/cfg/owners.go @@ -1,11 +1,10 @@ // Copyright 2020-2026 Project Capsule Authors // SPDX-License-Identifier: Apache-2.0 -package validation +package cfg import ( "context" - "regexp" "k8s.io/client-go/tools/events" "sigs.k8s.io/controller-runtime/pkg/client" @@ -14,32 +13,31 @@ import ( capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" ad "github.com/projectcapsule/capsule/pkg/runtime/admission" "github.com/projectcapsule/capsule/pkg/runtime/handlers" + "github.com/projectcapsule/capsule/pkg/tenant" ) -var compiler *regexp.Regexp = regexp.MustCompile(`^.*:.*:.*(:.*)?$`) +type ownerHandler struct{} -type saNameHandler struct{} - -func ServiceAccountNameHandler() handlers.TypedHandler[*capsulev1beta2.Tenant] { - return &saNameHandler{} +func OwnerHandler() handlers.TypedHandler[*capsulev1beta2.CapsuleConfiguration] { + return &ownerHandler{} } -func (h *saNameHandler) OnCreate( +func (h *ownerHandler) OnCreate( _ client.Client, _ client.Reader, - tnt *capsulev1beta2.Tenant, + cfg *capsulev1beta2.CapsuleConfiguration, _ admission.Decoder, _ events.EventRecorder, ) handlers.Func { return func(_ context.Context, req admission.Request) *admission.Response { - return h.validateServiceAccountName(tnt, req) + return h.handle(cfg, req) } } -func (h *saNameHandler) OnDelete( +func (h *ownerHandler) OnDelete( client.Client, client.Reader, - *capsulev1beta2.Tenant, + *capsulev1beta2.CapsuleConfiguration, admission.Decoder, events.EventRecorder, ) handlers.Func { @@ -48,27 +46,25 @@ func (h *saNameHandler) OnDelete( } } -func (h *saNameHandler) OnUpdate( +func (h *ownerHandler) OnUpdate( _ client.Client, _ client.Reader, - tnt *capsulev1beta2.Tenant, - old *capsulev1beta2.Tenant, + cfg *capsulev1beta2.CapsuleConfiguration, + old *capsulev1beta2.CapsuleConfiguration, _ admission.Decoder, _ events.EventRecorder, ) handlers.Func { return func(_ context.Context, req admission.Request) *admission.Response { - return h.validateServiceAccountName(tnt, req) + return h.handle(cfg, req) } } -func (h *saNameHandler) validateServiceAccountName(tnt *capsulev1beta2.Tenant, req admission.Request) *admission.Response { - for _, owner := range tnt.Spec.Owners { - if owner.Kind != "ServiceAccount" { - continue - } - - if !compiler.MatchString(owner.Name) { - return ad.Denyf("owner name %s is not a valid Service Account name", owner.Name) +func (h *ownerHandler) handle(config *capsulev1beta2.CapsuleConfiguration, req admission.Request) *admission.Response { + for _, owner := range config.Spec.Users { + if err := tenant.ValidateTenantOwner(owner); err != nil { + return ad.Deny( + err.Error(), + ) } } diff --git a/internal/webhook/owners/subject_validation.go b/internal/webhook/owners/subject_validation.go new file mode 100644 index 00000000..7d686280 --- /dev/null +++ b/internal/webhook/owners/subject_validation.go @@ -0,0 +1,78 @@ +// Copyright 2020-2026 Project Capsule Authors +// SPDX-License-Identifier: Apache-2.0 + +package owners + +import ( + "context" + + "k8s.io/client-go/tools/events" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" + ad "github.com/projectcapsule/capsule/pkg/runtime/admission" + "github.com/projectcapsule/capsule/pkg/runtime/handlers" + "github.com/projectcapsule/capsule/pkg/tenant" +) + +type ownerSubjectHandler struct{} + +func UserMetadataHandler() handlers.Handler { + return &ownerSubjectHandler{} +} + +func (r *ownerSubjectHandler) OnCreate( + _ client.Client, + _ client.Reader, + decoder admission.Decoder, + _ events.EventRecorder, +) handlers.Func { + return func(_ context.Context, req admission.Request) *admission.Response { + owner := &capsulev1beta2.TenantOwner{} + if err := decoder.Decode(req, owner); err != nil { + return ad.ErroredResponse(err) + } + + return r.handle(owner) + } +} + +func (r *ownerSubjectHandler) OnUpdate( + _ client.Client, + _ client.Reader, + decoder admission.Decoder, + recorder events.EventRecorder, +) handlers.Func { + return func(_ context.Context, req admission.Request) *admission.Response { + owner := &capsulev1beta2.TenantOwner{} + if err := decoder.Decode(req, owner); err != nil { + return ad.ErroredResponse(err) + } + + return r.handle(owner) + } +} + +func (r *ownerSubjectHandler) OnDelete( + client.Client, + client.Reader, + admission.Decoder, + events.EventRecorder, +) handlers.Func { + return func(context.Context, admission.Request) *admission.Response { + return nil + } +} + +func (h *ownerSubjectHandler) handle( + owner *capsulev1beta2.TenantOwner, +) *admission.Response { + if err := tenant.ValidateTenantOwner(owner.Spec.UserSpec); err != nil { + return ad.Deny( + err.Error(), + ) + } + + return nil +} diff --git a/internal/webhook/route/tenant_owners.go b/internal/webhook/route/tenant_owners.go new file mode 100644 index 00000000..cc2594f8 --- /dev/null +++ b/internal/webhook/route/tenant_owners.go @@ -0,0 +1,22 @@ +// Copyright 2020-2026 Project Capsule Authors +// SPDX-License-Identifier: Apache-2.0 + +package route + +import "github.com/projectcapsule/capsule/pkg/runtime/handlers" + +type tenantOwnersValidating struct { + handlers []handlers.Handler +} + +func TenantOwnersValidation(handler ...handlers.Handler) handlers.Webhook { + return &tenantOwnersValidating{handlers: handler} +} + +func (w *tenantOwnersValidating) GetHandlers() []handlers.Handler { + return w.handlers +} + +func (w *tenantOwnersValidating) GetPath() string { + return "/tenantowners/validating" +} diff --git a/internal/webhook/tenant/validation/owners.go b/internal/webhook/tenant/validation/owners.go new file mode 100644 index 00000000..4e722d8a --- /dev/null +++ b/internal/webhook/tenant/validation/owners.go @@ -0,0 +1,78 @@ +// Copyright 2020-2026 Project Capsule Authors +// SPDX-License-Identifier: Apache-2.0 + +package validation + +import ( + "context" + + "k8s.io/client-go/tools/events" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" + ad "github.com/projectcapsule/capsule/pkg/runtime/admission" + "github.com/projectcapsule/capsule/pkg/runtime/handlers" + "github.com/projectcapsule/capsule/pkg/tenant" +) + +type ownersHandler struct{} + +func OwnersHandler() handlers.TypedHandler[*capsulev1beta2.Tenant] { + return &ownersHandler{} +} + +func (h *ownersHandler) OnCreate( + _ client.Client, + _ client.Reader, + tnt *capsulev1beta2.Tenant, + _ admission.Decoder, + _ events.EventRecorder, +) handlers.Func { + return func(context.Context, admission.Request) *admission.Response { + return h.handle(tnt) + } +} + +func (h *ownersHandler) OnDelete( + _ client.Client, + _ client.Reader, + tnt *capsulev1beta2.Tenant, + _ admission.Decoder, + _ events.EventRecorder, +) handlers.Func { + return func(ctx context.Context, req admission.Request) *admission.Response { + if tnt.Spec.PreventDeletion { + return ad.Deny("tenant is protected and cannot be deleted") + } + + return nil + } +} + +func (h *ownersHandler) OnUpdate( + _ client.Client, + _ client.Reader, + tnt *capsulev1beta2.Tenant, + _ *capsulev1beta2.Tenant, + _ admission.Decoder, + _ events.EventRecorder, +) handlers.Func { + return func(context.Context, admission.Request) *admission.Response { + return h.handle(tnt) + } +} + +func (h *ownersHandler) handle( + tnt *capsulev1beta2.Tenant, +) *admission.Response { + for _, owner := range tnt.Spec.Owners { + if err := tenant.ValidateTenantOwner(owner.UserSpec); err != nil { + return ad.Deny( + err.Error(), + ) + } + } + + return nil +} diff --git a/internal/webhook/tenant/validation/racing_namespaces.go b/internal/webhook/tenant/validation/racing_namespaces.go deleted file mode 100644 index b0bb063b..00000000 --- a/internal/webhook/tenant/validation/racing_namespaces.go +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright 2020-2026 Project Capsule Authors -// SPDX-License-Identifier: Apache-2.0 - -package validation - -import ( - "context" - - corev1 "k8s.io/api/core/v1" - "k8s.io/client-go/tools/events" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" - ad "github.com/projectcapsule/capsule/pkg/runtime/admission" - "github.com/projectcapsule/capsule/pkg/runtime/handlers" - namespaceindex "github.com/projectcapsule/capsule/pkg/runtime/indexers/namespace" -) - -type remainingNamespaceHandler struct{} - -func RemainingNamespaceHandler() handlers.TypedHandler[*capsulev1beta2.Tenant] { - return &remainingNamespaceHandler{} -} - -func (h *remainingNamespaceHandler) OnCreate( - client.Client, - client.Reader, - *capsulev1beta2.Tenant, - admission.Decoder, - events.EventRecorder, -) handlers.Func { - return func(context.Context, admission.Request) *admission.Response { - return nil - } -} - -// This happens when a tenant has not yet reconciled it's namespaces but is deleted -// and in the meantime a new namespace was created referencing the same tenant. -func (h *remainingNamespaceHandler) OnDelete( - c client.Client, - _ client.Reader, - tnt *capsulev1beta2.Tenant, - _ admission.Decoder, - _ events.EventRecorder, -) handlers.Func { - return func(ctx context.Context, req admission.Request) *admission.Response { - list := &corev1.NamespaceList{} - - err := c.List(ctx, list, client.MatchingFields{namespaceindex.OwnerReferenceIndex: tnt.GetName()}) - if err != nil { - return ad.ErroredResponse(err) - } - - if len(list.Items) == 0 { - return nil - } - - for _, ns := range list.Items { - instance := tnt.Status.GetInstance(&capsulev1beta2.TenantStatusNamespaceItem{ - Name: ns.GetName(), - UID: ns.GetUID(), - }) - - if instance == nil { - return ad.Deny("tenant has remaining namespace referencing it (" + ns.GetName() + ")") - } - } - - return nil - } -} - -func (h *remainingNamespaceHandler) OnUpdate( - client.Client, - client.Reader, - *capsulev1beta2.Tenant, - *capsulev1beta2.Tenant, - admission.Decoder, - events.EventRecorder, -) handlers.Func { - return func(context.Context, admission.Request) *admission.Response { - return nil - } -} diff --git a/pkg/runtime/configuration/client.go b/pkg/runtime/configuration/client.go index 9250c021..3aa82812 100644 --- a/pkg/runtime/configuration/client.go +++ b/pkg/runtime/configuration/client.go @@ -14,6 +14,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apiserver/pkg/authentication/serviceaccount" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" @@ -164,6 +165,18 @@ func (c *capsuleConfiguration) Users() rbac.UserListSpec { out := rbac.UserListSpec{} for _, user := range c.UserNames() { + // Old Spec.UserNames may contain ServiceAccount usernames. + // If SplitUsername succeeds, the value is a ServiceAccount. + _, _, err := serviceaccount.SplitUsername(user) + if err == nil { + out.Upsert(rbac.UserSpec{ + Kind: rbac.ServiceAccountOwner, + Name: user, + }) + + continue + } + out.Upsert(rbac.UserSpec{ Kind: rbac.UserOwner, Name: user, diff --git a/pkg/tenant/owners.go b/pkg/tenant/owners.go index a6c25455..7561432c 100644 --- a/pkg/tenant/owners.go +++ b/pkg/tenant/owners.go @@ -86,3 +86,14 @@ func GetOwnersWithKinds(tenant *capsulev1beta2.Tenant) (owners []string) { func OwnerKindIndexKey(kind, name string) string { return fmt.Sprintf("%s:%s", kind, name) } + +func ValidateTenantOwner(owner rbac.UserSpec) error { + if owner.Kind == rbac.ServiceAccountOwner { + _, _, err := serviceaccount.SplitUsername(owner.Name) + if err != nil { + return err + } + } + + return nil +} diff --git a/pkg/tenant/owners_test.go b/pkg/tenant/owners_test.go index 9e998a87..8d80c688 100644 --- a/pkg/tenant/owners_test.go +++ b/pkg/tenant/owners_test.go @@ -97,3 +97,133 @@ func TestGetOwnersWithKinds_EmptyNameStillIncluded(t *testing.T) { t.Fatalf("unexpected owners:\nwant=%v\ngot =%v", want, owners) } } + +func TestValidateTenantOwner(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + owner rbac.CoreOwnerSpec + wantErr bool + }{ + { + name: "valid service account owner", + owner: rbac.CoreOwnerSpec{ + UserSpec: rbac.UserSpec{ + Kind: rbac.ServiceAccountOwner, + Name: "system:serviceaccount:tenant-a:builder", + }, + }, + wantErr: false, + }, + { + name: "invalid service account owner without serviceaccount prefix", + owner: rbac.CoreOwnerSpec{ + UserSpec: rbac.UserSpec{ + Kind: rbac.ServiceAccountOwner, + Name: "tenant-a:builder", + }, + }, + wantErr: true, + }, + { + name: "invalid service account owner with missing namespace", + owner: rbac.CoreOwnerSpec{ + UserSpec: rbac.UserSpec{ + Kind: rbac.ServiceAccountOwner, + Name: "system:serviceaccount::builder", + }, + }, + wantErr: true, + }, + { + name: "invalid service account owner with missing name", + owner: rbac.CoreOwnerSpec{ + UserSpec: rbac.UserSpec{ + Kind: rbac.ServiceAccountOwner, + Name: "system:serviceaccount:tenant-a:", + }, + }, + wantErr: true, + }, + { + name: "invalid service account owner with plain name", + owner: rbac.CoreOwnerSpec{ + UserSpec: rbac.UserSpec{ + Kind: rbac.ServiceAccountOwner, + Name: "builder", + }, + }, + wantErr: true, + }, + { + name: "user owner is not validated as service account", + owner: rbac.CoreOwnerSpec{ + UserSpec: rbac.UserSpec{ + Kind: rbac.UserOwner, + Name: "alice", + }, + }, + wantErr: false, + }, + { + name: "group owner is not validated as service account", + owner: rbac.CoreOwnerSpec{ + UserSpec: rbac.UserSpec{ + Kind: rbac.GroupOwner, + Name: "developers", + }, + }, + wantErr: false, + }, + { + name: "empty user owner name is currently accepted", + owner: rbac.CoreOwnerSpec{ + UserSpec: rbac.UserSpec{ + Kind: rbac.UserOwner, + Name: "", + }, + }, + wantErr: false, + }, + { + name: "empty group owner name is currently accepted", + owner: rbac.CoreOwnerSpec{ + UserSpec: rbac.UserSpec{ + Kind: rbac.GroupOwner, + Name: "", + }, + }, + wantErr: false, + }, + { + name: "empty service account owner name is rejected", + owner: rbac.CoreOwnerSpec{ + UserSpec: rbac.UserSpec{ + + Kind: rbac.ServiceAccountOwner, + Name: "", + }, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := tenant.ValidateTenantOwner(tt.owner.UserSpec) + + if tt.wantErr && err == nil { + t.Fatalf("ValidateTenantOwner() expected error, got nil") + } + + if !tt.wantErr && err != nil { + t.Fatalf("ValidateTenantOwner() unexpected error: %v", err) + } + }) + } +}