Skip to content

Commit 78048f0

Browse files
ThomasK33ThomasK33
committed
fix: harden provisioner key drift recovery
1 parent 9ed7cd6 commit 78048f0

2 files changed

Lines changed: 92 additions & 9 deletions

File tree

‎internal/controller/coderprovisioner_controller.go‎

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -209,11 +209,49 @@ func (r *CoderProvisionerReconciler) Reconcile(ctx context.Context, req ctrl.Req
209209
}
210210
keyMaterial = response.Key
211211
if keyMaterial == "" {
212-
setCondition(provisioner, coderv1alpha1.CoderProvisionerConditionProvisionerKeyReady,
213-
metav1.ConditionFalse, "ProvisionerKeyFailed",
214-
fmt.Sprintf("Provisioner key %q returned empty material after drift rotation", keyName))
215-
_ = r.Status().Update(ctx, provisioner)
216-
return ctrl.Result{}, fmt.Errorf("assertion failed: provisioner key returned empty material after drift rotation")
212+
log.Info("drift-rotated key exists in coderd but returned no plaintext, rotating to recover",
213+
"keyName", keyName)
214+
215+
if deleteErr := r.BootstrapClient.DeleteProvisionerKey(
216+
ctx, controlPlane.Status.URL, sessionToken, organizationName, keyName,
217+
); deleteErr != nil {
218+
setCondition(provisioner, coderv1alpha1.CoderProvisionerConditionProvisionerKeyReady,
219+
metav1.ConditionFalse, "ProvisionerKeyFailed",
220+
fmt.Sprintf("Failed to delete provisioner key %q for drift recovery", keyName))
221+
_ = r.Status().Update(ctx, provisioner)
222+
return ctrl.Result{}, fmt.Errorf("delete provisioner key %q for drift recovery: %w", keyName, deleteErr)
223+
}
224+
rotated, rotateErr := r.BootstrapClient.EnsureProvisionerKey(ctx, coderbootstrap.EnsureProvisionerKeyRequest{
225+
CoderURL: controlPlane.Status.URL,
226+
SessionToken: sessionToken,
227+
OrganizationName: organizationName,
228+
KeyName: keyName,
229+
Tags: provisioner.Spec.Tags,
230+
})
231+
if rotateErr != nil {
232+
setCondition(provisioner, coderv1alpha1.CoderProvisionerConditionProvisionerKeyReady,
233+
metav1.ConditionFalse, "ProvisionerKeyFailed",
234+
fmt.Sprintf("Failed to recreate provisioner key %q after drift recovery", keyName))
235+
_ = r.Status().Update(ctx, provisioner)
236+
return ctrl.Result{}, fmt.Errorf("recreate provisioner key %q after drift recovery: %w", keyName, rotateErr)
237+
}
238+
if rotated.OrganizationID != uuid.Nil {
239+
organizationID = rotated.OrganizationID.String()
240+
}
241+
if rotated.KeyID != uuid.Nil {
242+
provisionerKeyID = rotated.KeyID.String()
243+
}
244+
if rotated.KeyName != "" {
245+
provisionerKeyName = rotated.KeyName
246+
}
247+
keyMaterial = rotated.Key
248+
if keyMaterial == "" {
249+
setCondition(provisioner, coderv1alpha1.CoderProvisionerConditionProvisionerKeyReady,
250+
metav1.ConditionFalse, "ProvisionerKeyFailed",
251+
fmt.Sprintf("Provisioner key %q returned empty material after drift recovery rotation", keyName))
252+
_ = r.Status().Update(ctx, provisioner)
253+
return ctrl.Result{}, fmt.Errorf("assertion failed: provisioner key %q returned empty material after drift recovery rotation", keyName)
254+
}
217255
}
218256
appliedOrgName = organizationName
219257
appliedTagsHash = desiredTagsHash
@@ -922,6 +960,18 @@ func provisionerKeyConfig(provisioner *coderv1alpha1.CoderProvisioner) (string,
922960
keyName = provisioner.Name
923961
}
924962

963+
const maxKeyNameLength = 128
964+
if len(keyName) > maxKeyNameLength {
965+
hasher := fnv.New32a()
966+
_, _ = hasher.Write([]byte(keyName))
967+
suffix := fmt.Sprintf("%08x", hasher.Sum32())
968+
available := maxKeyNameLength - len(suffix) - 1
969+
if available < 1 {
970+
available = 1
971+
}
972+
keyName = fmt.Sprintf("%s-%s", keyName[:available], suffix)
973+
}
974+
925975
secretName := provisioner.Spec.Key.SecretName
926976
if secretName == "" {
927977
const secretNameSuffix = "-provisioner-key"

‎internal/controller/coderprovisioner_controller_test.go‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,23 @@ func expectedProvisionerServiceAccountName(name string) string {
117117
return fmt.Sprintf("%s-%s%s", name[:available], hashSuffix, suffix)
118118
}
119119

120+
func expectedProvisionerKeyName(name string) string {
121+
const maxKeyNameLength = 128
122+
if len(name) <= maxKeyNameLength {
123+
return name
124+
}
125+
126+
hasher := fnv.New32a()
127+
_, _ = hasher.Write([]byte(name))
128+
suffix := fmt.Sprintf("%08x", hasher.Sum32())
129+
available := maxKeyNameLength - len(suffix) - 1
130+
if available < 1 {
131+
available = 1
132+
}
133+
134+
return fmt.Sprintf("%s-%s", name[:available], suffix)
135+
}
136+
120137
func reconcileProvisioner(ctx context.Context, t *testing.T, reconciler *controller.CoderProvisionerReconciler, namespacedName types.NamespacedName) {
121138
t.Helper()
122139

@@ -681,6 +698,7 @@ func TestCoderProvisionerReconciler_TagsDrift(t *testing.T) {
681698
bootstrapClient := &fakeBootstrapClient{
682699
provisionerKeyResponses: []coderbootstrap.EnsureProvisionerKeyResponse{
683700
{KeyName: "tags-drift-key", Key: "initial-key-material"},
701+
{KeyName: "tags-drift-key", Key: ""},
684702
{KeyName: "tags-drift-key", Key: "rotated-key-material"},
685703
},
686704
}
@@ -700,8 +718,11 @@ func TestCoderProvisionerReconciler_TagsDrift(t *testing.T) {
700718

701719
reconcileProvisioner(ctx, t, reconciler, request)
702720

703-
require.Equal(t, 2, bootstrapClient.provisionerKeyCalls)
704-
require.GreaterOrEqual(t, bootstrapClient.deleteKeyCalls, 1)
721+
require.Equal(t, 3, bootstrapClient.provisionerKeyCalls)
722+
require.Equal(t, 2, bootstrapClient.deleteKeyCalls)
723+
require.Len(t, bootstrapClient.deleteKeyRequests, 2)
724+
require.Equal(t, "tags-drift-key", bootstrapClient.deleteKeyRequests[0].KeyName)
725+
require.Equal(t, "tags-drift-key", bootstrapClient.deleteKeyRequests[1].KeyName)
705726

706727
keySecret := &corev1.Secret{}
707728
require.NoError(t, k8sClient.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("%s-provisioner-key", provisioner.Name), Namespace: provisioner.Namespace}, keySecret))
@@ -917,11 +938,15 @@ func TestCoderProvisionerReconciler_LongNameTruncation(t *testing.T) {
917938
controlPlane := createTestControlPlane(ctx, t, namespace, "controlplane-longname", "https://coder.example.com")
918939
bootstrapSecret := createBootstrapSecret(ctx, t, namespace, "bootstrap-creds", coderv1alpha1.DefaultTokenSecretKey, "session-token")
919940

920-
provisionerName := strings.Repeat("a", 70)
941+
provisionerName := strings.Repeat("a", 180)
921942
deploymentCandidateName := fmt.Sprintf("provisioner-%s", provisionerName)
922943
serviceAccountCandidateName := fmt.Sprintf("%s-provisioner", provisionerName)
944+
keyNameCandidate := provisionerName
945+
expectedKeyName := expectedProvisionerKeyName(provisionerName)
923946
require.Greater(t, len(deploymentCandidateName), 63)
924947
require.Greater(t, len(serviceAccountCandidateName), 63)
948+
require.Greater(t, len(keyNameCandidate), 128)
949+
require.Len(t, expectedKeyName, 128)
925950

926951
provisioner := &coderv1alpha1.CoderProvisioner{
927952
ObjectMeta: metav1.ObjectMeta{Name: provisionerName, Namespace: namespace},
@@ -939,7 +964,7 @@ func TestCoderProvisionerReconciler_LongNameTruncation(t *testing.T) {
939964

940965
bootstrapClient := &fakeBootstrapClient{
941966
provisionerKeyResponses: []coderbootstrap.EnsureProvisionerKeyResponse{{
942-
KeyName: provisionerName,
967+
KeyName: expectedKeyName,
943968
Key: "provisioner-key-material",
944969
}},
945970
}
@@ -954,6 +979,14 @@ func TestCoderProvisionerReconciler_LongNameTruncation(t *testing.T) {
954979
require.LessOrEqual(t, len(deploymentName), 63)
955980
require.LessOrEqual(t, len(serviceAccountName), 63)
956981

982+
require.Len(t, bootstrapClient.provisionerKeyRequests, 1)
983+
require.Equal(t, expectedKeyName, bootstrapClient.provisionerKeyRequests[0].KeyName)
984+
985+
reconciledProvisioner := &coderv1alpha1.CoderProvisioner{}
986+
require.NoError(t, k8sClient.Get(ctx, request, reconciledProvisioner))
987+
require.Equal(t, expectedKeyName, reconciledProvisioner.Status.ProvisionerKeyName)
988+
require.LessOrEqual(t, len(reconciledProvisioner.Status.ProvisionerKeyName), 128)
989+
957990
deployment := &appsv1.Deployment{}
958991
require.NoError(t, k8sClient.Get(ctx, types.NamespacedName{Name: deploymentName, Namespace: namespace}, deployment))
959992

0 commit comments

Comments
 (0)