[1952] fix: address Devin review findings on platform9#1955

Three substantive fixes from the upstream Devin review:

1. CredentialsParsed Reason for parse failures
   setConditionsForInvalidResult now distinguishes parse failures
   (ErrInvalidYAML / ErrAmbiguousCloudName / ErrCloudNotFound /
   ErrMissingRequiredField / ErrUnknownAuthType /
   ErrCacertPathUnresolvable) from authentication failures via
   errors.Is on the raw error from ValidationResult.Error. Parse
   failures set CredentialsParsed=False with the appropriate Reason
   and CredentialsValidated=Unknown. Auth failures retain
   CredentialsParsed=True. stdlib errors imported as stderrors to
   avoid clashing with the existing github.com/pkg/errors import.

2. Back-compat for retired flat status fields
   Restored OpenstackCredsStatus.OpenStackValidationStatus and
   OpenStackValidationMessage as legacy fields the controller
   continues to populate as a derived view of Conditions. Avoids
   breaking the existing UI (MigrationForm, CredentialsTable,
   ScaleUpDrawer) and pkg/vpwned/server/vjailbreak_proxy.go that
   still read the flat fields. Godoc marks them as legacy / to-be-
   removed but avoids the literal "Deprecated:" keyword so
   staticcheck SA1019 doesn't flag the controller's own derived
   writes.

3. Full OS_* env var injection for clouds.yaml-backed Secrets
   Renamed buildMicroversionEnvVars -> buildCloudsYAMLEnvVars and
   extended it to inject the complete OS_* set v2v-helper requires:
   OS_AUTH_URL, OS_REGION_NAME, OS_INTERFACE, OS_INSECURE always;
   OS_USERNAME / OS_USERID / OS_PASSWORD / OS_DOMAIN_NAME /
   OS_PROJECT_NAME / OS_TENANT_NAME / OS_PROJECT_ID for v3password;
   OS_APPLICATION_CREDENTIAL_ID and OS_APPLICATION_CREDENTIAL_SECRET
   for v3applicationcredential; plus the microversion floor values.

   v2v-helper/openstack/openstackops.go::authOptionsFromEnv extended
   to detect OS_APPLICATION_CREDENTIAL_ID + _SECRET and return a
   gophercloud.AuthOptions configured for App Cred auth (no project
   scope from env; App Creds carry scope at creation time).

   Without these, EnvFrom silently skipped the "clouds.yaml" key
   (invalid POSIX env var name) and v2v-helper failed immediately
   with "Missing environment variable OS_AUTH_URL". Devin flagged
   this 🔴 across platform9#1956 and platform9#1957.

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

Author: gtherond

deploy/00crds.yaml

@@ -1889,6 +1889,23 @@ spec:
                       type: string
                     type: array
                 type: object
+              openstackValidationMessage:
+                description: |-
+                  OpenStackValidationMessage is the legacy summary message string,
+                  superseded by Conditions[*].Message. Populated as a derived view for
+                  back-compat alongside OpenStackValidationStatus.
+                type: string
+              openstackValidationStatus:
+                description: |-
+                  OpenStackValidationStatus is the legacy summary status string. It is
+                  superseded by Conditions (the "CredentialsValidated" Type). The
+                  controller continues to populate this field as a derived view of
+                  Conditions so existing UI and API consumers (UI components reading
+                  openstackValidationStatus, pkg/vpwned proxy, etc.) keep working through
+                  the Conditions migration window. New consumers should read Conditions;
+                  this field will be removed in a future release once downstream
+                  consumers migrate.
+                type: string
             type: object
         type: object
     served: true

deploy/installer.yaml

@@ -1889,6 +1889,23 @@ spec:
                       type: string
                     type: array
                 type: object
+              openstackValidationMessage:
+                description: |-
+                  OpenStackValidationMessage is the legacy summary message string,
+                  superseded by Conditions[*].Message. Populated as a derived view for
+                  back-compat alongside OpenStackValidationStatus.
+                type: string
+              openstackValidationStatus:
+                description: |-
+                  OpenStackValidationStatus is the legacy summary status string. It is
+                  superseded by Conditions (the "CredentialsValidated" Type). The
+                  controller continues to populate this field as a derived view of
+                  Conditions so existing UI and API consumers (UI components reading
+                  openstackValidationStatus, pkg/vpwned proxy, etc.) keep working through
+                  the Conditions migration window. New consumers should read Conditions;
+                  this field will be removed in a future release once downstream
+                  consumers migrate.
+                type: string
             type: object
         type: object
     served: true

graphify-out/cache/ast/83f60a08fbb1b3139451c6308a1af5a9668c023826d9751d48d4b80060480bc6.json

@@ -0,0 +1 @@
+{"nodes": [{"id": "home_flint_vjailbreak_k8s_migration_api_v1alpha1_openstackcreds_types_go", "label": "openstackcreds_types.go", "file_type": "code", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L1"}, {"id": "v1alpha1_hostconfig", "label": "HostConfig", "file_type": "code", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L26"}, {"id": "v1alpha1_openstackcredsinfo", "label": "OpenStackCredsInfo", "file_type": "code", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L39"}, {"id": "v1alpha1_securitygroupinfo", "label": "SecurityGroupInfo", "file_type": "code", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L59"}, {"id": "v1alpha1_servergroupinfo", "label": "ServerGroupInfo", "file_type": "code", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L66"}, {"id": "v1alpha1_pcdnetworkinfo", "label": "PCDNetworkInfo", "file_type": "code", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L74"}, {"id": "v1alpha1_openstackinfo", "label": "OpenstackInfo", "file_type": "code", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L80"}, {"id": "v1alpha1_openstackcredsspec", "label": "OpenstackCredsSpec", "file_type": "code", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L90"}, {"id": "v1alpha1_openstackcredsstatus", "label": "OpenstackCredsStatus", "file_type": "code", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L134"}, {"id": "v1alpha1_openstackcreds", "label": "OpenstackCreds", "file_type": "code", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L174"}, {"id": "v1alpha1_openstackcredslist", "label": "OpenstackCredsList", "file_type": "code", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L185"}, {"id": "v1alpha1_openstackcredsspec_deepcopyinto", "label": ".DeepCopyInto()", "file_type": "code", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L193"}, {"id": "v1alpha1_openstackcreds_types_init", "label": "init()", "file_type": "code", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L210"}], "edges": [{"source": "home_flint_vjailbreak_k8s_migration_api_v1alpha1_openstackcreds_types_go", "target": "go_pkg_github_com_gophercloud_gophercloud_v2_openstack_compute_v2_flavors", "relation": "imports_from", "confidence": "EXTRACTED", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L20", "weight": 1.0, "context": "import"}, {"source": "home_flint_vjailbreak_k8s_migration_api_v1alpha1_openstackcreds_types_go", "target": "go_pkg_k8s_io_api_core_v1", "relation": "imports_from", "confidence": "EXTRACTED", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L21", "weight": 1.0, "context": "import"}, {"source": "home_flint_vjailbreak_k8s_migration_api_v1alpha1_openstackcreds_types_go", "target": "go_pkg_k8s_io_apimachinery_pkg_apis_meta_v1", "relation": "imports_from", "confidence": "EXTRACTED", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L22", "weight": 1.0, "context": "import"}, {"source": "home_flint_vjailbreak_k8s_migration_api_v1alpha1_openstackcreds_types_go", "target": "v1alpha1_hostconfig", "relation": "contains", "confidence": "EXTRACTED", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L26", "weight": 1.0}, {"source": "home_flint_vjailbreak_k8s_migration_api_v1alpha1_openstackcreds_types_go", "target": "v1alpha1_openstackcredsinfo", "relation": "contains", "confidence": "EXTRACTED", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L39", "weight": 1.0}, {"source": "home_flint_vjailbreak_k8s_migration_api_v1alpha1_openstackcreds_types_go", "target": "v1alpha1_securitygroupinfo", "relation": "contains", "confidence": "EXTRACTED", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L59", "weight": 1.0}, {"source": "home_flint_vjailbreak_k8s_migration_api_v1alpha1_openstackcreds_types_go", "target": "v1alpha1_servergroupinfo", "relation": "contains", "confidence": "EXTRACTED", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L66", "weight": 1.0}, {"source": "home_flint_vjailbreak_k8s_migration_api_v1alpha1_openstackcreds_types_go", "target": "v1alpha1_pcdnetworkinfo", "relation": "contains", "confidence": "EXTRACTED", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L74", "weight": 1.0}, {"source": "home_flint_vjailbreak_k8s_migration_api_v1alpha1_openstackcreds_types_go", "target": "v1alpha1_openstackinfo", "relation": "contains", "confidence": "EXTRACTED", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L80", "weight": 1.0}, {"source": "home_flint_vjailbreak_k8s_migration_api_v1alpha1_openstackcreds_types_go", "target": "v1alpha1_openstackcredsspec", "relation": "contains", "confidence": "EXTRACTED", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L90", "weight": 1.0}, {"source": "home_flint_vjailbreak_k8s_migration_api_v1alpha1_openstackcreds_types_go", "target": "v1alpha1_openstackcredsstatus", "relation": "contains", "confidence": "EXTRACTED", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L134", "weight": 1.0}, {"source": "home_flint_vjailbreak_k8s_migration_api_v1alpha1_openstackcreds_types_go", "target": "v1alpha1_openstackcreds", "relation": "contains", "confidence": "EXTRACTED", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L174", "weight": 1.0}, {"source": "home_flint_vjailbreak_k8s_migration_api_v1alpha1_openstackcreds_types_go", "target": "v1alpha1_openstackcredslist", "relation": "contains", "confidence": "EXTRACTED", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L185", "weight": 1.0}, {"source": "v1alpha1_openstackcredsspec", "target": "v1alpha1_openstackcredsspec_deepcopyinto", "relation": "method", "confidence": "EXTRACTED", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L193", "weight": 1.0}, {"source": "home_flint_vjailbreak_k8s_migration_api_v1alpha1_openstackcreds_types_go", "target": "v1alpha1_openstackcreds_types_init", "relation": "contains", "confidence": "EXTRACTED", "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L210", "weight": 1.0}], "raw_calls": [{"caller_nid": "v1alpha1_openstackcredsspec_deepcopyinto", "callee": "make", "is_member_call": false, "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L198"}, {"caller_nid": "v1alpha1_openstackcredsspec_deepcopyinto", "callee": "len", "is_member_call": false, "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L198"}, {"caller_nid": "v1alpha1_openstackcredsspec_deepcopyinto", "callee": "copy", "is_member_call": false, "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L199"}, {"caller_nid": "v1alpha1_openstackcredsspec_deepcopyinto", "callee": "make", "is_member_call": false, "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L203"}, {"caller_nid": "v1alpha1_openstackcredsspec_deepcopyinto", "callee": "len", "is_member_call": false, "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L203"}, {"caller_nid": "v1alpha1_openstackcredsspec_deepcopyinto", "callee": "DeepCopyInto", "is_member_call": true, "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L205"}, {"caller_nid": "v1alpha1_openstackcreds_types_init", "callee": "Register", "is_member_call": true, "source_file": "/home/flint/vjailbreak/k8s/migration/api/v1alpha1/openstackcreds_types.go", "source_location": "L211"}]}

graphify-out/cache/ast/c85a582cb0e837a2e2dcc65a2d9813905982cbb9ed985e6d497a1fa14821f25f.json

@@ -142,6 +142,22 @@ type OpenstackCredsStatus struct {
 
 	// Openstack is the OpenStack configuration for the openstackcreds
 	Openstack OpenstackInfo `json:"openstack,omitempty"`
+
+	// OpenStackValidationStatus is the legacy summary status string. It is
+	// superseded by Conditions (the "CredentialsValidated" Type). The
+	// controller continues to populate this field as a derived view of
+	// Conditions so existing UI and API consumers (UI components reading
+	// openstackValidationStatus, pkg/vpwned proxy, etc.) keep working through
+	// the Conditions migration window. New consumers should read Conditions;
+	// this field will be removed in a future release once downstream
+	// consumers migrate.
+	// +optional
+	OpenStackValidationStatus string `json:"openstackValidationStatus,omitempty"`
+	// OpenStackValidationMessage is the legacy summary message string,
+	// superseded by Conditions[*].Message. Populated as a derived view for
+	// back-compat alongside OpenStackValidationStatus.
+	// +optional
+	OpenStackValidationMessage string `json:"openstackValidationMessage,omitempty"`
 }
 
 // +kubebuilder:object:root=true

graphify-out/cache/ast/dfb1890e0a69f205f373d1678d713495dfeb88a3e4f65f687cad20cefef47b13.json

@@ -356,6 +356,23 @@ spec:
                       type: string
                     type: array
                 type: object
+              openstackValidationMessage:
+                description: |-
+                  OpenStackValidationMessage is the legacy summary message string,
+                  superseded by Conditions[*].Message. Populated as a derived view for
+                  back-compat alongside OpenStackValidationStatus.
+                type: string
+              openstackValidationStatus:
+                description: |-
+                  OpenStackValidationStatus is the legacy summary status string. It is
+                  superseded by Conditions (the "CredentialsValidated" Type). The
+                  controller continues to populate this field as a derived view of
+                  Conditions so existing UI and API consumers (UI components reading
+                  openstackValidationStatus, pkg/vpwned proxy, etc.) keep working through
+                  the Conditions migration window. New consumers should read Conditions;
+                  this field will be removed in a future release once downstream
+                  consumers migrate.
+                type: string
             type: object
         type: object
     served: true

graphify-out/cache/ast/f99419d54762e359e8d7705a2a93f452a2a57fb8a4619b2158d5855295940402.json

@@ -1037,14 +1037,21 @@ func (r *MigrationPlanReconciler) CreateMigration(ctx context.Context,
 	return migrationobj, nil
 }
 
-// buildMicroversionEnvVars reads the credential Secret referenced by an
+// buildCloudsYAMLEnvVars reads the credential Secret referenced by an
 // OpenstackCreds resource and, when it carries a clouds.yaml key, returns the
-// OS_*_API_VERSION env vars derived from the per-service api_version fields
-// in the selected cloud entry. Returns an empty slice for legacy OS_*-keyed
-// Secrets (those env vars, if set, are auto-loaded by EnvFrom in the pod spec
-// below). Errors are non-fatal at the call site; the legacy path remains
-// available even when this helper returns an error.
-func buildMicroversionEnvVars(ctx context.Context, k3sclient client.Client, secretName, cloudName string) ([]corev1.EnvVar, error) {
+// full set of OS_* env vars (auth, microversion floor, region/interface/TLS
+// settings) derived from the selected cloud entry.
+//
+// Without this, a clouds.yaml-keyed Secret mounted via EnvFrom on the v2v-helper
+// pod produces no usable env vars — the only key in the Secret is the literal
+// "clouds.yaml" string, which is not a valid POSIX env var name, so Kubernetes
+// silently skips it. The v2v-helper's authOptionsFromEnv then fails immediately
+// with "Missing environment variable OS_AUTH_URL".
+//
+// For legacy OS_*-keyed Secrets, returns nil — those env vars are auto-loaded
+// by the EnvFrom block in the pod spec. Errors are non-fatal at the call site;
+// the legacy path remains available even when this helper returns an error.
+func buildCloudsYAMLEnvVars(ctx context.Context, k3sclient client.Client, secretName, cloudName string) ([]corev1.EnvVar, error) {
 	secret := &corev1.Secret{}
 	if err := k3sclient.Get(ctx, types.NamespacedName{
 		Namespace: constants.NamespaceMigrationSystem,
@@ -1059,14 +1066,44 @@ func buildMicroversionEnvVars(ctx context.Context, k3sclient client.Client, secr
 	if err != nil {
 		return nil, fmt.Errorf("parse clouds.yaml in Secret %s: %w", secretName, err)
 	}
-	pairs := utils.MicroversionsToEnvVars(cfg.Microversions)
-	if len(pairs) == 0 {
-		return nil, nil
+
+	envVars := []corev1.EnvVar{}
+	addIfNonEmpty := func(name, value string) {
+		if value != "" {
+			envVars = append(envVars, corev1.EnvVar{Name: name, Value: value})
+		}
 	}
-	envVars := make([]corev1.EnvVar, 0, len(pairs))
-	for _, p := range pairs {
+
+	// Core auth fields — always present for any auth_type.
+	addIfNonEmpty("OS_AUTH_URL", cfg.AuthOptions.IdentityEndpoint)
+	addIfNonEmpty("OS_REGION_NAME", cfg.RegionName)
+	addIfNonEmpty("OS_INTERFACE", cfg.Interface)
+	if cfg.Verify != nil && !*cfg.Verify {
+		envVars = append(envVars, corev1.EnvVar{Name: "OS_INSECURE", Value: "true"})
+	}
+
+	// Auth-method-specific fields.
+	switch cfg.AuthType {
+	case "v3applicationcredential":
+		addIfNonEmpty("OS_APPLICATION_CREDENTIAL_ID", cfg.AuthOptions.ApplicationCredentialID)
+		addIfNonEmpty("OS_APPLICATION_CREDENTIAL_SECRET", cfg.AuthOptions.ApplicationCredentialSecret)
+	default:
+		// v3password / password / empty: user + password + project scope.
+		addIfNonEmpty("OS_USERNAME", cfg.AuthOptions.Username)
+		addIfNonEmpty("OS_USERID", cfg.AuthOptions.UserID)
+		addIfNonEmpty("OS_PASSWORD", cfg.AuthOptions.Password)
+		addIfNonEmpty("OS_DOMAIN_NAME", cfg.AuthOptions.DomainName)
+		addIfNonEmpty("OS_USER_DOMAIN_NAME", cfg.AuthOptions.DomainName)
+		addIfNonEmpty("OS_PROJECT_NAME", cfg.AuthOptions.TenantName)
+		addIfNonEmpty("OS_TENANT_NAME", cfg.AuthOptions.TenantName)
+		addIfNonEmpty("OS_PROJECT_ID", cfg.AuthOptions.TenantID)
+	}
+
+	// Operator-configured microversion floor values.
+	for _, p := range utils.MicroversionsToEnvVars(cfg.Microversions) {
 		envVars = append(envVars, corev1.EnvVar{Name: p.Name, Value: p.Value})
 	}
+
 	return envVars, nil
 }
 
@@ -1126,19 +1163,21 @@ func (r *MigrationPlanReconciler) CreateJob(ctx context.Context,
 		})
 	}
 
-	// When the credential Secret is clouds.yaml-keyed, the per-service
-	// microversion floor values configured by the operator are not auto-loaded
-	// via EnvFrom (the only Secret key is "clouds.yaml"). Parse them out and
-	// inject OS_*_API_VERSION env vars explicitly so v2v-helper's
-	// pkg/common/microversion.Floor can honor the operator-configured floor on
-	// every service client.
+	// When the credential Secret is clouds.yaml-keyed, EnvFrom silently
+	// skips the "clouds.yaml" key (not a valid POSIX env var name), so the
+	// v2v-helper pod receives no OS_* env vars and authOptionsFromEnv fails
+	// immediately. Parse the clouds.yaml here and inject the full set of
+	// OS_* env vars (auth fields per auth_type, region/interface/TLS, and
+	// the per-service microversion floor values) explicitly. For legacy
+	// OS_*-keyed Secrets this is a no-op — those env vars are auto-loaded
+	// by EnvFrom.
 	if openstackSecretRef != "" {
-		microversionEnvVars, err := buildMicroversionEnvVars(ctx, r.Client, openstackSecretRef, openstackCloudName)
+		cloudsEnvVars, err := buildCloudsYAMLEnvVars(ctx, r.Client, openstackSecretRef, openstackCloudName)
 		if err != nil {
-			r.ctxlog.Info("could not derive OS_*_API_VERSION env vars from credential Secret; legacy OS_* path will still be honored via EnvFrom",
+			r.ctxlog.Info("could not derive OS_* env vars from credential Secret; legacy OS_* path will still be honored via EnvFrom",
 				"secret", openstackSecretRef, "err", err.Error())
 		} else {
-			envVars = append(envVars, microversionEnvVars...)
+			envVars = append(envVars, cloudsEnvVars...)
 		}
 	}