fix: eventsource: deterministic volume/volumeMount order (#2811)

Signed-off-by: Tommi Hovi <tommi.hovi@gmail.com>
argoproj · Sep 25, 2023 · 5b8c754 · 5b8c754
1 parent 11d9aef
commit 5b8c754
Show file tree

Hide file tree

Showing 4 changed files with 140 additions and 0 deletions.
diff --git a/controllers/eventsource/resource.go b/controllers/eventsource/resource.go
@@ -5,6 +5,7 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
+	"sort"
 
 	"github.com/imdario/mergo"
 	"go.uber.org/zap"
@@ -268,6 +269,14 @@ func buildDeployment(args *AdaptorArgs, eventBus *eventbusv1alpha1.EventBus) (*a
 	volumeMounts = append(volumeMounts, volCofigMapMounts...)
 	volumes = append(volumes, volConfigMaps...)
 
+	// Order volumes and volumemounts based on name to make the order deterministic
+	sort.Slice(volumes, func(i, j int) bool {
+		return volumes[i].Name < volumes[j].Name
+	})
+	sort.Slice(volumeMounts, func(i, j int) bool {
+		return volumeMounts[i].Name < volumeMounts[j].Name
+	})
+
 	deploymentSpec.Template.Spec.Containers[0].Env = append(deploymentSpec.Template.Spec.Containers[0].Env, env...)
 	deploymentSpec.Template.Spec.Containers[0].VolumeMounts = append(deploymentSpec.Template.Spec.Containers[0].VolumeMounts, volumeMounts...)
 	deploymentSpec.Template.Spec.Volumes = append(deploymentSpec.Template.Spec.Volumes, volumes...)

diff --git a/controllers/eventsource/resource_test.go b/controllers/eventsource/resource_test.go
@@ -120,6 +120,91 @@ func Test_BuildDeployment(t *testing.T) {
 		assert.True(t, hasTLSSecretVolume)
 		assert.True(t, hasTLSSecretVolumeMount)
 	})
+
+	t.Run("test secret volume and volumemount order deterministic", func(t *testing.T) {
+		args := &AdaptorArgs{
+			Image:       testImage,
+			EventSource: testEventSource,
+			Labels:      testLabels,
+		}
+
+		webhooksWithSecrets := map[string]v1alpha1.WebhookEventSource{
+			"webhook4": {
+				WebhookContext: v1alpha1.WebhookContext{
+					URL:      "http://a.b",
+					Endpoint: "/webhook4",
+					Port:     "1234",
+					AuthSecret: &corev1.SecretKeySelector{
+						LocalObjectReference: corev1.LocalObjectReference{Name: "webhook4"},
+						Key:                  "secret",
+					},
+				},
+			},
+			"webhook3": {
+				WebhookContext: v1alpha1.WebhookContext{
+					URL:      "http://a.b",
+					Endpoint: "/webhook3",
+					Port:     "1234",
+					AuthSecret: &corev1.SecretKeySelector{
+						LocalObjectReference: corev1.LocalObjectReference{Name: "webhook3"},
+						Key:                  "secret",
+					},
+				},
+			},
+			"webhook1": {
+				WebhookContext: v1alpha1.WebhookContext{
+					URL:      "http://a.b",
+					Endpoint: "/webhook1",
+					Port:     "1234",
+					AuthSecret: &corev1.SecretKeySelector{
+						LocalObjectReference: corev1.LocalObjectReference{Name: "webhook1"},
+						Key:                  "secret",
+					},
+				},
+			},
+			"webhook2": {
+				WebhookContext: v1alpha1.WebhookContext{
+					URL:      "http://a.b",
+					Endpoint: "/webhook2",
+					Port:     "1234",
+					AuthSecret: &corev1.SecretKeySelector{
+						LocalObjectReference: corev1.LocalObjectReference{Name: "webhook2"},
+						Key:                  "secret",
+					},
+				},
+			},
+		}
+		args.EventSource.Spec.Webhook = webhooksWithSecrets
+
+		wantVolumeNames := []string{"auth-volume", "cm-test-cm", "secret-test-secret", "secret-webhook1", "secret-webhook2", "secret-webhook3", "secret-webhook4", "tmp"}
+		wantVolumeMountNames := []string{"auth-volume", "cm-test-cm", "secret-test-secret", "secret-webhook1", "secret-webhook2", "secret-webhook3", "secret-webhook4", "tmp"}
+
+		deployment, err := buildDeployment(args, fakeEventBus)
+		assert.Nil(t, err)
+		assert.NotNil(t, deployment)
+		gotVolumes := deployment.Spec.Template.Spec.Volumes
+		gotVolumeMounts := deployment.Spec.Template.Spec.Containers[0].VolumeMounts
+
+		var gotVolumeNames []string
+		var gotVolumeMountNames []string
+
+		for _, v := range gotVolumes {
+			gotVolumeNames = append(gotVolumeNames, v.Name)
+		}
+		for _, v := range gotVolumeMounts {
+			gotVolumeMountNames = append(gotVolumeMountNames, v.Name)
+		}
+
+		assert.Equal(t, len(gotVolumes), len(wantVolumeNames))
+		assert.Equal(t, len(gotVolumeMounts), len(wantVolumeMountNames))
+
+		for i := range gotVolumeNames {
+			assert.Equal(t, gotVolumeNames[i], wantVolumeNames[i])
+		}
+		for i := range gotVolumeMountNames {
+			assert.Equal(t, gotVolumeMountNames[i], wantVolumeMountNames[i])
+		}
+	})
 }
 
 func TestResourceReconcile(t *testing.T) {

diff --git a/controllers/sensor/resource.go b/controllers/sensor/resource.go
@@ -21,6 +21,7 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
+	"sort"
 
 	"github.com/imdario/mergo"
 	"go.uber.org/zap"
@@ -228,6 +229,14 @@ func buildDeployment(args *AdaptorArgs, eventBus *eventbusv1alpha1.EventBus) (*a
 	volumeMounts = append(volumeMounts, volCofigMapMounts...)
 	volumes = append(volumes, volConfigMaps...)
 
+	// Order volumes and volumemounts based on name to make the order deterministic
+	sort.Slice(volumes, func(i, j int) bool {
+		return volumes[i].Name < volumes[j].Name
+	})
+	sort.Slice(volumeMounts, func(i, j int) bool {
+		return volumeMounts[i].Name < volumeMounts[j].Name
+	})
+
 	deploymentSpec.Template.Spec.Containers[0].Env = append(deploymentSpec.Template.Spec.Containers[0].Env, env...)
 	deploymentSpec.Template.Spec.Containers[0].VolumeMounts = append(deploymentSpec.Template.Spec.Containers[0].VolumeMounts, volumeMounts...)
 	deploymentSpec.Template.Spec.Volumes = append(deploymentSpec.Template.Spec.Volumes, volumes...)

diff --git a/controllers/sensor/resource_test.go b/controllers/sensor/resource_test.go
@@ -262,6 +262,43 @@ func Test_BuildDeployment(t *testing.T) {
 		assert.True(t, hasTLSSecretVolume)
 		assert.True(t, hasTLSSecretVolumeMount)
 	})
+
+	t.Run("test secret volume and volumemount order deterministic", func(t *testing.T) {
+		args := &AdaptorArgs{
+			Image:  testImage,
+			Sensor: sensorObj,
+			Labels: testLabels,
+		}
+
+		wantVolumeNames := []string{"test-data", "auth-volume", "tmp"}
+		wantVolumeMountNames := []string{"test-data", "auth-volume", "tmp"}
+
+		deployment, err := buildDeployment(args, fakeEventBus)
+		assert.Nil(t, err)
+		assert.NotNil(t, deployment)
+		gotVolumes := deployment.Spec.Template.Spec.Volumes
+		gotVolumeMounts := deployment.Spec.Template.Spec.Containers[0].VolumeMounts
+
+		var gotVolumeNames []string
+		var gotVolumeMountNames []string
+
+		for _, v := range gotVolumes {
+			gotVolumeNames = append(gotVolumeNames, v.Name)
+		}
+		for _, v := range gotVolumeMounts {
+			gotVolumeMountNames = append(gotVolumeMountNames, v.Name)
+		}
+
+		assert.Equal(t, len(gotVolumes), len(wantVolumeNames))
+		assert.Equal(t, len(gotVolumeMounts), len(wantVolumeMountNames))
+
+		for i := range gotVolumeNames {
+			assert.Equal(t, gotVolumeNames[i], wantVolumeNames[i])
+		}
+		for i := range gotVolumeMountNames {
+			assert.Equal(t, gotVolumeMountNames[i], wantVolumeMountNames[i])
+		}
+	})
 }
 
 func TestResourceReconcile(t *testing.T) {