From ada75c0bb86cae202ee2379e5c36f9a1d1a814fd Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 17 Oct 2024 15:40:53 +0200 Subject: [PATCH 1/2] test/e2e: test quadlet with and without --user This seems to be a testing gap, we need to test both for full coverage. Signed-off-by: Paul Holzinger --- test/e2e/quadlet_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/e2e/quadlet_test.go b/test/e2e/quadlet_test.go index 2f2f251d2b..cba32c4481 100644 --- a/test/e2e/quadlet_test.go +++ b/test/e2e/quadlet_test.go @@ -660,7 +660,12 @@ var _ = Describe("quadlet system generator", func() { } // Run quadlet to convert the file - session := podmanTest.Quadlet([]string{"--user", "--no-kmsg-log", generatedDir}, quadletDir) + var args []string + if isRootless() { + args = append(args, "--user") + } + args = append(args, "--no-kmsg-log", generatedDir) + session := podmanTest.Quadlet(args, quadletDir) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(exitCode)) From 9c6b1e20a3624ebf7eec563ba9f98fff9360e758 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 17 Oct 2024 15:42:12 +0200 Subject: [PATCH 2/2] quadlet: do not reject RemapUsers=keep-id as root This is simply wrong, as of commit de63ad7044 --userns=keep-id is also allowed as root. Signed-off-by: Paul Holzinger --- pkg/systemd/quadlet/quadlet.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/pkg/systemd/quadlet/quadlet.go b/pkg/systemd/quadlet/quadlet.go index 4c9ca689df..a369482fdf 100644 --- a/pkg/systemd/quadlet/quadlet.go +++ b/pkg/systemd/quadlet/quadlet.go @@ -782,7 +782,7 @@ func ConvertContainer(container *parser.UnitFile, isUser bool, unitsInfoMap map[ return nil, err } - if err := handleUserMappings(container, ContainerGroup, podman, isUser, true); err != nil { + if err := handleUserMappings(container, ContainerGroup, podman, true); err != nil { return nil, err } @@ -1224,7 +1224,7 @@ func ConvertKube(kube *parser.UnitFile, unitsInfoMap map[string]*UnitInfo, isUse handleLogDriver(kube, KubeGroup, execStart) handleLogOpt(kube, KubeGroup, execStart) - if err := handleUserMappings(kube, KubeGroup, execStart, isUser, false); err != nil { + if err := handleUserMappings(kube, KubeGroup, execStart, false); err != nil { return nil, err } @@ -1613,7 +1613,7 @@ func ConvertPod(podUnit *parser.UnitFile, name string, unitsInfoMap map[string]* "--replace", ) - if err := handleUserMappings(podUnit, PodGroup, execStartPre, isUser, true); err != nil { + if err := handleUserMappings(podUnit, PodGroup, execStartPre, true); err != nil { return nil, err } @@ -1684,7 +1684,7 @@ func handleUser(unitFile *parser.UnitFile, groupName string, podman *PodmanCmdli return nil } -func handleUserMappings(unitFile *parser.UnitFile, groupName string, podman *PodmanCmdline, isUser, supportManual bool) error { +func handleUserMappings(unitFile *parser.UnitFile, groupName string, podman *PodmanCmdline, supportManual bool) error { mappingsDefined := false if userns, ok := unitFile.Lookup(groupName, KeyUserNS); ok && len(userns) > 0 { @@ -1724,10 +1724,10 @@ func handleUserMappings(unitFile *parser.UnitFile, groupName string, podman *Pod return nil } - return handleUserRemap(unitFile, groupName, podman, isUser, supportManual) + return handleUserRemap(unitFile, groupName, podman, supportManual) } -func handleUserRemap(unitFile *parser.UnitFile, groupName string, podman *PodmanCmdline, isUser, supportManual bool) error { +func handleUserRemap(unitFile *parser.UnitFile, groupName string, podman *PodmanCmdline, supportManual bool) error { uidMaps := unitFile.LookupAllStrv(groupName, KeyRemapUid) gidMaps := unitFile.LookupAllStrv(groupName, KeyRemapGid) remapUsers, _ := unitFile.LookupLast(groupName, KeyRemapUsers) @@ -1765,10 +1765,6 @@ func handleUserRemap(unitFile *parser.UnitFile, groupName string, podman *Podman podman.add("--userns", usernsOpts("auto", autoOpts)) case "keep-id": - if !isUser { - return fmt.Errorf("RemapUsers=keep-id is unsupported for system units") - } - keepidOpts := make([]string, 0) if len(uidMaps) > 0 { if len(uidMaps) > 1 {