diff --git a/daemon/network.go b/daemon/network.go index bf8a2f53d2..667db62e62 100644 --- a/daemon/network.go +++ b/daemon/network.go @@ -31,18 +31,27 @@ func (daemon *Daemon) NetworkControllerEnabled() bool { // FindNetwork function finds a network for a given string that can represent network name or id func (daemon *Daemon) FindNetwork(idName string) (libnetwork.Network, error) { - // Find by Name - n, err := daemon.GetNetworkByName(idName) - if err != nil && !isNoSuchNetworkError(err) { - return nil, err + // 1. match by full ID. + n, err := daemon.GetNetworkByID(idName) + if err == nil || !isNoSuchNetworkError(err) { + return n, err } - if n != nil { - return n, nil + // 2. match by full name + n, err = daemon.GetNetworkByName(idName) + if err == nil || !isNoSuchNetworkError(err) { + return n, err } - // Find by id - return daemon.GetNetworkByID(idName) + // 3. match by ID prefix + list := daemon.GetNetworksByIDPrefix(idName) + if len(list) == 0 { + return nil, errors.WithStack(networkNotFound(idName)) + } + if len(list) > 1 { + return nil, errors.WithStack(invalidIdentifier(idName)) + } + return list[0], nil } func isNoSuchNetworkError(err error) bool { @@ -50,18 +59,14 @@ func isNoSuchNetworkError(err error) bool { return ok } -// GetNetworkByID function returns a network whose ID begins with the given prefix. -// It fails with an error if no matching, or more than one matching, networks are found. -func (daemon *Daemon) GetNetworkByID(partialID string) (libnetwork.Network, error) { - list := daemon.GetNetworksByID(partialID) - - if len(list) == 0 { - return nil, errors.WithStack(networkNotFound(partialID)) +// GetNetworkByID function returns a network whose ID matches the given ID. +// It fails with an error if no matching network is found. +func (daemon *Daemon) GetNetworkByID(id string) (libnetwork.Network, error) { + c := daemon.netController + if c == nil { + return nil, libnetwork.ErrNoSuchNetwork(id) } - if len(list) > 1 { - return nil, errors.WithStack(invalidIdentifier(partialID)) - } - return list[0], nil + return c.NetworkByID(id) } // GetNetworkByName function returns a network for a given network name. @@ -77,8 +82,8 @@ func (daemon *Daemon) GetNetworkByName(name string) (libnetwork.Network, error) return c.NetworkByName(name) } -// GetNetworksByID returns a list of networks whose ID partially matches zero or more networks -func (daemon *Daemon) GetNetworksByID(partialID string) []libnetwork.Network { +// GetNetworksByIDPrefix returns a list of networks whose ID partially matches zero or more networks +func (daemon *Daemon) GetNetworksByIDPrefix(partialID string) []libnetwork.Network { c := daemon.netController if c == nil { return nil diff --git a/docs/api/version-history.md b/docs/api/version-history.md index e835629b94..9ac31dcaaf 100644 --- a/docs/api/version-history.md +++ b/docs/api/version-history.md @@ -31,6 +31,11 @@ keywords: "API, Docker, rcli, REST, documentation" * `POST /containers/create` now accepts additional values for the `HostConfig.IpcMode` property. New values are `private`, `shareable`, and `none`. +* `DELETE /networks/{id or name}` fixed issue where a `name` equal to another + network's name was able to mask that `id`. If both a network with the given + _name_ exists, and a network with the given _id_, the network with the given + _id_ is now deleted. This change is not versioned, and affects all API versions + if the daemon has this patch. ## v1.31 API changes diff --git a/integration/network/delete_test.go b/integration/network/delete_test.go new file mode 100644 index 0000000000..0a3b219bf2 --- /dev/null +++ b/integration/network/delete_test.go @@ -0,0 +1,72 @@ +package network + +import ( + "context" + "testing" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/integration/util/request" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func containsNetwork(nws []types.NetworkResource, nw types.NetworkCreateResponse) bool { + for _, n := range nws { + if n.ID == nw.ID { + return true + } + } + return false +} + +// createAmbiguousNetworks creates three networks, of which the second network +// uses a prefix of the first network's ID as name. The third network uses the +// first network's ID as name. +// +// After successful creation, properties of all three networks is returned +func createAmbiguousNetworks(t *testing.T) (types.NetworkCreateResponse, types.NetworkCreateResponse, types.NetworkCreateResponse) { + client := request.NewAPIClient(t) + ctx := context.Background() + + testNet, err := client.NetworkCreate(ctx, "testNet", types.NetworkCreate{}) + require.NoError(t, err) + idPrefixNet, err := client.NetworkCreate(ctx, testNet.ID[:12], types.NetworkCreate{}) + require.NoError(t, err) + fullIDNet, err := client.NetworkCreate(ctx, testNet.ID, types.NetworkCreate{}) + require.NoError(t, err) + + nws, err := client.NetworkList(ctx, types.NetworkListOptions{}) + require.NoError(t, err) + + assert.Equal(t, true, containsNetwork(nws, testNet), "failed to create network testNet") + assert.Equal(t, true, containsNetwork(nws, idPrefixNet), "failed to create network idPrefixNet") + assert.Equal(t, true, containsNetwork(nws, fullIDNet), "failed to create network fullIDNet") + return testNet, idPrefixNet, fullIDNet +} + +// TestDockerNetworkDeletePreferID tests that if a network with a name +// equal to another network's ID exists, the Network with the given +// ID is removed, and not the network with the given name. +func TestDockerNetworkDeletePreferID(t *testing.T) { + defer setupTest(t)() + client := request.NewAPIClient(t) + ctx := context.Background() + testNet, idPrefixNet, fullIDNet := createAmbiguousNetworks(t) + + // Delete the network using a prefix of the first network's ID as name. + // This should the network name with the id-prefix, not the original network. + err := client.NetworkRemove(ctx, testNet.ID[:12]) + require.NoError(t, err) + + // Delete the network using networkID. This should remove the original + // network, not the network with the name equal to the networkID + err = client.NetworkRemove(ctx, testNet.ID) + require.NoError(t, err) + + // networks "testNet" and "idPrefixNet" should be removed, but "fullIDNet" should still exist + nws, err := client.NetworkList(ctx, types.NetworkListOptions{}) + require.NoError(t, err) + assert.Equal(t, false, containsNetwork(nws, testNet), "Network testNet not removed") + assert.Equal(t, false, containsNetwork(nws, idPrefixNet), "Network idPrefixNet not removed") + assert.Equal(t, true, containsNetwork(nws, fullIDNet), "Network fullIDNet not found") +} diff --git a/integration/network/main_test.go b/integration/network/main_test.go new file mode 100644 index 0000000000..fffe37dcc4 --- /dev/null +++ b/integration/network/main_test.go @@ -0,0 +1,28 @@ +package network + +import ( + "fmt" + "os" + "testing" + + "github.com/docker/docker/internal/test/environment" +) + +var testEnv *environment.Execution + +func TestMain(m *testing.M) { + var err error + testEnv, err = environment.New() + if err != nil { + fmt.Println(err) + os.Exit(1) + } + + testEnv.Print() + os.Exit(m.Run()) +} + +func setupTest(t *testing.T) func() { + environment.ProtectAll(t, testEnv) + return func() { testEnv.Clean(t) } +}