OADP-8448: Added service accounts to 1.6

internal/controller/cli_download_controller.go

@@ -22,12 +22,13 @@ import (
 )
 
 const (
-	cliServerDeploymentName = "openshift-adp-oadp-cli-server"
-	cliServerServiceName    = "openshift-adp-cli-server"
-	cliServerRouteName      = "oadp-cli-server-route"
-	cliDownloadName         = "openshift-adp-oadp-cli"
-	managedByLabel          = "app.kubernetes.io/managed-by"
-	operatorName            = "oadp-operator"
+	cliServerDeploymentName     = "openshift-adp-oadp-cli-server"
+	cliServerServiceName        = "openshift-adp-cli-server"
+	cliServerServiceAccountName = "openshift-adp-cli-server"
+	cliServerRouteName          = "oadp-cli-server-route"
+	cliDownloadName             = "openshift-adp-oadp-cli"
+	managedByLabel              = "app.kubernetes.io/managed-by"
+	operatorName                = "oadp-operator"
 )
 
 // CLIDownloadSetup is a runnable that sets up CLI download resources when the operator starts
@@ -74,9 +75,33 @@ func (c *CLIDownloadSetup) Start(ctx context.Context) error {
 // reconcileCLIResources creates or updates all CLI-related resources
 // This is idempotent - it will reuse existing resources if they already exist
 func (c *CLIDownloadSetup) reconcileCLIResources(ctx context.Context, operatorDeployment *appsv1.Deployment, cliServerImage string) error {
-	// 1. Create or update the deployment
-	deployment := &appsv1.Deployment{}
+	// 1. Create the dedicated ServiceAccount used by the CLI server pod.
+	// A non-default ServiceAccount is required (rather than relying on the
+	// namespace's "default" SA) to satisfy access-control best practices,
+	// even though this workload needs no Kubernetes API permissions.
+	serviceAccount := &corev1.ServiceAccount{}
 	err := c.Client.Get(ctx, client.ObjectKey{
+		Name:      cliServerServiceAccountName,
+		Namespace: c.Namespace,
+	}, serviceAccount)
+
+	if errors.IsNotFound(err) {
+		serviceAccount = buildCLIServerServiceAccount(c.Namespace)
+		if err := controllerutil.SetOwnerReference(operatorDeployment, serviceAccount, c.Client.Scheme()); err != nil {
+			return fmt.Errorf("failed to set owner reference on service account: %w", err)
+		}
+		err = c.Client.Create(ctx, serviceAccount)
+		if err != nil && !errors.IsAlreadyExists(err) {
+			return fmt.Errorf("failed to create CLI server service account: %w", err)
+		}
+		c.Log.Info("Created CLI server service account")
+	} else if err != nil {
+		return fmt.Errorf("failed to get CLI server service account: %w", err)
+	}
+
+	// 2. Create or update the deployment
+	deployment := &appsv1.Deployment{}
+	err = c.Client.Get(ctx, client.ObjectKey{
 		Name:      cliServerDeploymentName,
 		Namespace: c.Namespace,
 	}, deployment)
@@ -95,19 +120,30 @@ func (c *CLIDownloadSetup) reconcileCLIResources(ctx context.Context, operatorDe
 		c.Log.Info("Created CLI server deployment", "image", cliServerImage)
 	} else if err != nil {
 		return fmt.Errorf("failed to get CLI server deployment: %w", err)
-	} else if len(deployment.Spec.Template.Spec.Containers) > 0 &&
-		deployment.Spec.Template.Spec.Containers[0].ReadinessProbe == nil {
-		// Deployment exists from a version before probes were added; backfill them.
+	} else {
+		// Deployment exists from a version before probes/service account were added; backfill them.
 		desired := buildCLIServerDeployment(c.Namespace, cliServerImage)
-		deployment.Spec.Template.Spec.Containers[0].ReadinessProbe = desired.Spec.Template.Spec.Containers[0].ReadinessProbe
-		deployment.Spec.Template.Spec.Containers[0].LivenessProbe = desired.Spec.Template.Spec.Containers[0].LivenessProbe
-		if err := c.Client.Update(ctx, deployment); err != nil {
-			return fmt.Errorf("failed to update CLI server deployment with probes: %w", err)
+		needsUpdate := false
+		if len(deployment.Spec.Template.Spec.Containers) > 0 &&
+			deployment.Spec.Template.Spec.Containers[0].ReadinessProbe == nil {
+			deployment.Spec.Template.Spec.Containers[0].ReadinessProbe = desired.Spec.Template.Spec.Containers[0].ReadinessProbe
+			deployment.Spec.Template.Spec.Containers[0].LivenessProbe = desired.Spec.Template.Spec.Containers[0].LivenessProbe
+			needsUpdate = true
+		}
+		if deployment.Spec.Template.Spec.ServiceAccountName != cliServerServiceAccountName {
+			deployment.Spec.Template.Spec.ServiceAccountName = desired.Spec.Template.Spec.ServiceAccountName
+			deployment.Spec.Template.Spec.AutomountServiceAccountToken = desired.Spec.Template.Spec.AutomountServiceAccountToken
+			needsUpdate = true
+		}
+		if needsUpdate {
+			if err := c.Client.Update(ctx, deployment); err != nil {
+				return fmt.Errorf("failed to update CLI server deployment: %w", err)
+			}
+			c.Log.Info("Updated CLI server deployment with readiness/liveness probes and/or service account")
 		}
-		c.Log.Info("Updated CLI server deployment with readiness/liveness probes")
 	}
 
-	// 2. Create or update the service
+	// 3. Create or update the service
 	service := &corev1.Service{}
 	err = c.Client.Get(ctx, client.ObjectKey{
 		Name:      cliServerServiceName,
@@ -130,7 +166,7 @@ func (c *CLIDownloadSetup) reconcileCLIResources(ctx context.Context, operatorDe
 		return fmt.Errorf("failed to get CLI server service: %w", err)
 	}
 
-	// 3. Create or get the route
+	// 4. Create or get the route
 	route := &routev1.Route{}
 	err = c.Client.Get(ctx, client.ObjectKey{
 		Name:      cliServerRouteName,
@@ -200,7 +236,7 @@ func (c *CLIDownloadSetup) reconcileCLIResources(ctx context.Context, operatorDe
 		}
 	}
 
-	// 4. Create or update ConsoleCLIDownload (cluster-scoped)
+	// 5. Create or update ConsoleCLIDownload (cluster-scoped)
 	// Note: This is idempotent. If the ConsoleCLIDownload already exists from a previous
 	// installation, we'll reuse it and update the URL to point to our route.
 	// We use a fixed name to prevent duplicates across operator reinstalls.
@@ -278,6 +314,7 @@ func buildCLIServerDeployment(namespace, image string) *appsv1.Deployment {
 	runAsNonRoot := true
 	allowPrivilegeEscalation := false
 	readOnlyRootFilesystem := true
+	automountServiceAccountToken := false
 
 	return &appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
@@ -302,6 +339,8 @@ func buildCLIServerDeployment(namespace, image string) *appsv1.Deployment {
 					},
 				},
 				Spec: corev1.PodSpec{
+					ServiceAccountName:           cliServerServiceAccountName,
+					AutomountServiceAccountToken: &automountServiceAccountToken,
 					SecurityContext: &corev1.PodSecurityContext{
 						RunAsNonRoot: &runAsNonRoot,
 					},
@@ -362,6 +401,21 @@ func buildCLIServerDeployment(namespace, image string) *appsv1.Deployment {
 	}
 }
 
+func buildCLIServerServiceAccount(namespace string) *corev1.ServiceAccount {
+	automountServiceAccountToken := false
+	return &corev1.ServiceAccount{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      cliServerServiceAccountName,
+			Namespace: namespace,
+			Labels: map[string]string{
+				"app":          "oadp-cli",
+				managedByLabel: operatorName,
+			},
+		},
+		AutomountServiceAccountToken: &automountServiceAccountToken,
+	}
+}
+
 func buildCLIServerService(namespace string) *corev1.Service {
 	return &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{

internal/controller/cli_download_controller_test.go

@@ -0,0 +1,34 @@
+package controller
+
+import "testing"
+
+func TestBuildCLIServerDeployment_ServiceAccount(t *testing.T) {
+	deployment := buildCLIServerDeployment("openshift-adp", "test-image")
+	podSpec := deployment.Spec.Template.Spec
+
+	if podSpec.ServiceAccountName != cliServerServiceAccountName {
+		t.Errorf("expected serviceAccountName %q, got %q", cliServerServiceAccountName, podSpec.ServiceAccountName)
+	}
+	if podSpec.AutomountServiceAccountToken == nil || *podSpec.AutomountServiceAccountToken {
+		t.Error("expected AutomountServiceAccountToken to be false")
+	}
+}
+
+func TestBuildCLIServerServiceAccount(t *testing.T) {
+	const testNamespace = "openshift-adp"
+
+	sa := buildCLIServerServiceAccount(testNamespace)
+
+	if sa.Name != cliServerServiceAccountName {
+		t.Errorf("expected name %q, got %q", cliServerServiceAccountName, sa.Name)
+	}
+	if sa.Namespace != testNamespace {
+		t.Errorf("expected namespace %q, got %q", testNamespace, sa.Namespace)
+	}
+	if sa.AutomountServiceAccountToken == nil || *sa.AutomountServiceAccountToken {
+		t.Error("expected AutomountServiceAccountToken to be false")
+	}
+	if sa.Labels[managedByLabel] != operatorName {
+		t.Errorf("expected label %q=%q, got %q", managedByLabel, operatorName, sa.Labels[managedByLabel])
+	}
+}

internal/controller/vmdp_download_controller.go

@@ -22,10 +22,11 @@ import (
 )
 
 const (
-	vmdpServerDeploymentName = "openshift-adp-oadp-vmdp-server"
-	vmdpServerServiceName    = "openshift-adp-vmdp-server"
-	vmdpServerRouteName      = "oadp-vmdp-server-route"
-	vmdpDownloadName         = "openshift-adp-oadp-vmdp"
+	vmdpServerDeploymentName     = "openshift-adp-oadp-vmdp-server"
+	vmdpServerServiceName        = "openshift-adp-vmdp-server"
+	vmdpServerServiceAccountName = "openshift-adp-vmdp-server"
+	vmdpServerRouteName          = "oadp-vmdp-server-route"
+	vmdpDownloadName             = "openshift-adp-oadp-vmdp"
 )
 
 // VMDPDownloadSetup is a runnable that sets up VMDP download resources when the operator starts
@@ -71,9 +72,33 @@ func (v *VMDPDownloadSetup) Start(ctx context.Context) error {
 
 // reconcileVMDPResources creates or updates all VMDP download-related resources
 func (v *VMDPDownloadSetup) reconcileVMDPResources(ctx context.Context, operatorDeployment *appsv1.Deployment, vmdpServerImage string) error {
-	// 1. Create or update the deployment
-	deployment := &appsv1.Deployment{}
+	// 1. Create the dedicated ServiceAccount used by the VMDP server pod.
+	// A non-default ServiceAccount is required (rather than relying on the
+	// namespace's "default" SA) to satisfy access-control best practices,
+	// even though this workload needs no Kubernetes API permissions.
+	serviceAccount := &corev1.ServiceAccount{}
 	err := v.Client.Get(ctx, client.ObjectKey{
+		Name:      vmdpServerServiceAccountName,
+		Namespace: v.Namespace,
+	}, serviceAccount)
+
+	if errors.IsNotFound(err) {
+		serviceAccount = buildVMDPServerServiceAccount(v.Namespace)
+		if err := controllerutil.SetOwnerReference(operatorDeployment, serviceAccount, v.Client.Scheme()); err != nil {
+			return fmt.Errorf("failed to set owner reference on service account: %w", err)
+		}
+		err = v.Client.Create(ctx, serviceAccount)
+		if err != nil && !errors.IsAlreadyExists(err) {
+			return fmt.Errorf("failed to create VMDP server service account: %w", err)
+		}
+		v.Log.Info("Created VMDP server service account")
+	} else if err != nil {
+		return fmt.Errorf("failed to get VMDP server service account: %w", err)
+	}
+
+	// 2. Create or update the deployment
+	deployment := &appsv1.Deployment{}
+	err = v.Client.Get(ctx, client.ObjectKey{
 		Name:      vmdpServerDeploymentName,
 		Namespace: v.Namespace,
 	}, deployment)
@@ -90,19 +115,30 @@ func (v *VMDPDownloadSetup) reconcileVMDPResources(ctx context.Context, operator
 		v.Log.Info("Created VMDP server deployment", "image", vmdpServerImage)
 	} else if err != nil {
 		return fmt.Errorf("failed to get VMDP server deployment: %w", err)
-	} else if len(deployment.Spec.Template.Spec.Containers) > 0 &&
-		deployment.Spec.Template.Spec.Containers[0].ReadinessProbe == nil {
-		// Deployment exists from a version before probes were added; backfill them.
+	} else {
+		// Deployment exists from a version before probes/service account were added; backfill them.
 		desired := buildVMDPServerDeployment(v.Namespace, vmdpServerImage)
-		deployment.Spec.Template.Spec.Containers[0].ReadinessProbe = desired.Spec.Template.Spec.Containers[0].ReadinessProbe
-		deployment.Spec.Template.Spec.Containers[0].LivenessProbe = desired.Spec.Template.Spec.Containers[0].LivenessProbe
-		if err := v.Client.Update(ctx, deployment); err != nil {
-			return fmt.Errorf("failed to update VMDP server deployment with probes: %w", err)
+		needsUpdate := false
+		if len(deployment.Spec.Template.Spec.Containers) > 0 &&
+			deployment.Spec.Template.Spec.Containers[0].ReadinessProbe == nil {
+			deployment.Spec.Template.Spec.Containers[0].ReadinessProbe = desired.Spec.Template.Spec.Containers[0].ReadinessProbe
+			deployment.Spec.Template.Spec.Containers[0].LivenessProbe = desired.Spec.Template.Spec.Containers[0].LivenessProbe
+			needsUpdate = true
+		}
+		if deployment.Spec.Template.Spec.ServiceAccountName != vmdpServerServiceAccountName {
+			deployment.Spec.Template.Spec.ServiceAccountName = desired.Spec.Template.Spec.ServiceAccountName
+			deployment.Spec.Template.Spec.AutomountServiceAccountToken = desired.Spec.Template.Spec.AutomountServiceAccountToken
+			needsUpdate = true
+		}
+		if needsUpdate {
+			if err := v.Client.Update(ctx, deployment); err != nil {
+				return fmt.Errorf("failed to update VMDP server deployment: %w", err)
+			}
+			v.Log.Info("Updated VMDP server deployment with readiness/liveness probes and/or service account")
 		}
-		v.Log.Info("Updated VMDP server deployment with readiness/liveness probes")
 	}
 
-	// 2. Create or update the service
+	// 3. Create or update the service
 	service := &corev1.Service{}
 	err = v.Client.Get(ctx, client.ObjectKey{
 		Name:      vmdpServerServiceName,
@@ -123,7 +159,7 @@ func (v *VMDPDownloadSetup) reconcileVMDPResources(ctx context.Context, operator
 		return fmt.Errorf("failed to get VMDP server service: %w", err)
 	}
 
-	// 3. Create or get the route
+	// 4. Create or get the route
 	route := &routev1.Route{}
 	err = v.Client.Get(ctx, client.ObjectKey{
 		Name:      vmdpServerRouteName,
@@ -188,7 +224,7 @@ func (v *VMDPDownloadSetup) reconcileVMDPResources(ctx context.Context, operator
 		}
 	}
 
-	// 4. Create or update ConsoleCLIDownload (cluster-scoped)
+	// 5. Create or update ConsoleCLIDownload (cluster-scoped)
 	downloadURL := fmt.Sprintf("https://%s/", hostname)
 
 	consoleCLIDownload := &consolev1.ConsoleCLIDownload{}
@@ -260,6 +296,7 @@ func buildVMDPServerDeployment(namespace, image string) *appsv1.Deployment {
 	runAsNonRoot := true
 	allowPrivilegeEscalation := false
 	readOnlyRootFilesystem := true
+	automountServiceAccountToken := false
 
 	return &appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
@@ -284,6 +321,8 @@ func buildVMDPServerDeployment(namespace, image string) *appsv1.Deployment {
 					},
 				},
 				Spec: corev1.PodSpec{
+					ServiceAccountName:           vmdpServerServiceAccountName,
+					AutomountServiceAccountToken: &automountServiceAccountToken,
 					SecurityContext: &corev1.PodSecurityContext{
 						RunAsNonRoot: &runAsNonRoot,
 					},
@@ -344,6 +383,21 @@ func buildVMDPServerDeployment(namespace, image string) *appsv1.Deployment {
 	}
 }
 
+func buildVMDPServerServiceAccount(namespace string) *corev1.ServiceAccount {
+	automountServiceAccountToken := false
+	return &corev1.ServiceAccount{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      vmdpServerServiceAccountName,
+			Namespace: namespace,
+			Labels: map[string]string{
+				"app":          "oadp-vmdp",
+				managedByLabel: operatorName,
+			},
+		},
+		AutomountServiceAccountToken: &automountServiceAccountToken,
+	}
+}
+
 func buildVMDPServerService(namespace string) *corev1.Service {
 	return &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{

internal/controller/vmdp_download_controller_test.go

@@ -0,0 +1,34 @@
+package controller
+
+import "testing"
+
+func TestBuildVMDPServerDeployment_ServiceAccount(t *testing.T) {
+	deployment := buildVMDPServerDeployment("openshift-adp", "test-image")
+	podSpec := deployment.Spec.Template.Spec
+
+	if podSpec.ServiceAccountName != vmdpServerServiceAccountName {
+		t.Errorf("expected serviceAccountName %q, got %q", vmdpServerServiceAccountName, podSpec.ServiceAccountName)
+	}
+	if podSpec.AutomountServiceAccountToken == nil || *podSpec.AutomountServiceAccountToken {
+		t.Error("expected AutomountServiceAccountToken to be false")
+	}
+}
+
+func TestBuildVMDPServerServiceAccount(t *testing.T) {
+	const testNamespace = "openshift-adp"
+
+	sa := buildVMDPServerServiceAccount(testNamespace)
+
+	if sa.Name != vmdpServerServiceAccountName {
+		t.Errorf("expected name %q, got %q", vmdpServerServiceAccountName, sa.Name)
+	}
+	if sa.Namespace != testNamespace {
+		t.Errorf("expected namespace %q, got %q", testNamespace, sa.Namespace)
+	}
+	if sa.AutomountServiceAccountToken == nil || *sa.AutomountServiceAccountToken {
+		t.Error("expected AutomountServiceAccountToken to be false")
+	}
+	if sa.Labels[managedByLabel] != operatorName {
+		t.Errorf("expected label %q=%q, got %q", managedByLabel, operatorName, sa.Labels[managedByLabel])
+	}
+}