Skip to content

Commit

Permalink
Include exposed ports in inspect output when net=host
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
mheon committed Sep 27, 2024
1 parent 87dcf9d commit a619c03
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 27 deletions.
1 change: 1 addition & 0 deletions libpod/container_inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion libpod/networking_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
17 changes: 15 additions & 2 deletions libpod/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")
Expand All @@ -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 {
Expand Down
15 changes: 7 additions & 8 deletions libpod/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, ",")
Expand All @@ -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)
Expand All @@ -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.
Expand Down
26 changes: 10 additions & 16 deletions pkg/specgen/generate/namespaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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 {
Expand Down
32 changes: 32 additions & 0 deletions test/e2e/run_networking_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
Expand Down

0 comments on commit a619c03

Please sign in to comment.