Merge pull request #743 from monopole/deleteDeadCode

Delete some dead code and fix nits.

pkg/commands/build/build.go

@@ -29,15 +29,15 @@ import (
 	"sigs.k8s.io/kustomize/pkg/target"
 )
 
-// BuildOptions contain the options for running a build
+// Options contain the options for running a build
-type BuildOptions struct {
+type Options struct {
 	kustomizationPath string
 	outputPath        string
 }
 
-// NewBuildOptions creates a BuildOptions object
+// NewOptions creates a Options object
-func NewBuildOptions(p, o string) *BuildOptions {
+func NewOptions(p, o string) *Options {
-	return &BuildOptions{
+	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)

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 {

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"

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) {
+func TestNewRepoSpecFromUrl_CloneSpecs(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) {
 	testcases := []struct {
 		input string
 		repo  string

pkg/image/append.go

@@ -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,
-	}
-}

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"`
 

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"
+	p := "a/b/c/d/e/f/g"
-	if left, right := splitOnNthSlash(path, 2); left != "a/b" || right != "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)

pkg/transformers/config/factory.go

@@ -28,36 +28,9 @@ type Factory struct {
 	ldr ifc.Loader
 }
 
-// TODO(#606): Setting this to false satisfies the feature
+// MakeTransformerConfig returns a merger of custom config,
-// 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,
 // if any, with default config.
-func mergeCustomConfigWithDefaults(
+func MakeTransformerConfig(
 	ldr ifc.Loader, paths []string) (*TransformerConfig, error) {
 	t1 := MakeDefaultConfig()
 	if len(paths) == 0 {

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()
+	fSys := fs.MakeFakeFS()
-	fs.WriteFile(path, []byte(content))
+	fSys.WriteFile(path, []byte(content))
-	return loader.NewFileLoaderAtRoot(fs)
+	return loader.NewFileLoaderAtRoot(fSys)
 }
 
 func TestFromFiles(t *testing.T) {

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 {
+		for _, img := range pt.images {
-			if isImageMatched(imageName, image.Name) {
+			if isImageMatched(imageName, img.Name) {
 				name, tag := split(imageName)
-
+				if img.NewName != "" {
-				if image.NewName != "" {
+					name = img.NewName
-					name = image.NewName
 				}
-
+				if img.NewTag != "" {
-				if image.NewTag != "" {
+					tag = ":" + img.NewTag
-					tag = ":" + image.NewTag
 				}
-
+				if img.Digest != "" {
-				if image.Digest != "" {
+					tag = "@" + img.Digest
-					tag = "@" + image.Digest
 				}
-
 				container["image"] = name + tag
 				break
 			}