Merge pull request #16818 from SoMuchForSubtlety/api-port-bindings

api: remove unmapped ports from PortBindings

libpod/container_inspect.go

@@ -554,7 +554,7 @@ func (c *Container) generateInspectContainerHostConfig(ctrSpec *spec.Spec, named
 	// Port bindings.
 	// Only populate if we're using CNI to configure the network.
 	if c.config.CreateNetNS {
-		hostConfig.PortBindings = makeInspectPortBindings(c.config.PortMappings, c.config.ExposedPorts)
+		hostConfig.PortBindings = makeInspectPortBindings(c.config.PortMappings)
 	} else {
 		hostConfig.PortBindings = make(map[string][]define.InspectHostPort)
 	}

libpod/networking_common.go

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

libpod/pod_api.go

@@ -705,7 +705,7 @@ func (p *Pod) Inspect() (*define.InspectPodData, error) {
 			infraConfig.Networks = netNames
 		}
 		infraConfig.NetworkOptions = infra.config.ContainerNetworkConfig.NetworkOptions
-		infraConfig.PortBindings = makeInspectPortBindings(infra.config.ContainerNetworkConfig.PortMappings, nil)
+		infraConfig.PortBindings = makeInspectPortBindings(infra.config.ContainerNetworkConfig.PortMappings)
 	}
 
 	inspectData := define.InspectPodData{

libpod/util.go

@@ -312,7 +312,12 @@ func writeHijackHeader(r *http.Request, conn io.Writer) {
 }
 
 // Convert OCICNI port bindings into Inspect-formatted port bindings.
-func makeInspectPortBindings(bindings []types.PortMapping, expose map[uint16][]string) map[string][]define.InspectHostPort {
+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, ",")

pkg/api/handlers/compat/containers.go

@@ -519,7 +519,7 @@ func LibpodToContainerJSON(l *libpod.Container, sz bool) (*types.ContainerJSON,
 	stopTimeout := int(l.StopTimeout())
 
 	exposedPorts := make(nat.PortSet)
-	for ep := range inspect.HostConfig.PortBindings {
+	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)

test/apiv2/python/rest_api/test_v2_0_0_container.py

@@ -1,7 +1,9 @@
+import io
 import multiprocessing
 import queue
 import random
 import subprocess
+import tarfile
 import threading
 import unittest
 
@@ -359,6 +361,43 @@ class ContainerTestCase(APITestCase):
         self.assertEqual(2000, out["HostConfig"]["MemorySwap"])
         self.assertEqual(1000, out["HostConfig"]["Memory"])
 
+    def test_host_config_port_bindings(self):
+        # create a container with two ports exposed, but only one of the ports bound
+        r = requests.post(
+            self.podman_url + "/v1.40/containers/create",
+            json={
+                "Name": "memory",
+                "Cmd": ["top"],
+                "Image": "alpine:latest",
+                "HostConfig": {
+                    "PortBindings": {
+                        "8080": [{"HostPort": "87634"}]
+                    }
+                },
+                "ExposedPorts": {
+                    "8080": {},
+                    "8081": {}
+                }
+            },
+        )
+        self.assertEqual(r.status_code, 201, r.text)
+        payload = r.json()
+        container_id = payload["Id"]
+        self.assertIsNotNone(container_id)
+
+        r = requests.get(self.podman_url +
+                         f"/v1.40/containers/{container_id}/json")
+        self.assertEqual(r.status_code, 200, r.text)
+        inspect_response = r.json()
+        # both ports are in the config
+        self.assertEqual(2, len(inspect_response["Config"]["ExposedPorts"]))
+        self.assertTrue("8080/tcp" in inspect_response["Config"]["ExposedPorts"])
+        self.assertTrue("8081/tcp" in inspect_response["Config"]["ExposedPorts"])
+        # only 8080 one port is bound
+        self.assertEqual(1, len(inspect_response["HostConfig"]["PortBindings"]))
+        self.assertTrue("8080/tcp" in inspect_response["HostConfig"]["PortBindings"])
+        self.assertFalse("8081/tcp" in inspect_response["HostConfig"]["PortBindings"])
+
 def execute_process(cmd):
     return subprocess.run(
                 cmd,