Skip to content

Commit

Permalink
quadlet: allow variables in PublishPort
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@Luap99
Luap99 committed Oct 1, 2024
1 parent a3ba5ff commit abc4cfb
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 76 deletions.
73 changes: 5 additions & 68 deletions pkg/systemd/quadlet/quadlet.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
13 changes: 9 additions & 4 deletions test/e2e/quadlet/ports.container
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
8 changes: 4 additions & 4 deletions test/e2e/quadlet/ports.kube
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit abc4cfb

Please sign in to comment.