simplify controller logic + lock down cr

Author: ehearne-redhat

manifests/03-rbac-role-cluster.yaml

@@ -153,12 +153,20 @@ rules:
     resources:
       - serviceaccounts
     verbs:
-      - get
       - list
-      - delete
+      - watch
+      - get
+  - apiGroups:
+      - ""
+    resources:
+      - serviceaccounts
+    verbs:
       - create
       - update
-      - watch
+      - delete
+    resourceNames:
+      - console
+      - downloads
 ---
 kind: ClusterRole
 apiVersion: rbac.authorization.k8s.io/v1

pkg/api/api.go

@@ -56,18 +56,16 @@ 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
-	OpenShiftConsoleDownloadsServiceAccountName     = OpenShiftConsoleDownloadsDeploymentName
-	OpenshiftConsoleServiceAccountName              = OpenShiftConsoleDeploymentName
-	RedirectContainerTargetPort                     = RedirectContainerPort
-	OpenShiftConsoleSASyncControllerSuffix          = "ConsoleServiceAccountSyncController"
-	OpenshiftConsoleDownloadsSASyncControllerPrefix = "DownloadsServiceAccountSyncController"
+	OAuthClientName                             = OpenShiftConsoleName
+	OpenShiftConsoleDeploymentName              = OpenShiftConsoleName
+	OpenShiftConsoleDownloadsDeploymentName     = DownloadsResourceName
+	OpenShiftConsoleDownloadsPDBName            = DownloadsResourceName
+	OpenShiftConsoleDownloadsRouteName          = DownloadsResourceName
+	OpenShiftConsoleNamespace                   = TargetNamespace
+	OpenShiftConsolePDBName                     = OpenShiftConsoleName
+	OpenShiftConsoleRouteName                   = OpenShiftConsoleName
+	OpenShiftConsoleServiceName                 = OpenShiftConsoleName
+	OpenShiftConsoleDownloadsServiceAccountName = OpenShiftConsoleDownloadsDeploymentName
+	OpenShiftConsoleServiceAccountName          = OpenShiftConsoleDeploymentName
+	RedirectContainerTargetPort                 = RedirectContainerPort
 )

pkg/console/controllers/serviceaccounts/controller.go

@@ -3,6 +3,7 @@ package serviceaccounts
 import (
 	"context"
 	"fmt"
+	"strings"
 	"time"
 
 	operatorv1 "github.com/openshift/api/operator/v1"
@@ -11,16 +12,17 @@ import (
 	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/bindata"
 	"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"
+	subresourceutil "github.com/openshift/console-operator/pkg/console/subresource/util"
 	"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/resource/resourceread"
 	"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"
@@ -54,8 +56,6 @@ func NewServiceAccountSyncController(
 	serviceAccountName string,
 	// controllerName,
 	controllerName string,
-	// controllerSuffix
-	controllerSuffix string,
 ) factory.Controller {
 	configV1Informers := configInformer.Config().V1()
 
@@ -81,7 +81,7 @@ func NewServiceAccountSyncController(
 		serviceAccountNameFilter,
 		serviceAccountInformer.Informer(),
 	).ResyncEvery(time.Minute).WithSync(ctrl.Sync).
-		ToController(controllerName, recorder.WithComponentSuffix(controllerSuffix))
+		ToController(fmt.Sprintf("%sServiceAccountController", strings.Title(controllerName)), recorder.WithComponentSuffix(fmt.Sprintf("%s-service-account-controller", controllerName)))
 }
 
 func (c *ServiceAccountSyncController) Sync(ctx context.Context, controllerContext factory.SyncContext) error {
@@ -93,19 +93,19 @@ func (c *ServiceAccountSyncController) Sync(ctx context.Context, controllerConte
 
 	switch operatorConfigCopy.Spec.ManagementState {
 	case operatorv1.Managed:
-		klog.V(4).Infoln("console is in a managed state: syncing service account")
+		klog.V(4).Infoln("console is in a managed state: syncing serviceaccount")
 	case operatorv1.Unmanaged:
-		klog.V(4).Infoln("console is in an unmanaged state: skipping service account sync")
+		klog.V(4).Infoln("console is in an unmanaged state: skipping serviceaccount sync")
 		return nil
 	case operatorv1.Removed:
-		klog.V(4).Infoln("console is in a removed state: removing synced service account")
+		klog.V(4).Infoln("console is in a removed state: removing synced serviceaccount")
 		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)
+	serviceAccountErr := c.SyncServiceAccount(ctx, operatorConfigCopy, controllerContext)
 	statusHandler.AddConditions(status.HandleProgressingOrDegraded("ServiceAccountSync", "FailedApply", serviceAccountErr))
 	if serviceAccountErr != nil {
 		return statusHandler.FlushAndReturn(serviceAccountErr)
@@ -122,25 +122,41 @@ func (c *ServiceAccountSyncController) removeServiceAccount(ctx context.Context)
 	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
+func (c *ServiceAccountSyncController) SyncServiceAccount(ctx context.Context, operatorConfigCopy *operatorv1.Console, controllerContext factory.SyncContext) error {
+	serviceAccount := resourceread.ReadServiceAccountV1OrDie(
+		bindata.MustAsset(fmt.Sprintf("assets/serviceaccounts/%s-sa.yaml", c.serviceAccountName)),
+	)
+	if serviceAccount.Name == "" {
+		return fmt.Errorf("No service account found for name %v .", c.serviceAccountName)
 	}
 
-	// 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
+	ownerRefs := serviceAccount.GetOwnerReferences()
 
-	for oR := range requiredServiceAccount.OwnerReferences {
+	for oR := range ownerRefs {
 		falseBool := false
-		requiredServiceAccount.OwnerReferences[oR].Controller = &falseBool
+		ownerRefs[oR].Controller = &falseBool
 	}
 
-	return resourceapply.ApplyServiceAccount(ctx,
+	// Add owner reference from the Console operator config as the single controller owner.
+	if ownerRef := subresourceutil.OwnerRefFrom(operatorConfigCopy); ownerRef != nil {
+		truthy := true
+		ownerRef.Controller = &truthy
+		ownerRefs = append(ownerRefs, *ownerRef)
+	}
+
+	serviceAccount.SetOwnerReferences(ownerRefs)
+
+	_, _, err := resourceapply.ApplyServiceAccount(ctx,
 		c.serviceAccountClient,
 		controllerContext.Recorder(),
-		requiredServiceAccount,
+		serviceAccount,
 	)
+
+	if err != nil {
+		return err
+	}
+
+	return nil
 }

pkg/console/starter/starter.go

@@ -340,7 +340,6 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
 		recorder,
 		api.OpenshiftConsoleServiceAccountName,
 		api.OpenShiftConsoleName, // controller name
-		api.OpenShiftConsoleSASyncControllerSuffix,
 	)
 
 	downloadsServiceAccountController := serviceaccounts.NewServiceAccountSyncController(
@@ -357,7 +356,6 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
 
 		api.OpenShiftConsoleDownloadsServiceAccountName,
 		api.DownloadsResourceName,
-		api.OpenshiftConsoleDownloadsSASyncControllerPrefix,
 	)
 
 	downloadsDeploymentController := downloadsdeployment.NewDownloadsDeploymentSyncController(