diff --git a/components/engine/api/server/router/network/backend.go b/components/engine/api/server/router/network/backend.go index 0d1dfb0123..000ace6d66 100644 --- a/components/engine/api/server/router/network/backend.go +++ b/components/engine/api/server/router/network/backend.go @@ -11,8 +11,6 @@ import ( // to provide network specific functionality. type Backend interface { FindNetwork(idName string) (libnetwork.Network, error) - GetNetworkByName(idName string) (libnetwork.Network, error) - GetNetworksByID(partialID string) []libnetwork.Network GetNetworks() []libnetwork.Network CreateNetwork(nc types.NetworkCreateRequest) (*types.NetworkCreateResponse, error) ConnectContainerToNetwork(containerName, networkName string, endpointConfig *network.EndpointSettings) error diff --git a/components/engine/api/server/router/network/network_routes.go b/components/engine/api/server/router/network/network_routes.go index f9df7d57b5..970a391f3d 100644 --- a/components/engine/api/server/router/network/network_routes.go +++ b/components/engine/api/server/router/network/network_routes.go @@ -2,7 +2,9 @@ package network import ( "encoding/json" + "fmt" "net/http" + "strings" "golang.org/x/net/context" @@ -82,14 +84,80 @@ func (n *networkRouter) getNetwork(ctx context.Context, w http.ResponseWriter, r return err } - nw, err := n.backend.FindNetwork(vars["id"]) - if err != nil { - if nr, err := n.cluster.GetNetwork(vars["id"]); err == nil { - return httputils.WriteJSON(w, http.StatusOK, nr) + term := vars["id"] + + // In case multiple networks have duplicate names, return error. + // TODO (yongtang): should we wrap with version here for backward compatibility? + + // First find based on full ID, return immediately once one is found. + // If a network appears both in swarm and local, assume it is in local first + + // For full name and partial ID, save the result first, and process later + // in case multiple records was found based on the same term + listByFullName := map[string]types.NetworkResource{} + listByPartialID := map[string]types.NetworkResource{} + + nw := n.backend.GetNetworks() + for _, network := range nw { + if network.ID() == term { + return httputils.WriteJSON(w, http.StatusOK, *n.buildDetailedNetworkResources(network)) + } + if network.Name() == term { + // 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) + } + if strings.HasPrefix(network.ID(), term) { + // 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) } - return err } - return httputils.WriteJSON(w, http.StatusOK, n.buildDetailedNetworkResources(nw)) + + nr, _ := n.cluster.GetNetworks() + for _, network := range nr { + if network.ID == term { + return httputils.WriteJSON(w, http.StatusOK, network) + } + if network.Name == term { + // 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) + if _, ok := listByFullName[network.ID]; !ok { + listByFullName[network.ID] = network + } + } + if strings.HasPrefix(network.ID, term) { + // 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) + if _, ok := listByPartialID[network.ID]; !ok { + listByPartialID[network.ID] = network + } + } + } + + // Find based on full name, returns true only if no duplicates + if len(listByFullName) == 1 { + for _, v := range listByFullName { + return httputils.WriteJSON(w, http.StatusOK, v) + } + } + if len(listByFullName) > 1 { + return fmt.Errorf("network %s is ambiguous (%d matches found based on name)", term, len(listByFullName)) + } + + // Find based on partial ID, returns true only if no duplicates + if len(listByPartialID) == 1 { + for _, v := range listByPartialID { + return httputils.WriteJSON(w, http.StatusOK, v) + } + } + if len(listByPartialID) > 1 { + return fmt.Errorf("network %s is ambiguous (%d matches found based on ID prefix)", term, len(listByPartialID)) + } + + return libnetwork.ErrNoSuchNetwork(term) } func (n *networkRouter) postNetworkCreate(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { diff --git a/components/engine/integration-cli/docker_cli_swarm_test.go b/components/engine/integration-cli/docker_cli_swarm_test.go index 3a88a00394..1a64731868 100644 --- a/components/engine/integration-cli/docker_cli_swarm_test.go +++ b/components/engine/integration-cli/docker_cli_swarm_test.go @@ -14,6 +14,7 @@ import ( "strings" "time" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/integration-cli/checker" "github.com/docker/docker/integration-cli/daemon" @@ -1665,3 +1666,77 @@ func (s *DockerSwarmSuite) TestSwarmReadonlyRootfs(c *check.C) { c.Assert(err, checker.IsNil, check.Commentf(out)) c.Assert(strings.TrimSpace(out), checker.Equals, "true") } + +func (s *DockerSwarmSuite) TestNetworkInspectWithDuplicateNames(c *check.C) { + d := s.AddDaemon(c, true, true) + + name := "foo" + networkCreateRequest := types.NetworkCreateRequest{ + Name: name, + NetworkCreate: types.NetworkCreate{ + CheckDuplicate: false, + Driver: "bridge", + }, + } + + var n1 types.NetworkCreateResponse + status, body, err := d.SockRequest("POST", "/networks/create", networkCreateRequest) + c.Assert(err, checker.IsNil, check.Commentf(string(body))) + c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(body))) + c.Assert(json.Unmarshal(body, &n1), checker.IsNil) + + // Full ID always works + out, err := d.Cmd("network", "inspect", "--format", "{{.ID}}", n1.ID) + c.Assert(err, checker.IsNil, check.Commentf(out)) + c.Assert(strings.TrimSpace(out), checker.Equals, n1.ID) + + // Name works if it is unique + out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", name) + c.Assert(err, checker.IsNil, check.Commentf(out)) + c.Assert(strings.TrimSpace(out), checker.Equals, n1.ID) + + var n2 types.NetworkCreateResponse + status, body, err = d.SockRequest("POST", "/networks/create", networkCreateRequest) + c.Assert(err, checker.IsNil, check.Commentf(string(body))) + c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(body))) + c.Assert(json.Unmarshal(body, &n2), checker.IsNil) + + // Full ID always works + out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", n1.ID) + c.Assert(err, checker.IsNil, check.Commentf(out)) + c.Assert(strings.TrimSpace(out), checker.Equals, n1.ID) + + out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", n2.ID) + c.Assert(err, checker.IsNil, check.Commentf(out)) + c.Assert(strings.TrimSpace(out), checker.Equals, n2.ID) + + // Name with duplicates + out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", name) + c.Assert(err, checker.NotNil, check.Commentf(out)) + c.Assert(out, checker.Contains, "network foo is ambiguous (2 matches found based on name)") + + out, err = d.Cmd("network", "rm", n2.ID) + c.Assert(err, checker.IsNil, check.Commentf(out)) + + // Dupliates with name but with different driver + networkCreateRequest.NetworkCreate.Driver = "overlay" + + status, body, err = d.SockRequest("POST", "/networks/create", networkCreateRequest) + c.Assert(err, checker.IsNil, check.Commentf(string(body))) + c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(body))) + c.Assert(json.Unmarshal(body, &n2), checker.IsNil) + + // Full ID always works + out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", n1.ID) + c.Assert(err, checker.IsNil, check.Commentf(out)) + c.Assert(strings.TrimSpace(out), checker.Equals, n1.ID) + + out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", n2.ID) + c.Assert(err, checker.IsNil, check.Commentf(out)) + c.Assert(strings.TrimSpace(out), checker.Equals, n2.ID) + + // Name with duplicates + out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", name) + c.Assert(err, checker.NotNil, check.Commentf(out)) + c.Assert(out, checker.Contains, "network foo is ambiguous (2 matches found based on name)") +}