OCPBUGS-65622: add controller for downloads service account deletion logic

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: ""

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

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

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

manifests/06-sa.yaml

@@ -9,28 +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
----
-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
----

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"
 )

pkg/console/controllers/downloadsdeployment/controller.go

@@ -58,7 +58,7 @@ func NewDownloadsDeploymentSyncController(
 		operatorClient:        operatorClient,
 		consoleOperatorLister: operatorConfigInformer.Lister(),
 		infrastructureLister:  configInformer.Config().V1().Infrastructures().Lister(),
-		// client
+		// clients
 		deploymentClient: deploymentClient,
 	}
 

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,
+	)
+}

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,
@@ -335,6 +369,7 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
 
 		kubeClient.AppsV1(), // Deployments
 		kubeInformersNamespaced.Apps().V1().Deployments(), // Deployments
+
 		recorder,
 	)
 
@@ -632,6 +667,8 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
 		logLevelController,
 		managementStateController,
 		configUpgradeableController,
+		consoleServiceAccountController,
+		downloadsServiceAccountController,
 		consoleServiceController,
 		consoleRouteController,
 		downloadsServiceController,

pkg/console/subresource/serviceaccount/serviceaccount.go

@@ -0,0 +1,40 @@
+package serviceaccount
+
+import (
+	"fmt"
+
+	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
+}
+
+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)
+	}
+}

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
+	for oR := range ownerRefs {
+		falseBool := false
+		ownerRefs[oR].Controller = &falseBool
+	}
 	if obj != nil && ownerRef != nil {
-		obj.SetOwnerReferences(append(obj.GetOwnerReferences(), *ownerRef))
+		obj.SetOwnerReferences(append(ownerRefs, *ownerRef))
 	}
 }