diff --git a/config/controller-manager/overlays/core-control-plane/rbac/role.yaml b/config/controller-manager/overlays/core-control-plane/rbac/role.yaml index b13933b7..132ff34b 100644 --- a/config/controller-manager/overlays/core-control-plane/rbac/role.yaml +++ b/config/controller-manager/overlays/core-control-plane/rbac/role.yaml @@ -94,6 +94,7 @@ rules: resources: - groups/finalizers - platforminvitations/status + - userinvitations/finalizers - userinvitations/status verbs: - update @@ -149,6 +150,7 @@ rules: - delete - get - list + - patch - update - watch - apiGroups: @@ -295,6 +297,7 @@ rules: - organizationmemberships verbs: - create + - delete - get - list - watch diff --git a/internal/controllers/iam/user_controller.go b/internal/controllers/iam/user_controller.go index b1d1d05a..8b619d0c 100644 --- a/internal/controllers/iam/user_controller.go +++ b/internal/controllers/iam/user_controller.go @@ -6,6 +6,7 @@ import ( "strings" iamv1alpha1 "go.miloapis.com/milo/pkg/apis/iam/v1alpha1" + resourcemanagerv1alpha1 "go.miloapis.com/milo/pkg/apis/resourcemanager/v1alpha1" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -13,13 +14,16 @@ import ( "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) const ( - userFinalizerKey = "iam.miloapis.com/user" + // userMembershipCleanupFinalizer ensures OrganizationMembership resources are + // deleted before the User object is removed from the API server. + userMembershipCleanupFinalizer = "iam.miloapis.com/user-membership-cleanup" userReadyConditionType = "Ready" platformAccessApprovalIndexKey = "iam.miloapis.com/platformaccessapprovalkey" platformAccessRejectionIndexKey = "iam.miloapis.com/platformaccessrejectionkey" @@ -44,6 +48,7 @@ type UserController struct { // +kubebuilder:rbac:groups=iam.miloapis.com,resources=userpreferences,verbs=get;list;watch;update;patch // +kubebuilder:rbac:groups=iam.miloapis.com,resources=platformaccessapprovals,verbs=get;list;watch // +kubebuilder:rbac:groups=iam.miloapis.com,resources=platformaccessrejections,verbs=get;list;watch +// +kubebuilder:rbac:groups=resourcemanager.miloapis.com,resources=organizationmemberships,verbs=list;delete // Reconcile is the main reconciliation loop for the UserController. func (r *UserController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { @@ -59,12 +64,31 @@ func (r *UserController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. } log.Info("reconciling User", "user", user.Name) - // Stop reconciling if deletion in progress. + // When the user is being deleted, clean up OrganizationMembership resources + // before the object is removed. if !user.DeletionTimestamp.IsZero() { - log.Info("User is being deleted, skipping reconciliation", "user", user.Name) + if controllerutil.ContainsFinalizer(user, userMembershipCleanupFinalizer) { + if err := r.cleanupOrganizationMemberships(ctx, user); err != nil { + log.Error(err, "failed to clean up OrganizationMemberships during user deletion") + return ctrl.Result{}, err + } + controllerutil.RemoveFinalizer(user, userMembershipCleanupFinalizer) + if err := r.Client.Update(ctx, user); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to remove membership cleanup finalizer: %w", err) + } + } return ctrl.Result{}, nil } + // Ensure the membership-cleanup finalizer is present on every active User so + // that OrganizationMemberships are deleted before the User is removed. + if !controllerutil.ContainsFinalizer(user, userMembershipCleanupFinalizer) { + controllerutil.AddFinalizer(user, userMembershipCleanupFinalizer) + if err := r.Client.Update(ctx, user); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to add membership cleanup finalizer: %w", err) + } + } + // Ensure owner references are set on PolicyBinding and UserPreference resources if err := r.ensureOwnerReferences(ctx, user); err != nil { log.Error(err, "Failed to ensure owner references") @@ -237,6 +261,33 @@ func (r *UserController) ensureOwnerReferences(ctx context.Context, user *iamv1a return nil } +// cleanupOrganizationMemberships deletes all OrganizationMembership resources that +// reference the given user. This is called when a user is being deleted so that +// memberships (including last-owner memberships) are removed before the User +// object is garbage-collected. +// +// Depends on the "spec.userRef.name" field index registered by +// OrganizationMembershipController.SetupWithManager. Both controllers share the +// same manager, so the index is available when this function runs. +func (r *UserController) cleanupOrganizationMemberships(ctx context.Context, user *iamv1alpha1.User) error { + log := log.FromContext(ctx).WithName("cleanup-organization-memberships") + + var membershipList resourcemanagerv1alpha1.OrganizationMembershipList + if err := r.Client.List(ctx, &membershipList, client.MatchingFields{"spec.userRef.name": user.Name}); err != nil { + return fmt.Errorf("failed to list OrganizationMemberships for user %s: %w", user.Name, err) + } + + for i := range membershipList.Items { + membership := &membershipList.Items[i] + log.Info("deleting OrganizationMembership for deleted user", "membership", membership.Name, "namespace", membership.Namespace) + if err := r.Client.Delete(ctx, membership); err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to delete OrganizationMembership %s/%s: %w", membership.Namespace, membership.Name, err) + } + } + + return nil +} + // hasOwnerReference checks if the owner reference already exists func hasOwnerReference(refs []metav1.OwnerReference, ref metav1.OwnerReference) bool { for _, r := range refs { diff --git a/internal/controllers/iam/userinvitation_controller.go b/internal/controllers/iam/userinvitation_controller.go index 4b546b44..81a7f2ef 100644 --- a/internal/controllers/iam/userinvitation_controller.go +++ b/internal/controllers/iam/userinvitation_controller.go @@ -541,7 +541,7 @@ func (r *UserInvitationController) createOrganizationMembership(ctx context.Cont Namespace: fmt.Sprintf("organization-%s", ui.Spec.OrganizationRef.Name), OwnerReferences: []metav1.OwnerReference{ { - APIVersion: iamv1alpha1.SchemeGroupVersion.Group, + APIVersion: iamv1alpha1.SchemeGroupVersion.String(), Kind: "User", Name: user.GetName(), UID: user.GetUID(), diff --git a/internal/controllers/resourcemanager/organization_membership_controller.go b/internal/controllers/resourcemanager/organization_membership_controller.go index 98fff416..75e6dc42 100644 --- a/internal/controllers/resourcemanager/organization_membership_controller.go +++ b/internal/controllers/resourcemanager/organization_membership_controller.go @@ -131,6 +131,22 @@ func (r *OrganizationMembershipController) Reconcile(ctx context.Context, req ct if err := r.Client.Get(ctx, userKey, &user); err != nil { if apierrors.IsNotFound(err) { logger.Info("referenced user not found", "user", organizationMembership.Spec.UserRef.Name) + + // Two-pass self-delete: if we already set UserNotFound on a previous + // reconcile, delete the membership now so it does not linger after + // the owning User is gone. The second reconcile is triggered by the + // status condition update below, which re-enqueues the membership. + existingCondition := apimeta.FindStatusCondition(organizationMembership.Status.Conditions, OrganizationMembershipReady) + if existingCondition != nil && existingCondition.Reason == UserNotFoundReason { + logger.Info("deleting OrganizationMembership because referenced user no longer exists", + "membership", organizationMembership.Name, + "user", organizationMembership.Spec.UserRef.Name) + if err := r.Client.Delete(ctx, &organizationMembership); err != nil && !apierrors.IsNotFound(err) { + return ctrl.Result{}, fmt.Errorf("failed to self-delete organization membership: %w", err) + } + return ctrl.Result{}, nil + } + readyCondition.Status = metav1.ConditionFalse readyCondition.Reason = UserNotFoundReason readyCondition.Message = fmt.Sprintf("User '%s' does not exist. Please ensure the user name is correct and the user account has been created.", organizationMembership.Spec.UserRef.Name) diff --git a/internal/webhooks/resourcemanager/v1alpha1/organization_webhook.go b/internal/webhooks/resourcemanager/v1alpha1/organization_webhook.go index 4a89a7f8..247ddee1 100644 --- a/internal/webhooks/resourcemanager/v1alpha1/organization_webhook.go +++ b/internal/webhooks/resourcemanager/v1alpha1/organization_webhook.go @@ -181,6 +181,14 @@ func (v *OrganizationValidator) createOrganizationMembership(ctx context.Context ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("member-%s", user.Name), Namespace: fmt.Sprintf("organization-%s", org.Name), + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: iamv1alpha1.SchemeGroupVersion.String(), + Kind: "User", + Name: user.Name, + UID: user.UID, + }, + }, }, Spec: resourcemanagerv1alpha1.OrganizationMembershipSpec{ OrganizationRef: resourcemanagerv1alpha1.OrganizationReference{ diff --git a/internal/webhooks/resourcemanager/v1alpha1/organizationmembership_webhook.go b/internal/webhooks/resourcemanager/v1alpha1/organizationmembership_webhook.go index ce0f2586..f80fb41f 100644 --- a/internal/webhooks/resourcemanager/v1alpha1/organizationmembership_webhook.go +++ b/internal/webhooks/resourcemanager/v1alpha1/organizationmembership_webhook.go @@ -30,6 +30,7 @@ func SetupOrganizationMembershipWebhooksWithManager(mgr ctrl.Manager, organizati For(&resourcemanagerv1alpha1.OrganizationMembership{}). WithValidator(&OrganizationMembershipValidator{ client: mgr.GetClient(), + apiReader: mgr.GetAPIReader(), ownerRoleName: organizationOwnerRoleName, ownerRoleNamespace: organizationOwnerRoleNamespace, }). @@ -39,6 +40,7 @@ func SetupOrganizationMembershipWebhooksWithManager(mgr ctrl.Manager, organizati // OrganizationMembershipValidator validates OrganizationMemberships type OrganizationMembershipValidator struct { client client.Client + apiReader client.Reader decoder admission.Decoder ownerRoleName string ownerRoleNamespace string @@ -85,6 +87,13 @@ func (v *OrganizationMembershipValidator) ValidateDelete(ctx context.Context, ob return nil, nil } + // Allow deletion when the referenced user is itself being deleted — the + // UserController is responsible for cleaning up orphaned memberships and + // must not be blocked by the last-owner guard. + if v.allowDeletionBecauseUserIsBeingDeleted(ctx, membership) { + return nil, nil + } + if v.allowOwnerDeletionDuringTeardown(ctx, membership) { return nil, nil } @@ -141,6 +150,33 @@ func (v *OrganizationMembershipValidator) allowOwnerDeletionDuringTeardown(ctx c return organization.DeletionTimestamp != nil } +// allowDeletionBecauseUserIsBeingDeleted returns true when the membership should be +// allowed to be deleted because the referenced user has a non-zero DeletionTimestamp. +// The UserController adds a finalizer and drives membership cleanup; we must not +// block it with the last-owner guard. +// +// Uses the direct API reader (not the informer cache) to avoid stale reads that +// could incorrectly bypass the last-owner business invariant. +func (v *OrganizationMembershipValidator) allowDeletionBecauseUserIsBeingDeleted(ctx context.Context, membership *resourcemanagerv1alpha1.OrganizationMembership) bool { + userName := membership.Spec.UserRef.Name + if userName == "" { + return false + } + + var user iamv1alpha1.User + if err := v.apiReader.Get(ctx, client.ObjectKey{Name: userName}, &user); err != nil { + if apierrors.IsNotFound(err) { + // User is already gone — allow the membership deletion. + return true + } + organizationmembershiplog.Error(err, "failed to fetch user while validating delete", + "user", userName) + return false + } + + return user.DeletionTimestamp != nil +} + // isNamespaceTerminating returns true if the namespace is terminating or already deleted. func (v *OrganizationMembershipValidator) isNamespaceTerminating(ctx context.Context, namespace string) bool { if namespace == "" { diff --git a/internal/webhooks/resourcemanager/v1alpha1/organizationmembership_webhook_test.go b/internal/webhooks/resourcemanager/v1alpha1/organizationmembership_webhook_test.go index 67f6165d..5cf94635 100644 --- a/internal/webhooks/resourcemanager/v1alpha1/organizationmembership_webhook_test.go +++ b/internal/webhooks/resourcemanager/v1alpha1/organizationmembership_webhook_test.go @@ -72,6 +72,7 @@ func TestOrganizationMembershipValidator_ValidateCreate_Success(t *testing.T) { validator := &OrganizationMembershipValidator{ client: c, + apiReader: c, ownerRoleName: "resourcemanager.miloapis.com-organizationowner", ownerRoleNamespace: "milo-system", } @@ -122,6 +123,7 @@ func TestOrganizationMembershipValidator_ValidateCreate_DuplicateRoles(t *testin validator := &OrganizationMembershipValidator{ client: c, + apiReader: c, ownerRoleName: "resourcemanager.miloapis.com-organizationowner", ownerRoleNamespace: "milo-system", } @@ -168,6 +170,7 @@ func TestOrganizationMembershipValidator_ValidateCreate_NonexistentRole(t *testi validator := &OrganizationMembershipValidator{ client: c, + apiReader: c, ownerRoleName: "resourcemanager.miloapis.com-organizationowner", ownerRoleNamespace: "milo-system", } @@ -214,6 +217,7 @@ func TestOrganizationMembershipValidator_ValidateCreate_EmptyRoleName(t *testing validator := &OrganizationMembershipValidator{ client: c, + apiReader: c, ownerRoleName: "resourcemanager.miloapis.com-organizationowner", ownerRoleNamespace: "milo-system", } @@ -260,6 +264,7 @@ func TestOrganizationMembershipValidator_ValidateDelete_AllowsNonOwner(t *testin validator := &OrganizationMembershipValidator{ client: c, + apiReader: c, ownerRoleName: "resourcemanager.miloapis.com-organizationowner", ownerRoleNamespace: "milo-system", } @@ -322,6 +327,7 @@ func TestOrganizationMembershipValidator_ValidateDelete_AllowsWhenAnotherOwnerEx validator := &OrganizationMembershipValidator{ client: c, + apiReader: c, ownerRoleName: "resourcemanager.miloapis.com-organizationowner", ownerRoleNamespace: "milo-system", } @@ -390,13 +396,22 @@ func TestOrganizationMembershipValidator_ValidateDelete_BlocksLastOwner(t *testi }, } + // The user must exist without a DeletionTimestamp so the webhook does not + // bypass the last-owner guard. + alice := &iamv1alpha1.User{ + ObjectMeta: metav1.ObjectMeta{ + Name: "alice", + }, + } + c := fake.NewClientBuilder(). WithScheme(scheme). - WithObjects(target, nonOwner, namespace, organization). + WithObjects(target, nonOwner, namespace, organization, alice). Build() validator := &OrganizationMembershipValidator{ client: c, + apiReader: c, ownerRoleName: "resourcemanager.miloapis.com-organizationowner", ownerRoleNamespace: "milo-system", } @@ -460,6 +475,7 @@ func TestOrganizationMembershipValidator_ValidateDelete_AllowsWhenNamespaceTermi validator := &OrganizationMembershipValidator{ client: c, + apiReader: c, ownerRoleName: "resourcemanager.miloapis.com-organizationowner", ownerRoleNamespace: "milo-system", } @@ -516,6 +532,7 @@ func TestOrganizationMembershipValidator_ValidateDelete_AllowsWhenOrganizationDe validator := &OrganizationMembershipValidator{ client: c, + apiReader: c, ownerRoleName: "resourcemanager.miloapis.com-organizationowner", ownerRoleNamespace: "milo-system", } @@ -562,6 +579,7 @@ func TestOrganizationMembershipValidator_ValidateDelete_AllowsWhenOrganizationMi validator := &OrganizationMembershipValidator{ client: c, + apiReader: c, ownerRoleName: "resourcemanager.miloapis.com-organizationowner", ownerRoleNamespace: "milo-system", } @@ -571,6 +589,106 @@ func TestOrganizationMembershipValidator_ValidateDelete_AllowsWhenOrganizationMi } } +func TestOrganizationMembershipValidator_ValidateDelete_AllowsWhenUserIsBeingDeleted(t *testing.T) { + ctx := context.TODO() + scheme := getWebhookTestScheme() + + now := metav1.Now() + user := &iamv1alpha1.User{ + ObjectMeta: metav1.ObjectMeta{ + Name: "alice", + DeletionTimestamp: &now, + Finalizers: []string{"some-finalizer"}, + }, + } + + target := &resourcemanagerv1alpha1.OrganizationMembership{ + ObjectMeta: metav1.ObjectMeta{ + Name: "member-alice", + Namespace: "organization-test", + }, + Spec: resourcemanagerv1alpha1.OrganizationMembershipSpec{ + OrganizationRef: resourcemanagerv1alpha1.OrganizationReference{ + Name: "test-org", + }, + UserRef: resourcemanagerv1alpha1.MemberReference{ + Name: "alice", + }, + Roles: []resourcemanagerv1alpha1.RoleReference{ + { + Name: "resourcemanager.miloapis.com-organizationowner", + Namespace: "milo-system", + }, + }, + }, + } + + namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "organization-test"}} + org := &resourcemanagerv1alpha1.Organization{ObjectMeta: metav1.ObjectMeta{Name: "test-org"}} + + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(target, user, namespace, org). + Build() + + validator := &OrganizationMembershipValidator{ + client: c, + apiReader: c, + ownerRoleName: "resourcemanager.miloapis.com-organizationowner", + ownerRoleNamespace: "milo-system", + } + + if _, err := validator.ValidateDelete(ctx, target); err != nil { + t.Fatalf("expected deletion to succeed when user has a DeletionTimestamp, got error: %v", err) + } +} + +func TestOrganizationMembershipValidator_ValidateDelete_AllowsWhenUserAlreadyGone(t *testing.T) { + ctx := context.TODO() + scheme := getWebhookTestScheme() + + target := &resourcemanagerv1alpha1.OrganizationMembership{ + ObjectMeta: metav1.ObjectMeta{ + Name: "member-alice", + Namespace: "organization-test", + }, + Spec: resourcemanagerv1alpha1.OrganizationMembershipSpec{ + OrganizationRef: resourcemanagerv1alpha1.OrganizationReference{ + Name: "test-org", + }, + UserRef: resourcemanagerv1alpha1.MemberReference{ + Name: "alice", + }, + Roles: []resourcemanagerv1alpha1.RoleReference{ + { + Name: "resourcemanager.miloapis.com-organizationowner", + Namespace: "milo-system", + }, + }, + }, + } + + namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "organization-test"}} + org := &resourcemanagerv1alpha1.Organization{ObjectMeta: metav1.ObjectMeta{Name: "test-org"}} + + // User is not added to the client — simulating a user that was already deleted. + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(target, namespace, org). + Build() + + validator := &OrganizationMembershipValidator{ + client: c, + apiReader: c, + ownerRoleName: "resourcemanager.miloapis.com-organizationowner", + ownerRoleNamespace: "milo-system", + } + + if _, err := validator.ValidateDelete(ctx, target); err != nil { + t.Fatalf("expected deletion to succeed when user no longer exists, got error: %v", err) + } +} + func TestOrganizationMembershipValidator_ValidateUpdate_BlocksRemovingOwnerRoleForLastOwner(t *testing.T) { ctx := context.TODO() scheme := getWebhookTestScheme() @@ -609,6 +727,7 @@ func TestOrganizationMembershipValidator_ValidateUpdate_BlocksRemovingOwnerRoleF validator := &OrganizationMembershipValidator{ client: c, + apiReader: c, ownerRoleName: "resourcemanager.miloapis.com-organizationowner", ownerRoleNamespace: "milo-system", } @@ -681,6 +800,7 @@ func TestOrganizationMembershipValidator_ValidateUpdate_AllowsRemovingOwnerRoleW validator := &OrganizationMembershipValidator{ client: c, + apiReader: c, ownerRoleName: "resourcemanager.miloapis.com-organizationowner", ownerRoleNamespace: "milo-system", } @@ -734,6 +854,7 @@ func TestOrganizationMembershipValidator_ValidateUpdate_AllowsRemovingOwnerRoleD validator := &OrganizationMembershipValidator{ client: c, + apiReader: c, ownerRoleName: "resourcemanager.miloapis.com-organizationowner", ownerRoleNamespace: "milo-system", } diff --git a/test/iam/user-deletion-org-membership-cleanup/01-organization.yaml b/test/iam/user-deletion-org-membership-cleanup/01-organization.yaml new file mode 100644 index 00000000..5a3e1b61 --- /dev/null +++ b/test/iam/user-deletion-org-membership-cleanup/01-organization.yaml @@ -0,0 +1,8 @@ +apiVersion: resourcemanager.miloapis.com/v1alpha1 +kind: Organization +metadata: + name: org-membership-gc-test + annotations: + kubernetes.io/display-name: Org Membership GC Test +spec: + type: Standard diff --git a/test/iam/user-deletion-org-membership-cleanup/02-user.yaml b/test/iam/user-deletion-org-membership-cleanup/02-user.yaml new file mode 100644 index 00000000..84924f02 --- /dev/null +++ b/test/iam/user-deletion-org-membership-cleanup/02-user.yaml @@ -0,0 +1,8 @@ +apiVersion: iam.miloapis.com/v1alpha1 +kind: User +metadata: + name: org-membership-gc-user +spec: + email: org-membership-gc@example.com + givenName: OrgMembership + familyName: GCUser diff --git a/test/iam/user-deletion-org-membership-cleanup/README.md b/test/iam/user-deletion-org-membership-cleanup/README.md new file mode 100644 index 00000000..3ef8f455 --- /dev/null +++ b/test/iam/user-deletion-org-membership-cleanup/README.md @@ -0,0 +1,82 @@ +# Test: `user-deletion-org-membership-cleanup` + +Regression test for GitHub issue #536: OrganizationMembership resources are +not cleaned up when a User is deleted. + +The UserController adds a finalizer (iam.miloapis.com/user-membership-cleanup) +to every active User. When a User is deleted the finalizer runs +cleanupOrganizationMemberships, which lists and deletes all OrganizationMembership +resources that reference the user before the finalizer is removed and the User +object is garbage-collected. + +The OrganizationMembership validation webhook is extended to bypass the +last-owner guard when the referenced user has a non-zero DeletionTimestamp or +no longer exists in the API server, allowing the controller to proceed. + + +## Steps + +| # | Name | Bindings | Try | Catch | Finally | Cleanup | +|:-:|---|:-:|:-:|:-:|:-:|:-:| +| 1 | [setup-organization](#step-setup-organization) | 0 | 2 | 0 | 0 | 0 | +| 2 | [setup-user](#step-setup-user) | 0 | 3 | 0 | 0 | 0 | +| 3 | [create-membership](#step-create-membership) | 0 | 2 | 0 | 0 | 0 | +| 4 | [delete-user-and-verify-membership-cleaned-up](#step-delete-user-and-verify-membership-cleaned-up) | 0 | 3 | 0 | 0 | 0 | + +### Step: `setup-organization` + +Create the Organization and wait for its namespace to be provisioned. + + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | +| 2 | `wait` | 0 | 0 | *No description* | + +### Step: `setup-user` + +Create a User and wait for it to become Ready. The UserController will add +the membership-cleanup finalizer on the first reconcile. + + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | +| 2 | `wait` | 0 | 0 | *No description* | +| 3 | `assert` | 0 | 0 | Verify the membership-cleanup finalizer was added | + +### Step: `create-membership` + +Create an OrganizationMembership linking the user to the organization with +the owner role. The membership controller will reconcile it to Ready. + + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `apply` | 0 | 0 | *No description* | +| 2 | `wait` | 0 | 0 | *No description* | + +### Step: `delete-user-and-verify-membership-cleaned-up` + +Delete the User and verify the OrganizationMembership is removed by the +membership-cleanup finalizer. + + +#### Try + +| # | Operation | Bindings | Outputs | Description | +|:-:|---|:-:|:-:|---| +| 1 | `delete` | 0 | 0 | *No description* | +| 2 | `error` | 0 | 0 | Wait for the User to be fully removed from the API server | +| 3 | `error` | 0 | 0 | Assert the OrganizationMembership was deleted by the finalizer (fix for +issue #536). + | + +--- + diff --git a/test/iam/user-deletion-org-membership-cleanup/chainsaw-test.yaml b/test/iam/user-deletion-org-membership-cleanup/chainsaw-test.yaml new file mode 100644 index 00000000..e34c15fc --- /dev/null +++ b/test/iam/user-deletion-org-membership-cleanup/chainsaw-test.yaml @@ -0,0 +1,136 @@ +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: user-deletion-org-membership-cleanup +spec: + description: | + Regression test for GitHub issue #536: OrganizationMembership resources are + not cleaned up when a User is deleted. + + The UserController adds a finalizer (iam.miloapis.com/user-membership-cleanup) + to every active User. When a User is deleted the finalizer runs + cleanupOrganizationMemberships, which lists and deletes all OrganizationMembership + resources that reference the user before the finalizer is removed and the User + object is garbage-collected. + + The OrganizationMembership validation webhook is extended to bypass the + last-owner guard when the referenced user has a non-zero DeletionTimestamp or + no longer exists in the API server, allowing the controller to proceed. + + steps: + - name: setup-organization + description: Create the Organization and wait for its namespace. + try: + - apply: + file: 01-organization.yaml + - wait: + apiVersion: v1 + kind: Namespace + name: organization-org-membership-gc-test + timeout: 30s + for: + jsonPath: + path: '{.status.phase}' + value: Active + + - name: setup-user + description: | + Create a User and wait for it to become Ready. The UserController will add + the membership-cleanup finalizer on the first reconcile. + try: + - apply: + file: 02-user.yaml + - wait: + apiVersion: iam.miloapis.com/v1alpha1 + kind: User + name: org-membership-gc-user + timeout: 30s + for: + condition: + name: Ready + value: 'True' + - description: Verify the membership-cleanup finalizer was added + assert: + resource: + apiVersion: iam.miloapis.com/v1alpha1 + kind: User + metadata: + name: org-membership-gc-user + finalizers: + - iam.miloapis.com/user-membership-cleanup + + - name: create-membership + description: | + Create the owner role fixture and an OrganizationMembership linking the + user to the organization. The membership controller reconciles it to Ready. + try: + - description: Create the owner role so the membership webhook validates it + script: + timeout: 30s + content: | + cat <<'ROLE' | kubectl apply -f - + apiVersion: iam.miloapis.com/v1alpha1 + kind: Role + metadata: + name: resourcemanager.miloapis.com-organizationowner + namespace: milo-system + spec: + launchStage: Beta + includedPermissions: + - resourcemanager.miloapis.com/organizations.get + - resourcemanager.miloapis.com/organizations.update + - resourcemanager.miloapis.com/organizations.delete + ROLE + - apply: + resource: + apiVersion: resourcemanager.miloapis.com/v1alpha1 + kind: OrganizationMembership + metadata: + name: member-org-membership-gc-user + namespace: organization-org-membership-gc-test + spec: + organizationRef: + name: org-membership-gc-test + userRef: + name: org-membership-gc-user + roles: + - name: resourcemanager.miloapis.com-organizationowner + namespace: milo-system + - wait: + apiVersion: resourcemanager.miloapis.com/v1alpha1 + kind: OrganizationMembership + name: member-org-membership-gc-user + namespace: organization-org-membership-gc-test + timeout: 30s + for: + condition: + name: Ready + value: 'True' + + - name: delete-user-and-verify-membership-cleaned-up + description: | + Delete the User and verify the OrganizationMembership is removed by the + membership-cleanup finalizer. + try: + - delete: + ref: + apiVersion: iam.miloapis.com/v1alpha1 + kind: User + name: org-membership-gc-user + - description: Wait for the User to be fully removed + error: + resource: + apiVersion: iam.miloapis.com/v1alpha1 + kind: User + metadata: + name: org-membership-gc-user + timeout: 60s + - description: Assert the OrganizationMembership was deleted by the finalizer + error: + resource: + apiVersion: resourcemanager.miloapis.com/v1alpha1 + kind: OrganizationMembership + metadata: + name: member-org-membership-gc-user + namespace: organization-org-membership-gc-test + timeout: 60s