Fix issue in extraArgsCommand field

Signed-off-by: Rizwana777 <[email protected]>
argoproj-labs · Oct 8, 2024 · bb2bc84 · bb2bc84
1 parent 3334487
commit bb2bc84
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 14 deletions.
diff --git a/controllers/argocd/deployment.go b/controllers/argocd/deployment.go
@@ -288,11 +288,9 @@ func getArgoServerCommand(cr *argoproj.ArgoCD, useTLSForRedis bool) []string {
 		cmd = append(cmd, "--repo-server-strict-tls")
 	}
 
-	cmd = append(cmd, "--staticassets")
-	cmd = append(cmd, "/shared/app")
+	cmd = append(cmd, "--staticassets", "/shared/app")
 
-	cmd = append(cmd, "--dex-server")
-	cmd = append(cmd, getDexServerAddress(cr))
+	cmd = append(cmd, "--dex-server", getDexServerAddress(cr))
 
 	if cr.Spec.Repo.IsEnabled() {
 		cmd = append(cmd, "--repo-server", getRepoServerAddress(cr))
@@ -315,22 +313,17 @@ func getArgoServerCommand(cr *argoproj.ArgoCD, useTLSForRedis bool) []string {
 		}
 	}
 
-	cmd = append(cmd, "--loglevel")
-	cmd = append(cmd, getLogLevel(cr.Spec.Server.LogLevel))
-
-	cmd = append(cmd, "--logformat")
-	cmd = append(cmd, getLogFormat(cr.Spec.Server.LogFormat))
+	cmd = append(cmd, "--loglevel", getLogLevel(cr.Spec.Server.LogLevel))
+	cmd = append(cmd, "--logformat", getLogFormat(cr.Spec.Server.LogFormat))
 
+	// Merge extraArgs while ignoring duplicates
 	extraArgs := cr.Spec.Server.ExtraCommandArgs
-	err := isMergable(extraArgs, cmd)
-	if err != nil {
-		return cmd
-	}
+	cmd = appendUniqueArgs(cmd, extraArgs)
+
 	if len(cr.Spec.SourceNamespaces) > 0 {
 		cmd = append(cmd, "--application-namespaces", fmt.Sprint(strings.Join(cr.Spec.SourceNamespaces, ",")))
 	}
 
-	cmd = append(cmd, extraArgs...)
 	return cmd
 }
 
@@ -350,6 +343,55 @@ func isMergable(extraArgs []string, cmd []string) error {
 	return nil
 }
 
+// appendUniqueArgs appends extraArgs to cmd while ignoring any duplicate flags.
+func appendUniqueArgs(cmd []string, extraArgs []string) []string {
+	// Parse cmd into a map to track both flags and their associated values
+	existingArgs := make(map[string]string)
+	for i := 0; i < len(cmd); i++ {
+		arg := cmd[i]
+		if strings.HasPrefix(arg, "--") {
+			// Check if the next item is a value (not another flag)
+			if i+1 < len(cmd) && !strings.HasPrefix(cmd[i+1], "--") {
+				existingArgs[arg] = cmd[i+1] // Store flag-value pair
+				i++                          // Skip the value
+			} else {
+				existingArgs[arg] = "" // Store flag without value
+			}
+		}
+	}
+
+	// Iterate over extraArgs and append only if the flag and value do not already exist in cmd
+	for i := 0; i < len(extraArgs); i++ {
+		arg := extraArgs[i]
+		if strings.HasPrefix(arg, "--") {
+			// If this flag already exists in cmd, skip it
+			if _, exists := existingArgs[arg]; exists {
+				if i+1 < len(extraArgs) && !strings.HasPrefix(extraArgs[i+1], "--") {
+					i++ // Skip the associated value
+				}
+				continue
+			}
+
+			// Append the flag to cmd
+			cmd = append(cmd, arg)
+
+			// Check if this flag has an associated value
+			if i+1 < len(extraArgs) && !strings.HasPrefix(extraArgs[i+1], "--") {
+				cmd = append(cmd, extraArgs[i+1])
+				existingArgs[arg] = extraArgs[i+1] // Track flag-value pair
+				i++                                // Skip the value
+			} else {
+				existingArgs[arg] = "" // Track flag without a value
+			}
+		} else {
+			// If it's not a flag, append it directly (non-flag argument)
+			cmd = append(cmd, arg)
+		}
+	}
+
+	return cmd
+}
+
 // getDexServerAddress will return the Dex server address.
 func getDexServerAddress(cr *argoproj.ArgoCD) string {
 	return fmt.Sprintf("https://%s", fqdnServiceRef("dex-server", common.ArgoCDDefaultDexHTTPPort, cr))

diff --git a/controllers/argocd/deployment_test.go b/controllers/argocd/deployment_test.go
@@ -1317,6 +1317,34 @@ func TestArgoCDServerDeploymentCommand(t *testing.T) {
 
 	assert.Equal(t, baseCommand, deployment.Spec.Template.Spec.Containers[0].Command)
 
+	// When ExtraCommandArgs contains a non-duplicate argument along with a duplicate
+	a.Spec.Server.ExtraCommandArgs = []string{
+		"--rootpath",
+		"/argocd",
+		"--foo",
+		"bar",
+		"test",
+		"--logformat", // Duplicate flag and value
+		"text",
+		"--newarg", // Non-duplicate argument
+		"newvalue",
+		"--newarg", // Duplicate argument passing at once
+		"newvalue",
+	}
+
+	assert.NoError(t, r.reconcileServerDeployment(a, false))
+	assert.NoError(t, r.Client.Get(
+		context.TODO(),
+		types.NamespacedName{
+			Name:      "argocd-server",
+			Namespace: a.Namespace,
+		},
+		deployment))
+
+	// Non-duplicate argument "--newarg" should be added, duplicate "--newarg" which is added twice is ignored
+	cmd = append(cmd, "--newarg", "newvalue")
+	assert.Equal(t, cmd, deployment.Spec.Template.Spec.Containers[0].Command)
+
 	// Remove all the command arguments that were added.
 	a.Spec.Server.ExtraCommandArgs = []string{}