Add ExposedPorts to Inspect's ContainerConfig

[mheon]

A field we missed versus Docker. Matches the format of our existing Ports list in the NetworkConfig, but only includes exposed ports (and maps these to struct{}, as they never go to real ports on the host).

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

Does this PR introduce a user-facing change?

Added a new field to the output of inspect for containers, `Config.ExposedPorts`, which includes all exposed ports from the container, improving our Docker compatibility.

[Luap99]

WE do seems to set exposed ports in the compat API (pkg/api/handlers/compat/containers.go) but there it seems to read of the ports setting so this seems wrong and should be updated now.

Given the code there I also did another test on an image with no exposed ports

[root@fedora ~]# docker run --name test3 -d -p 8081:1234  alpine sleep 1000
323c1996cf21b1148f12cac52be99bcf187210953a4360a77c274a6f04dbf874
[root@fedora ~]# docker inspect --format '{{json .Config.ExposedPorts}}' test3
{"1234/tcp":{}}
[root@fedora ~]# docker inspect --format '{{json .NetworkSettings.Ports}}' test3
{"1234/tcp":[{"HostIp":"0.0.0.0","HostPort":"8081"},{"HostIp":"::","HostPort":"8081"}]}

So it seems even if there is no port exposed and only published it will still show under exposed ports. I do not think your code behaves the same way in that regard

[mheon]

Yep, I'll sneak that fix in.

[mheon]

@Luap99 Can you check what happens with --net=container: with non-expose port mappings being included? Do we get the other container's mappings?

[Luap99]

@Luap99 Can you check what happens with --net=container: with non-expose port mappings being included? Do we get the other container's mappings?

This is where test3 has a exposed port:

[root@fedora ~]# docker run --name test4 --network container:test3 -d alpine sleep 1000
36789e49da5324ce7d8ed71d1fbbdfc2458f80b6e033bdd995fe3ecd66a6a49a
[root@fedora ~]# docker inspect --format '{{json .NetworkSettings.Ports}}' test4
{}
[root@fedora ~]# docker inspect --format '{{json .Config.ExposedPorts}}' test4
null

So there do not include thee network info for the dependency container so this seems different from our podman logic in getContainerNetworkInfo() where we lookup the netns container first and then use that. But I guess as exposed ports are handled outside of that we can fix it at least for them

[Luap99]

the type documentation describes the protocol ass comma separated value which other callers looping over it respect, i.e. makeInspectPortBindings()

That should be impossible in practise to get here as our port validation on container creation should split them out per protocol but I think we shoould try to keep this looping consistent. I wonder if we need a helper function to correctly iterate over them. I think go 1.23 even added support for actual iterators so we could define a custom one.

Looking at nv it seems we ignore that possibility there so maybe it is to late anyway and we should simplify our looping in the other places instead and ensure that our port parsing will never add ports with comma separated protocols.

[mheon]

Instead of changing iteration, would you mind if I just add validation to the libpod container create code to make sure no port includes a comma-separated protocol list? I think, for sanity's sake, we should guarantee protocol is a single protocol per PortMapping struct within libpod.

[Luap99]

yeah that would work for me, though looking at the code I think it is guaranteed today already

podman/pkg/specgen/generate/ports_test.go

Line 120 in 639f3c6

name: "one port three protocols",

I am not sure where you would add the validation? In the With...() libpod functions where we pass the ports? With the current code the risk is that we missing parsing the ports somewhere I guess

[mheon]

I'd probably do it in container_validate.go to ensure it happens after we have a complete config, just in case.

[Luap99]

sounds good to me

[packit-as-a-service]

Ephemeral COPR build failed. @containers/packit-build please check.

[mheon]

Tests look to be going green, @containers/podman-maintainers PTAL

[Luap99]

LGTM

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, mheon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [Luap99,mheon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rhatdan]

/lgtm