daemon/network: match any type filter value

matchesType tests a network against only the first value of the type
filter. The values come from a map. Their order is random and changes
from one network to the next. A GET /networks request that filters on
both type=builtin and type=custom lists a random subset of the
networks. Before networks were filtered one at a time, the daemon
filtered the list once for each type value and listed every network
that matched any of them.

Match a network when its type is any of the filter's values.

integration-cli's TestDockerNetworkLsFilter checks this request but is
skipped in every run. Its OnlyDefaultNetworks requirement is met only
when GET /networks lists no networks at all. The daemon always lists
the predefined bridge, host and none networks. Port the test to
integration/network as TestNetworkListFilters, which uses the API
client. The new test checks the same id, name, type, label and driver
filter terms against two networks that it creates and the predefined
networks. It derives the expected results for type=custom and
driver=bridge from the unfiltered list, as the daemon can have other
networks. Remove the integration-cli test, its assertNwList helper and
the OnlyDefaultNetworks requirement. Nothing else uses them.

Add the combined type values to the filter unit test as well. Both
tests fail on the combined values without the fix.

Signed-off-by: Cory Snider <csnider@mirantis.com>

daemon/network/filter.go

@@ -1,6 +1,7 @@
 package network
 
 import (
+	"slices"
 	"time"
 
 	"github.com/moby/moby/v2/daemon/internal/filters"
@@ -176,13 +177,8 @@ func matchesUse(danglingOnly bool, nw FilterNetwork) bool {
 }
 
 func matchesType(netTypes []string, nw FilterNetwork) bool {
-	for _, netType := range netTypes {
-		switch netType {
-		case "builtin":
-			return IsPredefined(nw.Name())
-		case "custom":
-			return !IsPredefined(nw.Name())
-		}
+	if IsPredefined(nw.Name()) {
+		return slices.Contains(netTypes, "builtin")
 	}
-	return false
+	return slices.Contains(netTypes, "custom")
 }

daemon/network/filter_test.go

@@ -173,6 +173,11 @@ func TestFilter(t *testing.T) {
 			filter:  filters.NewArgs(filters.Arg("type", "builtin")),
 			results: []string{network.NetworkHost, network.NetworkBridge, network.NetworkNone},
 		},
+		{
+			subtest: "type=builtin,type=custom",
+			filter:  filters.NewArgs(filters.Arg("type", "builtin"), filters.Arg("type", "custom")),
+			results: []string{network.NetworkHost, network.NetworkBridge, network.NetworkNone, "myoverlay", "mydrivernet", "mykvnet", "networkwithcontainer", "networkwithservice", "idoverlap"},
+		},
 		{
 			subtest: "type=invalid",
 			filter:  filters.NewArgs(filters.Arg("type", "invalid")),

integration-cli/docker_cli_network_unix_test.go

@@ -273,22 +273,6 @@ func isNwPresent(t *testing.T, name string) bool {
 	return false
 }
 
-// assertNwList checks network list retrieved with ls command
-// equals to expected network list
-// note: out should be `network ls [option]` result
-func assertNwList(t *testing.T, out string, expectNws []string) {
-	lines := strings.Split(out, "\n")
-	var nwList []string
-	for _, line := range lines[1 : len(lines)-1] {
-		netFields := strings.Fields(line)
-		// wrap all network name in nwList
-		nwList = append(nwList, netFields[1])
-	}
-
-	// network ls should contains all expected networks
-	assert.DeepEqual(t, nwList, expectNws)
-}
-
 func getNwResource(t *testing.T, name string) *network.Inspect {
 	out := cli.DockerCmd(t, "network", "inspect", name).Stdout()
 	var nr []network.Inspect
@@ -306,63 +290,6 @@ func (s *DockerNetworkSuite) TestDockerNetworkCreateHostBind(c *testing.T) {
 	assert.Assert(c, is.Contains(out, "192.168.10.1:5000->5000/tcp"))
 }
 
-func (s *DockerNetworkSuite) TestDockerNetworkLsFilter(c *testing.T) {
-	testRequires(c, func() bool { return OnlyDefaultNetworks(testutil.GetContext(c)) })
-
-	testNet := "testnet1"
-	testLabel := "foo"
-	testValue := "bar"
-	out := cli.DockerCmd(c, "network", "create", "dev").Stdout()
-	defer func() {
-		cli.DockerCmd(c, "network", "rm", "dev")
-		cli.DockerCmd(c, "network", "rm", testNet)
-	}()
-	networkID := strings.TrimSpace(out)
-
-	// filter with partial ID
-	// only show 'dev' network
-	out = cli.DockerCmd(c, "network", "ls", "-f", "id="+networkID[0:5]).Stdout()
-	assertNwList(c, out, []string{"dev"})
-
-	out = cli.DockerCmd(c, "network", "ls", "-f", "name=dge").Stdout()
-	assertNwList(c, out, []string{"bridge"})
-
-	// only show built-in network (bridge, none, host)
-	out = cli.DockerCmd(c, "network", "ls", "-f", "type=builtin").Stdout()
-	assertNwList(c, out, []string{"bridge", "host", "none"})
-
-	// only show custom networks (dev)
-	out = cli.DockerCmd(c, "network", "ls", "-f", "type=custom").Stdout()
-	assertNwList(c, out, []string{"dev"})
-
-	// show all networks with filter
-	// it should be equivalent of ls without option
-	out = cli.DockerCmd(c, "network", "ls", "-f", "type=custom", "-f", "type=builtin").Stdout()
-	assertNwList(c, out, []string{"bridge", "dev", "host", "none"})
-
-	cli.DockerCmd(c, "network", "create", "--label", testLabel+"="+testValue, testNet)
-	assertNwIsAvailable(c, testNet)
-
-	out = cli.DockerCmd(c, "network", "ls", "-f", "label="+testLabel).Stdout()
-	assertNwList(c, out, []string{testNet})
-
-	out = cli.DockerCmd(c, "network", "ls", "-f", "label="+testLabel+"="+testValue).Stdout()
-	assertNwList(c, out, []string{testNet})
-
-	out = cli.DockerCmd(c, "network", "ls", "-f", "label=nonexistent").Stdout()
-	outArr := strings.Split(strings.TrimSpace(out), "\n")
-	assert.Equal(c, len(outArr), 1, out+"\n")
-
-	out = cli.DockerCmd(c, "network", "ls", "-f", "driver=null").Stdout()
-	assertNwList(c, out, []string{"none"})
-
-	out = cli.DockerCmd(c, "network", "ls", "-f", "driver=host").Stdout()
-	assertNwList(c, out, []string{"host"})
-
-	out = cli.DockerCmd(c, "network", "ls", "-f", "driver=bridge").Stdout()
-	assertNwList(c, out, []string{"bridge", "dev", testNet})
-}
-
 func (s *DockerNetworkSuite) TestDockerNetworkCreateDelete(c *testing.T) {
 	cli.DockerCmd(c, "network", "create", "test")
 	assertNwIsAvailable(c, "test")

integration-cli/requirements_test.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"context"
 	"errors"
 	"net"
 	"net/http"
@@ -16,7 +15,6 @@ import (
 
 	"github.com/containerd/containerd/v2/plugins"
 	"github.com/moby/moby/api/types/swarm"
-	"github.com/moby/moby/client"
 	"github.com/moby/moby/v2/internal/testutil/registry"
 )
 
@@ -28,18 +26,6 @@ func DaemonIsLinux() bool {
 	return testEnv.DaemonInfo.OSType == "linux"
 }
 
-func OnlyDefaultNetworks(ctx context.Context) bool {
-	apiClient, err := client.New(client.FromEnv)
-	if err != nil {
-		return false
-	}
-	res, err := apiClient.NetworkList(ctx, client.NetworkListOptions{})
-	if err != nil || len(res.Items) > 0 {
-		return false
-	}
-	return true
-}
-
 func IsAmd64() bool {
 	return testEnv.DaemonInfo.Architecture == "amd64"
 }

integration/network/network_linux_test.go

@@ -232,6 +232,121 @@ func TestForbidDuplicateNetworkNames(t *testing.T) {
 	assert.Error(t, err, "Error response from daemon: network with name testnet already exists", "2nd NetworkCreate call should have failed")
 }
 
+// TestNetworkListFilters checks the id, name, type, label and driver filter
+// terms of GET /networks. Expectations that depend on which other networks
+// the daemon has are derived from the unfiltered list.
+func TestNetworkListFilters(t *testing.T) {
+	ctx := setupTest(t)
+	apiClient := testEnv.APIClient()
+
+	const (
+		devName      = "test-list-filters-dev"
+		labelledName = "test-list-filters-labelled"
+		labelKey     = "com.example.test-list-filters"
+		labelValue   = "bar"
+	)
+	devID := network.CreateNoError(ctx, t, apiClient, devName)
+	defer network.RemoveNoError(ctx, t, apiClient, devName)
+	network.CreateNoError(ctx, t, apiClient, labelledName, func(o *client.NetworkCreateOptions) {
+		o.Labels = map[string]string{labelKey: labelValue}
+	})
+	defer network.RemoveNoError(ctx, t, apiClient, labelledName)
+
+	list := func(t *testing.T, filters client.Filters) []networktypes.Summary {
+		t.Helper()
+		res, err := apiClient.NetworkList(ctx, client.NetworkListOptions{Filters: filters})
+		assert.NilError(t, err)
+		return res.Items
+	}
+	names := func(nws []networktypes.Summary, match func(networktypes.Summary) bool) []string {
+		out := []string{}
+		for _, nw := range nws {
+			if match(nw) {
+				out = append(out, nw.Name)
+			}
+		}
+		slices.Sort(out)
+		return out
+	}
+	anyNetwork := func(networktypes.Summary) bool { return true }
+
+	all := list(t, nil)
+	assert.Assert(t, is.Contains(names(all, anyNetwork), devName))
+	assert.Assert(t, is.Contains(names(all, anyNetwork), labelledName))
+	predefined := []string{networktypes.NetworkBridge, networktypes.NetworkHost, networktypes.NetworkNone}
+
+	tests := []struct {
+		name     string
+		filters  client.Filters
+		expected []string
+	}{
+		{
+			name:     "id prefix",
+			filters:  make(client.Filters).Add("id", devID[:12]),
+			expected: []string{devName},
+		},
+		{
+			name:     "name substring",
+			filters:  make(client.Filters).Add("name", "list-filters-dev"),
+			expected: []string{devName},
+		},
+		{
+			name:     "type=builtin",
+			filters:  make(client.Filters).Add("type", "builtin"),
+			expected: predefined,
+		},
+		{
+			name:    "type=custom",
+			filters: make(client.Filters).Add("type", "custom"),
+			expected: names(all, func(nw networktypes.Summary) bool {
+				return !slices.Contains(predefined, nw.Name)
+			}),
+		},
+		{
+			name:     "type=builtin and type=custom",
+			filters:  make(client.Filters).Add("type", "builtin", "custom"),
+			expected: names(all, anyNetwork),
+		},
+		{
+			name:     "label key",
+			filters:  make(client.Filters).Add("label", labelKey),
+			expected: []string{labelledName},
+		},
+		{
+			name:     "label key=value",
+			filters:  make(client.Filters).Add("label", labelKey+"="+labelValue),
+			expected: []string{labelledName},
+		},
+		{
+			name:     "label key not set",
+			filters:  make(client.Filters).Add("label", labelKey+".unset"),
+			expected: []string{},
+		},
+		{
+			name:     "driver=null",
+			filters:  make(client.Filters).Add("driver", "null"),
+			expected: []string{networktypes.NetworkNone},
+		},
+		{
+			name:     "driver=host",
+			filters:  make(client.Filters).Add("driver", "host"),
+			expected: []string{networktypes.NetworkHost},
+		},
+		{
+			name:    "driver=bridge",
+			filters: make(client.Filters).Add("driver", "bridge"),
+			expected: names(all, func(nw networktypes.Summary) bool {
+				return nw.Driver == "bridge"
+			}),
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Check(t, is.DeepEqual(names(list(t, tc.filters), anyNetwork), tc.expected))
+		})
+	}
+}
+
 // TestHostGatewayFromDocker0 checks that, when docker0 has IPv6, host-gateway maps to both IPv4 and IPv6.
 func TestHostGatewayFromDocker0(t *testing.T) {
 	ctx := testutil.StartSpan(baseContext, t)