Revert remote url-related changes in kubernetes-sigs#4652

Revert due to code freeze issue kubernetes-sigs#4756

api/internal/git/repospec.go

@@ -11,7 +11,6 @@ import (
 	"strings"
 	"time"
 
-	"sigs.k8s.io/kustomize/kyaml/errors"
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 )
 
@@ -92,14 +91,17 @@ func NewRepoSpecFromURL(n string) (*RepoSpec, error) {
 	if filepath.IsAbs(n) {
 		return nil, fmt.Errorf("uri looks like abs path: %s", n)
 	}
-	repoSpec := parseGitURL(n)
-	if repoSpec.Host == "" {
-		return nil, errors.Errorf("url lacks host: %s", n)
+	host, orgRepo, path, gitRef, gitSubmodules, suffix, gitTimeout := parseGitURL(n)
+	if orgRepo == "" {
+		return nil, fmt.Errorf("url lacks orgRepo: %s", n)
 	}
-	if repoSpec.OrgRepo == "" {
-		return nil, errors.Errorf("url lacks orgRepo: %s", n)
+	if host == "" {
+		return nil, fmt.Errorf("url lacks host: %s", n)
 	}
-	return repoSpec, nil
+	return &RepoSpec{
+		raw: n, Host: host, OrgRepo: orgRepo,
+		Dir: notCloned, Path: path, Ref: gitRef, GitSuffix: suffix,
+		Submodules: gitSubmodules, Timeout: gitTimeout}, nil
 }
 
 const (
@@ -111,47 +113,44 @@ 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) *RepoSpec {
-	repoSpec := &RepoSpec{raw: n, Dir: notCloned}
-
+func parseGitURL(n string) (
+	host string, orgRepo string, path string, gitRef string, gitSubmodules bool, gitSuff string, gitTimeout time.Duration) {
 	if strings.Contains(n, gitDelimiter) {
 		index := strings.Index(n, gitDelimiter)
 		// Adding _git/ to host
-		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 = 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, n = parseHostSpec(n)
-
-	repoSpec.GitSuffix = gitSuffix
+	host, n = parseHostSpec(n)
+	gitSuff = gitSuffix
 	if strings.Contains(n, gitSuffix) {
 		index := strings.Index(n, gitSuffix)
-		repoSpec.OrgRepo = n[0:index]
+		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:]
 		}
-	} 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
+		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 {
 		j += i + 1
-		repoSpec.OrgRepo = n[:j]
-		n = n[j+1:]
+		orgRepo = n[:j]
+		path, gitRef, gitTimeout, gitSubmodules = peelQuery(n[j+1:])
+		return
 	}
-	repoSpec.Path, repoSpec.Ref, repoSpec.Timeout, repoSpec.Submodules = peelQuery(n)
-	return repoSpec
+	path = ""
+	orgRepo, gitRef, gitTimeout, gitSubmodules = peelQuery(n)
+	return host, orgRepo, path, gitRef, gitSubmodules, gitSuff, gitTimeout
 }
 
 // Clone git submodules by default.
@@ -203,7 +202,6 @@ 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.
@@ -214,29 +212,25 @@ func parseHostSpec(n string) (string, string) {
 			host += p
 		}
 	}
-	// ssh relative path
 	if host == "git@" {
-		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
-			}
+		i := strings.Index(n, "/")
+		if i > -1 {
 			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
 	}
 
+	// TODO(annasong): replace this array with Schemes
 	// If host is a http(s) or ssh URL, grab the domain part.
-	for _, p := range Schemes() {
+	for _, p := range []string{
+		"ssh://", "https://", "http://"} {
 		if strings.HasSuffix(host, p) {
 			i := strings.Index(n, "/")
 			if i > -1 {

api/internal/git/repospec_test.go

@@ -19,25 +19,22 @@ var pathNames = []string{"README.md", "foo/krusty.txt", ""}
 
 var hrefArgs = []string{"someBranch", "master", "v0.1.0", ""}
 
-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:"},
-	}
+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 makeURL(hostFmt, orgRepo, path, href string) string {
@@ -53,7 +50,7 @@ func makeURL(hostFmt, orgRepo, path, href string) string {
 
 func TestNewRepoSpecFromUrl(t *testing.T) {
 	var bad [][]string
-	for _, tuple := range hostsRawNormalizedAndDomain() {
+	for _, tuple := range hostNamesRawAndNormalized {
 		hostRaw := tuple[0]
 		hostSpec := tuple[1]
 		for _, orgRepo := range orgRepos {
@@ -94,10 +91,10 @@ func TestNewRepoSpecFromUrl(t *testing.T) {
 
 var badData = [][]string{
 	{"/tmp", "uri looks like abs path"},
-	{"iauhsdiuashduas", "url lacks host"},
+	{"iauhsdiuashduas", "url lacks orgRepo"},
 	{"htxxxtp://github.com/", "url lacks host"},
 	{"ssh://git.example.com", "url lacks orgRepo"},
-	{"git::___", "url lacks host"},
+	{"git::___", "url lacks orgRepo"},
 }
 
 func TestNewRepoSpecFromUrlErrors(t *testing.T) {
@@ -150,7 +147,6 @@ 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(),

api/loader/fileloader.go

@@ -295,37 +295,35 @@ func (fl *fileLoader) errIfRepoCycle(newRepoSpec *git.RepoSpec) error {
 	return fl.referrer.errIfRepoCycle(newRepoSpec)
 }
 
-func loadURL(hc *http.Client, path string) ([]byte, error) {
-	resp, err := hc.Get(path)
-	if err != nil {
-		return nil, errors.WrapPrefixf(err, "cannot GET url")
-	}
-	defer resp.Body.Close()
-	if resp.StatusCode < 200 || resp.StatusCode > 299 {
-		if _, err = git.NewRepoSpecFromURL(path); err == nil {
-			return nil, errors.Errorf("URL is a git repository")
-		}
-		return nil, fmt.Errorf("%w: status code %d (%s)", ErrHTTP, resp.StatusCode, http.StatusText(resp.StatusCode))
-	}
-	body, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return nil, errors.WrapPrefixf(err, "cannot read url content")
-	}
-	return body, nil
-}
-
 // Load returns the content of file at the given path,
 // else an error. Relative paths are taken relative
 // to the root.
 func (fl *fileLoader) Load(path string) ([]byte, error) {
-	if HasRemoteFileScheme(path) {
+	// TODO(annasong): replace this remote file check with HasRemoteFileScheme()
+	if u, err := url.Parse(path); err == nil && (u.Scheme == "http" || u.Scheme == "https") {
 		var hc *http.Client
 		if fl.http != nil {
 			hc = fl.http
 		} else {
 			hc = &http.Client{}
 		}
-		return loadURL(hc, path)
+		resp, err := hc.Get(path)
+		if err != nil {
+			return nil, err
+		}
+		defer resp.Body.Close()
+		if resp.StatusCode < 200 || resp.StatusCode > 299 {
+			_, err := git.NewRepoSpecFromURL(path)
+			if err == nil {
+				return nil, errors.Errorf("URL is a git repository")
+			}
+			return nil, fmt.Errorf("%w: status code %d (%s)", ErrHTTP, resp.StatusCode, http.StatusText(resp.StatusCode))
+		}
+		body, err := io.ReadAll(resp.Body)
+		if err != nil {
+			return nil, err
+		}
+		return body, nil
 	}
 	if !filepath.IsAbs(path) {
 		path = fl.root.Join(path)