Skip to content

Commit

Permalink
libnet: Make sure network names are unique
Browse files Browse the repository at this point in the history
Fixes moby#18864, moby#20648, moby#33561, moby#40901.

[This GH comment][1] makes clear network name uniqueness has never been
enforced due to the eventually consistent nature of Classic Swarm
datastores:

> there is no guaranteed way to check for duplicates across a cluster of
> docker hosts.

And this is further confirmed by other comments made by @mrjana in that
same issue, eg. [this one][2]:

> we want to adopt a schema which can pave the way in the future for a
> completely decentralized cluster of docker hosts (if scalability is
> needed).

This decentralized model is what Classic Swarm was. It's been superseded
since then by Docker Swarm, which has a centralized control plane.

To circumvent this drawback, the `NetworkCreate` endpoint accepts a
`CheckDuplicate` flag. However it's not perfectly reliable as it won't
catch concurrent requests.

Due to this design decision, API clients like Compose have to implement
workarounds to make sure names are really unique (eg.
docker/compose#9585). And the daemon itself has seen a string of issues
due to that decision, including some that aren't fixed to this day (for
instance moby#40901):

> The problem is, that if you specify a network for a container using
> the ID, it will add that network to the container but it will then
> change it to reference the network by using the name.

To summarize, this "feature" is broken, has no practical use and is a
source of pain for Docker users and API consumers. So let's just remove
it for _all_ API versions.

[1]: moby#18864 (comment)
[2]: moby#18864 (comment)

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
  • Loading branch information
akerouanton committed Aug 17, 2023
1 parent d83ead8 commit 280795e
Show file tree
Hide file tree
Showing 40 changed files with 107 additions and 329 deletions.
9 changes: 2 additions & 7 deletions api/swagger.yaml
Expand Up @@ -9958,13 +9958,8 @@ paths:
type: "string"
CheckDuplicate:
description: |
Check for networks with duplicate names. Since Network is
primarily keyed based on a random ID and not on the name, and
network name is strictly a user-friendly alias to the network
which is uniquely identified using ID, there is no guaranteed
way to check for duplicates. CheckDuplicate is there to provide
a best effort checking of any networks which has the same name
but it is not guaranteed to catch all name collisions.
Deprecated: CheckDuplicate is now ignored; a NetworkCreate
request with a duplicated network name will fail.
type: "boolean"
Driver:
description: "Name of the network driver plugin to use."
Expand Down
8 changes: 1 addition & 7 deletions api/types/types.go
Expand Up @@ -443,13 +443,7 @@ type EndpointResource struct {

// NetworkCreate is the expected body of the "create network" http request message
type NetworkCreate struct {
// Check for networks with duplicate names.
// Network is primarily keyed based on a random ID and not on the name.
// Network name is strictly a user-friendly alias to the network
// which is uniquely identified using ID.
// And there is no guaranteed way to check for duplicates.
// Option CheckDuplicate is there to provide a best effort checking of any networks
// which has the same name but it is not guaranteed to catch all name collisions.
// Deprecated: CheckDuplicate is now ignored; a NetworkCreate request with a duplicated network name will fail.
CheckDuplicate bool
Driver string
Scope string
Expand Down
7 changes: 3 additions & 4 deletions client/network_create_test.go
Expand Up @@ -53,10 +53,9 @@ func TestNetworkCreate(t *testing.T) {
}

networkResponse, err := client.NetworkCreate(context.Background(), "mynetwork", types.NetworkCreate{
CheckDuplicate: true,
Driver: "mydriver",
EnableIPv6: true,
Internal: true,
Driver: "mydriver",
EnableIPv6: true,
Internal: true,
Options: map[string]string{
"opt-key": "opt-value",
},
Expand Down
13 changes: 6 additions & 7 deletions daemon/cluster/executor/container/container.go
Expand Up @@ -639,13 +639,12 @@ func (c *containerConfig) networkCreateRequest(name string) (clustertypes.Networ

options := types.NetworkCreate{
// ID: na.Network.ID,
Labels: na.Network.Spec.Annotations.Labels,
Internal: na.Network.Spec.Internal,
Attachable: na.Network.Spec.Attachable,
Ingress: convert.IsIngressNetwork(na.Network),
EnableIPv6: na.Network.Spec.Ipv6Enabled,
CheckDuplicate: true,
Scope: scope.Swarm,
Labels: na.Network.Spec.Annotations.Labels,
Internal: na.Network.Spec.Internal,
Attachable: na.Network.Spec.Attachable,
Ingress: convert.IsIngressNetwork(na.Network),
EnableIPv6: na.Network.Spec.Ipv6Enabled,
Scope: scope.Swarm,
}

if na.Network.Spec.GetNetwork() != "" {
Expand Down
5 changes: 2 additions & 3 deletions daemon/cluster/executor/container/executor.go
Expand Up @@ -219,9 +219,8 @@ func (e *executor) Configure(ctx context.Context, node *api.Node) error {
IPAM: &network.IPAM{
Driver: ingressNA.Network.IPAM.Driver.Name,
},
Options: ingressNA.Network.DriverState.Options,
Ingress: true,
CheckDuplicate: true,
Options: ingressNA.Network.DriverState.Options,
Ingress: true,
}

for _, ic := range ingressNA.Network.IPAM.Configs {
Expand Down
21 changes: 1 addition & 20 deletions daemon/network.go
Expand Up @@ -302,24 +302,6 @@ func (daemon *Daemon) createNetwork(cfg *config.Config, create types.NetworkCrea
return nil, errdefs.Forbidden(errors.New(`This node is not a swarm manager. Use "docker swarm init" or "docker swarm join" to connect this node to swarm and try again.`))
}

var warning string
nw, err := daemon.GetNetworkByName(create.Name)
if err != nil {
if _, ok := err.(libnetwork.ErrNoSuchNetwork); !ok {
return nil, err
}
}
if nw != nil {
// check if user defined CheckDuplicate, if set true, return err
// otherwise prepare a warning message
if create.CheckDuplicate {
if !agent || nw.Dynamic() {
return nil, libnetwork.NetworkNameError(create.Name)
}
}
warning = fmt.Sprintf("Network with name %s (id : %s) already exists", nw.Name(), nw.ID())
}

networkOptions := make(map[string]string)
for k, v := range create.Options {
networkOptions[k] = v
Expand Down Expand Up @@ -388,8 +370,7 @@ func (daemon *Daemon) createNetwork(cfg *config.Config, create types.NetworkCrea
daemon.LogNetworkEvent(n, "create")

return &types.NetworkCreateResponse{
ID: n.ID(),
Warning: warning,
ID: n.ID(),
}, nil
}

Expand Down
7 changes: 1 addition & 6 deletions docs/api/v1.21.md
Expand Up @@ -2855,12 +2855,7 @@ Content-Type: application/json
**JSON parameters**:

- **Name** - The new network's name. this is a mandatory field
- **CheckDuplicate** - Requests daemon to check for networks with same name. Defaults to `false`.
Since Network is primarily keyed based on a random ID and not on the name,
and network name is strictly a user-friendly alias to the network which is uniquely identified using ID,
there is no guaranteed way to check for duplicates across a cluster of docker hosts.
This parameter CheckDuplicate is there to provide a best effort checking of any networks
which has the same name but it is not guaranteed to catch all name collisions.
- **CheckDuplicate** - Deprecated: it's now ignored; a `NetworkCreate` request with a duplicated network name will fail.
- **Driver** - Name of the network driver plugin to use. Defaults to `bridge` driver
- **IPAM** - Optional custom IP scheme for the network
- **Driver** - Name of the IPAM driver to use. Defaults to `default` driver
Expand Down
7 changes: 1 addition & 6 deletions docs/api/v1.22.md
Expand Up @@ -3185,12 +3185,7 @@ Content-Type: application/json
**JSON parameters**:

- **Name** - The new network's name. this is a mandatory field
- **CheckDuplicate** - Requests daemon to check for networks with same name. Defaults to `false`.
Since Network is primarily keyed based on a random ID and not on the name,
and network name is strictly a user-friendly alias to the network
which is uniquely identified using ID, there is no guaranteed way to check for duplicates.
This parameter CheckDuplicate is there to provide a best effort checking of any networks
which has the same name but it is not guaranteed to catch all name collisions.
- **CheckDuplicate** - Deprecated: it's now ignored; a `NetworkCreate` request with a duplicated network name will fail.
- **Driver** - Name of the network driver plugin to use. Defaults to `bridge` driver
- **IPAM** - Optional custom IP scheme for the network
- **Driver** - Name of the IPAM driver to use. Defaults to `default` driver
Expand Down
7 changes: 1 addition & 6 deletions docs/api/v1.23.md
Expand Up @@ -3298,12 +3298,7 @@ Content-Type: application/json
**JSON parameters**:

- **Name** - The new network's name. this is a mandatory field
- **CheckDuplicate** - Requests daemon to check for networks with same name. Defaults to `false`.
Since Network is primarily keyed based on a random ID and not on the name,
and network name is strictly a user-friendly alias to the network
which is uniquely identified using ID, there is no guaranteed way to check for duplicates.
This parameter CheckDuplicate is there to provide a best effort checking of any networks
which has the same name but it is not guaranteed to catch all name collisions.
- **CheckDuplicate** - Deprecated: it's now ignored; a `NetworkCreate` request with a duplicated network name will fail.
- **Driver** - Name of the network driver plugin to use. Defaults to `bridge` driver
- **Internal** - Restrict external access to the network
- **IPAM** - Optional custom IP scheme for the network
Expand Down
7 changes: 1 addition & 6 deletions docs/api/v1.24.md
Expand Up @@ -3366,12 +3366,7 @@ Content-Type: application/json
**JSON parameters**:

- **Name** - The new network's name. this is a mandatory field
- **CheckDuplicate** - Requests daemon to check for networks with same name. Defaults to `false`.
Since Network is primarily keyed based on a random ID and not on the name,
and network name is strictly a user-friendly alias to the network
which is uniquely identified using ID, there is no guaranteed way to check for duplicates.
This parameter CheckDuplicate is there to provide a best effort checking of any networks
which has the same name but it is not guaranteed to catch all name collisions.
- **CheckDuplicate** - Deprecated: it's now ignored; a `NetworkCreate` request with a duplicated network name will fail.
- **Driver** - Name of the network driver plugin to use. Defaults to `bridge` driver
- **Internal** - Restrict external access to the network
- **IPAM** - Optional custom IP scheme for the network
Expand Down
4 changes: 3 additions & 1 deletion docs/api/v1.25.yaml
Expand Up @@ -6174,7 +6174,9 @@ paths:
description: "The network's name."
type: "string"
CheckDuplicate:
description: "Check for networks with duplicate names."
description: |
Deprecated: CheckDuplicate is now ignored; a NetworkCreate
request with a duplicated network name will fail.
type: "boolean"
Driver:
description: "Name of the network driver plugin to use."
Expand Down
4 changes: 3 additions & 1 deletion docs/api/v1.26.yaml
Expand Up @@ -6183,7 +6183,9 @@ paths:
description: "The network's name."
type: "string"
CheckDuplicate:
description: "Check for networks with duplicate names."
description: |
Deprecated: CheckDuplicate is now ignored; a NetworkCreate
request with a duplicated network name will fail.
type: "boolean"
Driver:
description: "Name of the network driver plugin to use."
Expand Down
4 changes: 3 additions & 1 deletion docs/api/v1.27.yaml
Expand Up @@ -6254,7 +6254,9 @@ paths:
description: "The network's name."
type: "string"
CheckDuplicate:
description: "Check for networks with duplicate names."
description: |
Deprecated: CheckDuplicate is now ignored; a NetworkCreate
request with a duplicated network name will fail.
type: "boolean"
Driver:
description: "Name of the network driver plugin to use."
Expand Down
4 changes: 3 additions & 1 deletion docs/api/v1.28.yaml
Expand Up @@ -6383,7 +6383,9 @@ paths:
description: "The network's name."
type: "string"
CheckDuplicate:
description: "Check for networks with duplicate names. Since Network is primarily keyed based on a random ID and not on the name, and network name is strictly a user-friendly alias to the network which is uniquely identified using ID, there is no guaranteed way to check for duplicates. CheckDuplicate is there to provide a best effort checking of any networks which has the same name but it is not guaranteed to catch all name collisions."
description: |
Deprecated: CheckDuplicate is now ignored; a NetworkCreate
request with a duplicated network name will fail.
type: "boolean"
Driver:
description: "Name of the network driver plugin to use."
Expand Down
4 changes: 3 additions & 1 deletion docs/api/v1.29.yaml
Expand Up @@ -6421,7 +6421,9 @@ paths:
description: "The network's name."
type: "string"
CheckDuplicate:
description: "Check for networks with duplicate names. Since Network is primarily keyed based on a random ID and not on the name, and network name is strictly a user-friendly alias to the network which is uniquely identified using ID, there is no guaranteed way to check for duplicates. CheckDuplicate is there to provide a best effort checking of any networks which has the same name but it is not guaranteed to catch all name collisions."
description: |
Deprecated: CheckDuplicate is now ignored; a NetworkCreate
request with a duplicated network name will fail.
type: "boolean"
Driver:
description: "Name of the network driver plugin to use."
Expand Down
4 changes: 3 additions & 1 deletion docs/api/v1.30.yaml
Expand Up @@ -6684,7 +6684,9 @@ paths:
description: "The network's name."
type: "string"
CheckDuplicate:
description: "Check for networks with duplicate names. Since Network is primarily keyed based on a random ID and not on the name, and network name is strictly a user-friendly alias to the network which is uniquely identified using ID, there is no guaranteed way to check for duplicates. CheckDuplicate is there to provide a best effort checking of any networks which has the same name but it is not guaranteed to catch all name collisions."
description: |
Deprecated: CheckDuplicate is now ignored; a NetworkCreate
request with a duplicated network name will fail.
type: "boolean"
Driver:
description: "Name of the network driver plugin to use."
Expand Down
4 changes: 3 additions & 1 deletion docs/api/v1.31.yaml
Expand Up @@ -6782,7 +6782,9 @@ paths:
description: "The network's name."
type: "string"
CheckDuplicate:
description: "Check for networks with duplicate names. Since Network is primarily keyed based on a random ID and not on the name, and network name is strictly a user-friendly alias to the network which is uniquely identified using ID, there is no guaranteed way to check for duplicates. CheckDuplicate is there to provide a best effort checking of any networks which has the same name but it is not guaranteed to catch all name collisions."
description: |
Deprecated: CheckDuplicate is now ignored; a NetworkCreate
request with a duplicated network name will fail.
type: "boolean"
Driver:
description: "Name of the network driver plugin to use."
Expand Down
4 changes: 3 additions & 1 deletion docs/api/v1.32.yaml
Expand Up @@ -7823,7 +7823,9 @@ paths:
description: "The network's name."
type: "string"
CheckDuplicate:
description: "Check for networks with duplicate names. Since Network is primarily keyed based on a random ID and not on the name, and network name is strictly a user-friendly alias to the network which is uniquely identified using ID, there is no guaranteed way to check for duplicates. CheckDuplicate is there to provide a best effort checking of any networks which has the same name but it is not guaranteed to catch all name collisions."
description: |
Deprecated: CheckDuplicate is now ignored; a NetworkCreate
request with a duplicated network name will fail.
type: "boolean"
Driver:
description: "Name of the network driver plugin to use."
Expand Down
4 changes: 3 additions & 1 deletion docs/api/v1.33.yaml
Expand Up @@ -7832,7 +7832,9 @@ paths:
description: "The network's name."
type: "string"
CheckDuplicate:
description: "Check for networks with duplicate names. Since Network is primarily keyed based on a random ID and not on the name, and network name is strictly a user-friendly alias to the network which is uniquely identified using ID, there is no guaranteed way to check for duplicates. CheckDuplicate is there to provide a best effort checking of any networks which has the same name but it is not guaranteed to catch all name collisions."
description: |
Deprecated: CheckDuplicate is now ignored; a NetworkCreate
request with a duplicated network name will fail.
type: "boolean"
Driver:
description: "Name of the network driver plugin to use."
Expand Down
4 changes: 3 additions & 1 deletion docs/api/v1.34.yaml
Expand Up @@ -7873,7 +7873,9 @@ paths:
description: "The network's name."
type: "string"
CheckDuplicate:
description: "Check for networks with duplicate names. Since Network is primarily keyed based on a random ID and not on the name, and network name is strictly a user-friendly alias to the network which is uniquely identified using ID, there is no guaranteed way to check for duplicates. CheckDuplicate is there to provide a best effort checking of any networks which has the same name but it is not guaranteed to catch all name collisions."
description: |
Deprecated: CheckDuplicate is now ignored; a NetworkCreate
request with a duplicated network name will fail.
type: "boolean"
Driver:
description: "Name of the network driver plugin to use."
Expand Down
4 changes: 3 additions & 1 deletion docs/api/v1.35.yaml
Expand Up @@ -7885,7 +7885,9 @@ paths:
description: "The network's name."
type: "string"
CheckDuplicate:
description: "Check for networks with duplicate names. Since Network is primarily keyed based on a random ID and not on the name, and network name is strictly a user-friendly alias to the network which is uniquely identified using ID, there is no guaranteed way to check for duplicates. CheckDuplicate is there to provide a best effort checking of any networks which has the same name but it is not guaranteed to catch all name collisions."
description: |
Deprecated: CheckDuplicate is now ignored; a NetworkCreate
request with a duplicated network name will fail.
type: "boolean"
Driver:
description: "Name of the network driver plugin to use."
Expand Down
4 changes: 3 additions & 1 deletion docs/api/v1.36.yaml
Expand Up @@ -7927,7 +7927,9 @@ paths:
description: "The network's name."
type: "string"
CheckDuplicate:
description: "Check for networks with duplicate names. Since Network is primarily keyed based on a random ID and not on the name, and network name is strictly a user-friendly alias to the network which is uniquely identified using ID, there is no guaranteed way to check for duplicates. CheckDuplicate is there to provide a best effort checking of any networks which has the same name but it is not guaranteed to catch all name collisions."
description: |
Deprecated: CheckDuplicate is now ignored; a NetworkCreate
request with a duplicated network name will fail.
type: "boolean"
Driver:
description: "Name of the network driver plugin to use."
Expand Down
4 changes: 3 additions & 1 deletion docs/api/v1.37.yaml
Expand Up @@ -7970,7 +7970,9 @@ paths:
description: "The network's name."
type: "string"
CheckDuplicate:
description: "Check for networks with duplicate names. Since Network is primarily keyed based on a random ID and not on the name, and network name is strictly a user-friendly alias to the network which is uniquely identified using ID, there is no guaranteed way to check for duplicates. CheckDuplicate is there to provide a best effort checking of any networks which has the same name but it is not guaranteed to catch all name collisions."
description: |
Deprecated: CheckDuplicate is now ignored; a NetworkCreate
request with a duplicated network name will fail.
type: "boolean"
Driver:
description: "Name of the network driver plugin to use."
Expand Down
4 changes: 3 additions & 1 deletion docs/api/v1.38.yaml
Expand Up @@ -8031,7 +8031,9 @@ paths:
description: "The network's name."
type: "string"
CheckDuplicate:
description: "Check for networks with duplicate names. Since Network is primarily keyed based on a random ID and not on the name, and network name is strictly a user-friendly alias to the network which is uniquely identified using ID, there is no guaranteed way to check for duplicates. CheckDuplicate is there to provide a best effort checking of any networks which has the same name but it is not guaranteed to catch all name collisions."
description: |
Deprecated: CheckDuplicate is now ignored; a NetworkCreate
request with a duplicated network name will fail.
type: "boolean"
Driver:
description: "Name of the network driver plugin to use."
Expand Down

0 comments on commit 280795e

Please sign in to comment.