diff --git a/pkg/operator/constants/constants.go b/pkg/operator/constants/constants.go index acbc81ad12..f9e119069e 100644 --- a/pkg/operator/constants/constants.go +++ b/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" diff --git a/pkg/operator/credentialsrequest/credentialsrequest_controller.go b/pkg/operator/credentialsrequest/credentialsrequest_controller.go index 4eb72bdbd6..d7be84fa8d 100644 --- a/pkg/operator/credentialsrequest/credentialsrequest_controller.go +++ b/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 diff --git a/pkg/operator/credentialsrequest/credentialsrequest_controller_vsphere_test.go b/pkg/operator/credentialsrequest/credentialsrequest_controller_vsphere_test.go index bad8a8012f..a28cb417f0 100644 --- a/pkg/operator/credentialsrequest/credentialsrequest_controller_vsphere_test.go +++ b/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) + } + } + } + }) + } +} diff --git a/pkg/vsphere/actuator/actuator.go b/pkg/vsphere/actuator/actuator.go index a97e702041..b78dc1fea1 100644 --- a/pkg/vsphere/actuator/actuator.go +++ b/pkg/vsphere/actuator/actuator.go @@ -319,6 +319,44 @@ func (a *VSphereActuator) GetCredentialsRootSecretLocation() types.NamespacedNam func (a *VSphereActuator) GetCredentialsRootSecret(ctx context.Context, cr *minterv1.CredentialsRequest) (*corev1.Secret, error) { logger := a.getLogger(cr) + + // Check for a per-component override secret in the override namespace before + // falling back to the shared root credential. Override secrets are identified + // by annotations that map them to a specific CredentialsRequest's target secret. + overrideSecretList := &corev1.SecretList{} + if err := a.Client.List(ctx, overrideSecretList, client.InNamespace(constants.VSphereCredOverrideNamespace)); err != nil { + logger.WithError(err).Error("error listing secrets in override namespace") + return nil, &actuatoriface.ActuatorError{ + ErrReason: minterv1.CredentialsProvisionFailure, + Message: fmt.Sprintf("error listing secrets in %s: %v", constants.VSphereCredOverrideNamespace, err), + } + } + + for i := range overrideSecretList.Items { + s := &overrideSecretList.Items[i] + if s.Annotations == nil { + continue + } + targetNS := s.Annotations[constants.VSphereCredTargetSecretNamespaceAnnotation] + targetName := s.Annotations[constants.VSphereCredTargetSecretNameAnnotation] + if targetNS == cr.Spec.SecretRef.Namespace && targetName == cr.Spec.SecretRef.Name { + if !isSecretAnnotated(s) { + logger.WithField("secret", fmt.Sprintf("%s/%s", s.Namespace, s.Name)). + Error("per-component override secret not yet annotated") + return nil, &actuatoriface.ActuatorError{ + ErrReason: minterv1.CredentialsProvisionFailure, + Message: "cannot proceed without per-component override secret annotation", + } + } + logger.WithField("secret", fmt.Sprintf("%s/%s", s.Namespace, s.Name)). + Info("using per-component credential override secret") + return s, nil + } + } + + logger.Debug("no per-component override secret found, falling back to root credential") + + // Fall back to the shared root credential secret. cloudCredSecret := &corev1.Secret{} if err := a.RootCredClient.Get(ctx, a.GetCredentialsRootSecretLocation(), cloudCredSecret); err != nil { msg := "unable to fetch root cloud cred secret" diff --git a/pkg/vsphere/actuator/actuator_test.go b/pkg/vsphere/actuator/actuator_test.go new file mode 100644 index 0000000000..27dd8e01ce --- /dev/null +++ b/pkg/vsphere/actuator/actuator_test.go @@ -0,0 +1,451 @@ +/* +Copyright 2020 The OpenShift Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package actuator + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/scheme" + + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + minterv1 "github.com/openshift/cloud-credential-operator/pkg/apis/cloudcredential/v1" + "github.com/openshift/cloud-credential-operator/pkg/operator/constants" + schemeutils "github.com/openshift/cloud-credential-operator/pkg/util" +) + +const ( + testTargetNamespace = "openshift-machine-api" + testTargetSecret = "vsphere-cloud-credentials" +) + +func TestGetCredentialsRootSecret(t *testing.T) { + schemeutils.SetupScheme(scheme.Scheme) + + tests := []struct { + name string + // objects visible to the regular client (component namespaces, openshift-config) + existing []runtime.Object + // objects visible to the root credential client (kube-system) + existingRootCred []runtime.Object + // target namespace for the CredentialsRequest + targetNamespace string + // target secret name for the CredentialsRequest + targetSecretName string + // expected secret data key/value the method should return + expectDataKey string + expectDataValue string + expectErr bool + expectErrMsg string + }{ + { + name: "override secret with matching annotations: uses override data", + targetNamespace: "openshift-machine-api", + targetSecretName: testTargetSecret, + existing: []runtime.Object{ + testOverrideSecret("my-machine-api-creds", "openshift-machine-api", testTargetSecret, map[string][]byte{ + "username": []byte("override-user"), + "password": []byte("override-pass"), + }), + }, + existingRootCred: []runtime.Object{ + testRootSecret(), + }, + expectDataKey: "username", + expectDataValue: "override-user", + }, + { + name: "override secret absent: falls back to root secret", + targetNamespace: "openshift-machine-api", + targetSecretName: testTargetSecret, + existing: []runtime.Object{}, + existingRootCred: []runtime.Object{ + testRootSecret(), + }, + expectDataKey: "username", + expectDataValue: "root-user", + }, + { + name: "override for csi drivers", + targetNamespace: "openshift-cluster-csi-drivers", + targetSecretName: "vsphere-csi-credentials", + existing: []runtime.Object{ + testOverrideSecret("csi-driver-creds", "openshift-cluster-csi-drivers", "vsphere-csi-credentials", map[string][]byte{ + "username": []byte("csi-user"), + "password": []byte("csi-pass"), + }), + }, + existingRootCred: []runtime.Object{ + testRootSecret(), + }, + expectDataKey: "username", + expectDataValue: "csi-user", + }, + { + name: "override secret not mode-annotated: returns error", + targetNamespace: "openshift-machine-api", + targetSecretName: testTargetSecret, + existing: []runtime.Object{ + testUnannotatedOverrideSecret("my-creds", "openshift-machine-api", testTargetSecret), + }, + existingRootCred: []runtime.Object{ + testRootSecret(), + }, + expectErr: true, + expectErrMsg: "cannot proceed without per-component override secret annotation", + }, + { + name: "override with wrong target namespace annotation: ignored, falls back to root", + targetNamespace: "openshift-machine-api", + targetSecretName: testTargetSecret, + existing: []runtime.Object{ + testOverrideSecret("wrong-ns-creds", "openshift-wrong-ns", testTargetSecret, map[string][]byte{ + "username": []byte("wrong-user"), + "password": []byte("wrong-pass"), + }), + }, + existingRootCred: []runtime.Object{ + testRootSecret(), + }, + expectDataKey: "username", + expectDataValue: "root-user", + }, + { + name: "override with wrong target name annotation: ignored, falls back to root", + targetNamespace: "openshift-machine-api", + targetSecretName: testTargetSecret, + existing: []runtime.Object{ + testOverrideSecret("wrong-name-creds", "openshift-machine-api", "wrong-secret-name", map[string][]byte{ + "username": []byte("wrong-user"), + "password": []byte("wrong-pass"), + }), + }, + existingRootCred: []runtime.Object{ + testRootSecret(), + }, + expectDataKey: "username", + expectDataValue: "root-user", + }, + { + name: "override with missing annotations: ignored, falls back to root", + targetNamespace: "openshift-machine-api", + targetSecretName: testTargetSecret, + existing: []runtime.Object{ + // Secret in openshift-config but with no target annotations at all + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-other-secret", + Namespace: constants.VSphereCredOverrideNamespace, + }, + Data: map[string][]byte{ + "username": []byte("other-user"), + }, + }, + }, + existingRootCred: []runtime.Object{ + testRootSecret(), + }, + expectDataKey: "username", + expectDataValue: "root-user", + }, + { + name: "root secret not annotated: returns error", + targetNamespace: "openshift-machine-api", + targetSecretName: testTargetSecret, + existing: []runtime.Object{}, + existingRootCred: []runtime.Object{ + testUnannotatedRootSecret(), + }, + expectErr: true, + expectErrMsg: "cannot proceed without cloud cred secret annotation", + }, + { + name: "neither override nor root secret exists: returns error", + targetNamespace: "openshift-machine-api", + targetSecretName: testTargetSecret, + existing: []runtime.Object{}, + existingRootCred: []runtime.Object{}, + expectErr: true, + expectErrMsg: "unable to fetch root cloud cred secret", + }, + { + name: "multiple overrides: correct one matched by annotations", + targetNamespace: "openshift-cloud-controller-manager", + targetSecretName: "vsphere-ccm-credentials", + existing: []runtime.Object{ + // Override targeting machine-api (should NOT match) + testOverrideSecret("machine-api-creds", "openshift-machine-api", testTargetSecret, map[string][]byte{ + "username": []byte("machine-api-user"), + "password": []byte("machine-api-pass"), + }), + // Override targeting CCM (should match) + testOverrideSecret("ccm-creds", "openshift-cloud-controller-manager", "vsphere-ccm-credentials", map[string][]byte{ + "username": []byte("ccm-user"), + "password": []byte("ccm-pass"), + }), + // Override targeting CSI (should NOT match) + testOverrideSecret("csi-creds", "openshift-cluster-csi-drivers", "vsphere-csi-credentials", map[string][]byte{ + "username": []byte("csi-user"), + "password": []byte("csi-pass"), + }), + }, + existingRootCred: []runtime.Object{ + testRootSecret(), + }, + expectDataKey: "username", + expectDataValue: "ccm-user", + }, + { + name: "override secret name is arbitrary: matches by annotations not name", + targetNamespace: "openshift-machine-api", + targetSecretName: testTargetSecret, + existing: []runtime.Object{ + testOverrideSecret("arbitrary-admin-chosen-name", "openshift-machine-api", testTargetSecret, map[string][]byte{ + "username": []byte("arbitrary-user"), + "password": []byte("arbitrary-pass"), + }), + }, + existingRootCred: []runtime.Object{ + testRootSecret(), + }, + expectDataKey: "username", + expectDataValue: "arbitrary-user", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeClient := fake.NewClientBuilder().WithRuntimeObjects(tt.existing...).Build() + fakeRootCredClient := fake.NewClientBuilder().WithRuntimeObjects(tt.existingRootCred...).Build() + + actuator := &VSphereActuator{ + Client: fakeClient, + RootCredClient: fakeRootCredClient, + } + + cr := testCredentialsRequest(tt.targetNamespace, tt.targetSecretName) + + secret, err := actuator.GetCredentialsRootSecret(context.TODO(), cr) + if tt.expectErr { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.expectErrMsg) + return + } + require.NoError(t, err) + require.NotNil(t, secret) + assert.Equal(t, tt.expectDataValue, string(secret.Data[tt.expectDataKey])) + }) + } +} + +func TestNeedsUpdate(t *testing.T) { + schemeutils.SetupScheme(scheme.Scheme) + + tests := []struct { + name string + existing []runtime.Object + existingRootCred []runtime.Object + targetNamespace string + targetSecretName string + expectUpdate bool + }{ + { + name: "target matches override: no update needed", + targetNamespace: "openshift-machine-api", + targetSecretName: testTargetSecret, + existing: []runtime.Object{ + testOverrideSecret("machine-api-creds", "openshift-machine-api", testTargetSecret, map[string][]byte{ + "username": []byte("override-user"), + "password": []byte("override-pass"), + }), + testTargetSecretWithData("openshift-machine-api", testTargetSecret, map[string][]byte{ + "username": []byte("override-user"), + "password": []byte("override-pass"), + }), + }, + existingRootCred: []runtime.Object{ + testRootSecret(), + }, + expectUpdate: false, + }, + { + name: "target matches root, no override: no update needed", + targetNamespace: "openshift-machine-api", + targetSecretName: testTargetSecret, + existing: []runtime.Object{ + testTargetSecretWithData("openshift-machine-api", testTargetSecret, map[string][]byte{ + "username": []byte("root-user"), + "password": []byte("root-pass"), + }), + }, + existingRootCred: []runtime.Object{ + testRootSecret(), + }, + expectUpdate: false, + }, + { + name: "target has root data but override now exists: update needed", + targetNamespace: "openshift-machine-api", + targetSecretName: testTargetSecret, + existing: []runtime.Object{ + testOverrideSecret("machine-api-creds", "openshift-machine-api", testTargetSecret, map[string][]byte{ + "username": []byte("override-user"), + "password": []byte("override-pass"), + }), + testTargetSecretWithData("openshift-machine-api", testTargetSecret, map[string][]byte{ + "username": []byte("root-user"), + "password": []byte("root-pass"), + }), + }, + existingRootCred: []runtime.Object{ + testRootSecret(), + }, + expectUpdate: true, + }, + { + name: "target secret missing: update needed", + targetNamespace: "openshift-machine-api", + targetSecretName: testTargetSecret, + existing: []runtime.Object{}, + existingRootCred: []runtime.Object{ + testRootSecret(), + }, + expectUpdate: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeClient := fake.NewClientBuilder().WithRuntimeObjects(tt.existing...).Build() + fakeRootCredClient := fake.NewClientBuilder().WithRuntimeObjects(tt.existingRootCred...).Build() + + actuator := &VSphereActuator{ + Client: fakeClient, + RootCredClient: fakeRootCredClient, + } + + cr := testCredentialsRequest(tt.targetNamespace, tt.targetSecretName) + needsUpdate, err := actuator.needsUpdate(context.TODO(), cr) + require.NoError(t, err) + assert.Equal(t, tt.expectUpdate, needsUpdate) + }) + } +} + +// --- Test helpers --- + +func testCredentialsRequest(targetNamespace, targetSecretName string) *minterv1.CredentialsRequest { + vsphereProviderSpec := &minterv1.VSphereProviderSpec{} + providerSpec, _ := minterv1.Codec.EncodeProviderSpec(vsphereProviderSpec) + + return &minterv1.CredentialsRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cred-request", + Namespace: "openshift-cloud-credential-operator", + }, + Spec: minterv1.CredentialsRequestSpec{ + SecretRef: corev1.ObjectReference{ + Name: targetSecretName, + Namespace: targetNamespace, + }, + ProviderSpec: providerSpec, + }, + } +} + +func testRootSecret() *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.VSphereCloudCredSecretName, + Namespace: constants.CloudCredSecretNamespace, + Annotations: map[string]string{ + constants.AnnotationKey: constants.PassthroughAnnotation, + }, + }, + Data: map[string][]byte{ + "username": []byte("root-user"), + "password": []byte("root-pass"), + }, + } +} + +func testUnannotatedRootSecret() *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.VSphereCloudCredSecretName, + Namespace: constants.CloudCredSecretNamespace, + }, + Data: map[string][]byte{ + "username": []byte("root-user"), + "password": []byte("root-pass"), + }, + } +} + +// testOverrideSecret creates a per-component override secret in +// openshift-config with annotation-based targeting. The secret name is +// arbitrary; mapping is done via annotations. +func testOverrideSecret(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, + } +} + +// testUnannotatedOverrideSecret creates an override secret that has the +// target annotations but is missing the cloudcredential.openshift.io/mode +// annotation, simulating a secret the annotator has not yet processed. +func testUnannotatedOverrideSecret(name, targetNamespace, targetSecretName string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: constants.VSphereCredOverrideNamespace, + Annotations: map[string]string{ + constants.VSphereCredTargetSecretNamespaceAnnotation: targetNamespace, + constants.VSphereCredTargetSecretNameAnnotation: targetSecretName, + }, + }, + Data: map[string][]byte{ + "username": []byte("override-user"), + "password": []byte("override-pass"), + }, + } +} + +func testTargetSecretWithData(namespace, name string, data map[string][]byte) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Data: data, + } +}