From ebe76f1f055fd64829e99cb33543745a7d1759c0 Mon Sep 17 00:00:00 2001 From: Anna Song Date: Thu, 25 Aug 2022 17:52:39 -0700 Subject: [PATCH] Revert remote url-related changes in #4652 Revert due to code freeze issue #4756 --- api/internal/git/repospec.go | 98 +++++++++++++++---------------- api/internal/git/repospec_test.go | 42 ++++++------- api/loader/fileloader.go | 40 ++++++------- 3 files changed, 84 insertions(+), 96 deletions(-) diff --git a/api/internal/git/repospec.go b/api/internal/git/repospec.go index 017c66eb14..37f1d615e9 100644 --- a/api/internal/git/repospec.go +++ b/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 { diff --git a/api/internal/git/repospec_test.go b/api/internal/git/repospec_test.go index 5fd72b9918..41e89ca31e 100644 --- a/api/internal/git/repospec_test.go +++ b/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(), diff --git a/api/loader/fileloader.go b/api/loader/fileloader.go index df7def1e3b..fb57c588d4 100644 --- a/api/loader/fileloader.go +++ b/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)