diff --git a/.nextchanges/bundles/bundle-init-unsupported-protocol.md b/.nextchanges/bundles/bundle-init-unsupported-protocol.md new file mode 100644 index 00000000000..557acf0eee7 --- /dev/null +++ b/.nextchanges/bundles/bundle-init-unsupported-protocol.md @@ -0,0 +1 @@ +`databricks bundle init` now reports an actionable error when given a template URL with an unsupported protocol (`http://`, `git://`, `ftp://`, `ftps://`) instead of failing with a confusing "not a bundle template" message ([#5902](https://github.com/databricks/cli/pull/5902)). diff --git a/acceptance/bundle/templates-machinery/supported-url/out.test.toml b/acceptance/bundle/templates-machinery/supported-url/out.test.toml new file mode 100644 index 00000000000..f784a183258 --- /dev/null +++ b/acceptance/bundle/templates-machinery/supported-url/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/templates-machinery/supported-url/output.txt b/acceptance/bundle/templates-machinery/supported-url/output.txt new file mode 100644 index 00000000000..959b4c6fb82 --- /dev/null +++ b/acceptance/bundle/templates-machinery/supported-url/output.txt @@ -0,0 +1,4 @@ + +=== https:// URL is recognized as a Git URL +=== ssh:// URL is recognized as a Git URL +=== git@ URL is recognized as a Git URL \ No newline at end of file diff --git a/acceptance/bundle/templates-machinery/supported-url/script b/acceptance/bundle/templates-machinery/supported-url/script new file mode 100644 index 00000000000..e1465d61516 --- /dev/null +++ b/acceptance/bundle/templates-machinery/supported-url/script @@ -0,0 +1,17 @@ +# These hosts do not resolve (.invalid is reserved by RFC 6761), so the clone +# fails fast without network access. The point of the test is that these URLs +# are recognized as Git URLs and a clone is attempted, rather than being misread +# as local template paths (which would report a "not a bundle template" error +# instead). The git error text varies by platform, so we route it to LOG +# (excluded from the diff) and assert the invariant with contains.py. +title "https:// URL is recognized as a Git URL" +errcode $CLI bundle init https://nonexistent.databricks.invalid/databricks/cli &> LOG.https +contains.py "git clone https://nonexistent.databricks.invalid/databricks/cli" "!not a bundle template" < LOG.https > /dev/null + +title "ssh:// URL is recognized as a Git URL" +errcode $CLI bundle init ssh://git@nonexistent.databricks.invalid/databricks/cli &> LOG.ssh +contains.py "git clone ssh://git@nonexistent.databricks.invalid/databricks/cli" "!not a bundle template" < LOG.ssh > /dev/null + +title "git@ URL is recognized as a Git URL" +errcode $CLI bundle init git@nonexistent.databricks.invalid:databricks/cli.git &> LOG.git +contains.py "git clone git@nonexistent.databricks.invalid:databricks/cli.git" "!not a bundle template" < LOG.git > /dev/null diff --git a/acceptance/bundle/templates-machinery/unsupported-url/out.test.toml b/acceptance/bundle/templates-machinery/unsupported-url/out.test.toml new file mode 100644 index 00000000000..f784a183258 --- /dev/null +++ b/acceptance/bundle/templates-machinery/unsupported-url/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/templates-machinery/unsupported-url/output.txt b/acceptance/bundle/templates-machinery/unsupported-url/output.txt new file mode 100644 index 00000000000..53b7362a7db --- /dev/null +++ b/acceptance/bundle/templates-machinery/unsupported-url/output.txt @@ -0,0 +1,12 @@ + +>>> musterr [CLI] bundle init http://github.com/databricks/cli +Error: unsupported protocol in Git URL "http://github.com/databricks/cli": only https://, ssh://, git@ URLs are supported + +>>> musterr [CLI] bundle init git://github.com/databricks/cli +Error: unsupported protocol in Git URL "git://github.com/databricks/cli": only https://, ssh://, git@ URLs are supported + +>>> musterr [CLI] bundle init ftp://github.com/databricks/cli +Error: unsupported protocol in Git URL "ftp://github.com/databricks/cli": only https://, ssh://, git@ URLs are supported + +>>> musterr [CLI] bundle init ftps://github.com/databricks/cli +Error: unsupported protocol in Git URL "ftps://github.com/databricks/cli": only https://, ssh://, git@ URLs are supported diff --git a/acceptance/bundle/templates-machinery/unsupported-url/script b/acceptance/bundle/templates-machinery/unsupported-url/script new file mode 100644 index 00000000000..ccb5a3d9c11 --- /dev/null +++ b/acceptance/bundle/templates-machinery/unsupported-url/script @@ -0,0 +1,4 @@ +trace musterr $CLI bundle init http://github.com/databricks/cli +trace musterr $CLI bundle init git://github.com/databricks/cli +trace musterr $CLI bundle init ftp://github.com/databricks/cli +trace musterr $CLI bundle init ftps://github.com/databricks/cli diff --git a/cmd/bundle/debug/render_template_schema.go b/cmd/bundle/debug/render_template_schema.go index 475f73054ac..30743be6f1a 100644 --- a/cmd/bundle/debug/render_template_schema.go +++ b/cmd/bundle/debug/render_template_schema.go @@ -44,7 +44,10 @@ func NewRenderTemplateSchemaCommand() *cobra.Command { } // Resolve the template reader - reader, isGitReader := template.ResolveReader(templatePathOrUrl, templateDir, ref) + reader, isGitReader, err := template.ResolveReader(templatePathOrUrl, templateDir, ref) + if err != nil { + return err + } defer reader.Cleanup(ctx) // For git reader, load schema first to initialize the temp directory diff --git a/libs/template/resolver.go b/libs/template/resolver.go index f7ca1f9c829..0a59a4c47c0 100644 --- a/libs/template/resolver.go +++ b/libs/template/resolver.go @@ -3,40 +3,75 @@ package template import ( "context" "errors" + "fmt" "strings" "github.com/databricks/cli/libs/git" ) -// See https://git-scm.com/docs/git-clone#_git_urls for the set of supported -// Git URL forms. We deliberately exclude deprecated/insecure protocols (git, http, ftp[s]). -var gitUrlPrefixes = []string{ - "https://", - "ssh://", +type gitUrlPrefix struct { + prefix string + + // invalid marks a prefix that git recognizes as a URL but that we refuse to + // clone from, so we can report an actionable error instead of falling through + // to the local-path reader. + invalid bool +} + +// See https://git-scm.com/docs/git-clone#_git_urls for the set of Git URL forms. +// We deliberately reject the deprecated/insecure transports (git, http, ftp[s]). +var gitUrlPrefixes = []gitUrlPrefix{ + {prefix: "https://"}, + {prefix: "ssh://"}, // recognize git@ without ssh:// protocol because this is very common - "git@", + {prefix: "git@"}, + {prefix: "http://", invalid: true}, + {prefix: "git://", invalid: true}, + {prefix: "ftp://", invalid: true}, + {prefix: "ftps://", invalid: true}, } -func IsGitRepoUrl(url string) bool { - for _, prefix := range gitUrlPrefixes { - if strings.HasPrefix(url, prefix) { - return true +// matchGitUrlPrefix returns the matching prefix entry, or nil if the input does +// not look like a Git URL. +func matchGitUrlPrefix(url string) *gitUrlPrefix { + for i := range gitUrlPrefixes { + if strings.HasPrefix(url, gitUrlPrefixes[i].prefix) { + return &gitUrlPrefixes[i] } } - return false + return nil +} + +func IsGitRepoUrl(url string) bool { + p := matchGitUrlPrefix(url) + return p != nil && !p.invalid } // ResolveReader resolves a template path/URL to a Reader (built-in, git or local) -func ResolveReader(templatePathOrUrl, templateDir, ref string) (Reader, bool) { +func ResolveReader(templatePathOrUrl, templateDir, ref string) (Reader, bool, error) { if tmpl := GetDatabricksTemplate(TemplateName(templatePathOrUrl)); tmpl != nil { - return tmpl.Reader, false + return tmpl.Reader, false, nil } - if IsGitRepoUrl(templatePathOrUrl) { - return NewGitReader(templatePathOrUrl, ref, templateDir, git.Clone), true + if p := matchGitUrlPrefix(templatePathOrUrl); p != nil { + if p.invalid { + return nil, false, fmt.Errorf("unsupported protocol in Git URL %q: only %s URLs are supported", templatePathOrUrl, strings.Join(supportedGitUrlPrefixes(), ", ")) + } + return NewGitReader(templatePathOrUrl, ref, templateDir, git.Clone), true, nil } - return NewLocalReader(templatePathOrUrl), false + return NewLocalReader(templatePathOrUrl), false, nil +} + +// supportedGitUrlPrefixes returns the valid (non-rejected) Git URL prefixes. +func supportedGitUrlPrefixes() []string { + var prefixes []string + for i := range gitUrlPrefixes { + if !gitUrlPrefixes[i].invalid { + prefixes = append(prefixes, gitUrlPrefixes[i].prefix) + } + } + return prefixes } type Resolver struct { @@ -107,7 +142,10 @@ func (r Resolver) Resolve(ctx context.Context) (*Template, error) { // // We resolve the appropriate reader according to the reference provided by the user. if tmpl == nil { - reader, _ := ResolveReader(r.TemplatePathOrUrl, r.TemplateDir, ref) + reader, _, err := ResolveReader(r.TemplatePathOrUrl, r.TemplateDir, ref) + if err != nil { + return nil, err + } tmpl = &Template{ name: Custom, Reader: reader, diff --git a/libs/template/resolver_test.go b/libs/template/resolver_test.go index 12de32ab963..b70e8494b9a 100644 --- a/libs/template/resolver_test.go +++ b/libs/template/resolver_test.go @@ -120,22 +120,43 @@ func TestBundleInitIsGitRepoUrl(t *testing.T) { func TestResolveReader(t *testing.T) { t.Run("builtin template", func(t *testing.T) { - reader, isGit := ResolveReader("default-python", "", "") + reader, isGit, err := ResolveReader("default-python", "", "") + require.NoError(t, err) assert.False(t, isGit) assert.Equal(t, &builtinReader{name: "default-python"}, reader) }) - t.Run("git URL", func(t *testing.T) { - reader, isGit := ResolveReader("https://github.com/example/repo", "/template", "v1.0") - assert.True(t, isGit) - gitReader := reader.(*gitReader) - assert.Equal(t, "https://github.com/example/repo", gitReader.gitUrl) - assert.Equal(t, "/template", gitReader.templateDir) - assert.Equal(t, "v1.0", gitReader.ref) - }) + for _, url := range []string{ + "https://github.com/example/repo", + "ssh://git@github.com/example/repo", + "git@github.com:example/repo", + } { + t.Run("git URL "+url, func(t *testing.T) { + reader, isGit, err := ResolveReader(url, "/template", "v1.0") + require.NoError(t, err) + assert.True(t, isGit) + gitReader := reader.(*gitReader) + assert.Equal(t, url, gitReader.gitUrl) + assert.Equal(t, "/template", gitReader.templateDir) + assert.Equal(t, "v1.0", gitReader.ref) + }) + } + + for _, url := range []string{ + "http://github.com/example/repo", + "git://github.com/example/repo", + "ftp://github.com/example/repo", + "ftps://github.com/example/repo", + } { + t.Run("unsupported protocol "+url, func(t *testing.T) { + _, _, err := ResolveReader(url, "", "") + assert.ErrorContains(t, err, "unsupported protocol") + }) + } t.Run("local path", func(t *testing.T) { - reader, isGit := ResolveReader("/local/path", "", "") + reader, isGit, err := ResolveReader("/local/path", "", "") + require.NoError(t, err) assert.False(t, isGit) assert.Equal(t, "/local/path", reader.(*localReader).path) })