1. docker
  2. Commit 5f89a6a78e16b094e66e2878f092bda32459374f
Browse code

pkg/urlutil: deprecate, and move to builder/remotecontext/urlutil

pkg/urlutil (despite its poorly chosen name) is not really intended as a generic
utility to handle URLs, and should only be used by the builder to handle (remote)
build contexts.

- IsURL() only does a very rudimentary check for http(s):// prefixes, without any
other validation, but due to its name may give incorrect expectations.
- IsGitURL() is written specifically with docker build remote git contexts in
mind, and has handling for backward-compatibility, where strings that are
not URLs, but start with "github.com/" are accepted.

Because of the above, this patch:

- moves the package inside builder/remotecontext, close to where it's intended
to be used (ideally this would be part of build/remotecontext itself, but this
package imports many other dependencies, which would introduce those as extra
dependencies in the CLI).
- deprecates pkg/urlutil, but adds aliases as there are some external consumers.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Sebastiaan van Stijn authored on 2022/04/10 23:57:45
Showing 9 changed files
builder/dockerfile/copy.go
History View file @ 5f89a6a
... ... 
@@ -16,6 +16,7 @@ import (
16 16
17 17 
 	"github.com/docker/docker/builder"
18 18 
 	"github.com/docker/docker/builder/remotecontext"
19 
+	"github.com/docker/docker/builder/remotecontext/urlutil"
19 20 
 	"github.com/docker/docker/pkg/archive"
20 21 
 	"github.com/docker/docker/pkg/containerfs"
21 22 
 	"github.com/docker/docker/pkg/idtools"
... ... 
@@ -23,7 +24,6 @@ import (
23 23 
 	"github.com/docker/docker/pkg/progress"
24 24 
 	"github.com/docker/docker/pkg/streamformatter"
25 25 
 	"github.com/docker/docker/pkg/system"
26 
-	"github.com/docker/docker/pkg/urlutil"
27 26 
 	"github.com/moby/buildkit/frontend/dockerfile/instructions"
28 27 
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
29 28 
 	"github.com/pkg/errors"
builder/remotecontext/detect.go
History View file @ 5f89a6a
... ... 
@@ -11,9 +11,9 @@ import (
11 11 
 	"github.com/containerd/continuity/driver"
12 12 
 	"github.com/docker/docker/api/types/backend"
13 13 
 	"github.com/docker/docker/builder"
14 
+	"github.com/docker/docker/builder/remotecontext/urlutil"
14 15 
 	"github.com/docker/docker/errdefs"
15 16 
 	"github.com/docker/docker/pkg/fileutils"
16 
-	"github.com/docker/docker/pkg/urlutil"
17 17 
 	"github.com/moby/buildkit/frontend/dockerfile/dockerignore"
18 18 
 	"github.com/moby/buildkit/frontend/dockerfile/parser"
19 19 
 	"github.com/pkg/errors"
builder/remotecontext/urlutil/urlutil.go
History View file @ 5f89a6a
20 20 
new file mode 100644
... ... 
@@ -0,0 +1,46 @@
0 
+// Package urlutil provides helper function to check urls kind.
1 
+// It supports http and git urls.
2 
+package urlutil // import "github.com/docker/docker/builder/remotecontext/urlutil"
3 
+
4 
+import (
5 
+	"regexp"
6 
+	"strings"
7 
+)
8 
+
9 
+var (
10 
+	validPrefixes = map[string][]string{
11 
+		"url": {"http://", "https://"},
12 
+
13 
+		// The github.com/ prefix is a special case used to treat context-paths
14 
+		// starting with `github.com` as a git URL if the given path does not
15 
+		// exist locally. The "github.com/" prefix is kept for backward compatibility,
16 
+		// and is a legacy feature.
17 
+		//
18 
+		// Going forward, no additional prefixes should be added, and users should
19 
+		// be encouraged to use explicit URLs (https://github.com/user/repo.git) instead.
20 
+		"git": {"git://", "github.com/", "git@"},
21 
+	}
22 
+	urlPathWithFragmentSuffix = regexp.MustCompile(".git(?:#.+)?$")
23 
+)
24 
+
25 
+// IsURL returns true if the provided str is an HTTP(S) URL.
26 
+func IsURL(str string) bool {
27 
+	return checkURL(str, "url")
28 
+}
29 
+
30 
+// IsGitURL returns true if the provided str is a git repository URL.
31 
+func IsGitURL(str string) bool {
32 
+	if IsURL(str) && urlPathWithFragmentSuffix.MatchString(str) {
33 
+		return true
34 
+	}
35 
+	return checkURL(str, "git")
36 
+}
37 
+
38 
+func checkURL(str, kind string) bool {
39 
+	for _, prefix := range validPrefixes[kind] {
40 
+		if strings.HasPrefix(str, prefix) {
41 
+			return true
42 
+		}
43 
+	}
44 
+	return false
45 
+}
builder/remotecontext/urlutil/urlutil_test.go
History View file @ 5f89a6a
0 46 
new file mode 100644
... ... 
@@ -0,0 +1,41 @@
0 
+package urlutil // import "github.com/docker/docker/builder/remotecontext/urlutil"
1 
+
2 
+import "testing"
3 
+
4 
+var (
5 
+	gitUrls = []string{
6 
+		"git://github.com/docker/docker",
7 
+		"git@github.com:docker/docker.git",
8 
+		"git@bitbucket.org:atlassianlabs/atlassian-docker.git",
9 
+		"https://github.com/docker/docker.git",
10 
+		"http://github.com/docker/docker.git",
11 
+		"http://github.com/docker/docker.git#branch",
12 
+		"http://github.com/docker/docker.git#:dir",
13 
+	}
14 
+	incompleteGitUrls = []string{
15 
+		"github.com/docker/docker",
16 
+	}
17 
+	invalidGitUrls = []string{
18 
+		"http://github.com/docker/docker.git:#branch",
19 
+	}
20 
+)
21 
+
22 
+func TestIsGIT(t *testing.T) {
23 
+	for _, url := range gitUrls {
24 
+		if !IsGitURL(url) {
25 
+			t.Fatalf("%q should be detected as valid Git url", url)
26 
+		}
27 
+	}
28 
+
29 
+	for _, url := range incompleteGitUrls {
30 
+		if !IsGitURL(url) {
31 
+			t.Fatalf("%q should be detected as valid Git url", url)
32 
+		}
33 
+	}
34 
+
35 
+	for _, url := range invalidGitUrls {
36 
+		if IsGitURL(url) {
37 
+			t.Fatalf("%q should not be detected as valid Git prefix", url)
38 
+		}
39 
+	}
40 
+}
hack/make.ps1
History View file @ 5f89a6a
... ... 
@@ -251,6 +251,12 @@ Function Validate-PkgImports($headCommit, $upstreamCommit) {
251 251 
     $files=@(); $files = Invoke-Expression "git diff $upstreamCommit...$headCommit --diff-filter=ACMR --name-only -- `'pkg\*.go`'"
252 252 
     $badFiles=@(); $files | ForEach-Object{
253 253 
         $file=$_
254 
+        if ($file -eq "pkg\urlutil\deprecated.go") {
255 
+            # pkg/urlutil is deprecated, but has a temporary alias to help migration,
256 
+            # see https://github.com/moby/moby/pull/43477
257 
+            # TODO(thaJeztah) remove this exception once pkg/urlutil aliases are removed
258 
+            return
259 
+        }
254 260 
         # For the current changed file, get its list of dependencies, sorted and uniqued.
255 261 
         $imports = Invoke-Expression "go list -e -f `'{{ .Deps }}`' $file"
256 262 
         if ($LASTEXITCODE -ne 0) { Throw "Failed go list for dependencies on $file" }
hack/validate/pkg-imports
History View file @ 5f89a6a
... ... 
@@ -10,6 +10,12 @@ unset IFS
10 10
11 11 
 badFiles=()
12 12 
 for f in "${files[@]}"; do
13 
+	if [ "$f" = "pkg/urlutil/deprecated.go" ]; then
14 
+		# pkg/urlutil is deprecated, but has a temporary alias to help migration,
15 
+		# see https://github.com/moby/moby/pull/43477
16 
+		# TODO(thaJeztah) remove this exception once pkg/urlutil aliases are removed
17 
+		continue
18 
+	fi
13 19 
 	IFS=$'\n'
14 20 
 	badImports=($(go list -e -f '{{ join .Deps "\n" }}' "$f" | sort -u | grep -vE '^github.com/docker/docker/pkg/' | grep -vE '^github.com/docker/docker/vendor' | grep -E '^github.com/docker/docker' || true))
15 21 
 	unset IFS
pkg/urlutil/deprecated.go
History View file @ 5f89a6a
16 22 
new file mode 100644
... ... 
@@ -0,0 +1,21 @@
0 
+package urlutil // import "github.com/docker/docker/pkg/urlutil"
1 
+
2 
+import "github.com/docker/docker/builder/remotecontext/urlutil"
3 
+
4 
+// IsURL returns true if the provided str is an HTTP(S) URL.
5 
+//
6 
+// Deprecated: use github.com/docker/docker/builder/remotecontext/urlutil.IsURL
7 
+// to detect build-context type, or use strings.HasPrefix() to check if the
8 
+// string has a https:// or http:// prefix.
9 
+func IsURL(str string) bool {
10 
+	// TODO(thaJeztah) when removing this alias, remove the exception from hack/validate/pkg-imports and hack/make.ps1 (Validate-PkgImports)
11 
+	return urlutil.IsURL(str)
12 
+}
13 
+
14 
+// IsGitURL returns true if the provided str is a git repository URL.
15 
+//
16 
+// Deprecated: use github.com/docker/docker/builder/remotecontext/urlutil.IsGitURL
17 
+func IsGitURL(str string) bool {
18 
+	// TODO(thaJeztah) when removing this alias, remove the exception from hack/validate/pkg-imports and hack/make.ps1 (Validate-PkgImports)
19 
+	return urlutil.IsGitURL(str)
20 
+}
pkg/urlutil/urlutil.go
History View file @ 5f89a6a
0 21 
deleted file mode 100644
... ... 
@@ -1,46 +0,0 @@
1 
-// Package urlutil provides helper function to check urls kind.
2 
-// It supports http and git urls.
3 
-package urlutil // import "github.com/docker/docker/pkg/urlutil"
4 
-
5 
-import (
6 
-	"regexp"
7 
-	"strings"
8 
-)
9 
-
10 
-var (
11 
-	validPrefixes = map[string][]string{
12 
-		"url": {"http://", "https://"},
13 
-
14 
-		// The github.com/ prefix is a special case used to treat context-paths
15 
-		// starting with `github.com` as a git URL if the given path does not
16 
-		// exist locally. The "github.com/" prefix is kept for backward compatibility,
17 
-		// and is a legacy feature.
18 
-		//
19 
-		// Going forward, no additional prefixes should be added, and users should
20 
-		// be encouraged to use explicit URLs (https://github.com/user/repo.git) instead.
21 
-		"git": {"git://", "github.com/", "git@"},
22 
-	}
23 
-	urlPathWithFragmentSuffix = regexp.MustCompile(".git(?:#.+)?$")
24 
-)
25 
-
26 
-// IsURL returns true if the provided str is an HTTP(S) URL.
27 
-func IsURL(str string) bool {
28 
-	return checkURL(str, "url")
29 
-}
30 
-
31 
-// IsGitURL returns true if the provided str is a git repository URL.
32 
-func IsGitURL(str string) bool {
33 
-	if IsURL(str) && urlPathWithFragmentSuffix.MatchString(str) {
34 
-		return true
35 
-	}
36 
-	return checkURL(str, "git")
37 
-}
38 
-
39 
-func checkURL(str, kind string) bool {
40 
-	for _, prefix := range validPrefixes[kind] {
41 
-		if strings.HasPrefix(str, prefix) {
42 
-			return true
43 
-		}
44 
-	}
45 
-	return false
46 
-}
pkg/urlutil/urlutil_test.go
History View file @ 5f89a6a
47 1 
deleted file mode 100644
... ... 
@@ -1,41 +0,0 @@
1 
-package urlutil // import "github.com/docker/docker/pkg/urlutil"
2 
-
3 
-import "testing"
4 
-
5 
-var (
6 
-	gitUrls = []string{
7 
-		"git://github.com/docker/docker",
8 
-		"git@github.com:docker/docker.git",
9 
-		"git@bitbucket.org:atlassianlabs/atlassian-docker.git",
10 
-		"https://github.com/docker/docker.git",
11 
-		"http://github.com/docker/docker.git",
12 
-		"http://github.com/docker/docker.git#branch",
13 
-		"http://github.com/docker/docker.git#:dir",
14 
-	}
15 
-	incompleteGitUrls = []string{
16 
-		"github.com/docker/docker",
17 
-	}
18 
-	invalidGitUrls = []string{
19 
-		"http://github.com/docker/docker.git:#branch",
20 
-	}
21 
-)
22 
-
23 
-func TestIsGIT(t *testing.T) {
24 
-	for _, url := range gitUrls {
25 
-		if !IsGitURL(url) {
26 
-			t.Fatalf("%q should be detected as valid Git url", url)
27 
-		}
28 
-	}
29 
-
30 
-	for _, url := range incompleteGitUrls {
31 
-		if !IsGitURL(url) {
32 
-			t.Fatalf("%q should be detected as valid Git url", url)
33 
-		}
34 
-	}
35 
-
36 
-	for _, url := range invalidGitUrls {
37 
-		if IsGitURL(url) {
38 
-			t.Fatalf("%q should not be detected as valid Git prefix", url)
39 
-		}
40 
-	}
41 
-}