cli/command/service: fix panic when removing duplicate values

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)
 	}
 

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))
+		})
+	}
+}

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, " ")))

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))
+}