From 1f229dc7381dce4c1450ea3c226eeca6d37fd93c Mon Sep 17 00:00:00 2001 From: Viktor Voytovych Date: Tue, 20 Oct 2026 21:56:16 +0300 Subject: [PATCH 1/7] test: Run Milo's garbage collector in envtest, with the binding adopter's rows red on a stub gctest starts the forked collector against a test API server, as Kubernetes' own garbage collector integration tests do. The adopter's envtest rows (a binding goes with the object it targets) fail on a no-op adopter; the rows it must never act on pass, and one of them shows the collector deleting a dependent whose owner is gone. --- .../garbagecollector/gctest/gctest.go | 66 +++ .../controllers/iam/policybinding_adopter.go | 34 ++ .../iam/policybinding_adopter_envtest_test.go | 415 ++++++++++++++++++ 3 files changed, 515 insertions(+) create mode 100644 internal/controllers/garbagecollector/gctest/gctest.go create mode 100644 internal/controllers/iam/policybinding_adopter.go create mode 100644 internal/controllers/iam/policybinding_adopter_envtest_test.go diff --git a/internal/controllers/garbagecollector/gctest/gctest.go b/internal/controllers/garbagecollector/gctest/gctest.go new file mode 100644 index 000000000..00597628d --- /dev/null +++ b/internal/controllers/garbagecollector/gctest/gctest.go @@ -0,0 +1,66 @@ +// Package gctest runs Milo's garbage collector against a test API server, so a test can prove what the collector +// deletes. It follows the setup of Kubernetes' own garbage collector integration tests +// (test/integration/garbagecollector/garbage_collector_test.go, setupWithServer). +package gctest + +import ( + "context" + "fmt" + "time" + + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/discovery/cached/memory" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/metadata" + "k8s.io/client-go/metadata/metadatainformer" + "k8s.io/client-go/rest" + "k8s.io/client-go/restmapper" + "k8s.io/controller-manager/pkg/informerfactory" + + "go.miloapis.com/milo/internal/controllers/garbagecollector" +) + +// Options sets how the collector runs. The zero value runs it as Kubernetes' controller manager does by default. +type Options struct { + // Workers is the number of delete and orphan workers; 0 means 20, kube-controller-manager's --concurrent-gc-syncs. + Workers int + // SyncPeriod is how often the REST mapper is reset and discovery is read again; 0 means 5 s, as the upstream tests. + SyncPeriod time.Duration +} + +// Start runs the garbage collector against cfg until ctx is done. It returns once the collector is constructed; the +// collector's own first sync waits for its informers. +func Start(ctx context.Context, cfg *rest.Config, opts Options) error { + workers, period := opts.Workers, opts.SyncPeriod + if workers == 0 { + workers = 20 + } + if period == 0 { + period = 5 * time.Second + } + clientSet, err := kubernetes.NewForConfig(cfg) + if err != nil { + return fmt.Errorf("garbage collector clientset: %w", err) + } + metadataClient, err := metadata.NewForConfig(cfg) + if err != nil { + return fmt.Errorf("garbage collector metadata client: %w", err) + } + restMapper := restmapper.NewDeferredDiscoveryRESTMapper(memory.NewMemCacheClient(clientSet.Discovery())) + restMapper.Reset() + started := make(chan struct{}) + close(started) + gc, err := garbagecollector.NewGarbageCollector(ctx, clientSet, metadataClient, restMapper, + garbagecollector.DefaultIgnoredResources(), + informerfactory.NewInformerFactory(informers.NewSharedInformerFactory(clientSet, 0), + metadatainformer.NewSharedInformerFactory(metadataClient, 0)), + started) + if err != nil { + return fmt.Errorf("garbage collector: %w", err) + } + go wait.Until(restMapper.Reset, period, ctx.Done()) + go gc.Run(ctx, workers, period) + go gc.Sync(ctx, clientSet.Discovery(), period) + return nil +} diff --git a/internal/controllers/iam/policybinding_adopter.go b/internal/controllers/iam/policybinding_adopter.go new file mode 100644 index 000000000..9af79e667 --- /dev/null +++ b/internal/controllers/iam/policybinding_adopter.go @@ -0,0 +1,34 @@ +package iam + +import ( + "context" + + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + iamv1alpha1 "go.miloapis.com/milo/pkg/apis/iam/v1alpha1" +) + +// PolicyBindingAdopter makes a PolicyBinding a dependent of the one object its resourceRef names, so the garbage +// collector deletes the binding with that object. +type PolicyBindingAdopter struct { + Client client.Client + // APIReader reads the target from the API server, not the cache, so a re-created target is never mistaken for + // the one the binding names. + APIReader client.Reader + Scheme *runtime.Scheme +} + +// Reconcile adopts nothing yet. +func (r *PolicyBindingAdopter) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + return ctrl.Result{}, nil +} + +// SetupWithManager sets up the controller with the Manager. +func (r *PolicyBindingAdopter) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&iamv1alpha1.PolicyBinding{}). + Named("policybinding-adopter"). + Complete(r) +} diff --git a/internal/controllers/iam/policybinding_adopter_envtest_test.go b/internal/controllers/iam/policybinding_adopter_envtest_test.go new file mode 100644 index 000000000..524a55f8a --- /dev/null +++ b/internal/controllers/iam/policybinding_adopter_envtest_test.go @@ -0,0 +1,415 @@ +package iam + +import ( + "context" + "fmt" + "os" + "path/filepath" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/uuid" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/util/retry" + "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + ctrlconfig "sigs.k8s.io/controller-runtime/pkg/config" + "sigs.k8s.io/controller-runtime/pkg/envtest" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + + "go.miloapis.com/milo/internal/controllers/garbagecollector/gctest" + iamv1alpha1 "go.miloapis.com/milo/pkg/apis/iam/v1alpha1" + resourcemanagerv1alpha1 "go.miloapis.com/milo/pkg/apis/resourcemanager/v1alpha1" +) + +const ( + adopterNamespace = "organization-acme" + adopterOther = "organization-other" + // testHold keeps a deleted object in storage, deleting, until the test releases it. + testHold = "test.miloapis.com/hold" + // settle is how long a binding must keep its state to count as kept: longer than the garbage collector takes to + // delete a dependent whose owner is absent, which is milliseconds once its graph holds the dependent. + settle = 5 * time.Second +) + +type adopterEnv struct { + ctx context.Context + client client.Client // writes and reads through the API server, as a user would +} + +// TestPolicyBindingAdopter runs the adopter in a manager against an API server with Milo's iam and resourcemanager +// CRDs, with Milo's own garbage collector deleting dependents. Each case is one row of the deletion-path table in the +// design note for milo#845. +func TestPolicyBindingAdopter(t *testing.T) { + env := startAdopterEnv(t, nil) + + t.Run("D1 a binding on a Project is adopted and goes with its Project", func(t *testing.T) { + project := env.project(t, "d1") + pb := env.binding(t, "d1", env.refTo(project), nil) + env.waitOwnedBy(t, pb, project.UID) + require.NoError(t, env.client.Delete(env.ctx, project)) + env.waitGone(t, pb) + }) + + t.Run("D9 a binding on a namespaced object of a core kind in its namespace is adopted and goes with it", func(t *testing.T) { + cm := env.configMap(t, adopterNamespace, "d9") + pb := env.binding(t, "d9", iamv1alpha1.ResourceReference{ + Kind: "ConfigMap", Name: cm.Name, UID: string(cm.UID), Namespace: adopterNamespace, + }, nil) + env.waitOwnedBy(t, pb, cm.UID) + require.NoError(t, env.client.Delete(env.ctx, cm)) + env.waitGone(t, pb) + }) + + t.Run("D8 a namespaced target in another namespace is never an owner", func(t *testing.T) { + cm := env.configMap(t, adopterOther, "d8") + pb := env.binding(t, "d8", iamv1alpha1.ResourceReference{ + Kind: "ConfigMap", Name: cm.Name, UID: string(cm.UID), Namespace: adopterOther, + }, nil) + env.keepsOwners(t, pb, nil) + }) + + t.Run("D4 a binding owned by another object is never adopted and goes with that object", func(t *testing.T) { + project := env.project(t, "d4") + holder := env.configMap(t, adopterNamespace, "d4-holder") + owner := metav1.OwnerReference{APIVersion: "v1", Kind: "ConfigMap", Name: holder.Name, UID: holder.UID} + pb := env.binding(t, "d4", env.refTo(project), []metav1.OwnerReference{owner}) + env.keepsOwners(t, pb, []types.UID{holder.UID}) + require.NoError(t, env.client.Delete(env.ctx, holder)) + env.waitGone(t, pb) // the Project lives on: a second owner would have kept a removed member's grant + }) + + t.Run("D5 a binding naming another uid of a live target is never adopted", func(t *testing.T) { + project := env.project(t, "d5") + ref := env.refTo(project) + ref.UID = string(uuid.NewUUID()) + pb := env.binding(t, "d5", ref, nil) + env.keepsOwners(t, pb, nil) + }) + + t.Run("D3 a binding naming a target never stored is never adopted and stays", func(t *testing.T) { + pb := env.binding(t, "d3", iamv1alpha1.ResourceReference{ + APIGroup: resourcemanagerv1alpha1.GroupVersion.Group, Kind: "Project", Name: "d3-never", UID: string(uuid.NewUUID()), + }, nil) + env.keepsOwners(t, pb, nil) + }) + + t.Run("D2 a binding on a target being deleted is never adopted, and stays once the target is gone", func(t *testing.T) { + project := env.project(t, "d2", testHold) + require.NoError(t, env.client.Delete(env.ctx, project)) + pb := env.binding(t, "d2", env.refTo(project), nil) + env.keepsOwners(t, pb, nil) + env.release(t, project) + env.keepsOwners(t, pb, nil) + }) + + t.Run("D6 an orphaning delete of the target is not undone", func(t *testing.T) { + project := env.project(t, "d6", testHold) + pb := env.binding(t, "d6", env.refTo(project), nil) + env.waitOwnedBy(t, pb, project.UID) + require.NoError(t, env.client.Delete(env.ctx, project, client.PropagationPolicy(metav1.DeletePropagationOrphan))) + env.waitOwners(t, pb, nil) // the garbage collector orphans it; the hold keeps the Project deleting + env.keepsOwners(t, pb, nil) + env.release(t, project) + env.keepsOwners(t, pb, nil) + }) + + t.Run("D7 a foreground delete of the target deletes the binding and does not wait on it", func(t *testing.T) { + project := env.project(t, "d7") + pb := env.binding(t, "d7", env.refTo(project), nil) + env.waitOwnedBy(t, pb, project.UID) + require.NoError(t, env.client.Delete(env.ctx, project, client.PropagationPolicy(metav1.DeletePropagationForeground))) + env.waitGone(t, pb) + env.waitGone(t, project) + }) + + t.Run("D11 an owner reference a person removes is added back", func(t *testing.T) { + project := env.project(t, "d11") + pb := env.binding(t, "d11", env.refTo(project), nil) + env.waitOwnedBy(t, pb, project.UID) + require.NoError(t, retry.RetryOnConflict(retry.DefaultRetry, func() error { + if err := env.client.Get(env.ctx, client.ObjectKeyFromObject(pb), pb); err != nil { + return err + } + pb.OwnerReferences = nil + return env.client.Update(env.ctx, pb) + })) + env.waitOwnedBy(t, pb, project.UID) + }) + + t.Run("D13 a subject edited while the binding is adopted keeps both changes", func(t *testing.T) { + project := env.project(t, "d13") + pb := env.binding(t, "d13", env.refTo(project), nil) + require.NoError(t, retry.RetryOnConflict(retry.DefaultRetry, func() error { + if err := env.client.Get(env.ctx, client.ObjectKeyFromObject(pb), pb); err != nil { + return err + } + pb.Spec.Subjects = append(pb.Spec.Subjects, iamv1alpha1.Subject{Kind: "User", Name: "d13-second", UID: string(uuid.NewUUID())}) + return env.client.Update(env.ctx, pb) + })) + env.waitOwnedBy(t, pb, project.UID) + require.Len(t, pb.Spec.Subjects, 2, "the subject edit survives the adoption") + }) + + t.Run("stress: a burst of 60 bindings, made serially and in parallel, are all adopted and none is deleted", func(t *testing.T) { + const n = 30 + var wg sync.WaitGroup + results := make(chan error, 2*n) + race := func(name string) { + defer wg.Done() + results <- env.projectThenBinding(name) + } + for i := range n { + wg.Add(1) + race(fmt.Sprintf("burst-serial-%d", i)) + } + for i := range n { + wg.Add(1) + go race(fmt.Sprintf("burst-parallel-%d", i)) + } + wg.Wait() + close(results) + for err := range results { + require.NoError(t, err) + } + time.Sleep(settle) + env.allOwnedByTheirProjects(t, "burst-") + }) +} + +// TestPolicyBindingAdopterAdoptsExistingBindings is row D15: bindings made before the adopter runs are adopted by its +// first list, and the garbage collector deletes none of them. +func TestPolicyBindingAdopterAdoptsExistingBindings(t *testing.T) { + const n = 50 + env := startAdopterEnv(t, func(env *adopterEnv) { + for i := range n { + project := env.project(t, fmt.Sprintf("d15-%d", i)) + env.binding(t, fmt.Sprintf("d15-%d", i), env.refTo(project), nil) + } + }) + env.eventually(t, func() bool { return env.countOwned(t, "d15-") == n }, "every existing binding is adopted") + time.Sleep(settle) + require.Equal(t, n, env.countOwned(t, "d15-"), "none deleted after adoption") +} + +// startAdopterEnv starts an API server with Milo's iam and resourcemanager CRDs, Milo's garbage collector, and a +// manager running the adopter. preload, when set, runs after the collector starts and before the manager does. +func startAdopterEnv(t *testing.T, preload func(*adopterEnv)) *adopterEnv { + t.Helper() + if os.Getenv("ADOPTER_ENV_LOG") != "" { + ctrl.SetLogger(zap.New(zap.UseDevMode(true), zap.WriteTo(os.Stderr))) + } + if os.Getenv("KUBEBUILDER_ASSETS") == "" { + if os.Getenv("CI") != "" { + t.Fatal("KUBEBUILDER_ASSETS must be set in CI; this test needs an envtest API server") + } + t.Skip("KUBEBUILDER_ASSETS is not set; run setup-envtest to provide an envtest API server") + } + root := filepath.Join("..", "..", "..") + testEnv := &envtest.Environment{ + CRDDirectoryPaths: []string{ + filepath.Join(root, "config", "crd", "bases", "iam"), + filepath.Join(root, "config", "crd", "bases", "resourcemanager"), + }, + ErrorIfCRDPathMissing: true, + } + cfg, err := testEnv.Start() + require.NoError(t, err) + t.Cleanup(func() { _ = testEnv.Stop() }) + + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, iamv1alpha1.AddToScheme(scheme)) + require.NoError(t, resourcemanagerv1alpha1.AddToScheme(scheme)) + direct, err := client.New(cfg, client.Options{Scheme: scheme}) + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + require.NoError(t, gctest.Start(ctx, cfg, gctest.Options{})) + env := &adopterEnv{ctx: ctx, client: direct} + for _, ns := range []string{adopterNamespace, adopterOther} { + require.NoError(t, direct.Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}})) + } + require.NoError(t, direct.Create(ctx, &resourcemanagerv1alpha1.Organization{ + ObjectMeta: metav1.ObjectMeta{Name: "acme"}, Spec: resourcemanagerv1alpha1.OrganizationSpec{Type: "Standard"}, + })) + if preload != nil { + preload(env) + } + + mgr, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: scheme, + Metrics: metricsserver.Options{BindAddress: "0"}, + HealthProbeBindAddress: "0", + Controller: ctrlconfig.Controller{SkipNameValidation: ptr.To(true)}, + }) + require.NoError(t, err) + adopter := &PolicyBindingAdopter{Client: mgr.GetClient(), APIReader: mgr.GetAPIReader(), Scheme: scheme} + require.NoError(t, adopter.SetupWithManager(mgr)) + go func() { _ = mgr.Start(ctx) }() + require.True(t, mgr.GetCache().WaitForCacheSync(ctx)) + return env +} + +func (e *adopterEnv) project(t *testing.T, name string, finalizers ...string) *resourcemanagerv1alpha1.Project { + t.Helper() + p := &resourcemanagerv1alpha1.Project{ + ObjectMeta: metav1.ObjectMeta{Name: name, Finalizers: finalizers}, + Spec: resourcemanagerv1alpha1.ProjectSpec{ + OwnerRef: resourcemanagerv1alpha1.OwnerReference{Kind: "Organization", Name: "acme"}, + }, + } + require.NoError(t, e.client.Create(e.ctx, p)) + return p +} + +func (e *adopterEnv) configMap(t *testing.T, namespace, name string) *corev1.ConfigMap { + t.Helper() + cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: name}} + require.NoError(t, e.client.Create(e.ctx, cm)) + return cm +} + +func (e *adopterEnv) refTo(p *resourcemanagerv1alpha1.Project) iamv1alpha1.ResourceReference { + return iamv1alpha1.ResourceReference{ + APIGroup: resourcemanagerv1alpha1.GroupVersion.Group, Kind: "Project", Name: p.Name, UID: string(p.UID), + } +} + +func newBinding(name string, ref iamv1alpha1.ResourceReference, owners []metav1.OwnerReference) *iamv1alpha1.PolicyBinding { + return &iamv1alpha1.PolicyBinding{ + ObjectMeta: metav1.ObjectMeta{Namespace: adopterNamespace, Name: name, OwnerReferences: owners}, + Spec: iamv1alpha1.PolicyBindingSpec{ + RoleRef: iamv1alpha1.RoleReference{Name: "editor", Namespace: "milo-system"}, + Subjects: []iamv1alpha1.Subject{{Kind: "User", Name: name + "-user", UID: string(uuid.NewUUID())}}, + ResourceSelector: iamv1alpha1.ResourceSelector{ResourceRef: &ref}, + }, + } +} + +func (e *adopterEnv) binding(t *testing.T, name string, ref iamv1alpha1.ResourceReference, owners []metav1.OwnerReference) *iamv1alpha1.PolicyBinding { + t.Helper() + pb := newBinding(name, ref, owners) + require.NoError(t, e.client.Create(e.ctx, pb)) + return pb +} + +// projectThenBinding writes a Project and, at once, a binding naming it. The webhook's order, the binding stored first, +// cannot be made here: only the API server knows a Project's uid before it is stored. That race is row D16, tested on +// the adopter's poll with a stubbed reader and on kind against the Project webhook. +func (e *adopterEnv) projectThenBinding(name string) error { + p := &resourcemanagerv1alpha1.Project{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: resourcemanagerv1alpha1.ProjectSpec{OwnerRef: resourcemanagerv1alpha1.OwnerReference{Kind: "Organization", Name: "acme"}}, + } + if err := e.client.Create(e.ctx, p); err != nil { + return err + } + return e.client.Create(e.ctx, newBinding(name, e.refTo(p), nil)) +} + +func (e *adopterEnv) release(t *testing.T, obj client.Object) { + t.Helper() + require.NoError(t, retry.RetryOnConflict(retry.DefaultRetry, func() error { + if err := e.client.Get(e.ctx, client.ObjectKeyFromObject(obj), obj); err != nil { + return err + } + obj.SetFinalizers(nil) + return e.client.Update(e.ctx, obj) + })) + e.waitGone(t, obj) +} + +func ownerUIDs(pb *iamv1alpha1.PolicyBinding) []types.UID { + var uids []types.UID + for _, o := range pb.OwnerReferences { + uids = append(uids, o.UID) + } + return uids +} + +func (e *adopterEnv) eventually(t *testing.T, cond func() bool, msg string) { + t.Helper() + require.Eventually(t, cond, 30*time.Second, 100*time.Millisecond, msg) +} + +func (e *adopterEnv) waitOwnedBy(t *testing.T, pb *iamv1alpha1.PolicyBinding, uid types.UID) { + t.Helper() + e.waitOwners(t, pb, []types.UID{uid}) +} + +func (e *adopterEnv) waitOwners(t *testing.T, pb *iamv1alpha1.PolicyBinding, want []types.UID) { + t.Helper() + e.eventually(t, func() bool { + return e.client.Get(e.ctx, client.ObjectKeyFromObject(pb), pb) == nil && equalUIDs(ownerUIDs(pb), want) + }, fmt.Sprintf("%s's owners become %v", pb.Name, want)) +} + +// keepsOwners asserts that the binding exists with exactly these owners for the whole settle period. +func (e *adopterEnv) keepsOwners(t *testing.T, pb *iamv1alpha1.PolicyBinding, want []types.UID) { + t.Helper() + require.Never(t, func() bool { + err := e.client.Get(e.ctx, client.ObjectKeyFromObject(pb), pb) + return err != nil || !equalUIDs(ownerUIDs(pb), want) + }, settle, 100*time.Millisecond, "%s must stay, owned by %v", pb.Name, want) +} + +func (e *adopterEnv) waitGone(t *testing.T, obj client.Object) { + t.Helper() + e.eventually(t, func() bool { + return apierrors.IsNotFound(e.client.Get(e.ctx, client.ObjectKeyFromObject(obj), obj)) + }, fmt.Sprintf("%s is deleted", obj.GetName())) +} + +func (e *adopterEnv) countOwned(t *testing.T, prefix string) int { + t.Helper() + owned := 0 + for _, pb := range e.bindings(t, prefix) { + if len(pb.OwnerReferences) == 1 && string(pb.OwnerReferences[0].UID) == pb.Spec.ResourceSelector.ResourceRef.UID { + owned++ + } + } + return owned +} + +func (e *adopterEnv) allOwnedByTheirProjects(t *testing.T, prefix string) { + t.Helper() + bindings := e.bindings(t, prefix) + require.Len(t, bindings, 60, "no binding lost") + require.Equal(t, len(bindings), e.countOwned(t, prefix), "every binding owned by its Project") +} + +func (e *adopterEnv) bindings(t *testing.T, prefix string) []iamv1alpha1.PolicyBinding { + t.Helper() + var list iamv1alpha1.PolicyBindingList + require.NoError(t, e.client.List(e.ctx, &list, client.InNamespace(adopterNamespace))) + var out []iamv1alpha1.PolicyBinding + for _, pb := range list.Items { + if len(pb.Name) >= len(prefix) && pb.Name[:len(prefix)] == prefix { + out = append(out, pb) + } + } + return out +} + +func equalUIDs(got, want []types.UID) bool { + if len(got) != len(want) { + return false + } + for i := range got { + if got[i] != want[i] { + return false + } + } + return true +} From 7c562cabe05f3a292f2c01ac1641bedbc5cd9b90 Mon Sep 17 00:00:00 2001 From: Viktor Voytovych Date: Tue, 20 Oct 2026 22:02:36 +0300 Subject: [PATCH 2/7] test: The binding adopter's wait for its target, red on a stub A target absent at the first read is polled every second while it can still be stored (the request timeout and a margin, 120 s from the binding's creation) and left after; a failed read is returned and writes nothing. The adopter takes a REST mapper and a clock. --- .../controllers/iam/policybinding_adopter.go | 8 +- .../iam/policybinding_adopter_envtest_test.go | 2 +- .../iam/policybinding_adopter_test.go | 121 ++++++++++++++++++ 3 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 internal/controllers/iam/policybinding_adopter_test.go diff --git a/internal/controllers/iam/policybinding_adopter.go b/internal/controllers/iam/policybinding_adopter.go index 9af79e667..7a1a374d5 100644 --- a/internal/controllers/iam/policybinding_adopter.go +++ b/internal/controllers/iam/policybinding_adopter.go @@ -2,7 +2,9 @@ package iam import ( "context" + "time" + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -17,7 +19,11 @@ type PolicyBindingAdopter struct { // APIReader reads the target from the API server, not the cache, so a re-created target is never mistaken for // the one the binding names. APIReader client.Reader - Scheme *runtime.Scheme + // Mapper resolves the kind a resourceRef names, so any kind the API server serves can be a binding's owner. + Mapper meta.RESTMapper + Scheme *runtime.Scheme + // Now is the clock the target's creation window is measured with; nil means time.Now. + Now func() time.Time } // Reconcile adopts nothing yet. diff --git a/internal/controllers/iam/policybinding_adopter_envtest_test.go b/internal/controllers/iam/policybinding_adopter_envtest_test.go index 524a55f8a..2b996c861 100644 --- a/internal/controllers/iam/policybinding_adopter_envtest_test.go +++ b/internal/controllers/iam/policybinding_adopter_envtest_test.go @@ -254,7 +254,7 @@ func startAdopterEnv(t *testing.T, preload func(*adopterEnv)) *adopterEnv { Controller: ctrlconfig.Controller{SkipNameValidation: ptr.To(true)}, }) require.NoError(t, err) - adopter := &PolicyBindingAdopter{Client: mgr.GetClient(), APIReader: mgr.GetAPIReader(), Scheme: scheme} + adopter := &PolicyBindingAdopter{Client: mgr.GetClient(), APIReader: mgr.GetAPIReader(), Mapper: mgr.GetRESTMapper(), Scheme: scheme} require.NoError(t, adopter.SetupWithManager(mgr)) go func() { _ = mgr.Start(ctx) }() require.True(t, mgr.GetCache().WaitForCacheSync(ctx)) diff --git a/internal/controllers/iam/policybinding_adopter_test.go b/internal/controllers/iam/policybinding_adopter_test.go new file mode 100644 index 000000000..56ac96e73 --- /dev/null +++ b/internal/controllers/iam/policybinding_adopter_test.go @@ -0,0 +1,121 @@ +package iam + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + iamv1alpha1 "go.miloapis.com/milo/pkg/apis/iam/v1alpha1" + resourcemanagerv1alpha1 "go.miloapis.com/milo/pkg/apis/resourcemanager/v1alpha1" +) + +// bindingCreated is the binding's creationTimestamp in these cases; the window is measured from it. +var bindingCreated = time.Date(2026, 10, 20, 18, 0, 0, 0, time.UTC) + +// pollCase is one reconcile of a binding whose target the reader answers for. +type pollCase struct { + age time.Duration // the adopter's clock, after the binding's creation + absentReads int // how many reads of the target answer NotFound before it is found + readErr error // an error every read of the target returns instead +} + +// TestPolicyBindingAdopterWaitsForTheTarget covers rule 4 of the design: a target that is absent when its binding is +// first seen is polled while it can still be stored (the request timeout and a margin, 120 s from the binding's +// creation), and left after that; any other read error is returned, and nothing is written. +func TestPolicyBindingAdopterWaitsForTheTarget(t *testing.T) { + t.Run("D16 absent at the first read, stored within the window: polled, then adopted", func(t *testing.T) { + r, c, project := pollEnv(t, pollCase{age: 10 * time.Second, absentReads: 1}) + res, err := r.Reconcile(context.Background(), bindingRequest()) + require.NoError(t, err) + require.Equal(t, time.Second, res.RequeueAfter, "an absent target inside the window is polled every second") + require.Empty(t, ownersOf(t, c)) + res, err = r.Reconcile(context.Background(), bindingRequest()) + require.NoError(t, err) + require.Zero(t, res.RequeueAfter) + require.Equal(t, []types.UID{project.UID}, ownersOf(t, c)) + }) + + t.Run("D3 absent 119 s after the binding's creation: still polled", func(t *testing.T) { + r, c, _ := pollEnv(t, pollCase{age: 119 * time.Second, absentReads: 1 << 20}) + res, err := r.Reconcile(context.Background(), bindingRequest()) + require.NoError(t, err) + require.Equal(t, time.Second, res.RequeueAfter) + require.Empty(t, ownersOf(t, c)) + }) + + t.Run("D3 absent 121 s after the binding's creation: left, not polled", func(t *testing.T) { + r, c, _ := pollEnv(t, pollCase{age: 121 * time.Second, absentReads: 1 << 20}) + res, err := r.Reconcile(context.Background(), bindingRequest()) + require.NoError(t, err) + require.Equal(t, ctrl.Result{}, res) + require.Empty(t, ownersOf(t, c)) + }) + + t.Run("D14 a failed read of the target is returned and writes nothing", func(t *testing.T) { + readErr := apierrors.NewInternalError(context.DeadlineExceeded) + r, c, _ := pollEnv(t, pollCase{age: 10 * time.Second, readErr: readErr}) + _, err := r.Reconcile(context.Background(), bindingRequest()) + require.Error(t, err) + require.Empty(t, ownersOf(t, c)) + }) +} + +// pollEnv returns an adopter over a fake API server holding one Project and one binding naming it. The adopter's +// reads of the target, as metadata through the API reader, answer as the case says. +func pollEnv(t *testing.T, pc pollCase) (*PolicyBindingAdopter, client.Client, *resourcemanagerv1alpha1.Project) { + t.Helper() + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, iamv1alpha1.AddToScheme(scheme)) + require.NoError(t, resourcemanagerv1alpha1.AddToScheme(scheme)) + project := &resourcemanagerv1alpha1.Project{ObjectMeta: metav1.ObjectMeta{Name: "p", UID: "project-uid"}} + pb := newBinding("pb", iamv1alpha1.ResourceReference{ + APIGroup: resourcemanagerv1alpha1.GroupVersion.Group, Kind: "Project", Name: project.Name, UID: string(project.UID), + }, nil) + pb.CreationTimestamp = metav1.NewTime(bindingCreated) + // The fake's default mapper calls every kind namespaced; a Project is cluster-scoped, as Milo serves it. + mapper := meta.NewDefaultRESTMapper(nil) + mapper.Add(resourcemanagerv1alpha1.GroupVersion.WithKind("Project"), meta.RESTScopeRoot) + mapper.Add(iamv1alpha1.SchemeGroupVersion.WithKind("PolicyBinding"), meta.RESTScopeNamespace) + reads := 0 + c := fake.NewClientBuilder().WithScheme(scheme).WithRESTMapper(mapper).WithObjects(project, pb). + WithInterceptorFuncs(interceptor.Funcs{Get: func(ctx context.Context, cl client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, isTarget := obj.(*metav1.PartialObjectMetadata); isTarget { + if pc.readErr != nil { + return pc.readErr + } + if reads++; reads <= pc.absentReads { + return apierrors.NewNotFound(schema.GroupResource{Group: resourcemanagerv1alpha1.GroupVersion.Group, Resource: "projects"}, key.Name) + } + } + return cl.Get(ctx, key, obj, opts...) + }}).Build() + return &PolicyBindingAdopter{ + Client: c, APIReader: c, Mapper: mapper, Scheme: scheme, + Now: func() time.Time { return bindingCreated.Add(pc.age) }, + }, c, project +} + +func bindingRequest() ctrl.Request { + return ctrl.Request{NamespacedName: types.NamespacedName{Namespace: adopterNamespace, Name: "pb"}} +} + +func ownersOf(t *testing.T, c client.Client) []types.UID { + t.Helper() + var pb iamv1alpha1.PolicyBinding + require.NoError(t, c.Get(context.Background(), bindingRequest().NamespacedName, &pb)) + return ownerUIDs(&pb) +} From 546b71c8f09527c1b3a7d66d061410180cd43848 Mon Sep 17 00:00:00 2001 From: Viktor Voytovych Date: Tue, 20 Oct 2026 22:03:46 +0300 Subject: [PATCH 3/7] test: Keep the binding adopter's clock unexported, as the quota cleanup controller and the project bootstrap do --- internal/controllers/iam/policybinding_adopter.go | 4 ++-- internal/controllers/iam/policybinding_adopter_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/controllers/iam/policybinding_adopter.go b/internal/controllers/iam/policybinding_adopter.go index 7a1a374d5..465a073c0 100644 --- a/internal/controllers/iam/policybinding_adopter.go +++ b/internal/controllers/iam/policybinding_adopter.go @@ -22,8 +22,8 @@ type PolicyBindingAdopter struct { // Mapper resolves the kind a resourceRef names, so any kind the API server serves can be a binding's owner. Mapper meta.RESTMapper Scheme *runtime.Scheme - // Now is the clock the target's creation window is measured with; nil means time.Now. - Now func() time.Time + // now is the clock the target's creation window is measured with; nil means time.Now. + now func() time.Time } // Reconcile adopts nothing yet. diff --git a/internal/controllers/iam/policybinding_adopter_test.go b/internal/controllers/iam/policybinding_adopter_test.go index 56ac96e73..64e14aa72 100644 --- a/internal/controllers/iam/policybinding_adopter_test.go +++ b/internal/controllers/iam/policybinding_adopter_test.go @@ -105,7 +105,7 @@ func pollEnv(t *testing.T, pc pollCase) (*PolicyBindingAdopter, client.Client, * }}).Build() return &PolicyBindingAdopter{ Client: c, APIReader: c, Mapper: mapper, Scheme: scheme, - Now: func() time.Time { return bindingCreated.Add(pc.age) }, + now: func() time.Time { return bindingCreated.Add(pc.age) }, }, c, project } From e8ce387c05a914ed9baebd6d9f94389c2e5ab217 Mon Sep 17 00:00:00 2001 From: Viktor Voytovych Date: Tue, 20 Oct 2026 22:24:35 +0300 Subject: [PATCH 4/7] fix: Make a PolicyBinding a dependent of the object it targets A controller adds a binding's target as its owner once the target is stored with the uid the binding names, so the garbage collector deletes the binding with its target. It reads any served kind as metadata through the REST mapper, adopts only a binding with no owner, never onto a deleting target, and polls a target that is not stored yet for as long as its create request can still commit. The Project webhook no longer sets the owner reference on the creator's binding: the Project is not stored at that moment, so the garbage collector could find no owner and delete the binding. --- .../controller-manager/controllermanager.go | 7 +- cmd/milo/controller-manager/policybindings.go | 30 +++++ .../controllers/iam/policybinding_adopter.go | 119 +++++++++++++++++- .../iam/policybinding_adopter_envtest_test.go | 45 +++++++ .../iam/policybinding_adopter_test.go | 65 ++++++++-- .../v1alpha1/project_webhook.go | 14 +-- 6 files changed, 252 insertions(+), 28 deletions(-) create mode 100644 cmd/milo/controller-manager/policybindings.go diff --git a/cmd/milo/controller-manager/controllermanager.go b/cmd/milo/controller-manager/controllermanager.go index da06b760e..6c4c7e3a3 100644 --- a/cmd/milo/controller-manager/controllermanager.go +++ b/cmd/milo/controller-manager/controllermanager.go @@ -589,11 +589,8 @@ func Run(ctx context.Context, c *config.CompletedConfig, opts *Options) error { klog.FlushAndExit(klog.ExitFlushTimeout, 1) } - subjectBindingReaperCtrl := resourcemanagercontroller.SubjectBindingReaperController{ - Client: ctrl.GetClient(), - } - if err := subjectBindingReaperCtrl.SetupWithManager(ctrl); err != nil { - logger.Error(err, "Error setting up subject binding reaper controller") + if err := setupPolicyBindingLifecycle(ctrl); err != nil { + logger.Error(err, "Error setting up the policy binding lifecycle controllers") klog.FlushAndExit(klog.ExitFlushTimeout, 1) } diff --git a/cmd/milo/controller-manager/policybindings.go b/cmd/milo/controller-manager/policybindings.go new file mode 100644 index 000000000..494476cfd --- /dev/null +++ b/cmd/milo/controller-manager/policybindings.go @@ -0,0 +1,30 @@ +package app + +import ( + "fmt" + + controllerruntime "sigs.k8s.io/controller-runtime" + + iamcontroller "go.miloapis.com/milo/internal/controllers/iam" + resourcemanagercontroller "go.miloapis.com/milo/internal/controllers/resourcemanager" +) + +// setupPolicyBindingLifecycle sets up the controllers that end a PolicyBinding with what it names: the subject reaper +// deletes a binding whose User is gone, and the adopter makes a binding a dependent of its target, so the garbage +// collector deletes it with that target. +func setupPolicyBindingLifecycle(mgr controllerruntime.Manager) error { + reaper := resourcemanagercontroller.SubjectBindingReaperController{Client: mgr.GetClient()} + if err := reaper.SetupWithManager(mgr); err != nil { + return fmt.Errorf("subject binding reaper: %w", err) + } + adopter := iamcontroller.PolicyBindingAdopter{ + Client: mgr.GetClient(), + APIReader: mgr.GetAPIReader(), + Mapper: mgr.GetRESTMapper(), + Scheme: mgr.GetScheme(), + } + if err := adopter.SetupWithManager(mgr); err != nil { + return fmt.Errorf("policy binding adopter: %w", err) + } + return nil +} diff --git a/internal/controllers/iam/policybinding_adopter.go b/internal/controllers/iam/policybinding_adopter.go index 465a073c0..bb7e51f2b 100644 --- a/internal/controllers/iam/policybinding_adopter.go +++ b/internal/controllers/iam/policybinding_adopter.go @@ -2,16 +2,41 @@ package iam import ( "context" + "errors" "time" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" iamv1alpha1 "go.miloapis.com/milo/pkg/apis/iam/v1alpha1" ) +const ( + // targetStoreWindow is how long after a binding's creation its target can still be stored. The API server stamps + // a new object's uid before admission, and the Project webhook writes the creator's binding inside that + // admission, so the binding can exist before its target. The target is stored within its create request's timeout + // (60 s, the API server's default) or never; one more timeout covers clock error between the API server and this + // controller and a write that commits after its request answered 504. + targetStoreWindow = 120 * time.Second + // targetPollInterval is how often an absent target is read again inside the window. + targetPollInterval = time.Second +) + +// errTargetNamespace is a resourceRef that names a namespaced kind without its namespace. +var errTargetNamespace = errors.New("resourceRef names a namespaced kind without a namespace") + // PolicyBindingAdopter makes a PolicyBinding a dependent of the one object its resourceRef names, so the garbage // collector deletes the binding with that object. type PolicyBindingAdopter struct { @@ -20,21 +45,103 @@ type PolicyBindingAdopter struct { // the one the binding names. APIReader client.Reader // Mapper resolves the kind a resourceRef names, so any kind the API server serves can be a binding's owner. - Mapper meta.RESTMapper - Scheme *runtime.Scheme + Mapper meta.RESTMapper + Scheme *runtime.Scheme + EventRecorder record.EventRecorder // now is the clock the target's creation window is measured with; nil means time.Now. now func() time.Time } -// Reconcile adopts nothing yet. +// +kubebuilder:rbac:groups=iam.miloapis.com,resources=policybindings,verbs=get;list;watch;patch +// +kubebuilder:rbac:groups=iam.miloapis.com,resources=users;userpreferences;userinvitations,verbs=get +// +kubebuilder:rbac:groups=resourcemanager.miloapis.com,resources=projects;organizations;organizationmemberships,verbs=get +// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch + +// Reconcile adds a binding's target as its owner once the target is stored with the uid the binding names. func (r *PolicyBindingAdopter) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - return ctrl.Result{}, nil + var pb iamv1alpha1.PolicyBinding + if err := r.Client.Get(ctx, req.NamespacedName, &pb); err != nil || !adoptable(&pb) { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + return r.adoptLiveTarget(ctx, &pb) +} + +// adoptable reports whether the binding names one target and has no owner. A binding with any owner is left alone: +// the garbage collector keeps a dependent while any owner exists, so a second owner would extend its life. +func adoptable(pb *iamv1alpha1.PolicyBinding) bool { + return pb.Spec.ResourceSelector.ResourceRef != nil && pb.DeletionTimestamp == nil && len(pb.OwnerReferences) == 0 +} + +func (r *PolicyBindingAdopter) adoptLiveTarget(ctx context.Context, pb *iamv1alpha1.PolicyBinding) (ctrl.Result, error) { + target, err := r.liveTarget(ctx, pb.Spec.ResourceSelector.ResourceRef) + switch { + case apierrors.IsNotFound(err) || meta.IsNoMatchError(err): + return r.pollWhileStorable(pb), nil + case errors.Is(err, errTargetNamespace): + r.EventRecorder.Event(pb, corev1.EventTypeWarning, "OwnerNotAllowed", err.Error()) + return ctrl.Result{}, nil + case err != nil: + return ctrl.Result{}, err + } + return ctrl.Result{}, r.adoptOnto(ctx, pb, target) } -// SetupWithManager sets up the controller with the Manager. +// liveTarget reads the target's metadata from the API server, through the REST mapper, so any served kind works. +func (r *PolicyBindingAdopter) liveTarget(ctx context.Context, ref *iamv1alpha1.ResourceReference) (*metav1.PartialObjectMetadata, error) { + mapping, err := r.Mapper.RESTMapping(schema.GroupKind{Group: ref.APIGroup, Kind: ref.Kind}) + if err != nil { + return nil, err + } + key := client.ObjectKey{Name: ref.Name} + if mapping.Scope.Name() == meta.RESTScopeNameNamespace { + if ref.Namespace == "" { + return nil, errTargetNamespace + } + key.Namespace = ref.Namespace + } + target := &metav1.PartialObjectMetadata{} + target.SetGroupVersionKind(mapping.GroupVersionKind) + return target, r.APIReader.Get(ctx, key, target) +} + +// adoptOnto sets the target as the binding's owner when it is the object the binding names and is not being deleted. +// A deleting target is never adopted onto: an orphaning delete has just stripped its dependents' references. +func (r *PolicyBindingAdopter) adoptOnto(ctx context.Context, pb *iamv1alpha1.PolicyBinding, target *metav1.PartialObjectMetadata) error { + if string(target.GetUID()) != pb.Spec.ResourceSelector.ResourceRef.UID || target.GetDeletionTimestamp() != nil { + return nil + } + base := pb.DeepCopy() + if err := controllerutil.SetOwnerReference(target, pb, r.Scheme); err != nil { + r.EventRecorder.Event(pb, corev1.EventTypeWarning, "OwnerNotAllowed", err.Error()) + return nil + } + return r.Client.Patch(ctx, pb, client.MergeFromWithOptions(base, client.MergeFromWithOptimisticLock{})) +} + +// pollWhileStorable reads an absent target again while it can still be stored, and leaves the binding after that. +func (r *PolicyBindingAdopter) pollWhileStorable(pb *iamv1alpha1.PolicyBinding) ctrl.Result { + now := time.Now + if r.now != nil { + now = r.now + } + if now().Sub(pb.CreationTimestamp.Time) < targetStoreWindow { + return ctrl.Result{RequeueAfter: targetPollInterval} + } + return ctrl.Result{} +} + +// ownerReferencesChanged passes an update that changes the owner references, so a removed reference is set again. +var ownerReferencesChanged = predicate.Funcs{UpdateFunc: func(e event.UpdateEvent) bool { + return !equality.Semantic.DeepEqual(e.ObjectOld.GetOwnerReferences(), e.ObjectNew.GetOwnerReferences()) +}} + +// SetupWithManager sets up the controller with the Manager. A status-only update does not reconcile: the provider +// writes a binding's status on every reconcile of its own. func (r *PolicyBindingAdopter) SetupWithManager(mgr ctrl.Manager) error { + r.EventRecorder = mgr.GetEventRecorderFor("policybinding-adopter") return ctrl.NewControllerManagedBy(mgr). - For(&iamv1alpha1.PolicyBinding{}). + For(&iamv1alpha1.PolicyBinding{}, + builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, ownerReferencesChanged))). Named("policybinding-adopter"). Complete(r) } diff --git a/internal/controllers/iam/policybinding_adopter_envtest_test.go b/internal/controllers/iam/policybinding_adopter_envtest_test.go index 2b996c861..a51bf84e3 100644 --- a/internal/controllers/iam/policybinding_adopter_envtest_test.go +++ b/internal/controllers/iam/policybinding_adopter_envtest_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -56,6 +57,8 @@ func TestPolicyBindingAdopter(t *testing.T) { project := env.project(t, "d1") pb := env.binding(t, "d1", env.refTo(project), nil) env.waitOwnedBy(t, pb, project.UID) + require.Nil(t, pb.OwnerReferences[0].Controller, "the target is an owner, not the binding's controller") + require.Nil(t, pb.OwnerReferences[0].BlockOwnerDeletion, "the binding never holds its target's deletion") require.NoError(t, env.client.Delete(env.ctx, project)) env.waitGone(t, pb) }) @@ -78,6 +81,19 @@ func TestPolicyBindingAdopter(t *testing.T) { env.keepsOwners(t, pb, nil) }) + t.Run("D19 a status-only update does not reconcile: one Warning for a cross-namespace binding, none per status write", func(t *testing.T) { + cm := env.configMap(t, adopterOther, "d19") + pb := env.binding(t, "d19", iamv1alpha1.ResourceReference{ + Kind: "ConfigMap", Name: cm.Name, UID: string(cm.UID), Namespace: adopterOther, + }, nil) + env.eventually(t, func() bool { return env.warnings(t, pb) == 1 }, "one Warning for the create") + for i := range 3 { + env.writeStatus(t, pb, fmt.Sprintf("status write %d", i)) + } + time.Sleep(settle) + require.Equal(t, int32(1), env.warnings(t, pb), "no Warning for a status-only update") + }) + t.Run("D4 a binding owned by another object is never adopted and goes with that object", func(t *testing.T) { project := env.project(t, "d4") holder := env.configMap(t, adopterNamespace, "d4-holder") @@ -318,6 +334,35 @@ func (e *adopterEnv) projectThenBinding(name string) error { return e.client.Create(e.ctx, newBinding(name, e.refTo(p), nil)) } +// writeStatus writes a condition to the binding's status, as the authorization provider does on its reconciles. +func (e *adopterEnv) writeStatus(t *testing.T, pb *iamv1alpha1.PolicyBinding, message string) { + t.Helper() + require.NoError(t, retry.RetryOnConflict(retry.DefaultRetry, func() error { + if err := e.client.Get(e.ctx, client.ObjectKeyFromObject(pb), pb); err != nil { + return err + } + meta.SetStatusCondition(&pb.Status.Conditions, metav1.Condition{ + Type: "Ready", Status: metav1.ConditionFalse, Reason: "Test", Message: message, + }) + return e.client.Status().Update(e.ctx, pb) + })) +} + +// warnings counts the adopter's OwnerNotAllowed Warnings on the binding; the recorder folds repeats into one Event's +// count. +func (e *adopterEnv) warnings(t *testing.T, pb *iamv1alpha1.PolicyBinding) int32 { + t.Helper() + var events corev1.EventList + require.NoError(t, e.client.List(e.ctx, &events, client.InNamespace(pb.Namespace))) + var count int32 + for _, ev := range events.Items { + if ev.InvolvedObject.Name == pb.Name && ev.Reason == "OwnerNotAllowed" { + count += max(ev.Count, 1) + } + } + return count +} + func (e *adopterEnv) release(t *testing.T, obj client.Object) { t.Helper() require.NoError(t, retry.RetryOnConflict(retry.DefaultRetry, func() error { diff --git a/internal/controllers/iam/policybinding_adopter_test.go b/internal/controllers/iam/policybinding_adopter_test.go index 64e14aa72..8a67e7ccb 100644 --- a/internal/controllers/iam/policybinding_adopter_test.go +++ b/internal/controllers/iam/policybinding_adopter_test.go @@ -13,6 +13,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -27,9 +28,26 @@ var bindingCreated = time.Date(2026, 10, 20, 18, 0, 0, 0, time.UTC) // pollCase is one reconcile of a binding whose target the reader answers for. type pollCase struct { - age time.Duration // the adopter's clock, after the binding's creation - absentReads int // how many reads of the target answer NotFound before it is found - readErr error // an error every read of the target returns instead + age time.Duration // the adopter's clock, after the binding's creation + absentReads int // how many reads of the target answer NotFound before it is found + readErr error // an error every read of the target returns instead + unseenKinds int // how many mappings of the target's kind answer no match before the kind is known + beforePatch func(client.WithWatch) // runs once, just before the adopter's patch reaches the API server +} + +// unseenFirst is a REST mapper that has not yet seen the target's kind for its first misses lookups, as the shared +// DeferredDiscoveryRESTMapper answers until its next reset. +type unseenFirst struct { + meta.RESTMapper + misses int +} + +func (m *unseenFirst) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) { + if m.misses > 0 { + m.misses-- + return nil, &meta.NoKindMatchError{GroupKind: gk} + } + return m.RESTMapper.RESTMapping(gk, versions...) } // TestPolicyBindingAdopterWaitsForTheTarget covers rule 4 of the design: a target that is absent when its binding is @@ -48,6 +66,32 @@ func TestPolicyBindingAdopterWaitsForTheTarget(t *testing.T) { require.Equal(t, []types.UID{project.UID}, ownersOf(t, c)) }) + t.Run("D20 a kind the mapper has not seen yet counts as absent: polled, then adopted", func(t *testing.T) { + r, c, project := pollEnv(t, pollCase{age: 10 * time.Second, unseenKinds: 1}) + res, err := r.Reconcile(context.Background(), bindingRequest()) + require.NoError(t, err) + require.Equal(t, time.Second, res.RequeueAfter) + require.Empty(t, ownersOf(t, c)) + _, err = r.Reconcile(context.Background(), bindingRequest()) + require.NoError(t, err) + require.Equal(t, []types.UID{project.UID}, ownersOf(t, c)) + }) + + t.Run("D13 an owner written between the read and the patch is kept: the patch conflicts, the retry leaves it", func(t *testing.T) { + other := metav1.OwnerReference{APIVersion: "v1", Kind: "ConfigMap", Name: "holder", UID: "holder-uid"} + r, c, _ := pollEnv(t, pollCase{age: 10 * time.Second, beforePatch: func(cl client.WithWatch) { + var pb iamv1alpha1.PolicyBinding + require.NoError(t, cl.Get(context.Background(), bindingRequest().NamespacedName, &pb)) + pb.OwnerReferences = append(pb.OwnerReferences, other) + require.NoError(t, cl.Update(context.Background(), &pb)) + }}) + _, err := r.Reconcile(context.Background(), bindingRequest()) + require.True(t, apierrors.IsConflict(err), "the stale patch conflicts: %v", err) + _, err = r.Reconcile(context.Background(), bindingRequest()) + require.NoError(t, err) + require.Equal(t, []types.UID{other.UID}, ownersOf(t, c), "the other owner stays, and no second owner is added") + }) + t.Run("D3 absent 119 s after the binding's creation: still polled", func(t *testing.T) { r, c, _ := pollEnv(t, pollCase{age: 119 * time.Second, absentReads: 1 << 20}) res, err := r.Reconcile(context.Background(), bindingRequest()) @@ -87,10 +131,10 @@ func pollEnv(t *testing.T, pc pollCase) (*PolicyBindingAdopter, client.Client, * }, nil) pb.CreationTimestamp = metav1.NewTime(bindingCreated) // The fake's default mapper calls every kind namespaced; a Project is cluster-scoped, as Milo serves it. - mapper := meta.NewDefaultRESTMapper(nil) + mapper := meta.NewDefaultRESTMapper([]schema.GroupVersion{resourcemanagerv1alpha1.GroupVersion, iamv1alpha1.SchemeGroupVersion}) mapper.Add(resourcemanagerv1alpha1.GroupVersion.WithKind("Project"), meta.RESTScopeRoot) mapper.Add(iamv1alpha1.SchemeGroupVersion.WithKind("PolicyBinding"), meta.RESTScopeNamespace) - reads := 0 + reads, patched := 0, false c := fake.NewClientBuilder().WithScheme(scheme).WithRESTMapper(mapper).WithObjects(project, pb). WithInterceptorFuncs(interceptor.Funcs{Get: func(ctx context.Context, cl client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { if _, isTarget := obj.(*metav1.PartialObjectMetadata); isTarget { @@ -102,10 +146,17 @@ func pollEnv(t *testing.T, pc pollCase) (*PolicyBindingAdopter, client.Client, * } } return cl.Get(ctx, key, obj, opts...) + }, Patch: func(ctx context.Context, cl client.WithWatch, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + if pc.beforePatch != nil && !patched { + patched = true + pc.beforePatch(cl) + } + return cl.Patch(ctx, obj, patch, opts...) }}).Build() return &PolicyBindingAdopter{ - Client: c, APIReader: c, Mapper: mapper, Scheme: scheme, - now: func() time.Time { return bindingCreated.Add(pc.age) }, + Client: c, APIReader: c, Mapper: &unseenFirst{RESTMapper: mapper, misses: pc.unseenKinds}, Scheme: scheme, + EventRecorder: record.NewFakeRecorder(10), + now: func() time.Time { return bindingCreated.Add(pc.age) }, }, c, project } diff --git a/internal/webhooks/resourcemanager/v1alpha1/project_webhook.go b/internal/webhooks/resourcemanager/v1alpha1/project_webhook.go index 649547dbe..8ea47f00d 100644 --- a/internal/webhooks/resourcemanager/v1alpha1/project_webhook.go +++ b/internal/webhooks/resourcemanager/v1alpha1/project_webhook.go @@ -231,18 +231,12 @@ func (v *ProjectValidator) createOwnerPolicyBinding(ctx context.Context, project Namespace: fmt.Sprintf("organization-%s", project.Spec.OwnerRef.Name), Labels: map[string]string{ // Lets the subject-binding reaper controller find this binding - // when foundUser is deleted; the OwnerReference below only - // covers cleanup when the Project itself is deleted. + // when foundUser is deleted. v1alpha1.SubjectUserNameLabel: foundUser.Name, }, - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: v1alpha1.GroupVersion.String(), - Kind: "Project", - Name: project.Name, - UID: project.UID, - }, - }, + // No owner reference: the Project is not stored yet, so the garbage + // collector would find no owner and delete the binding. The + // PolicyBinding adopter sets it once the Project is stored. }, Spec: iamv1alpha1.PolicyBindingSpec{ RoleRef: iamv1alpha1.RoleReference{ From d7b4958f9113bd150f22a094a8dcb0626f67b49c Mon Sep 17 00:00:00 2001 From: Viktor Voytovych Date: Tue, 20 Oct 2026 22:25:02 +0300 Subject: [PATCH 5/7] chore: Regenerate the controller manager's role for the binding adopter's events --- .../overlays/core-control-plane/rbac/role.yaml | 7 +++++++ 1 file changed, 7 insertions(+) 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 c8a2013ff..a7dd1fe37 100644 --- a/config/controller-manager/overlays/core-control-plane/rbac/role.yaml +++ b/config/controller-manager/overlays/core-control-plane/rbac/role.yaml @@ -4,6 +4,13 @@ kind: ClusterRole metadata: name: milo-controller-manager rules: +- apiGroups: + - "" + resources: + - events + verbs: + - create + - patch - apiGroups: - "" resources: From a44fa01ab15a90cce56c209505ac906be325f606 Mon Sep 17 00:00:00 2001 From: Viktor Voytovych Date: Wed, 21 Oct 2026 08:56:19 +0300 Subject: [PATCH 6/7] fix: Leave a User's self-manage bindings to the binding adopter The User controller set the User as the owner of the User's two self-manage bindings while the adopter set each binding's target, so the userpreference-self-manage binding's owners depended on which ran first. The adopter is now their one writer: user-self-manage is owned by the User, userpreference-self-manage by the UserPreference, which the User owns, so both go with the User. The User controller keeps the UserPreference's owner. --- .../iam/policybinding_adopter_envtest_test.go | 36 +++++++++++++- internal/controllers/iam/user_controller.go | 45 ++--------------- .../iam/user_owner_references_test.go | 49 +++++++++++++++++++ ...ference-policy-binding-with-owner-ref.yaml | 4 +- .../chainsaw-test.yaml | 3 +- 5 files changed, 93 insertions(+), 44 deletions(-) create mode 100644 internal/controllers/iam/user_owner_references_test.go diff --git a/internal/controllers/iam/policybinding_adopter_envtest_test.go b/internal/controllers/iam/policybinding_adopter_envtest_test.go index a51bf84e3..78f2a4baf 100644 --- a/internal/controllers/iam/policybinding_adopter_envtest_test.go +++ b/internal/controllers/iam/policybinding_adopter_envtest_test.go @@ -176,6 +176,31 @@ func TestPolicyBindingAdopter(t *testing.T) { require.Len(t, pb.Spec.Subjects, 2, "the subject edit survives the adoption") }) + t.Run("D21 a User's two self-manage bindings have one owner each and go with the User through its UserPreference", func(t *testing.T) { + user := &iamv1alpha1.User{ObjectMeta: metav1.ObjectMeta{Name: "d21"}, Spec: iamv1alpha1.UserSpec{Email: "d21@example.com"}} + require.NoError(t, env.client.Create(env.ctx, user)) + // The User controller makes the User the owner of its UserPreference; this test stands in for it. + pref := &iamv1alpha1.UserPreference{ + ObjectMeta: metav1.ObjectMeta{Name: "userpreference-d21", OwnerReferences: []metav1.OwnerReference{ + {APIVersion: iamv1alpha1.SchemeGroupVersion.String(), Kind: "User", Name: user.Name, UID: user.UID}, + }}, + Spec: iamv1alpha1.UserPreferenceSpec{UserRef: iamv1alpha1.UserReference{Name: user.Name}}, + } + require.NoError(t, env.client.Create(env.ctx, pref)) + selfManage := env.bindingIn(t, "milo-system", "user-self-manage-d21", iamv1alpha1.ResourceReference{ + APIGroup: iamv1alpha1.SchemeGroupVersion.Group, Kind: "User", Name: user.Name, UID: string(user.UID), + }) + prefSelfManage := env.bindingIn(t, "milo-system", "userpreference-self-manage-d21", iamv1alpha1.ResourceReference{ + APIGroup: iamv1alpha1.SchemeGroupVersion.Group, Kind: "UserPreference", Name: pref.Name, UID: string(pref.UID), + }) + env.waitOwnedBy(t, selfManage, user.UID) + env.waitOwnedBy(t, prefSelfManage, pref.UID) + require.NoError(t, env.client.Delete(env.ctx, user)) + env.waitGone(t, pref) + env.waitGone(t, selfManage) + env.waitGone(t, prefSelfManage) + }) + t.Run("stress: a burst of 60 bindings, made serially and in parallel, are all adopted and none is deleted", func(t *testing.T) { const n = 30 var wg sync.WaitGroup @@ -253,7 +278,7 @@ func startAdopterEnv(t *testing.T, preload func(*adopterEnv)) *adopterEnv { t.Cleanup(cancel) require.NoError(t, gctest.Start(ctx, cfg, gctest.Options{})) env := &adopterEnv{ctx: ctx, client: direct} - for _, ns := range []string{adopterNamespace, adopterOther} { + for _, ns := range []string{adopterNamespace, adopterOther, "milo-system"} { require.NoError(t, direct.Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}})) } require.NoError(t, direct.Create(ctx, &resourcemanagerv1alpha1.Organization{ @@ -313,6 +338,15 @@ func newBinding(name string, ref iamv1alpha1.ResourceReference, owners []metav1. } } +// bindingIn creates an unowned binding in a namespace other than the tests' organization namespace. +func (e *adopterEnv) bindingIn(t *testing.T, namespace, name string, ref iamv1alpha1.ResourceReference) *iamv1alpha1.PolicyBinding { + t.Helper() + pb := newBinding(name, ref, nil) + pb.Namespace = namespace + require.NoError(t, e.client.Create(e.ctx, pb)) + return pb +} + func (e *adopterEnv) binding(t *testing.T, name string, ref iamv1alpha1.ResourceReference, owners []metav1.OwnerReference) *iamv1alpha1.PolicyBinding { t.Helper() pb := newBinding(name, ref, owners) diff --git a/internal/controllers/iam/user_controller.go b/internal/controllers/iam/user_controller.go index 7c1b56042..8f0e8eacf 100644 --- a/internal/controllers/iam/user_controller.go +++ b/internal/controllers/iam/user_controller.go @@ -33,7 +33,6 @@ type UserController struct { // +kubebuilder:rbac:groups=iam.miloapis.com,resources=users,verbs=get;list;watch;update // +kubebuilder:rbac:groups=iam.miloapis.com,resources=users/status,verbs=update -// +kubebuilder:rbac:groups=iam.miloapis.com,resources=policybindings,verbs=get;list;watch;update;patch // +kubebuilder:rbac:groups=iam.miloapis.com,resources=userpreferences,verbs=get;list;watch;update;patch // +kubebuilder:rbac:groups=iam.miloapis.com,resources=platformaccesses,verbs=get;list;watch // +kubebuilder:rbac:groups=resourcemanager.miloapis.com,resources=organizationmemberships,verbs=list;delete @@ -77,7 +76,7 @@ func (r *UserController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. } } - // Ensure owner references are set on PolicyBinding and UserPreference resources + // Make the User the owner of its UserPreference if err := r.ensureOwnerReferences(ctx, user); err != nil { log.Error(err, "Failed to ensure owner references") return ctrl.Result{}, err @@ -159,11 +158,12 @@ func (r *UserController) reconcileNameReviewAnnotation(ctx context.Context, user return nil } -// ensureOwnerReferences ensures that PolicyBinding and UserPreference resources have proper owner references +// ensureOwnerReferences makes the User the owner of its UserPreference, so the garbage collector deletes the +// preference with the User. The User's self-manage PolicyBindings get their owner from the PolicyBinding adopter, which +// makes each binding a dependent of the object it targets. func (r *UserController) ensureOwnerReferences(ctx context.Context, user *iamv1alpha1.User) error { log := log.FromContext(ctx).WithName("ensure-owner-references") - // Create owner reference for the user ownerRef := metav1.OwnerReference{ APIVersion: iamv1alpha1.SchemeGroupVersion.String(), Kind: "User", @@ -171,27 +171,9 @@ func (r *UserController) ensureOwnerReferences(ctx context.Context, user *iamv1a UID: user.UID, } - // Update PolicyBinding for user self-management - policyBindingName := fmt.Sprintf("user-self-manage-%s", user.Name) - policyBinding := &iamv1alpha1.PolicyBinding{} - err := r.Client.Get(ctx, types.NamespacedName{Name: policyBindingName, Namespace: "milo-system"}, policyBinding) - if apierrors.IsNotFound(err) { - // PolicyBinding doesn't exist, webhook should have created it - log.Info("PolicyBinding not found, skipping (webhook should create it)", "user", user.Name, "policyBinding", policyBindingName) - } else if err != nil { - return fmt.Errorf("failed to get policy binding: %w", err) - } else if !hasOwnerReference(policyBinding.OwnerReferences, ownerRef) { - policyBinding.OwnerReferences = append(policyBinding.OwnerReferences, ownerRef) - if err := r.Client.Update(ctx, policyBinding); err != nil { - return fmt.Errorf("failed to update policy binding with owner reference: %w", err) - } - log.Info("Updated PolicyBinding with owner reference", "user", user.Name) - } - - // Update UserPreference userPreferenceName := fmt.Sprintf("userpreference-%s", user.Name) userPreference := &iamv1alpha1.UserPreference{} - err = r.Client.Get(ctx, types.NamespacedName{Name: userPreferenceName}, userPreference) + err := r.Client.Get(ctx, types.NamespacedName{Name: userPreferenceName}, userPreference) if apierrors.IsNotFound(err) { // UserPreference doesn't exist, webhook should have created it log.Info("UserPreference not found, skipping (webhook should create it)", "user", user.Name, "userPreference", userPreferenceName) @@ -205,23 +187,6 @@ func (r *UserController) ensureOwnerReferences(ctx context.Context, user *iamv1a log.Info("Updated UserPreference with owner reference", "user", user.Name) } - // Update UserPreference PolicyBinding for user preference management - userPreferencePolicyBindingName := fmt.Sprintf("userpreference-self-manage-%s", user.Name) - userPreferencePolicyBinding := &iamv1alpha1.PolicyBinding{} - err = r.Client.Get(ctx, types.NamespacedName{Name: userPreferencePolicyBindingName, Namespace: "milo-system"}, userPreferencePolicyBinding) - if apierrors.IsNotFound(err) { - // UserPreference PolicyBinding doesn't exist, webhook should have created it - log.Info("UserPreference PolicyBinding not found, skipping (webhook should create it)", "user", user.Name, "policyBinding", userPreferencePolicyBindingName) - } else if err != nil { - return fmt.Errorf("failed to get user preference policy binding: %w", err) - } else if !hasOwnerReference(userPreferencePolicyBinding.OwnerReferences, ownerRef) { - userPreferencePolicyBinding.OwnerReferences = append(userPreferencePolicyBinding.OwnerReferences, ownerRef) - if err := r.Client.Update(ctx, userPreferencePolicyBinding); err != nil { - return fmt.Errorf("failed to update user preference policy binding with owner reference: %w", err) - } - log.Info("Updated UserPreference PolicyBinding with owner reference", "user", user.Name) - } - return nil } diff --git a/internal/controllers/iam/user_owner_references_test.go b/internal/controllers/iam/user_owner_references_test.go new file mode 100644 index 000000000..236daa2d9 --- /dev/null +++ b/internal/controllers/iam/user_owner_references_test.go @@ -0,0 +1,49 @@ +package iam + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + iamv1alpha1 "go.miloapis.com/milo/pkg/apis/iam/v1alpha1" +) + +// TestEnsureOwnerReferencesLeavesBindingsToTheAdopter: the User controller makes the User the owner of its +// UserPreference and writes no owner on the User's self-manage bindings. The PolicyBinding adopter is their one writer, +// so a binding's owner does not depend on which controller runs first. +func TestEnsureOwnerReferencesLeavesBindingsToTheAdopter(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, iamv1alpha1.AddToScheme(scheme)) + user := &iamv1alpha1.User{ObjectMeta: metav1.ObjectMeta{Name: "u", UID: "user-uid"}} + pref := &iamv1alpha1.UserPreference{ObjectMeta: metav1.ObjectMeta{Name: "userpreference-u"}} + selfManage := selfManageBinding("user-self-manage-u") + prefSelfManage := selfManageBinding("userpreference-self-manage-u") + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(user, pref, selfManage, prefSelfManage).Build() + + require.NoError(t, (&UserController{Client: c}).ensureOwnerReferences(context.Background(), user)) + + require.NoError(t, c.Get(context.Background(), client.ObjectKeyFromObject(pref), pref)) + require.Equal(t, []types.UID{user.UID}, preferenceOwners(pref), "the User owns its UserPreference") + for _, pb := range []*iamv1alpha1.PolicyBinding{selfManage, prefSelfManage} { + require.NoError(t, c.Get(context.Background(), client.ObjectKeyFromObject(pb), pb)) + require.Empty(t, pb.OwnerReferences, "%s's owner is the adopter's to write", pb.Name) + } +} + +func selfManageBinding(name string) *iamv1alpha1.PolicyBinding { + return &iamv1alpha1.PolicyBinding{ObjectMeta: metav1.ObjectMeta{Namespace: "milo-system", Name: name}} +} + +func preferenceOwners(pref *iamv1alpha1.UserPreference) []types.UID { + var uids []types.UID + for _, o := range pref.OwnerReferences { + uids = append(uids, o.UID) + } + return uids +} diff --git a/test/iam/user-deletion-garbage-collection/assertions/user-preference-policy-binding-with-owner-ref.yaml b/test/iam/user-deletion-garbage-collection/assertions/user-preference-policy-binding-with-owner-ref.yaml index 15bf307f8..b495f7892 100644 --- a/test/iam/user-deletion-garbage-collection/assertions/user-preference-policy-binding-with-owner-ref.yaml +++ b/test/iam/user-deletion-garbage-collection/assertions/user-preference-policy-binding-with-owner-ref.yaml @@ -5,5 +5,5 @@ metadata: namespace: milo-system ownerReferences: - apiVersion: iam.miloapis.com/v1alpha1 - kind: User - name: test-gc-user \ No newline at end of file + kind: UserPreference + name: userpreference-test-gc-user \ No newline at end of file diff --git a/test/iam/user-deletion-garbage-collection/chainsaw-test.yaml b/test/iam/user-deletion-garbage-collection/chainsaw-test.yaml index 6b19f0ebe..73ed84415 100644 --- a/test/iam/user-deletion-garbage-collection/chainsaw-test.yaml +++ b/test/iam/user-deletion-garbage-collection/chainsaw-test.yaml @@ -50,8 +50,9 @@ spec: timeout: 30s for: jsonPath: + # The binding's target, the UserPreference, is its owner; the User owns the UserPreference. path: '{.metadata.ownerReferences[0].name}' - value: test-gc-user + value: userpreference-test-gc-user - name: delete-user description: Delete the User resource and verify associated resources are garbage collected From 959067d1cdb7e301dfe03c2055cb437bb7871001 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 21 Oct 2026 09:51:55 +0000 Subject: [PATCH 7/7] chore: add missing newlines at end of files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Automatically added newlines to 1 file(s) Co-Authored-By: github-actions[bot] --- .../user-preference-policy-binding-with-owner-ref.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/iam/user-deletion-garbage-collection/assertions/user-preference-policy-binding-with-owner-ref.yaml b/test/iam/user-deletion-garbage-collection/assertions/user-preference-policy-binding-with-owner-ref.yaml index b495f7892..635baf4d3 100644 --- a/test/iam/user-deletion-garbage-collection/assertions/user-preference-policy-binding-with-owner-ref.yaml +++ b/test/iam/user-deletion-garbage-collection/assertions/user-preference-policy-binding-with-owner-ref.yaml @@ -6,4 +6,4 @@ metadata: ownerReferences: - apiVersion: iam.miloapis.com/v1alpha1 kind: UserPreference - name: userpreference-test-gc-user \ No newline at end of file + name: userpreference-test-gc-user