From bb2bc84527b62289c399ba3936d36db75df9eeac Mon Sep 17 00:00:00 2001 From: Rizwana777 Date: Mon, 9 Sep 2024 19:48:45 +0530 Subject: [PATCH 1/2] Fix issue in extraArgsCommand field Signed-off-by: Rizwana777 --- controllers/argocd/deployment.go | 70 +++++++++++++++++++++------ controllers/argocd/deployment_test.go | 28 +++++++++++ 2 files changed, 84 insertions(+), 14 deletions(-) diff --git a/controllers/argocd/deployment.go b/controllers/argocd/deployment.go index b9cba8219..da43c5419 100644 --- 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 index de18f8fa6..d478daaa5 100644 --- 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{} From 0079ddb961af020cc3c70bee5d7cb00fc988d96e Mon Sep 17 00:00:00 2001 From: Rizwana777 Date: Fri, 8 Nov 2024 16:56:32 +0530 Subject: [PATCH 2/2] Override the default values Signed-off-by: Rizwana777 --- controllers/argocd/deployment.go | 48 +++++++++++++++------------ controllers/argocd/deployment_test.go | 18 +++++++++- 2 files changed, 43 insertions(+), 23 deletions(-) diff --git a/controllers/argocd/deployment.go b/controllers/argocd/deployment.go index da43c5419..4b44f5347 100644 --- a/controllers/argocd/deployment.go +++ b/controllers/argocd/deployment.go @@ -345,46 +345,50 @@ func isMergable(extraArgs []string, cmd []string) error { // 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) + // Parse cmd into a map to track flags and their indices + existingArgs := make(map[string]int) // Map to track index of each flag in cmd 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 + existingArgs[arg] = i + i++ // Skip the value } else { - existingArgs[arg] = "" // Store flag without value + existingArgs[arg] = i } } } - // Iterate over extraArgs and append only if the flag and value do not already exist in cmd + // Iterate over extraArgs to append or override existing flags 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 index, exists := existingArgs[arg]; exists { + // If the flag exists, check if we need to override its value if i+1 < len(extraArgs) && !strings.HasPrefix(extraArgs[i+1], "--") { - i++ // Skip the associated value + // If the flag has a value in extraArgs,update it in cmd + if index+1 < len(cmd) && !strings.HasPrefix(cmd[index+1], "--") { + cmd[index+1] = extraArgs[i+1] // Override the existing value + } else { + // Insert the new value if it didn't previously have one + cmd = append(cmd[:index+1], append([]string{extraArgs[i+1]}, cmd[index+1:]...)...) + } + i++ // Skip the value in extraArgs } - 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 + // Append the new flag and its value if not present + cmd = append(cmd, arg) + if i+1 < len(extraArgs) && !strings.HasPrefix(extraArgs[i+1], "--") { + cmd = append(cmd, extraArgs[i+1]) + existingArgs[arg] = len(cmd) - 2 // Update index tracking + i++ // Skip the value + } else { + existingArgs[arg] = len(cmd) - 1 // Update index tracking + } } } else { - // If it's not a flag, append it directly (non-flag argument) + // Append non-flag arguments directly cmd = append(cmd, arg) } } diff --git a/controllers/argocd/deployment_test.go b/controllers/argocd/deployment_test.go index d478daaa5..d3d5dd882 100644 --- a/controllers/argocd/deployment_test.go +++ b/controllers/argocd/deployment_test.go @@ -1306,6 +1306,22 @@ func TestArgoCDServerDeploymentCommand(t *testing.T) { "foo.scv.cluster.local:6379", } + wantCmd := []string{ + "argocd-server", + "--staticassets", + "/shared/app", + "--dex-server", + "https://argocd-dex-server.argocd.svc.cluster.local:5556", + "--repo-server", + "argocd-repo-server.argocd.svc.cluster.local:8081", + "--redis", + "foo.scv.cluster.local:6379", + "--loglevel", + "info", + "--logformat", + "text", + } + assert.NoError(t, r.reconcileServerDeployment(a, false)) assert.NoError(t, r.Client.Get( context.TODO(), @@ -1315,7 +1331,7 @@ func TestArgoCDServerDeploymentCommand(t *testing.T) { }, deployment)) - assert.Equal(t, baseCommand, deployment.Spec.Template.Spec.Containers[0].Command) + assert.Equal(t, wantCmd, deployment.Spec.Template.Spec.Containers[0].Command) // When ExtraCommandArgs contains a non-duplicate argument along with a duplicate a.Spec.Server.ExtraCommandArgs = []string{