Fix LCOW DNS search bug #2594

rawahars · 2026-02-02T18:34:24Z

While github.com/samber/lo seems to be a relatively popular, do we need to check with Microsoft security about the package meeting Microsoft security bar?

we haven't done any such checks (if there is a process for Golang packages)
the most we do is validate the license type in component governance

-Original file line number
+Diff line change
@@ Expand Up / @@ -50,6 +50,7 @@ require ( @@
     	github.com/opencontainers/runtime-spec v1.2.1
     	github.com/pelletier/go-toml v1.9.5
     	github.com/pkg/errors v0.9.1
+    	github.com/samber/lo v1.52.0
     	github.com/sirupsen/logrus v1.9.3
     	github.com/urfave/cli v1.22.16
     	github.com/urfave/cli/v2 v2.27.6
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up @@
     github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
     github.com/ruudk/golang-pdf417 v0.0.0-20181029194003-1af4ab5afa58/go.mod h1:6lfFZQK844Gfx8o5WFuvpxWRwnSoipWe/p622j1v06w=
     github.com/ruudk/golang-pdf417 v0.0.0-20201230142125-a7e3863a1245/go.mod h1:pQAZKsJ8yyVxGRWYNEm9oFB8ieLgKFnamEyDmSA0BRk=
+    github.com/samber/lo v1.52.0 h1:Rvi+3BFHES3A8meP33VPAxiBZX/Aws5RxrschYGjomw=
+    github.com/samber/lo v1.52.0/go.mod h1:4+MXEGsJzbKGaUEQFKBq2xtfuznW9oz/WrgyzMzRoM0=
     github.com/seccomp/libseccomp-golang v0.10.0 h1:aA4bp+/Zzi0BnWZ2F1wgNBs5gTpm+na2rWM6M9YjLpY=
     github.com/seccomp/libseccomp-golang v0.10.0/go.mod h1:JA8cRccbGaA1s33RQf7Y1+q9gHmZX1yB/z9WDN1C6fg=
     github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ=
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up @@
     	return content, nil
     }
-    // MergeValues merges `first` and `second` maintaining order `first, second`.
-    func MergeValues(first, second []string) []string {
-    	if len(first) == 0 {
-    		return second
-    	}
-    	if len(second) == 0 {
-    		return first
-    	}
-    	values := make([]string, len(first), len(first)+len(second))
-    	copy(values, first)
-    	for _, v := range second {
-    		found := false
-    		for i := 0; i < len(values); i++ {
-    			if v == values[i] {
-    				found = true
-    				break
-    			}
-    		}
-    		if !found {
-    			values = append(values, v)
-    		}
-    	}
-    	return values
-    }
     // InstanceIDToName converts from the given instance ID (a GUID generated on the
     // Windows host) to its corresponding interface name (e.g. "eth0").
     //
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -70,58 +70,6 @@ func Test_GenerateResolvConfContent(t *testing.T) { @@
     	}
     }
-    func Test_MergeValues(t *testing.T) {
-    	type testcase struct {
-    		name string
-    		first  []string
-    		second []string
-    		expected []string
-    	}
-    	testcases := []*testcase{
-    		{
-    			name: "BothEmpty",
-    		},
-    		{
-    			name:     "FirstEmpty",
-    			second:   []string{"a", "b"},
-    			expected: []string{"a", "b"},
-    		},
-    		{
-    			name:     "SecondEmpty",
-    			first:    []string{"a", "b"},
-    			expected: []string{"a", "b"},
-    		},
-    		{
-    			name:     "AllUnique",
-    			first:    []string{"a", "c", "d"},
-    			second:   []string{"b", "e"},
-    			expected: []string{"a", "c", "d", "b", "e"},
-    		},
-    		{
-    			name:     "NonUnique",
-    			first:    []string{"a", "c", "d"},
-    			second:   []string{"a", "b", "c", "d"},
-    			expected: []string{"a", "c", "d", "b"},
-    		},
-    	}
-    	for _, tc := range testcases {
-    		t.Run(tc.name, func(t *testing.T) {
-    			m := MergeValues(tc.first, tc.second)
-    			if len(m) != len(tc.expected) {
-    				t.Fatalf("expected %d entries got: %d", len(tc.expected), len(m))
-    			}
-    			for i := 0; i < len(tc.expected); i++ {
-    				if tc.expected[i] != m[i] {
-    					t.Logf("%v :: %v", tc.expected, m)
-    					t.Fatalf("expected value: %q at index: %d got: %q", tc.expected[i], i, m[i])
-    				}
-    			}
-    		})
-    	}
-    }
     func Test_GenerateEtcHostsContent(t *testing.T) {
     	type testcase struct {
     		name string
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -6,11 +6,13 @@ package hcsv2 @@
     import (
     	"context"
     	"fmt"
+    	"iter"
     	"strings"
     	"sync"
     	"time"
     	"github.com/pkg/errors"
+    	"github.com/samber/lo"
     	"github.com/vishvananda/netns"
     	"go.opencensus.io/trace"
@@ Expand Down Expand Up @@
     	return nil
     }
-    // Adapters returns a copy of the adapters assigned to `n` at the time of the
-    // call.
-    func (n *namespace) Adapters() []*guestresource.LCOWNetworkAdapter {
-    	n.m.Lock()
-    	defer n.m.Unlock()
-    	adps := make([]*guestresource.LCOWNetworkAdapter, len(n.nics))
-    	for i, nin := range n.nics {
-    		adps[i] = nin.adapter
-    	}
-    	return adps
-    }
     // AddAdapter adds `adp` to `n` but does NOT move the adapter into the network
     // namespace assigned to `n`. A user must call `Sync()` to complete this
     // operation.
@@ Expand Down Expand Up @@
     	defer n.m.Unlock()
     	// TODO: do we need to remove anything guestside from a sandbox namespace?
+    	// TODO: use [slices.DeleteFunc], since we can delete in place
     	i := -1
     	for j, nic := range n.nics {
@@ Expand Down Expand Up @@
     	return nil
     }
+    func (n *namespace) dnsConfig(_ context.Context) (searches []string, servers []string) {
+    	for _, nic := range n.allNICs() {
+    		if len(nic.DNSSuffix) > 0 {
+    			searches = append(searches, strings.Split(nic.DNSSuffix, ",")...)
+    		}
+    		if len(nic.DNSServerList) > 0 {
+    			servers = append(servers, strings.Split(nic.DNSServerList, ",")...)
+    		}
+    	}
+    	// TODO: parse values as [net/netip.Addr] to avoid duplicates in server list due to IP formatting
+    	cancoc := func(s string, _ int) string {
+    		// trim in case a space is added between values when joining (e.g., "a.b, a.b")
+    		// zone identifiers in IPv6 addresses really, really shouldn't be case sensitive, but ... *shrug*
+    		return strings.ToLower(strings.TrimSpace(s))
+    	}
+    	return lo.UniqMap(searches, cancoc), lo.UniqMap(servers, cancoc)
+    }
+    // allNICs iterates over NICs in the namespace.
+    //
+    // NOTE: the namespace's mutex, n.m, is held during iteration.
+    func (n *namespace) allNICs() iter.Seq2[int, *guestresource.LCOWNetworkAdapter] {
+    	return func(yield func(int, *guestresource.LCOWNetworkAdapter) bool) {
+    		n.m.Lock()
+    		defer n.m.Unlock()
+    		for i, nic := range n.nics {
+    			if !yield(i, nic.adapter) {
+    				return
+    			}
+    		}
+    	}
+    }
     // nicInNamespace represents a single network adapter that has been added to the
     // guest and its mapping to the linux `ifname`.
     type nicInNamespace struct {
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix LCOW DNS search bug #2594

Diff view

Diff view

There are no files selected for viewing

rawahars Feb 2, 2026

Uh oh!

helsaawy Feb 2, 2026

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ Expand Up / @@ -5,8 +5,13 @@ package hcsv2 @@
     import (
     	"context"
+    	"fmt"
+    	"strings"
     	"testing"
+    	"github.com/google/go-cmp/cmp"
+    	"github.com/google/go-cmp/cmp/cmpopts"
     	"github.com/Microsoft/hcsshim/internal/protocol/guestresource"
     )
@@ Expand Down Expand Up / @@ -98,7 +103,7 @@ func Test_removeNetworkNamespace_HasAdapters(t *testing.T) { @@
     	ns := GetOrAddNetworkNamespace(t.Name())
-    	networkInstanceIDToName = func(ctx context.Context, id string, _ bool) (string, error) {
+    	networkInstanceIDToName = func(_ context.Context, _ string, _ bool) (string, error) {
     		return "/dev/sdz", nil
     	}
     	err := ns.AddAdapter(context.Background(), &guestresource.LCOWNetworkAdapter{ID: "test"})
@@ Expand All @@
     		t.Fatalf("should not have failed to delete empty namepace got: %v", err)
     	}
     }
+    func TestDNSConfig(t *testing.T) {
+    	t.Cleanup(func() {
+    		err := RemoveNetworkNamespace(context.Background(), t.Name())
+    		if err != nil {
+    			t.Errorf("failed to remove ns with error: %v", err)
+    		}
+    	})
+    	nsOld := networkInstanceIDToName
+    	networkInstanceIDToName = func(_ context.Context, _ string, _ bool) (string, error) {
+    		return "/dev/sdz", nil
+    	}
+    	t.Cleanup(func() {
+    		networkInstanceIDToName = nsOld
+    	})
+    	ctx := t.Context()
+    	ns := GetOrAddNetworkNamespace(t.Name())
+    	for i, tc := range []struct {
+    		searches     string
+    		servers      string
+    		wantSearches []string
+    		wantServers  []string
+    	}{
+    		{},
+    		{
+    			servers:     "1.1.1.1",
+    			wantServers: []string{"1.1.1.1"},
+    		},
+    		{
+    			searches:     "azure-dns.com",
+    			wantSearches: []string{"azure-dns.com"},
+    			wantServers:  []string{"1.1.1.1"},
+    		},
+    		{
+    			searches: strings.Join([]string{
+    				"Azure-DNS.com",
+    				"service.svc.cluster.local",
+    				"svc.cluster.local",
+    				"cluster.local",
+    			}, ","),
+    			servers: "10.11.12.13, 1.1.1.1, 8.8.8.8",
+    			wantSearches: []string{
+    				"azure-dns.com",
+    				"service.svc.cluster.local",
+    				"svc.cluster.local",
+    				"cluster.local",
+    			},
+    			wantServers: []string{
+    				"1.1.1.1",
+    				"10.11.12.13",
+    				"8.8.8.8",
+    			},
+    		},
+    		{
+    			wantSearches: []string{
+    				"azure-dns.com",
+    				"service.svc.cluster.local",
+    				"svc.cluster.local",
+    				"cluster.local",
+    			},
+    			wantServers: []string{
+    				"1.1.1.1",
+    				"10.11.12.13",
+    				"8.8.8.8",
+    			},
+    		},
+    		{
+    			servers: "FC00::A",
+    			wantSearches: []string{
+    				"azure-dns.com",
+    				"service.svc.cluster.local",
+    				"svc.cluster.local",
+    				"cluster.local",
+    			},
+    			wantServers: []string{
+    				"1.1.1.1",
+    				"10.11.12.13",
+    				"8.8.8.8",
+    				"fc00::a",
+    			},
+    		},
+    	} {
+    		id := fmt.Sprintf("test-nic%d", i)
+    		t.Logf("adding NIC %d: %s", i+1, id)
+    		t.Logf("searches: %q", tc.searches)
+    		t.Logf("servers:  %q", tc.servers)
+    		if err := ns.AddAdapter(ctx, &guestresource.LCOWNetworkAdapter{
+    			ID:            id,
+    			DNSSuffix:     tc.searches,
+    			DNSServerList: tc.servers,
+    		}); err != nil {
+    			t.Fatalf("failed to add adapter: %v", err)
+    		}
+    		t.Cleanup(func() {
+    			if err := ns.RemoveAdapter(ctx, id); err != nil {
+    				t.Errorf("failed to remove adapter: %v", err)
+    			}
+    		})
+    		searches, servers := ns.dnsConfig(ctx)
+    		if diff := cmp.Diff(tc.wantSearches, searches, cmpopts.EquateEmpty()); diff != "" {
+    			t.Errorf("DNS searches mismatch (-want +got):\n%s", diff)
+    		}
+    		if diff := cmp.Diff(tc.wantServers, servers, cmpopts.EquateEmpty()); diff != "" {
+    			t.Errorf("DNS servers mismatch (-want +got):\n%s", diff)
+    		}
+    		if t.Failed() {
+    			// don't keep failing if one of the DNS config arrays is invalid
+    			break
+    		}
+    	}
+    }

-Original file line number
+Diff line change
@@ Expand Up @@
     		// Networking is skipped, do not error out
     		log.G(ctx).Infof("setupSandboxContainerSpec: Did not find NS spec %v, err %v", spec, err)
     	} else {
-    		var searches, servers []string
-    		for _, n := range ns.Adapters() {
-    			if len(n.DNSSuffix) > 0 {
-    				searches = network.MergeValues(searches, strings.Split(n.DNSSuffix, ","))
-    			}
-    			if len(n.DNSServerList) > 0 {
-    				servers = network.MergeValues(servers, strings.Split(n.DNSServerList, ","))
-    			}
-    		}
+    		searches, servers := ns.dnsConfig(ctx)
     		resolvContent, err := network.GenerateResolvConfContent(ctx, searches, servers, nil)
     		if err != nil {
     			return errors.Wrap(err, "failed to generate sandbox resolv.conf content")
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up @@
     		req.Routes = append(req.Routes, newRoute)
     	}
-    	req.DNSSuffix = endpoint.Dns.Domain
+    	// !NOTE:
+    	// the `DNSSuffix` field is explicitly used as the search list for host-name lookup in
+    	// the guest's `resolv.conf`, and not as the DNS suffix.
+    	// The name is a legacy hold over.
+    	// get the non-empty DNS search names, using the domain as the first (default) value
+    	searches := slices.DeleteFunc(
+    		append([]string{endpoint.Dns.Domain}, endpoint.Dns.Search...),
+    		func(s string) bool { return s == "" })
+    	req.DNSSuffix = strings.Join(searches, ",")
     	req.DNSServerList = strings.Join(endpoint.Dns.ServerList, ",")
     	for _, p := range endpoint.Policies {
@@ Expand Down @@

Fix LCOW DNS search bug #2594

Are you sure you want to change the base?

Fix LCOW DNS search bug #2594

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

rawahars Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

helsaawy Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!