From 4583c4a9de4f6b07ae8cc2d9dd46e225b400e2d1 Mon Sep 17 00:00:00 2001 From: jregan Date: Wed, 19 Dec 2018 19:01:56 -0800 Subject: [PATCH] More custom transform coverage. --- pkg/target/baseandoverlaymedium_test.go | 6 +- pkg/target/baseandoverlaysmall_test.go | 9 +- pkg/target/customconfig_test.go | 341 ++++++++++++++++++++++++ pkg/target/kusttarget.go | 34 ++- pkg/target/kusttarget_test.go | 6 +- pkg/target/resourceconflict_test.go | 3 +- pkg/target/utils_for_test.go | 20 +- 7 files changed, 408 insertions(+), 11 deletions(-) create mode 100644 pkg/target/customconfig_test.go diff --git a/pkg/target/baseandoverlaymedium_test.go b/pkg/target/baseandoverlaymedium_test.go index e51c92d35..0ffd7d2b0 100644 --- a/pkg/target/baseandoverlaymedium_test.go +++ b/pkg/target/baseandoverlaymedium_test.go @@ -91,7 +91,8 @@ func TestMediumBase(t *testing.T) { if err != nil { t.Fatalf("Err: %v", err) } - th.assertActualEqualsExpected(m, `apiVersion: v1 + th.assertActualEqualsExpected(m, ` +apiVersion: v1 kind: Service metadata: annotations: @@ -210,7 +211,8 @@ spec: if err != nil { t.Fatalf("Err: %v", err) } - th.assertActualEqualsExpected(m, `apiVersion: v1 + th.assertActualEqualsExpected(m, ` +apiVersion: v1 data: app-init.ini: |2 diff --git a/pkg/target/baseandoverlaysmall_test.go b/pkg/target/baseandoverlaysmall_test.go index e2bc9d4ac..c38112ce3 100644 --- a/pkg/target/baseandoverlaysmall_test.go +++ b/pkg/target/baseandoverlaysmall_test.go @@ -64,7 +64,8 @@ func TestSmallBase(t *testing.T) { if err != nil { t.Fatalf("Err: %v", err) } - th.assertActualEqualsExpected(m, `apiVersion: v1 + th.assertActualEqualsExpected(m, ` +apiVersion: v1 kind: Service metadata: labels: @@ -112,7 +113,8 @@ patchesStrategicMerge: - deployment/deployment.yaml imageTags: - name: whatever - newTag: 1.8.0`) + newTag: 1.8.0 +`) th.writeF("/app/overlay/configmap/app.env", ` DB_USERNAME=admin @@ -134,7 +136,8 @@ spec: if err != nil { t.Fatalf("Err: %v", err) } - th.assertActualEqualsExpected(m, `apiVersion: v1 + th.assertActualEqualsExpected(m, ` +apiVersion: v1 kind: Service metadata: labels: diff --git a/pkg/target/customconfig_test.go b/pkg/target/customconfig_test.go new file mode 100644 index 000000000..9c0c6b0e3 --- /dev/null +++ b/pkg/target/customconfig_test.go @@ -0,0 +1,341 @@ +/* +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 target + +import ( + "testing" +) + +func makeBaseReferencingCustomConfig(th *KustTestHarness) { + th.writeK("/app/base", ` +namePrefix: x- +commonLabels: + app: myApp +vars: +- name: APRIL_DIET + objref: + kind: Giraffe + name: april + fieldref: + fieldpath: spec.diet +- name: KOKO_DIET + objref: + kind: Gorilla + name: koko + fieldref: + fieldpath: spec.diet +resources: +- giraffes.yaml +- gorilla.yaml +- animalPark.yaml +configurations: +- config/defaults.yaml +- config/custom.yaml +`) + th.writeF("/app/base/giraffes.yaml", ` +kind: Giraffe +metadata: + name: may +spec: + diet: acacia + location: SE +--- +kind: Giraffe +metadata: + name: april +spec: + diet: mimosa + location: NE +`) + th.writeF("/app/base/gorilla.yaml", ` +kind: Gorilla +metadata: + name: koko +spec: + diet: bambooshoots + location: SW +`) + th.writeF("/app/base/animalPark.yaml", ` +kind: AnimalPark +metadata: + name: sandiego +spec: + gorillaRef: + name: koko + giraffeRef: + name: april + food: + - "$(APRIL_DIET)" + - "$(KOKO_DIET)" +`) +} + +func TestCustomConfig(t *testing.T) { + th := NewKustTestHarness(t, "/app/base") + makeBaseReferencingCustomConfig(th) + th.writeDefaultConfigs("/app/base/config/defaults.yaml") + th.writeF("/app/base/config/custom.yaml", ` +nameReference: +- kind: Gorilla + fieldSpecs: + - kind: AnimalPark + path: spec/gorillaRef/name +- kind: Giraffe + fieldSpecs: + - kind: AnimalPark + path: spec/giraffeRef/name +varReference: +- path: spec/food + kind: AnimalPark +`) + m, err := th.makeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Err: %v", err) + } + th.assertActualEqualsExpected(m, ` +kind: AnimalPark +metadata: + labels: + app: myApp + name: x-sandiego +spec: + food: + - mimosa + - bambooshoots + giraffeRef: + name: x-april + gorillaRef: + name: x-koko +--- +kind: Giraffe +metadata: + labels: + app: myApp + name: x-april +spec: + diet: mimosa + location: NE +--- +kind: Giraffe +metadata: + labels: + app: myApp + name: x-may +spec: + diet: acacia + location: SE +--- +kind: Gorilla +metadata: + labels: + app: myApp + name: x-koko +spec: + diet: bambooshoots + location: SW +`) +} + +// TODO: this test demonstrates #658 +// The prefix "x-" is applied twice (search for x-x-sandiego), because the +// config from the base isn't properly merged with the config in the overlay. +func TestCustomConfigWithDefaultOverspecification(t *testing.T) { + th := NewKustTestHarness(t, "/app/base") + makeBaseReferencingCustomConfig(th) + th.writeDefaultConfigs("/app/base/config/defaults.yaml") + // Specifying namePrefix here conflicts with (is the same as) + // the defaults written above. + th.writeF("/app/base/config/custom.yaml", ` +namePrefix: +- path: metadata/name +nameReference: +- kind: Gorilla + fieldSpecs: + - kind: AnimalPark + path: spec/gorillaRef/name +- kind: Giraffe + fieldSpecs: + - kind: AnimalPark + path: spec/giraffeRef/name +varReference: +- path: spec/food + kind: AnimalPark +`) + m, err := th.makeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Err: %v", err) + } + th.assertActualEqualsExpected(m, ` +kind: AnimalPark +metadata: + labels: + app: myApp + name: x-x-sandiego +spec: + food: + - mimosa + - bambooshoots + giraffeRef: + name: x-x-april + gorillaRef: + name: x-x-koko +--- +kind: Giraffe +metadata: + labels: + app: myApp + name: x-x-april +spec: + diet: mimosa + location: NE +--- +kind: Giraffe +metadata: + labels: + app: myApp + name: x-x-may +spec: + diet: acacia + location: SE +--- +kind: Gorilla +metadata: + labels: + app: myApp + name: x-x-koko +spec: + diet: bambooshoots + location: SW +`) +} + +// TODO: Test demonstrates bug #605. +// The customization supplied in a base isn't available to the overlay. +func TestBug605(t *testing.T) { + th := NewKustTestHarness(t, "/app/overlay") + makeBaseReferencingCustomConfig(th) + th.writeDefaultConfigs("/app/base/config/defaults.yaml") + th.writeF("/app/base/config/custom.yaml", ` +nameReference: +- kind: Gorilla + fieldSpecs: + - kind: AnimalPark + path: spec/gorillaRef/name +- kind: Giraffe + fieldSpecs: + - kind: AnimalPark + path: spec/giraffeRef/name +varReference: +- path: spec/food + kind: AnimalPark +`) + th.writeK("/app/overlay", ` +namePrefix: o- +commonLabels: + movie: planetOfTheApes +patchesStrategicMerge: +- animalPark.yaml +resources: +- ursus.yaml +bases: +- ../base +`) + th.writeF("/app/overlay/ursus.yaml", ` +kind: Gorilla +metadata: + name: ursus +spec: + diet: heston + location: Arizona +`) + // The following replaces the gorillaRef in the AnimalPark. + th.writeF("/app/overlay/animalPark.yaml", ` +kind: AnimalPark +metadata: + name: sandiego +spec: + gorillaRef: + name: ursus +`) + + m, err := th.makeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Err: %v", err) + } + // Problems in the expected result: + // - The variables are not replaced in the "food" fields. + // - The name of the AnimalPark should be x-o-sandiego, since + // AnimalPark appears in the base. + // - The giraffe and gorilla name are incorrect in AnimalPark; + // they should be o-x-april and o-ursus respectively. The + // Gorilla ursus doesn't get an x because it's not in the base. + th.assertActualEqualsExpected(m, ` +kind: AnimalPark +metadata: + labels: + app: myApp + movie: planetOfTheApes + name: o-sandiego +spec: + food: + - $(APRIL_DIET) + - $(KOKO_DIET) + giraffeRef: + name: april + gorillaRef: + name: ursus +--- +kind: Giraffe +metadata: + labels: + app: myApp + movie: planetOfTheApes + name: o-x-april +spec: + diet: mimosa + location: NE +--- +kind: Giraffe +metadata: + labels: + app: myApp + movie: planetOfTheApes + name: o-x-may +spec: + diet: acacia + location: SE +--- +kind: Gorilla +metadata: + labels: + app: myApp + movie: planetOfTheApes + name: o-x-koko +spec: + diet: bambooshoots + location: SW +--- +kind: Gorilla +metadata: + labels: + movie: planetOfTheApes + name: o-ursus +spec: + diet: heston + location: Arizona +`) +} diff --git a/pkg/target/kusttarget.go b/pkg/target/kusttarget.go index ed61910b2..02a094709 100644 --- a/pkg/target/kusttarget.go +++ b/pkg/target/kusttarget.go @@ -97,9 +97,24 @@ func unmarshal(y []byte, o interface{}) error { return dec.Decode(o) } -// makeTransformerConfig returns a complete TransformerConfig object -// from either files or the default configs +// Maybe switch to the false path permanently (desired by #606), +// or expose this as a CLI flag. +const demandExplicitConfig = true + func makeTransformerConfig( + ldr ifc.Loader, paths []string) (*config.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) (*config.TransformerConfig, error) { if paths == nil || len(paths) == 0 { return config.NewFactory(nil).DefaultConfig(), nil @@ -107,6 +122,21 @@ func makeTransformerConfig( return config.NewFactory(ldr).FromFiles(paths) } +// mergeCustomConfigWithDefaults returns a merger of custom config, +// if any, with default config. +func mergeCustomConfigWithDefaults( + ldr ifc.Loader, paths []string) (*config.TransformerConfig, error) { + t1 := config.NewFactory(nil).DefaultConfig() + if len(paths) == 0 { + return t1, nil + } + t2, err := config.NewFactory(ldr).FromFiles(paths) + if err != nil { + return nil, err + } + return t1.Merge(t2), nil +} + // MakeCustomizedResMap creates a ResMap per kustomization instructions. // The Resources in the returned ResMap are fully customized. func (kt *KustTarget) MakeCustomizedResMap() (resmap.ResMap, error) { diff --git a/pkg/target/kusttarget_test.go b/pkg/target/kusttarget_test.go index 31e2b5c0e..ae4eec49d 100644 --- a/pkg/target/kusttarget_test.go +++ b/pkg/target/kusttarget_test.go @@ -74,12 +74,14 @@ secretGenerator: USER: "sleep 2" type: Opaque ` - deploymentContent = `apiVersion: apps/v1 + deploymentContent = ` +apiVersion: apps/v1 metadata: name: dply1 kind: Deployment ` - namespaceContent = `apiVersion: v1 + namespaceContent = ` +apiVersion: v1 kind: Namespace metadata: name: ns1 diff --git a/pkg/target/resourceconflict_test.go b/pkg/target/resourceconflict_test.go index 0002e0120..991805a03 100644 --- a/pkg/target/resourceconflict_test.go +++ b/pkg/target/resourceconflict_test.go @@ -79,7 +79,8 @@ func TestMultibasesNoConflict(t *testing.T) { if err != nil { t.Fatalf("Unexpected err: %v", err) } - th.assertActualEqualsExpected(m, `apiVersion: v1 + th.assertActualEqualsExpected(m, ` +apiVersion: v1 kind: ServiceAccount metadata: name: a-base-serviceaccount-suffix-suffixA diff --git a/pkg/target/utils_for_test.go b/pkg/target/utils_for_test.go index 4cafbb13e..d3fedce95 100644 --- a/pkg/target/utils_for_test.go +++ b/pkg/target/utils_for_test.go @@ -21,6 +21,7 @@ package target import ( "fmt" "path/filepath" + "sigs.k8s.io/kustomize/pkg/transformers/config/defaultconfig" "strings" "testing" @@ -83,6 +84,18 @@ func (th *KustTestHarness) fromMap(m map[string]interface{}) *resource.Resource return th.rf.RF().FromMap(m) } +func (th *KustTestHarness) writeDefaultConfigs(fName string) { + m := defaultconfig.GetDefaultFieldSpecsAsMap() + var content []byte + for _, tCfg := range m { + content = append(content, []byte(tCfg)...) + } + err := th.ldr.AddFile(fName, content) + if err != nil { + th.t.Fatalf("unable to add file %s", fName) + } +} + func tabToSpace(input string) string { var result []string for _, i := range input { @@ -115,12 +128,16 @@ func hint(a, b string) string { return "X" } -// Pretty printing of file differences. func (th *KustTestHarness) assertActualEqualsExpected( m resmap.ResMap, expected string) { if m == nil { th.t.Fatalf("Map should not be nil.") } + // Ignore leading linefeed in expected value + // to ease readability of tests. + if len(expected) > 0 && expected[0] == 10 { + expected = expected[1:] + } actual, err := m.EncodeAsYaml() if err != nil { th.t.Fatalf("Unexpected err: %v", err) @@ -130,6 +147,7 @@ func (th *KustTestHarness) assertActualEqualsExpected( } } +// Pretty printing of file differences. func (th *KustTestHarness) reportDiffAndFail(actual []byte, expected string) { sE, maxLen := convertToArray(expected) sA, _ := convertToArray(string(actual))