Merge pull request #24333 from TomSweeneyRedHat/dev/sweeney/accel299-4.9-rhel

[v4.9-rhel] Fix exposed ports

Author: openshift-merge-bot[bot]

libpod/container_inspect.go

@@ -206,6 +206,11 @@ func (c *Container) getContainerInspectData(size bool, driverData *define.Driver
 		return nil, err
 	}
 	data.NetworkSettings = networkConfig
+	// Ports in NetworkSettings includes exposed ports for network modes that are not host,
+	// and not container.
+	if !(c.config.NetNsCtr != "" || c.NetworkMode() == "host") {
+		addInspectPortsExpose(c.config.ExposedPorts, data.NetworkSettings.Ports)
+	}
 
 	inspectConfig := c.generateInspectContainerConfig(ctrSpec)
 	data.Config = inspectConfig
@@ -434,6 +439,25 @@ func (c *Container) generateInspectContainerConfig(spec *spec.Spec) *define.Insp
 
 	ctrConfig.SdNotifyMode = c.config.SdNotifyMode
 	ctrConfig.SdNotifySocket = c.config.SdNotifySocket
+
+	// Exosed ports consists of all exposed ports and all port mappings for
+	// this container. It does *NOT* follow to another container if we share
+	// the network namespace.
+	exposedPorts := make(map[string]struct{})
+	for port, protocols := range c.config.ExposedPorts {
+		for _, proto := range protocols {
+			exposedPorts[fmt.Sprintf("%d/%s", port, proto)] = struct{}{}
+		}
+	}
+	for _, mapping := range c.config.PortMappings {
+		for i := uint16(0); i < mapping.Range; i++ {
+			exposedPorts[fmt.Sprintf("%d/%s", mapping.ContainerPort+i, mapping.Protocol)] = struct{}{}
+		}
+	}
+	if len(exposedPorts) > 0 {
+		ctrConfig.ExposedPorts = exposedPorts
+	}
+
 	return ctrConfig
 }
 

libpod/container_validate.go

@@ -5,6 +5,7 @@ package libpod
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/containers/image/v5/docker"
 	"github.com/containers/image/v5/pkg/shortnames"
@@ -164,6 +165,13 @@ func (c *Container) validate() error {
 		return fmt.Errorf("cannot set a startup healthcheck when there is no regular healthcheck: %w", define.ErrInvalidArg)
 	}
 
+	// Ensure all ports list a single protocol
+	for _, p := range c.config.PortMappings {
+		if strings.Contains(p.Protocol, ",") {
+			return fmt.Errorf("each port mapping must define a single protocol, got a comma-separated list for container port %d (protocols requested %q): %w", p.ContainerPort, p.Protocol, define.ErrInvalidArg)
+		}
+	}
+
 	return nil
 }
 

libpod/define/container_inspect.go

@@ -85,6 +85,8 @@ type InspectContainerConfig struct {
 	SdNotifyMode string `json:"sdNotifyMode,omitempty"`
 	// SdNotifySocket is the NOTIFY_SOCKET in use by/configured for the container.
 	SdNotifySocket string `json:"sdNotifySocket,omitempty"`
+	// ExposedPorts includes ports the container has exposed.
+	ExposedPorts map[string]struct{} `json:"ExposedPorts,omitempty"`
 }
 
 // InspectRestartPolicy holds information about the container's restart policy.

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/api/handlers/compat/containers.go

@@ -522,19 +522,6 @@ func LibpodToContainerJSON(l *libpod.Container, sz bool) (*types.ContainerJSON,
 	}
 	stopTimeout := int(l.StopTimeout())
 
-	exposedPorts := make(nat.PortSet)
-	for ep := range inspect.NetworkSettings.Ports {
-		splitp := strings.SplitN(ep, "/", 2)
-		if len(splitp) != 2 {
-			return nil, fmt.Errorf("PORT/PROTOCOL Format required for %q", ep)
-		}
-		exposedPort, err := nat.NewPort(splitp[1], splitp[0])
-		if err != nil {
-			return nil, err
-		}
-		exposedPorts[exposedPort] = struct{}{}
-	}
-
 	var healthcheck *container.HealthConfig
 	if inspect.Config.Healthcheck != nil {
 		healthcheck = &container.HealthConfig{
@@ -546,6 +533,16 @@ func LibpodToContainerJSON(l *libpod.Container, sz bool) (*types.ContainerJSON,
 		}
 	}
 
+	// Apparently the compiler can't convert a map[string]struct{} into a nat.PortSet
+	// (Despite a nat.PortSet being that exact struct with some types added)
+	var exposedPorts nat.PortSet
+	if len(inspect.Config.ExposedPorts) > 0 {
+		exposedPorts = make(nat.PortSet)
+		for p := range inspect.Config.ExposedPorts {
+			exposedPorts[nat.Port(p)] = struct{}{}
+		}
+	}
+
 	config := container.Config{
 		Hostname:        l.Hostname(),
 		Domainname:      inspect.Config.DomainName,

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/container_inspect_test.go

@@ -1,6 +1,8 @@
 package integration
 
 import (
+	"fmt"
+
 	"github.com/containers/podman/v4/libpod/define"
 	"github.com/containers/podman/v4/pkg/annotations"
 	. "github.com/containers/podman/v4/test/utils"
@@ -30,7 +32,14 @@ var _ = Describe("Podman container inspect", func() {
 
 		Expect(data).To(HaveLen(1))
 		Expect(data[0].NetworkSettings.Ports).
-			To(Equal(map[string][]define.InspectHostPort{"8787/udp": nil}))
+			To(Equal(map[string][]define.InspectHostPort{"8787/udp": nil, "99/sctp": nil}))
+		Expect(data[0].Config.ExposedPorts).
+			To(Equal(map[string]struct{}{"8787/udp": {}, "99/sctp": {}}))
+
+		session = podmanTest.Podman([]string{"ps", "--format", "{{.Ports}}"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(ExitCleanly())
+		Expect(session.OutputToString()).To(Equal("99/sctp, 8787/udp"))
 	})
 
 	It("podman inspect shows exposed ports on image", func() {
@@ -44,4 +53,25 @@ var _ = Describe("Podman container inspect", func() {
 		Expect(data[0].NetworkSettings.Ports).
 			To(Equal(map[string][]define.InspectHostPort{"80/tcp": nil, "8989/tcp": nil}))
 	})
+
+	It("podman inspect exposed ports includes published ports", func() {
+		c1 := "ctr1"
+		c1s := podmanTest.Podman([]string{"run", "-d", "--expose", "22/tcp", "-p", "8080:80/tcp", "--name", c1, ALPINE, "top"})
+		c1s.WaitWithDefaultTimeout()
+		Expect(c1s).Should(ExitCleanly())
+
+		c2 := "ctr2"
+		c2s := podmanTest.Podman([]string{"run", "-d", "--net", fmt.Sprintf("container:%s", c1), "--name", c2, ALPINE, "top"})
+		c2s.WaitWithDefaultTimeout()
+		Expect(c2s).Should(ExitCleanly())
+
+		data1 := podmanTest.InspectContainer(c1)
+		Expect(data1).To(HaveLen(1))
+		Expect(data1[0].Config.ExposedPorts).
+			To(Equal(map[string]struct{}{"22/tcp": {}, "80/tcp": {}}))
+
+		data2 := podmanTest.InspectContainer(c2)
+		Expect(data2).To(HaveLen(1))
+		Expect(data2[0].Config.ExposedPorts).To(BeNil())
+	})
 })

test/e2e/run_networking_test.go

@@ -434,6 +434,43 @@ EXPOSE 2004-2005/tcp`, ALPINE)
 		Expect(inspectOut[0].HostConfig.PublishAllPorts).To(BeTrue())
 	})
 
+	It("podman run --net=host --expose includes ports in inspect output", func() {
+		containerName := "testctr"
+		session := podmanTest.Podman([]string{"run", "--net=host", "--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))
+
+		// Ports is empty. ExposedPorts is not.
+		Expect(inspectOut[0].NetworkSettings.Ports).To(BeEmpty())
+
+		// 80 from the image, 8080 from the expose
+		Expect(inspectOut[0].Config.ExposedPorts).To(HaveLen(2))
+		Expect(inspectOut[0].Config.ExposedPorts).To(HaveKey("80/tcp"))
+		Expect(inspectOut[0].Config.ExposedPorts).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))
+		// Ports will not be populated. ExposedPorts will be.
+		Expect(inspectOut[0].NetworkSettings.Ports).To(BeEmpty())
+		Expect(inspectOut[0].Config.ExposedPorts).To(HaveLen(1))
+		Expect(inspectOut[0].Config.ExposedPorts).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"})