fix(security): use secretKeyRef for S3 credentials

config/deployment-template/manager.yaml

@@ -14,9 +14,15 @@ spec:
         - --log-level=${LOG_LEVEL}
         env:
         - name: PUSHCACHE_AWS_ACCESS_KEY_ID
-          value: ${PUSHCACHE_AWS_ACCESS_KEY_ID}
+          valueFrom:
+            secretKeyRef:
+              name: ${PUSHCACHE_AWS_SECRET_NAME}
+              key: aws-access-key-id
         - name: PUSHCACHE_AWS_SECRET_ACCESS_KEY
-          value: ${PUSHCACHE_AWS_SECRET_ACCESS_KEY}
+          valueFrom:
+            secretKeyRef:
+              name: ${PUSHCACHE_AWS_SECRET_NAME}
+              key: aws-secret-access-key
         - name: PUSHCACHE_AWS_BUCKET_NAME
           value: ${PUSHCACHE_AWS_BUCKET_NAME}
         - name: PUSHCACHE_AWS_REGION

controllers/reconcile.go

@@ -36,8 +36,9 @@ import (
 )
 
 const (
-	RoutePrefixDefault      = "apps"
-	AkamaiSecretNameDefault = "akamai"
+	RoutePrefixDefault             = "apps"
+	AkamaiSecretNameDefault        = "akamai"
+	PushCacheCredentialsSecretName = "pushcache-s3-credentials" //nolint:gosec // G101: not a credential, just the Secret resource name
 )
 
 type FrontendReconciliation struct {
@@ -527,11 +528,15 @@ func (r *FrontendReconciliation) populatePushCacheContainer(j *batchv1.Job) erro
 	}
 
 	bucketName := objectStoreInfo.Name
-	awsUsername := objectStoreInfo.AccessKey
-	awsPassword := objectStoreInfo.SecretKey
 	hostname := objectStoreInfo.Endpoint
 	port := objectStoreInfo.Port
 
+	// Ensure the S3 credentials Secret exists so that the Job can reference
+	// them via valueFrom.secretKeyRef instead of embedding literal values.
+	if err := ensurePushCacheCredentialsSecret(r.Ctx, r.Client, r.Frontend.Namespace, *objectStoreInfo.AccessKey, *objectStoreInfo.SecretKey); err != nil {
+		return err
+	}
+
 	// Determine which image to use for the valpop container
 	// Use ValpopImage if specified, otherwise fall back to frontend image
 	assetsPath := "/assets"
@@ -583,14 +588,17 @@ func (r *FrontendReconciliation) populatePushCacheContainer(j *batchv1.Job) erro
 	}
 	j.Spec.Template.Spec.InitContainers = []v1.Container{initContainer}
 
-	// Construct the pushcache startup command; removing the sleep command will result in the pushcache job being spin up continously, without delay, and uploading the assets to s3
-	command := fmt.Sprintf("valpop populate -r %s -s %s -i %s --valpop-image %s --timeout 172800 --min-asset-records %d --bucket %s --hostname %s --port %s --username %s --password %s", r.Frontend.Name, assetsPath, r.Frontend.Spec.Image, valpopImage, minAssetRecords, *bucketName, *hostname, *port, *awsUsername, *awsPassword)
+	// Construct the pushcache startup command; credentials are injected via env vars
+	// sourced from the pushcache-s3-credentials Secret (not literal values)
+	command := fmt.Sprintf("valpop populate -r %s -s %s -i %s --valpop-image %s --timeout 172800 --min-asset-records %d --bucket %s --hostname %s --port %s --username \"$PUSHCACHE_AWS_ACCESS_KEY_ID\" --password \"$PUSHCACHE_AWS_SECRET_ACCESS_KEY\"", r.Frontend.Name, assetsPath, r.Frontend.Spec.Image, valpopImage, minAssetRecords, *bucketName, *hostname, *port)
 
 	// Modify the object to set the things we care about
 	pushCacheContainer := v1.Container{
 		Name:         "valpop-pushcache",
 		Image:        valpopImage,
 		VolumeMounts: volumeMounts,
+		// Credentials sourced from Secret via env vars, not embedded in the command
+		Env: pushCacheCredentialEnvVars(),
 		// Run the pushcache startup command
 		Command: []string{"/bin/bash", "-c", command},
 		Resources: v1.ResourceRequirements{
@@ -781,6 +789,66 @@ func getObjectStoreConfig(ctx context.Context, c client.Client, namespace string
 	return extractBucketConfigFromSecret(ctx, c, namespace)
 }
 
+// ensurePushCacheCredentialsSecret creates or updates an opaque Secret containing
+// S3 access credentials so that generated Deployments and Jobs can reference them
+// via valueFrom.secretKeyRef instead of embedding literal values.
+func ensurePushCacheCredentialsSecret(ctx context.Context, c client.Client, namespace, accessKey, secretKey string) error {
+	nn := types.NamespacedName{Name: PushCacheCredentialsSecretName, Namespace: namespace}
+
+	secret := &v1.Secret{}
+	secret.SetName(PushCacheCredentialsSecretName)
+	secret.SetNamespace(namespace)
+	secret.Type = v1.SecretTypeOpaque
+	secret.StringData = map[string]string{
+		"aws-access-key-id":     accessKey,
+		"aws-secret-access-key": secretKey,
+	}
+
+	if err := c.Create(ctx, secret); err != nil {
+		if !k8serr.IsAlreadyExists(err) {
+			return err
+		}
+		existing := &v1.Secret{}
+		if err := c.Get(ctx, nn, existing); err != nil {
+			return err
+		}
+		existing.StringData = map[string]string{
+			"aws-access-key-id":     accessKey,
+			"aws-secret-access-key": secretKey,
+		}
+		if err := c.Update(ctx, existing); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// pushCacheCredentialEnvVars returns env vars that reference the S3 credentials
+// Secret via valueFrom.secretKeyRef instead of literal values.
+func pushCacheCredentialEnvVars() []v1.EnvVar {
+	return []v1.EnvVar{
+		{
+			Name: "PUSHCACHE_AWS_ACCESS_KEY_ID",
+			ValueFrom: &v1.EnvVarSource{
+				SecretKeyRef: &v1.SecretKeySelector{
+					LocalObjectReference: v1.LocalObjectReference{Name: PushCacheCredentialsSecretName},
+					Key:                  "aws-access-key-id",
+				},
+			},
+		},
+		{
+			Name: "PUSHCACHE_AWS_SECRET_ACCESS_KEY",
+			ValueFrom: &v1.EnvVarSource{
+				SecretKeyRef: &v1.SecretKeySelector{
+					LocalObjectReference: v1.LocalObjectReference{Name: PushCacheCredentialsSecretName},
+					Key:                  "aws-secret-access-key",
+				},
+			},
+		},
+	}
+}
+
 // Add the env vars if eny are set
 func (r *FrontendReconciliation) populateEnvVars(d *apps.Deployment, frontendEnvironment *crd.FrontendEnvironment) {
 	envVars := []v1.EnvVar{}

controllers/reconcile_reverse_proxy.go

@@ -132,21 +132,72 @@ func (r *ReverseProxyReconciliation) updateReverseProxyDeployment(existing *apps
 	return r.Client.Update(r.Ctx, desired)
 }
 
-// compareEnvVars compares two environment variable slices for equality
+// compareEnvVars compares two environment variable slices for equality.
+// Handles both literal Value and ValueFrom (secretKeyRef) env vars.
 func (r *ReverseProxyReconciliation) compareEnvVars(existing, desired []v1.EnvVar) bool {
 	if len(existing) != len(desired) {
 		return false
 	}
 
-	existingMap := make(map[string]string)
+	existingByName := make(map[string]v1.EnvVar)
 	for _, env := range existing {
-		existingMap[env.Name] = env.Value
+		existingByName[env.Name] = env
 	}
 
-	for _, env := range desired {
-		if value, exists := existingMap[env.Name]; !exists || value != env.Value {
+	for _, d := range desired {
+		e, exists := existingByName[d.Name]
+		if !exists {
 			return false
 		}
+		// Compare literal values
+		if d.Value != e.Value {
+			return false
+		}
+		// Compare ValueFrom references
+		if (d.ValueFrom == nil) != (e.ValueFrom == nil) {
+			return false
+		}
+		if d.ValueFrom != nil && e.ValueFrom != nil {
+			// SecretKeyRef
+			if (d.ValueFrom.SecretKeyRef == nil) != (e.ValueFrom.SecretKeyRef == nil) {
+				return false
+			}
+			if d.ValueFrom.SecretKeyRef != nil && e.ValueFrom.SecretKeyRef != nil {
+				if d.ValueFrom.SecretKeyRef.Name != e.ValueFrom.SecretKeyRef.Name ||
+					d.ValueFrom.SecretKeyRef.Key != e.ValueFrom.SecretKeyRef.Key {
+					return false
+				}
+			}
+			// ConfigMapKeyRef
+			if (d.ValueFrom.ConfigMapKeyRef == nil) != (e.ValueFrom.ConfigMapKeyRef == nil) {
+				return false
+			}
+			if d.ValueFrom.ConfigMapKeyRef != nil && e.ValueFrom.ConfigMapKeyRef != nil {
+				if d.ValueFrom.ConfigMapKeyRef.Name != e.ValueFrom.ConfigMapKeyRef.Name ||
+					d.ValueFrom.ConfigMapKeyRef.Key != e.ValueFrom.ConfigMapKeyRef.Key {
+					return false
+				}
+			}
+			// FieldRef
+			if (d.ValueFrom.FieldRef == nil) != (e.ValueFrom.FieldRef == nil) {
+				return false
+			}
+			if d.ValueFrom.FieldRef != nil && e.ValueFrom.FieldRef != nil {
+				if d.ValueFrom.FieldRef.FieldPath != e.ValueFrom.FieldRef.FieldPath {
+					return false
+				}
+			}
+			// ResourceFieldRef
+			if (d.ValueFrom.ResourceFieldRef == nil) != (e.ValueFrom.ResourceFieldRef == nil) {
+				return false
+			}
+			if d.ValueFrom.ResourceFieldRef != nil && e.ValueFrom.ResourceFieldRef != nil {
+				if d.ValueFrom.ResourceFieldRef.ContainerName != e.ValueFrom.ResourceFieldRef.ContainerName ||
+					d.ValueFrom.ResourceFieldRef.Resource != e.ValueFrom.ResourceFieldRef.Resource {
+					return false
+				}
+			}
+		}
 	}
 
 	return true
@@ -403,19 +454,23 @@ func (r *ReverseProxyReconciliation) createReverseProxyService() error {
 
 // createReverseProxyContainer configures the reverse proxy container
 func (r *ReverseProxyReconciliation) createReverseProxyContainer() (v1.Container, error) {
-	// Get object store configuration from environment variables (same as push cache)
-	objectStoreInfo, err := ExtractBucketConfigFromEnv()
+	// Get object store configuration (env vars first, then Clowder secret)
+	objectStoreInfo, err := getObjectStoreConfig(r.Ctx, r.Client, r.Namespace)
 	if err != nil {
 		return v1.Container{}, err
 	}
 
+	// Ensure the S3 credentials Secret exists so that the Deployment can
+	// reference them via valueFrom.secretKeyRef instead of literal values.
+	if err := ensurePushCacheCredentialsSecret(r.Ctx, r.Client, r.Namespace, *objectStoreInfo.AccessKey, *objectStoreInfo.SecretKey); err != nil {
+		return v1.Container{}, err
+	}
+
 	// Get default values
 	minioPort := *objectStoreInfo.Port
-	minioEndpoint := *objectStoreInfo.Endpoint    // PUSHCACHE_AWS_ENDPOINT
-	bucketPathPrefix := *objectStoreInfo.Name     // PUSHCACHE_AWS_BUCKET_NAME
-	accessKeyID := *objectStoreInfo.AccessKey     // PUSHCACHE_AWS_ACCESS_KEY_ID
-	secretAccessKey := *objectStoreInfo.SecretKey // PUSHCACHE_AWS_SECRET_ACCESS_KEY
-	region := *objectStoreInfo.Region             // PUSHCACHE_AWS_REGION
+	minioEndpoint := *objectStoreInfo.Endpoint // PUSHCACHE_AWS_ENDPOINT
+	bucketPathPrefix := *objectStoreInfo.Name  // PUSHCACHE_AWS_BUCKET_NAME
+	region := *objectStoreInfo.Region          // PUSHCACHE_AWS_REGION
 	var minioUpstreamURL string
 	var protocol string
 	// Construct upstream URL with appropriate scheme based on port
@@ -464,16 +519,11 @@ func (r *ReverseProxyReconciliation) createReverseProxyContainer() (v1.Container
 			Name:  "LOG_LEVEL",
 			Value: logLevel,
 		},
-		{
-			Name:  "PUSHCACHE_AWS_ACCESS_KEY_ID",
-			Value: accessKeyID,
-		},
-		{
-			Name:  "PUSHCACHE_AWS_SECRET_ACCESS_KEY",
-			Value: secretAccessKey,
-		},
 	}
 
+	// S3 credentials are sourced from Secret via secretKeyRef, not literal values
+	envVars = append(envVars, pushCacheCredentialEnvVars()...)
+
 	// Add SSL environment variables if SSL is enabled (similar to main reconciler)
 	if r.FrontendEnvironment.Spec.SSL {
 		envVars = append(envVars, v1.EnvVar{

controllers/reconcile_reverse_proxy_test.go

@@ -239,8 +239,8 @@ func TestUpdateReverseProxyDeployment(t *testing.T) {
 				{Name: "BUCKET_PATH_PREFIX", Value: "frontend"},
 				{Name: "SPA_ENTRYPOINT_PATH", Value: "/index.html"},
 				{Name: "AWS_REGION", Value: "us-east-1"},
-				{Name: "PUSHCACHE_AWS_ACCESS_KEY_ID", Value: "test-access-key"},
-				{Name: "PUSHCACHE_AWS_SECRET_ACCESS_KEY", Value: "test-secret-key"},
+				{Name: "PUSHCACHE_AWS_ACCESS_KEY_ID", ValueFrom: &v1.EnvVarSource{SecretKeyRef: &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: PushCacheCredentialsSecretName}, Key: "aws-access-key-id"}}},
+				{Name: "PUSHCACHE_AWS_SECRET_ACCESS_KEY", ValueFrom: &v1.EnvVarSource{SecretKeyRef: &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: PushCacheCredentialsSecretName}, Key: "aws-secret-access-key"}}},
 				{Name: "LOG_LEVEL", Value: "DEBUG"},
 			},
 			expectUpdate:       false,
@@ -506,6 +506,36 @@ func TestEnvVarsEqual(t *testing.T) {
 			desired:  []v1.EnvVar{},
 			expected: true,
 		},
+		{
+			name: "Equal secretKeyRef env vars",
+			existing: []v1.EnvVar{
+				{Name: "SECRET_VAR", ValueFrom: &v1.EnvVarSource{SecretKeyRef: &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: "my-secret"}, Key: "my-key"}}},
+			},
+			desired: []v1.EnvVar{
+				{Name: "SECRET_VAR", ValueFrom: &v1.EnvVarSource{SecretKeyRef: &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: "my-secret"}, Key: "my-key"}}},
+			},
+			expected: true,
+		},
+		{
+			name: "Different secretKeyRef name",
+			existing: []v1.EnvVar{
+				{Name: "SECRET_VAR", ValueFrom: &v1.EnvVarSource{SecretKeyRef: &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: "old-secret"}, Key: "my-key"}}},
+			},
+			desired: []v1.EnvVar{
+				{Name: "SECRET_VAR", ValueFrom: &v1.EnvVarSource{SecretKeyRef: &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: "new-secret"}, Key: "my-key"}}},
+			},
+			expected: false,
+		},
+		{
+			name: "Value vs ValueFrom mismatch",
+			existing: []v1.EnvVar{
+				{Name: "VAR1", Value: "literal"},
+			},
+			desired: []v1.EnvVar{
+				{Name: "VAR1", ValueFrom: &v1.EnvVarSource{SecretKeyRef: &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: "my-secret"}, Key: "my-key"}}},
+			},
+			expected: false,
+		},
 	}
 
 	for _, tt := range tests {

controllers/reverse_proxy_controller.go

@@ -40,7 +40,7 @@ type ReverseProxyController struct {
 
 //+kubebuilder:rbac:groups=cloud.redhat.com,resources=frontendenvironments,verbs=get;list;watch
 //+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete
-//+kubebuilder:rbac:groups="",resources=services,verbs=get;list;watch;create;update;patch;delete
+//+kubebuilder:rbac:groups="",resources=services;secrets,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups=networking.k8s.io,resources=ingresses,verbs=get;list;watch;create;update;patch;delete
 
 // Reconcile handles the reverse proxy reconciliation for a FrontendEnvironment

deploy.yml

@@ -2001,9 +2001,15 @@ objects:
           - /manager
           env:
           - name: PUSHCACHE_AWS_ACCESS_KEY_ID
-            value: ${PUSHCACHE_AWS_ACCESS_KEY_ID}
+            valueFrom:
+              secretKeyRef:
+                key: aws-access-key-id
+                name: ${PUSHCACHE_AWS_SECRET_NAME}
           - name: PUSHCACHE_AWS_SECRET_ACCESS_KEY
-            value: ${PUSHCACHE_AWS_SECRET_ACCESS_KEY}
+            valueFrom:
+              secretKeyRef:
+                key: aws-secret-access-key
+                name: ${PUSHCACHE_AWS_SECRET_NAME}
           - name: PUSHCACHE_AWS_BUCKET_NAME
             value: ${PUSHCACHE_AWS_BUCKET_NAME}
           - name: PUSHCACHE_AWS_REGION
@@ -2049,10 +2055,8 @@ parameters:
   required: true
 - name: IMAGE
   value: quay.io/cloudservices/frontend-operator
-- name: PUSHCACHE_AWS_ACCESS_KEY_ID
-  value: ''
-- name: PUSHCACHE_AWS_SECRET_ACCESS_KEY
-  value: ''
+- name: PUSHCACHE_AWS_SECRET_NAME
+  value: pushcache-aws-credentials
 - name: PUSHCACHE_AWS_BUCKET_NAME
   value: ''
 - name: PUSHCACHE_AWS_REGION

template.yml

@@ -7,10 +7,8 @@ parameters:
   required: true
 - name: IMAGE
   value: quay.io/cloudservices/frontend-operator
-- name: PUSHCACHE_AWS_ACCESS_KEY_ID
-  value: ""
-- name: PUSHCACHE_AWS_SECRET_ACCESS_KEY
-  value: ""
+- name: PUSHCACHE_AWS_SECRET_NAME
+  value: pushcache-aws-credentials
 - name: PUSHCACHE_AWS_BUCKET_NAME
   value: ""
 - name: PUSHCACHE_AWS_REGION

tests/e2e/pushcache/02-assert.yaml

@@ -109,7 +109,18 @@ spec:
           command:
             - /bin/bash
             - '-c'
-            - 'valpop populate -r chrome-test-defaults -s /assets -i quay.io/cloudservices/insights-chrome-frontend:720317c --valpop-image quay.io/redhat-user-workloads/hcc-platex-services-tenant/valpop:9843de0 --timeout 172800 --min-asset-records 3 --bucket frontend --hostname minio-service.minio-env.svc.cluster.local --port 9000 --username minioadmin --password minioadmin'
+            - 'valpop populate -r chrome-test-defaults -s /assets -i quay.io/cloudservices/insights-chrome-frontend:720317c --valpop-image quay.io/redhat-user-workloads/hcc-platex-services-tenant/valpop:9843de0 --timeout 172800 --min-asset-records 3 --bucket frontend --hostname minio-service.minio-env.svc.cluster.local --port 9000 --username "$PUSHCACHE_AWS_ACCESS_KEY_ID" --password "$PUSHCACHE_AWS_SECRET_ACCESS_KEY"'
+          env:
+            - name: PUSHCACHE_AWS_ACCESS_KEY_ID
+              valueFrom:
+                secretKeyRef:
+                  name: pushcache-s3-credentials
+                  key: aws-access-key-id
+            - name: PUSHCACHE_AWS_SECRET_ACCESS_KEY
+              valueFrom:
+                secretKeyRef:
+                  name: pushcache-s3-credentials
+                  key: aws-secret-access-key
           resources:
             requests:
               cpu: 100m