From 0504779983e38faab7cbc6e2766a3207c17d503d Mon Sep 17 00:00:00 2001 From: Evan Hearne Date: Sat, 14 Feb 2026 08:26:41 +0000 Subject: [PATCH 1/5] add controller for downloads service account deletion logic --- .../deployments/downloads-deployment.yaml | 2 +- .../assets/serviceaccounts/downloads-sa.yaml | 11 ++ manifests/03-rbac-role-cluster.yaml | 11 ++ manifests/06-sa.yaml | 12 -- .../downloadsserviceaccount/controller.go | 115 ++++++++++++++++++ pkg/console/starter/starter.go | 13 ++ .../serviceaccount/serviceaccount.go | 18 +++ 7 files changed, 169 insertions(+), 13 deletions(-) create mode 100644 bindata/assets/serviceaccounts/downloads-sa.yaml create mode 100644 pkg/console/controllers/downloadsserviceaccount/controller.go create mode 100644 pkg/console/subresource/serviceaccount/serviceaccount.go diff --git a/bindata/assets/deployments/downloads-deployment.yaml b/bindata/assets/deployments/downloads-deployment.yaml index dc3c695a5..f4d96034e 100644 --- a/bindata/assets/deployments/downloads-deployment.yaml +++ b/bindata/assets/deployments/downloads-deployment.yaml @@ -8,7 +8,6 @@ metadata: component: downloads annotations: {} spec: - serviceAccountName: downloads selector: matchLabels: app: console @@ -25,6 +24,7 @@ spec: target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}' openshift.io/required-scc: restricted-v2 spec: + serviceAccountName: downloads nodeSelector: kubernetes.io/os: linux node-role.kubernetes.io/master: "" diff --git a/bindata/assets/serviceaccounts/downloads-sa.yaml b/bindata/assets/serviceaccounts/downloads-sa.yaml new file mode 100644 index 000000000..11052c5e4 --- /dev/null +++ b/bindata/assets/serviceaccounts/downloads-sa.yaml @@ -0,0 +1,11 @@ +apiVersion: v1 +kind: ServiceAccount +metadata: + name: downloads + namespace: openshift-console + annotations: + include.release.openshift.io/hypershift: "true" + include.release.openshift.io/ibm-cloud-managed: "true" + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" + capability.openshift.io/name: Console diff --git a/manifests/03-rbac-role-cluster.yaml b/manifests/03-rbac-role-cluster.yaml index 344932d38..4d8d44572 100644 --- a/manifests/03-rbac-role-cluster.yaml +++ b/manifests/03-rbac-role-cluster.yaml @@ -148,6 +148,17 @@ rules: - list - watch - delete + - apiGroups: + - "" + resources: + - serviceaccounts + verbs: + - get + - list + - delete + - create + - update + - watch --- kind: ClusterRole apiVersion: rbac.authorization.k8s.io/v1 diff --git a/manifests/06-sa.yaml b/manifests/06-sa.yaml index 34bca4db8..92ba8bc01 100644 --- a/manifests/06-sa.yaml +++ b/manifests/06-sa.yaml @@ -21,16 +21,4 @@ metadata: include.release.openshift.io/self-managed-high-availability: "true" include.release.openshift.io/single-node-developer: "true" capability.openshift.io/name: Console ---- -apiVersion: v1 -kind: ServiceAccount -metadata: - name: downloads - namespace: openshift-console - annotations: - include.release.openshift.io/hypershift: "true" - include.release.openshift.io/ibm-cloud-managed: "true" - include.release.openshift.io/self-managed-high-availability: "true" - include.release.openshift.io/single-node-developer: "true" - capability.openshift.io/name: Console --- \ No newline at end of file diff --git a/pkg/console/controllers/downloadsserviceaccount/controller.go b/pkg/console/controllers/downloadsserviceaccount/controller.go new file mode 100644 index 000000000..4ed918e57 --- /dev/null +++ b/pkg/console/controllers/downloadsserviceaccount/controller.go @@ -0,0 +1,115 @@ +package downloadsserviceaccount + +import ( + "context" + "fmt" + "time" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + coreinformersv1 "k8s.io/client-go/informers/core/v1" + coreclientv1 "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/klog/v2" + + operatorv1 "github.com/openshift/api/operator/v1" + operatorinformerv1 "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" + operatorlistersv1 "github.com/openshift/client-go/operator/listers/operator/v1" + "github.com/openshift/console-operator/pkg/api" + "github.com/openshift/console-operator/pkg/console/controllers/util" + "github.com/openshift/console-operator/pkg/console/status" + serviceaccountsub "github.com/openshift/console-operator/pkg/console/subresource/serviceaccount" + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/resource/resourceapply" + "github.com/openshift/library-go/pkg/operator/v1helpers" +) + +type DownloadsServiceAccountSyncController struct { + operatorClient v1helpers.OperatorClient + // configs + consoleOperatorLister operatorlistersv1.ConsoleLister + // core kube + serviceAccountClient coreclientv1.ServiceAccountsGetter +} + +func NewDownloadsServiceAccountSyncController( + // clients + operatorClient v1helpers.OperatorClient, + // informer + operatorConfigInformer operatorinformerv1.ConsoleInformer, + // core kube + serviceAccountClient coreclientv1.ServiceAccountsGetter, + serviceAccountInformer coreinformersv1.ServiceAccountInformer, + // events + recorder events.Recorder, +) factory.Controller { + ctrl := &DownloadsServiceAccountSyncController{ + // configs + operatorClient: operatorClient, + consoleOperatorLister: operatorConfigInformer.Lister(), + // client + serviceAccountClient: serviceAccountClient, + } + + configNameFilter := util.IncludeNamesFilter(api.ConfigResourceName) + downloadsNameFilter := util.IncludeNamesFilter(api.DownloadsResourceName) + + return factory.New(). + WithFilteredEventsInformers( // configs + configNameFilter, + operatorConfigInformer.Informer(), + ).WithFilteredEventsInformers( // downloads service account + downloadsNameFilter, + serviceAccountInformer.Informer(), + ).ResyncEvery(time.Minute).WithSync(ctrl.Sync). + ToController("ConsoleDownloadsServiceAccountSyncController", recorder.WithComponentSuffix("console-downloads-service-account-controller")) +} + +func (c *DownloadsServiceAccountSyncController) Sync(ctx context.Context, controllerContext factory.SyncContext) error { + operatorConfig, err := c.consoleOperatorLister.Get(api.ConfigResourceName) + if err != nil { + return err + } + operatorConfigCopy := operatorConfig.DeepCopy() + + switch operatorConfigCopy.Spec.ManagementState { + case operatorv1.Managed: + klog.V(4).Infoln("console is in a managed state: syncing downloads service account") + case operatorv1.Unmanaged: + klog.V(4).Infoln("console is in an unmanaged state: skipping downloads service account sync") + return nil + case operatorv1.Removed: + klog.V(4).Infoln("console is in a removed state: removing downloads service account") + return c.removeDownloadsServiceAccount(ctx) + default: + return fmt.Errorf("unknown state: %v", operatorConfigCopy.Spec.ManagementState) + } + statusHandler := status.NewStatusHandler(c.operatorClient) + + _, _, serviceAccountErr := c.SyncDownloadsServiceAccount(ctx, operatorConfigCopy, controllerContext) + statusHandler.AddConditions(status.HandleProgressingOrDegraded("DownloadsServiceAccountSync", "FailedApply", serviceAccountErr)) + if serviceAccountErr != nil { + return statusHandler.FlushAndReturn(serviceAccountErr) + } + + return statusHandler.FlushAndReturn(nil) +} + +func (c *DownloadsServiceAccountSyncController) SyncDownloadsServiceAccount(ctx context.Context, operatorConfigCopy *operatorv1.Console, controllerContext factory.SyncContext) (*corev1.ServiceAccount, bool, error) { + requiredDownloadsServiceAccount := serviceaccountsub.DefaultDownloadsServiceAccount(operatorConfigCopy) + + return resourceapply.ApplyServiceAccount(ctx, + c.serviceAccountClient, + controllerContext.Recorder(), + requiredDownloadsServiceAccount, + ) +} + +func (c *DownloadsServiceAccountSyncController) removeDownloadsServiceAccount(ctx context.Context) error { + err := c.serviceAccountClient.ServiceAccounts(api.OpenShiftConsoleNamespace).Delete(ctx, api.DownloadsResourceName, metav1.DeleteOptions{}) + if apierrors.IsNotFound(err) { + return nil + } + return err +} diff --git a/pkg/console/starter/starter.go b/pkg/console/starter/starter.go index bc2e893d8..133aa712c 100644 --- a/pkg/console/starter/starter.go +++ b/pkg/console/starter/starter.go @@ -30,6 +30,7 @@ import ( "github.com/openshift/console-operator/pkg/console/controllers/clidownloads" "github.com/openshift/console-operator/pkg/console/controllers/clioidcclientstatus" "github.com/openshift/console-operator/pkg/console/controllers/downloadsdeployment" + "github.com/openshift/console-operator/pkg/console/controllers/downloadsserviceaccount" "github.com/openshift/console-operator/pkg/console/controllers/healthcheck" "github.com/openshift/console-operator/pkg/console/controllers/migration" "github.com/openshift/console-operator/pkg/console/controllers/oauthclients" @@ -338,6 +339,17 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle recorder, ) + downloadsServiceAccountController := downloadsserviceaccount.NewDownloadsServiceAccountSyncController( + // clients + operatorClient, + // operator + operatorConfigInformers.Operator().V1().Consoles(), + // service accounts + kubeClient.CoreV1(), + kubeInformersNamespaced.Core().V1().ServiceAccounts(), + recorder, + ) + cliDownloadsController := clidownloads.NewCLIDownloadsSyncController( // top level config configClient.ConfigV1(), @@ -639,6 +651,7 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle consoleOperator, cliDownloadsController, downloadsDeploymentController, + downloadsServiceAccountController, consoleRouteHealthCheckController, consolePDBController, downloadsPDBController, diff --git a/pkg/console/subresource/serviceaccount/serviceaccount.go b/pkg/console/subresource/serviceaccount/serviceaccount.go new file mode 100644 index 000000000..edfe65de1 --- /dev/null +++ b/pkg/console/subresource/serviceaccount/serviceaccount.go @@ -0,0 +1,18 @@ +package serviceaccount + +import ( + corev1 "k8s.io/api/core/v1" + + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/openshift/console-operator/bindata" + "github.com/openshift/console-operator/pkg/console/subresource/util" + "github.com/openshift/library-go/pkg/operator/resource/resourceread" +) + +func DefaultDownloadsServiceAccount(operatorConfig *operatorv1.Console) *corev1.ServiceAccount { + serviceAccount := resourceread.ReadServiceAccountV1OrDie( + bindata.MustAsset("assets/serviceaccounts/downloads-sa.yaml"), + ) + util.AddOwnerRef(serviceAccount, util.OwnerRefFrom(operatorConfig)) + return serviceAccount +} From 4fa2f2e1e4344f59054e0cfc9c998d52436bde0f Mon Sep 17 00:00:00 2001 From: Evan Hearne Date: Tue, 17 Feb 2026 13:03:09 +0000 Subject: [PATCH 2/5] moved service account logic to downloads deployment --- .../downloadsdeployment/controller.go | 48 ++++++-- .../downloadsserviceaccount/controller.go | 115 ------------------ pkg/console/starter/starter.go | 9 -- 3 files changed, 39 insertions(+), 133 deletions(-) delete mode 100644 pkg/console/controllers/downloadsserviceaccount/controller.go diff --git a/pkg/console/controllers/downloadsdeployment/controller.go b/pkg/console/controllers/downloadsdeployment/controller.go index c5cfcb5ec..356f23820 100644 --- a/pkg/console/controllers/downloadsdeployment/controller.go +++ b/pkg/console/controllers/downloadsdeployment/controller.go @@ -2,14 +2,18 @@ package downloadsdeployment import ( "context" + "errors" "fmt" "time" appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" appsinformersv1 "k8s.io/client-go/informers/apps/v1" + coreinformersv1 "k8s.io/client-go/informers/core/v1" appsclientv1 "k8s.io/client-go/kubernetes/typed/apps/v1" + coreclientv1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/klog/v2" configv1 "github.com/openshift/api/config/v1" @@ -28,6 +32,7 @@ import ( "github.com/openshift/console-operator/pkg/console/controllers/util" deploymentsub "github.com/openshift/console-operator/pkg/console/subresource/deployment" + serviceaccountsub "github.com/openshift/console-operator/pkg/console/subresource/serviceaccount" ) type DownloadsDeploymentSyncController struct { @@ -36,7 +41,8 @@ type DownloadsDeploymentSyncController struct { consoleOperatorLister operatorlistersv1.ConsoleLister infrastructureLister configlistersv1.InfrastructureLister // core kube - deploymentClient appsclientv1.DeploymentsGetter + deploymentClient appsclientv1.DeploymentsGetter + serviceAccountClient coreclientv1.ServiceAccountsGetter } func NewDownloadsDeploymentSyncController( @@ -48,6 +54,8 @@ func NewDownloadsDeploymentSyncController( // core kube deploymentClient appsclientv1.DeploymentsGetter, deploymentInformer appsinformersv1.DeploymentInformer, + serviceAccountClient coreclientv1.ServiceAccountsGetter, + serviceAccountInformer coreinformersv1.ServiceAccountInformer, // events recorder events.Recorder, ) factory.Controller { @@ -58,8 +66,9 @@ func NewDownloadsDeploymentSyncController( operatorClient: operatorClient, consoleOperatorLister: operatorConfigInformer.Lister(), infrastructureLister: configInformer.Config().V1().Infrastructures().Lister(), - // client - deploymentClient: deploymentClient, + // clients + deploymentClient: deploymentClient, + serviceAccountClient: serviceAccountClient, } configNameFilter := util.IncludeNamesFilter(api.ConfigResourceName) @@ -73,6 +82,7 @@ func NewDownloadsDeploymentSyncController( ).WithFilteredEventsInformers( // downloads deployment downloadsNameFilter, deploymentInformer.Informer(), + serviceAccountInformer.Informer(), ).ResyncEvery(time.Minute).WithSync(ctrl.Sync). ToController("ConsoleDownloadsDeploymentSyncController", recorder.WithComponentSuffix("console-downloads-deployment-controller")) } @@ -86,13 +96,13 @@ func (c *DownloadsDeploymentSyncController) Sync(ctx context.Context, controller switch operatorConfigCopy.Spec.ManagementState { case operatorv1.Managed: - klog.V(4).Infoln("console is in a managed state: syncing downloads deployment") + klog.V(4).Infoln("console is in a managed state: syncing downloads deployment and service account") case operatorv1.Unmanaged: - klog.V(4).Infoln("console is in an unmanaged state: skipping downloads deployment sync") + klog.V(4).Infoln("console is in an unmanaged state: skipping downloads deployment sync and service account") return nil case operatorv1.Removed: - klog.V(4).Infoln("console is in an removed state: removing synced downloads deployment") - return c.removeDownloadsDeployment(ctx) + klog.V(4).Infoln("console is in an removed state: removing synced downloads deployment and service account") + return c.removeDownloadsDeploymentAndServiceAccount(ctx) default: return fmt.Errorf("unknown state: %v", operatorConfigCopy.Spec.ManagementState) } @@ -104,6 +114,12 @@ func (c *DownloadsDeploymentSyncController) Sync(ctx context.Context, controller return statusHandler.FlushAndReturn(err) } + _, _, serviceAccountErr := c.SyncDownloadsServiceAccount(ctx, operatorConfigCopy, controllerContext) + statusHandler.AddConditions(status.HandleProgressingOrDegraded("DownloadsServiceAccountSync", "FailedApply", serviceAccountErr)) + if serviceAccountErr != nil { + return statusHandler.FlushAndReturn(serviceAccountErr) + } + actualDownloadsDownloadsDeployment, _, downloadsDeploymentErr := c.SyncDownloadsDeployment(ctx, operatorConfigCopy, infrastructureConfig, controllerContext) statusHandler.AddConditions(status.HandleProgressingOrDegraded("DownloadsDeploymentSync", "FailedApply", downloadsDeploymentErr)) if downloadsDeploymentErr != nil { @@ -126,10 +142,24 @@ func (c *DownloadsDeploymentSyncController) SyncDownloadsDeployment(ctx context. ) } -func (c *DownloadsDeploymentSyncController) removeDownloadsDeployment(ctx context.Context) error { +func (c *DownloadsDeploymentSyncController) SyncDownloadsServiceAccount(ctx context.Context, operatorConfigCopy *operatorv1.Console, controllerContext factory.SyncContext) (*corev1.ServiceAccount, bool, error) { + requiredDownloadsServiceAccount := serviceaccountsub.DefaultDownloadsServiceAccount(operatorConfigCopy) + + return resourceapply.ApplyServiceAccount(ctx, + c.serviceAccountClient, + controllerContext.Recorder(), + requiredDownloadsServiceAccount, + ) +} + +func (c *DownloadsDeploymentSyncController) removeDownloadsDeploymentAndServiceAccount(ctx context.Context) error { err := c.deploymentClient.Deployments(api.OpenShiftConsoleNamespace).Delete(ctx, api.OpenShiftConsoleDownloadsDeploymentName, metav1.DeleteOptions{}) if apierrors.IsNotFound(err) { return nil } - return err + err1 := c.serviceAccountClient.ServiceAccounts(api.OpenShiftConsoleNamespace).Delete(ctx, api.DownloadsResourceName, metav1.DeleteOptions{}) + if apierrors.IsNotFound(err) { + return nil + } + return errors.Join(err, err1) } diff --git a/pkg/console/controllers/downloadsserviceaccount/controller.go b/pkg/console/controllers/downloadsserviceaccount/controller.go deleted file mode 100644 index 4ed918e57..000000000 --- a/pkg/console/controllers/downloadsserviceaccount/controller.go +++ /dev/null @@ -1,115 +0,0 @@ -package downloadsserviceaccount - -import ( - "context" - "fmt" - "time" - - corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - coreinformersv1 "k8s.io/client-go/informers/core/v1" - coreclientv1 "k8s.io/client-go/kubernetes/typed/core/v1" - "k8s.io/klog/v2" - - operatorv1 "github.com/openshift/api/operator/v1" - operatorinformerv1 "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" - operatorlistersv1 "github.com/openshift/client-go/operator/listers/operator/v1" - "github.com/openshift/console-operator/pkg/api" - "github.com/openshift/console-operator/pkg/console/controllers/util" - "github.com/openshift/console-operator/pkg/console/status" - serviceaccountsub "github.com/openshift/console-operator/pkg/console/subresource/serviceaccount" - "github.com/openshift/library-go/pkg/controller/factory" - "github.com/openshift/library-go/pkg/operator/events" - "github.com/openshift/library-go/pkg/operator/resource/resourceapply" - "github.com/openshift/library-go/pkg/operator/v1helpers" -) - -type DownloadsServiceAccountSyncController struct { - operatorClient v1helpers.OperatorClient - // configs - consoleOperatorLister operatorlistersv1.ConsoleLister - // core kube - serviceAccountClient coreclientv1.ServiceAccountsGetter -} - -func NewDownloadsServiceAccountSyncController( - // clients - operatorClient v1helpers.OperatorClient, - // informer - operatorConfigInformer operatorinformerv1.ConsoleInformer, - // core kube - serviceAccountClient coreclientv1.ServiceAccountsGetter, - serviceAccountInformer coreinformersv1.ServiceAccountInformer, - // events - recorder events.Recorder, -) factory.Controller { - ctrl := &DownloadsServiceAccountSyncController{ - // configs - operatorClient: operatorClient, - consoleOperatorLister: operatorConfigInformer.Lister(), - // client - serviceAccountClient: serviceAccountClient, - } - - configNameFilter := util.IncludeNamesFilter(api.ConfigResourceName) - downloadsNameFilter := util.IncludeNamesFilter(api.DownloadsResourceName) - - return factory.New(). - WithFilteredEventsInformers( // configs - configNameFilter, - operatorConfigInformer.Informer(), - ).WithFilteredEventsInformers( // downloads service account - downloadsNameFilter, - serviceAccountInformer.Informer(), - ).ResyncEvery(time.Minute).WithSync(ctrl.Sync). - ToController("ConsoleDownloadsServiceAccountSyncController", recorder.WithComponentSuffix("console-downloads-service-account-controller")) -} - -func (c *DownloadsServiceAccountSyncController) Sync(ctx context.Context, controllerContext factory.SyncContext) error { - operatorConfig, err := c.consoleOperatorLister.Get(api.ConfigResourceName) - if err != nil { - return err - } - operatorConfigCopy := operatorConfig.DeepCopy() - - switch operatorConfigCopy.Spec.ManagementState { - case operatorv1.Managed: - klog.V(4).Infoln("console is in a managed state: syncing downloads service account") - case operatorv1.Unmanaged: - klog.V(4).Infoln("console is in an unmanaged state: skipping downloads service account sync") - return nil - case operatorv1.Removed: - klog.V(4).Infoln("console is in a removed state: removing downloads service account") - return c.removeDownloadsServiceAccount(ctx) - default: - return fmt.Errorf("unknown state: %v", operatorConfigCopy.Spec.ManagementState) - } - statusHandler := status.NewStatusHandler(c.operatorClient) - - _, _, serviceAccountErr := c.SyncDownloadsServiceAccount(ctx, operatorConfigCopy, controllerContext) - statusHandler.AddConditions(status.HandleProgressingOrDegraded("DownloadsServiceAccountSync", "FailedApply", serviceAccountErr)) - if serviceAccountErr != nil { - return statusHandler.FlushAndReturn(serviceAccountErr) - } - - return statusHandler.FlushAndReturn(nil) -} - -func (c *DownloadsServiceAccountSyncController) SyncDownloadsServiceAccount(ctx context.Context, operatorConfigCopy *operatorv1.Console, controllerContext factory.SyncContext) (*corev1.ServiceAccount, bool, error) { - requiredDownloadsServiceAccount := serviceaccountsub.DefaultDownloadsServiceAccount(operatorConfigCopy) - - return resourceapply.ApplyServiceAccount(ctx, - c.serviceAccountClient, - controllerContext.Recorder(), - requiredDownloadsServiceAccount, - ) -} - -func (c *DownloadsServiceAccountSyncController) removeDownloadsServiceAccount(ctx context.Context) error { - err := c.serviceAccountClient.ServiceAccounts(api.OpenShiftConsoleNamespace).Delete(ctx, api.DownloadsResourceName, metav1.DeleteOptions{}) - if apierrors.IsNotFound(err) { - return nil - } - return err -} diff --git a/pkg/console/starter/starter.go b/pkg/console/starter/starter.go index 133aa712c..710038efb 100644 --- a/pkg/console/starter/starter.go +++ b/pkg/console/starter/starter.go @@ -30,7 +30,6 @@ import ( "github.com/openshift/console-operator/pkg/console/controllers/clidownloads" "github.com/openshift/console-operator/pkg/console/controllers/clioidcclientstatus" "github.com/openshift/console-operator/pkg/console/controllers/downloadsdeployment" - "github.com/openshift/console-operator/pkg/console/controllers/downloadsserviceaccount" "github.com/openshift/console-operator/pkg/console/controllers/healthcheck" "github.com/openshift/console-operator/pkg/console/controllers/migration" "github.com/openshift/console-operator/pkg/console/controllers/oauthclients" @@ -336,14 +335,7 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle kubeClient.AppsV1(), // Deployments kubeInformersNamespaced.Apps().V1().Deployments(), // Deployments - recorder, - ) - downloadsServiceAccountController := downloadsserviceaccount.NewDownloadsServiceAccountSyncController( - // clients - operatorClient, - // operator - operatorConfigInformers.Operator().V1().Consoles(), // service accounts kubeClient.CoreV1(), kubeInformersNamespaced.Core().V1().ServiceAccounts(), @@ -651,7 +643,6 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle consoleOperator, cliDownloadsController, downloadsDeploymentController, - downloadsServiceAccountController, consoleRouteHealthCheckController, consolePDBController, downloadsPDBController, From 9612471905946f9bc80025cd389c4425a45150f8 Mon Sep 17 00:00:00 2001 From: Evan Hearne Date: Tue, 17 Feb 2026 13:03:36 +0000 Subject: [PATCH 3/5] ensure ownerrefs have one or less controller true --- pkg/console/subresource/util/util.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/console/subresource/util/util.go b/pkg/console/subresource/util/util.go index a69074cb4..d14a56f11 100644 --- a/pkg/console/subresource/util/util.go +++ b/pkg/console/subresource/util/util.go @@ -54,8 +54,17 @@ func SharedMeta() metav1.ObjectMeta { // objects can have more than one ownerRef, potentially func AddOwnerRef(obj metav1.Object, ownerRef *metav1.OwnerReference) { + ownerRefs := obj.GetOwnerReferences() + // if the object has one or more ownerRef objects, then we must + // ensure that their controller attribute is set to false. + // Only one ownerRef.controller == true . + // https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/#owner-references-in-object-specifications + falseBool := false + for ownerRef := range ownerRefs { + ownerRefs[ownerRef].Controller = &falseBool + } if obj != nil && ownerRef != nil { - obj.SetOwnerReferences(append(obj.GetOwnerReferences(), *ownerRef)) + obj.SetOwnerReferences(append(ownerRefs, *ownerRef)) } } From 480f7f21b99632c7caf8b80f7b293b5916a9a179 Mon Sep 17 00:00:00 2001 From: Evan Hearne Date: Wed, 18 Feb 2026 14:01:27 +0000 Subject: [PATCH 4/5] use dedicated sa controller for console and downloads sa management --- .../assets/serviceaccounts/console-sa.yaml | 11 ++ manifests/06-sa.yaml | 13 -- pkg/api/api.go | 24 ++-- .../downloadsdeployment/controller.go | 46 ++---- .../controllers/serviceaccounts/controller.go | 136 ++++++++++++++++++ pkg/console/starter/starter.go | 39 ++++- .../serviceaccount/serviceaccount.go | 22 +++ 7 files changed, 227 insertions(+), 64 deletions(-) create mode 100644 bindata/assets/serviceaccounts/console-sa.yaml create mode 100644 pkg/console/controllers/serviceaccounts/controller.go diff --git a/bindata/assets/serviceaccounts/console-sa.yaml b/bindata/assets/serviceaccounts/console-sa.yaml new file mode 100644 index 000000000..fad948a6f --- /dev/null +++ b/bindata/assets/serviceaccounts/console-sa.yaml @@ -0,0 +1,11 @@ +apiVersion: v1 +kind: ServiceAccount +metadata: + name: console + namespace: openshift-console + annotations: + include.release.openshift.io/hypershift: "true" + include.release.openshift.io/ibm-cloud-managed: "true" + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" + capability.openshift.io/name: Console diff --git a/manifests/06-sa.yaml b/manifests/06-sa.yaml index 92ba8bc01..b0091be8e 100644 --- a/manifests/06-sa.yaml +++ b/manifests/06-sa.yaml @@ -9,16 +9,3 @@ metadata: include.release.openshift.io/self-managed-high-availability: "true" include.release.openshift.io/single-node-developer: "true" capability.openshift.io/name: Console ---- -apiVersion: v1 -kind: ServiceAccount -metadata: - name: console - namespace: openshift-console - annotations: - include.release.openshift.io/hypershift: "true" - include.release.openshift.io/ibm-cloud-managed: "true" - include.release.openshift.io/self-managed-high-availability: "true" - include.release.openshift.io/single-node-developer: "true" - capability.openshift.io/name: Console ---- \ No newline at end of file diff --git a/pkg/api/api.go b/pkg/api/api.go index f85eb7744..adf8dd133 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -56,14 +56,18 @@ const ( DefaultIngressController = "default" IngressControllerNamespace = "openshift-ingress-operator" - OAuthClientName = OpenShiftConsoleName - OpenShiftConsoleDeploymentName = OpenShiftConsoleName - OpenShiftConsoleDownloadsDeploymentName = DownloadsResourceName - OpenShiftConsoleDownloadsPDBName = DownloadsResourceName - OpenShiftConsoleDownloadsRouteName = DownloadsResourceName - OpenShiftConsoleNamespace = TargetNamespace - OpenShiftConsolePDBName = OpenShiftConsoleName - OpenShiftConsoleRouteName = OpenShiftConsoleName - OpenShiftConsoleServiceName = OpenShiftConsoleName - RedirectContainerTargetPort = RedirectContainerPort + OAuthClientName = OpenShiftConsoleName + OpenShiftConsoleDeploymentName = OpenShiftConsoleName + OpenShiftConsoleDownloadsDeploymentName = DownloadsResourceName + OpenShiftConsoleDownloadsPDBName = DownloadsResourceName + OpenShiftConsoleDownloadsRouteName = DownloadsResourceName + OpenShiftConsoleNamespace = TargetNamespace + OpenShiftConsolePDBName = OpenShiftConsoleName + OpenShiftConsoleRouteName = OpenShiftConsoleName + OpenShiftConsoleServiceName = OpenShiftConsoleName + OpenShiftConsoleDownloadsServiceAccountName = OpenShiftConsoleDownloadsDeploymentName + OpenshiftConsoleServiceAccountName = OpenShiftConsoleDeploymentName + RedirectContainerTargetPort = RedirectContainerPort + OpenShiftConsoleSASyncControllerSuffix = "ConsoleServiceAccountSyncController" + OpenshiftConsoleDownloadsSASyncControllerPrefix = "DownloadsServiceAccountSyncController" ) diff --git a/pkg/console/controllers/downloadsdeployment/controller.go b/pkg/console/controllers/downloadsdeployment/controller.go index 356f23820..aacd32a11 100644 --- a/pkg/console/controllers/downloadsdeployment/controller.go +++ b/pkg/console/controllers/downloadsdeployment/controller.go @@ -2,18 +2,14 @@ package downloadsdeployment import ( "context" - "errors" "fmt" "time" appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" appsinformersv1 "k8s.io/client-go/informers/apps/v1" - coreinformersv1 "k8s.io/client-go/informers/core/v1" appsclientv1 "k8s.io/client-go/kubernetes/typed/apps/v1" - coreclientv1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/klog/v2" configv1 "github.com/openshift/api/config/v1" @@ -32,7 +28,6 @@ import ( "github.com/openshift/console-operator/pkg/console/controllers/util" deploymentsub "github.com/openshift/console-operator/pkg/console/subresource/deployment" - serviceaccountsub "github.com/openshift/console-operator/pkg/console/subresource/serviceaccount" ) type DownloadsDeploymentSyncController struct { @@ -41,8 +36,7 @@ type DownloadsDeploymentSyncController struct { consoleOperatorLister operatorlistersv1.ConsoleLister infrastructureLister configlistersv1.InfrastructureLister // core kube - deploymentClient appsclientv1.DeploymentsGetter - serviceAccountClient coreclientv1.ServiceAccountsGetter + deploymentClient appsclientv1.DeploymentsGetter } func NewDownloadsDeploymentSyncController( @@ -54,8 +48,6 @@ func NewDownloadsDeploymentSyncController( // core kube deploymentClient appsclientv1.DeploymentsGetter, deploymentInformer appsinformersv1.DeploymentInformer, - serviceAccountClient coreclientv1.ServiceAccountsGetter, - serviceAccountInformer coreinformersv1.ServiceAccountInformer, // events recorder events.Recorder, ) factory.Controller { @@ -67,8 +59,7 @@ func NewDownloadsDeploymentSyncController( consoleOperatorLister: operatorConfigInformer.Lister(), infrastructureLister: configInformer.Config().V1().Infrastructures().Lister(), // clients - deploymentClient: deploymentClient, - serviceAccountClient: serviceAccountClient, + deploymentClient: deploymentClient, } configNameFilter := util.IncludeNamesFilter(api.ConfigResourceName) @@ -82,7 +73,6 @@ func NewDownloadsDeploymentSyncController( ).WithFilteredEventsInformers( // downloads deployment downloadsNameFilter, deploymentInformer.Informer(), - serviceAccountInformer.Informer(), ).ResyncEvery(time.Minute).WithSync(ctrl.Sync). ToController("ConsoleDownloadsDeploymentSyncController", recorder.WithComponentSuffix("console-downloads-deployment-controller")) } @@ -96,13 +86,13 @@ func (c *DownloadsDeploymentSyncController) Sync(ctx context.Context, controller switch operatorConfigCopy.Spec.ManagementState { case operatorv1.Managed: - klog.V(4).Infoln("console is in a managed state: syncing downloads deployment and service account") + klog.V(4).Infoln("console is in a managed state: syncing downloads deployment") case operatorv1.Unmanaged: - klog.V(4).Infoln("console is in an unmanaged state: skipping downloads deployment sync and service account") + klog.V(4).Infoln("console is in an unmanaged state: skipping downloads deployment sync") return nil case operatorv1.Removed: - klog.V(4).Infoln("console is in an removed state: removing synced downloads deployment and service account") - return c.removeDownloadsDeploymentAndServiceAccount(ctx) + klog.V(4).Infoln("console is in an removed state: removing synced downloads deployment") + return c.removeDownloadsDeployment(ctx) default: return fmt.Errorf("unknown state: %v", operatorConfigCopy.Spec.ManagementState) } @@ -114,12 +104,6 @@ func (c *DownloadsDeploymentSyncController) Sync(ctx context.Context, controller return statusHandler.FlushAndReturn(err) } - _, _, serviceAccountErr := c.SyncDownloadsServiceAccount(ctx, operatorConfigCopy, controllerContext) - statusHandler.AddConditions(status.HandleProgressingOrDegraded("DownloadsServiceAccountSync", "FailedApply", serviceAccountErr)) - if serviceAccountErr != nil { - return statusHandler.FlushAndReturn(serviceAccountErr) - } - actualDownloadsDownloadsDeployment, _, downloadsDeploymentErr := c.SyncDownloadsDeployment(ctx, operatorConfigCopy, infrastructureConfig, controllerContext) statusHandler.AddConditions(status.HandleProgressingOrDegraded("DownloadsDeploymentSync", "FailedApply", downloadsDeploymentErr)) if downloadsDeploymentErr != nil { @@ -142,24 +126,10 @@ func (c *DownloadsDeploymentSyncController) SyncDownloadsDeployment(ctx context. ) } -func (c *DownloadsDeploymentSyncController) SyncDownloadsServiceAccount(ctx context.Context, operatorConfigCopy *operatorv1.Console, controllerContext factory.SyncContext) (*corev1.ServiceAccount, bool, error) { - requiredDownloadsServiceAccount := serviceaccountsub.DefaultDownloadsServiceAccount(operatorConfigCopy) - - return resourceapply.ApplyServiceAccount(ctx, - c.serviceAccountClient, - controllerContext.Recorder(), - requiredDownloadsServiceAccount, - ) -} - -func (c *DownloadsDeploymentSyncController) removeDownloadsDeploymentAndServiceAccount(ctx context.Context) error { +func (c *DownloadsDeploymentSyncController) removeDownloadsDeployment(ctx context.Context) error { err := c.deploymentClient.Deployments(api.OpenShiftConsoleNamespace).Delete(ctx, api.OpenShiftConsoleDownloadsDeploymentName, metav1.DeleteOptions{}) if apierrors.IsNotFound(err) { return nil } - err1 := c.serviceAccountClient.ServiceAccounts(api.OpenShiftConsoleNamespace).Delete(ctx, api.DownloadsResourceName, metav1.DeleteOptions{}) - if apierrors.IsNotFound(err) { - return nil - } - return errors.Join(err, err1) + return err } diff --git a/pkg/console/controllers/serviceaccounts/controller.go b/pkg/console/controllers/serviceaccounts/controller.go new file mode 100644 index 000000000..5cfd66efe --- /dev/null +++ b/pkg/console/controllers/serviceaccounts/controller.go @@ -0,0 +1,136 @@ +package serviceaccounts + +import ( + "context" + "fmt" + "time" + + operatorv1 "github.com/openshift/api/operator/v1" + configinformer "github.com/openshift/client-go/config/informers/externalversions" + configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" + operatorinformerv1 "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" + operatorlistersv1 "github.com/openshift/client-go/operator/listers/operator/v1" + + "github.com/openshift/console-operator/pkg/api" + "github.com/openshift/console-operator/pkg/console/controllers/util" + "github.com/openshift/console-operator/pkg/console/status" + serviceaccountssub "github.com/openshift/console-operator/pkg/console/subresource/serviceaccount" + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/resource/resourceapply" + "github.com/openshift/library-go/pkg/operator/v1helpers" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + coreinformersv1 "k8s.io/client-go/informers/core/v1" + coreclientv1 "k8s.io/client-go/kubernetes/typed/core/v1" + + "k8s.io/klog/v2" +) + +type ServiceAccountSyncController struct { + serviceAccountName string + operatorClient v1helpers.OperatorClient + // configs + consoleOperatorLister operatorlistersv1.ConsoleLister + infrastructureLister configlistersv1.InfrastructureLister + // core kube + serviceAccountClient coreclientv1.ServiceAccountsGetter +} + +func NewServiceAccountSyncController( + // clients + operatorClient v1helpers.OperatorClient, + // informer + configInformer configinformer.SharedInformerFactory, + operatorConfigInformer operatorinformerv1.ConsoleInformer, + // core kube + serviceAccountClient coreclientv1.ServiceAccountsGetter, + serviceAccountInformer coreinformersv1.ServiceAccountInformer, + // events + recorder events.Recorder, + // serviceAccountName + serviceAccountName string, + // controllerName, + controllerName string, + // controllerSuffix + controllerSuffix string, +) factory.Controller { + configV1Informers := configInformer.Config().V1() + + ctrl := &ServiceAccountSyncController{ + serviceAccountName: serviceAccountName, + // configs + operatorClient: operatorClient, + consoleOperatorLister: operatorConfigInformer.Lister(), + infrastructureLister: configInformer.Config().V1().Infrastructures().Lister(), + // clients + serviceAccountClient: serviceAccountClient, + } + + configNameFilter := util.IncludeNamesFilter(api.ConfigResourceName) + serviceAccountNameFilter := util.IncludeNamesFilter(serviceAccountName) + + return factory.New(). + WithFilteredEventsInformers( // infrastructure configs + configNameFilter, + operatorConfigInformer.Informer(), + configV1Informers.Infrastructures().Informer(), + ).WithFilteredEventsInformers( // service account + serviceAccountNameFilter, + serviceAccountInformer.Informer(), + ).ResyncEvery(time.Minute).WithSync(ctrl.Sync). + ToController(controllerName, recorder.WithComponentSuffix(controllerSuffix)) +} + +func (c *ServiceAccountSyncController) Sync(ctx context.Context, controllerContext factory.SyncContext) error { + operatorConfig, err := c.consoleOperatorLister.Get(api.ConfigResourceName) + if err != nil { + return err + } + operatorConfigCopy := operatorConfig.DeepCopy() + + switch operatorConfigCopy.Spec.ManagementState { + case operatorv1.Managed: + klog.V(4).Infoln("console is in a managed state: syncing service account") + case operatorv1.Unmanaged: + klog.V(4).Infoln("console is in an unmanaged state: skipping service account sync") + return nil + case operatorv1.Removed: + klog.V(4).Infoln("console is in a removed state: removing synced service account") + return c.removeServiceAccount(ctx) + default: + return fmt.Errorf("unknown state: %v", operatorConfigCopy.Spec.ManagementState) + } + statusHandler := status.NewStatusHandler(c.operatorClient) + + _, _, serviceAccountErr := c.SyncServiceAccount(ctx, operatorConfigCopy, controllerContext) + statusHandler.AddConditions(status.HandleProgressingOrDegraded("ServiceAccountSync", "FailedApply", serviceAccountErr)) + if serviceAccountErr != nil { + return statusHandler.FlushAndReturn(serviceAccountErr) + } + + return statusHandler.FlushAndReturn(nil) +} + +func (c *ServiceAccountSyncController) removeServiceAccount(ctx context.Context) error { + err := c.serviceAccountClient.ServiceAccounts(api.OpenShiftConsoleNamespace).Delete(ctx, c.serviceAccountName, metav1.DeleteOptions{}) + if apierrors.IsNotFound(err) { + return nil + } + return err +} + +func (c *ServiceAccountSyncController) SyncServiceAccount(ctx context.Context, operatorConfigCopy *operatorv1.Console, controllerContext factory.SyncContext) (*corev1.ServiceAccount, bool, error) { + requiredServiceAccount, err := serviceaccountssub.DefaultServiceAccountFactory(c.serviceAccountName, operatorConfigCopy) + if err != nil { + return nil, false, err + } + + return resourceapply.ApplyServiceAccount(ctx, + c.serviceAccountClient, + controllerContext.Recorder(), + requiredServiceAccount, + ) +} diff --git a/pkg/console/starter/starter.go b/pkg/console/starter/starter.go index 710038efb..07588472a 100644 --- a/pkg/console/starter/starter.go +++ b/pkg/console/starter/starter.go @@ -38,6 +38,7 @@ import ( pdb "github.com/openshift/console-operator/pkg/console/controllers/poddisruptionbudget" "github.com/openshift/console-operator/pkg/console/controllers/route" "github.com/openshift/console-operator/pkg/console/controllers/service" + "github.com/openshift/console-operator/pkg/console/controllers/serviceaccounts" "github.com/openshift/console-operator/pkg/console/controllers/storageversionmigration" upgradenotification "github.com/openshift/console-operator/pkg/console/controllers/upgradenotification" "github.com/openshift/console-operator/pkg/console/controllers/util" @@ -326,6 +327,39 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle recorder, ) + consoleServiceAccountController := serviceaccounts.NewServiceAccountSyncController( + // clients + operatorClient, + configInformers, + // operator + operatorConfigInformers.Operator().V1().Consoles(), + + kubeClient.CoreV1(), // ServiceAccount + kubeInformersNamespaced.Core().V1().ServiceAccounts(), // ServiceAccount + + recorder, + api.OpenshiftConsoleServiceAccountName, + api.OpenShiftConsoleName, // controller name + api.OpenShiftConsoleSASyncControllerSuffix, + ) + + downloadsServiceAccountController := serviceaccounts.NewServiceAccountSyncController( + // clients + operatorClient, + configInformers, + // operator + operatorConfigInformers.Operator().V1().Consoles(), + + kubeClient.CoreV1(), // ServiceAccount + kubeInformersNamespaced.Core().V1().ServiceAccounts(), // ServiceAccount + + recorder, + + api.OpenShiftConsoleDownloadsServiceAccountName, + api.DownloadsResourceName, + api.OpenshiftConsoleDownloadsSASyncControllerPrefix, + ) + downloadsDeploymentController := downloadsdeployment.NewDownloadsDeploymentSyncController( // clients operatorClient, @@ -336,9 +370,6 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle kubeClient.AppsV1(), // Deployments kubeInformersNamespaced.Apps().V1().Deployments(), // Deployments - // service accounts - kubeClient.CoreV1(), - kubeInformersNamespaced.Core().V1().ServiceAccounts(), recorder, ) @@ -636,6 +667,8 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle logLevelController, managementStateController, configUpgradeableController, + consoleServiceAccountController, + downloadsServiceAccountController, consoleServiceController, consoleRouteController, downloadsServiceController, diff --git a/pkg/console/subresource/serviceaccount/serviceaccount.go b/pkg/console/subresource/serviceaccount/serviceaccount.go index edfe65de1..cd3f9040c 100644 --- a/pkg/console/subresource/serviceaccount/serviceaccount.go +++ b/pkg/console/subresource/serviceaccount/serviceaccount.go @@ -1,6 +1,8 @@ package serviceaccount import ( + "fmt" + corev1 "k8s.io/api/core/v1" operatorv1 "github.com/openshift/api/operator/v1" @@ -16,3 +18,23 @@ func DefaultDownloadsServiceAccount(operatorConfig *operatorv1.Console) *corev1. util.AddOwnerRef(serviceAccount, util.OwnerRefFrom(operatorConfig)) return serviceAccount } + +func DefaultConsoleServiceAccount(operatorConfig *operatorv1.Console) *corev1.ServiceAccount { + serviceAccount := resourceread.ReadServiceAccountV1OrDie( + bindata.MustAsset("assets/serviceaccounts/console-sa.yaml"), + ) + util.AddOwnerRef(serviceAccount, util.OwnerRefFrom(operatorConfig)) + return serviceAccount +} + +// helper function to determine service account in controller +func DefaultServiceAccountFactory(serviceAccountName string, operatorConfig *operatorv1.Console) (*corev1.ServiceAccount, error) { + switch serviceAccountName { + case "downloads": + return DefaultDownloadsServiceAccount(operatorConfig), nil + case "console": + return DefaultConsoleServiceAccount(operatorConfig), nil + default: + return nil, fmt.Errorf("No service account found for name %v .", serviceAccountName) + } +} From 860016b0e349fb085d090950c3174309ec461b08 Mon Sep 17 00:00:00 2001 From: Evan Hearne Date: Wed, 18 Feb 2026 14:12:00 +0000 Subject: [PATCH 5/5] ensure ownerref doesn't get overwritten --- pkg/console/subresource/util/util.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/console/subresource/util/util.go b/pkg/console/subresource/util/util.go index d14a56f11..c9e330075 100644 --- a/pkg/console/subresource/util/util.go +++ b/pkg/console/subresource/util/util.go @@ -59,9 +59,9 @@ func AddOwnerRef(obj metav1.Object, ownerRef *metav1.OwnerReference) { // ensure that their controller attribute is set to false. // Only one ownerRef.controller == true . // https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/#owner-references-in-object-specifications - falseBool := false - for ownerRef := range ownerRefs { - ownerRefs[ownerRef].Controller = &falseBool + for oR := range ownerRefs { + falseBool := false + ownerRefs[oR].Controller = &falseBool } if obj != nil && ownerRef != nil { obj.SetOwnerReferences(append(ownerRefs, *ownerRef))