Skip to content

Commit 2b5bbff

Browse files
authored
Sort automount configmaps and secrets to ensure deterministic ordering (#1578)
* Sort automount configmaps and secrets to ensure deterministic ordering * Add tests for sorting automount secrets and confimaps Signed-off-by: David Kwon <dakwon@redhat.com> Assisted-by: Claude
1 parent 6d3ae7a commit 2b5bbff

File tree

3 files changed

+260
-0
lines changed

3 files changed

+260
-0
lines changed

controllers/workspace/devworkspace_controller_test.go

Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,264 @@ var _ = Describe("DevWorkspace Controller", func() {
580580
}
581581
})
582582

583+
It("Sorts automount secrets in consistent order", func() {
584+
By("Creating automount secrets in non-sorted order")
585+
secretZ := generateSecret("secret-z", corev1.SecretTypeOpaque)
586+
secretZ.Labels[constants.DevWorkspaceMountLabel] = "true"
587+
secretZ.Annotations[constants.DevWorkspaceMountPathAnnotation] = "/secret/z"
588+
createObject(secretZ)
589+
defer deleteObject(secretZ)
590+
591+
secretA := generateSecret("secret-a", corev1.SecretTypeOpaque)
592+
secretA.Labels[constants.DevWorkspaceMountLabel] = "true"
593+
secretA.Annotations[constants.DevWorkspaceMountPathAnnotation] = "/secret/a"
594+
createObject(secretA)
595+
defer deleteObject(secretA)
596+
597+
secretM := generateSecret("secret-m", corev1.SecretTypeOpaque)
598+
secretM.Labels[constants.DevWorkspaceMountLabel] = "true"
599+
secretM.Annotations[constants.DevWorkspaceMountPathAnnotation] = "/secret/m"
600+
createObject(secretM)
601+
defer deleteObject(secretM)
602+
603+
// Create secrets with numeric suffixes to test numeric sorting
604+
secret15 := generateSecret("automount-secret-15", corev1.SecretTypeOpaque)
605+
secret15.Labels[constants.DevWorkspaceMountLabel] = "true"
606+
secret15.Annotations[constants.DevWorkspaceMountPathAnnotation] = "/secret/15"
607+
createObject(secret15)
608+
defer deleteObject(secret15)
609+
610+
secret02 := generateSecret("automount-secret-02", corev1.SecretTypeOpaque)
611+
secret02.Labels[constants.DevWorkspaceMountLabel] = "true"
612+
secret02.Annotations[constants.DevWorkspaceMountPathAnnotation] = "/secret/02"
613+
createObject(secret02)
614+
defer deleteObject(secret02)
615+
616+
secret08 := generateSecret("automount-secret-08", corev1.SecretTypeOpaque)
617+
secret08.Labels[constants.DevWorkspaceMountLabel] = "true"
618+
secret08.Annotations[constants.DevWorkspaceMountPathAnnotation] = "/secret/08"
619+
createObject(secret08)
620+
defer deleteObject(secret08)
621+
622+
By("Creating DevWorkspace")
623+
createDevWorkspace(devWorkspaceName, "test-devworkspace.yaml")
624+
devworkspace := getExistingDevWorkspace(devWorkspaceName)
625+
workspaceID := devworkspace.Status.DevWorkspaceId
626+
627+
By("Manually making Routing ready to continue")
628+
markRoutingReady(testURL, common.DevWorkspaceRoutingName(workspaceID))
629+
630+
deploy := &appsv1.Deployment{}
631+
deployNN := namespacedName(common.DeploymentName(workspaceID), testNamespace)
632+
Eventually(func() error {
633+
return k8sClient.Get(ctx, deployNN, deploy)
634+
}, timeout, interval).Should(Succeed(), "Getting workspace deployment from cluster")
635+
636+
By("Verifying secrets are sorted in deployment volumes")
637+
638+
expectedSecretNames := []string{"secret-a", "secret-m", "secret-z", "automount-secret-02", "automount-secret-08", "automount-secret-15"}
639+
var automountVolumes []corev1.Volume
640+
for _, vol := range deploy.Spec.Template.Spec.Volumes {
641+
if vol.Secret != nil {
642+
for _, name := range expectedSecretNames {
643+
if vol.Name == name && vol.Secret.SecretName == name {
644+
automountVolumes = append(automountVolumes, vol)
645+
break
646+
}
647+
}
648+
}
649+
}
650+
651+
// Verify we found all expected volumes
652+
Expect(automountVolumes).Should(HaveLen(6), "Should have 6 automount secret volumes")
653+
654+
// Verify volumes are in sorted order (alphabetically by volume name, which matches secret name)
655+
expectedOrder := []string{
656+
"automount-secret-02",
657+
"automount-secret-08",
658+
"automount-secret-15",
659+
"secret-a",
660+
"secret-m",
661+
"secret-z",
662+
}
663+
664+
actualOrder := make([]string, len(automountVolumes))
665+
for i, vol := range automountVolumes {
666+
actualOrder[i] = vol.Name
667+
}
668+
669+
Expect(actualOrder).Should(Equal(expectedOrder), "Automount secret volumes should be sorted alphabetically by volume name")
670+
})
671+
672+
It("Sorts automount configmaps in consistent order", func() {
673+
By("Creating automount configmaps in non-sorted order")
674+
configmapZ := generateConfigMap("configmap-z")
675+
configmapZ.Labels[constants.DevWorkspaceMountLabel] = "true"
676+
configmapZ.Annotations[constants.DevWorkspaceMountPathAnnotation] = "/configmap/z"
677+
createObject(configmapZ)
678+
defer deleteObject(configmapZ)
679+
680+
configmapA := generateConfigMap("configmap-a")
681+
configmapA.Labels[constants.DevWorkspaceMountLabel] = "true"
682+
configmapA.Annotations[constants.DevWorkspaceMountPathAnnotation] = "/configmap/a"
683+
createObject(configmapA)
684+
defer deleteObject(configmapA)
685+
686+
configmapM := generateConfigMap("configmap-m")
687+
configmapM.Labels[constants.DevWorkspaceMountLabel] = "true"
688+
configmapM.Annotations[constants.DevWorkspaceMountPathAnnotation] = "/configmap/m"
689+
createObject(configmapM)
690+
defer deleteObject(configmapM)
691+
692+
// Create configmaps with numeric suffixes to test numeric sorting
693+
configmap15 := generateConfigMap("automount-cm-15")
694+
configmap15.Labels[constants.DevWorkspaceMountLabel] = "true"
695+
configmap15.Annotations[constants.DevWorkspaceMountPathAnnotation] = "/configmap/15"
696+
createObject(configmap15)
697+
defer deleteObject(configmap15)
698+
699+
configmap02 := generateConfigMap("automount-cm-02")
700+
configmap02.Labels[constants.DevWorkspaceMountLabel] = "true"
701+
configmap02.Annotations[constants.DevWorkspaceMountPathAnnotation] = "/configmap/02"
702+
createObject(configmap02)
703+
defer deleteObject(configmap02)
704+
705+
configmap08 := generateConfigMap("automount-cm-08")
706+
configmap08.Labels[constants.DevWorkspaceMountLabel] = "true"
707+
configmap08.Annotations[constants.DevWorkspaceMountPathAnnotation] = "/configmap/08"
708+
createObject(configmap08)
709+
defer deleteObject(configmap08)
710+
711+
By("Creating DevWorkspace")
712+
createDevWorkspace(devWorkspaceName, "test-devworkspace.yaml")
713+
devworkspace := getExistingDevWorkspace(devWorkspaceName)
714+
workspaceID := devworkspace.Status.DevWorkspaceId
715+
716+
By("Manually making Routing ready to continue")
717+
markRoutingReady(testURL, common.DevWorkspaceRoutingName(workspaceID))
718+
719+
deploy := &appsv1.Deployment{}
720+
deployNN := namespacedName(common.DeploymentName(workspaceID), testNamespace)
721+
Eventually(func() error {
722+
return k8sClient.Get(ctx, deployNN, deploy)
723+
}, timeout, interval).Should(Succeed(), "Getting workspace deployment from cluster")
724+
725+
By("Verifying configmaps are sorted in deployment volumes")
726+
727+
expectedConfigMapNames := []string{"configmap-a", "configmap-m", "configmap-z", "automount-cm-02", "automount-cm-08", "automount-cm-15"}
728+
var automountVolumes []corev1.Volume
729+
for _, vol := range deploy.Spec.Template.Spec.Volumes {
730+
if vol.ConfigMap != nil {
731+
for _, name := range expectedConfigMapNames {
732+
if vol.Name == name && vol.ConfigMap.Name == name {
733+
automountVolumes = append(automountVolumes, vol)
734+
break
735+
}
736+
}
737+
}
738+
}
739+
740+
// Verify we found all expected volumes
741+
Expect(automountVolumes).Should(HaveLen(6), "Should have 6 automount configmap volumes")
742+
743+
// Verify volumes are in sorted order (alphabetically by volume name, which matches configmap name)
744+
expectedOrder := []string{
745+
"automount-cm-02",
746+
"automount-cm-08",
747+
"automount-cm-15",
748+
"configmap-a",
749+
"configmap-m",
750+
"configmap-z",
751+
}
752+
753+
actualOrder := make([]string, len(automountVolumes))
754+
for i, vol := range automountVolumes {
755+
actualOrder[i] = vol.Name
756+
}
757+
758+
Expect(actualOrder).Should(Equal(expectedOrder), "Automount configmap volumes should be sorted alphabetically by volume name")
759+
})
760+
761+
It("Sorts mixed automount secrets and configmaps together", func() {
762+
By("Creating automount secrets and configmaps in non-sorted order")
763+
secretB := generateSecret("secret-b", corev1.SecretTypeOpaque)
764+
secretB.Labels[constants.DevWorkspaceMountLabel] = "true"
765+
secretB.Annotations[constants.DevWorkspaceMountPathAnnotation] = "/secret/b"
766+
createObject(secretB)
767+
defer deleteObject(secretB)
768+
769+
secretD := generateSecret("secret-d", corev1.SecretTypeOpaque)
770+
secretD.Labels[constants.DevWorkspaceMountLabel] = "true"
771+
secretD.Annotations[constants.DevWorkspaceMountPathAnnotation] = "/secret/d"
772+
createObject(secretD)
773+
defer deleteObject(secretD)
774+
775+
configmapA := generateConfigMap("configmap-a")
776+
configmapA.Labels[constants.DevWorkspaceMountLabel] = "true"
777+
configmapA.Annotations[constants.DevWorkspaceMountPathAnnotation] = "/configmap/a"
778+
createObject(configmapA)
779+
defer deleteObject(configmapA)
780+
781+
configmapC := generateConfigMap("configmap-c")
782+
configmapC.Labels[constants.DevWorkspaceMountLabel] = "true"
783+
configmapC.Annotations[constants.DevWorkspaceMountPathAnnotation] = "/configmap/c"
784+
createObject(configmapC)
785+
defer deleteObject(configmapC)
786+
787+
By("Creating DevWorkspace")
788+
createDevWorkspace(devWorkspaceName, "test-devworkspace.yaml")
789+
devworkspace := getExistingDevWorkspace(devWorkspaceName)
790+
workspaceID := devworkspace.Status.DevWorkspaceId
791+
792+
By("Manually making Routing ready to continue")
793+
markRoutingReady(testURL, common.DevWorkspaceRoutingName(workspaceID))
794+
795+
deploy := &appsv1.Deployment{}
796+
deployNN := namespacedName(common.DeploymentName(workspaceID), testNamespace)
797+
Eventually(func() error {
798+
return k8sClient.Get(ctx, deployNN, deploy)
799+
}, timeout, interval).Should(Succeed(), "Getting workspace deployment from cluster")
800+
801+
By("Verifying secrets and configmaps are sorted together")
802+
expectedNames := []string{"configmap-a", "configmap-c", "secret-b", "secret-d"}
803+
var automountVolumes []corev1.Volume
804+
for _, vol := range deploy.Spec.Template.Spec.Volumes {
805+
if vol.Secret != nil {
806+
for _, name := range expectedNames {
807+
if vol.Name == name && vol.Secret.SecretName == name {
808+
automountVolumes = append(automountVolumes, vol)
809+
break
810+
}
811+
}
812+
}
813+
if vol.ConfigMap != nil {
814+
for _, name := range expectedNames {
815+
if vol.Name == name && vol.ConfigMap.Name == name {
816+
automountVolumes = append(automountVolumes, vol)
817+
break
818+
}
819+
}
820+
}
821+
}
822+
823+
Expect(automountVolumes).Should(HaveLen(4), "Should have 4 automount volumes (2 secrets + 2 configmaps)")
824+
825+
// All volumes should be sorted together alphabetically
826+
expectedOrder := []string{
827+
"configmap-a",
828+
"configmap-c",
829+
"secret-b",
830+
"secret-d",
831+
}
832+
833+
actualOrder := make([]string, len(automountVolumes))
834+
for i, vol := range automountVolumes {
835+
actualOrder[i] = vol.Name
836+
}
837+
838+
Expect(actualOrder).Should(Equal(expectedOrder), "Automount volumes (secrets and configmaps) should be sorted together alphabetically")
839+
})
840+
583841
It("Detects changes to automount resources and reconciles", func() {
584842
// NOTE: timeout for this test is reduced, as eventually DWO will reconcile the workspace by coincidence and notice
585843
// the automount secret.

pkg/provision/automount/configmap.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ func getDevWorkspaceConfigmaps(namespace string, api sync.ClusterAPI) (*Resource
3636
}); err != nil {
3737
return nil, err
3838
}
39+
sortConfigmaps(configmaps.Items)
3940
var allAutoMountResouces []Resources
4041
for _, configmap := range configmaps.Items {
4142
if msg := checkAutomountVolumeForPotentialError(&configmap); msg != "" {

pkg/provision/automount/secret.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ func getDevWorkspaceSecrets(namespace string, api sync.ClusterAPI) (*Resources,
3636
}); err != nil {
3737
return nil, err
3838
}
39+
sortSecrets(secrets.Items)
3940
var allAutoMountResouces []Resources
4041
for _, secret := range secrets.Items {
4142
if msg := checkAutomountVolumeForPotentialError(&secret); msg != "" {

0 commit comments

Comments
 (0)