Merge pull request containers#9455 from Luap99/fix-network-ids

Fix podman network IDs handling

libpod/network/files.go

@@ -81,9 +81,9 @@ func GetCNIConfigPathByNameOrID(config *config.Config, name string) (string, err
 	return "", errors.Wrap(define.ErrNoSuchNetwork, fmt.Sprintf("unable to find network configuration for %s", name))
 }
 
-// ReadRawCNIConfByName reads the raw CNI configuration for a CNI
+// ReadRawCNIConfByNameOrID reads the raw CNI configuration for a CNI
 // network by name
-func ReadRawCNIConfByName(config *config.Config, name string) ([]byte, error) {
+func ReadRawCNIConfByNameOrID(config *config.Config, name string) ([]byte, error) {
 	confFile, err := GetCNIConfigPathByNameOrID(config, name)
 	if err != nil {
 		return nil, err

libpod/network/network.go

@@ -7,6 +7,7 @@ import (
 	"net"
 	"os"
 
+	"github.com/containernetworking/cni/libcni"
 	"github.com/containernetworking/cni/pkg/types"
 	"github.com/containernetworking/plugins/plugins/ipam/host-local/backend/allocator"
 	"github.com/containers/common/pkg/config"
@@ -222,7 +223,7 @@ func RemoveNetwork(config *config.Config, name string) error {
 
 // InspectNetwork reads a CNI config and returns its configuration
 func InspectNetwork(config *config.Config, name string) (map[string]interface{}, error) {
-	b, err := ReadRawCNIConfByName(config, name)
+	b, err := ReadRawCNIConfByNameOrID(config, name)
 	if err != nil {
 		return nil, err
 	}
@@ -234,7 +235,7 @@ func InspectNetwork(config *config.Config, name string) (map[string]interface{},
 // Exists says whether a given network exists or not; it meant
 // specifically for restful responses so 404s can be used
 func Exists(config *config.Config, name string) (bool, error) {
-	_, err := ReadRawCNIConfByName(config, name)
+	_, err := ReadRawCNIConfByNameOrID(config, name)
 	if err != nil {
 		if errors.Cause(err) == define.ErrNoSuchNetwork {
 			return false, nil
@@ -277,3 +278,17 @@ func PruneNetworks(rtc *config.Config, usedNetworks map[string]bool) ([]*entitie
 	}
 	return reports, nil
 }
+
+// NormalizeName translates a network ID into a name.
+// If the input is a name the name is returned.
+func NormalizeName(config *config.Config, nameOrID string) (string, error) {
+	path, err := GetCNIConfigPathByNameOrID(config, nameOrID)
+	if err != nil {
+		return "", err
+	}
+	conf, err := libcni.ConfListFromFile(path)
+	if err != nil {
+		return "", err
+	}
+	return conf.Name, nil
+}

libpod/networking_linux.go

@@ -1139,13 +1139,12 @@ func (c *Container) NetworkDisconnect(nameOrID, netName string, force bool) erro
 		return err
 	}
 
-	exists, err := network.Exists(c.runtime.config, netName)
+	// check if network exists and if the input is a ID we get the name
+	// ocicni only uses names so it is important that we only use the name
+	netName, err = network.NormalizeName(c.runtime.config, netName)
 	if err != nil {
 		return err
 	}
-	if !exists {
-		return errors.Wrap(define.ErrNoSuchNetwork, netName)
-	}
 
 	index, nameExists := networks[netName]
 	if !nameExists && len(networks) > 0 {
@@ -1196,13 +1195,12 @@ func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) e
 		return err
 	}
 
-	exists, err := network.Exists(c.runtime.config, netName)
+	// check if network exists and if the input is a ID we get the name
+	// ocicni only uses names so it is important that we only use the name
+	netName, err = network.NormalizeName(c.runtime.config, netName)
 	if err != nil {
 		return err
 	}
-	if !exists {
-		return errors.Wrap(define.ErrNoSuchNetwork, netName)
-	}
 
 	c.lock.Lock()
 	defer c.lock.Unlock()

libpod/runtime_ctr.go

@@ -12,6 +12,7 @@ import (
 	"github.com/containers/common/pkg/config"
 	"github.com/containers/podman/v3/libpod/define"
 	"github.com/containers/podman/v3/libpod/events"
+	"github.com/containers/podman/v3/libpod/network"
 	"github.com/containers/podman/v3/libpod/shutdown"
 	"github.com/containers/podman/v3/pkg/cgroups"
 	"github.com/containers/podman/v3/pkg/domain/entities/reports"
@@ -285,6 +286,21 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
 		return nil, err
 	}
 
+	// normalize the networks to names
+	// ocicni only knows about cni names so we have to make
+	// sure we do not use ids internally
+	if len(ctr.config.Networks) > 0 {
+		netNames := make([]string, 0, len(ctr.config.Networks))
+		for _, nameOrID := range ctr.config.Networks {
+			netName, err := network.NormalizeName(r.config, nameOrID)
+			if err != nil {
+				return nil, err
+			}
+			netNames = append(netNames, netName)
+		}
+		ctr.config.Networks = netNames
+	}
+
 	// Inhibit shutdown until creation succeeds
 	shutdown.Inhibit()
 	defer shutdown.Uninhibit()

test/e2e/create_test.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 
 	. "github.com/containers/podman/v3/test/utils"
+	"github.com/containers/storage/pkg/stringid"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
@@ -576,15 +577,20 @@ var _ = Describe("Podman create", func() {
 		Expect(session.ExitCode()).ToNot(BeZero())
 	})
 
-	It("create container in pod with network should fail", func() {
+	It("create container in pod with network should not fail", func() {
 		name := "createwithnetwork"
 		pod := podmanTest.RunTopContainerInPod("", "new:"+name)
 		pod.WaitWithDefaultTimeout()
 		Expect(pod.ExitCode()).To(BeZero())
 
-		session := podmanTest.Podman([]string{"create", "--pod", name, "--network", "foobar", ALPINE, "top"})
+		netName := "pod" + stringid.GenerateNonCryptoID()
+		session := podmanTest.Podman([]string{"network", "create", netName})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+		defer podmanTest.removeCNINetwork(netName)
+
+		session = podmanTest.Podman([]string{"create", "--pod", name, "--network", netName, ALPINE, "top"})
 		session.WaitWithDefaultTimeout()
-		//Expect(session.ExitCode()).ToNot(BeZero())
 		Expect(session.ExitCode()).To(BeZero())
 	})
 

test/e2e/network_connect_disconnect_test.go

@@ -195,6 +195,55 @@ var _ = Describe("Podman network connect and disconnect", func() {
 		Expect(exec.ExitCode()).To(BeZero())
 	})
 
+	It("podman network connect and run with network ID", func() {
+		SkipIfRemote("remote flakes to much I will fix this in another PR")
+		SkipIfRootless("network connect and disconnect are only rootful")
+		netName := "ID" + stringid.GenerateNonCryptoID()
+		session := podmanTest.Podman([]string{"network", "create", netName})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+		defer podmanTest.removeCNINetwork(netName)
+
+		session = podmanTest.Podman([]string{"network", "ls", "--format", "{{.ID}}", "--filter", "name=" + netName})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+		netID := session.OutputToString()
+
+		ctr := podmanTest.Podman([]string{"run", "-dt", "--name", "test", "--network", netID, ALPINE, "top"})
+		ctr.WaitWithDefaultTimeout()
+		Expect(ctr.ExitCode()).To(BeZero())
+
+		exec := podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth0"})
+		exec.WaitWithDefaultTimeout()
+		Expect(exec.ExitCode()).To(BeZero())
+
+		// Create a second network
+		newNetName := "ID2" + stringid.GenerateNonCryptoID()
+		session = podmanTest.Podman([]string{"network", "create", newNetName})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+		defer podmanTest.removeCNINetwork(newNetName)
+
+		session = podmanTest.Podman([]string{"network", "ls", "--format", "{{.ID}}", "--filter", "name=" + newNetName})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+		newNetID := session.OutputToString()
+
+		connect := podmanTest.Podman([]string{"network", "connect", newNetID, "test"})
+		connect.WaitWithDefaultTimeout()
+		Expect(connect.ExitCode()).To(BeZero())
+
+		inspect := podmanTest.Podman([]string{"container", "inspect", "test", "--format", "{{.NetworkSettings.Networks}}"})
+		inspect.WaitWithDefaultTimeout()
+		Expect(inspect.ExitCode()).To(BeZero())
+		Expect(inspect.OutputToString()).To(ContainSubstring(netName))
+		Expect(inspect.OutputToString()).To(ContainSubstring(newNetName))
+
+		exec = podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth1"})
+		exec.WaitWithDefaultTimeout()
+		Expect(exec.ExitCode()).To(BeZero())
+	})
+
 	It("podman network disconnect when not running", func() {
 		SkipIfRootless("network connect and disconnect are only rootful")
 		netName1 := "aliasTest" + stringid.GenerateNonCryptoID()
@@ -234,4 +283,40 @@ var _ = Describe("Podman network connect and disconnect", func() {
 		exec.WaitWithDefaultTimeout()
 		Expect(exec.ExitCode()).ToNot(BeZero())
 	})
+
+	It("podman network disconnect and run with network ID", func() {
+		SkipIfRemote("remote flakes to much I will fix this in another PR")
+		SkipIfRootless("network connect and disconnect are only rootful")
+		netName := "aliasTest" + stringid.GenerateNonCryptoID()
+		session := podmanTest.Podman([]string{"network", "create", netName})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+		defer podmanTest.removeCNINetwork(netName)
+
+		session = podmanTest.Podman([]string{"network", "ls", "--format", "{{.ID}}", "--filter", "name=" + netName})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+		netID := session.OutputToString()
+
+		ctr := podmanTest.Podman([]string{"run", "-dt", "--name", "test", "--network", netID, ALPINE, "top"})
+		ctr.WaitWithDefaultTimeout()
+		Expect(ctr.ExitCode()).To(BeZero())
+
+		exec := podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth0"})
+		exec.WaitWithDefaultTimeout()
+		Expect(exec.ExitCode()).To(BeZero())
+
+		dis := podmanTest.Podman([]string{"network", "disconnect", netID, "test"})
+		dis.WaitWithDefaultTimeout()
+		Expect(dis.ExitCode()).To(BeZero())
+
+		inspect := podmanTest.Podman([]string{"container", "inspect", "test", "--format", "{{len .NetworkSettings.Networks}}"})
+		inspect.WaitWithDefaultTimeout()
+		Expect(inspect.ExitCode()).To(BeZero())
+		Expect(inspect.OutputToString()).To(Equal("0"))
+
+		exec = podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth0"})
+		exec.WaitWithDefaultTimeout()
+		Expect(exec.ExitCode()).ToNot(BeZero())
+	})
 })