From 0e89f2ff5b930209b325cab79be874fd270ef168 Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Mon, 23 Feb 2026 12:15:06 -0300 Subject: [PATCH 1/2] feat: PersonalOrganizationController now creates personal projects using an impersonated client to correctly attribute ownership, and the build workflow runs on all branches. --- .github/workflows/build-and-test.yaml | 1 + cmd/controller/manager.go | 7 +- .../personal_organization_controller.go | 66 +++++++++++++++++-- 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/.github/workflows/build-and-test.yaml b/.github/workflows/build-and-test.yaml index 0c1e343..9225e0e 100644 --- a/.github/workflows/build-and-test.yaml +++ b/.github/workflows/build-and-test.yaml @@ -4,6 +4,7 @@ on: push: branches: - main + - '**' pull_request: release: types: diff --git a/cmd/controller/manager.go b/cmd/controller/manager.go index 90677fd..9cef006 100644 --- a/cmd/controller/manager.go +++ b/cmd/controller/manager.go @@ -287,9 +287,10 @@ func runControllerManager( } if err = (&resourcemanagercontroller.PersonalOrganizationController{ - Client: mgr.GetClient(), - Config: serverConfig.PersonalOrganizationController, - Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Config: serverConfig.PersonalOrganizationController, + Scheme: mgr.GetScheme(), + RestConfig: mgr.GetConfig(), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "PersonalOrganization") return err diff --git a/internal/controller/resourcemanager/personal_organization_controller.go b/internal/controller/resourcemanager/personal_organization_controller.go index b9299c2..b3f4188 100644 --- a/internal/controller/resourcemanager/personal_organization_controller.go +++ b/internal/controller/resourcemanager/personal_organization_controller.go @@ -8,8 +8,10 @@ import ( "fmt" "hash/fnv" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -40,6 +42,9 @@ type PersonalOrganizationController struct { // The scheme is used to set the controller reference on the personal // organization. Scheme *runtime.Scheme + + // RestConfig is used to create an impersonated client for project creation. + RestConfig *rest.Config } // +kubebuilder:rbac:groups=iam.datumapis.com,resources=users,verbs=get;list;watch @@ -57,9 +62,18 @@ func (r *PersonalOrganizationController) Reconcile(ctx context.Context, req ctrl // Get the user. user := &iamv1alpha1.User{} if err := r.Client.Get(ctx, req.NamespacedName, user); err != nil { + if apierrors.IsNotFound(err) { + logger.Info("User not found, skipping reconciliation", "user", req.NamespacedName) + return ctrl.Result{}, nil + } return ctrl.Result{}, fmt.Errorf("failed to get user: %w", err) } + if !user.DeletionTimestamp.IsZero() { + logger.Info("User is being deleted, skipping reconciliation", "user", user.Name) + return ctrl.Result{}, nil + } + // Automatically create a personal organization for the user. They should not // be able to modify or delete the organization. personalOrg := &resourcemanagerv1alpha1.Organization{ @@ -116,18 +130,51 @@ func (r *PersonalOrganizationController) Reconcile(ctx context.Context, req ctrl } // Create a default personal project in the personal organization. + // The project webhook requires parent context in UserInfo.Extra fields, + // and also looks up the requesting user by UID to create a PolicyBinding + // granting them ownership. We impersonate the actual user so the webhook + // sees the correct identity and creates the right PolicyBinding. personalProjectID := hashPersonalOrgName(string(user.UID)) personalProject := &resourcemanagerv1alpha1.Project{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("personal-project-%s", personalProjectID), }, } - _, err = controllerutil.CreateOrUpdate(ctx, r.Client, personalProject, func() error { - logger.Info("Creating or updating personal project", "organization", personalOrg.Name, "project", personalProject.Name) + + impersonatedConfig := rest.CopyConfig(r.RestConfig) + impersonatedConfig.Impersonate = rest.ImpersonationConfig{ + UserName: user.Name, + UID: user.Name, + Groups: []string{"system:authenticated"}, + Extra: map[string][]string{ + "iam.miloapis.com/parent-name": {personalOrg.Name}, + "iam.miloapis.com/parent-type": {"Organization"}, + "iam.miloapis.com/parent-api-group": {"resourcemanager.miloapis.com"}, + }, + } + + impersonatedClient, err := client.New(impersonatedConfig, client.Options{Scheme: r.Scheme}) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to create impersonated client: %w", err) + } + + // Use the controller's own client (cluster-scope RBAC) to check whether + // the project already exists. The impersonated user only has org-scoped + // permissions and cannot GET projects at the cluster scope. + existingProject := &resourcemanagerv1alpha1.Project{} + err = r.Client.Get(ctx, client.ObjectKeyFromObject(personalProject), existingProject) + if err != nil { + if !apierrors.IsNotFound(err) { + return ctrl.Result{}, fmt.Errorf("failed to check for existing personal project: %w", err) + } + + // Project does not exist — create it via the impersonated client so + // the webhook sees the actual user's identity. + logger.Info("Creating personal project", "organization", personalOrg.Name, "project", personalProject.Name) metav1.SetMetaDataAnnotation(&personalProject.ObjectMeta, "kubernetes.io/display-name", "Personal Project") metav1.SetMetaDataAnnotation(&personalProject.ObjectMeta, "kubernetes.io/description", fmt.Sprintf("%s %s's Personal Project", user.Spec.GivenName, user.Spec.FamilyName)) if err := controllerutil.SetControllerReference(user, personalProject, r.Scheme); err != nil { - return fmt.Errorf("failed to set controller reference: %w", err) + return ctrl.Result{}, fmt.Errorf("failed to set controller reference: %w", err) } personalProject.Spec = resourcemanagerv1alpha1.ProjectSpec{ OwnerRef: resourcemanagerv1alpha1.OwnerReference{ @@ -135,10 +182,15 @@ func (r *PersonalOrganizationController) Reconcile(ctx context.Context, req ctrl Name: personalOrg.Name, }, } - return nil - }) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to create or update personal project: %w", err) + + if err := impersonatedClient.Create(ctx, personalProject); err != nil { + if apierrors.IsAlreadyExists(err) { + logger.Info("Personal project already exists (race)", "project", personalProject.Name) + } else { + logger.Error(err, "Failed to create personal project") + return ctrl.Result{}, fmt.Errorf("failed to create personal project: %w", err) + } + } } logger.Info("Successfully created or updated personal organization resources", "organization", personalOrg.Name) From 8d9acee4ded60159d2b0ae48c2e92767eadc6278 Mon Sep 17 00:00:00 2001 From: Jose Szychowski Date: Mon, 23 Feb 2026 12:23:36 -0300 Subject: [PATCH 2/2] Refactor: relocate impersonated client initialization to the project creation path and remove controller reference and owner spec assignments. --- .../personal_organization_controller.go | 51 ++++++++----------- 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/internal/controller/resourcemanager/personal_organization_controller.go b/internal/controller/resourcemanager/personal_organization_controller.go index b3f4188..6466885 100644 --- a/internal/controller/resourcemanager/personal_organization_controller.go +++ b/internal/controller/resourcemanager/personal_organization_controller.go @@ -130,10 +130,6 @@ func (r *PersonalOrganizationController) Reconcile(ctx context.Context, req ctrl } // Create a default personal project in the personal organization. - // The project webhook requires parent context in UserInfo.Extra fields, - // and also looks up the requesting user by UID to create a PolicyBinding - // granting them ownership. We impersonate the actual user so the webhook - // sees the correct identity and creates the right PolicyBinding. personalProjectID := hashPersonalOrgName(string(user.UID)) personalProject := &resourcemanagerv1alpha1.Project{ ObjectMeta: metav1.ObjectMeta{ @@ -141,23 +137,6 @@ func (r *PersonalOrganizationController) Reconcile(ctx context.Context, req ctrl }, } - impersonatedConfig := rest.CopyConfig(r.RestConfig) - impersonatedConfig.Impersonate = rest.ImpersonationConfig{ - UserName: user.Name, - UID: user.Name, - Groups: []string{"system:authenticated"}, - Extra: map[string][]string{ - "iam.miloapis.com/parent-name": {personalOrg.Name}, - "iam.miloapis.com/parent-type": {"Organization"}, - "iam.miloapis.com/parent-api-group": {"resourcemanager.miloapis.com"}, - }, - } - - impersonatedClient, err := client.New(impersonatedConfig, client.Options{Scheme: r.Scheme}) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to create impersonated client: %w", err) - } - // Use the controller's own client (cluster-scope RBAC) to check whether // the project already exists. The impersonated user only has org-scoped // permissions and cannot GET projects at the cluster scope. @@ -168,20 +147,32 @@ func (r *PersonalOrganizationController) Reconcile(ctx context.Context, req ctrl return ctrl.Result{}, fmt.Errorf("failed to check for existing personal project: %w", err) } + // The project webhook requires parent context in UserInfo.Extra fields, + // and also looks up the requesting user by UID to create a PolicyBinding + // granting them ownership. We impersonate the actual user so the webhook + // sees the correct identity and creates the right PolicyBinding. + impersonatedConfig := rest.CopyConfig(r.RestConfig) + impersonatedConfig.Impersonate = rest.ImpersonationConfig{ + UserName: user.Name, + UID: user.Name, + Groups: []string{"system:authenticated"}, + Extra: map[string][]string{ + "iam.miloapis.com/parent-name": {personalOrg.Name}, + "iam.miloapis.com/parent-type": {"Organization"}, + "iam.miloapis.com/parent-api-group": {"resourcemanager.miloapis.com"}, + }, + } + + impersonatedClient, err := client.New(impersonatedConfig, client.Options{Scheme: r.Scheme}) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to create impersonated client: %w", err) + } + // Project does not exist — create it via the impersonated client so // the webhook sees the actual user's identity. logger.Info("Creating personal project", "organization", personalOrg.Name, "project", personalProject.Name) metav1.SetMetaDataAnnotation(&personalProject.ObjectMeta, "kubernetes.io/display-name", "Personal Project") metav1.SetMetaDataAnnotation(&personalProject.ObjectMeta, "kubernetes.io/description", fmt.Sprintf("%s %s's Personal Project", user.Spec.GivenName, user.Spec.FamilyName)) - if err := controllerutil.SetControllerReference(user, personalProject, r.Scheme); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to set controller reference: %w", err) - } - personalProject.Spec = resourcemanagerv1alpha1.ProjectSpec{ - OwnerRef: resourcemanagerv1alpha1.OwnerReference{ - Kind: "Organization", - Name: personalOrg.Name, - }, - } if err := impersonatedClient.Create(ctx, personalProject); err != nil { if apierrors.IsAlreadyExists(err) {