From a619c03eff2a8230733a6dfcc8581603597322b2 Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Fri, 27 Sep 2024 08:44:35 -0400 Subject: [PATCH] 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 --- libpod/container_inspect.go | 1 + libpod/networking_common.go | 2 +- libpod/options.go | 17 ++++++++++++++-- libpod/util.go | 15 +++++++------- pkg/specgen/generate/namespaces.go | 26 ++++++++++-------------- test/e2e/run_networking_test.go | 32 ++++++++++++++++++++++++++++++ 6 files changed, 66 insertions(+), 27 deletions(-) diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go index 55abe11840..5a80ca7cbf 100644 --- a/libpod/container_inspect.go +++ b/libpod/container_inspect.go @@ -211,6 +211,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 diff --git a/libpod/networking_common.go b/libpod/networking_common.go index daf840df0f..bac0d48622 100644 --- a/libpod/networking_common.go +++ b/libpod/networking_common.go @@ -204,7 +204,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 { diff --git a/libpod/options.go b/libpod/options.go index ca9f999810..2569f9fd11 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1067,7 +1067,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 @@ -1077,7 +1077,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") @@ -1088,6 +1087,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 { diff --git a/libpod/util.go b/libpod/util.go index 21d864d98d..49c9f10c27 100644 --- a/libpod/util.go +++ b/libpod/util.go @@ -223,13 +223,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, ",") @@ -249,7 +244,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) @@ -258,7 +258,6 @@ func makeInspectPorts(bindings []types.PortMapping, expose map[uint16][]string) } } } - return portBindings } // Write a given string to a new file at a given path. diff --git a/pkg/specgen/generate/namespaces.go b/pkg/specgen/generate/namespaces.go index 8b8338dc0d..5b0e90415e 100644 --- a/pkg/specgen/generate/namespaces.go +++ b/pkg/specgen/generate/namespaces.go @@ -297,6 +297,13 @@ func namespaceOptions(s *specgen.SpecGenerator, rt *libpod.Runtime, pod *libpod. postConfigureNetNS := needPostConfigureNetNS(s) + // 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 { @@ -316,28 +323,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 @@ -364,7 +358,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 != nil && *s.UseImageHosts { diff --git a/test/e2e/run_networking_test.go b/test/e2e/run_networking_test.go index 22a64eb531..99f32e336b 100644 --- a/test/e2e/run_networking_test.go +++ b/test/e2e/run_networking_test.go @@ -441,6 +441,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"})