Skip to content

Commit 36944c8

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 6b4be09 commit 36944c8

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
@@ -1864,15 +1864,18 @@ func (r *MigrationPlanReconciler) checkStatusSuccess(ctx context.Context,
18641864
if !ok {
18651865
return false, errors.Wrap(err, "failed to convert credentials to OpenstackCreds")
18661866
}
1867-
// Prefer the Kubernetes Condition (CredentialsValidated=True). Fall
1868-
// back to the legacy flat OpenStackValidationStatus field for
1869-
// resources that haven't been re-reconciled by the upgraded
1870-
// OpenstackCreds controller yet — those resources still carry the
1871-
// old flat status but no Conditions slice, and we shouldn't block
1872-
// MigrationPlan reconciliation on the upgrade window.
1873-
conditionReady := meta.IsStatusConditionTrue(openstackCreds.Status.Conditions, utils.ConditionCredentialsValidated)
1874-
legacyReady := openstackCreds.Status.OpenStackValidationStatus == string(corev1.PodSucceeded)
1875-
if !conditionReady && !legacyReady {
1867+
// Conditions are authoritative when present (the controller and the
1868+
// vpwned proxy both write them alongside the flat status field).
1869+
// Fall back to the legacy flat field only when the Conditions slice
1870+
// is entirely empty — that case exists for pre-upgrade resources
1871+
// not yet re-reconciled by the upgraded OpenstackCreds controller.
1872+
var ready bool
1873+
if len(openstackCreds.Status.Conditions) > 0 {
1874+
ready = meta.IsStatusConditionTrue(openstackCreds.Status.Conditions, utils.ConditionCredentialsValidated)
1875+
} else {
1876+
ready = openstackCreds.Status.OpenStackValidationStatus == string(corev1.PodSucceeded)
1877+
}
1878+
if !ready {
18761879
return false, errors.Errorf("openstackcreds '%s' CR is not validated", openstackCreds.Name)
18771880
}
18781881
}

pkg/vpwned/server/vjailbreak_proxy.go

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

0 commit comments

Comments
 (0)