Load and calculate Destination of files for kustomize localize (#4652)

* Implement file localization for localize Implement file localization for new command kustomize localize * Patch file localization Fix lint errors, address feedback, begin transition to file loader from localizer * Add Repo() to Loader * Implement locLoader factory + cleanup * Add domain to RepoSpec Fix ssh relative url host parsing for non-github domain on the side * Implement Load(), New(), Root(), Dst() for LocLoader * Address repospec code review comments * Address 1st round of code review feedback * Address feedback #2 Removed localized path calculations to cover edge case and improve readability. * Remove ldr Cleanup logging * Address code review round #3 * Address code review feedback #4
kubernetes-sigs · Aug 19, 2022 · bf17fe1 · bf17fe1
1 parent a8010ac
commit bf17fe1
Show file tree

Hide file tree

Showing 27 changed files with 792 additions and 100 deletions.
diff --git a/api/go.mod b/api/go.mod
@@ -8,7 +8,7 @@ require (
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
 	github.com/imdario/mergo v0.3.6
 	github.com/pkg/errors v0.9.1
-	github.com/stretchr/testify v1.7.0
+	github.com/stretchr/testify v1.8.0
 	gopkg.in/yaml.v2 v2.4.0
 	k8s.io/kube-openapi v0.0.0-20220401212409-b28bf2818661
 	sigs.k8s.io/kustomize/kyaml v0.13.9
@@ -34,5 +34,5 @@ require (
 	golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e // indirect
 	golang.org/x/text v0.3.7 // indirect
 	google.golang.org/protobuf v1.28.0 // indirect
-	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
diff --git a/api/go.sum b/api/go.sum
@@ -99,12 +99,15 @@ github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTd
 github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
 github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/objx v0.2.0 h1:Hbg2NidpLE8veEBkEZTL3CvlkUIVzuU9jDplZO54c48=
+github.com/stretchr/objx v0.4.0 h1:M2gUjqZET1qApGOWNSnZ49BAIMX4F/1plDv3+l31EJ4=
+github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
-github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk=
+github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/xlab/treeprint v1.1.0 h1:G/1DjNkPpfZCFt9CSh6b5/nY4VimlbHF3Rh4obvtzDk=
 github.com/xlab/treeprint v1.1.0/go.mod h1:gj5Gd3gPdKtR1ikdDK6fnFLdmIS0X30kTTuNd/WEJu0=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
@@ -199,8 +202,9 @@ gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
 gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
-gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b h1:h8qDotaEPuJATrMmW04NCwg7v22aHH28wwpauUhK9Oo=
 gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
+gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
+gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 k8s.io/gengo v0.0.0-20210813121822-485abfe95c7c/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E=

diff --git a/api/ifc/ifc.go b/api/ifc/ifc.go
@@ -28,12 +28,20 @@ type KvLoader interface {
 
 // Loader interface exposes methods to read bytes.
 type Loader interface {
+
+	// Repo returns the repo location and true if this Loader
+	// was created from a url; otherwise the empty string and false.
+	Repo() (string, bool)
+
 	// Root returns the root location for this Loader.
 	Root() string
+
 	// New returns Loader located at newRoot.
 	New(newRoot string) (Loader, error)
+
 	// Load returns the bytes read from the location or an error.
 	Load(location string) ([]byte, error)
+
 	// Cleanup cleans the loader
 	Cleanup() error
 }

diff --git a/api/internal/git/repospec.go b/api/internal/git/repospec.go
@@ -11,6 +11,7 @@ import (
 	"strings"
 	"time"
 
+	"sigs.k8s.io/kustomize/kyaml/errors"
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 )
 
@@ -21,13 +22,18 @@ import (
 // with a unique name that isn't created until clone time.
 const notCloned = filesys.ConfirmedDir("/notCloned")
 
+// Schemes returns the schemes that repo urls may have
+func Schemes() []string {
+	return []string{"https://", "http://", "ssh://"}
+}
+
 // RepoSpec specifies a git repository and a branch and path therein.
 type RepoSpec struct {
 	// Raw, original spec, used to look for cycles.
 	// TODO(monopole): Drop raw, use processed fields instead.
 	raw string
 
-	// Host, e.g. github.com
+	// Host, e.g. https://github.com/
 	Host string
 
 	// orgRepo name (organization/repoName),
@@ -86,17 +92,14 @@ func NewRepoSpecFromURL(n string) (*RepoSpec, error) {
 	if filepath.IsAbs(n) {
 		return nil, fmt.Errorf("uri looks like abs path: %s", n)
 	}
-	host, orgRepo, path, gitRef, gitSubmodules, suffix, gitTimeout := parseGitURL(n)
-	if orgRepo == "" {
-		return nil, fmt.Errorf("url lacks orgRepo: %s", n)
+	repoSpec := parseGitURL(n)
+	if repoSpec.Host == "" {
+		return nil, errors.Errorf("url lacks host: %s", n)
 	}
-	if host == "" {
-		return nil, fmt.Errorf("url lacks host: %s", n)
+	if repoSpec.OrgRepo == "" {
+		return nil, errors.Errorf("url lacks orgRepo: %s", n)
 	}
-	return &RepoSpec{
-		raw: n, Host: host, OrgRepo: orgRepo,
-		Dir: notCloned, Path: path, Ref: gitRef, GitSuffix: suffix,
-		Submodules: gitSubmodules, Timeout: gitTimeout}, nil
+	return repoSpec, nil
 }
 
 const (
@@ -108,44 +111,47 @@ const (
 // From strings like git@github.com:someOrg/someRepo.git or
 // https://github.com/someOrg/someRepo?ref=someHash, extract
 // the parts.
-func parseGitURL(n string) (
-	host string, orgRepo string, path string, gitRef string, gitSubmodules bool, gitSuff string, gitTimeout time.Duration) {
+func parseGitURL(n string) *RepoSpec {
+	repoSpec := &RepoSpec{raw: n, Dir: notCloned}
+
 	if strings.Contains(n, gitDelimiter) {
 		index := strings.Index(n, gitDelimiter)
 		// Adding _git/ to host
-		host = normalizeGitHostSpec(n[:index+len(gitDelimiter)])
-		orgRepo = strings.Split(strings.Split(n[index+len(gitDelimiter):], "/")[0], "?")[0]
-		path, gitRef, gitTimeout, gitSubmodules = peelQuery(n[index+len(gitDelimiter)+len(orgRepo):])
-		return
+		repoSpec.Host = normalizeGitHostSpec(n[:index+len(gitDelimiter)])
+		// url before next /, though make sure before ? query string delimiter
+		repoSpec.OrgRepo = strings.Split(strings.Split(n[index+len(gitDelimiter):], "/")[0], "?")[0]
+		repoSpec.Path, repoSpec.Ref, repoSpec.Timeout, repoSpec.Submodules =
+			peelQuery(n[index+len(gitDelimiter)+len(repoSpec.OrgRepo):])
+		return repoSpec
 	}
-	host, n = parseHostSpec(n)
-	gitSuff = gitSuffix
+
+	repoSpec.Host, n = parseHostSpec(n)
+
+	repoSpec.GitSuffix = gitSuffix
 	if strings.Contains(n, gitSuffix) {
 		index := strings.Index(n, gitSuffix)
-		orgRepo = n[0:index]
+		repoSpec.OrgRepo = n[0:index]
 		n = n[index+len(gitSuffix):]
+		// always try to include / in previous component
 		if len(n) > 0 && n[0] == '/' {
 			n = n[1:]
 		}
-		path, gitRef, gitTimeout, gitSubmodules = peelQuery(n)
-		return
-	}
-
-	i := strings.Index(n, "/")
-	if i < 1 {
-		path, gitRef, gitTimeout, gitSubmodules = peelQuery(n)
-		return
-	}
-	j := strings.Index(n[i+1:], "/")
-	if j >= 0 {
+	} else if strings.Contains(n, "/") {
+		// there exist at least 2 components that we can interpret as org + repo
+		i := strings.Index(n, "/")
+		j := strings.Index(n[i+1:], "/")
+		// only 2 components, so can use peelQuery to group everything before ? query string delimiter as OrgRepo
+		if j == -1 {
+			repoSpec.OrgRepo, repoSpec.Ref, repoSpec.Timeout, repoSpec.Submodules = peelQuery(n)
+			return repoSpec
+		}
+		// extract first 2 components
 		j += i + 1
-		orgRepo = n[:j]
-		path, gitRef, gitTimeout, gitSubmodules = peelQuery(n[j+1:])
-		return
+		repoSpec.OrgRepo = n[:j]
+		n = n[j+1:]
 	}
-	path = ""
-	orgRepo, gitRef, gitTimeout, gitSubmodules = peelQuery(n)
-	return host, orgRepo, path, gitRef, gitSubmodules, gitSuff, gitTimeout
+	repoSpec.Path, repoSpec.Ref, repoSpec.Timeout, repoSpec.Submodules = peelQuery(n)
+	return repoSpec
 }
 
 // Clone git submodules by default.
@@ -197,6 +203,7 @@ func peelQuery(arg string) (string, string, time.Duration, bool) {
 
 func parseHostSpec(n string) (string, string) {
 	var host string
+	// TODO(annasong): handle ports, non-github ssh with userinfo, github.com sub-domains
 	// Start accumulating the host part.
 	for _, p := range []string{
 		// Order matters here.
@@ -207,24 +214,29 @@ func parseHostSpec(n string) (string, string) {
 			host += p
 		}
 	}
+	// ssh relative path
 	if host == "git@" {
-		i := strings.Index(n, "/")
-		if i > -1 {
+		var i int
+		// git orgRepo delimiter for ssh relative path
+		j := strings.Index(n, ":")
+		// orgRepo delimiter only valid if not preceded by /
+		k := strings.Index(n, "/")
+		// orgRepo delimiter exists, so extend host
+		if j > -1 && (k == -1 || k > j) {
+			i = j
+			// should try to group / with previous component
+			// this is possible if git url username expansion follows
+			if k == i+1 {
+				i = k
+			}
 			host += n[:i+1]
 			n = n[i+1:]
-		} else {
-			i = strings.Index(n, ":")
-			if i > -1 {
-				host += n[:i+1]
-				n = n[i+1:]
-			}
 		}
 		return host, n
 	}
 
 	// If host is a http(s) or ssh URL, grab the domain part.
-	for _, p := range []string{
-		"ssh://", "https://", "http://"} {
+	for _, p := range Schemes() {
 		if strings.HasSuffix(host, p) {
 			i := strings.Index(n, "/")
 			if i > -1 {

diff --git a/api/internal/git/repospec_test.go b/api/internal/git/repospec_test.go
@@ -19,22 +19,25 @@ var pathNames = []string{"README.md", "foo/krusty.txt", ""}
 
 var hrefArgs = []string{"someBranch", "master", "v0.1.0", ""}
 
-var hostNamesRawAndNormalized = [][]string{
-	{"gh:", "gh:"},
-	{"GH:", "gh:"},
-	{"github.com/", "https://github.com/"},
-	{"github.com:", "https://github.com/"},
-	{"http://github.com/", "https://github.com/"},
-	{"https://github.com/", "https://github.com/"},
-	{"hTTps://github.com/", "https://github.com/"},
-	{"https://git-codecommit.us-east-2.amazonaws.com/", "https://git-codecommit.us-east-2.amazonaws.com/"},
-	{"https://fabrikops2.visualstudio.com/", "https://fabrikops2.visualstudio.com/"},
-	{"ssh://git.example.com:7999/", "ssh://git.example.com:7999/"},
-	{"git::https://gitlab.com/", "https://gitlab.com/"},
-	{"git::http://git.example.com/", "http://git.example.com/"},
-	{"git::https://git.example.com/", "https://git.example.com/"},
-	{"git@github.com:", "git@github.com:"},
-	{"git@github.com/", "git@github.com:"},
+func hostsRawNormalizedAndDomain() [][]string {
+	return [][]string{
+		{"gh:", "gh:"},
+		{"GH:", "gh:"},
+		{"github.com/", "https://github.com/"},
+		{"github.com:", "https://github.com/"},
+		{"http://github.com/", "https://github.com/"},
+		{"https://github.com/", "https://github.com/"},
+		{"hTTps://github.com/", "https://github.com/"},
+		{"https://git-codecommit.us-east-2.amazonaws.com/", "https://git-codecommit.us-east-2.amazonaws.com/"},
+		{"https://fabrikops2.visualstudio.com/", "https://fabrikops2.visualstudio.com/"},
+		{"ssh://git.example.com:7999/", "ssh://git.example.com:7999/"},
+		{"git::https://gitlab.com/", "https://gitlab.com/"},
+		{"git::http://git.example.com/", "http://git.example.com/"},
+		{"git::https://git.example.com/", "https://git.example.com/"},
+		{"git@github.com:", "git@github.com:"},
+		{"git@github.com/", "git@github.com:"},
+		{"git@gitlab.com:", "git@gitlab.com:"},
+	}
 }
 
 func makeURL(hostFmt, orgRepo, path, href string) string {
@@ -50,7 +53,7 @@ func makeURL(hostFmt, orgRepo, path, href string) string {
 
 func TestNewRepoSpecFromUrl(t *testing.T) {
 	var bad [][]string
-	for _, tuple := range hostNamesRawAndNormalized {
+	for _, tuple := range hostsRawNormalizedAndDomain() {
 		hostRaw := tuple[0]
 		hostSpec := tuple[1]
 		for _, orgRepo := range orgRepos {
@@ -91,10 +94,10 @@ func TestNewRepoSpecFromUrl(t *testing.T) {
 
 var badData = [][]string{
 	{"/tmp", "uri looks like abs path"},
-	{"iauhsdiuashduas", "url lacks orgRepo"},
+	{"iauhsdiuashduas", "url lacks host"},
 	{"htxxxtp://github.com/", "url lacks host"},
 	{"ssh://git.example.com", "url lacks orgRepo"},
-	{"git::___", "url lacks orgRepo"},
+	{"git::___", "url lacks host"},
 }
 
 func TestNewRepoSpecFromUrlErrors(t *testing.T) {
@@ -147,6 +150,7 @@ func TestNewRepoSpecFromUrl_CloneSpecs(t *testing.T) {
 			ref:       "",
 		},
 		"t6": {
+			// TODO(annasong): replace illegal test case with legal variations
 			input:     "git@gitlab2.sqtools.ru:10022/infra/kubernetes/thanos-base.git?ref=v0.1.0",
 			cloneSpec: "git@gitlab2.sqtools.ru:10022/infra/kubernetes/thanos-base.git",
 			absPath:   notCloned.String(),

diff --git a/api/internal/localizer/doc.go b/api/internal/localizer/doc.go
@@ -0,0 +1,7 @@
+// Copyright 2022 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+// This package contains utilities for the command kustomize localize, which is
+// documented under proposals/localize-command or at
+// https://github.com/kubernetes-sigs/kustomize/blob/master/proposals/22-04-localize-command.md
+package localizer