feat(cmp): pass empty env vars to plugins (#18720) (#22096)

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

cmpserver/plugin/plugin.go

@@ -157,7 +157,7 @@ func newCmdError(args string, cause error, stderr string) *CmdError {
 func environ(envVars []*apiclient.EnvEntry) []string {
 	var environ []string
 	for _, item := range envVars {
-		if item != nil && item.Name != "" && item.Value != "" {
+		if item != nil && item.Name != "" {
 			environ = append(environ, fmt.Sprintf("%s=%s", item.Name, item.Value))
 		}
 	}

cmpserver/plugin/plugin_test.go

@@ -514,19 +514,20 @@ func TestEnviron(t *testing.T) {
 		env := environ([]*apiclient.EnvEntry{})
 		assert.Nil(t, env)
 	})
-	t.Run("env vars with empty names or values", func(t *testing.T) {
+	t.Run("env vars with empty names", func(t *testing.T) {
 		env := environ([]*apiclient.EnvEntry{
 			{Value: "test"},
 			{Name: "test"},
 		})
-		assert.Nil(t, env)
+		assert.Equal(t, []string{"test="}, env)
 	})
 	t.Run("proper env vars", func(t *testing.T) {
 		env := environ([]*apiclient.EnvEntry{
 			{Name: "name1", Value: "value1"},
 			{Name: "name2", Value: "value2"},
+			{Name: "name3", Value: ""},
 		})
-		assert.Equal(t, []string{"name1=value1", "name2=value2"}, env)
+		assert.Equal(t, []string{"name1=value1", "name2=value2", "name3="}, env)
 	})
 }
 

docs/operator-manual/upgrading/2.14-3.0.md

@@ -189,3 +189,21 @@ Application, it will not delete the previously managed resources.
 
 It is recommended to perform any cleanup or migration to existing in-cluster Application before upgrading
 when in-cluster is disabled. To perform cleanup post-migration, the in-cluster will need to be enabled temporarily.
+
+### Empty Environment Variables in Plugins
+
+In Argo CD 3.0, empty environment variables are now passed to config management plugins.
+
+```yaml
+apiVersion: argoproj.io/v1alpha1
+kind: Application
+spec:
+  source:
+    plugin:
+      name: example-plugin
+      env:
+        - name: VERSION
+          value: "1.2.3"
+        - name: DATA  # Even though this is empty, it will be passed to the plugin as ARGOCD_ENV_DATA="".
+          value: ""
+```

test/e2e/custom_tool_test.go

@@ -163,6 +163,9 @@ func TestCustomToolWithEnv(t *testing.T) {
 				Env: Env{{
 					Name:  "FOO",
 					Value: "bar",
+				}, {
+					Name:  "EMPTY",
+					Value: "",
 				}},
 			}
 		}).

test/e2e/testdata/cmp-fileName/plugin.yaml

@@ -5,6 +5,7 @@ metadata:
 spec:
   version: v1.0
   generate:
-    command: [sh, -c, 'echo "{\"kind\": \"ConfigMap\", \"apiVersion\": \"v1\", \"metadata\": { \"name\": \"$ARGOCD_APP_NAME\", \"namespace\": \"$ARGOCD_APP_NAMESPACE\", \"annotations\": {\"Foo\": \"$ARGOCD_ENV_FOO\", \"KubeVersion\": \"$KUBE_VERSION\", \"KubeApiVersion\": \"$KUBE_API_VERSIONS\",\"Bar\": \"baz\"}}}"']
+    # The -z test is to ensure that environment variables are made available to the plugin, even if they're empty. https://serverfault.com/a/382740/462962
+    command: [sh, -c, 'if [ -z "${ARGOCD_ENV_EMPTY+set}" ]; then echo "ARGOCD_ENV_EMPTY should have been set." && exit 1; fi; echo "{\"kind\": \"ConfigMap\", \"apiVersion\": \"v1\", \"metadata\": { \"name\": \"$ARGOCD_APP_NAME\", \"namespace\": \"$ARGOCD_APP_NAMESPACE\", \"annotations\": {\"Foo\": \"$ARGOCD_ENV_FOO\", \"KubeVersion\": \"$KUBE_VERSION\", \"KubeApiVersion\": \"$KUBE_API_VERSIONS\",\"Bar\": \"baz\"}}}"']
   discover:
     fileName: "subdir/s*.yaml"