Merge pull request #16299 from alexlarsson/quadlet-shortname-warning

quadlet: Warn in generator if using short names

cmd/quadlet/main.go

@@ -8,6 +8,7 @@ import (
 	"path"
 	"path/filepath"
 	"strings"
+	"unicode"
 
 	"github.com/containers/podman/v4/pkg/systemd/parser"
 	"github.com/containers/podman/v4/pkg/systemd/quadlet"
@@ -210,6 +211,71 @@ func enableServiceFile(outputPath string, service *parser.UnitFile) {
 	}
 }
 
+func isImageID(imageName string) bool {
+	// All sha25:... names are assumed by podman to be fully specified
+	if strings.HasPrefix(imageName, "sha256:") {
+		return true
+	}
+
+	// However, podman also accepts image ids as pure hex strings,
+	// but only those of length 64 are unambigous image ids
+	if len(imageName) != 64 {
+		return false
+	}
+
+	for _, c := range imageName {
+		if !unicode.Is(unicode.Hex_Digit, c) {
+			return false
+		}
+	}
+
+	return true
+}
+
+func isUnambiguousName(imageName string) bool {
+	// Fully specified image ids are unambigous
+	if isImageID(imageName) {
+		return true
+	}
+
+	// Otherwise we require a fully qualified name
+	firstSlash := strings.Index(imageName, "/")
+	if firstSlash == -1 {
+		// No domain or path, not fully qualified
+		return false
+	}
+
+	// What is before the first slash can be a domain or a path
+	domain := imageName[:firstSlash]
+
+	// If its a a domain (has dot or port or is "localhost") it is considered fq
+	if strings.ContainsAny(domain, ".:") || domain == "localhost" {
+		return true
+	}
+
+	return false
+}
+
+// warns if input is an ambigious name, i.e. a partial image id or a short
+// name (i.e. is missing a registry)
+//
+// Examples:
+//   - short names: "image:tag", "library/fedora"
+//   - fully qualified names: "quay.io/image", "localhost/image:tag",
+//     "server.org:5000/lib/image", "sha256:..."
+//
+// We implement a simple version of this from scratch here to avoid
+// a huge dependency in the generator just for a warning.
+func warnIfAmbigiousName(container *parser.UnitFile) {
+	imageName, ok := container.Lookup(quadlet.ContainerGroup, quadlet.KeyImage)
+	if !ok {
+		return
+	}
+	if !isUnambiguousName(imageName) {
+		Logf("Warning: %s specifies the image \"%s\" which not a fully qualified image name. This is not ideal for performance and security reasons. See the podman-pull manpage discussion of short-name-aliases.conf for details.", container.Filename, imageName)
+	}
+}
+
 func main() {
 	prgname := path.Base(os.Args[0])
 	isUser = strings.Contains(prgname, "user")
@@ -252,6 +318,7 @@ func main() {
 
 		switch {
 		case strings.HasSuffix(name, ".container"):
+			warnIfAmbigiousName(unit)
 			service, err = quadlet.ConvertContainer(unit, isUser)
 		case strings.HasSuffix(name, ".volume"):
 			service, err = quadlet.ConvertVolume(unit, name)

cmd/quadlet/main_test.go

@@ -0,0 +1,42 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIsUnambiguousName(t *testing.T) {
+	tests := []struct {
+		input string
+		res   bool
+	}{
+		// Ambiguous names
+		{"fedora", false},
+		{"fedora:latest", false},
+		{"library/fedora", false},
+		{"library/fedora:latest", false},
+		{"busybox@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a", false},
+		{"busybox:latest@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a", false},
+		{"d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05", false},
+		{"d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05aa", false},
+
+		// Unambiguous names
+		{"quay.io/fedora", true},
+		{"docker.io/fedora", true},
+		{"docker.io/library/fedora:latest", true},
+		{"localhost/fedora", true},
+		{"localhost:5000/fedora:latest", true},
+		{"example.foo.this.may.be.garbage.but.maybe.not:1234/fedora:latest", true},
+		{"docker.io/library/busybox@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a", true},
+		{"docker.io/library/busybox:latest@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a", true},
+		{"docker.io/fedora@sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a", true},
+		{"sha256:d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a", true},
+		{"d366a4665ab44f0648d7a00ae3fae139d55e32f9712c67accd604bb55df9d05a", true},
+	}
+
+	for _, test := range tests {
+		res := isUnambiguousName(test.input)
+		assert.Equal(t, res, test.res, "%q", test.input)
+	}
+}

test/e2e/quadlet/annotation.container

@@ -1,11 +1,11 @@
-## assert-podman-final-args imagename
+## assert-podman-final-args localhost/imagename
 ## assert-podman-args "--annotation" "org.foo.Arg0=arg0"
 ## assert-podman-args "--annotation" "org.foo.Arg1=arg1"
 ## assert-podman-args "--annotation" "org.foo.Arg2=arg 2"
 ## assert-podman-args "--annotation" "org.foo.Arg3=arg3"
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 Annotation=org.foo.Arg1=arg1 "org.foo.Arg2=arg 2" \
   org.foo.Arg3=arg3
 

test/e2e/quadlet/basepodman.container

@@ -1,7 +1,7 @@
-## assert-podman-final-args run --name=systemd-%N --cidfile=%t/%N.cid --replace --rm -d --log-driver passthrough --pull=never --runtime /usr/bin/crun --cgroups=split --sdnotify=conmon imagename
+## assert-podman-final-args run --name=systemd-%N --cidfile=%t/%N.cid --replace --rm -d --log-driver passthrough --pull=never --runtime /usr/bin/crun --cgroups=split --sdnotify=conmon localhost/imagename
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 
 # Disable all default features to get as empty podman run command as we can
 ReadOnly=no

test/e2e/quadlet/basic.container

@@ -1,4 +1,4 @@
-## assert-podman-final-args imagename
+## assert-podman-final-args localhost/imagename
 ## assert-podman-args "--name=systemd-%N"
 ## assert-podman-args "--cidfile=%t/%N.cid"
 ## assert-podman-args "--rm"
@@ -25,4 +25,4 @@
 ## assert-key-is "Service" "Environment" "PODMAN_SYSTEMD_UNIT=%n"
 
 [Container]
-Image=imagename
+Image=localhost/imagename

test/e2e/quadlet/capabilities.container

@@ -4,7 +4,7 @@
 ## assert-podman-args "--cap-add=cap_ipc_owner"
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 # Verify that we can reset to the default cap set
 DropCapability=
 AddCapability=CAP_DAC_OVERRIDE CAP_AUDIT_WRITE

test/e2e/quadlet/env.container

@@ -1,12 +1,12 @@
-## assert-podman-final-args imagename
+## assert-podman-final-args localhost/imagename
 ## assert-podman-args --env "FOO1=foo1"
 ## assert-podman-args --env "FOO2=foo2 "
 ## assert-podman-args --env "FOO3=foo3"
 ## assert-podman-args --env "REPLACE=replaced"
 ## assert-podman-args --env "FOO4=foo\\nfoo"
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 Environment=FOO1=foo1 "FOO2=foo2 " \
                      FOO3=foo3 REPLACE=replace
 Environment=REPLACE=replaced 'FOO4=foo\nfoo'

test/e2e/quadlet/escapes.container

@@ -1,5 +1,5 @@
 ## assert-podman-final-args "/some/path" "an arg" "a;b\\nc\\td'e" "a;b\\nc\\td" "a\"b"
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 Exec=/some/path "an arg" "a;b\nc\td'e" a;b\nc\td 'a"b'

test/e2e/quadlet/exec.container

@@ -1,6 +1,6 @@
-## assert-podman-final-args imagename "/some/binary file" "--arg1" "arg 2"
+## assert-podman-final-args localhost/imagename "/some/binary file" "--arg1" "arg 2"
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 Exec="/some/binary file" --arg1 \
   "arg 2"

test/e2e/quadlet/image.container

@@ -1,4 +1,4 @@
-## assert-podman-final-args imagename
+## assert-podman-final-args localhost/imagename
 
 [Container]
-Image=imagename
+Image=localhost/imagename

test/e2e/quadlet/install.container

@@ -9,7 +9,7 @@
 ## assert-symlink req3.service.requires/install.service ../install.service
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 
 [Install]
 Alias=alias.service \

test/e2e/quadlet/label.container

@@ -1,11 +1,11 @@
-## assert-podman-final-args imagename
+## assert-podman-final-args localhost/imagename
 ## assert-podman-args "--label" "org.foo.Arg0=arg0"
 ## assert-podman-args "--label" "org.foo.Arg1=arg1"
 ## assert-podman-args "--label" "org.foo.Arg2=arg 2"
 ## assert-podman-args "--label" "org.foo.Arg3=arg3"
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 Label=org.foo.Arg1=arg1 "org.foo.Arg2=arg 2" \
   org.foo.Arg3=arg3
 

test/e2e/quadlet/name.container

@@ -1,5 +1,5 @@
 ## assert-podman-args "--name=foobar"
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 ContainerName=foobar

test/e2e/quadlet/noremapuser.container

@@ -2,5 +2,5 @@
 ## !assert-podman-args --gidmap
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 RemapUsers=no

test/e2e/quadlet/noremapuser2.container

@@ -20,7 +20,7 @@
 ## assert-podman-args --gidmap 1002:1002:4294966293
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 RemapUsers=no
 User=1000
 Group=1001

test/e2e/quadlet/notify.container

@@ -1,5 +1,5 @@
 ## assert-podman-args "--sdnotify=container"
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 Notify=yes

test/e2e/quadlet/other-sections.container

@@ -1,10 +1,10 @@
-## assert-podman-final-args imagename
+## assert-podman-final-args localhost/imagename
 ## assert-key-is "Unit" "Foo" "bar1" "bar2"
-## assert-key-is "X-Container" "Image" "imagename"
+## assert-key-is "X-Container" "Image" "localhost/imagename"
 
 [Unit]
 Foo=bar1
 Foo=bar2
 
 [Container]
-Image=imagename
+Image=localhost/imagename

test/e2e/quadlet/podmanargs.container

@@ -3,7 +3,7 @@
 ## assert-podman-args "--also"
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 PodmanArgs="--foo" \
   --bar
 PodmanArgs=--also

test/e2e/quadlet/ports.container

@@ -1,5 +1,5 @@
 [Container]
-Image=imagename
+Image=localhost/imagename
 ## assert-podman-args --expose=1000
 ExposeHostPort=1000
 ## assert-podman-args --expose=2000-3000

test/e2e/quadlet/ports_ipv6.container

@@ -1,5 +1,5 @@
 [Container]
-Image=imagename
+Image=localhost/imagename
 ## assert-podman-args -p=[::1]:80:90
 PublishPort=[::1]:80:90
 

test/e2e/quadlet/shortname.container

@@ -0,0 +1,4 @@
+## assert-stderr-contains "not a fully qualified image name"
+
+[Container]
+Image=shortname

test/e2e/quadlet/socketactivated.container

@@ -5,5 +5,5 @@
 ## assert-key-is "Service" "NotifyAccess" "all"
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 SocketActivated=yes

test/e2e/quadlet/timezone.container

@@ -1,5 +1,5 @@
 ## assert-podman-args --tz=foo
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 Timezone=foo

test/e2e/quadlet/user-host.container

@@ -11,7 +11,7 @@
 ## assert-podman-args --gidmap 1002:101000:99000
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 User=1000
 HostUser=900
 Group=1001

test/e2e/quadlet/user-root1.container

@@ -14,7 +14,7 @@
 # This means container root must map to something else
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 User=1000
 # Also test name parsing
 HostUser=root

test/e2e/quadlet/user-root2.container

@@ -10,7 +10,7 @@
 # Map container uid root to host root
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 User=0
 # Also test name parsing
 HostUser=root

test/e2e/quadlet/user.container

@@ -1,7 +1,7 @@
-## assert-podman-final-args imagename
+## assert-podman-final-args localhost/imagename
 ## assert-podman-args "--user" "998:999"
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 User=998
 Group=999

test/e2e/quadlet/volume.container

@@ -1,10 +1,10 @@
 ## assert-podman-args -v /host/dir:/container/volume
 ## assert-podman-args -v /host/dir2:/container/volume2:Z
 ## assert-podman-args -v named:/container/named
-## assert-podman-args -v systemd-quadlet:/container/quadlet imagename
+## assert-podman-args -v systemd-quadlet:/container/quadlet localhost/imagename
 
 [Container]
-Image=imagename
+Image=localhost/imagename
 Volume=/host/dir:/container/volume
 Volume=/host/dir2:/container/volume2:Z
 Volume=/container/empty

test/e2e/quadlet_test.go

@@ -291,6 +291,7 @@ var _ = Describe("quadlet system generator", func() {
 		Entry("readwrite.container", "readwrite.container"),
 		Entry("readwrite-notmpfs.container", "readwrite-notmpfs.container"),
 		Entry("seccomp.container", "seccomp.container"),
+		Entry("shortname.container", "shortname.container"),
 		Entry("timezone.container", "timezone.container"),
 		Entry("user.container", "user.container"),
 		Entry("user-host.container", "user-host.container"),