Expand drop-in search paths

* top-level (pod.d)
* truncated (unit-.container.d)

Signed-off-by: Bennie Milburn-Town <63211101+benniekiss@users.noreply.github.com>

cmd/quadlet/main.go

@@ -158,9 +158,12 @@ func appendSubPaths(dirs []string, path string, isUserFlag bool, filterPtr func(
 	}
 
 	err = filepath.WalkDir(resolvedPath, func(_path string, info os.DirEntry, err error) error {
-		if info == nil || info.IsDir() {
+		// Ignore drop-in directory subpaths
-			if filterPtr == nil || filterPtr(_path, isUserFlag) {
+		if !strings.HasSuffix(_path, ".d") {
-				dirs = append(dirs, _path)
+			if info == nil || info.IsDir() {
+				if filterPtr == nil || filterPtr(_path, isUserFlag) {
+					dirs = append(dirs, _path)
+				}
 			}
 		}
 		return err
@@ -256,16 +259,11 @@ func loadUnitDropins(unit *parser.UnitFile, sourcePaths []string) error {
 	}
 
 	dropinDirs := []string{}
+	unitDropinPaths := unit.GetUnitDropinPaths()
 
 	for _, sourcePath := range sourcePaths {
-		dropinDirs = append(dropinDirs, path.Join(sourcePath, unit.Filename+".d"))
+		for _, dropinPath := range unitDropinPaths {
-	}
+			dropinDirs = append(dropinDirs, path.Join(sourcePath, dropinPath))
-
-	// For instantiated templates, also look in the non-instanced template dropin dirs
-	templateBase, templateInstance := unit.GetTemplateParts()
-	if templateBase != "" && templateInstance != "" {
-		for _, sourcePath := range sourcePaths {
-			dropinDirs = append(dropinDirs, path.Join(sourcePath, templateBase+".d"))
 		}
 	}
 
@@ -359,15 +357,14 @@ func enableServiceFile(outputPath string, service *parser.UnitFile) {
 	}
 
 	serviceFilename := service.Filename
-	templateBase, templateInstance := service.GetTemplateParts()
+	templateBase, templateInstance, isTemplate := service.GetTemplateParts()
 
 	// For non-instantiated template service we only support installs if a
 	// DefaultInstance is given. Otherwise we ignore the Install group, but
 	// it is still useful when instantiating the unit via a symlink.
-	if templateBase != "" && templateInstance == "" {
+	if isTemplate && templateInstance == "" {
 		if defaultInstance, ok := service.Lookup(quadlet.InstallGroup, "DefaultInstance"); ok {
-			parts := strings.SplitN(templateBase, "@", 2)
+			serviceFilename = templateBase + "@" + defaultInstance + filepath.Ext(serviceFilename)
-			serviceFilename = parts[0] + "@" + defaultInstance + parts[1]
 		} else {
 			serviceFilename = ""
 		}

docs/source/markdown/podman-systemd.unit.5.md

@@ -50,11 +50,15 @@ other sections are passed on untouched, allowing the use of any normal systemd c
 like dependencies or cgroup limits.
 
 The source files also support drop-ins in the same [way systemd does](https://www.freedesktop.org/software/systemd/man/latest/systemd.unit.html).
-For a given source file (say `foo.container`), the corresponding `.d`directory (in this
+For a given source file (`foo.container`), the corresponding `.d` directory (`foo.container.d`) will
-case `foo.container.d`) will be scanned for files with a `.conf` extension that are merged into
+be scanned for files with a `.conf` extension, which are then merged into the base file in alphabetical
-the base file in alphabetical order. The format of these drop-in files is the same as the base file.
+order. Top-level type drop-ins (`container.d`) will also be included. If the unit contains dashes ("-")
-This is useful to alter or add configuration settings for a unit, without having to modify unit
+in the name (`foo-bar-baz.container`), then the drop-in directories generated by truncating the name after
-files.
+the dash are searched as well (`foo-.container.d` and `foo-bar-.container.d`). Drop-in files with the same name
+further down the hierarchy override those further up (`foo-bar-baz.container.d/10-override.conf` overrides
+`foo-bar-.container.d/10-override.conf`, which overrides `foo-.service.d/10-override.conf`, which overrides
+`container.d/10-override.conf`). The format of these drop-in files is the same as the base file. This is useful
+to alter or add configuration settings for a unit, without having to modify unit files.
 
 For rootless containers, when administrators place Quadlet files in the
 /etc/containers/systemd/users directory, all users' sessions execute the

pkg/systemd/parser/unitfile.go

@@ -939,14 +939,63 @@ func (f *UnitFile) PrependUnitLine(groupName string, key string, value string) {
 	group.prependLine(newUnitLine(key, value, false))
 }
 
-func (f *UnitFile) GetTemplateParts() (string, string) {
+func (f *UnitFile) GetTemplateParts() (string, string, bool) {
 	ext := filepath.Ext(f.Filename)
 	basename := strings.TrimSuffix(f.Filename, ext)
 	parts := strings.SplitN(basename, "@", 2)
 	if len(parts) < 2 {
-		return "", ""
+		return parts[0], "", false
 	}
-	return parts[0] + "@" + ext, parts[1]
+	return parts[0], parts[1], true
+}
+
+func (f *UnitFile) GetUnitDropinPaths() []string {
+	unitName, instanceName, isTemplate := f.GetTemplateParts()
+
+	ext := filepath.Ext(f.Filename)
+	dropinExt := ext + ".d"
+
+	dropinPaths := []string{}
+
+	// Add top-level drop-in location (pod.d, container.d, etc)
+	topLevelDropIn := strings.TrimPrefix(dropinExt, ".")
+	dropinPaths = append(dropinPaths, topLevelDropIn)
+
+	truncatedParts := strings.Split(unitName, "-")
+	// If the unit contains any '-', then there are truncated paths to search.
+	if len(truncatedParts) > 1 {
+		// We don't need the last item because that would be the full path
+		truncatedParts = truncatedParts[:len(truncatedParts)-1]
+		// Truncated instance names are not included in the drop-in search path
+		// i.e. template-unit@base-instance.service does not search template-unit@base-.service
+		// So we only search truncations of the template name, i.e. template-@.service, and unit name, i.e. template-.service
+		// or only the unit name if it is not a template.
+		for i := range truncatedParts {
+			truncatedUnitPath := strings.Join(truncatedParts[:i+1], "-") + "-"
+			dropinPaths = append(dropinPaths, truncatedUnitPath+dropinExt)
+			// If the unit is a template, add the truncated template name as well.
+			if isTemplate {
+				truncatedTemplatePath := truncatedUnitPath + "@"
+				dropinPaths = append(dropinPaths, truncatedTemplatePath+dropinExt)
+			}
+		}
+	}
+	// For instanced templates, add the base template unit search path
+	if instanceName != "" {
+		dropinPaths = append(dropinPaths, unitName+"@"+dropinExt)
+	}
+	// Add the drop-in directory for the full filename
+	dropinPaths = append(dropinPaths, f.Filename+".d")
+	// Finally, reverse the list so that when drop-ins are parsed,
+	// the most specific are applied instead of the most broad.
+	// dropinPaths should be a list where the items are in order of specific -> broad
+	// i.e., the most specific search path is dropinPaths[0], and broadest search path is dropinPaths[len(dropinPaths)-1]
+	// Uses https://go.dev/wiki/SliceTricks#reversing
+	for i := len(dropinPaths)/2 - 1; i >= 0; i-- {
+		opp := len(dropinPaths) - 1 - i
+		dropinPaths[i], dropinPaths[opp] = dropinPaths[opp], dropinPaths[i]
+	}
+	return dropinPaths
 }
 
 func PathEscape(path string) string {

pkg/systemd/parser/unitfile_test.go

@@ -1,6 +1,7 @@
 package parser
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -226,6 +227,43 @@ Also=systemd-networkd-wait-online.service
 
 var samples = []string{memcachedService, systemdloginService, systemdnetworkdService}
 
+const sampleDropinService string = "sample-unit.service"
+
+var sampleDropinServicePaths = []string{
+	"sample-unit.service.d",
+	"sample-.service.d",
+	"service.d",
+}
+
+const sampleDropinTemplate string = "sample-template-unit@.service"
+
+var sampleDropinTemplatePaths = []string{
+	"sample-template-unit@.service.d",
+	"sample-template-@.service.d",
+	"sample-template-.service.d",
+	"sample-@.service.d",
+	"sample-.service.d",
+	"service.d",
+}
+
+const sampleDropinTemplateInstance string = "sample-template-unit@base-instance.service"
+
+var sampleDropinTemplateInstancePaths = []string{
+	"sample-template-unit@base-instance.service.d",
+	"sample-template-unit@.service.d",
+	"sample-template-@.service.d",
+	"sample-template-.service.d",
+	"sample-@.service.d",
+	"sample-.service.d",
+	"service.d",
+}
+
+var sampleDropinPaths = map[string][]string{
+	sampleDropinService:          sampleDropinServicePaths,
+	sampleDropinTemplate:         sampleDropinTemplatePaths,
+	sampleDropinTemplateInstance: sampleDropinTemplateInstancePaths,
+}
+
 func TestRanges_Roundtrip(t *testing.T) {
 	for i := range samples {
 		sample := samples[i]
@@ -243,3 +281,14 @@ func TestRanges_Roundtrip(t *testing.T) {
 		assert.Equal(t, sample, asStr)
 	}
 }
+
+func TestUnitDropinPaths_Search(t *testing.T) {
+	for filename, expectedPaths := range sampleDropinPaths {
+		f := UnitFile{
+			Filename: filename,
+		}
+		generatedPaths := f.GetUnitDropinPaths()
+
+		assert.True(t, reflect.DeepEqual(expectedPaths, generatedPaths))
+	}
+}

test/system/252-quadlet.bats

@@ -64,14 +64,18 @@ function run_quadlet() {
     assert $status -eq 0 "Failed to convert quadlet file: $sourcefile"
     is "$output" "" "quadlet should report no errors"
 
+    run cat $UNIT_DIR/$service
+    assert $status -eq 0 "Could not cat $UNIT_DIR/$service"
+    echo "$output"
+    local content="$output"
+
     # Ensure this is teared down
     UNIT_FILES+=("$UNIT_DIR/$service")
 
     QUADLET_SERVICE_NAME="$service"
+    QUADLET_SERVICE_CONTENT="$content"
     QUADLET_SYSLOG_ID="$(basename $service .service)"
     QUADLET_CONTAINER_NAME="systemd-$QUADLET_SYSLOG_ID"
-
-    cat $UNIT_DIR/$QUADLET_SERVICE_NAME
 }
 
 function service_setup() {
@@ -1599,4 +1603,58 @@ EOF
     run_podman rmi $untagged_image:latest $built_image $(pause_image)
     run_podman network rm podman-default-kube-network
 }
+
+@test "quadlet - drop-in files" {
+    local quadlet_tmpdir="${PODMAN_TMPDIR}/dropins"
+
+    local quadlet_file="truncated-$(random_string).container"
+
+    local -A dropin_dirs=(
+        [toplevel]=container.d
+        [truncated]=truncated-.container.d
+        [quadlet]="${quadlet_file}.d"
+    )
+
+    # Table of drop-in .conf files. Format is:
+    #
+    #    apply | dir | filename | [Section] | Content=...
+    local dropin_files="
+y | toplevel  | 10 | [Unit]      | Description=Test File for Dropin Configuration
+n | toplevel  | 99 | [Install]   | WantedBy=default.target
+y | truncated | 50 | [Container] | ContainerName=truncated-dropins
+n | truncated | 99 | [Service]   | Restart=always
+n | truncated | 99 | [Install]   | WantedBy=multiuser.target
+y | quadlet   | 99 | [Service]   | RestartSec=60s
+"
+
+    # Pass 1: Create all drop-in directories and files
+    while read apply dir file section content; do
+        local d="${quadlet_tmpdir}/${dropin_dirs[${dir}]}"
+        mkdir -p "${d}"
+
+        local f="${d}/${file}.conf"
+        echo "${section}" >>"${f}"
+        echo "${content}" >>"${f}"
+    done < <(parse_table "${dropin_files}")
+
+    # Create the base quadlet file
+    quadlet_base="${PODMAN_TMPDIR}/${quadlet_file}"
+    cat > "${quadlet_base}" <<EOF
+[Container]
+Image="${IMAGE}"
+EOF
+
+    # Generate the quadlet file from the base file and any drop-in .conf files.
+    run_quadlet "${quadlet_base}" "${quadlet_tmpdir}"
+
+    # Pass 2: test whether the expected .conf files are applied
+    # and the overridden .conf files are not.
+    while read apply dir file section content; do
+        if [[ "${apply}" = "y" ]]; then
+            assert "${QUADLET_SERVICE_CONTENT}" =~ "${content}" "Set in ${dir}/${file}.conf"
+        else
+            assert "${QUADLET_SERVICE_CONTENT}" !~ "${content}" "Set in ${dir}/${file}.conf but should have been overridden"
+        fi
+    done < <(parse_table "${dropin_files}")
+}
 # vim: filetype=sh