Switch to RFC compliant opaque template:name URLs

The old style template://name URLs are malformed because they do not
contain a valid AUTHORITY (host name), but treat the host name as part
of the path. They are still fully supported, but will emit a warning.

Signed-off-by: Jan Dubois <[email protected]>

Author: jandubois

cmd/limactl/template.go

@@ -52,13 +52,13 @@ func newValidateCommand() *cobra.Command {
 	return validateCommand
 }
 
-var templateCopyExample = `  Template locators are local files, file://, https://, or template:// URLs
+var templateCopyExample = `  Template locators are local files, file://, https://, or template: URLs
 
   # Copy default template to STDOUT
-  limactl template copy template://default -
+  limactl template copy template:default -
 
   # Copy template from web location to local file and embed all external references
-  # (this does not embed template:// references)
+  # (this does not embed template: references)
   limactl template copy --embed https://example.com/lima.yaml mighty-machine.yaml
 `
 
@@ -177,10 +177,10 @@ External references are embedded and default values are filled in
 before the YQ expression is evaluated.
 
 Example:
-  limactl template yq template://default '.images[].location'
+  limactl template yq template:default '.images[].location'
 
 The example command is equivalent to using an external yq command like this:
-  limactl template copy --fill template://default - | yq '.images[].location'
+  limactl template copy --fill template:default - | yq '.images[].location'
 `
 
 func newTemplateYQCommand() *cobra.Command {

pkg/limainfo/limainfo.go

@@ -48,7 +48,7 @@ type GuestAgent struct {
 }
 
 // New returns a LimaInfo object with the Lima version, a list of all Templates and their location,
-// the DefaultTemplate corresponding to template://default with all defaults filled in, the
+// the DefaultTemplate corresponding to template:default with all defaults filled in, the
 // LimaHome location, a list of all supported VMTypes, and a map of GuestAgents for each architecture.
 func New(ctx context.Context) (*LimaInfo, error) {
 	b, err := templatestore.Read(templatestore.Default)

pkg/limatmpl/abs.go

@@ -89,6 +89,9 @@ func basePath(locator string) (string, error) {
 	// Single-letter schemes will be drive names on Windows, e.g. "c:/foo"
 	if err == nil && len(u.Scheme) > 1 {
 		// path.Dir("") returns ".", which must be removed for url.JoinPath() to do the right thing later
+		if u.Opaque != "" {
+			return u.Scheme + ":" + strings.TrimSuffix(path.Dir(u.Opaque), "."), nil
+		}
 		return u.Scheme + "://" + strings.TrimSuffix(path.Dir(path.Join(u.Host, u.Path)), "."), nil
 	}
 	base, err := filepath.Abs(filepath.Dir(locator))
@@ -132,6 +135,10 @@ func absPath(locator, basePath string) (string, error) {
 			return "", err
 		}
 		if len(u.Scheme) > 1 {
+			// Treat empty "template:" URL as opaque
+			if u.Opaque != "" || (u.Scheme == "template" && u.Host == "") {
+				return u.Scheme + ":" + path.Join(u.Opaque, locator), nil
+			}
 			return u.JoinPath(locator).String(), nil
 		}
 		locator = filepath.Join(basePath, locator)

pkg/limatmpl/abs_test.go

@@ -23,67 +23,73 @@ type useAbsLocatorsTestCase struct {
 var useAbsLocatorsTestCases = []useAbsLocatorsTestCase{
 	{
 		"Template without base or script file",
-		"template://foo",
+		"template:foo",
 		`arch: aarch64`,
 		`arch: aarch64`,
 	},
 	{
 		"Single string base template",
+		"template:foo",
+		`base: bar.yaml`,
+		`base: template:bar.yaml`,
+	},
+	{
+		"Legacy template:// base template",
 		"template://foo",
 		`base: bar.yaml`,
-		`base: template://bar.yaml`,
+		`base: template:bar.yaml`,
 	},
 	{
 		"Flow style array of one base template",
-		"template://foo",
+		"template:foo",
 		`base: [{url: bar.yaml, digest: deadbeef}]`,
 		// not sure why the quotes around the URL were added; maybe because we don't copy the style from the source
-		`base: [{url: 'template://bar.yaml', digest: deadbeef}]`,
+		`base: [{url: 'template:bar.yaml', digest: deadbeef}]`,
 	},
 	{
 		"Flow style array of sequence of two base URLs",
-		"template://foo",
+		"template:foo",
 		`base: [bar.yaml, baz.yaml]`,
-		`base: ['template://bar.yaml', 'template://baz.yaml']`,
+		`base: ['template:bar.yaml', 'template:baz.yaml']`,
 	},
 	{
 		"Flow style array of sequence of two base locator objects",
-		"template://foo",
+		"template:foo",
 		`base: [{url: bar.yaml, digest: deadbeef}, {url: baz.yaml, digest: decafbad}]`,
-		`base: [{url: 'template://bar.yaml', digest: deadbeef}, {url: 'template://baz.yaml', digest: decafbad}]`,
+		`base: [{url: 'template:bar.yaml', digest: deadbeef}, {url: 'template:baz.yaml', digest: decafbad}]`,
 	},
 	{
 		"Block style array of one base template",
-		"template://foo",
+		"template:foo",
 		`
 base:
 - bar.yaml
 `,
 		`
 base:
-- template://bar.yaml`,
+- template:bar.yaml`,
 	},
 	{
 		"Block style of four base templates",
-		"template://foo",
+		"template:foo",
 		`
 base:
 - bar.yaml
-- template://my
+- template:my
 - https://example.com/my.yaml
 - baz.yaml
 `,
 		`
 base:
-- template://bar.yaml
-- template://my
+- template:bar.yaml
+- template:my
 - https://example.com/my.yaml
-- template://baz.yaml
+- template:baz.yaml
 `,
 	},
 	{
 		"Provisioning and probe scripts",
-		"template://experimental/foo",
+		"template:experimental/foo",
 		`
 provision:
 - mode: user
@@ -101,15 +107,15 @@ probes:
 		`
 provision:
 - mode: user
-  file: template://experimental/userscript.sh
+  file: template:experimental/userscript.sh
 - mode: system
   file:
-    url: template://experimental/systemscript.sh
+    url: template:experimental/systemscript.sh
     digest: abc123
 probes:
-- file: template://experimental/probe.sh
+- file: template:experimental/probe.sh
 - file:
-    url: template://experimental/probe.sh
+    url: template:experimental/probe.sh
     digest: digest
 `,
 	},
@@ -153,15 +159,15 @@ func TestBasePath(t *testing.T) {
 	})
 
 	t.Run("", func(t *testing.T) {
-		actual, err := basePath("template://foo")
+		actual, err := basePath("template:foo")
 		assert.NilError(t, err)
-		assert.Equal(t, actual, "template://")
+		assert.Equal(t, actual, "template:")
 	})
 
 	t.Run("", func(t *testing.T) {
-		actual, err := basePath("template://foo/bar")
+		actual, err := basePath("template:foo/bar")
 		assert.NilError(t, err)
-		assert.Equal(t, actual, "template://foo")
+		assert.Equal(t, actual, "template:foo")
 	})
 
 	t.Run("", func(t *testing.T) {
@@ -216,9 +222,9 @@ func TestAbsPath(t *testing.T) {
 	})
 
 	t.Run("", func(t *testing.T) {
-		actual, err := absPath("template://foo", volume+"/root")
+		actual, err := absPath("template:foo", volume+"/root")
 		assert.NilError(t, err)
-		assert.Equal(t, actual, "template://foo")
+		assert.Equal(t, actual, "template:foo")
 	})
 
 	t.Run("", func(t *testing.T) {
@@ -272,15 +278,15 @@ func TestAbsPath(t *testing.T) {
 	}
 
 	t.Run("", func(t *testing.T) {
-		actual, err := absPath("foo", "template://")
+		actual, err := absPath("foo", "template:")
 		assert.NilError(t, err)
-		assert.Equal(t, actual, "template://foo")
+		assert.Equal(t, actual, "template:foo")
 	})
 
 	t.Run("", func(t *testing.T) {
-		actual, err := absPath("bar", "template://foo")
+		actual, err := absPath("bar", "template:foo")
 		assert.NilError(t, err)
-		assert.Equal(t, actual, "template://foo/bar")
+		assert.Equal(t, actual, "template:foo/bar")
 	})
 
 	t.Run("", func(t *testing.T) {

pkg/limatmpl/embed.go

@@ -76,15 +76,15 @@ func (tmpl *Template) embedAllBases(ctx context.Context, embedAll, defaultBase b
 		}
 		isTemplate, _ := SeemsTemplateURL(baseLocator.URL)
 		if isTemplate && !embedAll {
-			// Once we skip a template:// URL we can no longer embed any other base template
+			// Once we skip a template: URL we can no longer embed any other base template
 			for i := 1; i < len(tmpl.Config.Base); i++ {
 				isTemplate, _ = SeemsTemplateURL(tmpl.Config.Base[i].URL)
 				if !isTemplate {
 					return fmt.Errorf("cannot embed template %q after not embedding %q", tmpl.Config.Base[i].URL, baseLocator.URL)
 				}
 			}
 			break
-			// TODO should we track embedding of template:// URLs so we can warn if we embed a non-template:// URL afterwards?
+			// TODO should we track embedding of template: URLs so we can warn if we embed a non-template: URL afterwards?
 		}
 
 		if seen[baseLocator.URL] {

pkg/limatmpl/embed_test.go

@@ -181,24 +181,24 @@ mounts:
 		`mounts: [{location: loc1}, {location: loc1, mountPoint: loc2, writable: true, sshfs: {followSymlinks: true}}, {location: loc1, mountPoint: loc3, writable: true}]`,
 	},
 	{
-		"template:// URLs are not embedded when embedAll is false",
+		"template: URLs are not embedded when embedAll is false",
 		// also tests file.url format
 		``,
 		`
-base: template://default
+base: template:default
 provision:
 - file:
-    url: template://provision.sh
+    url: template:provision.sh
 probes:
 - file:
-    url: template://probe.sh
+    url: template:probe.sh
 `,
 		`
-base: template://default
+base: template:default
 provision:
-- file: template://provision.sh
+- file: template:provision.sh
 probes:
-- file: template://probe.sh
+- file: template:probe.sh
 `,
 	},
 	{
@@ -214,18 +214,18 @@ vmType: qemu
 		`base template loop detected`,
 	},
 	{
-		"ERROR All bases following template:// bases must be template:// URLs too when embedAll is false",
+		"ERROR All bases following template: bases must be template: URLs too when embedAll is false",
 		``,
-		`base: [template://default, base1.yaml]`,
+		`base: [template:default, base1.yaml]`,
 		"after not embedding",
 	},
 	{
-		"ERROR All bases following template:// bases must be template:// URLs too when embedAll is false",
+		"ERROR All bases following template: bases must be template: URLs too when embedAll is false",
 		``,
 		`
 base: [base1.yaml, base2.yaml]
 ---
-base: template://default
+base: template:default
 ---
 base: baseX.yaml`,
 		"after not embedding",

pkg/limatmpl/locator.go

@@ -248,7 +248,10 @@ func SeemsTemplateURL(arg string) (isTemplate bool, templateName string) {
 		return false, ""
 	}
 	if u.Scheme == "template" {
-		return true, path.Join(u.Host, u.Path)
+		if u.Opaque == "" {
+			return true, path.Join(u.Host, u.Path)
+		}
+		return true, u.Opaque
 	}
 	return false, ""
 }
@@ -299,6 +302,16 @@ func TransformCustomURL(ctx context.Context, locator string) (string, error) {
 		return locator, nil
 	}
 
+	if u.Scheme == "template" {
+		if u.Opaque != "" {
+			return locator, nil
+		}
+		// Fix malformed "template:" URLs.
+		newLocator := "template:" + path.Join(u.Host, u.Path)
+		logrus.Warnf("Template locator %q should be written %q", locator, newLocator)
+		return newLocator, nil
+	}
+
 	plugin, err := plugins.Find("url-" + u.Scheme)
 	if err != nil {
 		return "", err

pkg/limatmpl/locator_test.go

@@ -64,7 +64,7 @@ func TestInstNameFromImageURL(t *testing.T) {
 }
 
 func TestSeemsTemplateURL(t *testing.T) {
-	arg := "template://foo/bar"
+	arg := "template:foo/bar"
 	t.Run(arg, func(t *testing.T) {
 		is, name := limatmpl.SeemsTemplateURL(arg)
 		assert.Equal(t, is, true)
@@ -96,7 +96,7 @@ func TestSeemsHTTPURL(t *testing.T) {
 	}
 	notHTTPURLs := []string{
 		"file:///foo",
-		"template://foo",
+		"template:foo",
 		"foo",
 	}
 	for _, arg := range notHTTPURLs {
@@ -114,7 +114,7 @@ func TestSeemsFileURL(t *testing.T) {
 	notFileURLs := []string{
 		"http://foo",
 		"https://foo",
-		"template://foo",
+		"template:foo",
 		"foo",
 	}
 	for _, arg := range notFileURLs {