Skip to content

Commit

Permalink
Change way how arguments are split to allow multiple --set-host-prope…
Browse files Browse the repository at this point in the history
…rty arguments again.

Allow arguments without value.
  • Loading branch information
StefanHauth committed Feb 25, 2025
1 parent e32edf0 commit 087c5a3
Show file tree
Hide file tree
Showing 4 changed files with 193 additions and 21 deletions.
16 changes: 5 additions & 11 deletions pkg/api/v1beta3/dynakube/oneagent/props.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,19 +252,13 @@ func (oa *OneAgent) GetHostGroupAsParam() string {
return hostGroup
}

func splitArg(arg string) (key, value string) {
split := strings.Split(arg, "=")

const expectedLen = 2

if len(split) != expectedLen {
return
func splitArg(arg string) (string, string) {
key, value, found := strings.Cut(arg, "=")
if !found {
return arg, ""
}

key = split[0]
value = split[1]

return
return key, value
}

func (oa *OneAgent) IsCommunicationRouteClear() bool {
Expand Down
126 changes: 126 additions & 0 deletions pkg/api/v1beta3/dynakube/oneagent/props_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,132 @@ func TestOneAgentHostGroup(t *testing.T) {
})
}

func TestOneAgentArgumentsMap(t *testing.T) {
t.Run("straight forward argument list", func(t *testing.T) {
dk := OneAgent{Spec: &Spec{
CloudNativeFullStack: &CloudNativeFullStackSpec{
HostInjectSpec: HostInjectSpec{
Args: []string{
"--set-host-id-source=k8s-node-name",
"--set-host-property=OperatorVersion=$(DT_OPERATOR_VERSION)",
"--set-host-property=dt.security_context=kubernetes_clusters",
"--set-host-property=dynakube-name=$(CUSTOM_CRD_NAME)",
"--set-no-proxy=",
"--set-proxy=",
"--set-tenant=$(DT_TENANT)",
"--set-server=dynatrace.com",
"--set-host-property=prop1=val1",
"--set-host-property=prop2=val2",
"--set-host-property=prop3=val3",
"--set-host-tag=tag1",
"--set-host-tag=tag2",
"--set-host-tag=tag3",
},
},
},
HostGroup: "field",
},
}
argMap := dk.GetArgumentsMap()
require.Len(t, argMap, 7)

require.Len(t, argMap["--set-host-id-source"], 1)
assert.Equal(t, "k8s-node-name", argMap["--set-host-id-source"][0])

require.Len(t, argMap["--set-host-property"], 6)
assert.Equal(t, "OperatorVersion=$(DT_OPERATOR_VERSION)", argMap["--set-host-property"][0])
assert.Equal(t, "dt.security_context=kubernetes_clusters", argMap["--set-host-property"][1])
assert.Equal(t, "dynakube-name=$(CUSTOM_CRD_NAME)", argMap["--set-host-property"][2])
assert.Equal(t, "prop1=val1", argMap["--set-host-property"][3])
assert.Equal(t, "prop2=val2", argMap["--set-host-property"][4])
assert.Equal(t, "prop3=val3", argMap["--set-host-property"][5])

require.Len(t, argMap["--set-no-proxy"], 1)
assert.Equal(t, "", argMap["--set-no-proxy"][0])

require.Len(t, argMap["--set-proxy"], 1)
assert.Equal(t, "", argMap["--set-proxy"][0])

require.Len(t, argMap["--set-tenant"], 1)
assert.Equal(t, "$(DT_TENANT)", argMap["--set-tenant"][0])

require.Len(t, argMap["--set-server"], 1)
assert.Equal(t, "dynatrace.com", argMap["--set-server"][0])
})

t.Run("multiple --set-host-property arguments", func(t *testing.T) {
dk := OneAgent{Spec: &Spec{
CloudNativeFullStack: &CloudNativeFullStackSpec{
HostInjectSpec: HostInjectSpec{
Args: []string{
"--set-host-property=prop1=val1",
"--set-host-property=prop2=val2",
"--set-host-property=prop3=val3",
"--set-host-property=prop3=val3",
},
},
},
HostGroup: "field",
},
}
argMap := dk.GetArgumentsMap()
require.Len(t, argMap, 1)
require.Len(t, argMap["--set-host-property"], 4)

assert.Equal(t, "prop1=val1", argMap["--set-host-property"][0])
assert.Equal(t, "prop2=val2", argMap["--set-host-property"][1])
assert.Equal(t, "prop3=val3", argMap["--set-host-property"][2])
})

t.Run("multiple --set-host-tag arguments", func(t *testing.T) {
dk := OneAgent{Spec: &Spec{
CloudNativeFullStack: &CloudNativeFullStackSpec{
HostInjectSpec: HostInjectSpec{
Args: []string{
"--set-host-tag=tag1=1",
"--set-host-tag=tag1=2",
"--set-host-tag=tag1=3",
"--set-host-tag=tag2",
"--set-host-tag=tag3",
},
},
},
HostGroup: "field",
},
}
argMap := dk.GetArgumentsMap()
require.Len(t, argMap, 1)
require.Len(t, argMap["--set-host-tag"], 5)

assert.Equal(t, "tag1=1", argMap["--set-host-tag"][0])
assert.Equal(t, "tag1=2", argMap["--set-host-tag"][1])
assert.Equal(t, "tag1=3", argMap["--set-host-tag"][2])
assert.Equal(t, "tag2", argMap["--set-host-tag"][3])
assert.Equal(t, "tag3", argMap["--set-host-tag"][4])
})

t.Run("arguments without value", func(t *testing.T) {
dk := OneAgent{Spec: &Spec{
CloudNativeFullStack: &CloudNativeFullStackSpec{
HostInjectSpec: HostInjectSpec{
Args: []string{
"--enable-feature-a",
"--enable-feature-b",
"--enable-feature-c",
},
},
},
HostGroup: "field",
},
}
argMap := dk.GetArgumentsMap()
require.Len(t, argMap, 3)
require.Len(t, argMap["--enable-feature-a"], 1)
require.Len(t, argMap["--enable-feature-b"], 1)
require.Len(t, argMap["--enable-feature-c"], 1)
})
}

func setupDisabledCSIEnv(t *testing.T) {
t.Helper()
installconfig.SetModulesOverride(t, installconfig.Modules{
Expand Down
42 changes: 32 additions & 10 deletions pkg/api/validation/dynakube/oneagent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -599,34 +599,56 @@ func TestOneAgentArguments(t *testing.T) {
testName: "duplicate host property",
arguments: []string{
"--set-server=foo",
"--set-host-property=foo",
"--set-host-property=bar",
"--set-host-property=dow",
"--set-host-property=foo1=bar1",
"--set-host-property=foo2=bar2",
"--set-host-property=foo3=bar3",
"--set-host-property=foo3=bar3",
"--set-host-property=foo2=bar2",
"--set-host-property=foo1=bar1",
},
expectedError: "",
},
{
testName: "duplicate host tag",
arguments: []string{
"--set-server=foo",
"--set-host-tag=foo",
"--set-host-tag=bar",
"--set-host-tag=dow",
"--set-host-tag=foo=1",
"--set-host-tag=bar=1",
"--set-host-tag=dow=1",
},
expectedError: "",
},
{
testName: "duplicate host tag with same value",
arguments: []string{
"--set-host-tag=foo",
"--set-host-tag=foo=1",
"--set-host-tag=bar",
"--set-host-tag=foo",
"--set-host-tag=foo=1",
"--set-host-tag=bar",
"--set-host-tag=doh",
"--set-host-tag=bar",
"--set-host-tag=foo",
"--set-host-tag=foo=1",
},
expectedError: fmt.Sprintf(errorSameHostTagMultipleTimes, "[foo=1 bar]"),
},
{
testName: "arguments without value",
arguments: []string{
"--enable-feature-a",
"--enable-feature-b",
"--enable-feature-c",
},
expectedError: "",
},
{
testName: "duplicate arguments without value",
arguments: []string{
"--enable-feature-a",
"--enable-feature-b",
"--enable-feature-a",
"--enable-feature-c",
},
expectedError: fmt.Sprintf(errorSameHostTagMultipleTimes, "[foo bar]"),
expectedError: fmt.Sprintf(errorDuplicateOneAgentArgument, "--enable-feature-a"),
},
}

Expand Down
30 changes: 30 additions & 0 deletions pkg/controllers/dynakube/oneagent/daemonset/arguments_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,36 @@ func TestArguments(t *testing.T) {

assert.Contains(t, arguments, "--set-no-proxy=*.dev.dynatracelabs.com,dynakube-activegate.dynatrace")
})
t.Run("allow arguments without value, but deduplicate", func(t *testing.T) {
custArgs := []string{
"--enable-feature-a",
"--enable-feature-b",
"--enable-feature-c",
"--enable-feature-c",
"--enable-feature-a",
"--enable-feature-b",
}
builder := builder{
dk: &dynakube.DynaKube{Spec: dynakube.DynaKubeSpec{OneAgent: oneagent.Spec{ClassicFullStack: &oneagent.HostInjectSpec{}}}},
hostInjectSpec: &oneagent.HostInjectSpec{Args: custArgs},
deploymentType: deploymentmetadata.CloudNativeDeploymentType,
}

arguments, _ := builder.arguments()

expectedDefaultArguments := []string{
"--enable-feature-a",
"--enable-feature-b",
"--enable-feature-c",
"--set-host-id-source=auto",
"--set-host-property=OperatorVersion=$(DT_OPERATOR_VERSION)",
"--set-no-proxy=",
"--set-proxy=",
"--set-server={$(DT_SERVER)}",
"--set-tenant=$(DT_TENANT)",
}
assert.Equal(t, expectedDefaultArguments, arguments)
})
}

func TestPodSpec_Arguments(t *testing.T) {
Expand Down

0 comments on commit 087c5a3

Please sign in to comment.