[v4.9-rhel] Include exposed ports in inspect output...

when net=host

Previously, we didn't bother including exposed ports in the
container config when creating a container with --net=host. Per
Docker this isn't really correct; host-net containers are still
considered to have exposed ports, even though that specific
container can be guaranteed to never use them.

We could just fix this for host container, but we might as well
make it generic. This patch unconditionally adds exposed ports to
the container config - it was previously conditional on a network
namespace being configured. The behavior of `podman inspect` with
exposed ports when using `--net=container:` has also been
corrected. Previously, we used exposed ports from the container
sharing its network namespace, which was not correct. Now, we use
regular port bindings from the namespace container, but exposed
ports from our own container.

Fixes https://issues.redhat.com/browse/RHEL-60382

Signed-off-by: Matt Heon <mheon@redhat.com>
(cherry picked from commit a619c03eff)
Signed-off-by: tomsweeneyredhat <tsweeney@redhat.com>

libpod/container_inspect.go

@@ -206,6 +206,7 @@ func (c *Container) getContainerInspectData(size bool, driverData *define.Driver
 		return nil, err
 	}
 	data.NetworkSettings = networkConfig
+	addInspectPortsExpose(c.config.ExposedPorts, data.NetworkSettings.Ports)
 
 	inspectConfig := c.generateInspectContainerConfig(ctrSpec)
 	data.Config = inspectConfig

libpod/networking_common.go

@@ -234,7 +234,7 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e
 	}
 
 	settings := new(define.InspectNetworkSettings)
-	settings.Ports = makeInspectPorts(c.config.PortMappings, c.config.ExposedPorts)
+	settings.Ports = makeInspectPortBindings(c.config.PortMappings)
 
 	networks, err := c.networks()
 	if err != nil {

libpod/options.go

@@ -1069,7 +1069,7 @@ func WithDependencyCtrs(ctrs []*Container) CtrCreateOption {
 // namespace with a minimal configuration.
 // An optional array of port mappings can be provided.
 // Conflicts with WithNetNSFrom().
-func WithNetNS(portMappings []nettypes.PortMapping, exposedPorts map[uint16][]string, postConfigureNetNS bool, netmode string, networks map[string]nettypes.PerNetworkOptions) CtrCreateOption {
+func WithNetNS(portMappings []nettypes.PortMapping, postConfigureNetNS bool, netmode string, networks map[string]nettypes.PerNetworkOptions) CtrCreateOption {
 	return func(ctr *Container) error {
 		if ctr.valid {
 			return define.ErrCtrFinalized
@@ -1079,7 +1079,6 @@ func WithNetNS(portMappings []nettypes.PortMapping, exposedPorts map[uint16][]st
 		ctr.config.NetMode = namespaces.NetworkMode(netmode)
 		ctr.config.CreateNetNS = true
 		ctr.config.PortMappings = portMappings
-		ctr.config.ExposedPorts = exposedPorts
 
 		if !ctr.config.NetMode.IsBridge() && len(networks) > 0 {
 			return errors.New("cannot use networks when network mode is not bridge")
@@ -1090,6 +1089,20 @@ func WithNetNS(portMappings []nettypes.PortMapping, exposedPorts map[uint16][]st
 	}
 }
 
+// WithExposedPorts includes a set of ports that were exposed by the image in
+// the container config, e.g. for display when the container is inspected.
+func WithExposedPorts(exposedPorts map[uint16][]string) CtrCreateOption {
+	return func(ctr *Container) error {
+		if ctr.valid {
+			return define.ErrCtrFinalized
+		}
+
+		ctr.config.ExposedPorts = exposedPorts
+
+		return nil
+	}
+}
+
 // WithNetworkOptions sets additional options for the networks.
 func WithNetworkOptions(options map[string][]string) CtrCreateOption {
 	return func(ctr *Container) error {

libpod/util.go

@@ -220,13 +220,8 @@ func writeHijackHeader(r *http.Request, conn io.Writer, tty bool) {
 	}
 }
 
-// Convert OCICNI port bindings into Inspect-formatted port bindings.
+// Generate inspect-formatted port mappings from the format used in our config file
 func makeInspectPortBindings(bindings []types.PortMapping) map[string][]define.InspectHostPort {
-	return makeInspectPorts(bindings, nil)
-}
-
-// Convert OCICNI port bindings into Inspect-formatted port bindings with exposed, but not bound ports set to nil.
-func makeInspectPorts(bindings []types.PortMapping, expose map[uint16][]string) map[string][]define.InspectHostPort {
 	portBindings := make(map[string][]define.InspectHostPort)
 	for _, port := range bindings {
 		protocols := strings.Split(port.Protocol, ",")
@@ -242,7 +237,12 @@ func makeInspectPorts(bindings []types.PortMapping, expose map[uint16][]string)
 			}
 		}
 	}
-	// add exposed ports without host port information to match docker
+
+	return portBindings
+}
+
+// Add exposed ports to inspect port bindings. These must be done on a per-container basis, not per-netns basis.
+func addInspectPortsExpose(expose map[uint16][]string, portBindings map[string][]define.InspectHostPort) {
 	for port, protocols := range expose {
 		for _, protocol := range protocols {
 			key := fmt.Sprintf("%d/%s", port, protocol)
@@ -251,7 +251,6 @@ func makeInspectPorts(bindings []types.PortMapping, expose map[uint16][]string)
 			}
 		}
 	}
-	return portBindings
 }
 
 // Write a given string to a new file at a given path.

pkg/specgen/generate/namespaces.go

@@ -298,6 +298,13 @@ func namespaceOptions(s *specgen.SpecGenerator, rt *libpod.Runtime, pod *libpod.
 
 	postConfigureNetNS := !s.UserNS.IsHost()
 
+	// Network
+	portMappings, expose, err := createPortMappings(s, imageData)
+	if err != nil {
+		return nil, err
+	}
+	toReturn = append(toReturn, libpod.WithExposedPorts(expose))
+
 	switch s.NetNS.NSMode {
 	case specgen.FromPod:
 		if pod == nil || infraCtr == nil {
@@ -317,28 +324,15 @@ func namespaceOptions(s *specgen.SpecGenerator, rt *libpod.Runtime, pod *libpod.
 			toReturn = append(toReturn, libpod.WithNetNSFrom(netCtr))
 		}
 	case specgen.Slirp:
-		portMappings, expose, err := createPortMappings(s, imageData)
-		if err != nil {
-			return nil, err
-		}
 		val := "slirp4netns"
 		if s.NetNS.Value != "" {
 			val = fmt.Sprintf("slirp4netns:%s", s.NetNS.Value)
 		}
-		toReturn = append(toReturn, libpod.WithNetNS(portMappings, expose, postConfigureNetNS, val, nil))
+		toReturn = append(toReturn, libpod.WithNetNS(portMappings, postConfigureNetNS, val, nil))
 	case specgen.Pasta:
-		portMappings, expose, err := createPortMappings(s, imageData)
-		if err != nil {
-			return nil, err
-		}
 		val := "pasta"
-		toReturn = append(toReturn, libpod.WithNetNS(portMappings, expose, postConfigureNetNS, val, nil))
+		toReturn = append(toReturn, libpod.WithNetNS(portMappings, postConfigureNetNS, val, nil))
 	case specgen.Bridge, specgen.Private, specgen.Default:
-		portMappings, expose, err := createPortMappings(s, imageData)
-		if err != nil {
-			return nil, err
-		}
-
 		rtConfig, err := rt.GetConfigNoCopy()
 		if err != nil {
 			return nil, err
@@ -365,7 +359,7 @@ func namespaceOptions(s *specgen.SpecGenerator, rt *libpod.Runtime, pod *libpod.
 			s.Networks[rtConfig.Network.DefaultNetwork] = opts
 			delete(s.Networks, "default")
 		}
-		toReturn = append(toReturn, libpod.WithNetNS(portMappings, expose, postConfigureNetNS, "bridge", s.Networks))
+		toReturn = append(toReturn, libpod.WithNetNS(portMappings, postConfigureNetNS, "bridge", s.Networks))
 	}
 
 	if s.UseImageHosts {

test/e2e/run_networking_test.go

@@ -434,6 +434,38 @@ EXPOSE 2004-2005/tcp`, ALPINE)
 		Expect(inspectOut[0].HostConfig.PublishAllPorts).To(BeTrue())
 	})
 
+	It("podman run --net=host --expose includes port in inspect output", func() {
+		containerName := "testctr"
+		session := podmanTest.Podman([]string{"run", "--name", containerName, "-d", "--expose", "8080/tcp", NGINX_IMAGE, "sleep", "+inf"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(ExitCleanly())
+
+		inspectOut := podmanTest.InspectContainer(containerName)
+		Expect(inspectOut).To(HaveLen(1))
+
+		// 80 from the image, 8080 from the expose
+		Expect(inspectOut[0].NetworkSettings.Ports).To(HaveLen(2))
+		Expect(inspectOut[0].NetworkSettings.Ports).To(HaveKey("80/tcp"))
+		Expect(inspectOut[0].NetworkSettings.Ports).To(HaveKey("8080/tcp"))
+	})
+
+	It("podman run --net=container --expose exposed port from own container", func() {
+		ctr1 := "test1"
+		session1 := podmanTest.Podman([]string{"run", "-d", "--name", ctr1, "--expose", "8080/tcp", ALPINE, "top"})
+		session1.WaitWithDefaultTimeout()
+		Expect(session1).Should(ExitCleanly())
+
+		ctr2 := "test2"
+		session2 := podmanTest.Podman([]string{"run", "-d", "--name", ctr2, "--net", fmt.Sprintf("container:%s", ctr1), "--expose", "8090/tcp", ALPINE, "top"})
+		session2.WaitWithDefaultTimeout()
+		Expect(session2).Should(ExitCleanly())
+
+		inspectOut := podmanTest.InspectContainer(ctr2)
+		Expect(inspectOut).To(HaveLen(1))
+		Expect(inspectOut[0].NetworkSettings.Ports).To(HaveLen(1))
+		Expect(inspectOut[0].NetworkSettings.Ports).To(HaveKey("8090/tcp"))
+	})
+
 	It("podman run -p 127.0.0.1::8980/udp", func() {
 		name := "testctr"
 		session := podmanTest.Podman([]string{"create", "-t", "-p", "127.0.0.1::8980/udp", "--name", name, ALPINE, "/bin/sh"})