diff --git a/bindata/assets/deployments/downloads-deployment.yaml b/bindata/assets/deployments/downloads-deployment.yaml index dc3c695a57..f4d96034eb 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/console-sa.yaml b/bindata/assets/serviceaccounts/console-sa.yaml new file mode 100644 index 0000000000..fad948a6fc --- /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/bindata/assets/serviceaccounts/downloads-sa.yaml b/bindata/assets/serviceaccounts/downloads-sa.yaml new file mode 100644 index 0000000000..11052c5e4e --- /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 344932d38a..146ff6234d 100644 --- a/manifests/03-rbac-role-cluster.yaml +++ b/manifests/03-rbac-role-cluster.yaml @@ -148,6 +148,30 @@ rules: - list - watch - delete + - apiGroups: + - "" + resources: + - serviceaccounts + verbs: + - list + - watch + - get + - apiGroups: + - "" + resources: + - serviceaccounts + verbs: + - create + - apiGroups: + - "" + resources: + - serviceaccounts + verbs: + - update + - delete + resourceNames: + - console + - downloads --- kind: ClusterRole apiVersion: rbac.authorization.k8s.io/v1 diff --git a/manifests/06-sa.yaml b/manifests/06-sa.yaml index 34bca4db88..b0091be8eb 100644 --- a/manifests/06-sa.yaml +++ b/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 ---- \ No newline at end of file diff --git a/pkg/api/api.go b/pkg/api/api.go index f85eb77440..5a10033d1c 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -56,14 +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 - 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 ) diff --git a/pkg/console/controllers/downloadsdeployment/controller.go b/pkg/console/controllers/downloadsdeployment/controller.go index c5cfcb5ecf..aacd32a110 100644 --- a/pkg/console/controllers/downloadsdeployment/controller.go +++ b/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, } diff --git a/pkg/console/controllers/serviceaccounts/controller.go b/pkg/console/controllers/serviceaccounts/controller.go new file mode 100644 index 0000000000..801b2703ff --- /dev/null +++ b/pkg/console/controllers/serviceaccounts/controller.go @@ -0,0 +1,169 @@ +package serviceaccounts + +import ( + "context" + "fmt" + "strings" + "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/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" + "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" + + 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 + conditionType 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, +) factory.Controller { + configV1Informers := configInformer.Config().V1() + + ctrl := &ServiceAccountSyncController{ + serviceAccountName: serviceAccountName, + conditionType: fmt.Sprintf("%sServiceAccountSync", controllerName), + // 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(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 { + operatorConfig, err := c.consoleOperatorLister.Get(api.ConfigResourceName) + if err != nil { + return fmt.Errorf("failed to get console operator config %s: %w", api.ConfigResourceName, err) + } + operatorConfigCopy := operatorConfig.DeepCopy() + + switch operatorConfigCopy.Spec.ManagementState { + case operatorv1.Managed: + 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 serviceaccount sync") + return nil + case operatorv1.Removed: + klog.V(4).Infoln("console is in a removed state: removing synced serviceaccount") + statusHandler := status.NewStatusHandler(c.operatorClient) + removeErr := c.removeServiceAccount(ctx) + statusHandler.AddConditions(status.HandleProgressingOrDegraded(c.conditionType, "FailedRemove", removeErr)) + return statusHandler.FlushAndReturn(removeErr) + 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(c.conditionType, "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) 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) + } + + // Fetch the live SA to get its actual ownerRefs (e.g. ClusterVersion/version + // set by CVO from the old manifests/06-sa.yaml, or any previously set + // Console/cluster ref). We need these on the *required* object so that + // MergeOwnerRefs will update them — it only modifies refs that appear in + // required (matched by Name+Kind+APIVersion.Group). + existing, err := c.serviceAccountClient.ServiceAccounts(api.OpenShiftConsoleNamespace).Get(ctx, c.serviceAccountName, metav1.GetOptions{}) + if err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to get service account %s: %w", c.serviceAccountName, err) + } + if err == nil { + // Clear controller:true from all existing ownerRefs to satisfy the + // Kubernetes invariant that only one ownerRef may have controller:true. + // https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/ + falseBool := false + ownerRefs := existing.DeepCopy().GetOwnerReferences() + for i := range ownerRefs { + ownerRefs[i].Controller = &falseBool + } + serviceAccount.SetOwnerReferences(ownerRefs) + } + + _, _, err = resourceapply.ApplyServiceAccount(ctx, + c.serviceAccountClient, + controllerContext.Recorder(), + serviceAccount, + ) + + if err != nil { + return fmt.Errorf("failed to apply service account %s: %w", c.serviceAccountName, err) + } + + return nil +} diff --git a/pkg/console/starter/starter.go b/pkg/console/starter/starter.go index bc2e893d8c..579132269e 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,37 @@ 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 + ) + + downloadsServiceAccountController := serviceaccounts.NewServiceAccountSyncController( + // clients + operatorClient, + configInformers, + // operator + operatorConfigInformers.Operator().V1().Consoles(), + + kubeClient.CoreV1(), // ServiceAccount + kubeInformersNamespaced.Core().V1().ServiceAccounts(), // ServiceAccount + + recorder, + + api.OpenShiftConsoleDownloadsServiceAccountName, + api.DownloadsResourceName, + ) + downloadsDeploymentController := downloadsdeployment.NewDownloadsDeploymentSyncController( // clients operatorClient, @@ -335,6 +367,7 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle kubeClient.AppsV1(), // Deployments kubeInformersNamespaced.Apps().V1().Deployments(), // Deployments + recorder, ) @@ -632,6 +665,8 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle logLevelController, managementStateController, configUpgradeableController, + consoleServiceAccountController, + downloadsServiceAccountController, consoleServiceController, consoleRouteController, downloadsServiceController, diff --git a/pkg/console/subresource/deployment/deployment_test.go b/pkg/console/subresource/deployment/deployment_test.go index 7a97bda9a0..3f63e74f76 100644 --- a/pkg/console/subresource/deployment/deployment_test.go +++ b/pkg/console/subresource/deployment/deployment_test.go @@ -1609,6 +1609,7 @@ func TestDefaultDownloadsDeployment(t *testing.T) { configv1.SingleReplicaTopologyMode) downloadsDeploymentPodSpecSingleReplica := corev1.PodSpec{ + ServiceAccountName: "downloads", NodeSelector: map[string]string{ "kubernetes.io/os": "linux", "node-role.kubernetes.io/master": "",