diff --git a/pkg/commands/build/build.go b/pkg/commands/build/build.go index 0c3d1e425..43c1a982f 100644 --- a/pkg/commands/build/build.go +++ b/pkg/commands/build/build.go @@ -29,15 +29,15 @@ import ( "sigs.k8s.io/kustomize/pkg/target" ) -// BuildOptions contain the options for running a build -type BuildOptions struct { +// Options contain the options for running a build +type Options struct { kustomizationPath string outputPath string } -// NewBuildOptions creates a BuildOptions object -func NewBuildOptions(p, o string) *BuildOptions { - return &BuildOptions{ +// NewOptions creates a Options object +func NewOptions(p, o string) *Options { + return &Options{ kustomizationPath: p, outputPath: o, } @@ -63,7 +63,7 @@ func NewCmdBuild( out io.Writer, fs fs.FileSystem, rf *resmap.Factory, ptf transformer.Factory) *cobra.Command { - var o BuildOptions + var o Options cmd := &cobra.Command{ Use: "build [path]", @@ -86,7 +86,7 @@ func NewCmdBuild( } // Validate validates build command. -func (o *BuildOptions) Validate(args []string) error { +func (o *Options) Validate(args []string) error { if len(args) > 1 { return errors.New("specify one path to " + constants.KustomizationFileNames[0]) } @@ -100,7 +100,7 @@ func (o *BuildOptions) Validate(args []string) error { } // RunBuild runs build command. -func (o *BuildOptions) RunBuild( +func (o *Options) RunBuild( out io.Writer, fSys fs.FileSystem, rf *resmap.Factory, ptf transformer.Factory) error { ldr, err := loader.NewLoader(o.kustomizationPath, fSys) diff --git a/pkg/commands/build/build_test.go b/pkg/commands/build/build_test.go index 95859e529..169de1ad4 100644 --- a/pkg/commands/build/build_test.go +++ b/pkg/commands/build/build_test.go @@ -36,7 +36,7 @@ func TestBuildValidate(t *testing.T) { "", "specify one path to " + constants.KustomizationFileNames[0]}, } for _, mycase := range cases { - opts := BuildOptions{} + opts := Options{} e := opts.Validate(mycase.args) if len(mycase.erMsg) > 0 { if e == nil { diff --git a/pkg/git/repospec.go b/pkg/git/repospec.go index 046d92be9..db8b1b908 100644 --- a/pkg/git/repospec.go +++ b/pkg/git/repospec.go @@ -72,19 +72,6 @@ func (x *RepoSpec) Cleaner(fSys fs.FileSystem) func() error { return func() error { return fSys.RemoveAll(x.cloneDir.String()) } } -// IsRepoUrl checks if a string is likely a github repo Url. -func IsRepoUrl(arg string) bool { - arg = strings.ToLower(arg) - return !filepath.IsAbs(arg) && - (strings.HasPrefix(arg, "git::") || - strings.HasPrefix(arg, "gh:") || - strings.HasPrefix(arg, "ssh:") || - strings.HasPrefix(arg, "github.com") || - strings.HasPrefix(arg, "git@") || - strings.Index(arg, "github.com/") > -1 || - isAzureHost(arg) || isAWSHost(arg)) -} - // From strings like git@github.com:someOrg/someRepo.git or // https://github.com/someOrg/someRepo?ref=someHash, extract // the parts. @@ -104,11 +91,6 @@ func NewRepoSpecFromUrl(n string) (*RepoSpec, error) { path: path, ref: gitRef}, nil } -func NewRepoSpec( - raw string, cloneDir fs.ConfirmedDir, path string) *RepoSpec { - return &RepoSpec{raw: raw, cloneDir: cloneDir, path: path} -} - const ( refQuery = "?ref=" gitSuffix = ".git" diff --git a/pkg/git/repospec_test.go b/pkg/git/repospec_test.go index fca5f45ca..0413af6d7 100644 --- a/pkg/git/repospec_test.go +++ b/pkg/git/repospec_test.go @@ -23,106 +23,12 @@ import ( "testing" ) -func TestIsRepoURL(t *testing.T) { - - testcases := []struct { - input string - expected bool - }{ - { - input: "https://github.com/org/repo", - expected: true, - }, - { - input: "github.com/org/repo", - expected: true, - }, - { - input: "git@github.com:org/repo", - expected: true, - }, - { - input: "gh:org/repo", - expected: true, - }, - { - input: "git::https://gitlab.com/org/repo", - expected: true, - }, - { - input: "git@gitlab2.sqtools.ru:10022/infra/kubernetes/thanos-base.git?ref=v0.1.0", - expected: true, - }, - { - input: "git@bitbucket.org:org/repo.git", - expected: true, - }, - { - input: "git::http://git.example.com/org/repo.git", - expected: true, - }, - { - input: "git::https://git.example.com/org/repo.git", - expected: true, - }, - { - input: "ssh://git.example.com:7999/org/repo.git", - expected: true, - }, - { - input: "/github.com/org/repo", - expected: false, - }, - { - input: "/abs/path/to/file", - expected: false, - }, - { - input: "../relative", - expected: false, - }, - { - input: "foo", - expected: false, - }, - { - input: ".", - expected: false, - }, - { - input: "", - expected: false, - }, - } - for _, tc := range testcases { - actual := IsRepoUrl(tc.input) - if actual != tc.expected { - t.Errorf("unexpected error: unexpected result %t for input %s", actual, tc.input) - } - } -} - var orgRepos = []string{"someOrg/someRepo", "kubernetes/website"} var pathNames = []string{"README.md", "foo/krusty.txt", ""} var hrefArgs = []string{"someBranch", "master", "v0.1.0", ""} -var hostNamesRawAndNormalizedOld = map[string]string{ - "gh:%s": "gh:", - "GH:%s": "gh:", - "git@github.com:%s": "git@github.com:", - "gitHub.com/%s": "https://github.com/", - "https://github.com/%s": "https://github.com/", - "hTTps://github.com/%s": "https://github.com/", - "git::https://gitlab.com/%s": "https://gitlab.com/", - "github.com:%s": "https://github.com/", - "git::http://git.example.com/%s": "http://git.example.com/", - "git::https://git.example.com/%s": "https://git.example.com/", - "ssh://git.example.com:7999/%s": "ssh://git.example.com:7999/", - "https://git-codecommit.us-east-2.amazonaws.com/%s": "https://git-codecommit.us-east-2.amazonaws.com/", -} - var hostNamesRawAndNormalized = [][]string{ {"gh:", "gh:"}, {"GH:", "gh:"}, @@ -142,17 +48,6 @@ var hostNamesRawAndNormalized = [][]string{ {"git@gitlab2.sqtools.ru:10022/", "git@gitlab2.sqtools.ru:10022/"}, } -func makeUrlOld(hostFmt, orgRepo, path, href string) string { - if len(path) > 0 { - orgRepo = filepath.Join(orgRepo, path) - } - url := fmt.Sprintf(hostFmt, orgRepo) - if href != "" { - url += refQuery + href - } - return url -} - func makeUrl(hostFmt, orgRepo, path, href string) string { if len(path) > 0 { orgRepo = filepath.Join(orgRepo, path) @@ -225,47 +120,7 @@ func TestNewRepoSpecFromUrlErrors(t *testing.T) { } } -func TestParseGithubUrl(t *testing.T) { - for hostRawFmt, hostSpec := range hostNamesRawAndNormalizedOld { - for _, orgRepo := range orgRepos { - for _, pathName := range pathNames { - for _, hrefArg := range hrefArgs { - input := makeUrlOld(hostRawFmt, orgRepo, pathName, hrefArg) - if !IsRepoUrl(input) { - t.Errorf("Should smell like github arg: %s\n", input) - } - host, repo, path, gitRef := parseGithubUrl(input) - if host != hostSpec { - t.Errorf("\n"+ - " from %s\n"+ - " actual host %s\n"+ - "expected host %s\n", input, host, hostSpec) - } - if repo != orgRepo { - t.Errorf("\n"+ - " from %s\n"+ - " actual Repo %s\n"+ - "expected Repo %s\n", input, repo, orgRepo) - } - if path != pathName { - t.Errorf("\n"+ - " from %s\n"+ - " actual Path %s\n"+ - "expected Path %s\n", input, path, pathName) - } - if gitRef != hrefArg { - t.Errorf("\n"+ - " from %s\n"+ - " actual Href %s\n"+ - "expected Href %s\n", input, gitRef, hrefArg) - } - } - } - } - } -} - -func TestNewRepoSpecFromUrlOld(t *testing.T) { +func TestNewRepoSpecFromUrl_CloneSpecs(t *testing.T) { testcases := []struct { input string repo string diff --git a/pkg/image/append.go b/pkg/image/append.go deleted file mode 100644 index ae479b0f1..000000000 --- a/pkg/image/append.go +++ /dev/null @@ -1,33 +0,0 @@ -/* -Copyright 2019 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package image - -// Append appends a slice of type Tag to slice of type Image -func Append(images []Image, tags ...ImageTag) []Image { - for _, tag := range tags { - images = append(images, toImage(tag)) - } - return images -} - -func toImage(tag ImageTag) Image { - return Image{ - Name: tag.Name, - NewTag: tag.NewTag, - Digest: tag.Digest, - } -} diff --git a/pkg/image/imagetag.go b/pkg/image/deprecatedimage.go similarity index 89% rename from pkg/image/imagetag.go rename to pkg/image/deprecatedimage.go index acff10a50..65db4051b 100644 --- a/pkg/image/imagetag.go +++ b/pkg/image/deprecatedimage.go @@ -16,9 +16,10 @@ limitations under the License. package image -// ImageTag contains an image and a new tag, which will replace the original tag. +// DeprecatedImage contains an image and a new tag, +// which will replace the original tag. // Deprecated, instead use Image. -type ImageTag struct { +type DeprecatedImage struct { // Name is a tag-less image name. Name string `json:"name,omitempty" yaml:"name,omitempty"` diff --git a/pkg/loader/fileloader_test.go b/pkg/loader/fileloader_test.go index 8b50b5756..92ae3368e 100644 --- a/pkg/loader/fileloader_test.go +++ b/pkg/loader/fileloader_test.go @@ -320,14 +320,14 @@ func splitOnNthSlash(v string, n int) (string, string) { } func TestSplit(t *testing.T) { - path := "a/b/c/d/e/f/g" - if left, right := splitOnNthSlash(path, 2); left != "a/b" || right != "c/d/e/f/g" { + p := "a/b/c/d/e/f/g" + if left, right := splitOnNthSlash(p, 2); left != "a/b" || right != "c/d/e/f/g" { t.Fatalf("got left='%s', right='%s'", left, right) } - if left, right := splitOnNthSlash(path, 3); left != "a/b/c" || right != "d/e/f/g" { + if left, right := splitOnNthSlash(p, 3); left != "a/b/c" || right != "d/e/f/g" { t.Fatalf("got left='%s', right='%s'", left, right) } - if left, right := splitOnNthSlash(path, 6); left != "a/b/c/d/e/f" || right != "g" { + if left, right := splitOnNthSlash(p, 6); left != "a/b/c/d/e/f" || right != "g" { t.Fatalf("got left='%s', right='%s'", left, right) } } @@ -336,10 +336,6 @@ func TestNewLoaderAtGitClone(t *testing.T) { rootUrl := "github.com/someOrg/someRepo" pathInRepo := "foo/base" url := rootUrl + "/" + pathInRepo - if !git.IsRepoUrl(url) { - t.Fatalf("'%s' should be accepted as a repo url", url) - } - coRoot := "/tmp" fSys := fs.MakeFakeFS() fSys.MkdirAll(coRoot) @@ -374,9 +370,6 @@ whatever pathInRepo = "foo/overlay" fSys.MkdirAll(coRoot + "/" + pathInRepo) url = rootUrl + "/" + pathInRepo - if !git.IsRepoUrl(url) { - t.Fatalf("'%s' should be accepted as a repo url", url) - } l2, err := l.New(url) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/pkg/transformers/config/factory.go b/pkg/transformers/config/factory.go index 9def6a6e2..d0ea0d1dd 100644 --- a/pkg/transformers/config/factory.go +++ b/pkg/transformers/config/factory.go @@ -28,36 +28,9 @@ type Factory struct { ldr ifc.Loader } -// TODO(#606): Setting this to false satisfies the feature -// request in 606. The todo is to delete the non-active -// code path in a subsequent PR. -const demandExplicitConfig = false - -func MakeTransformerConfig( - ldr ifc.Loader, paths []string) (*TransformerConfig, error) { - if demandExplicitConfig { - return loadConfigFromDiskOrDefaults(ldr, paths) - } - return mergeCustomConfigWithDefaults(ldr, paths) -} - -// loadConfigFromDiskOrDefaults returns a TransformerConfig object -// built from either files or the hardcoded default configs. -// There's no merging, it's one or the other. This is preferred -// if one wants all configuration to be explicit in version -// control, as opposed to relying on a mix of files and -// hard-coded config. -func loadConfigFromDiskOrDefaults( - ldr ifc.Loader, paths []string) (*TransformerConfig, error) { - if paths == nil || len(paths) == 0 { - return MakeDefaultConfig(), nil - } - return NewFactory(ldr).FromFiles(paths) -} - -// mergeCustomConfigWithDefaults returns a merger of custom config, +// MakeTransformerConfig returns a merger of custom config, // if any, with default config. -func mergeCustomConfigWithDefaults( +func MakeTransformerConfig( ldr ifc.Loader, paths []string) (*TransformerConfig, error) { t1 := MakeDefaultConfig() if len(paths) == 0 { diff --git a/pkg/transformers/config/factory_test.go b/pkg/transformers/config/factory_test.go index a380af74c..04ad211d5 100644 --- a/pkg/transformers/config/factory_test.go +++ b/pkg/transformers/config/factory_test.go @@ -31,9 +31,9 @@ func TestMakeDefaultConfig(t *testing.T) { } func makeTestLoader(path, content string) ifc.Loader { - fs := fs.MakeFakeFS() - fs.WriteFile(path, []byte(content)) - return loader.NewFileLoaderAtRoot(fs) + fSys := fs.MakeFakeFS() + fSys.WriteFile(path, []byte(content)) + return loader.NewFileLoaderAtRoot(fSys) } func TestFromFiles(t *testing.T) { diff --git a/pkg/transformers/image.go b/pkg/transformers/image.go index 6dbc94412..f6f999d1e 100644 --- a/pkg/transformers/image.go +++ b/pkg/transformers/image.go @@ -84,22 +84,18 @@ func (pt *imageTransformer) updateContainers(obj map[string]interface{}, path st } imageName := containerImage.(string) - for _, image := range pt.images { - if isImageMatched(imageName, image.Name) { + for _, img := range pt.images { + if isImageMatched(imageName, img.Name) { name, tag := split(imageName) - - if image.NewName != "" { - name = image.NewName + if img.NewName != "" { + name = img.NewName } - - if image.NewTag != "" { - tag = ":" + image.NewTag + if img.NewTag != "" { + tag = ":" + img.NewTag } - - if image.Digest != "" { - tag = "@" + image.Digest + if img.Digest != "" { + tag = "@" + img.Digest } - container["image"] = name + tag break }