Skip to content

Commit ff5fd22

Browse files
gtherondclaude
andcommitted
[1952] fix: vpwned proxy writes Conditions; checkStatusSuccess prefers them
Devin found a real regression in the previous OR-based fallback at checkStatusSuccess: when the vpwned proxy's RevalidateCredentials endpoint writes OpenStackValidationStatus="Failed" (e.g. after a password rotation), Conditions still carry stale CredentialsValidated=True from the controller's last reconcile. The OR check (ready if either is True) returned True from stale Conditions and allowed migrations against known-bad credentials. Two-part fix: 1. vpwned proxy now writes Conditions alongside the flat fields. updateOpenstackValidationStatus mirrors validationStatus into utils.ConditionCredentialsValidated with an appropriate Reason: - Succeeded -> True, AuthSucceeded - Failed -> False, CredentialInvalidOrRevoked - Revalidating -> Unknown, ValidationStatusRevalidating Conditions and flat fields stay in sync regardless of which writer touched the resource last. 2. checkStatusSuccess updated semantics: Conditions are authoritative when present (now true for both controller and vpwned writes). Falls back to the legacy flat field only when the Conditions slice is entirely empty — that case still exists for pre-upgrade resources not yet re-reconciled. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0e65128 commit ff5fd22

4 files changed

Lines changed: 35 additions & 9 deletions

File tree

graphify-out/cache/ast/815e5b6373ace2a937287b033e1f82d306e5eb009378f15d83d1b4d0a5ef809f.json

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

graphify-out/cache/ast/ddb53e61dd66d7d56e521f01df4a6723e62066e37cf308c7c837dfeb184b8aaf.json

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

k8s/migration/internal/controller/migrationplan_controller.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1905,15 +1905,18 @@ func (r *MigrationPlanReconciler) checkStatusSuccess(ctx context.Context,
19051905
if !ok {
19061906
return false, errors.Wrap(err, "failed to convert credentials to OpenstackCreds")
19071907
}
1908-
// Prefer the Kubernetes Condition (CredentialsValidated=True). Fall
1909-
// back to the legacy flat OpenStackValidationStatus field for
1910-
// resources that haven't been re-reconciled by the upgraded
1911-
// OpenstackCreds controller yet — those resources still carry the
1912-
// old flat status but no Conditions slice, and we shouldn't block
1913-
// MigrationPlan reconciliation on the upgrade window.
1914-
conditionReady := meta.IsStatusConditionTrue(openstackCreds.Status.Conditions, utils.ConditionCredentialsValidated)
1915-
legacyReady := openstackCreds.Status.OpenStackValidationStatus == string(corev1.PodSucceeded)
1916-
if !conditionReady && !legacyReady {
1908+
// Conditions are authoritative when present (the controller and the
1909+
// vpwned proxy both write them alongside the flat status field).
1910+
// Fall back to the legacy flat field only when the Conditions slice
1911+
// is entirely empty — that case exists for pre-upgrade resources
1912+
// not yet re-reconciled by the upgraded OpenstackCreds controller.
1913+
var ready bool
1914+
if len(openstackCreds.Status.Conditions) > 0 {
1915+
ready = meta.IsStatusConditionTrue(openstackCreds.Status.Conditions, utils.ConditionCredentialsValidated)
1916+
} else {
1917+
ready = openstackCreds.Status.OpenStackValidationStatus == string(corev1.PodSucceeded)
1918+
}
1919+
if !ready {
19171920
return false, errors.Errorf("openstackcreds '%s' CR is not validated", openstackCreds.Name)
19181921
}
19191922
}

pkg/vpwned/server/vjailbreak_proxy.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,27 @@ func (p *vjailbreakProxy) updateOpenstackValidationStatus(ctx context.Context, n
438438
}
439439
creds.Status.OpenStackValidationStatus = validationStatus
440440
creds.Status.OpenStackValidationMessage = validationMessage
441+
442+
// Mirror the flat status into the Conditions slice. Without this, a
443+
// vpwned write of "Failed" (e.g. from RevalidateCredentials after a
444+
// password rotation) would leave stale CredentialsValidated=True from
445+
// the controller's last reconcile, and downstream checks that prefer
446+
// Conditions would allow migrations against known-bad credentials.
447+
switch validationStatus {
448+
case string(corev1.PodSucceeded):
449+
migrationutils.SetCondition(&creds.Status.Conditions,
450+
migrationutils.ConditionCredentialsValidated, metav1.ConditionTrue,
451+
migrationutils.ReasonAuthSucceeded, validationMessage)
452+
case constants.ValidationStatusFailed:
453+
migrationutils.SetCondition(&creds.Status.Conditions,
454+
migrationutils.ConditionCredentialsValidated, metav1.ConditionFalse,
455+
migrationutils.ReasonCredentialInvalidOrRevoked, validationMessage)
456+
case constants.ValidationStatusRevalidating:
457+
migrationutils.SetCondition(&creds.Status.Conditions,
458+
migrationutils.ConditionCredentialsValidated, metav1.ConditionUnknown,
459+
constants.ValidationStatusRevalidating, validationMessage)
460+
}
461+
441462
return p.K8sClient.Status().Update(ctx, creds)
442463
})
443464
}

0 commit comments

Comments
 (0)