[1952] fix: round 2 of Devin review findings on platform9#1955

Two new findings from Devin's re-run on the previous fix commit:

1. Post-validation enrichment failed for clouds.yaml-backed Secrets.
   GetOpenStackClients (used by fetchAndUpdateFlavors / updateOpenstackInfo)
   and GetOpenstackCredentialsFromSecret (used by syncProjectName) only
   read OS_* keys. For clouds.yaml-backed Secrets those keys are
   missing, so after successful validation the post-validation
   enrichment loop spun forever logging "OS_AUTH_URL is missing".

   New helper GetOpenstackCredsInfoFromCreds(ctx, k3sclient, *OpenstackCreds)
   branches on SecretContainsCloudsYAML: for clouds.yaml, parses with
   Spec.CloudName and returns an OpenStackCredsInfo populated from the
   parsed CloudConfig (auth_type=v3applicationcredential leaves
   Username/Password/TenantName empty since App Creds carry scope at
   creation time). For legacy OS_*-keyed Secrets, delegates to the
   existing GetOpenstackCredentialsFromSecret path unchanged.

   Four call sites switched to the new helper:
   - GetOpenStackClients (credutils.go:320, region for endpoint)
   - GetOpenstackInfo's security-group lookup (credutils.go:277,
     TenantName for filter)
   - syncProjectName (openstackcreds_controller.go:673,
     TenantName -> Spec.ProjectName)
   - Cinder backend pool discovery (credutils.go:2212, region)

   The ValidateAndGetProviderClient legacy branch (credutils.go:374)
   is unchanged — it only runs when SecretContainsCloudsYAML is false.

2. checkStatusSuccess broke during upgrade window. Previous patch
   changed migrationplan_controller.go:1869 to read only Conditions;
   pre-upgrade OpenstackCreds resources have the flat
   OpenStackValidationStatus = Succeeded but an empty Conditions
   slice until the OpenstackCreds controller re-reconciles. During
   that small window MigrationPlan reconciliation blocked for those
   credentials.

   Added a legacy fallback: ready if either CredentialsValidated=True
   (new) OR OpenStackValidationStatus == PodSucceeded (legacy). Both
   paths produce the same operational signal; the OR is a transient
   bridge across the upgrade window.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: gtherond

graphify-out/cache/ast/99b618dd91866e242d11720ad340b249d1f8e6b12c86330e819af288addb2ae7.json

@@ -1864,9 +1864,15 @@ func (r *MigrationPlanReconciler) checkStatusSuccess(ctx context.Context,
 		if !ok {
 			return false, errors.Wrap(err, "failed to convert credentials to OpenstackCreds")
 		}
-		// Check the standard Kubernetes Condition rather than the retired flat
-		// status fields. A resource is ready when CredentialsValidated=True.
-		if !meta.IsStatusConditionTrue(openstackCreds.Status.Conditions, utils.ConditionCredentialsValidated) {
+		// Prefer the Kubernetes Condition (CredentialsValidated=True). Fall
+		// back to the legacy flat OpenStackValidationStatus field for
+		// resources that haven't been re-reconciled by the upgraded
+		// OpenstackCreds controller yet — those resources still carry the
+		// old flat status but no Conditions slice, and we shouldn't block
+		// MigrationPlan reconciliation on the upgrade window.
+		conditionReady := meta.IsStatusConditionTrue(openstackCreds.Status.Conditions, utils.ConditionCredentialsValidated)
+		legacyReady := openstackCreds.Status.OpenStackValidationStatus == string(corev1.PodSucceeded)
+		if !conditionReady && !legacyReady {
 			return false, errors.Errorf("openstackcreds '%s' CR is not validated", openstackCreds.Name)
 		}
 	}

graphify-out/cache/ast/b717f121844d1836654cdc16d645ba1bf2475573abca1400911877840148ff44.json

@@ -670,7 +670,7 @@ func fetchAndUpdateFlavors(ctx context.Context, r *OpenstackCredsReconciler, sco
 
 func syncProjectName(ctx context.Context, r *OpenstackCredsReconciler, scope *scope.OpenstackCredsScope) error {
 	ctxlog := scope.Logger
-	openstackCredential, err := utils.GetOpenstackCredentialsFromSecret(ctx, r.Client, scope.OpenstackCreds.Spec.SecretRef.Name)
+	openstackCredential, err := utils.GetOpenstackCredsInfoFromCreds(ctx, r.Client, scope.OpenstackCreds)
 	if err != nil {
 		ctxlog.Error(err, "Failed to get OpenStack credentials from secret", "secretName", scope.OpenstackCreds.Spec.SecretRef.Name)
 		return errors.Wrap(err, "failed to get Openstack credentials from secret")

graphify-out/cache/ast/dca3fbfa816b37c135741217ddf8128ceac0f9284f831f53b7841f5ad7d3a353.json

@@ -112,11 +112,59 @@ func GetVMwareCredentialsFromSecret(ctx context.Context, k3sclient client.Client
 	return vmwarecommon.GetVMwareCredentialsFromSecret(ctx, k3sclient, secretName)
 }
 
-// GetOpenstackCredentialsFromSecret retrieves and checks the secret
+// GetOpenstackCredentialsFromSecret retrieves and checks the secret. Handles
+// only the legacy OS_*-keyed Secret format. Callers that may encounter a
+// clouds.yaml-backed Secret must use GetOpenstackCredsInfoFromCreds (which
+// branches on Secret content and uses the OpenstackCreds.Spec.CloudName for
+// multi-entry clouds.yaml selection).
 func GetOpenstackCredentialsFromSecret(ctx context.Context, k3sclient client.Client, secretName string) (vjailbreakv1alpha1.OpenStackCredsInfo, error) {
 	return openstackcommon.GetOpenstackCredentialsFromSecret(ctx, k3sclient, secretName)
 }
 
+// GetOpenstackCredsInfoFromCreds returns an OpenStackCredsInfo for the given
+// OpenstackCreds resource, supporting both clouds.yaml-keyed Secrets (the new
+// path that consults Spec.CloudName for multi-entry selection) and legacy
+// OS_*-keyed Secrets (the existing path).
+//
+// Post-validation enrichment functions (flavor discovery, network/volume-type
+// listing, security group listing, project name sync, Cinder backend pool
+// discovery) must use this helper rather than GetOpenstackCredentialsFromSecret
+// directly so they work for clouds.yaml-backed credentials. For
+// auth_type=v3applicationcredential, the returned info has empty
+// Username/Password/TenantName since Application Credentials carry scope at
+// creation time; consumers that require those fields must check explicitly.
+func GetOpenstackCredsInfoFromCreds(ctx context.Context, k3sclient client.Client, openstackcreds *vjailbreakv1alpha1.OpenstackCreds) (vjailbreakv1alpha1.OpenStackCredsInfo, error) {
+	if openstackcreds == nil {
+		return vjailbreakv1alpha1.OpenStackCredsInfo{}, errors.New("openstackcreds cannot be nil")
+	}
+	secret := &corev1.Secret{}
+	if err := k3sclient.Get(ctx, k8stypes.NamespacedName{
+		Namespace: constants.NamespaceMigrationSystem,
+		Name:      openstackcreds.Spec.SecretRef.Name,
+	}, secret); err != nil {
+		return vjailbreakv1alpha1.OpenStackCredsInfo{}, errors.Wrapf(err, "failed to get secret %q", openstackcreds.Spec.SecretRef.Name)
+	}
+	if SecretContainsCloudsYAML(secret.Data) {
+		cfg, err := ParseCloudsYAML(secret.Data["clouds.yaml"], openstackcreds.Spec.CloudName)
+		if err != nil {
+			return vjailbreakv1alpha1.OpenStackCredsInfo{}, errors.Wrap(err, "failed to parse clouds.yaml")
+		}
+		info := vjailbreakv1alpha1.OpenStackCredsInfo{
+			AuthURL:    cfg.AuthOptions.IdentityEndpoint,
+			Username:   cfg.AuthOptions.Username,
+			Password:   cfg.AuthOptions.Password,
+			RegionName: cfg.RegionName,
+			TenantName: cfg.AuthOptions.TenantName,
+			DomainName: cfg.AuthOptions.DomainName,
+		}
+		if cfg.Verify != nil && !*cfg.Verify {
+			info.Insecure = true
+		}
+		return info, nil
+	}
+	return openstackcommon.GetOpenstackCredentialsFromSecret(ctx, k3sclient, openstackcreds.Spec.SecretRef.Name)
+}
+
 // VerifyNetworks verifies the existence of specified networks in OpenStack
 func VerifyNetworks(ctx context.Context, k3sclient client.Client, openstackcreds *vjailbreakv1alpha1.OpenstackCreds, targetnetworks []string) error {
 	openstackClients, err := GetOpenStackClients(ctx, k3sclient, openstackcreds)
@@ -274,7 +322,7 @@ func GetOpenstackInfo(ctx context.Context, k3sclient client.Client, openstackcre
 		})
 	}
 
-	credsInfo, err := GetOpenstackCredentialsFromSecret(ctx, k3sclient, openstackcreds.Spec.SecretRef.Name)
+	credsInfo, err := GetOpenstackCredsInfoFromCreds(ctx, k3sclient, openstackcreds)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to get openstack credentials for project lookup")
 	}
@@ -317,7 +365,7 @@ func GetOpenStackClients(ctx context.Context, k3sclient client.Client, openstack
 		return nil, errors.New("openstackcreds cannot be nil")
 	}
 
-	openstackCredential, err := GetOpenstackCredentialsFromSecret(ctx, k3sclient, openstackcreds.Spec.SecretRef.Name)
+	openstackCredential, err := GetOpenstackCredsInfoFromCreds(ctx, k3sclient, openstackcreds)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to get openstack credentials from secret")
 	}
@@ -2209,7 +2257,7 @@ func GetBackendPools(ctx context.Context, k3sclient client.Client, openstackcred
 	ctxlog.Info("Discovering backend pools from OpenStack Cinder")
 
 	// Get OpenStack credentials to extract region
-	openstackCredential, err := GetOpenstackCredentialsFromSecret(ctx, k3sclient, openstackcreds.Spec.SecretRef.Name)
+	openstackCredential, err := GetOpenstackCredsInfoFromCreds(ctx, k3sclient, openstackcreds)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to get OpenStack credentials from secret")
 	}