Merge pull request docker#9520 from milas/bugfix-network-ambiguity

fix: prevent network name ambiguity
glours · Jun 2, 2022 · d5528f3 · d5528f3
2 parents c64dbbc + 7f32f02
commit d5528f3
Showing 1 changed file with 67 additions and 60 deletions.
diff --git a/pkg/compose/create.go b/pkg/compose/create.go
@@ -31,6 +31,7 @@ import (
 	moby "github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/blkiodev"
 	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/api/types/network"
 	"github.com/docker/docker/api/types/strslice"
@@ -1007,75 +1008,81 @@ func getAliases(s types.ServiceConfig, c *types.ServiceNetworkConfig) []string {
 }
 
 func (s *composeService) ensureNetwork(ctx context.Context, n types.NetworkConfig) error {
-	_, err := s.apiClient().NetworkInspect(ctx, n.Name, moby.NetworkInspectOptions{})
+	// NetworkInspect will match on ID prefix, so NetworkList with a name
+	// filter is used to look for an exact match to prevent e.g. a network
+	// named `db` from getting erroneously matched to a network with an ID
+	// like `db9086999caf`
+	networks, err := s.apiClient().NetworkList(ctx, moby.NetworkListOptions{
+		Filters: filters.NewArgs(filters.Arg("name", n.Name)),
+	})
 	if err != nil {
-		if errdefs.IsNotFound(err) {
-			if n.External.External {
-				if n.Driver == "overlay" {
-					// Swarm nodes do not register overlay networks that were
-					// created on a different node unless they're in use.
-					// Here we assume `driver` is relevant for a network we don't manage
-					// which is a non-sense, but this is our legacy ¯\(ツ)/¯
-					// networkAttach will later fail anyway if network actually doesn't exists
-					return nil
-				}
-				return fmt.Errorf("network %s declared as external, but could not be found", n.Name)
+		return err
+	}
+	if len(networks) == 0 {
+		if n.External.External {
+			if n.Driver == "overlay" {
+				// Swarm nodes do not register overlay networks that were
+				// created on a different node unless they're in use.
+				// Here we assume `driver` is relevant for a network we don't manage
+				// which is a non-sense, but this is our legacy ¯\(ツ)/¯
+				// networkAttach will later fail anyway if network actually doesn't exists
+				return nil
 			}
-			var ipam *network.IPAM
-			if n.Ipam.Config != nil {
-				var config []network.IPAMConfig
-				for _, pool := range n.Ipam.Config {
-					config = append(config, network.IPAMConfig{
-						Subnet:     pool.Subnet,
-						IPRange:    pool.IPRange,
-						Gateway:    pool.Gateway,
-						AuxAddress: pool.AuxiliaryAddresses,
-					})
-				}
-				ipam = &network.IPAM{
-					Driver: n.Ipam.Driver,
-					Config: config,
-				}
+			return fmt.Errorf("network %s declared as external, but could not be found", n.Name)
+		}
+		var ipam *network.IPAM
+		if n.Ipam.Config != nil {
+			var config []network.IPAMConfig
+			for _, pool := range n.Ipam.Config {
+				config = append(config, network.IPAMConfig{
+					Subnet:     pool.Subnet,
+					IPRange:    pool.IPRange,
+					Gateway:    pool.Gateway,
+					AuxAddress: pool.AuxiliaryAddresses,
+				})
 			}
-			createOpts := moby.NetworkCreate{
-				// TODO NameSpace Labels
-				Labels:     n.Labels,
-				Driver:     n.Driver,
-				Options:    n.DriverOpts,
-				Internal:   n.Internal,
-				Attachable: n.Attachable,
-				IPAM:       ipam,
-				EnableIPv6: n.EnableIPv6,
+			ipam = &network.IPAM{
+				Driver: n.Ipam.Driver,
+				Config: config,
 			}
+		}
+		createOpts := moby.NetworkCreate{
+			// TODO NameSpace Labels
+			Labels:     n.Labels,
+			Driver:     n.Driver,
+			Options:    n.DriverOpts,
+			Internal:   n.Internal,
+			Attachable: n.Attachable,
+			IPAM:       ipam,
+			EnableIPv6: n.EnableIPv6,
+		}
 
-			if n.Ipam.Driver != "" || len(n.Ipam.Config) > 0 {
-				createOpts.IPAM = &network.IPAM{}
-			}
+		if n.Ipam.Driver != "" || len(n.Ipam.Config) > 0 {
+			createOpts.IPAM = &network.IPAM{}
+		}
 
-			if n.Ipam.Driver != "" {
-				createOpts.IPAM.Driver = n.Ipam.Driver
-			}
+		if n.Ipam.Driver != "" {
+			createOpts.IPAM.Driver = n.Ipam.Driver
+		}
 
-			for _, ipamConfig := range n.Ipam.Config {
-				config := network.IPAMConfig{
-					Subnet:     ipamConfig.Subnet,
-					IPRange:    ipamConfig.IPRange,
-					Gateway:    ipamConfig.Gateway,
-					AuxAddress: ipamConfig.AuxiliaryAddresses,
-				}
-				createOpts.IPAM.Config = append(createOpts.IPAM.Config, config)
+		for _, ipamConfig := range n.Ipam.Config {
+			config := network.IPAMConfig{
+				Subnet:     ipamConfig.Subnet,
+				IPRange:    ipamConfig.IPRange,
+				Gateway:    ipamConfig.Gateway,
+				AuxAddress: ipamConfig.AuxiliaryAddresses,
 			}
-			networkEventName := fmt.Sprintf("Network %s", n.Name)
-			w := progress.ContextWriter(ctx)
-			w.Event(progress.CreatingEvent(networkEventName))
-			if _, err := s.apiClient().NetworkCreate(ctx, n.Name, createOpts); err != nil {
-				w.Event(progress.ErrorEvent(networkEventName))
-				return errors.Wrapf(err, "failed to create network %s", n.Name)
-			}
-			w.Event(progress.CreatedEvent(networkEventName))
-			return nil
+			createOpts.IPAM.Config = append(createOpts.IPAM.Config, config)
 		}
-		return err
+		networkEventName := fmt.Sprintf("Network %s", n.Name)
+		w := progress.ContextWriter(ctx)
+		w.Event(progress.CreatingEvent(networkEventName))
+		if _, err := s.apiClient().NetworkCreate(ctx, n.Name, createOpts); err != nil {
+			w.Event(progress.ErrorEvent(networkEventName))
+			return errors.Wrapf(err, "failed to create network %s", n.Name)
+		}
+		w.Event(progress.CreatedEvent(networkEventName))
+		return nil
 	}
 	return nil
 }