Add scope filter in /networks/<id>

This fix tries to add a `scope` in the query of `/networks/<id>`
(`NetworkInspect`) so that in case of duplicate network names,
it is possible to locate the network ID based on the network
scope (`local`, 'swarm', or `global`).

Multiple networks might exist in different scopes, which is a legitimate case.
For example, a network name `foo` might exists locally and in swarm network.

However, before this PR it was not possible to query a network name `foo`
in a specific scope like swarm.

This fix fixes the issue by allowing a `scope` query in `/networks/<id>`.

Additional test cases have been added to unit tests and integration tests.

This fix is related to docker/cli#167, moby/moby#30897, moby/moby#33561, moby/moby#30242

This fix fixes docker/cli#167

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

api/server/router/network/network_routes.go

@@ -98,6 +98,14 @@ func (n *networkRouter) getNetwork(ctx context.Context, w http.ResponseWriter, r
 			return errors.NewBadRequestError(err)
 		}
 	}
+	scope := r.URL.Query().Get("scope")
+
+	isMatchingScope := func(scope, term string) bool {
+		if term != "" {
+			return scope == term
+		}
+		return true
+	}
 
 	// In case multiple networks have duplicate names, return error.
 	// TODO (yongtang): should we wrap with version here for backward compatibility?
@@ -112,15 +120,15 @@ func (n *networkRouter) getNetwork(ctx context.Context, w http.ResponseWriter, r
 
 	nw := n.backend.GetNetworks()
 	for _, network := range nw {
-		if network.ID() == term {
+		if network.ID() == term && isMatchingScope(network.Info().Scope(), scope) {
 			return httputils.WriteJSON(w, http.StatusOK, *n.buildDetailedNetworkResources(network, verbose))
 		}
-		if network.Name() == term {
+		if network.Name() == term && isMatchingScope(network.Info().Scope(), scope) {
 			// No need to check the ID collision here as we are still in
 			// local scope and the network ID is unique in this scope.
 			listByFullName[network.ID()] = *n.buildDetailedNetworkResources(network, verbose)
 		}
-		if strings.HasPrefix(network.ID(), term) {
+		if strings.HasPrefix(network.ID(), term) && isMatchingScope(network.Info().Scope(), scope) {
 			// No need to check the ID collision here as we are still in
 			// local scope and the network ID is unique in this scope.
 			listByPartialID[network.ID()] = *n.buildDetailedNetworkResources(network, verbose)
@@ -129,10 +137,10 @@ func (n *networkRouter) getNetwork(ctx context.Context, w http.ResponseWriter, r
 
 	nr, _ := n.cluster.GetNetworks()
 	for _, network := range nr {
-		if network.ID == term {
+		if network.ID == term && isMatchingScope(network.Scope, scope) {
 			return httputils.WriteJSON(w, http.StatusOK, network)
 		}
-		if network.Name == term {
+		if network.Name == term && isMatchingScope(network.Scope, scope) {
 			// Check the ID collision as we are in swarm scope here, and
 			// the map (of the listByFullName) may have already had a
 			// network with the same ID (from local scope previously)
@@ -140,7 +148,7 @@ func (n *networkRouter) getNetwork(ctx context.Context, w http.ResponseWriter, r
 				listByFullName[network.ID] = network
 			}
 		}
-		if strings.HasPrefix(network.ID, term) {
+		if strings.HasPrefix(network.ID, term) && isMatchingScope(network.Scope, scope) {
 			// Check the ID collision as we are in swarm scope here, and
 			// the map (of the listByPartialID) may have already had a
 			// network with the same ID (from local scope previously)

api/swagger.yaml

@@ -6437,6 +6437,10 @@ paths:
           description: "Detailed inspect output for troubleshooting"
           type: "boolean"
           default: false
+        - name: "scope"
+          in: "query"
+          description: "Filter the network by scope (swarm, global, or local)"
+          type: "string"
       tags: ["Network"]
 
     delete:

api/types/types.go

@@ -468,6 +468,12 @@ type NetworkDisconnect struct {
 	Force     bool
 }
 
+// NetworkInspectOptions holds parameters to inspect network
+type NetworkInspectOptions struct {
+	Scope   string
+	Verbose bool
+}
+
 // Checkpoint represents the details of a checkpoint
 type Checkpoint struct {
 	Name string // Name is the name of the checkpoint

client/interface.go

@@ -99,8 +99,8 @@ type NetworkAPIClient interface {
 	NetworkConnect(ctx context.Context, networkID, container string, config *network.EndpointSettings) error
 	NetworkCreate(ctx context.Context, name string, options types.NetworkCreate) (types.NetworkCreateResponse, error)
 	NetworkDisconnect(ctx context.Context, networkID, container string, force bool) error
-	NetworkInspect(ctx context.Context, networkID string, verbose bool) (types.NetworkResource, error)
-	NetworkInspectWithRaw(ctx context.Context, networkID string, verbose bool) (types.NetworkResource, []byte, error)
+	NetworkInspect(ctx context.Context, networkID string, options types.NetworkInspectOptions) (types.NetworkResource, error)
+	NetworkInspectWithRaw(ctx context.Context, networkID string, options types.NetworkInspectOptions) (types.NetworkResource, []byte, error)
 	NetworkList(ctx context.Context, options types.NetworkListOptions) ([]types.NetworkResource, error)
 	NetworkRemove(ctx context.Context, networkID string) error
 	NetworksPrune(ctx context.Context, pruneFilter filters.Args) (types.NetworksPruneReport, error)

client/network_inspect.go

@@ -12,22 +12,25 @@ import (
 )
 
 // NetworkInspect returns the information for a specific network configured in the docker host.
-func (cli *Client) NetworkInspect(ctx context.Context, networkID string, verbose bool) (types.NetworkResource, error) {
-	networkResource, _, err := cli.NetworkInspectWithRaw(ctx, networkID, verbose)
+func (cli *Client) NetworkInspect(ctx context.Context, networkID string, options types.NetworkInspectOptions) (types.NetworkResource, error) {
+	networkResource, _, err := cli.NetworkInspectWithRaw(ctx, networkID, options)
 	return networkResource, err
 }
 
 // NetworkInspectWithRaw returns the information for a specific network configured in the docker host and its raw representation.
-func (cli *Client) NetworkInspectWithRaw(ctx context.Context, networkID string, verbose bool) (types.NetworkResource, []byte, error) {
+func (cli *Client) NetworkInspectWithRaw(ctx context.Context, networkID string, options types.NetworkInspectOptions) (types.NetworkResource, []byte, error) {
 	var (
 		networkResource types.NetworkResource
 		resp            serverResponse
 		err             error
 	)
 	query := url.Values{}
-	if verbose {
+	if options.Verbose {
 		query.Set("verbose", "true")
 	}
+	if options.Scope != "" {
+		query.Set("scope", options.Scope)
+	}
 	resp, err = cli.get(ctx, "/networks/"+networkID, query, nil)
 	if err != nil {
 		if resp.statusCode == http.StatusNotFound {

client/network_inspect_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/network"
+	"github.com/stretchr/testify/assert"
 	"golang.org/x/net/context"
 )
 
@@ -19,7 +20,7 @@ func TestNetworkInspectError(t *testing.T) {
 		client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")),
 	}
 
-	_, err := client.NetworkInspect(context.Background(), "nothing", false)
+	_, err := client.NetworkInspect(context.Background(), "nothing", types.NetworkInspectOptions{})
 	if err == nil || err.Error() != "Error response from daemon: Server error" {
 		t.Fatalf("expected a Server Error, got %v", err)
 	}
@@ -30,7 +31,7 @@ func TestNetworkInspectContainerNotFound(t *testing.T) {
 		client: newMockClient(errorMock(http.StatusNotFound, "Server error")),
 	}
 
-	_, err := client.NetworkInspect(context.Background(), "unknown", false)
+	_, err := client.NetworkInspect(context.Background(), "unknown", types.NetworkInspectOptions{})
 	if err == nil || !IsErrNetworkNotFound(err) {
 		t.Fatalf("expected a networkNotFound error, got %v", err)
 	}
@@ -51,7 +52,14 @@ func TestNetworkInspect(t *testing.T) {
 				content []byte
 				err     error
 			)
-			if strings.HasPrefix(req.URL.RawQuery, "verbose=true") {
+			if strings.Contains(req.URL.RawQuery, "scope=global") {
+				return &http.Response{
+					StatusCode: http.StatusNotFound,
+					Body:       ioutil.NopCloser(bytes.NewReader(content)),
+				}, nil
+			}
+
+			if strings.Contains(req.URL.RawQuery, "verbose=true") {
 				s := map[string]network.ServiceInfo{
 					"web": {},
 				}
@@ -74,7 +82,7 @@ func TestNetworkInspect(t *testing.T) {
 		}),
 	}
 
-	r, err := client.NetworkInspect(context.Background(), "network_id", false)
+	r, err := client.NetworkInspect(context.Background(), "network_id", types.NetworkInspectOptions{})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -82,7 +90,7 @@ func TestNetworkInspect(t *testing.T) {
 		t.Fatalf("expected `mynetwork`, got %s", r.Name)
 	}
 
-	r, err = client.NetworkInspect(context.Background(), "network_id", true)
+	r, err = client.NetworkInspect(context.Background(), "network_id", types.NetworkInspectOptions{Verbose: true})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -93,4 +101,7 @@ func TestNetworkInspect(t *testing.T) {
 	if !ok {
 		t.Fatalf("expected service `web` missing in the verbose output")
 	}
+
+	_, err = client.NetworkInspect(context.Background(), "network_id", types.NetworkInspectOptions{Scope: "global"})
+	assert.EqualError(t, err, "Error: No such network: network_id")
 }

docs/api/version-history.md

@@ -21,6 +21,7 @@ keywords: "API, Docker, rcli, REST, documentation"
 * `POST /secrets/create` now returns status code 409 instead of 500 when creating an already existing secret.
 * `POST /secrets/(name)/update` now returns status code 400 instead of 500 when updating a secret's content which is not the labels.
 * `POST /nodes/(name)/update` now returns status code 400 instead of 500 when demoting last node fails.
+* `GET /networks/(id or name)` now takes an optional query parameter `scope` that will filter the network based on the scope (`local`, `swarm`, or `global`).
 
 ## v1.30 API changes
 

integration-cli/docker_api_swarm_test.go

@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"net"
 	"net/http"
+	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
@@ -1002,3 +1003,36 @@ func (s *DockerSwarmSuite) TestSwarmRepeatedRootRotation(c *check.C) {
 		currentTrustRoot = clusterTLSInfo.TrustRoot
 	}
 }
+
+func (s *DockerSwarmSuite) TestAPINetworkInspectWithScope(c *check.C) {
+	d := s.AddDaemon(c, true, true)
+
+	name := "foo"
+	networkCreateRequest := types.NetworkCreateRequest{
+		Name: name,
+	}
+
+	var n types.NetworkCreateResponse
+	networkCreateRequest.NetworkCreate.Driver = "overlay"
+
+	status, out, err := d.SockRequest("POST", "/networks/create", networkCreateRequest)
+	c.Assert(err, checker.IsNil, check.Commentf(string(out)))
+	c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(out)))
+	c.Assert(json.Unmarshal(out, &n), checker.IsNil)
+
+	var r types.NetworkResource
+
+	status, body, err := d.SockRequest("GET", "/networks/"+name, nil)
+	c.Assert(err, checker.IsNil, check.Commentf(string(out)))
+	c.Assert(status, checker.Equals, http.StatusOK, check.Commentf(string(out)))
+	c.Assert(json.Unmarshal(body, &r), checker.IsNil)
+	c.Assert(r.Scope, checker.Equals, "swarm")
+	c.Assert(r.ID, checker.Equals, n.ID)
+
+	v := url.Values{}
+	v.Set("scope", "local")
+
+	status, body, err = d.SockRequest("GET", "/networks/"+name+"?"+v.Encode(), nil)
+	c.Assert(err, checker.IsNil, check.Commentf(string(out)))
+	c.Assert(status, checker.Equals, http.StatusNotFound, check.Commentf(string(out)))
+}