Skip to content

Commit

Permalink
fixes for kverey's comments
Browse files Browse the repository at this point in the history
  • Loading branch information
mightyguava committed Sep 8, 2022
1 parent 1287caf commit 74cc1c0
Show file tree
Hide file tree
Showing 4 changed files with 168 additions and 29 deletions.
36 changes: 20 additions & 16 deletions api/internal/git/repospec.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func NewRepoSpecFromURL(n string) (*RepoSpec, error) {
return nil, fmt.Errorf("uri looks like abs path: %s", n)
}
host, orgRepo, path, gitRef, gitSubmodules, suffix, gitTimeout := parseGitURL(n)
if orgRepo == "" && !strings.HasPrefix(host, fileScheme) {
if orgRepo == "" {
return nil, fmt.Errorf("url lacks orgRepo: %s", n)
}
if host == "" {
Expand Down Expand Up @@ -119,23 +119,13 @@ func parseGitURL(n string) (
path, gitRef, gitTimeout, gitSubmodules = peelQuery(n[index+len(gitDelimiter)+len(orgRepo):])
return
}
if strings.HasPrefix(n, fileScheme) {
idx := strings.Index(n, gitSuffix)
if idx == -1 {
// invalid, must have .git somewhere
return
}
host += n[:idx+len(gitSuffix)]
n = n[idx+len(gitSuffix):]
if len(n) > 0 && n[0] == '/' {
n = n[1:]
}
path, gitRef, gitTimeout, gitSubmodules = peelQuery(n)
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 @@ -146,6 +136,20 @@ func parseGitURL(n string) (
return
}

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

i := strings.Index(n, "/")
if i < 1 {
path, gitRef, gitTimeout, gitSubmodules = peelQuery(n)
Expand Down
65 changes: 63 additions & 2 deletions api/internal/git/repospec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,16 +288,77 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
},
{
name: "t15",
input: "https://github.com/kubernetes-sigs/kustomize//examples/multibases/dev/?submodules=0&ref=v1.0.6&timeout=300",
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://a/b/c/someRepo.git",
OrgRepo: "",
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",
},
},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
Expand Down
94 changes: 84 additions & 10 deletions api/krusty/remoteloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sigs.k8s.io/kustomize/api/krusty"
"sigs.k8s.io/kustomize/api/loader"
"sigs.k8s.io/kustomize/api/resmap"
"sigs.k8s.io/kustomize/kyaml/filesys"
"sigs.k8s.io/kustomize/kyaml/yaml"
Expand Down Expand Up @@ -107,6 +108,7 @@ spec:
- image: nginx:1.7.9
name: nginx
`
var simpleBuildWithNginx2 = strings.ReplaceAll(simpleBuild, "nginx:1.7.9", "nginx:2")
var multibaseDevExampleBuild = strings.ReplaceAll(simpleBuild, "myapp-pod", "dev-myapp-pod")

repos := createGitRepos(t)
Expand Down Expand Up @@ -140,7 +142,7 @@ resources:
- "file://$ROOT/simple.git?ref=change-image"
`,

expected: strings.ReplaceAll(simpleBuild, "nginx:1.7.9", "nginx:2"),
expected: simpleBuildWithNginx2,
},
{
// Version is the same as ref
Expand All @@ -149,13 +151,29 @@ resources:
resources:
- file://$ROOT/simple.git?version=change-image
`,
expected: strings.ReplaceAll(simpleBuild, "nginx:1.7.9", "nginx:2"),
expected: simpleBuildWithNginx2,
},
{
name: "has submodule",
kustomization: `
resources:
- file://$ROOT/with-submodule.git/submodule
`,
expected: simpleBuild,
},
{
name: "has timeout",
kustomization: `
resources:
- file://$ROOT/simple.git?timeout=500
`,
expected: simpleBuild,
},
{
name: "triple slash absolute path",
kustomization: `
resources:
- file:///$ROOT/simple.git
`,
expected: simpleBuild,
},
Expand All @@ -165,7 +183,7 @@ resources:
resources:
- file://$ROOT/with-submodule.git/submodule?submodules=0
`,
err: "unable to find",
err: "unable to find one of 'kustomization.yaml', 'kustomization.yml' or 'Kustomization' in directory",
},
{
name: "has origin annotation",
Expand All @@ -192,10 +210,10 @@ spec:
`,
},
{
name: "has ref path and origin annotation",
name: "has ref path timeout and origin annotation",
kustomization: `
resources:
- file://$ROOT/multibase.git/dev?version=main
- file://$ROOT/multibase.git/dev?version=main&timeout=500
buildMetadata: [originAnnotations]
`,
expected: `apiVersion: v1
Expand All @@ -215,6 +233,14 @@ spec:
name: nginx
`,
},
{
name: "repo does not exist ",
kustomization: `
resources:
- file:///not/a/real/repo
`,
err: "fatal: '/not/a/real/repo' does not appear to be a git repository",
},
}

for _, test := range tests {
Expand Down Expand Up @@ -248,17 +274,26 @@ spec:

func TestRemoteLoad_RemoteProtocols(t *testing.T) {
// Slow remote tests with long timeouts.
// TODO: Maybe they should retry too.
// TODO: If these end up flaking, they should retry. If not, remove this TODO.
tests := []struct {
name string
kustomization string
err string
errT error
beforeTest func(t *testing.T, )
}{
{
name: "https",
kustomization: `
resources:
- https://github.com/kubernetes-sigs/kustomize//examples/multibases/dev/?submodules=0&ref=v1.0.6&timeout=300
- https://github.com/kubernetes-sigs/kustomize//examples/multibases/dev/?submodules=0&ref=kustomize%2Fv4.5.7&timeout=300
`,
},
{
name: "git double-colon https",
kustomization: `
resources:
- git::https://github.com/kubernetes-sigs/kustomize//examples/multibases/dev/?submodules=0&ref=kustomize%2Fv4.5.7&timeout=300
`,
},
{
Expand All @@ -270,17 +305,53 @@ namePrefix: dev-
`,
},
{
name: "ssh",
name: "ssh",
beforeTest: configureGitSSHCommand,
kustomization: `
resources:
- ssh://[email protected]/kubernetes-sigs/kustomize/examples/multibases/dev?submodules=0&ref=v1.0.6&timeout=300
- [email protected]/kubernetes-sigs/kustomize/examples/multibases/dev?submodules=0&ref=kustomize%2Fv4.5.7&timeout=300
`,
},
{
name: "ssh with colon",
beforeTest: configureGitSSHCommand,
kustomization: `
resources:
- [email protected]:kubernetes-sigs/kustomize/examples/multibases/dev?submodules=0&ref=kustomize%2Fv4.5.7&timeout=300
`,
},
{
name: "ssh without username",
beforeTest: configureGitSSHCommand,
kustomization: `
resources:
- github.com/kubernetes-sigs/kustomize/examples/multibases/dev?submodules=0&ref=kustomize%2Fv4.5.7&timeout=300
`,
},
{
name: "ssh scheme",
beforeTest: configureGitSSHCommand,
kustomization: `
resources:
- ssh://[email protected]/kubernetes-sigs/kustomize/examples/multibases/dev?submodules=0&ref=kustomize%2Fv4.5.7&timeout=300
`,
},
{
name: "http error",
kustomization: `
resources:
- https://github.com/thisisa404.yaml
`,
err: "accumulating resources: accumulating resources from 'https://github.com/thisisa404.yaml': HTTP Error: status code 404 (Not Found)",
errT: loader.ErrHTTP,
},
}

configureGitSSHCommand(t)
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
if test.beforeTest != nil {
test.beforeTest(t)
}
fSys, tmpDir := createKustDir(t, test.kustomization)

b := krusty.MakeKustomizer(krusty.MakeDefaultOptions())
Expand All @@ -293,6 +364,9 @@ resources:
if err != nil {
assert.Contains(t, err.Error(), test.err)
}
if test.errT != nil {
assert.ErrorIs(t, err, test.errT)
}
} else {
assert.NoError(t, err)
if err == nil {
Expand Down
2 changes: 1 addition & 1 deletion api/resource/origin.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (origin *Origin) Append(path string) *Origin {
originCopy := origin.Copy()
repoSpec, err := git.NewRepoSpecFromURL(path)
if err == nil {
originCopy.Repo = repoSpec.Host + repoSpec.OrgRepo
originCopy.Repo = repoSpec.CloneSpec()
absPath := repoSpec.AbsPath()
path = absPath[strings.Index(absPath[1:], "/")+1:][1:]
originCopy.Path = ""
Expand Down

0 comments on commit 74cc1c0

Please sign in to comment.