Skip to content

Commit 0c159a0

Browse files
committed
🤖 fix: handle duplicate rollback license uploads as converged
Treat duplicate-license upload responses as converged success and persist `licenseLastApplied*`/Applied condition, so rollback rotations (A→B→A) do not loop on non-idempotent backend errors. Also adds a regression test for rollback duplicate-upload convergence. --- _Generated with `mux` • Model: `openai:gpt-5.3-codex` • Thinking: `xhigh` • Cost: `$1.21`_ <!-- mux-attribution: model=openai:gpt-5.3-codex thinking=xhigh costs=1.21 -->
1 parent b09da13 commit 0c159a0

2 files changed

Lines changed: 173 additions & 0 deletions

File tree

‎internal/controller/codercontrolplane_controller.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,23 @@ func (r *CoderControlPlaneReconciler) reconcileLicense(
651651
}
652652

653653
if err := r.LicenseUploader.AddLicense(ctx, nextStatus.URL, operatorToken, licenseJWT); err != nil {
654+
if isDuplicateLicenseUploadError(err) {
655+
now := metav1.Now()
656+
nextStatus.LicenseLastApplied = &now
657+
nextStatus.LicenseLastAppliedHash = licenseHash
658+
if err := setControlPlaneCondition(
659+
nextStatus,
660+
coderControlPlane.Generation,
661+
coderv1alpha1.CoderControlPlaneConditionLicenseApplied,
662+
metav1.ConditionTrue,
663+
licenseConditionReasonApplied,
664+
"Configured license already exists in coderd.",
665+
); err != nil {
666+
return ctrl.Result{}, err
667+
}
668+
return ctrl.Result{}, nil
669+
}
670+
654671
var sdkErr *codersdk.Error
655672
if errors.As(err, &sdkErr) {
656673
switch sdkErr.StatusCode() {
@@ -1053,6 +1070,30 @@ func (r *CoderControlPlaneReconciler) reconcileRequestsForLicenseSecret(
10531070
return requests
10541071
}
10551072

1073+
func isDuplicateLicenseUploadError(err error) bool {
1074+
var sdkErr *codersdk.Error
1075+
if !errors.As(err, &sdkErr) {
1076+
return false
1077+
}
1078+
1079+
message := strings.ToLower(strings.TrimSpace(sdkErr.Message + " " + sdkErr.Detail))
1080+
if message == "" {
1081+
return false
1082+
}
1083+
1084+
if strings.Contains(message, "licenses_jwt_key") {
1085+
return true
1086+
}
1087+
if strings.Contains(message, "duplicate key") {
1088+
return true
1089+
}
1090+
if strings.Contains(message, "already exists") {
1091+
return true
1092+
}
1093+
1094+
return false
1095+
}
1096+
10561097
func mergeControlPlaneStatusDelta(
10571098
baseStatus coderv1alpha1.CoderControlPlaneStatus,
10581099
nextStatus coderv1alpha1.CoderControlPlaneStatus,

‎internal/controller/codercontrolplane_controller_test.go‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ type licenseUploadCall struct {
5151

5252
type fakeLicenseUploader struct {
5353
err error
54+
addLicenseErrs []error
5455
hasAnyLicenseErr error
5556
hasAnyLicense *bool
5657
hasAnyLicenseCall int
@@ -63,6 +64,11 @@ func (f *fakeLicenseUploader) AddLicense(_ context.Context, coderURL, sessionTok
6364
sessionToken: sessionToken,
6465
licenseJWT: licenseJWT,
6566
})
67+
if len(f.addLicenseErrs) > 0 {
68+
err := f.addLicenseErrs[0]
69+
f.addLicenseErrs = f.addLicenseErrs[1:]
70+
return err
71+
}
6672
return f.err
6773
}
6874

@@ -779,6 +785,132 @@ func TestReconcile_LicenseRotationUploadsNewSecretValue(t *testing.T) {
779785
}
780786
}
781787

788+
func TestReconcile_LicenseRollbackDuplicateUploadConverges(t *testing.T) {
789+
ctx := context.Background()
790+
791+
licenseSecret := &corev1.Secret{
792+
ObjectMeta: metav1.ObjectMeta{Name: "test-license-rollback-secret", Namespace: "default"},
793+
Data: map[string][]byte{
794+
coderv1alpha1.DefaultLicenseSecretKey: []byte("license-jwt-a"),
795+
},
796+
}
797+
if err := k8sClient.Create(ctx, licenseSecret); err != nil {
798+
t.Fatalf("create license secret: %v", err)
799+
}
800+
t.Cleanup(func() {
801+
_ = k8sClient.Delete(ctx, licenseSecret)
802+
})
803+
804+
cp := &coderv1alpha1.CoderControlPlane{
805+
ObjectMeta: metav1.ObjectMeta{Name: "test-license-rollback", Namespace: "default"},
806+
Spec: coderv1alpha1.CoderControlPlaneSpec{
807+
ExtraEnv: []corev1.EnvVar{{
808+
Name: "CODER_PG_CONNECTION_URL",
809+
Value: "postgres://example/license-rollback",
810+
}},
811+
LicenseSecretRef: &coderv1alpha1.SecretKeySelector{Name: licenseSecret.Name},
812+
},
813+
}
814+
if err := k8sClient.Create(ctx, cp); err != nil {
815+
t.Fatalf("create test CoderControlPlane: %v", err)
816+
}
817+
t.Cleanup(func() {
818+
_ = k8sClient.Delete(ctx, cp)
819+
})
820+
821+
duplicateErr := codersdk.NewTestError(http.StatusInternalServerError, http.MethodPost, "/api/v2/licenses")
822+
duplicateErr.Message = "duplicate key value violates unique constraint \"licenses_jwt_key\""
823+
uploader := &fakeLicenseUploader{addLicenseErrs: []error{nil, nil, duplicateErr}}
824+
provisioner := &fakeOperatorAccessProvisioner{token: "operator-token-license-rollback"}
825+
r := &controller.CoderControlPlaneReconciler{
826+
Client: k8sClient,
827+
Scheme: scheme,
828+
OperatorAccessProvisioner: provisioner,
829+
LicenseUploader: uploader,
830+
}
831+
832+
if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: cp.Name, Namespace: cp.Namespace}}); err != nil {
833+
t.Fatalf("first reconcile control plane: %v", err)
834+
}
835+
deployment := &appsv1.Deployment{}
836+
if err := k8sClient.Get(ctx, types.NamespacedName{Name: cp.Name, Namespace: cp.Namespace}, deployment); err != nil {
837+
t.Fatalf("get reconciled deployment: %v", err)
838+
}
839+
deployment.Status.ReadyReplicas = 1
840+
deployment.Status.Replicas = 1
841+
if err := k8sClient.Status().Update(ctx, deployment); err != nil {
842+
t.Fatalf("update deployment status: %v", err)
843+
}
844+
845+
if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: cp.Name, Namespace: cp.Namespace}}); err != nil {
846+
t.Fatalf("second reconcile control plane: %v", err)
847+
}
848+
if len(uploader.calls) != 1 {
849+
t.Fatalf("expected initial upload call count 1, got %d", len(uploader.calls))
850+
}
851+
852+
reconciledAfterInitial := &coderv1alpha1.CoderControlPlane{}
853+
if err := k8sClient.Get(ctx, types.NamespacedName{Name: cp.Name, Namespace: cp.Namespace}, reconciledAfterInitial); err != nil {
854+
t.Fatalf("get reconciled control plane after initial apply: %v", err)
855+
}
856+
hashA := reconciledAfterInitial.Status.LicenseLastAppliedHash
857+
if hashA == "" {
858+
t.Fatalf("expected hash after initial apply")
859+
}
860+
861+
secretToRotate := &corev1.Secret{}
862+
if err := k8sClient.Get(ctx, types.NamespacedName{Name: licenseSecret.Name, Namespace: licenseSecret.Namespace}, secretToRotate); err != nil {
863+
t.Fatalf("get license secret: %v", err)
864+
}
865+
secretToRotate.Data[coderv1alpha1.DefaultLicenseSecretKey] = []byte("license-jwt-b")
866+
if err := k8sClient.Update(ctx, secretToRotate); err != nil {
867+
t.Fatalf("rotate license to B: %v", err)
868+
}
869+
if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: cp.Name, Namespace: cp.Namespace}}); err != nil {
870+
t.Fatalf("third reconcile control plane: %v", err)
871+
}
872+
873+
reconciledAfterB := &coderv1alpha1.CoderControlPlane{}
874+
if err := k8sClient.Get(ctx, types.NamespacedName{Name: cp.Name, Namespace: cp.Namespace}, reconciledAfterB); err != nil {
875+
t.Fatalf("get reconciled control plane after B apply: %v", err)
876+
}
877+
if reconciledAfterB.Status.LicenseLastAppliedHash == hashA {
878+
t.Fatalf("expected hash to change after applying B")
879+
}
880+
881+
secretToRotateBack := &corev1.Secret{}
882+
if err := k8sClient.Get(ctx, types.NamespacedName{Name: licenseSecret.Name, Namespace: licenseSecret.Namespace}, secretToRotateBack); err != nil {
883+
t.Fatalf("get license secret for rollback: %v", err)
884+
}
885+
secretToRotateBack.Data[coderv1alpha1.DefaultLicenseSecretKey] = []byte("license-jwt-a")
886+
if err := k8sClient.Update(ctx, secretToRotateBack); err != nil {
887+
t.Fatalf("rollback license to A: %v", err)
888+
}
889+
890+
result, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: cp.Name, Namespace: cp.Namespace}})
891+
if err != nil {
892+
t.Fatalf("fourth reconcile control plane: %v", err)
893+
}
894+
if result.RequeueAfter > 0 {
895+
t.Fatalf("expected duplicate rollback upload handling to converge without requeue, got %+v", result)
896+
}
897+
if len(uploader.calls) != 3 {
898+
t.Fatalf("expected three upload attempts across A->B->A rollback, got %d", len(uploader.calls))
899+
}
900+
901+
reconciledAfterRollback := &coderv1alpha1.CoderControlPlane{}
902+
if err := k8sClient.Get(ctx, types.NamespacedName{Name: cp.Name, Namespace: cp.Namespace}, reconciledAfterRollback); err != nil {
903+
t.Fatalf("get reconciled control plane after rollback: %v", err)
904+
}
905+
if reconciledAfterRollback.Status.LicenseLastAppliedHash != hashA {
906+
t.Fatalf("expected rollback to converge to original hash %q, got %q", hashA, reconciledAfterRollback.Status.LicenseLastAppliedHash)
907+
}
908+
licenseCondition := findCondition(t, reconciledAfterRollback.Status.Conditions, coderv1alpha1.CoderControlPlaneConditionLicenseApplied)
909+
if licenseCondition.Status != metav1.ConditionTrue {
910+
t.Fatalf("expected license condition true after duplicate rollback handling, got %q", licenseCondition.Status)
911+
}
912+
}
913+
782914
func TestReconcile_LicenseNotSupportedSetsConditionWithoutRequeue(t *testing.T) {
783915
ctx := context.Background()
784916

0 commit comments

Comments
 (0)