SPLAT-2899: support per-component credential overrides in openshift-config

pkg/operator/constants/constants.go

@@ -123,6 +123,22 @@ const (
 	// for vSphere are stored.
 	VSphereCloudCredSecretName = "vsphere-creds"
 
+	// VSphereCredOverrideNamespace is the namespace where per-component vSphere
+	// credential override secrets can be placed. When a secret carrying the
+	// appropriate target annotations exists in this namespace, it will be used
+	// instead of the root credential secret in kube-system.
+	VSphereCredOverrideNamespace = "openshift-config"
+
+	// VSphereCredTargetSecretNamespaceAnnotation is the annotation key on a
+	// per-component vSphere credential override secret that specifies the
+	// target namespace of the CredentialsRequest it applies to.
+	VSphereCredTargetSecretNamespaceAnnotation = "cloudcredential.openshift.io/target-secret-namespace"
+
+	// VSphereCredTargetSecretNameAnnotation is the annotation key on a
+	// per-component vSphere credential override secret that specifies the
+	// target secret name of the CredentialsRequest it applies to.
+	VSphereCredTargetSecretNameAnnotation = "cloudcredential.openshift.io/target-secret-name"
+
 	// KubevirtCloudCredSecretName is the name of the secret where credentials
 	// for Kubevirt are stored.
 	KubevirtCloudCredSecretName = "kubevirt-credentials"

pkg/operator/credentialsrequest/credentialsrequest_controller.go

@@ -233,6 +233,28 @@ func add(mgr, adminMgr manager.Manager, r reconcile.Reconciler) error {
 		return err
 	}
 
+	// Watch for per-component vSphere credential override secrets in openshift-config.
+	// When an override secret is created, updated, or deleted, all CredentialsRequests
+	// are reconciled so the actuator can pick up the change.
+	vsphereOverrideSecretPredicate := predicate.TypedFuncs[*corev1.Secret]{
+		UpdateFunc: func(e event.TypedUpdateEvent[*corev1.Secret]) bool {
+			return IsVSphereOverrideSecret(e.ObjectNew.GetNamespace(), e.ObjectNew.GetAnnotations())
+		},
+		CreateFunc: func(e event.TypedCreateEvent[*corev1.Secret]) bool {
+			return IsVSphereOverrideSecret(e.Object.GetNamespace(), e.Object.GetAnnotations())
+		},
+		DeleteFunc: func(e event.TypedDeleteEvent[*corev1.Secret]) bool {
+			return IsVSphereOverrideSecret(e.Object.GetNamespace(), e.Object.GetAnnotations())
+		},
+	}
+	err = c.Watch(
+		source.Kind(mgr.GetCache(), &corev1.Secret{},
+			secretAllCredRequestsMapFn,
+			vsphereOverrideSecretPredicate))
+	if err != nil {
+		return err
+	}
+
 	// infraAllCredRequestsMapFn simply looks up all CredentialsRequests and requests they be reconciled.
 	infraAllCredRequestsMapFn := handler.TypedEnqueueRequestsFromMapFunc(func(ctx context.Context, a *configv1.Infrastructure) []reconcile.Request {
 		log.Info("requeueing all CredentialsRequests")
@@ -597,6 +619,26 @@ func IsAdminCredSecret(namespace, secretName string) bool {
 	return false
 }
 
+// IsVSphereOverrideSecret returns true if the given secret is a per-component
+// vSphere credential override in the openshift-config namespace. Override
+// secrets are identified by carrying both the target-secret-namespace and
+// target-secret-name annotations.
+func IsVSphereOverrideSecret(namespace string, annotations map[string]string) bool {
+	if namespace != constants.VSphereCredOverrideNamespace {
+		return false
+	}
+	if annotations == nil {
+		return false
+	}
+	_, hasTargetNS := annotations[constants.VSphereCredTargetSecretNamespaceAnnotation]
+	_, hasTargetName := annotations[constants.VSphereCredTargetSecretNameAnnotation]
+	if hasTargetNS && hasTargetName {
+		log.WithField("namespace", namespace).Info("observed vSphere credential override secret event")
+		return true
+	}
+	return false
+}
+
 var _ reconcile.Reconciler = &ReconcileCredentialsRequest{}
 
 // ReconcileCredentialsRequest reconciles a CredentialsRequest object

pkg/operator/credentialsrequest/credentialsrequest_controller_vsphere_test.go

@@ -49,6 +49,10 @@ var (
 		"key1": []byte("key1data"),
 		"key2": []byte("key2data"),
 	}
+	testVSphereOverrideCredsSecretData = map[string][]byte{
+		"key1": []byte("override-key1data"),
+		"key2": []byte("override-key2data"),
+	}
 )
 
 func init() {
@@ -313,3 +317,304 @@ func testSecret(namespace, name string, secretData map[string][]byte) *corev1.Se
 	}
 	return s
 }
+
+// testVSphereOverrideSecret creates a per-component override secret in
+// openshift-config with both target annotations and the mode annotation.
+func testVSphereOverrideSecret(name, targetNamespace, targetSecretName string, data map[string][]byte) *corev1.Secret {
+	return &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: constants.VSphereCredOverrideNamespace,
+			Annotations: map[string]string{
+				constants.AnnotationKey:                              constants.PassthroughAnnotation,
+				constants.VSphereCredTargetSecretNamespaceAnnotation: targetNamespace,
+				constants.VSphereCredTargetSecretNameAnnotation:      targetSecretName,
+			},
+		},
+		Data: data,
+	}
+}
+
+func TestIsVSphereOverrideSecret(t *testing.T) {
+	tests := []struct {
+		name        string
+		namespace   string
+		annotations map[string]string
+		expected    bool
+	}{
+		{
+			name:      "valid override secret: correct namespace and both target annotations",
+			namespace: constants.VSphereCredOverrideNamespace,
+			annotations: map[string]string{
+				constants.VSphereCredTargetSecretNamespaceAnnotation: "openshift-machine-api",
+				constants.VSphereCredTargetSecretNameAnnotation:      "vsphere-cloud-credentials",
+			},
+			expected: true,
+		},
+		{
+			name:      "wrong namespace: returns false",
+			namespace: "kube-system",
+			annotations: map[string]string{
+				constants.VSphereCredTargetSecretNamespaceAnnotation: "openshift-machine-api",
+				constants.VSphereCredTargetSecretNameAnnotation:      "vsphere-cloud-credentials",
+			},
+			expected: false,
+		},
+		{
+			name:        "nil annotations: returns false",
+			namespace:   constants.VSphereCredOverrideNamespace,
+			annotations: nil,
+			expected:    false,
+		},
+		{
+			name:        "empty annotations: returns false",
+			namespace:   constants.VSphereCredOverrideNamespace,
+			annotations: map[string]string{},
+			expected:    false,
+		},
+		{
+			name:      "only target namespace annotation: returns false",
+			namespace: constants.VSphereCredOverrideNamespace,
+			annotations: map[string]string{
+				constants.VSphereCredTargetSecretNamespaceAnnotation: "openshift-machine-api",
+			},
+			expected: false,
+		},
+		{
+			name:      "only target name annotation: returns false",
+			namespace: constants.VSphereCredOverrideNamespace,
+			annotations: map[string]string{
+				constants.VSphereCredTargetSecretNameAnnotation: "vsphere-cloud-credentials",
+			},
+			expected: false,
+		},
+		{
+			name:      "unrelated annotations only: returns false",
+			namespace: constants.VSphereCredOverrideNamespace,
+			annotations: map[string]string{
+				"some-other-key": "some-value",
+			},
+			expected: false,
+		},
+		{
+			name:      "both annotations plus extras: still returns true",
+			namespace: constants.VSphereCredOverrideNamespace,
+			annotations: map[string]string{
+				constants.VSphereCredTargetSecretNamespaceAnnotation: "openshift-cluster-csi-drivers",
+				constants.VSphereCredTargetSecretNameAnnotation:      "vsphere-csi-credentials",
+				constants.AnnotationKey:                              constants.PassthroughAnnotation,
+			},
+			expected: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := IsVSphereOverrideSecret(tt.namespace, tt.annotations)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestCredentialsRequestVSphereReconcileWithOverride(t *testing.T) {
+	schemeutils.SetupScheme(scheme.Scheme)
+
+	tests := []struct {
+		name          string
+		existing      []runtime.Object
+		existingAdmin []runtime.Object
+		expectErr     bool
+		validate      func(client.Client, *testing.T)
+		// Expected conditions on the credentials request:
+		expectedConditions []ExpectedCondition
+		// Expected conditions on the credentials cluster operator:
+		expectedCOConditions []ExpectedCOCondition
+	}{
+		{
+			name: "new credentialsrequest with override secret: uses override data",
+			existing: []runtime.Object{
+				testOperatorConfig(""),
+				createTestNamespace(testNamespace),
+				createTestNamespace(testSecretNamespace),
+				testVSphereCredentialsRequest(t),
+				testVSphereOverrideSecret("machine-api-override", testSecretNamespace, testSecretName, testVSphereOverrideCredsSecretData),
+			},
+			existingAdmin: []runtime.Object{
+				testVSphereCredsSecretPassthrough(),
+			},
+			validate: func(c client.Client, t *testing.T) {
+				targetSecret := getCredRequestTargetSecret(c)
+				require.NotNil(t, targetSecret, "expected non-empty target secret to exist")
+				assert.Equal(t, testVSphereOverrideCredsSecretData, targetSecret.Data)
+				cr := getCredRequest(c)
+				assert.NotNil(t, cr)
+				assert.True(t, cr.Status.Provisioned)
+				assert.Equal(t, int64(testCRGeneration), int64(cr.Status.LastSyncGeneration))
+				assert.NotNil(t, cr.Status.LastSyncTimestamp)
+			},
+		},
+		{
+			name: "existing target with root data but override now present: updates to override data",
+			existing: []runtime.Object{
+				testOperatorConfig(""),
+				createTestNamespace(testSecretNamespace),
+				testVSphereCredentialsRequest(t),
+				testSecret(testSecretNamespace, testSecretName, testVSphereCloudCredsSecretData),
+				testVSphereOverrideSecret("machine-api-override", testSecretNamespace, testSecretName, testVSphereOverrideCredsSecretData),
+			},
+			existingAdmin: []runtime.Object{
+				testVSphereCredsSecretPassthrough(),
+			},
+			validate: func(c client.Client, t *testing.T) {
+				targetSecret := getCredRequestTargetSecret(c)
+				require.NotNil(t, targetSecret, "expected non-empty target secret to exist")
+				// Target secret should now hold the override data
+				assert.Equal(t, testVSphereOverrideCredsSecretData, targetSecret.Data)
+				cr := getCredRequest(c)
+				assert.NotNil(t, cr)
+				assert.True(t, cr.Status.Provisioned)
+			},
+		},
+		{
+			name: "override for non-matching target: falls back to root",
+			existing: []runtime.Object{
+				testOperatorConfig(""),
+				createTestNamespace(testNamespace),
+				createTestNamespace(testSecretNamespace),
+				testVSphereCredentialsRequest(t),
+				// Override targets a DIFFERENT namespace/secret than the CR's SecretRef
+				testVSphereOverrideSecret("csi-override", "openshift-cluster-csi-drivers", "vsphere-csi-credentials", testVSphereOverrideCredsSecretData),
+			},
+			existingAdmin: []runtime.Object{
+				testVSphereCredsSecretPassthrough(),
+			},
+			validate: func(c client.Client, t *testing.T) {
+				targetSecret := getCredRequestTargetSecret(c)
+				require.NotNil(t, targetSecret, "expected non-empty target secret to exist")
+				// Should use root data since override doesn't match
+				assert.Equal(t, testVSphereCloudCredsSecretData, targetSecret.Data)
+			},
+		},
+		{
+			name: "override secret without mode annotation: returns error",
+			existing: []runtime.Object{
+				testOperatorConfig(""),
+				createTestNamespace(testNamespace),
+				createTestNamespace(testSecretNamespace),
+				testVSphereCredentialsRequest(t),
+				// Override secret that has target annotations but is missing the
+				// cloudcredential.openshift.io/mode annotation (not yet processed
+				// by the secret annotator).
+				&corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "unannotated-override",
+						Namespace: constants.VSphereCredOverrideNamespace,
+						Annotations: map[string]string{
+							constants.VSphereCredTargetSecretNamespaceAnnotation: testSecretNamespace,
+							constants.VSphereCredTargetSecretNameAnnotation:      testSecretName,
+						},
+					},
+					Data: testVSphereOverrideCredsSecretData,
+				},
+			},
+			existingAdmin: []runtime.Object{
+				testVSphereCredsSecretPassthrough(),
+			},
+			expectErr: true,
+			validate: func(c client.Client, t *testing.T) {
+				targetSecret := getCredRequestTargetSecret(c)
+				assert.Nil(t, targetSecret, "target secret should not be created when override is unannotated")
+				cr := getCredRequest(c)
+				assert.False(t, cr.Status.Provisioned)
+			},
+			expectedCOConditions: []ExpectedCOCondition{
+				{
+					conditionType: configv1.OperatorProgressing,
+					status:        corev1.ConditionTrue,
+				},
+			},
+		},
+		{
+			name: "override present with no root secret: uses override only",
+			existing: []runtime.Object{
+				testOperatorConfig(""),
+				createTestNamespace(testNamespace),
+				createTestNamespace(testSecretNamespace),
+				testVSphereCredentialsRequest(t),
+				testVSphereOverrideSecret("machine-api-override", testSecretNamespace, testSecretName, testVSphereOverrideCredsSecretData),
+			},
+			existingAdmin: []runtime.Object{},
+			validate: func(c client.Client, t *testing.T) {
+				targetSecret := getCredRequestTargetSecret(c)
+				require.NotNil(t, targetSecret, "expected target secret to exist even without root credential")
+				assert.Equal(t, testVSphereOverrideCredsSecretData, targetSecret.Data)
+				cr := getCredRequest(c)
+				assert.NotNil(t, cr)
+				assert.True(t, cr.Status.Provisioned)
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			mockCtrl := gomock.NewController(t)
+			defer mockCtrl.Finish()
+
+			fakeClient := fake.NewClientBuilder().
+				WithStatusSubresource(&minterv1.CredentialsRequest{}).
+				WithRuntimeObjects(test.existing...).Build()
+			fakeAdminClient := fake.NewClientBuilder().
+				WithRuntimeObjects(test.existingAdmin...).Build()
+			rcr := &ReconcileCredentialsRequest{
+				Client:      fakeClient,
+				AdminClient: fakeAdminClient,
+				Actuator: &actuator.VSphereActuator{
+					Client:         fakeClient,
+					RootCredClient: fakeAdminClient,
+				},
+				platformType: configv1.VSpherePlatformType,
+			}
+
+			_, err := rcr.Reconcile(context.TODO(), reconcile.Request{
+				NamespacedName: types.NamespacedName{
+					Name:      testCRName,
+					Namespace: testNamespace,
+				},
+			})
+
+			if test.validate != nil {
+				test.validate(fakeClient, t)
+			}
+
+			if err != nil && !test.expectErr {
+				t.Errorf("Unexpected error: %v", err)
+			}
+			if err == nil && test.expectErr {
+				t.Errorf("Expected error but got none")
+			}
+
+			cr := getCredRequest(fakeClient)
+			for _, condition := range test.expectedConditions {
+				foundCondition := utils.FindCredentialsRequestCondition(cr.Status.Conditions, condition.conditionType)
+				assert.NotNil(t, foundCondition)
+				assert.Exactly(t, condition.status, foundCondition.Status)
+				assert.Exactly(t, condition.reason, foundCondition.Reason)
+			}
+
+			if test.expectedCOConditions != nil {
+				logger := log.WithFields(log.Fields{"controller": controllerName})
+				currentConditions, err := rcr.GetConditions(logger)
+				require.NoError(t, err, "failed getting conditions")
+
+				for _, expectedCondition := range test.expectedCOConditions {
+					foundCondition := utils.FindClusterOperatorCondition(currentConditions, expectedCondition.conditionType)
+					require.NotNil(t, foundCondition)
+					assert.Equal(t, string(expectedCondition.status), string(foundCondition.Status), "condition %s had unexpected status", expectedCondition.conditionType)
+					if expectedCondition.reason != "" {
+						assert.Exactly(t, expectedCondition.reason, foundCondition.Reason)
+					}
+				}
+			}
+		})
+	}
+}