Skip to content

fix: Make a binding on a Project a dependent of that Project - #845

Open
vvoytovych wants to merge 3 commits into
mainfrom
fix/735-project-binding-owner
Open

vvoytovych wants to merge 3 commits into
mainfrom
fix/735-project-binding-owner

Conversation

@vvoytovych

@vvoytovych vvoytovych commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Editor and viewer PolicyBindings on a Project stay behind when the Project is deleted, and each one then keeps PolicyBindingsNotReady firing. Only the project owner's binding carries an owner reference to its Project, from createOwnerPolicyBinding.

Before After
A binding whose resourceRef names a Project No owner reference, except the owner binding An owner reference to the Project, from the PolicyBinding mutating webhook on create and update
Owner references already on a binding Kept Kept; the webhook only appends. An already-owned binding's update reads nothing
Other target kinds and resourceKind selectors No owner reference Unchanged
  • The webhook reads the Project live, through the manager's API reader, and adds the reference only when the Project carries the uid the binding names. The garbage collector deletes a binding whose owner is missing or carries another uid.
  • A request through a project control plane gets no reference, because the binding would be stored where the collector finds no Project. A project control plane serves no PolicyBinding today, and the guard covers the scope PolicyBindingValidator already names as future.
  • The reference sets neither controller nor blockOwnerDeletion, as on the owner binding.
  • A binding made before this change gains the reference on its next update. A no-op patch is enough.

Test plan

  • 18 unit cases and 15 planted defects, each caught. The cases cover the uid check, a stale cache, a missing Project, a failed read, a project control plane, other groups and kinds, existing references, an update, an owned binding's update, and the wiring.
  • test/iam/policybinding-project-owner-gc, against Milo's own garbage collector. A binding gains the reference and goes with its Project. A binding that names another uid gets none and stays. An update that strips the reference gets it back. It fails on main, where the binding gets no owner reference.
  • test/iam and test/resource-management on the test-infra cluster, beside main under the same parallel load. Both runs had two cleanups time out while a Project was being deleted, and nothing else failed.
  • go test ./...

Fixes #735

Related to #740

An update of a binding that already carries an owner reference to the
uid it names now makes no live read. After the backfill every update of
an owned binding takes this path, and with failurePolicy=fail a failed
or slow read would refuse it.
@vvoytovych
vvoytovych marked this pull request as draft October 15, 2026 14:03
The test created the datum-cloud namespace and a Role in it, which
policybinding-org-restrictions also creates and deletes in its cleanup.
Run in parallel, the Role's create met the terminating namespace and
timed out. The bindings are created as the platform admin, which needs no
assignable role, so the test now creates neither.
@vvoytovych
vvoytovych marked this pull request as ready for review October 15, 2026 15:18
@vvoytovych
vvoytovych requested a review from savme October 15, 2026 15:22

@scotwells scotwells left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vvoytovych we'll need to do this in a controller and "adopt" a policy binding by a project. This is because there's a webhook on project creation that adds a policy binding for the authenticated user creating the project to become the owner of the project. If the policy binding gets created with the owner reference before the project is created, the garbage collector will remove the policy binding.

@vvoytovych

Copy link
Copy Markdown
Contributor Author

Looking deeper into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PolicyBindings orphaned after Project deletion (missing ownerReferences)

3 participants