fix: Allow override of default /tmp volume mount in repo server (#1459)

Signed-off-by: Jonathan West <[email protected]>
argoproj-labs · Jul 17, 2024 · a9218b8 · a9218b8
1 parent 806551f
commit a9218b8
Show file tree

Hide file tree

Showing 3 changed files with 103 additions and 21 deletions.
diff --git a/controllers/argocd/deployment.go b/controllers/argocd/deployment.go
@@ -895,6 +895,15 @@ func (r *ReconcileArgoCD) reconcileRepoDeployment(cr *argoproj.ArgoCD, useTLSFor
 		deploy.Spec.Template.Spec.InitContainers = append(deploy.Spec.Template.Spec.InitContainers, cr.Spec.Repo.InitContainers...)
 	}
 
+	// If the user has specified a custom volume mount that overrides the existing /tmp mount, then we should use the user's custom mount, rather than the default.
+	volumeMountOverridesTmpVolume := false
+	for _, volumeMount := range cr.Spec.Repo.VolumeMounts {
+		if volumeMount.MountPath == "/tmp" {
+			volumeMountOverridesTmpVolume = true
+			break
+		}
+	}
+
 	repoServerVolumeMounts := []corev1.VolumeMount{
 		{
 			Name:      "ssh-known-hosts",
@@ -912,10 +921,6 @@ func (r *ReconcileArgoCD) reconcileRepoDeployment(cr *argoproj.ArgoCD, useTLSFor
 			Name:      "gpg-keyring",
 			MountPath: "/app/config/gpg/keys",
 		},
-		{
-			Name:      "tmp",
-			MountPath: "/tmp",
-		},
 		{
 			Name:      "argocd-repo-server-tls",
 			MountPath: "/app/config/reposerver/tls",
@@ -930,6 +935,15 @@ func (r *ReconcileArgoCD) reconcileRepoDeployment(cr *argoproj.ArgoCD, useTLSFor
 		},
 	}
 
+	if !volumeMountOverridesTmpVolume {
+
+		repoServerVolumeMounts = append(repoServerVolumeMounts, corev1.VolumeMount{
+			Name:      "tmp",
+			MountPath: "/tmp",
+		})
+
+	}
+
 	if cr.Spec.Repo.VolumeMounts != nil {
 		repoServerVolumeMounts = append(repoServerVolumeMounts, cr.Spec.Repo.VolumeMounts...)
 	}
@@ -1024,12 +1038,6 @@ func (r *ReconcileArgoCD) reconcileRepoDeployment(cr *argoproj.ArgoCD, useTLSFor
 				EmptyDir: &corev1.EmptyDirVolumeSource{},
 			},
 		},
-		{
-			Name: "tmp",
-			VolumeSource: corev1.VolumeSource{
-				EmptyDir: &corev1.EmptyDirVolumeSource{},
-			},
-		},
 		{
 			Name: "argocd-repo-server-tls",
 			VolumeSource: corev1.VolumeSource{
@@ -1062,6 +1070,16 @@ func (r *ReconcileArgoCD) reconcileRepoDeployment(cr *argoproj.ArgoCD, useTLSFor
 		},
 	}
 
+	// If the user is not used a custom /tmp mount, then just use the default
+	if !volumeMountOverridesTmpVolume {
+		repoServerVolumes = append(repoServerVolumes, corev1.Volume{
+			Name: "tmp",
+			VolumeSource: corev1.VolumeSource{
+				EmptyDir: &corev1.EmptyDirVolumeSource{},
+			},
+		})
+	}
+
 	if cr.Spec.Repo.Volumes != nil {
 		repoServerVolumes = append(repoServerVolumes, cr.Spec.Repo.Volumes...)
 	}

diff --git a/controllers/argocd/deployment_test.go b/controllers/argocd/deployment_test.go
@@ -497,6 +497,70 @@ func TestReconcileArgoCD_reconcileRepoDeployment_mounts(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Contains(t, deployment.Spec.Template.Spec.Containers[0].VolumeMounts, testMount)
 	})
+
+	t.Run("Add extra volume mount and volume that override default /tmp volume mount and volume", func(t *testing.T) {
+		testMount := corev1.VolumeMount{
+			Name:      "test-mount",
+			MountPath: "/tmp",
+		}
+
+		logf.SetLogger(ZapLogger(true))
+		a := makeTestArgoCD(func(a *argoproj.ArgoCD) {
+			a.Spec.Repo.VolumeMounts = []corev1.VolumeMount{testMount}
+			a.Spec.Repo.Volumes = []corev1.Volume{{Name: "test-mount"}}
+		})
+
+		resObjs := []client.Object{a}
+		subresObjs := []client.Object{a}
+		runtimeObjs := []runtime.Object{}
+		sch := makeTestReconcilerScheme(argoproj.AddToScheme)
+		cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
+		r := makeTestReconciler(cl, sch)
+
+		err := r.reconcileRepoDeployment(a, false)
+		assert.NoError(t, err)
+
+		deployment := &appsv1.Deployment{}
+		err = r.Client.Get(context.TODO(), types.NamespacedName{
+			Name:      "argocd-repo-server",
+			Namespace: testNamespace,
+		}, deployment)
+		assert.NoError(t, err)
+
+		assert.Len(t, deployment.Spec.Template.Spec.Containers, 1)
+
+		container := deployment.Spec.Template.Spec.Containers[0]
+
+		containsTestMount := false
+		containsDefaultMount := false
+
+		for _, volumeMount := range container.VolumeMounts {
+
+			if volumeMount.Name == testMount.Name {
+				containsTestMount = true
+			} else if volumeMount.MountPath == "/tmp" {
+				containsDefaultMount = true
+			}
+		}
+
+		assert.True(t, containsTestMount, "should contain test-mount volume mount")
+		assert.False(t, containsDefaultMount, "should not contain the default mount, since this is being overriden by the test-mount")
+
+		containsTestMountVolume := false
+		containsDefaultVolume := false
+		for _, volume := range deployment.Spec.Template.Spec.Volumes {
+			if volume.Name == "test-mount" {
+				containsTestMountVolume = true
+			}
+			if volume.Name == "tmp" {
+				containsDefaultVolume = true
+			}
+		}
+
+		assert.True(t, containsTestMountVolume, "should contain test-mount molume")
+		assert.False(t, containsDefaultVolume, "should not contain default tmp volume")
+
+	})
 }
 
 func TestReconcileArgoCD_reconcileRepoDeployment_initContainers(t *testing.T) {
@@ -1776,12 +1840,6 @@ func repoServerDefaultVolumes() []corev1.Volume {
 				EmptyDir: &corev1.EmptyDirVolumeSource{},
 			},
 		},
-		{
-			Name: "tmp",
-			VolumeSource: corev1.VolumeSource{
-				EmptyDir: &corev1.EmptyDirVolumeSource{},
-			},
-		},
 		{
 			Name: "argocd-repo-server-tls",
 			VolumeSource: corev1.VolumeSource{
@@ -1812,6 +1870,12 @@ func repoServerDefaultVolumes() []corev1.Volume {
 				EmptyDir: &corev1.EmptyDirVolumeSource{},
 			},
 		},
+		{
+			Name: "tmp",
+			VolumeSource: corev1.VolumeSource{
+				EmptyDir: &corev1.EmptyDirVolumeSource{},
+			},
+		},
 	}
 	return volumes
 }
@@ -1823,10 +1887,10 @@ func repoServerDefaultVolumeMounts() []corev1.VolumeMount {
 		{Name: "tls-certs", MountPath: "/app/config/tls"},
 		{Name: "gpg-keys", MountPath: "/app/config/gpg/source"},
 		{Name: "gpg-keyring", MountPath: "/app/config/gpg/keys"},
-		{Name: "tmp", MountPath: "/tmp"},
 		{Name: "argocd-repo-server-tls", MountPath: "/app/config/reposerver/tls"},
 		{Name: common.ArgoCDRedisServerTLSSecretName, MountPath: "/app/config/reposerver/tls/redis"},
 		{Name: "plugins", MountPath: "/home/argocd/cmp-server/plugins"},
+		{Name: "tmp", MountPath: "/tmp"},
 	}
 	return mounts
 }

diff --git a/tests/k8s/1-007_validate_volume_mounts/01-assert.yaml b/tests/k8s/1-007_validate_volume_mounts/01-assert.yaml
@@ -63,14 +63,14 @@ spec:
           name: gpg-keys
         - mountPath: /app/config/gpg/keys
           name: gpg-keyring
-        - mountPath: /tmp
-          name: tmp
         - mountPath: /app/config/reposerver/tls
           name: argocd-repo-server-tls
         - mountPath: /app/config/reposerver/tls/redis
           name: argocd-operator-redis-tls
         - mountPath: /home/argocd/cmp-server/plugins
           name: plugins
+        - mountPath: /tmp
+          name: tmp
       volumes:
       - configMap:
           defaultMode: 420
@@ -86,8 +86,6 @@ spec:
         name: gpg-keys
       - emptyDir: {}
         name: gpg-keyring
-      - emptyDir: {}
-        name: tmp
       - name: argocd-repo-server-tls
         secret:
           defaultMode: 420
@@ -102,6 +100,8 @@ spec:
         name: var-files
       - emptyDir: {}
         name: plugins
+      - emptyDir: {}
+        name: tmp
 ---
 apiVersion: apps/v1
 kind: StatefulSet