From 0b479c70a54daa7528a3fa8dfc7c5b1c0a44fc25 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Mon, 1 Jun 2026 15:51:59 +0200 Subject: [PATCH 01/12] feat(teams): create Role and RoleBinding for support-group SA token requests On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- .../manager/templates/rbac/manager-role.yaml | 11 ++ internal/controller/team/team_controller.go | 139 ++++++++++++++++-- .../controller/team/team_controller_test.go | 82 +++++++++-- 3 files changed, 207 insertions(+), 25 deletions(-) diff --git a/charts/manager/templates/rbac/manager-role.yaml b/charts/manager/templates/rbac/manager-role.yaml index 208ea3151..934dda81a 100644 --- a/charts/manager/templates/rbac/manager-role.yaml +++ b/charts/manager/templates/rbac/manager-role.yaml @@ -202,10 +202,21 @@ rules: resources: - clusterrolebindings - clusterroles + verbs: + - create + - get + - list + - patch + - update + - watch +- apiGroups: + - rbac.authorization.k8s.io + resources: - rolebindings - roles verbs: - create + - delete - get - list - patch diff --git a/internal/controller/team/team_controller.go b/internal/controller/team/team_controller.go index 01f30a671..3fe0b9dc0 100644 --- a/internal/controller/team/team_controller.go +++ b/internal/controller/team/team_controller.go @@ -10,6 +10,7 @@ import ( "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" @@ -53,6 +54,8 @@ type TeamController struct { //+kubebuilder:rbac:groups=greenhouse.sap,resources=organizations,verbs=get //+kubebuilder:rbac:groups="events.k8s.io",resources=events,verbs=get;list;watch;update;patch //+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=roles,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=rolebindings,verbs=get;list;watch;create;update;patch;delete // SetupWithManager sets up the controller with the Manager. func (r *TeamController) SetupWithManager(name string, mgr ctrl.Manager) error { @@ -168,9 +171,15 @@ func (r *TeamController) EnsureCreated(ctx context.Context, object lifecycle.Run if err := r.reconcileSupportGroupServiceAccount(ctx, team); err != nil { return ctrl.Result{}, lifecycle.Failed, err } + if err := r.reconcileSupportGroupRole(ctx, team); err != nil { + return ctrl.Result{}, lifecycle.Failed, err + } + if err := r.reconcileSupportGroupRoleBinding(ctx, team); err != nil { + return ctrl.Result{}, lifecycle.Failed, err + } } else { - // If the support-group label was removed, delete the SA if it still exists. - if err := r.deleteSupportGroupServiceAccountIfExists(ctx, team); err != nil { + // If the support-group label was removed, delete the SA, Role, and RoleBinding if they still exist. + if err := r.deleteSupportGroupResourcesIfExist(ctx, team); err != nil { return ctrl.Result{}, lifecycle.Failed, err } } @@ -282,20 +291,124 @@ func (r *TeamController) reconcileSupportGroupServiceAccount(ctx context.Context return nil } -func (r *TeamController) deleteSupportGroupServiceAccountIfExists(ctx context.Context, team *greenhousev1alpha1.Team) error { - serviceAccount := &corev1.ServiceAccount{} - err := r.Get(ctx, types.NamespacedName{ - Name: team.Name + "-sa", - Namespace: team.Namespace, - }, serviceAccount) +func (r *TeamController) reconcileSupportGroupRole(ctx context.Context, team *greenhousev1alpha1.Team) error { + saName := team.Name + "-sa" + role := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: team.Name + "-sa-token-request", + Namespace: team.Namespace, + }, + } + + result, err := clientutil.CreateOrPatch(ctx, r.Client, role, func() error { + role.Rules = []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"serviceaccounts/token"}, + Verbs: []string{"create"}, + ResourceNames: []string{saName}, + }, + } + return controllerutil.SetControllerReference(team, role, r.Scheme()) + }) if err != nil { - return client.IgnoreNotFound(err) + return err + } + + switch result { + case clientutil.OperationResultCreated: + log.FromContext(ctx).Info("created support group role", "name", role.Name, "namespace", role.Namespace) + r.recorder.Eventf(team, role, corev1.EventTypeNormal, "CreatedRole", "reconciling support group role", "Created Role %s/%s", role.Namespace, role.Name) + case clientutil.OperationResultUpdated: + log.FromContext(ctx).Info("updated support group role", "name", role.Name, "namespace", role.Namespace) + r.recorder.Eventf(team, role, corev1.EventTypeNormal, "UpdatedRole", "reconciling support group role", "Updated Role %s/%s", role.Namespace, role.Name) } - if err := r.Delete(ctx, serviceAccount); err != nil { - return client.IgnoreNotFound(err) + + return nil +} + +func (r *TeamController) reconcileSupportGroupRoleBinding(ctx context.Context, team *greenhousev1alpha1.Team) error { + saName := team.Name + "-sa" + roleName := team.Name + "-sa-token-request" + roleBinding := &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: roleName, + Namespace: team.Namespace, + }, } - log.FromContext(ctx).Info("deleted support group service account", "name", serviceAccount.Name, "namespace", serviceAccount.Namespace) - r.recorder.Eventf(team, serviceAccount, corev1.EventTypeNormal, "DeletedServiceAccount", "support-group label removed", "Deleted ServiceAccount %s/%s", serviceAccount.Namespace, serviceAccount.Name) + + result, err := clientutil.CreateOrPatch(ctx, r.Client, roleBinding, func() error { + roleBinding.RoleRef = rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "Role", + Name: roleName, + } + roleBinding.Subjects = []rbacv1.Subject{ + { + APIGroup: rbacv1.GroupName, + Kind: rbacv1.GroupKind, + Name: "support-group:" + team.Name, + }, + { + Kind: rbacv1.ServiceAccountKind, + Name: saName, + Namespace: team.Namespace, + }, + } + return controllerutil.SetControllerReference(team, roleBinding, r.Scheme()) + }) + if err != nil { + return err + } + + switch result { + case clientutil.OperationResultCreated: + log.FromContext(ctx).Info("created support group role binding", "name", roleBinding.Name, "namespace", roleBinding.Namespace) + r.recorder.Eventf(team, roleBinding, corev1.EventTypeNormal, "CreatedRoleBinding", "reconciling support group role binding", "Created RoleBinding %s/%s", roleBinding.Namespace, roleBinding.Name) + case clientutil.OperationResultUpdated: + log.FromContext(ctx).Info("updated support group role binding", "name", roleBinding.Name, "namespace", roleBinding.Namespace) + r.recorder.Eventf(team, roleBinding, corev1.EventTypeNormal, "UpdatedRoleBinding", "reconciling support group role binding", "Updated RoleBinding %s/%s", roleBinding.Namespace, roleBinding.Name) + } + + return nil +} + +func (r *TeamController) deleteSupportGroupResourcesIfExist(ctx context.Context, team *greenhousev1alpha1.Team) error { + resourceName := team.Name + "-sa-token-request" + + roleBinding := &rbacv1.RoleBinding{} + if err := r.Get(ctx, types.NamespacedName{Name: resourceName, Namespace: team.Namespace}, roleBinding); err == nil { + if err := r.Delete(ctx, roleBinding); client.IgnoreNotFound(err) != nil { + return err + } + log.FromContext(ctx).Info("deleted support group role binding", "name", roleBinding.Name, "namespace", roleBinding.Namespace) + r.recorder.Eventf(team, roleBinding, corev1.EventTypeNormal, "DeletedRoleBinding", "support-group label removed", "Deleted RoleBinding %s/%s", roleBinding.Namespace, roleBinding.Name) + } else if client.IgnoreNotFound(err) != nil { + return err + } + + role := &rbacv1.Role{} + if err := r.Get(ctx, types.NamespacedName{Name: resourceName, Namespace: team.Namespace}, role); err == nil { + if err := r.Delete(ctx, role); client.IgnoreNotFound(err) != nil { + return err + } + log.FromContext(ctx).Info("deleted support group role", "name", role.Name, "namespace", role.Namespace) + r.recorder.Eventf(team, role, corev1.EventTypeNormal, "DeletedRole", "support-group label removed", "Deleted Role %s/%s", role.Namespace, role.Name) + } else if client.IgnoreNotFound(err) != nil { + return err + } + + serviceAccount := &corev1.ServiceAccount{} + if err := r.Get(ctx, types.NamespacedName{Name: team.Name + "-sa", Namespace: team.Namespace}, serviceAccount); err == nil { + if err := r.Delete(ctx, serviceAccount); client.IgnoreNotFound(err) != nil { + return err + } + log.FromContext(ctx).Info("deleted support group service account", "name", serviceAccount.Name, "namespace", serviceAccount.Namespace) + r.recorder.Eventf(team, serviceAccount, corev1.EventTypeNormal, "DeletedServiceAccount", "support-group label removed", "Deleted ServiceAccount %s/%s", serviceAccount.Namespace, serviceAccount.Name) + } else if client.IgnoreNotFound(err) != nil { + return err + } + return nil } diff --git a/internal/controller/team/team_controller_test.go b/internal/controller/team/team_controller_test.go index 22a68a8ef..c11e47e65 100644 --- a/internal/controller/team/team_controller_test.go +++ b/internal/controller/team/team_controller_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/gomega" "github.com/onsi/gomega/gbytes" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -273,37 +274,94 @@ var _ = Describe("TeamController", Ordered, func() { }).Should(Succeed(), "ServiceAccount should be created for support-group team") }) - It("should delete the ServiceAccount when support-group label is removed from Team", func() { + It("should create a Role and RoleBinding for a support-group Team", func() { By("creating a support-group team") team := setup.CreateTeam(test.Ctx, supportGroupTeamName, test.WithMappedIDPGroup(validIdpGroupName), test.WithTeamLabel(greenhouseapis.LabelKeySupportGroup, "true"), ) + expectedRBACName := team.Name + "-sa-token-request" expectedSAName := team.Name + "-sa" - By("waiting for the ServiceAccount to be created") Eventually(func(g Gomega) { - sa := &corev1.ServiceAccount{} + role := &rbacv1.Role{} err := setup.Get(test.Ctx, types.NamespacedName{ + Name: expectedRBACName, + Namespace: setup.Namespace(), + }, role) + g.Expect(err).ShouldNot(HaveOccurred(), "Role should have been created") + g.Expect(role.Rules).To(HaveLen(1), "Role should have exactly one rule") + g.Expect(role.Rules[0].APIGroups).To(ConsistOf(""), "rule should target core API group") + g.Expect(role.Rules[0].Resources).To(ConsistOf("serviceaccounts/token"), "rule should target serviceaccounts/token") + g.Expect(role.Rules[0].Verbs).To(ConsistOf("create"), "rule should allow create verb") + g.Expect(role.Rules[0].ResourceNames).To(ConsistOf(expectedSAName), "rule should be scoped to the team SA") + g.Expect(role.OwnerReferences).To(HaveLen(1), "Role should have exactly one owner reference") + g.Expect(role.OwnerReferences[0].Name).To(Equal(team.Name), "Role owner reference should point to the Team") + }).Should(Succeed(), "Role should be created for support-group team") + + Eventually(func(g Gomega) { + rb := &rbacv1.RoleBinding{} + err := setup.Get(test.Ctx, types.NamespacedName{ + Name: expectedRBACName, + Namespace: setup.Namespace(), + }, rb) + g.Expect(err).ShouldNot(HaveOccurred(), "RoleBinding should have been created") + g.Expect(rb.RoleRef.Kind).To(Equal("Role"), "RoleRef should reference a Role") + g.Expect(rb.RoleRef.Name).To(Equal(expectedRBACName), "RoleRef should reference the correct Role") + g.Expect(rb.Subjects).To(HaveLen(2), "RoleBinding should have exactly two subjects") + g.Expect(rb.Subjects).To(ContainElement(rbacv1.Subject{ + APIGroup: rbacv1.GroupName, + Kind: rbacv1.GroupKind, + Name: "support-group:" + team.Name, + }), "RoleBinding should include the support-group group") + g.Expect(rb.Subjects).To(ContainElement(rbacv1.Subject{ + Kind: rbacv1.ServiceAccountKind, Name: expectedSAName, Namespace: setup.Namespace(), - }, sa) - g.Expect(err).ShouldNot(HaveOccurred(), "ServiceAccount should have been created") - }).Should(Succeed(), "ServiceAccount should be created for support-group team") + }), "RoleBinding should include the team ServiceAccount") + g.Expect(rb.OwnerReferences).To(HaveLen(1), "RoleBinding should have exactly one owner reference") + g.Expect(rb.OwnerReferences[0].Name).To(Equal(team.Name), "RoleBinding owner reference should point to the Team") + }).Should(Succeed(), "RoleBinding should be created for support-group team") + }) + + It("should delete the ServiceAccount, Role, and RoleBinding when support-group label is removed from Team", func() { + By("creating a support-group team") + team := setup.CreateTeam(test.Ctx, supportGroupTeamName, + test.WithMappedIDPGroup(validIdpGroupName), + test.WithTeamLabel(greenhouseapis.LabelKeySupportGroup, "true"), + ) + + expectedSAName := team.Name + "-sa" + expectedRBACName := team.Name + "-sa-token-request" + + By("waiting for the ServiceAccount, Role, and RoleBinding to be created") + Eventually(func(g Gomega) { + sa := &corev1.ServiceAccount{} + g.Expect(setup.Get(test.Ctx, types.NamespacedName{Name: expectedSAName, Namespace: setup.Namespace()}, sa)).To(Succeed()) + role := &rbacv1.Role{} + g.Expect(setup.Get(test.Ctx, types.NamespacedName{Name: expectedRBACName, Namespace: setup.Namespace()}, role)).To(Succeed()) + rb := &rbacv1.RoleBinding{} + g.Expect(setup.Get(test.Ctx, types.NamespacedName{Name: expectedRBACName, Namespace: setup.Namespace()}, rb)).To(Succeed()) + }).Should(Succeed(), "SA, Role, and RoleBinding should be created for support-group team") By("removing the support-group label from the Team") test.MustRemoveLabel(test.Ctx, setup.Client, team, greenhouseapis.LabelKeySupportGroup) - By("ensuring the ServiceAccount is deleted") + By("ensuring the ServiceAccount, Role, and RoleBinding are deleted") Eventually(func(g Gomega) { sa := &corev1.ServiceAccount{} - err := setup.Get(test.Ctx, types.NamespacedName{ - Name: expectedSAName, - Namespace: setup.Namespace(), - }, sa) + err := setup.Get(test.Ctx, types.NamespacedName{Name: expectedSAName, Namespace: setup.Namespace()}, sa) g.Expect(apierrors.IsNotFound(err)).To(BeTrue(), "ServiceAccount should have been deleted after label removal") - }).Should(Succeed(), "ServiceAccount should be deleted when support-group label is removed") + + role := &rbacv1.Role{} + err = setup.Get(test.Ctx, types.NamespacedName{Name: expectedRBACName, Namespace: setup.Namespace()}, role) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue(), "Role should have been deleted after label removal") + + rb := &rbacv1.RoleBinding{} + err = setup.Get(test.Ctx, types.NamespacedName{Name: expectedRBACName, Namespace: setup.Namespace()}, rb) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue(), "RoleBinding should have been deleted after label removal") + }).Should(Succeed(), "SA, Role, and RoleBinding should be deleted when support-group label is removed") }) }) From a4e425c53408267ae7048019f31ebcdb74a5c4e1 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Mon, 1 Jun 2026 18:28:50 +0200 Subject: [PATCH 02/12] docs: add section about requesting a token for team sa On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- docs/user-guides/team/authorization.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/user-guides/team/authorization.md b/docs/user-guides/team/authorization.md index dbd93bb61..bb0467fdb 100644 --- a/docs/user-guides/team/authorization.md +++ b/docs/user-guides/team/authorization.md @@ -58,6 +58,16 @@ Use this ServiceAccount for CI/CD pipelines, custom controllers, or scheduled jo kubectl get serviceaccount my-team-sa -n my-organization ``` +### Requesting a token + +Greenhouse creates a Role and RoleBinding that allow both support-group members and the ServiceAccount itself to request tokens. Use `kubectl create token` to obtain a credential for CI/CD use: + +```bash +kubectl create token my-team-sa -n my-organization --duration=2160h +``` + +The actual token lifetime is capped by the API server's `--service-account-max-token-expiration` setting. The token can be used as a `Bearer` token when authenticating against the Greenhouse API server. + > **Note**: The ServiceAccount is created during Team reconciliation, which requires `spec.mappedIdPGroup` to be set and the Organization's SCIM integration to be configured. See [Setting up Team members synchronization](../../organization/creation#setting-up-team-members-synchronization-with-greenhouse) for SCIM configuration details. > **Note**: The `greenhouse.sap/owned-by` label on the ServiceAccount is immutable once set — it cannot be changed or removed. From b35faa1956df9a5045f4614616a1901a6f9e414c Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Mon, 1 Jun 2026 18:43:06 +0200 Subject: [PATCH 03/12] fix: correct invalid rbac apiGroup marker in cluster controller On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- charts/manager/templates/rbac/manager-role.yaml | 11 ----------- internal/controller/cluster/cluster_controller.go | 2 +- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/charts/manager/templates/rbac/manager-role.yaml b/charts/manager/templates/rbac/manager-role.yaml index 934dda81a..fa24dc58e 100644 --- a/charts/manager/templates/rbac/manager-role.yaml +++ b/charts/manager/templates/rbac/manager-role.yaml @@ -186,17 +186,6 @@ rules: - patch - update - watch -- apiGroups: - - rbac - resources: - - clusterrolebindings - verbs: - - create - - get - - list - - patch - - update - - watch - apiGroups: - rbac.authorization.k8s.io resources: diff --git a/internal/controller/cluster/cluster_controller.go b/internal/controller/cluster/cluster_controller.go index 1619a2196..aee86cfab 100644 --- a/internal/controller/cluster/cluster_controller.go +++ b/internal/controller/cluster/cluster_controller.go @@ -46,7 +46,7 @@ type RemoteClusterReconciler struct { //+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=get;list;watch;update;patch;create //+kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch;update;patch;create;delete //+kubebuilder:rbac:groups="events.k8s.io",resources=events,verbs=get;list;watch;update;patch -//+kubebuilder:rbac:groups="rbac",resources=clusterrolebindings,verbs=get;list;watch;update;patch;create +//+kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=clusterrolebindings,verbs=get;list;watch;update;patch;create // SetupWithManager sets up the controller with the Manager. func (r *RemoteClusterReconciler) SetupWithManager(name string, mgr ctrl.Manager) error { From 4975d1087e6024ac4d851d2ec89ed71a3d1295b7 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Tue, 2 Jun 2026 16:19:45 +0200 Subject: [PATCH 04/12] fix: move support group resources deletion before scim check, fix logs for role, rolebinding and sa removal On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- internal/controller/team/team_controller.go | 47 +++++++++++++-------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/internal/controller/team/team_controller.go b/internal/controller/team/team_controller.go index 3fe0b9dc0..0f897a284 100644 --- a/internal/controller/team/team_controller.go +++ b/internal/controller/team/team_controller.go @@ -120,6 +120,13 @@ func (r *TeamController) EnsureCreated(ctx context.Context, object lifecycle.Run initTeamStatus(team) + // Clean up support-group resources immediately if the label is not set, regardless of SCIM readiness. + if team.Labels[greenhouseapis.LabelKeySupportGroup] != "true" { + if err := r.deleteSupportGroupResourcesIfExist(ctx, team); err != nil { + return ctrl.Result{}, lifecycle.Failed, err + } + } + var organization = new(greenhousev1alpha1.Organization) if err := r.Get(ctx, types.NamespacedName{Name: object.GetNamespace()}, organization); err != nil { return ctrl.Result{}, lifecycle.Failed, client.IgnoreNotFound(err) @@ -166,7 +173,7 @@ func (r *TeamController) EnsureCreated(ctx context.Context, object lifecycle.Run updateTeamMembersCountMetric(team, len(users)) - // Reconcile ServiceAccount for support group teams + // Reconcile ServiceAccount, Role, and RoleBinding for support-group teams. if team.Labels[greenhouseapis.LabelKeySupportGroup] == "true" { if err := r.reconcileSupportGroupServiceAccount(ctx, team); err != nil { return ctrl.Result{}, lifecycle.Failed, err @@ -177,11 +184,6 @@ func (r *TeamController) EnsureCreated(ctx context.Context, object lifecycle.Run if err := r.reconcileSupportGroupRoleBinding(ctx, team); err != nil { return ctrl.Result{}, lifecycle.Failed, err } - } else { - // If the support-group label was removed, delete the SA, Role, and RoleBinding if they still exist. - if err := r.deleteSupportGroupResourcesIfExist(ctx, team); err != nil { - return ctrl.Result{}, lifecycle.Failed, err - } } return ctrl.Result{ @@ -378,33 +380,42 @@ func (r *TeamController) deleteSupportGroupResourcesIfExist(ctx context.Context, roleBinding := &rbacv1.RoleBinding{} if err := r.Get(ctx, types.NamespacedName{Name: resourceName, Namespace: team.Namespace}, roleBinding); err == nil { - if err := r.Delete(ctx, roleBinding); client.IgnoreNotFound(err) != nil { - return err + if err := r.Delete(ctx, roleBinding); err != nil { + if client.IgnoreNotFound(err) != nil { + return err + } + } else { + log.FromContext(ctx).Info("deleted support group role binding", "name", roleBinding.Name, "namespace", roleBinding.Namespace) + r.recorder.Eventf(team, roleBinding, corev1.EventTypeNormal, "DeletedRoleBinding", "support-group label removed", "Deleted RoleBinding %s/%s", roleBinding.Namespace, roleBinding.Name) } - log.FromContext(ctx).Info("deleted support group role binding", "name", roleBinding.Name, "namespace", roleBinding.Namespace) - r.recorder.Eventf(team, roleBinding, corev1.EventTypeNormal, "DeletedRoleBinding", "support-group label removed", "Deleted RoleBinding %s/%s", roleBinding.Namespace, roleBinding.Name) } else if client.IgnoreNotFound(err) != nil { return err } role := &rbacv1.Role{} if err := r.Get(ctx, types.NamespacedName{Name: resourceName, Namespace: team.Namespace}, role); err == nil { - if err := r.Delete(ctx, role); client.IgnoreNotFound(err) != nil { - return err + if err := r.Delete(ctx, role); err != nil { + if client.IgnoreNotFound(err) != nil { + return err + } + } else { + log.FromContext(ctx).Info("deleted support group role", "name", role.Name, "namespace", role.Namespace) + r.recorder.Eventf(team, role, corev1.EventTypeNormal, "DeletedRole", "support-group label removed", "Deleted Role %s/%s", role.Namespace, role.Name) } - log.FromContext(ctx).Info("deleted support group role", "name", role.Name, "namespace", role.Namespace) - r.recorder.Eventf(team, role, corev1.EventTypeNormal, "DeletedRole", "support-group label removed", "Deleted Role %s/%s", role.Namespace, role.Name) } else if client.IgnoreNotFound(err) != nil { return err } serviceAccount := &corev1.ServiceAccount{} if err := r.Get(ctx, types.NamespacedName{Name: team.Name + "-sa", Namespace: team.Namespace}, serviceAccount); err == nil { - if err := r.Delete(ctx, serviceAccount); client.IgnoreNotFound(err) != nil { - return err + if err := r.Delete(ctx, serviceAccount); err != nil { + if client.IgnoreNotFound(err) != nil { + return err + } + } else { + log.FromContext(ctx).Info("deleted support group service account", "name", serviceAccount.Name, "namespace", serviceAccount.Namespace) + r.recorder.Eventf(team, serviceAccount, corev1.EventTypeNormal, "DeletedServiceAccount", "support-group label removed", "Deleted ServiceAccount %s/%s", serviceAccount.Namespace, serviceAccount.Name) } - log.FromContext(ctx).Info("deleted support group service account", "name", serviceAccount.Name, "namespace", serviceAccount.Namespace) - r.recorder.Eventf(team, serviceAccount, corev1.EventTypeNormal, "DeletedServiceAccount", "support-group label removed", "Deleted ServiceAccount %s/%s", serviceAccount.Namespace, serviceAccount.Name) } else if client.IgnoreNotFound(err) != nil { return err } From 044e3466110de1b4efca9825c72d8588e163b043 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Tue, 2 Jun 2026 16:42:00 +0200 Subject: [PATCH 05/12] fix function name typo On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- internal/controller/team/team_controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/controller/team/team_controller.go b/internal/controller/team/team_controller.go index 0f897a284..54433eb26 100644 --- a/internal/controller/team/team_controller.go +++ b/internal/controller/team/team_controller.go @@ -122,7 +122,7 @@ func (r *TeamController) EnsureCreated(ctx context.Context, object lifecycle.Run // Clean up support-group resources immediately if the label is not set, regardless of SCIM readiness. if team.Labels[greenhouseapis.LabelKeySupportGroup] != "true" { - if err := r.deleteSupportGroupResourcesIfExist(ctx, team); err != nil { + if err := r.deleteSupportGroupResourcesIfExists(ctx, team); err != nil { return ctrl.Result{}, lifecycle.Failed, err } } @@ -375,7 +375,7 @@ func (r *TeamController) reconcileSupportGroupRoleBinding(ctx context.Context, t return nil } -func (r *TeamController) deleteSupportGroupResourcesIfExist(ctx context.Context, team *greenhousev1alpha1.Team) error { +func (r *TeamController) deleteSupportGroupResourcesIfExists(ctx context.Context, team *greenhousev1alpha1.Team) error { resourceName := team.Name + "-sa-token-request" roleBinding := &rbacv1.RoleBinding{} From b98827e9861c7fafbe051357baa94a6c2bc8fda7 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Wed, 10 Jun 2026 13:22:41 +0200 Subject: [PATCH 06/12] fix(teams): watch owned Role, RoleBinding, ServiceAccount and rename deletion helper On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- internal/controller/team/team_controller.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/controller/team/team_controller.go b/internal/controller/team/team_controller.go index 54433eb26..d6f734607 100644 --- a/internal/controller/team/team_controller.go +++ b/internal/controller/team/team_controller.go @@ -65,6 +65,9 @@ func (r *TeamController) SetupWithManager(name string, mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). Named(name). For(&greenhousev1alpha1.Team{}). + Owns(&rbacv1.Role{}). + Owns(&rbacv1.RoleBinding{}). + Owns(&corev1.ServiceAccount{}). // If an Organization's .Spec was changed, reconcile relevant Teams. Watches(&greenhousev1alpha1.Organization{}, handler.EnqueueRequestsFromMapFunc(r.enqueueAllTeamsForOrganization), builder.WithPredicates( @@ -122,7 +125,7 @@ func (r *TeamController) EnsureCreated(ctx context.Context, object lifecycle.Run // Clean up support-group resources immediately if the label is not set, regardless of SCIM readiness. if team.Labels[greenhouseapis.LabelKeySupportGroup] != "true" { - if err := r.deleteSupportGroupResourcesIfExists(ctx, team); err != nil { + if err := r.ensureSupportGroupResourcesDeleted(ctx, team); err != nil { return ctrl.Result{}, lifecycle.Failed, err } } @@ -375,7 +378,7 @@ func (r *TeamController) reconcileSupportGroupRoleBinding(ctx context.Context, t return nil } -func (r *TeamController) deleteSupportGroupResourcesIfExists(ctx context.Context, team *greenhousev1alpha1.Team) error { +func (r *TeamController) ensureSupportGroupResourcesDeleted(ctx context.Context, team *greenhousev1alpha1.Team) error { resourceName := team.Name + "-sa-token-request" roleBinding := &rbacv1.RoleBinding{} From 65f8887e3e330ca5406cc8dead9ba93ba3358c88 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Wed, 10 Jun 2026 15:04:40 +0200 Subject: [PATCH 07/12] feat(teams): add TokenRequest mutating webhook to cap SA token expiration at 90 days, add tests On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- .../manager/templates/webhook/webhooks.yaml | 19 +++ cmd/greenhouse/webhooks.go | 1 + .../webhook/v1alpha1/tokenrequest_webhook.go | 65 +++++++++ .../v1alpha1/tokenrequest_webhook_test.go | 137 ++++++++++++++++++ .../webhook/v1alpha1/webhook_suite_test.go | 1 + 5 files changed, 223 insertions(+) create mode 100644 internal/webhook/v1alpha1/tokenrequest_webhook.go create mode 100644 internal/webhook/v1alpha1/tokenrequest_webhook_test.go diff --git a/charts/manager/templates/webhook/webhooks.yaml b/charts/manager/templates/webhook/webhooks.yaml index 8014156e0..57ea63091 100644 --- a/charts/manager/templates/webhook/webhooks.yaml +++ b/charts/manager/templates/webhook/webhooks.yaml @@ -213,6 +213,25 @@ webhooks: resources: - teamroles sideEffects: None + - admissionReviewVersions: + - v1 + clientConfig: + service: + name: greenhouse-webhook-service + namespace: greenhouse + path: /mutate-authentication-k8s-io-v1-tokenrequest + failurePolicy: Fail + name: mtokenrequest.kb.io + rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - CREATE + resources: + - serviceaccounts/token + sideEffects: None --- apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration diff --git a/cmd/greenhouse/webhooks.go b/cmd/greenhouse/webhooks.go index b7185c571..9184452fc 100644 --- a/cmd/greenhouse/webhooks.go +++ b/cmd/greenhouse/webhooks.go @@ -22,4 +22,5 @@ var knownWebhooks = map[string]func(mgr ctrl.Manager) error{ "team": webhookv1alpha1.SetupTeamWebhookWithManager, "clusterPluginDefinition": webhookv1alpha1.SetupClusterPluginDefinitionWebhookWithManager, "serviceaccounts": webhookv1alpha1.SetupServiceAccountWebhookWithManager, + "tokenrequest": webhookv1alpha1.SetupTokenRequestWebhookWithManager, } diff --git a/internal/webhook/v1alpha1/tokenrequest_webhook.go b/internal/webhook/v1alpha1/tokenrequest_webhook.go new file mode 100644 index 000000000..ae5167fac --- /dev/null +++ b/internal/webhook/v1alpha1/tokenrequest_webhook.go @@ -0,0 +1,65 @@ +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and Greenhouse contributors +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +import ( + "context" + + authenticationv1 "k8s.io/api/authentication/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + greenhouseapis "github.com/cloudoperators/greenhouse/api" + "github.com/cloudoperators/greenhouse/internal/webhook" +) + +const ( + // maxTokenExpirationSeconds is 90 days in seconds. + maxTokenExpirationSeconds = int64(90 * 24 * 60 * 60) +) + +// SetupTokenRequestWebhookWithManager registers the mutating webhook for TokenRequest. +func SetupTokenRequestWebhookWithManager(mgr ctrl.Manager) error { + return webhook.SetupWebhook(mgr, + &authenticationv1.TokenRequest{}, + webhook.WebhookFuncs[*authenticationv1.TokenRequest]{ + DefaultFunc: defaultTokenRequest, + }, + ) +} + +//+kubebuilder:webhook:path=/mutate-authentication-k8s-io-v1-tokenrequest,mutating=true,failurePolicy=fail,sideEffects=None,groups="",resources=serviceaccounts/token,verbs=create,versions=v1,name=mtokenrequest.kb.io,admissionReviewVersions=v1 + +func defaultTokenRequest(ctx context.Context, c client.Client, tokenRequest *authenticationv1.TokenRequest) error { + // Do not intercept pod-bound token requests (projected volumes, /var/run/secrets). + if tokenRequest.Spec.BoundObjectRef != nil { + return nil + } + + req, err := admission.RequestFromContext(ctx) + if err != nil { + return apierrors.NewInternalError(err) + } + + // Only cap tokens for greenhouse team SAs (identified by the owned-by label). + sa := &corev1.ServiceAccount{} + if err := c.Get(ctx, types.NamespacedName{Name: req.Name, Namespace: req.Namespace}, sa); err != nil { + return apierrors.NewInternalError(err) + } + if sa.Labels[greenhouseapis.LabelKeyOwnedBy] == "" { + return nil + } + + if tokenRequest.Spec.ExpirationSeconds == nil || *tokenRequest.Spec.ExpirationSeconds > maxTokenExpirationSeconds { + ctrl.LoggerFrom(ctx).Info("requested expiration shortened", "from", tokenRequest.Spec.ExpirationSeconds, "to", maxTokenExpirationSeconds) + tokenRequest.Spec.ExpirationSeconds = new(int64) + *tokenRequest.Spec.ExpirationSeconds = maxTokenExpirationSeconds + } + + return nil +} diff --git a/internal/webhook/v1alpha1/tokenrequest_webhook_test.go b/internal/webhook/v1alpha1/tokenrequest_webhook_test.go new file mode 100644 index 000000000..fc1ab2ff7 --- /dev/null +++ b/internal/webhook/v1alpha1/tokenrequest_webhook_test.go @@ -0,0 +1,137 @@ +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and Greenhouse contributors +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + admissionv1 "k8s.io/api/admission/v1" + authenticationv1 "k8s.io/api/authentication/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + greenhouseapis "github.com/cloudoperators/greenhouse/api" + "github.com/cloudoperators/greenhouse/internal/test" +) + +var _ = Describe("TokenRequest Webhook", func() { + ctxWithRequest := func(saName, saNamespace string) admission.Request { + return admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Name: saName, + Namespace: saNamespace, + }, + } + } + + makeTokenRequest := func(expSeconds *int64) *authenticationv1.TokenRequest { + return &authenticationv1.TokenRequest{ + Spec: authenticationv1.TokenRequestSpec{ + ExpirationSeconds: expSeconds, + }, + } + } + + ptr := func(i int64) *int64 { return &i } + + Describe("defaultTokenRequest (unit)", func() { + It("should skip pod-bound token requests", func() { + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "team-a-sa", + Namespace: "test-org", + Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: "team-a"}, + }, + } + fakeClient := fake.NewClientBuilder().WithScheme(test.GreenhouseV1Alpha1Scheme()).WithObjects(sa).Build() + + tr := makeTokenRequest(ptr(99999999)) + tr.Spec.BoundObjectRef = &authenticationv1.BoundObjectReference{Name: "some-pod"} + + ctx := admission.NewContextWithRequest(test.Ctx, ctxWithRequest(sa.Name, sa.Namespace)) + Expect(defaultTokenRequest(ctx, fakeClient, tr)).To(Succeed()) + Expect(*tr.Spec.ExpirationSeconds).To(Equal(int64(99999999)), "pod-bound token should not be modified") + }) + + It("should skip token requests for SAs without the owned-by label", func() { + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: "unmanaged-sa", Namespace: "test-org"}, + } + fakeClient := fake.NewClientBuilder().WithScheme(test.GreenhouseV1Alpha1Scheme()).WithObjects(sa).Build() + + tr := makeTokenRequest(ptr(99999999)) + ctx := admission.NewContextWithRequest(test.Ctx, ctxWithRequest(sa.Name, sa.Namespace)) + Expect(defaultTokenRequest(ctx, fakeClient, tr)).To(Succeed()) + Expect(*tr.Spec.ExpirationSeconds).To(Equal(int64(99999999)), "unmanaged SA token should not be modified") + }) + + It("should cap expiration when it exceeds 90 days", func() { + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "team-a-sa", + Namespace: "test-org", + Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: "team-a"}, + }, + } + fakeClient := fake.NewClientBuilder().WithScheme(test.GreenhouseV1Alpha1Scheme()).WithObjects(sa).Build() + + tr := makeTokenRequest(ptr(99999999)) + ctx := admission.NewContextWithRequest(test.Ctx, ctxWithRequest(sa.Name, sa.Namespace)) + Expect(defaultTokenRequest(ctx, fakeClient, tr)).To(Succeed()) + Expect(*tr.Spec.ExpirationSeconds).To(Equal(maxTokenExpirationSeconds)) + }) + + It("should cap expiration when it is nil", func() { + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "team-a-sa", + Namespace: "test-org", + Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: "team-a"}, + }, + } + fakeClient := fake.NewClientBuilder().WithScheme(test.GreenhouseV1Alpha1Scheme()).WithObjects(sa).Build() + + tr := makeTokenRequest(nil) + ctx := admission.NewContextWithRequest(test.Ctx, ctxWithRequest(sa.Name, sa.Namespace)) + Expect(defaultTokenRequest(ctx, fakeClient, tr)).To(Succeed()) + Expect(tr.Spec.ExpirationSeconds).NotTo(BeNil()) + Expect(*tr.Spec.ExpirationSeconds).To(Equal(maxTokenExpirationSeconds)) + }) + + It("should leave expiration unchanged when it is within 90 days", func() { + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "team-a-sa", + Namespace: "test-org", + Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: "team-a"}, + }, + } + fakeClient := fake.NewClientBuilder().WithScheme(test.GreenhouseV1Alpha1Scheme()).WithObjects(sa).Build() + + oneHour := int64(3600) + tr := makeTokenRequest(&oneHour) + ctx := admission.NewContextWithRequest(test.Ctx, ctxWithRequest(sa.Name, sa.Namespace)) + Expect(defaultTokenRequest(ctx, fakeClient, tr)).To(Succeed()) + Expect(*tr.Spec.ExpirationSeconds).To(Equal(oneHour)) + }) + + It("should leave expiration unchanged when it is exactly 90 days", func() { + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "team-a-sa", + Namespace: "test-org", + Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: "team-a"}, + }, + } + fakeClient := fake.NewClientBuilder().WithScheme(test.GreenhouseV1Alpha1Scheme()).WithObjects(sa).Build() + + tr := makeTokenRequest(ptr(maxTokenExpirationSeconds)) + ctx := admission.NewContextWithRequest(test.Ctx, ctxWithRequest(sa.Name, sa.Namespace)) + Expect(defaultTokenRequest(ctx, fakeClient, tr)).To(Succeed()) + Expect(*tr.Spec.ExpirationSeconds).To(Equal(maxTokenExpirationSeconds)) + }) + }) +}) diff --git a/internal/webhook/v1alpha1/webhook_suite_test.go b/internal/webhook/v1alpha1/webhook_suite_test.go index 6ef62f0ec..4314b1a4e 100644 --- a/internal/webhook/v1alpha1/webhook_suite_test.go +++ b/internal/webhook/v1alpha1/webhook_suite_test.go @@ -29,6 +29,7 @@ var _ = BeforeSuite(func() { test.RegisterWebhook("teamRoleWebhook", SetupTeamRoleWebhookWithManager) test.RegisterWebhook("teamRolebindingV1alpha2Webhook", v1alpha2.SetupTeamRoleBindingWebhookWithManager) test.RegisterWebhook("serviceAccountWebhook", SetupServiceAccountWebhookWithManager) + test.RegisterWebhook("tokenRequestWebhook", SetupTokenRequestWebhookWithManager) test.TestBeforeSuite() }) From 6bd6a3f0a73d48a4d1a7c105335788fa8f501daa Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Wed, 10 Jun 2026 15:07:05 +0200 Subject: [PATCH 08/12] docs(teams): clarify token lifetime is capped at 90 days by Greenhouse On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- docs/user-guides/team/authorization.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/user-guides/team/authorization.md b/docs/user-guides/team/authorization.md index bb0467fdb..66d454732 100644 --- a/docs/user-guides/team/authorization.md +++ b/docs/user-guides/team/authorization.md @@ -63,10 +63,10 @@ kubectl get serviceaccount my-team-sa -n my-organization Greenhouse creates a Role and RoleBinding that allow both support-group members and the ServiceAccount itself to request tokens. Use `kubectl create token` to obtain a credential for CI/CD use: ```bash -kubectl create token my-team-sa -n my-organization --duration=2160h +kubectl create token my-team-sa -n my-organization ``` -The actual token lifetime is capped by the API server's `--service-account-max-token-expiration` setting. The token can be used as a `Bearer` token when authenticating against the Greenhouse API server. +Token lifetimes are capped at 90 days by Greenhouse. The token can be used as a `Bearer` token when authenticating against the Greenhouse API server. > **Note**: The ServiceAccount is created during Team reconciliation, which requires `spec.mappedIdPGroup` to be set and the Organization's SCIM integration to be configured. See [Setting up Team members synchronization](../../organization/creation#setting-up-team-members-synchronization-with-greenhouse) for SCIM configuration details. From b935141213de731280c1f854e499f28bbe552047 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Wed, 10 Jun 2026 15:21:42 +0200 Subject: [PATCH 09/12] fix(teams): preserve original error from SA lookup in TokenRequest webhook On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- internal/webhook/v1alpha1/tokenrequest_webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/webhook/v1alpha1/tokenrequest_webhook.go b/internal/webhook/v1alpha1/tokenrequest_webhook.go index ae5167fac..9a2105bc4 100644 --- a/internal/webhook/v1alpha1/tokenrequest_webhook.go +++ b/internal/webhook/v1alpha1/tokenrequest_webhook.go @@ -49,7 +49,7 @@ func defaultTokenRequest(ctx context.Context, c client.Client, tokenRequest *aut // Only cap tokens for greenhouse team SAs (identified by the owned-by label). sa := &corev1.ServiceAccount{} if err := c.Get(ctx, types.NamespacedName{Name: req.Name, Namespace: req.Namespace}, sa); err != nil { - return apierrors.NewInternalError(err) + return err } if sa.Labels[greenhouseapis.LabelKeyOwnedBy] == "" { return nil From 606da84e86b26857e19c0d69f376e56619cfc8df Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Wed, 10 Jun 2026 16:39:21 +0200 Subject: [PATCH 10/12] fix(teams): exclude system namespaces from TokenRequest webhook via namespaceSelector On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- charts/manager/templates/webhook/webhooks.yaml | 9 +++++++++ hack/helmify | 1 + 2 files changed, 10 insertions(+) diff --git a/charts/manager/templates/webhook/webhooks.yaml b/charts/manager/templates/webhook/webhooks.yaml index 57ea63091..2634a1c24 100644 --- a/charts/manager/templates/webhook/webhooks.yaml +++ b/charts/manager/templates/webhook/webhooks.yaml @@ -232,6 +232,15 @@ webhooks: resources: - serviceaccounts/token sideEffects: None + namespaceSelector: + matchExpressions: + - key: kubernetes.io/metadata.name + operator: NotIn + values: + - kube-system + - kube-public + - kube-node-lease + - greenhouse --- apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration diff --git a/hack/helmify b/hack/helmify index 854c4bbe7..a6f45ed7b 100755 --- a/hack/helmify +++ b/hack/helmify @@ -45,6 +45,7 @@ yq -i ' yq -i '(.webhooks[] | select(.name == "*secret.kb.io") | .namespaceSelector) |= {"matchExpressions": [{"key":"kubernetes.io/metadata.name", "operator": "NotIn", "values":["kube-system"]}]}' "$TEMPLATES_DIR/webhook/webhooks.yaml" yq -i '(.webhooks[] | select(.name == "*serviceaccount.kb.io") | .namespaceSelector) |= {"matchExpressions": [{"key":"kubernetes.io/metadata.name", "operator": "NotIn", "values":["kube-system"]}]}' "$TEMPLATES_DIR/webhook/webhooks.yaml" +yq -i '(.webhooks[] | select(.name == "mtokenrequest.kb.io") | .namespaceSelector) |= {"matchExpressions": [{"key":"kubernetes.io/metadata.name", "operator": "NotIn", "values":["kube-system","kube-public","kube-node-lease","greenhouse"]}]}' "$TEMPLATES_DIR/webhook/webhooks.yaml" yq -i '(.metadata.annotations += {"cert-manager.io/inject-ca-from": "greenhouse/greenhouse-client-cert"})' "$TEMPLATES_DIR/webhook/webhooks.yaml" # Helmify RBAC. From 6c48697a4663e5d60a680bc3100fbba406767633 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Wed, 10 Jun 2026 17:40:39 +0200 Subject: [PATCH 11/12] fix(teams): log actual expiration value instead of pointer address in TokenRequest webhook On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- internal/webhook/v1alpha1/tokenrequest_webhook.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/webhook/v1alpha1/tokenrequest_webhook.go b/internal/webhook/v1alpha1/tokenrequest_webhook.go index 9a2105bc4..8fc3a8d45 100644 --- a/internal/webhook/v1alpha1/tokenrequest_webhook.go +++ b/internal/webhook/v1alpha1/tokenrequest_webhook.go @@ -56,9 +56,13 @@ func defaultTokenRequest(ctx context.Context, c client.Client, tokenRequest *aut } if tokenRequest.Spec.ExpirationSeconds == nil || *tokenRequest.Spec.ExpirationSeconds > maxTokenExpirationSeconds { - ctrl.LoggerFrom(ctx).Info("requested expiration shortened", "from", tokenRequest.Spec.ExpirationSeconds, "to", maxTokenExpirationSeconds) - tokenRequest.Spec.ExpirationSeconds = new(int64) - *tokenRequest.Spec.ExpirationSeconds = maxTokenExpirationSeconds + var from any + if tokenRequest.Spec.ExpirationSeconds != nil { + from = *tokenRequest.Spec.ExpirationSeconds + } + ctrl.LoggerFrom(ctx).Info("requested expiration shortened", "from", from, "to", maxTokenExpirationSeconds) + exp := maxTokenExpirationSeconds + tokenRequest.Spec.ExpirationSeconds = &exp } return nil From 433213df3f90176cd14bcbdcc4f54eadd0176377 Mon Sep 17 00:00:00 2001 From: Zaggy21 Date: Tue, 16 Jun 2026 14:54:44 +0200 Subject: [PATCH 12/12] feat: remove TokenRequest webhook On-behalf-of: @SAP krzysztof.zagorski@sap.com Signed-off-by: Zaggy21 --- .../manager/templates/webhook/webhooks.yaml | 28 ---- cmd/greenhouse/webhooks.go | 1 - docs/user-guides/team/authorization.md | 4 +- hack/helmify | 1 - .../webhook/v1alpha1/tokenrequest_webhook.go | 69 --------- .../v1alpha1/tokenrequest_webhook_test.go | 137 ------------------ .../webhook/v1alpha1/webhook_suite_test.go | 1 - 7 files changed, 2 insertions(+), 239 deletions(-) delete mode 100644 internal/webhook/v1alpha1/tokenrequest_webhook.go delete mode 100644 internal/webhook/v1alpha1/tokenrequest_webhook_test.go diff --git a/charts/manager/templates/webhook/webhooks.yaml b/charts/manager/templates/webhook/webhooks.yaml index 2634a1c24..8014156e0 100644 --- a/charts/manager/templates/webhook/webhooks.yaml +++ b/charts/manager/templates/webhook/webhooks.yaml @@ -213,34 +213,6 @@ webhooks: resources: - teamroles sideEffects: None - - admissionReviewVersions: - - v1 - clientConfig: - service: - name: greenhouse-webhook-service - namespace: greenhouse - path: /mutate-authentication-k8s-io-v1-tokenrequest - failurePolicy: Fail - name: mtokenrequest.kb.io - rules: - - apiGroups: - - "" - apiVersions: - - v1 - operations: - - CREATE - resources: - - serviceaccounts/token - sideEffects: None - namespaceSelector: - matchExpressions: - - key: kubernetes.io/metadata.name - operator: NotIn - values: - - kube-system - - kube-public - - kube-node-lease - - greenhouse --- apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration diff --git a/cmd/greenhouse/webhooks.go b/cmd/greenhouse/webhooks.go index 9184452fc..b7185c571 100644 --- a/cmd/greenhouse/webhooks.go +++ b/cmd/greenhouse/webhooks.go @@ -22,5 +22,4 @@ var knownWebhooks = map[string]func(mgr ctrl.Manager) error{ "team": webhookv1alpha1.SetupTeamWebhookWithManager, "clusterPluginDefinition": webhookv1alpha1.SetupClusterPluginDefinitionWebhookWithManager, "serviceaccounts": webhookv1alpha1.SetupServiceAccountWebhookWithManager, - "tokenrequest": webhookv1alpha1.SetupTokenRequestWebhookWithManager, } diff --git a/docs/user-guides/team/authorization.md b/docs/user-guides/team/authorization.md index 66d454732..bf9611ad2 100644 --- a/docs/user-guides/team/authorization.md +++ b/docs/user-guides/team/authorization.md @@ -63,10 +63,10 @@ kubectl get serviceaccount my-team-sa -n my-organization Greenhouse creates a Role and RoleBinding that allow both support-group members and the ServiceAccount itself to request tokens. Use `kubectl create token` to obtain a credential for CI/CD use: ```bash -kubectl create token my-team-sa -n my-organization +kubectl create token my-team-sa -n my-organization --duration 2160h ``` -Token lifetimes are capped at 90 days by Greenhouse. The token can be used as a `Bearer` token when authenticating against the Greenhouse API server. +The token can be used as a `Bearer` token when authenticating against the Greenhouse API server. > **Note**: The ServiceAccount is created during Team reconciliation, which requires `spec.mappedIdPGroup` to be set and the Organization's SCIM integration to be configured. See [Setting up Team members synchronization](../../organization/creation#setting-up-team-members-synchronization-with-greenhouse) for SCIM configuration details. diff --git a/hack/helmify b/hack/helmify index a6f45ed7b..854c4bbe7 100755 --- a/hack/helmify +++ b/hack/helmify @@ -45,7 +45,6 @@ yq -i ' yq -i '(.webhooks[] | select(.name == "*secret.kb.io") | .namespaceSelector) |= {"matchExpressions": [{"key":"kubernetes.io/metadata.name", "operator": "NotIn", "values":["kube-system"]}]}' "$TEMPLATES_DIR/webhook/webhooks.yaml" yq -i '(.webhooks[] | select(.name == "*serviceaccount.kb.io") | .namespaceSelector) |= {"matchExpressions": [{"key":"kubernetes.io/metadata.name", "operator": "NotIn", "values":["kube-system"]}]}' "$TEMPLATES_DIR/webhook/webhooks.yaml" -yq -i '(.webhooks[] | select(.name == "mtokenrequest.kb.io") | .namespaceSelector) |= {"matchExpressions": [{"key":"kubernetes.io/metadata.name", "operator": "NotIn", "values":["kube-system","kube-public","kube-node-lease","greenhouse"]}]}' "$TEMPLATES_DIR/webhook/webhooks.yaml" yq -i '(.metadata.annotations += {"cert-manager.io/inject-ca-from": "greenhouse/greenhouse-client-cert"})' "$TEMPLATES_DIR/webhook/webhooks.yaml" # Helmify RBAC. diff --git a/internal/webhook/v1alpha1/tokenrequest_webhook.go b/internal/webhook/v1alpha1/tokenrequest_webhook.go deleted file mode 100644 index 8fc3a8d45..000000000 --- a/internal/webhook/v1alpha1/tokenrequest_webhook.go +++ /dev/null @@ -1,69 +0,0 @@ -// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and Greenhouse contributors -// SPDX-License-Identifier: Apache-2.0 - -package v1alpha1 - -import ( - "context" - - authenticationv1 "k8s.io/api/authentication/v1" - corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/types" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - greenhouseapis "github.com/cloudoperators/greenhouse/api" - "github.com/cloudoperators/greenhouse/internal/webhook" -) - -const ( - // maxTokenExpirationSeconds is 90 days in seconds. - maxTokenExpirationSeconds = int64(90 * 24 * 60 * 60) -) - -// SetupTokenRequestWebhookWithManager registers the mutating webhook for TokenRequest. -func SetupTokenRequestWebhookWithManager(mgr ctrl.Manager) error { - return webhook.SetupWebhook(mgr, - &authenticationv1.TokenRequest{}, - webhook.WebhookFuncs[*authenticationv1.TokenRequest]{ - DefaultFunc: defaultTokenRequest, - }, - ) -} - -//+kubebuilder:webhook:path=/mutate-authentication-k8s-io-v1-tokenrequest,mutating=true,failurePolicy=fail,sideEffects=None,groups="",resources=serviceaccounts/token,verbs=create,versions=v1,name=mtokenrequest.kb.io,admissionReviewVersions=v1 - -func defaultTokenRequest(ctx context.Context, c client.Client, tokenRequest *authenticationv1.TokenRequest) error { - // Do not intercept pod-bound token requests (projected volumes, /var/run/secrets). - if tokenRequest.Spec.BoundObjectRef != nil { - return nil - } - - req, err := admission.RequestFromContext(ctx) - if err != nil { - return apierrors.NewInternalError(err) - } - - // Only cap tokens for greenhouse team SAs (identified by the owned-by label). - sa := &corev1.ServiceAccount{} - if err := c.Get(ctx, types.NamespacedName{Name: req.Name, Namespace: req.Namespace}, sa); err != nil { - return err - } - if sa.Labels[greenhouseapis.LabelKeyOwnedBy] == "" { - return nil - } - - if tokenRequest.Spec.ExpirationSeconds == nil || *tokenRequest.Spec.ExpirationSeconds > maxTokenExpirationSeconds { - var from any - if tokenRequest.Spec.ExpirationSeconds != nil { - from = *tokenRequest.Spec.ExpirationSeconds - } - ctrl.LoggerFrom(ctx).Info("requested expiration shortened", "from", from, "to", maxTokenExpirationSeconds) - exp := maxTokenExpirationSeconds - tokenRequest.Spec.ExpirationSeconds = &exp - } - - return nil -} diff --git a/internal/webhook/v1alpha1/tokenrequest_webhook_test.go b/internal/webhook/v1alpha1/tokenrequest_webhook_test.go deleted file mode 100644 index fc1ab2ff7..000000000 --- a/internal/webhook/v1alpha1/tokenrequest_webhook_test.go +++ /dev/null @@ -1,137 +0,0 @@ -// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and Greenhouse contributors -// SPDX-License-Identifier: Apache-2.0 - -package v1alpha1 - -import ( - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - admissionv1 "k8s.io/api/admission/v1" - authenticationv1 "k8s.io/api/authentication/v1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - greenhouseapis "github.com/cloudoperators/greenhouse/api" - "github.com/cloudoperators/greenhouse/internal/test" -) - -var _ = Describe("TokenRequest Webhook", func() { - ctxWithRequest := func(saName, saNamespace string) admission.Request { - return admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Name: saName, - Namespace: saNamespace, - }, - } - } - - makeTokenRequest := func(expSeconds *int64) *authenticationv1.TokenRequest { - return &authenticationv1.TokenRequest{ - Spec: authenticationv1.TokenRequestSpec{ - ExpirationSeconds: expSeconds, - }, - } - } - - ptr := func(i int64) *int64 { return &i } - - Describe("defaultTokenRequest (unit)", func() { - It("should skip pod-bound token requests", func() { - sa := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "team-a-sa", - Namespace: "test-org", - Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: "team-a"}, - }, - } - fakeClient := fake.NewClientBuilder().WithScheme(test.GreenhouseV1Alpha1Scheme()).WithObjects(sa).Build() - - tr := makeTokenRequest(ptr(99999999)) - tr.Spec.BoundObjectRef = &authenticationv1.BoundObjectReference{Name: "some-pod"} - - ctx := admission.NewContextWithRequest(test.Ctx, ctxWithRequest(sa.Name, sa.Namespace)) - Expect(defaultTokenRequest(ctx, fakeClient, tr)).To(Succeed()) - Expect(*tr.Spec.ExpirationSeconds).To(Equal(int64(99999999)), "pod-bound token should not be modified") - }) - - It("should skip token requests for SAs without the owned-by label", func() { - sa := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{Name: "unmanaged-sa", Namespace: "test-org"}, - } - fakeClient := fake.NewClientBuilder().WithScheme(test.GreenhouseV1Alpha1Scheme()).WithObjects(sa).Build() - - tr := makeTokenRequest(ptr(99999999)) - ctx := admission.NewContextWithRequest(test.Ctx, ctxWithRequest(sa.Name, sa.Namespace)) - Expect(defaultTokenRequest(ctx, fakeClient, tr)).To(Succeed()) - Expect(*tr.Spec.ExpirationSeconds).To(Equal(int64(99999999)), "unmanaged SA token should not be modified") - }) - - It("should cap expiration when it exceeds 90 days", func() { - sa := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "team-a-sa", - Namespace: "test-org", - Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: "team-a"}, - }, - } - fakeClient := fake.NewClientBuilder().WithScheme(test.GreenhouseV1Alpha1Scheme()).WithObjects(sa).Build() - - tr := makeTokenRequest(ptr(99999999)) - ctx := admission.NewContextWithRequest(test.Ctx, ctxWithRequest(sa.Name, sa.Namespace)) - Expect(defaultTokenRequest(ctx, fakeClient, tr)).To(Succeed()) - Expect(*tr.Spec.ExpirationSeconds).To(Equal(maxTokenExpirationSeconds)) - }) - - It("should cap expiration when it is nil", func() { - sa := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "team-a-sa", - Namespace: "test-org", - Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: "team-a"}, - }, - } - fakeClient := fake.NewClientBuilder().WithScheme(test.GreenhouseV1Alpha1Scheme()).WithObjects(sa).Build() - - tr := makeTokenRequest(nil) - ctx := admission.NewContextWithRequest(test.Ctx, ctxWithRequest(sa.Name, sa.Namespace)) - Expect(defaultTokenRequest(ctx, fakeClient, tr)).To(Succeed()) - Expect(tr.Spec.ExpirationSeconds).NotTo(BeNil()) - Expect(*tr.Spec.ExpirationSeconds).To(Equal(maxTokenExpirationSeconds)) - }) - - It("should leave expiration unchanged when it is within 90 days", func() { - sa := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "team-a-sa", - Namespace: "test-org", - Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: "team-a"}, - }, - } - fakeClient := fake.NewClientBuilder().WithScheme(test.GreenhouseV1Alpha1Scheme()).WithObjects(sa).Build() - - oneHour := int64(3600) - tr := makeTokenRequest(&oneHour) - ctx := admission.NewContextWithRequest(test.Ctx, ctxWithRequest(sa.Name, sa.Namespace)) - Expect(defaultTokenRequest(ctx, fakeClient, tr)).To(Succeed()) - Expect(*tr.Spec.ExpirationSeconds).To(Equal(oneHour)) - }) - - It("should leave expiration unchanged when it is exactly 90 days", func() { - sa := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "team-a-sa", - Namespace: "test-org", - Labels: map[string]string{greenhouseapis.LabelKeyOwnedBy: "team-a"}, - }, - } - fakeClient := fake.NewClientBuilder().WithScheme(test.GreenhouseV1Alpha1Scheme()).WithObjects(sa).Build() - - tr := makeTokenRequest(ptr(maxTokenExpirationSeconds)) - ctx := admission.NewContextWithRequest(test.Ctx, ctxWithRequest(sa.Name, sa.Namespace)) - Expect(defaultTokenRequest(ctx, fakeClient, tr)).To(Succeed()) - Expect(*tr.Spec.ExpirationSeconds).To(Equal(maxTokenExpirationSeconds)) - }) - }) -}) diff --git a/internal/webhook/v1alpha1/webhook_suite_test.go b/internal/webhook/v1alpha1/webhook_suite_test.go index 4314b1a4e..6ef62f0ec 100644 --- a/internal/webhook/v1alpha1/webhook_suite_test.go +++ b/internal/webhook/v1alpha1/webhook_suite_test.go @@ -29,7 +29,6 @@ var _ = BeforeSuite(func() { test.RegisterWebhook("teamRoleWebhook", SetupTeamRoleWebhookWithManager) test.RegisterWebhook("teamRolebindingV1alpha2Webhook", v1alpha2.SetupTeamRoleBindingWebhookWithManager) test.RegisterWebhook("serviceAccountWebhook", SetupServiceAccountWebhookWithManager) - test.RegisterWebhook("tokenRequestWebhook", SetupTokenRequestWebhookWithManager) test.TestBeforeSuite() })