Switch to black box testing of KustTarget and Resource

pkg/commands/build/build.go

@@ -108,7 +108,7 @@ func (o *Options) RunBuild(
 		return err
 	}
 	defer ldr.Cleanup()
-	kt, err := target.NewKustTarget(ldr, fSys, rf, ptf)
+	kt, err := target.NewKustTarget(ldr, rf, ptf)
 	if err != nil {
 		return err
 	}

pkg/resource/factory_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package resource
+package resource_test
 
 import (
 	"reflect"
@@ -22,6 +22,7 @@ import (
 
 	"sigs.k8s.io/kustomize/pkg/internal/loadertest"
 	"sigs.k8s.io/kustomize/pkg/patch"
+	. "sigs.k8s.io/kustomize/pkg/resource"
 )
 
 func TestSliceFromPatches(t *testing.T) {

pkg/resource/resource.go

@@ -38,7 +38,7 @@ func (r *Resource) String() string {
 	if err != nil {
 		return "<" + err.Error() + ">"
 	}
-	return strings.TrimSpace(string(bs))
+	return strings.TrimSpace(string(bs)) + r.options.String()
 }
 
 // DeepCopy returns a new copy of resource

pkg/resource/resource_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package resource
+package resource_test
 
 import (
 	"testing"
@@ -22,6 +22,7 @@ import (
 	"sigs.k8s.io/kustomize/k8sdeps/kunstruct"
 	"sigs.k8s.io/kustomize/pkg/gvk"
 	"sigs.k8s.io/kustomize/pkg/resid"
+	. "sigs.k8s.io/kustomize/pkg/resource"
 )
 
 var factory = NewFactory(
@@ -37,7 +38,9 @@ var testConfigMap = factory.FromMap(
 		},
 	})
 
-const testConfigMapString = `{"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"winnie","namespace":"hundred-acre-wood"}}`
+const genArgOptions = "{nsfx:false,beh:unspecified}"
+
+const configMapAsString = `{"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"winnie","namespace":"hundred-acre-wood"}}`
 
 var testDeployment = factory.FromMap(
 	map[string]interface{}{
@@ -48,7 +51,7 @@ var testDeployment = factory.FromMap(
 		},
 	})
 
-const testDeploymentString = `{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"name":"pooh"}}`
+const deploymentAsString = `{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"name":"pooh"}}`
 
 func TestResourceString(t *testing.T) {
 	tests := []struct {
@@ -57,11 +60,11 @@ func TestResourceString(t *testing.T) {
 	}{
 		{
 			in: testConfigMap,
-			s:  testConfigMapString,
+			s:  configMapAsString + genArgOptions,
 		},
 		{
 			in: testDeployment,
-			s:  testDeploymentString,
+			s:  deploymentAsString + genArgOptions,
 		},
 	}
 	for _, test := range tests {

pkg/target/baseandoverlaymedium_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package target
+package target_test
 
 import (
 	"testing"

pkg/target/baseandoverlaysmall_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package target
+package target_test
 
 import (
 	"testing"

pkg/target/configmaps_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package target
+package target_test
 
 import (
 	"testing"

pkg/target/crd_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package target
+package target_test
 
 import (
 	"testing"

pkg/target/customconfig_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package target
+package target_test
 
 import (
 	"testing"

pkg/target/generatormergeandreplace_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package target
+package target_test
 
 import (
 	"testing"

pkg/target/generatoroptions_test.go

@@ -11,7 +11,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package target
+package target_test
 
 import (
 	"testing"

pkg/target/kusttarget.go

@@ -26,7 +26,6 @@ import (
 	"github.com/ghodss/yaml"
 	"github.com/pkg/errors"
 	"sigs.k8s.io/kustomize/pkg/constants"
-	"sigs.k8s.io/kustomize/pkg/fs"
 	"sigs.k8s.io/kustomize/pkg/ifc"
 	"sigs.k8s.io/kustomize/pkg/ifc/transformer"
 	interror "sigs.k8s.io/kustomize/pkg/internal/error"
@@ -42,7 +41,6 @@ import (
 type KustTarget struct {
 	kustomization *types.Kustomization
 	ldr           ifc.Loader
-	fSys          fs.FileSystem
 	rFactory      *resmap.Factory
 	tFactory      transformer.Factory
 }
@@ -50,7 +48,6 @@ type KustTarget struct {
 // NewKustTarget returns a new instance of KustTarget primed with a Loader.
 func NewKustTarget(
 	ldr ifc.Loader,
-	fSys fs.FileSystem,
 	rFactory *resmap.Factory,
 	tFactory transformer.Factory) (*KustTarget, error) {
 	content, err := loadKustFile(ldr)
@@ -70,7 +67,6 @@ func NewKustTarget(
 	return &KustTarget{
 		kustomization: &k,
 		ldr:           ldr,
-		fSys:          fSys,
 		rFactory:      rFactory,
 		tFactory:      tFactory,
 	}, nil
@@ -123,7 +119,7 @@ func unmarshal(y []byte, o interface{}) error {
 // MakeCustomizedResMap creates a ResMap per kustomization instructions.
 // The Resources in the returned ResMap are fully customized.
 func (kt *KustTarget) MakeCustomizedResMap() (resmap.ResMap, error) {
-	ra, err := kt.accumulateTarget()
+	ra, err := kt.AccumulateTarget()
 	if err != nil {
 		return nil, err
 	}
@@ -147,11 +143,11 @@ func (kt *KustTarget) shouldAddHashSuffixesToGeneratedResources() bool {
 		!kt.kustomization.GeneratorOptions.DisableNameSuffixHash
 }
 
-// accumulateTarget returns a new ResAccumulator,
+// AccumulateTarget returns a new ResAccumulator,
 // holding customized resources and the data/rules used
 // to do so.  The name back references and vars are
 // not yet fixed.
-func (kt *KustTarget) accumulateTarget() (
+func (kt *KustTarget) AccumulateTarget() (
 	ra *ResAccumulator, err error) {
 	// TODO(monopole): Get rid of the KustomizationErrors accumulator.
 	// It's not consistently used, and complicates tests.
@@ -249,15 +245,15 @@ func (kt *KustTarget) accumulateBases() (
 			continue
 		}
 		subKt, err := NewKustTarget(
-			ldr, kt.fSys, kt.rFactory, kt.tFactory)
+			ldr, kt.rFactory, kt.tFactory)
 		if err != nil {
 			errs.Append(errors.Wrap(err, "couldn't make target for "+path))
 			ldr.Cleanup()
 			continue
 		}
-		subRa, err := subKt.accumulateTarget()
+		subRa, err := subKt.AccumulateTarget()
 		if err != nil {
-			errs.Append(errors.Wrap(err, "accumulateTarget"))
+			errs.Append(errors.Wrap(err, "AccumulateTarget"))
 			ldr.Cleanup()
 			continue
 		}

pkg/target/kusttarget_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package target
+package target_test
 
 import (
 	"encoding/base64"
@@ -23,18 +23,18 @@ import (
 	"strings"
 	"testing"
 
-	"sigs.k8s.io/kustomize/pkg/fs"
 	"sigs.k8s.io/kustomize/pkg/gvk"
 	"sigs.k8s.io/kustomize/pkg/ifc"
 	"sigs.k8s.io/kustomize/pkg/internal/loadertest"
 	"sigs.k8s.io/kustomize/pkg/resid"
 	"sigs.k8s.io/kustomize/pkg/resmap"
 	"sigs.k8s.io/kustomize/pkg/resource"
+	. "sigs.k8s.io/kustomize/pkg/target"
 	"sigs.k8s.io/kustomize/pkg/types"
 )
 
 const (
-	kustomizationContent1 = `
+	kustomizationContent = `
 apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
 namePrefix: foo-
@@ -47,6 +47,8 @@ commonAnnotations:
 resources:
   - deployment.yaml
   - namespace.yaml
+generatorOptions:
+  disableNameSuffixHash: false
 configMapGenerator:
 - name: literalConfigMap
   literals:
@@ -83,9 +85,9 @@ metadata:
 ]`
 )
 
-func TestResources1(t *testing.T) {
+func TestResources(t *testing.T) {
 	th := NewKustTestHarness(t, "/whatever")
-	th.writeK("/whatever/", kustomizationContent1)
+	th.writeK("/whatever/", kustomizationContent)
 	th.writeF("/whatever/deployment.yaml", deploymentContent)
 	th.writeF("/whatever/namespace.yaml", namespaceContent)
 	th.writeF("/whatever/jsonpatch.json", jsonpatchContent)
@@ -146,7 +148,9 @@ func TestResources1(t *testing.T) {
 					"DB_USERNAME": "admin",
 					"DB_PASSWORD": "somepw",
 				},
-			}, &types.GeneratorArgs{}, nil),
+			},
+			&types.GeneratorArgs{},
+			&types.GeneratorOptions{}),
 		resid.NewResIdWithPrefixSuffixNamespace(
 			gvk.Gvk{Version: "v1", Kind: "Secret"},
 			"secret", "foo-", "-bar", "ns1"): th.fromMapAndOption(
@@ -168,7 +172,9 @@ func TestResources1(t *testing.T) {
 					"DB_USERNAME": base64.StdEncoding.EncodeToString([]byte("admin")),
 					"DB_PASSWORD": base64.StdEncoding.EncodeToString([]byte("somepw")),
 				},
-			}, &types.GeneratorArgs{}, nil),
+			},
+			&types.GeneratorArgs{},
+			&types.GeneratorOptions{}),
 		resid.NewResIdWithPrefixSuffixNamespace(
 			gvk.Gvk{Version: "v1", Kind: "Namespace"},
 			"ns1", "foo-", "-bar", ""): th.fromMap(
@@ -198,8 +204,7 @@ func TestResources1(t *testing.T) {
 }
 
 func TestKustomizationNotFound(t *testing.T) {
-	_, err := NewKustTarget(
-		loadertest.NewFakeLoader("/foo"), fs.MakeFakeFS(), nil, nil)
+	_, err := NewKustTarget(loadertest.NewFakeLoader("/foo"), nil, nil)
 	if err == nil {
 		t.Fatalf("expected an error")
 	}
@@ -211,7 +216,7 @@ func TestKustomizationNotFound(t *testing.T) {
 
 func TestResourceNotFound(t *testing.T) {
 	th := NewKustTestHarness(t, "/whatever")
-	th.writeK("/whatever", kustomizationContent1)
+	th.writeK("/whatever", kustomizationContent)
 	_, err := th.makeKustTarget().MakeCustomizedResMap()
 	if err == nil {
 		t.Fatalf("Didn't get the expected error for an unknown resource")
@@ -232,13 +237,12 @@ func findSecret(m resmap.ResMap) *resource.Resource {
 
 func TestDisableNameSuffixHash(t *testing.T) {
 	th := NewKustTestHarness(t, "/whatever")
-	th.writeK("/whatever/", kustomizationContent1)
+	th.writeK("/whatever/", kustomizationContent)
 	th.writeF("/whatever/deployment.yaml", deploymentContent)
 	th.writeF("/whatever/namespace.yaml", namespaceContent)
 	th.writeF("/whatever/jsonpatch.json", jsonpatchContent)
 
-	kt := th.makeKustTarget()
-	m, err := kt.MakeCustomizedResMap()
+	m, err := th.makeKustTarget().MakeCustomizedResMap()
 	if err != nil {
 		t.Fatalf("unexpected Resources error %v", err)
 	}
@@ -250,9 +254,11 @@ func TestDisableNameSuffixHash(t *testing.T) {
 		t.Errorf("unexpected secret resource name: %s", secret.GetName())
 	}
 
-	kt.kustomization.GeneratorOptions = &types.GeneratorOptions{
-		DisableNameSuffixHash: true}
-	m, err = kt.MakeCustomizedResMap()
+	th.writeK("/whatever/",
+		strings.Replace(kustomizationContent,
+			"disableNameSuffixHash: false",
+			"disableNameSuffixHash: true", -1))
+	m, err = th.makeKustTarget().MakeCustomizedResMap()
 	if err != nil {
 		t.Fatalf("unexpected Resources error %v", err)
 	}
@@ -338,11 +344,11 @@ vars:
       name: heron
       apiVersion: v300
 `)
-	ra, err := th.makeKustTarget().accumulateTarget()
+	ra, err := th.makeKustTarget().AccumulateTarget()
 	if err != nil {
 		t.Fatalf("Err: %v", err)
 	}
-	vars := ra.varSet.Set()
+	vars := ra.Vars()
 	if len(vars) != 2 {
 		t.Fatalf("unexpected size %d", len(vars))
 	}
@@ -388,11 +394,11 @@ vars:
 bases:
 - ../o1
 `)
-	ra, err := th.makeKustTarget().accumulateTarget()
+	ra, err := th.makeKustTarget().AccumulateTarget()
 	if err != nil {
 		t.Fatalf("Err: %v", err)
 	}
-	vars := ra.varSet.Set()
+	vars := ra.Vars()
 	if len(vars) != 4 {
 		for i, v := range vars {
 			fmt.Printf("%v: %v\n", i, v)
@@ -441,7 +447,7 @@ vars:
 bases:
 - ../o1
 `)
-	_, err := th.makeKustTarget().accumulateTarget()
+	_, err := th.makeKustTarget().AccumulateTarget()
 	if err == nil {
 		t.Fatalf("expected var collision")
 	}

pkg/target/kusttestharness_test.go

@@ -14,24 +14,24 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package target
+package target_test
 
 // A collection of utilities used in target tests.
 
 import (
 	"fmt"
 	"path/filepath"
-	"sigs.k8s.io/kustomize/pkg/transformers/config/defaultconfig"
 	"strings"
 	"testing"
 
 	"sigs.k8s.io/kustomize/k8sdeps/kunstruct"
 	"sigs.k8s.io/kustomize/k8sdeps/transformer"
 	"sigs.k8s.io/kustomize/pkg/constants"
-	"sigs.k8s.io/kustomize/pkg/fs"
 	"sigs.k8s.io/kustomize/pkg/internal/loadertest"
 	"sigs.k8s.io/kustomize/pkg/resmap"
 	"sigs.k8s.io/kustomize/pkg/resource"
+	. "sigs.k8s.io/kustomize/pkg/target"
+	"sigs.k8s.io/kustomize/pkg/transformers/config/defaultconfig"
 	"sigs.k8s.io/kustomize/pkg/types"
 )
 
@@ -50,17 +50,8 @@ func NewKustTestHarness(t *testing.T, path string) *KustTestHarness {
 }
 
 func (th *KustTestHarness) makeKustTarget() *KustTarget {
-	// Warning: the following filesystem - a fake - must be rooted at /.
-	// This fs root is used as the working directory for the shell spawned by
-	// the secretgenerator, and has nothing to do with the filesystem used
-	// to load relative paths from the fake filesystem.
-	// This trick only works for secret generator commands that don't actually
-	// try to read the file system, because these tests don't write to the
-	// real "/" directory.  See use of exec package in the secretfactory.
-	fakeFs := fs.MakeFakeFS()
-	fakeFs.Mkdir("/")
 	kt, err := NewKustTarget(
-		th.ldr, fakeFs, th.rf, transformer.NewFactoryImpl())
+		th.ldr, th.rf, transformer.NewFactoryImpl())
 	if err != nil {
 		th.t.Fatalf("Unexpected construction error %v", err)
 	}

pkg/target/multiplepatch_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package target
+package target_test
 
 import (
 	"strings"

pkg/target/namespacedgenerators_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package target
+package target_test
 
 import (
 	"testing"

pkg/target/nullvalues_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package target
+package target_test
 
 import (
 	"testing"

pkg/target/resaccumulator.go

@@ -55,6 +55,11 @@ func (ra *ResAccumulator) ResMap() resmap.ResMap {
 	return result
 }
 
+// Vars returns a copy of underlying vars.
+func (ra *ResAccumulator) Vars() []types.Var {
+	return ra.varSet.Set()
+}
+
 func (ra *ResAccumulator) MergeResourcesWithErrorOnIdCollision(
 	resources resmap.ResMap) (err error) {
 	ra.resMap, err = resmap.MergeWithErrorOnIdCollision(

pkg/target/resaccumulator_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package target
+package target_test
 
 import (
 	"strings"
@@ -25,6 +25,7 @@ import (
 	"sigs.k8s.io/kustomize/pkg/resid"
 	"sigs.k8s.io/kustomize/pkg/resmap"
 	"sigs.k8s.io/kustomize/pkg/resource"
+	. "sigs.k8s.io/kustomize/pkg/target"
 	"sigs.k8s.io/kustomize/pkg/transformers/config"
 	"sigs.k8s.io/kustomize/pkg/types"
 )

pkg/target/resourceconflict_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package target
+package target_test
 
 import (
 	"strings"

pkg/target/variableref_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package target
+package target_test
 
 import (
 	"testing"

pkg/types/genargs.go

@@ -16,6 +16,11 @@ limitations under the License.
 
 package types
 
+import (
+	"strconv"
+	"strings"
+)
+
 // GenArgs contains both generator args and options
 type GenArgs struct {
 	args *GeneratorArgs
@@ -30,6 +35,18 @@ func NewGenArgs(args *GeneratorArgs, opts *GeneratorOptions) *GenArgs {
 	}
 }
 
+func (g *GenArgs) String() string {
+	if g == nil {
+		return "{nilGenArgs}"
+	}
+	return "{" +
+		strings.Join([]string{
+			"nsfx:" + strconv.FormatBool(g.NeedsHashSuffix()),
+			"beh:" + g.Behavior().String()},
+			",") +
+		"}"
+}
+
 // NeedHashSuffix returns true if the hash suffix is needed.
 // It is needed when the two conditions are both met
 //  1) GenArgs is not nil

pkg/types/genargs_test.go

@@ -0,0 +1,47 @@
+/*
+Copyright 2018 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 types_test
+
+import (
+	"testing"
+
+	. "sigs.k8s.io/kustomize/pkg/types"
+)
+
+func TestGenArgs_String(t *testing.T) {
+	tests := []struct {
+		ga       *GenArgs
+		expected string
+	}{
+		{
+			ga:       nil,
+			expected: "{nilGenArgs}",
+		},
+		{
+			ga:       &GenArgs{},
+			expected: "{nsfx:false,beh:unspecified}",
+		},
+		{
+			ga: NewGenArgs(
+				&GeneratorArgs{Behavior: "merge"},
+				&GeneratorOptions{DisableNameSuffixHash: false}),
+			expected: "{nsfx:true,beh:merge}",
+		},
+	}
+	for _, test := range tests {
+		if test.ga.String() != test.expected {
+			t.Fatalf("Expected '%s', got '%s'", test.expected, test.ga.String())
+		}
+	}
+}

pkg/types/var.go

@@ -75,9 +75,11 @@ type VarSet struct {
 	set []Var
 }
 
-// Set returns the var set.
+// Set returns a copy of the var set.
 func (vs *VarSet) Set() []Var {
-	return vs.set
+	s := make([]Var, len(vs.set))
+	copy(s, vs.set)
+	return s
 }
 
 // MergeSet absorbs other vars with error on name collision.