Merge pull request containers#16893 from alexlarsson/quadlet-better-d…

…efault-handling

quadlet: Handle booleans that have defaults better

pkg/systemd/parser/unitfile.go

@@ -615,16 +615,26 @@ func (f *UnitFile) Lookup(groupName string, key string) (string, bool) {
 }
 
 // Lookup the last instance of a key and convert the value to a bool
-func (f *UnitFile) LookupBoolean(groupName string, key string, defaultValue bool) bool {
+func (f *UnitFile) LookupBoolean(groupName string, key string) (bool, bool) {
 	v, ok := f.Lookup(groupName, key)
 	if !ok {
-		return defaultValue
+		return false, false
 	}
 
 	return strings.EqualFold(v, "1") ||
 		strings.EqualFold(v, "yes") ||
 		strings.EqualFold(v, "true") ||
-		strings.EqualFold(v, "on")
+		strings.EqualFold(v, "on"), true
+}
+
+// Lookup the last instance of a key and convert the value to a bool
+func (f *UnitFile) LookupBooleanWithDefault(groupName string, key string, defaultValue bool) bool {
+	v, ok := f.LookupBoolean(groupName, key)
+	if !ok {
+		return defaultValue
+	}
+
+	return v
 }
 
 /* Mimics strol, which is what systemd uses */

pkg/systemd/quadlet/podmancmdline.go

@@ -57,6 +57,14 @@ func (c *PodmanCmdline) addAnnotations(annotations map[string]string) {
 	c.addKeys("--annotation", annotations)
 }
 
+func (c *PodmanCmdline) addBool(arg string, val bool) {
+	if val {
+		c.add(arg)
+	} else {
+		c.addf("%s=false", arg)
+	}
+}
+
 func NewPodmanCmdline(args ...string) *PodmanCmdline {
 	c := &PodmanCmdline{
 		Args: make([]string, 0),

pkg/systemd/quadlet/quadlet.go

@@ -295,13 +295,13 @@ func ConvertContainer(container *parser.UnitFile, isUser bool) (*parser.UnitFile
 	addNetworks(container, ContainerGroup, service, podman)
 
 	// Run with a pid1 init to reap zombies by default (as most apps don't do that)
-	runInit := container.LookupBoolean(ContainerGroup, KeyRunInit, false)
-	if runInit {
-		podman.add("--init")
+	runInit, ok := container.LookupBoolean(ContainerGroup, KeyRunInit)
+	if ok {
+		podman.addBool("--init", runInit)
 	}
 
 	// By default we handle startup notification with conmon, but allow passing it to the container with Notify=yes
-	notify := container.LookupBoolean(ContainerGroup, KeyNotify, false)
+	notify := container.LookupBooleanWithDefault(ContainerGroup, KeyNotify, false)
 	if notify {
 		podman.add("--sdnotify=container")
 	} else {
@@ -316,7 +316,7 @@ func ConvertContainer(container *parser.UnitFile, isUser bool) (*parser.UnitFile
 	}
 
 	// Default to no higher level privileges or caps
-	noNewPrivileges := container.LookupBoolean(ContainerGroup, KeyNoNewPrivileges, false)
+	noNewPrivileges := container.LookupBooleanWithDefault(ContainerGroup, KeyNoNewPrivileges, false)
 	if noNewPrivileges {
 		podman.add("--security-opt=no-new-privileges")
 	}
@@ -345,12 +345,12 @@ func ConvertContainer(container *parser.UnitFile, isUser bool) (*parser.UnitFile
 		podman.addf("--cap-add=%s", strings.ToLower(caps))
 	}
 
-	readOnly := container.LookupBoolean(ContainerGroup, KeyReadOnly, false)
-	if readOnly {
-		podman.add("--read-only")
+	readOnly, ok := container.LookupBoolean(ContainerGroup, KeyReadOnly)
+	if ok {
+		podman.addBool("--read-only", readOnly)
 	}
 
-	volatileTmp := container.LookupBoolean(ContainerGroup, KeyVolatileTmp, false)
+	volatileTmp := container.LookupBooleanWithDefault(ContainerGroup, KeyVolatileTmp, false)
 	if volatileTmp {
 		/* Read only mode already has a tmpfs by default */
 		if !readOnly {
@@ -537,7 +537,7 @@ func ConvertNetwork(network *parser.UnitFile, name string) (*parser.UnitFile, er
 
 	podman := NewPodmanCmdline("network", "create", "--ignore")
 
-	if disableDNS := network.LookupBoolean(NetworkGroup, KeyNetworkDisableDNS, false); disableDNS {
+	if disableDNS := network.LookupBooleanWithDefault(NetworkGroup, KeyNetworkDisableDNS, false); disableDNS {
 		podman.add("--disable-dns")
 	}
 
@@ -569,15 +569,15 @@ func ConvertNetwork(network *parser.UnitFile, name string) (*parser.UnitFile, er
 		return nil, fmt.Errorf("cannot set gateway or range without subnet")
 	}
 
-	if internal := network.LookupBoolean(NetworkGroup, KeyNetworkInternal, false); internal {
+	if internal := network.LookupBooleanWithDefault(NetworkGroup, KeyNetworkInternal, false); internal {
 		podman.add("--internal")
 	}
 
 	if ipamDriver, ok := network.Lookup(NetworkGroup, KeyNetworkIPAMDriver); ok && len(ipamDriver) > 0 {
 		podman.addf("--ipam-driver=%s", ipamDriver)
 	}
 
-	if ipv6 := network.LookupBoolean(NetworkGroup, KeyNetworkIPv6, false); ipv6 {
+	if ipv6 := network.LookupBooleanWithDefault(NetworkGroup, KeyNetworkIPv6, false); ipv6 {
 		podman.add("--ipv6")
 	}
 

test/e2e/quadlet/basepodman.container

@@ -4,9 +4,7 @@
 Image=localhost/imagename
 
 # Disable all default features to get as empty podman run command as we can
-ReadOnly=no
 NoNewPrivileges=no
 DropCapability=
-RunInit=no
 VolatileTmp=no
 Timezone=

test/e2e/quadlet/readwrite-notmpfs.container

@@ -1,3 +1,4 @@
+## assert-podman-args "--read-only=false"
 ## !assert-podman-args "--read-only"
 ## !assert-podman-args "--tmpfs" "/tmp:rw,size=512M,mode=1777"
 

test/e2e/quadlet/readwrite.container

@@ -1,4 +1,5 @@
 ## !assert-podman-args "--read-only"
+## assert-podman-args "--read-only=false"
 ## assert-podman-args "--tmpfs" "/tmp:rw,size=512M,mode=1777"
 
 [Container]