Bugfix/allow duplicate oneagent arguments (#2968)

pkg/controllers/dynakube/oneagent/daemonset/arguments.go

@@ -13,7 +13,11 @@ const customArgumentPriority = 2
 const defaultArgumentPriority = 1
 
 func (dsInfo *builderInfo) arguments() ([]string, error) {
-	argMap := prioritymap.New(prioritymap.WithSeparator(prioritymap.DefaultSeparator), prioritymap.WithPriority(defaultArgumentPriority))
+	argMap := prioritymap.New(
+		prioritymap.WithSeparator(prioritymap.DefaultSeparator),
+		prioritymap.WithPriority(defaultArgumentPriority),
+		prioritymap.WithAllowDuplicates(),
+	)
 
 	isProxyAsEnvDeprecated, err := isProxyAsEnvVarDeprecated(dsInfo.dynakube.OneAgentVersion())
 	if err != nil {

pkg/controllers/dynakube/oneagent/daemonset/arguments_test.go

@@ -79,7 +79,7 @@ func TestArguments(t *testing.T) {
 		}
 		assert.Equal(t, expectedDefaultArguments, arguments)
 	})
-	t.Run("when injected arguments are provided then they take precedence", func(t *testing.T) {
+	t.Run("when injected arguments are provided then they come last", func(t *testing.T) {
 		args := []string{
 			"--set-app-log-content-access=true",
 			"--set-host-id-source=lustiglustig",
@@ -98,6 +98,7 @@ func TestArguments(t *testing.T) {
 			"--set-host-group=APP_LUSTIG_PETER",
 			"--set-host-id-source=lustiglustig",
 			"--set-host-property=OperatorVersion=$(DT_OPERATOR_VERSION)",
+			"--set-server={$(DT_SERVER)}",
 			"--set-server=https://hyper.super.com:9999",
 			"--set-tenant=$(DT_TENANT)",
 		}
@@ -124,6 +125,37 @@ func TestArguments(t *testing.T) {
 		}
 		assert.Equal(t, expectedDefaultArguments, arguments)
 	})
+	t.Run("multiple set-host-property entries are possible", func(t *testing.T) {
+		args := []string{
+			"--set-app-log-content-access=true",
+			"--set-host-id-source=lustiglustig",
+			"--set-host-group=APP_LUSTIG_PETER",
+			"--set-server=https://hyper.super.com:9999",
+			"--set-host-property=item0=value0",
+			"--set-host-property=item1=value1",
+			"--set-host-property=item2=value2",
+		}
+		builder := builderInfo{
+			dynakube:       &dynatracev1beta1.DynaKube{},
+			hostInjectSpec: &dynatracev1beta1.HostInjectSpec{Args: args},
+		}
+
+		arguments, _ := builder.arguments()
+
+		expectedDefaultArguments := []string{
+			"--set-app-log-content-access=true",
+			"--set-host-group=APP_LUSTIG_PETER",
+			"--set-host-id-source=lustiglustig",
+			"--set-host-property=OperatorVersion=$(DT_OPERATOR_VERSION)",
+			"--set-host-property=item0=value0",
+			"--set-host-property=item1=value1",
+			"--set-host-property=item2=value2",
+			"--set-server={$(DT_SERVER)}",
+			"--set-server=https://hyper.super.com:9999",
+			"--set-tenant=$(DT_TENANT)",
+		}
+		assert.Equal(t, expectedDefaultArguments, arguments)
+	})
 }
 
 func TestPodSpec_Arguments(t *testing.T) {

pkg/controllers/dynakube/oneagent/daemonset/daemonset_test.go

@@ -836,8 +836,8 @@ func TestOneAgentHostGroup(t *testing.T) {
 		daemonset, err := builder.BuildDaemonSet()
 
 		require.NoError(t, err)
-		assert.NotNil(t, daemonset)
-		assert.Equal(t, "--set-host-group=newgroup", daemonset.Spec.Template.Spec.Containers[0].Args[0])
+		require.NotNil(t, daemonset)
+		assert.Equal(t, "--set-host-group=newgroup", daemonset.Spec.Template.Spec.Containers[0].Args[1])
 	})
 }
 
@@ -881,8 +881,10 @@ func TestDefaultArguments(t *testing.T) {
 		expectedDefaultArguments := []string{
 			"--set-app-log-content-access=true",
 			"--set-host-group=APP_LUSTIG_PETER",
+			"--set-host-id-source=auto",
 			"--set-host-id-source=fqdn",
 			"--set-host-property=OperatorVersion=$(DT_OPERATOR_VERSION)",
+			"--set-server={$(DT_SERVER)}",
 			"--set-server=https://hyper.super.com:9999",
 			"--set-tenant=$(DT_TENANT)",
 		}
@@ -906,6 +908,7 @@ func TestDefaultArguments(t *testing.T) {
 			"--set-host-group=APP_LUSTIG_PETER",
 			"--set-host-id-source=auto",
 			"--set-host-property=OperatorVersion=$(DT_OPERATOR_VERSION)",
+			"--set-server={$(DT_SERVER)}",
 			"--set-server=https://hyper.super.com:9999",
 			"--set-tenant=$(DT_TENANT)",
 		}

pkg/util/prioritymap/cmdline_parser.go

@@ -7,7 +7,7 @@ import (
 const DefaultSeparator = "="
 
 // ParseCommandLineArgument splits strings in the format of "--param=value" up in its components "--param", "=" and "value".
-// The separator is returned because to let the caller know if it was there
+// The separator is returned to let the caller know if it was there
 func ParseCommandLineArgument(arg string) (string, string, string) {
 	arg, value, foundSeparator := strings.Cut(arg, DefaultSeparator)
 

pkg/util/prioritymap/conversion.go

@@ -2,6 +2,7 @@ package prioritymap
 
 import (
 	"fmt"
+	"sort"
 
 	"golang.org/x/exp/slices"
 	corev1 "k8s.io/api/core/v1"
@@ -12,26 +13,28 @@ func (m Map) AsEnvVars() []corev1.EnvVar {
 	envVars := make([]corev1.EnvVar, 0, len(keys))
 
 	for _, key := range keys {
-		switch typedValue := m.entries[key].value.(type) {
-		case string:
-			envVars = append(envVars, corev1.EnvVar{
-				Name:  key,
-				Value: typedValue,
-			})
-		case corev1.EnvVar:
-			envVars = append(envVars, typedValue)
-		case *corev1.EnvVar:
-			envVars = append(envVars, *typedValue)
-		case *corev1.EnvVarSource:
-			envVars = append(envVars, corev1.EnvVar{
-				Name:      key,
-				ValueFrom: typedValue,
-			})
-		case corev1.EnvVarSource:
-			envVars = append(envVars, corev1.EnvVar{
-				Name:      key,
-				ValueFrom: &typedValue,
-			})
+		for _, entry := range m.entries[key] {
+			switch typedValue := entry.value.(type) {
+			case string:
+				envVars = append(envVars, corev1.EnvVar{
+					Name:  key,
+					Value: typedValue,
+				})
+			case corev1.EnvVar:
+				envVars = append(envVars, typedValue)
+			case *corev1.EnvVar:
+				envVars = append(envVars, *typedValue)
+			case *corev1.EnvVarSource:
+				envVars = append(envVars, corev1.EnvVar{
+					Name:      key,
+					ValueFrom: typedValue,
+				})
+			case corev1.EnvVarSource:
+				envVars = append(envVars, corev1.EnvVar{
+					Name:      key,
+					ValueFrom: &typedValue,
+				})
+			}
 		}
 	}
 
@@ -43,8 +46,13 @@ func (m Map) AsKeyValueStrings() []string {
 	valStrings := make([]string, 0)
 
 	for _, key := range keys {
-		val := m.entries[key]
-		valStrings = append(valStrings, fmt.Sprintf("%s%s%v", key, val.delimiter, val.value))
+		sort.SliceStable(m.entries[key], func(i, j int) bool {
+			return m.entries[key][i].priority < m.entries[key][j].priority
+		})
+
+		for _, entry := range m.entries[key] {
+			valStrings = append(valStrings, fmt.Sprintf("%s%s%v", key, entry.delimiter, entry.value))
+		}
 	}
 
 	return valStrings

pkg/util/prioritymap/map.go

@@ -6,22 +6,23 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
-type entry struct {
-	value     any
-	delimiter string
-	priority  int
-}
-
 const DefaultPriority = LowPriority
 const LowPriority = 1
 const MediumPriority = 5
 const HighPriority = 10
 
 type Map struct {
-	entries        map[string]entry
+	entries        map[string][]entry
 	defaultOptions []Option
 }
 
+type entry struct {
+	value           any
+	delimiter       string
+	priority        int
+	allowDuplicates bool
+}
+
 type Option func(a *entry)
 
 func WithPriority(priority int) Option {
@@ -36,9 +37,16 @@ func WithSeparator(separator string) Option {
 	}
 }
 
+// WithAllowDuplicatesForKey allows to add multiple values for the same key (covers all keys)
+func WithAllowDuplicates() Option {
+	return func(a *entry) {
+		a.allowDuplicates = true
+	}
+}
+
 func New(defaultOptions ...Option) *Map {
 	m := &Map{
-		entries:        make(map[string]entry),
+		entries:        make(map[string][]entry),
 		defaultOptions: defaultOptions,
 	}
 
@@ -51,8 +59,9 @@ func (m Map) Append(key string, value any, opts ...Option) {
 	}
 
 	newArg := entry{
-		value:    value,
-		priority: DefaultPriority,
+		value:           value,
+		priority:        DefaultPriority,
+		allowDuplicates: false,
 	}
 
 	for _, opt := range m.defaultOptions {
@@ -65,8 +74,12 @@ func (m Map) Append(key string, value any, opts ...Option) {
 
 	key, _ = strings.CutSuffix(key, newArg.delimiter)
 
-	if existingArg, exists := m.entries[key]; !exists || newArg.priority > existingArg.priority {
-		m.entries[key] = newArg
+	if existingArg, exists := m.entries[key]; !exists || newArg.allowDuplicates || newArg.priority > existingArg[0].priority {
+		if !exists || !newArg.allowDuplicates {
+			m.entries[key] = make([]entry, 0)
+		}
+
+		m.entries[key] = append(m.entries[key], newArg)
 	}
 }