diff --git a/cli/command/service/opts.go b/cli/command/service/opts.go index ba2735ace259..da83f8108498 100644 --- a/cli/command/service/opts.go +++ b/cli/command/service/opts.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "net/netip" + "slices" "sort" "strconv" "strings" @@ -687,15 +688,13 @@ func (options *serviceOptions) makeEnv() ([]string, error) { } currentEnv := make([]string, 0, len(envVariables)) for _, env := range envVariables { // need to process each var, in order - k, _, _ := strings.Cut(env, "=") - for i, current := range currentEnv { // remove duplicates - if current == env { - continue // no update required, may hide this behind flag to preserve order of envVariables - } - if strings.HasPrefix(current, k+"=") { - currentEnv = append(currentEnv[:i], currentEnv[i+1:]...) - } + if slices.Contains(currentEnv, env) { + continue // no update required, may hide this behind flag to preserve order of envVariables } + k, _, _ := strings.Cut(env, "=") + currentEnv = slices.DeleteFunc(currentEnv, func(current string) bool { // remove duplicates + return strings.HasPrefix(current, k+"=") + }) currentEnv = append(currentEnv, env) } diff --git a/cli/command/service/opts_test.go b/cli/command/service/opts_test.go index d11d9a2e1aa9..5ab11e262b36 100644 --- a/cli/command/service/opts_test.go +++ b/cli/command/service/opts_test.go @@ -373,3 +373,49 @@ func TestToServiceSysCtls(t *testing.T) { assert.NilError(t, err) assert.Check(t, is.DeepEqual(service.TaskTemplate.ContainerSpec.Sysctls, expected)) } + +func TestMakeEnv(t *testing.T) { + tests := []struct { + doc string + env []string + expected []string + }{ + { + doc: "no duplicates", + env: []string{"one=1", "two=2"}, + expected: []string{"one=1", "two=2"}, + }, + { + doc: "same variable repeated", + env: []string{"one=1", "one=1"}, + expected: []string{"one=1"}, + }, + { + doc: "same variable repeated, then overridden", + env: []string{"one=1", "one=1", "one=2"}, + expected: []string{"one=2"}, + }, + { + doc: "repeated variable last", + env: []string{"one=1", "two=2", "two=2"}, + expected: []string{"one=1", "two=2"}, + }, + { + doc: "last value wins", + env: []string{"one=1", "two=2", "one=3"}, + expected: []string{"two=2", "one=3"}, + }, + } + + for _, tc := range tests { + t.Run(tc.doc, func(t *testing.T) { + o := newServiceOptions() + for _, env := range tc.env { + assert.NilError(t, o.env.Set(env)) + } + actual, err := o.makeEnv() + assert.NilError(t, err) + assert.Check(t, is.DeepEqual(tc.expected, actual)) + }) + } +} diff --git a/cli/command/service/update.go b/cli/command/service/update.go index 62f5376ae29a..13af4bd746cf 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -1213,11 +1213,9 @@ func updateHosts(flags *pflag.FlagSet, hosts *[]string) error { if rm.IPAddr != "" && rm.IPAddr != ip { continue } - for i, h := range hostNames { - if h == rm.Host { - hostNames = append(hostNames[:i], hostNames[i+1:]...) - } - } + hostNames = slices.DeleteFunc(hostNames, func(h string) bool { + return h == rm.Host + }) } if len(hostNames) > 0 { newHosts = append(newHosts, fmt.Sprintf("%s %s", ip, strings.Join(hostNames, " "))) diff --git a/cli/command/service/update_test.go b/cli/command/service/update_test.go index bb346b2e6fb9..6158820951b1 100644 --- a/cli/command/service/update_test.go +++ b/cli/command/service/update_test.go @@ -1727,3 +1727,18 @@ func TestUpdateUlimits(t *testing.T) { }) } } + +func TestUpdateHostsRemoveRepeatedHost(t *testing.T) { + flags := newUpdateCommand(nil).Flags() + flags.Set("host-rm", "host1") + + //nolint:dupword // ignore "Duplicate words (host1) found" + hosts := []string{"127.0.0.1 host1 host1 host2", "127.0.0.2 host2 host1 host1"} + + err := updateHosts(flags, &hosts) + assert.NilError(t, err) + + // All occurrences of `host1` should be removed, also if the same host + // is listed multiple times in the same entry. + assert.Check(t, is.DeepEqual([]string{"127.0.0.1 host2", "127.0.0.2 host2"}, hosts)) +}