Skip to content

Commit

Permalink
Rewrite remoteload_test integration tests (#4783)
Browse files Browse the repository at this point in the history 
* Better error message when git clone fails

* support file:// URLs

* rewrite remoteload_test

* lint and test fix

* fixes for kverey's comments

* document file:// remote load

* replace assert with require where appropriate

* add tests for file:// without git suffix

* fixes plus pr review from natasha

* fixes for review, lint

* revert changes to error handling

* fix skipped test
  • Loading branch information
@mightyguava
mightyguava authored Sep 19, 2022
1 parent d6e40a3 commit e62480d
Show file tree
Hide file tree
Showing 16 changed files with 633 additions and 450 deletions.
6 changes: 1 addition & 5 deletions api/internal/git/cloner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,11 @@ func ClonerUsingGitExec(repoSpec *RepoSpec) error {
if err = r.run("init"); err != nil {
return err
}
if err = r.run(
"remote", "add", "origin", repoSpec.CloneSpec()); err != nil {
return err
}
ref := "HEAD"
if repoSpec.Ref != "" {
ref = repoSpec.Ref
}
if err = r.run("fetch", "--depth=1", "origin", ref); err != nil {
if err = r.run("fetch", "--depth=1", repoSpec.CloneSpec(), ref); err != nil {
return err
}
if err = r.run("checkout", "FETCH_HEAD"); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions api/internal/git/gitrunner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ func (r gitRunner) run(args ...string) error {
cmd.String(),
r.duration,
func() error {
_, err := cmd.CombinedOutput()
out, err := cmd.CombinedOutput()
if err != nil {
return errors.Wrapf(err, "git cmd = '%s'", cmd.String())
return errors.Wrapf(err, "failed to run '%s': %s", cmd.String(), string(out))
}
return err
})
Expand Down
21 changes: 19 additions & 2 deletions api/internal/git/repospec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,12 @@ func parseGitURL(n string) (
return
}
host, n = parseHostSpec(n)
gitSuff = gitSuffix
isLocal := strings.HasPrefix(host, "file://")
if !isLocal {
gitSuff = gitSuffix
}
if strings.Contains(n, gitSuffix) {
gitSuff = gitSuffix
index := strings.Index(n, gitSuffix)
orgRepo = n[0:index]
n = n[index+len(gitSuffix):]
Expand All @@ -131,6 +135,19 @@ func parseGitURL(n string) (
return
}

if isLocal {
if idx := strings.Index(n, "//"); idx > 0 {
orgRepo = n[:idx]
n = n[idx+2:]
path, gitRef, gitTimeout, gitSubmodules = peelQuery(n)
return
}
path, gitRef, gitTimeout, gitSubmodules = peelQuery(n)
orgRepo = path
path = ""
return
}

i := strings.Index(n, "/")
if i < 1 {
path, gitRef, gitTimeout, gitSubmodules = peelQuery(n)
Expand Down Expand Up @@ -200,7 +217,7 @@ func parseHostSpec(n string) (string, string) {
// Start accumulating the host part.
for _, p := range []string{
// Order matters here.
"git::", "gh:", "ssh://", "https://", "http://",
"git::", "gh:", "ssh://", "https://", "http://", "file://",
"git@", "github.com:", "github.com/"} {
if len(p) < len(n) && strings.ToLower(n[:len(p)]) == p {
n = n[len(p):]
Expand Down
128 changes: 127 additions & 1 deletion api/internal/git/repospec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestNewRepoSpecFromUrl_Permute(t *testing.T) {
Expand Down Expand Up @@ -61,6 +62,7 @@ func TestNewRepoSpecFromUrl_Permute(t *testing.T) {
rs, err := NewRepoSpecFromURL(uri)
if err != nil {
t.Errorf("problem %v", err)
continue
}
if rs.Host != hostSpec {
bad = append(bad, []string{"host", uri, rs.Host, hostSpec})
Expand Down Expand Up @@ -120,6 +122,7 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
repoSpec RepoSpec
cloneSpec string
absPath string
skip string
}{
{
name: "t1",
Expand Down Expand Up @@ -285,11 +288,134 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
GitSuffix: ".git",
},
},
{
name: "t15",
input: "https://github.com/kubernetes-sigs/kustomize//examples/multibases/dev/?ref=v1.0.6",
cloneSpec: "https://github.com/kubernetes-sigs/kustomize.git",
absPath: notCloned.Join("/examples/multibases/dev"),
repoSpec: RepoSpec{
Host: "https://github.com/",
OrgRepo: "kubernetes-sigs/kustomize",
Path: "/examples/multibases/dev/",
Ref: "v1.0.6",
GitSuffix: ".git",
},
},
{
name: "t16",
input: "file://a/b/c/someRepo.git/somepath?ref=someBranch",
cloneSpec: "file://a/b/c/someRepo.git",
absPath: notCloned.Join("somepath"),
repoSpec: RepoSpec{
Host: "file://",
OrgRepo: "a/b/c/someRepo",
Path: "somepath",
Ref: "someBranch",
GitSuffix: ".git",
},
},
{
name: "t17",
input: "file://a/b/c/someRepo//somepath?ref=someBranch",
cloneSpec: "file://a/b/c/someRepo",
absPath: notCloned.Join("somepath"),
repoSpec: RepoSpec{
Host: "file://",
OrgRepo: "a/b/c/someRepo",
Path: "somepath",
Ref: "someBranch",
},
},
{
name: "t18",
input: "file://a/b/c/someRepo?ref=someBranch",
cloneSpec: "file://a/b/c/someRepo",
absPath: notCloned.String(),
repoSpec: RepoSpec{
Host: "file://",
OrgRepo: "a/b/c/someRepo",
Ref: "someBranch",
},
},
{
name: "t19",
input: "file:///a/b/c/someRepo?ref=someBranch",
cloneSpec: "file:///a/b/c/someRepo",
absPath: notCloned.String(),
repoSpec: RepoSpec{
Host: "file://",
OrgRepo: "/a/b/c/someRepo",
Ref: "someBranch",
},
},
{
name: "t20",
input: "ssh://[email protected]/kubernetes-sigs/kustomize//examples/multibases/dev?ref=v1.0.6",
cloneSpec: "[email protected]:kubernetes-sigs/kustomize.git",
absPath: notCloned.Join("examples/multibases/dev"),
repoSpec: RepoSpec{
Host: "[email protected]:",
OrgRepo: "kubernetes-sigs/kustomize",
Path: "/examples/multibases/dev",
Ref: "v1.0.6",
GitSuffix: ".git",
},
},
{
name: "t21",
input: "file:///a/b/c/someRepo",
cloneSpec: "file:///a/b/c/someRepo",
absPath: notCloned.String(),
repoSpec: RepoSpec{
Host: "file://",
OrgRepo: "/a/b/c/someRepo",
},
},
{
name: "t22",
input: "file:///",
cloneSpec: "file:///",
absPath: notCloned.String(),
repoSpec: RepoSpec{
Host: "file://",
OrgRepo: "/",
},
},
{
name: "t23",
skip: "the `//` repo separator does not work",
input: "https://fake-git-hosting.org/path/to/repo//examples/multibases/dev",
cloneSpec: "https://fake-git-hosting.org/path/to.git",
absPath: notCloned.Join("/examples/multibases/dev"),
repoSpec: RepoSpec{
Host: "https://fake-git-hosting.org/",
OrgRepo: "path/to/repo",
Path: "/examples/multibases/dev",
GitSuffix: ".git",
},
},
{
name: "t24",
skip: "the `//` repo separator does not work",
input: "ssh://[email protected]/path/to/repo//examples/multibases/dev",
cloneSpec: "ssh://[email protected]/path/to/repo.git",
absPath: notCloned.Join("/examples/multibases/dev"),
repoSpec: RepoSpec{
Host: "ssh://[email protected]",
OrgRepo: "path/to/repo",
Path: "/examples/multibases/dev",
GitSuffix: ".git",
},
},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
if tc.skip != "" {
t.Skip(tc.skip)
}

rs, err := NewRepoSpecFromURL(tc.input)
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, tc.cloneSpec, rs.CloneSpec(), "cloneSpec mismatch")
assert.Equal(t, tc.absPath, rs.AbsPath(), "absPath mismatch")
// some values have defaults. Clear them here so test cases remain compact.
Expand Down
2 changes: 1 addition & 1 deletion api/krusty/originannotation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ buildMetadata: [originAnnotations]
metadata:
annotations:
config.kubernetes.io/origin: |
repo: https://github.com/kubernetes-sigs/kustomize
repo: https://github.com/kubernetes-sigs/kustomize.git
ref: v1.0.6
configuredIn: examples/ldap/base/kustomization.yaml
configuredBy:
Expand Down
Loading

0 comments on commit e62480d

Please sign in to comment.