From abc4cfb049c75a61ee4fb0d6cdb04e59c7056681 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 1 Oct 2024 09:58:23 +0200 Subject: quadlet: allow variables in PublishPort There is no reason to validate the args here, first podman may change the syntax so this is just duplication that may hurt us long term. It also added special handling of some options that just do not make sense, i.e. removing 0.0.0.0, podman should really be the only parser here. And more importantly this prevents variables from being used. Fixes #24081 Signed-off-by: Paul Holzinger --- pkg/systemd/quadlet/quadlet.go | 73 +++------------------------------------- test/e2e/quadlet/ports.container | 13 ++++--- test/e2e/quadlet/ports.kube | 8 ++--- 3 files changed, 18 insertions(+), 76 deletions(-) diff --git a/pkg/systemd/quadlet/quadlet.go b/pkg/systemd/quadlet/quadlet.go index ca3773da7..f81fd64b4 100644 --- a/pkg/systemd/quadlet/quadlet.go +++ b/pkg/systemd/quadlet/quadlet.go @@ -828,9 +828,7 @@ func ConvertContainer(container *parser.UnitFile, isUser bool, unitsInfoMap map[ podman.addf("--expose=%s", exposedPort) } - if err := handlePublishPorts(container, ContainerGroup, podman); err != nil { - return nil, err - } + handlePublishPorts(container, ContainerGroup, podman) podman.addEnv(podmanEnv) @@ -1302,9 +1300,7 @@ func ConvertKube(kube *parser.UnitFile, unitsInfoMap map[string]*UnitInfo, isUse execStart.add("--configmap", configMapPath) } - if err := handlePublishPorts(kube, KubeGroup, execStart); err != nil { - return nil, err - } + handlePublishPorts(kube, KubeGroup, execStart) handlePodmanArgs(kube, KubeGroup, execStart) @@ -1689,9 +1685,7 @@ func ConvertPod(podUnit *parser.UnitFile, name string, unitsInfoMap map[string]* return nil, err } - if err := handlePublishPorts(podUnit, PodGroup, execStartPre); err != nil { - return nil, err - } + handlePublishPorts(podUnit, PodGroup, execStartPre) if err := addNetworks(podUnit, PodGroup, service, unitsInfoMap, execStartPre); err != nil { return nil, err @@ -1960,68 +1954,11 @@ func getAbsolutePath(quadletUnitFile *parser.UnitFile, filePath string) (string, return filePath, nil } -func handlePublishPorts(unitFile *parser.UnitFile, groupName string, podman *PodmanCmdline) error { +func handlePublishPorts(unitFile *parser.UnitFile, groupName string, podman *PodmanCmdline) { publishPorts := unitFile.LookupAll(groupName, KeyPublishPort) for _, publishPort := range publishPorts { - publishPort = strings.TrimSpace(publishPort) // Allow whitespace after - - // IP address could have colons in it. For example: "[::]:8080:80/tcp, so use custom splitter - parts := splitPorts(publishPort) - - var containerPort string - ip := "" - hostPort := "" - - // format (from podman run): - // ip:hostPort:containerPort | ip::containerPort | hostPort:containerPort | containerPort - // - // ip could be IPv6 with minimum of these chars "[::]" - // containerPort can have a suffix of "/tcp" or "/udp" - // - - switch len(parts) { - case 1: - containerPort = parts[0] - - case 2: - hostPort = parts[0] - containerPort = parts[1] - - case 3: - ip = parts[0] - hostPort = parts[1] - containerPort = parts[2] - - default: - return fmt.Errorf("invalid published port '%s'", publishPort) - } - - if ip == "0.0.0.0" { - ip = "" - } - - if len(hostPort) > 0 && !isPortRange(hostPort) { - return fmt.Errorf("invalid port format '%s'", hostPort) - } - - if len(containerPort) > 0 && !isPortRange(containerPort) { - return fmt.Errorf("invalid port format '%s'", containerPort) - } - - podman.add("--publish") - switch { - case len(ip) > 0 && len(hostPort) > 0: - podman.addf("%s:%s:%s", ip, hostPort, containerPort) - case len(ip) > 0: - podman.addf("%s::%s", ip, containerPort) - case len(hostPort) > 0: - podman.addf("%s:%s", hostPort, containerPort) - default: - podman.addf("%s", containerPort) - } + podman.add("--publish", publishPort) } - - return nil } func handleLogDriver(unitFile *parser.UnitFile, groupName string, podman *PodmanCmdline) { diff --git a/test/e2e/quadlet/ports.container b/test/e2e/quadlet/ports.container index 22eff3e08..db0612c77 100644 --- a/test/e2e/quadlet/ports.container +++ b/test/e2e/quadlet/ports.container @@ -8,19 +8,19 @@ ExposeHostPort=2000-3000 ## assert-podman-args --publish 127.0.0.1:80:90 PublishPort=127.0.0.1:80:90 -## assert-podman-args --publish 80:91 +## assert-podman-args --publish 0.0.0.0:80:91 PublishPort=0.0.0.0:80:91 -## assert-podman-args --publish 80:92 +## assert-podman-args --publish :80:92 PublishPort=:80:92 ## assert-podman-args --publish 127.0.0.1::93 PublishPort=127.0.0.1::93 -## assert-podman-args --publish 94 +## assert-podman-args --publish 0.0.0.0::94 PublishPort=0.0.0.0::94 -## assert-podman-args --publish 95 +## assert-podman-args --publish ::95 PublishPort=::95 ## assert-podman-args --publish 80:96 @@ -47,6 +47,11 @@ PublishPort=1234:1234/tcp ## assert-podman-args --publish 127.0.0.1:1234:1234/tcp PublishPort=127.0.0.1:1234:1234/tcp +# https://github.com/containers/podman/issues/24081 +# Allow variables to be used as systemd expands them at runtime. +## assert-podman-args --publish ${PORT}:${PORT} +PublishPort=${PORT}:${PORT} + ## assert-podman-args --expose=2000-3000/udp ExposeHostPort=2000-3000/udp diff --git a/test/e2e/quadlet/ports.kube b/test/e2e/quadlet/ports.kube index cc07a31f0..376ed0941 100644 --- a/test/e2e/quadlet/ports.kube +++ b/test/e2e/quadlet/ports.kube @@ -4,19 +4,19 @@ Yaml=/opt/k8s/deployment.yml ## assert-podman-args --publish 127.0.0.1:80:90 PublishPort=127.0.0.1:80:90 -## assert-podman-args --publish 80:91 +## assert-podman-args --publish 0.0.0.0:80:91 PublishPort=0.0.0.0:80:91 -## assert-podman-args --publish 80:92 +## assert-podman-args --publish :80:92 PublishPort=:80:92 ## assert-podman-args --publish 127.0.0.1::93 PublishPort=127.0.0.1::93 -## assert-podman-args --publish 94 +## assert-podman-args --publish 0.0.0.0::94 PublishPort=0.0.0.0::94 -## assert-podman-args --publish 95 +## assert-podman-args --publish ::95 PublishPort=::95 ## assert-podman-args --publish 80:96 -- cgit v1.2.3-70-g09d2