diff --git a/pkg/target/baseandoverlaymedium_test.go b/pkg/target/baseandoverlaymedium_test.go index 0ffd7d2b0..fe2bf19bb 100644 --- a/pkg/target/baseandoverlaymedium_test.go +++ b/pkg/target/baseandoverlaymedium_test.go @@ -211,6 +211,9 @@ spec: if err != nil { t.Fatalf("Err: %v", err) } + // TODO(#669): The name of the patched Deployment is + // test-infra-baseprefix-mungebot, retaining the base + // prefix (example of correct behavior). th.assertActualEqualsExpected(m, ` apiVersion: v1 data: diff --git a/pkg/target/baseandoverlaysmall_test.go b/pkg/target/baseandoverlaysmall_test.go index c38112ce3..a8c72b7e6 100644 --- a/pkg/target/baseandoverlaysmall_test.go +++ b/pkg/target/baseandoverlaysmall_test.go @@ -136,6 +136,9 @@ spec: if err != nil { t.Fatalf("Err: %v", err) } + // TODO(#669): The name of the patched Deployment is + // b-a-myDeployment, retaining the base prefix + // (example of correct behavior). th.assertActualEqualsExpected(m, ` apiVersion: v1 kind: Service diff --git a/pkg/target/crd_test.go b/pkg/target/crd_test.go index 68e79b2a7..f6d955134 100644 --- a/pkg/target/crd_test.go +++ b/pkg/target/crd_test.go @@ -293,7 +293,7 @@ spec: if err != nil { t.Fatalf("Err: %v", err) } - // TODO(#605): Some output here is wrong due to #605 + // TODO(#669): Bee's name should be "prod-x-bee", not "prod-bee". th.assertActualEqualsExpected(m, ` apiVersion: v1 data: @@ -308,9 +308,9 @@ metadata: name: prod-x-mykind spec: beeRef: - name: bee + name: prod-bee secretRef: - name: crdsecret + name: prod-x-crdsecret --- apiVersion: v1beta1 kind: Bee diff --git a/pkg/target/customconfig_test.go b/pkg/target/customconfig_test.go index f965f3789..daf616681 100644 --- a/pkg/target/customconfig_test.go +++ b/pkg/target/customconfig_test.go @@ -221,9 +221,7 @@ spec: `) } -// TODO: Test demonstrates bug #605. -// The customization supplied in a base isn't available to the overlay. -func TestBug605(t *testing.T) { +func TestFixedBug605_BaseCustomizationAvailableInOverlay(t *testing.T) { th := NewKustTestHarness(t, "/app/overlay") makeBaseReferencingCustomConfig(th) th.writeDefaultConfigs("/app/base/config/defaults.yaml") @@ -274,13 +272,8 @@ spec: 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. + // TODO(#669): The name of AnimalPark should be x-o-sandiego, + // not o-sandiego, since AnimalPark appears in the base. th.assertActualEqualsExpected(m, ` kind: AnimalPark metadata: @@ -290,12 +283,12 @@ metadata: name: o-sandiego spec: food: - - $(APRIL_DIET) - - $(KOKO_DIET) + - mimosa + - bambooshoots giraffeRef: - name: april + name: o-x-april gorillaRef: - name: ursus + name: o-ursus --- kind: Giraffe metadata: diff --git a/pkg/target/kusttarget.go b/pkg/target/kusttarget.go index f42449b29..d79791682 100644 --- a/pkg/target/kusttarget.go +++ b/pkg/target/kusttarget.go @@ -32,7 +32,6 @@ import ( "sigs.k8s.io/kustomize/pkg/ifc/transformer" interror "sigs.k8s.io/kustomize/pkg/internal/error" patchtransformer "sigs.k8s.io/kustomize/pkg/patch/transformer" - "sigs.k8s.io/kustomize/pkg/resid" "sigs.k8s.io/kustomize/pkg/resmap" "sigs.k8s.io/kustomize/pkg/resource" "sigs.k8s.io/kustomize/pkg/transformers" @@ -46,7 +45,6 @@ type KustTarget struct { ldr ifc.Loader fSys fs.FileSystem rFactory *resmap.Factory - tConfig *config.TransformerConfig tFactory transformer.Factory } @@ -73,16 +71,11 @@ func NewKustTarget( if len(msgs) > 0 { log.Printf(strings.Join(msgs, "\n")) } - tConfig, err := makeTransformerConfig(ldr, k.Configurations) - if err != nil { - return nil, err - } return &KustTarget{ kustomization: &k, ldr: ldr, fSys: fSys, rFactory: rFactory, - tConfig: tConfig, tFactory: tFactory, }, nil } @@ -141,7 +134,7 @@ func mergeCustomConfigWithDefaults( // MakeCustomizedResMap creates a ResMap per kustomization instructions. // The Resources in the returned ResMap are fully customized. func (kt *KustTarget) MakeCustomizedResMap() (resmap.ResMap, error) { - m, err := kt.loadCustomizedResMap() + ra, err := kt.accumulateTarget() if err != nil { return nil, err } @@ -150,23 +143,20 @@ func (kt *KustTarget) MakeCustomizedResMap() (resmap.ResMap, error) { // It changes only the Name field in the // resource held in the ResMap's value, not // the Name in the key in the ResMap. - err := kt.tFactory.MakeHashTransformer().Transform(m) + err := ra.Transform(kt.tFactory.MakeHashTransformer()) if err != nil { return nil, err } } // Given that names have changed (prefixs/suffixes added), // fix all the back references to those names. - if kt.tConfig.NameReference != nil { - err = transformers.NewNameReferenceTransformer( - kt.tConfig.NameReference).Transform(m) - if err != nil { - return nil, err - } + err = ra.FixBackReferences() + if err != nil { + return nil, err } // With all the back references fixed, it's OK to resolve Vars. - err = kt.doVarReplacement(m) - return m, err + err = ra.ResolveVars() + return ra.ResMap(), err } func (kt *KustTarget) shouldAddHashSuffixesToGeneratedResources() bool { @@ -174,31 +164,46 @@ func (kt *KustTarget) shouldAddHashSuffixesToGeneratedResources() bool { !kt.kustomization.GeneratorOptions.DisableNameSuffixHash } -func (kt *KustTarget) doVarReplacement(m resmap.ResMap) error { - vars, err := kt.getAllVars() - if err != nil { - return err - } - varMap, err := kt.resolveVars(m, vars) - if err != nil { - return err - } - return transformers.NewRefVarTransformer( - varMap, kt.tConfig.VarReference).Transform(m) -} - -// loadCustomizedResMap loads and customizes resources to build a ResMap. -func (kt *KustTarget) loadCustomizedResMap() (resmap.ResMap, error) { +// 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() ( + ra *ResAccumulator, err error) { + // TODO(monopole): Get rid of the KustomizationErrors accumulator. + // It's not consistently used, and complicates tests. errs := &interror.KustomizationErrors{} - result, err := kt.loadResMapFromBasesAndResources() + ra, errs = kt.accumulateBases() + resources, err := kt.rFactory.FromFiles( + kt.ldr, kt.kustomization.Resources) if err != nil { - errs.Append(errors.Wrap(err, "loadResMapFromBasesAndResources")) + errs.Append(errors.Wrap(err, "rawResources failed to read Resources")) + } + if len(errs.Get()) > 0 { + return ra, errs + } + err = ra.MergeResourcesWithErrorOnIdCollision(resources) + if err != nil { + errs.Append(errors.Wrap(err, "MergeResourcesWithErrorOnIdCollision")) + } + tConfig, err := makeTransformerConfig( + kt.ldr, kt.kustomization.Configurations) + if err != nil { + return nil, err + } + err = ra.MergeConfig(tConfig) + if err != nil { + errs.Append(errors.Wrap(err, "MergeConfig")) + } + err = ra.MergeVars(kt.kustomization.Vars) + if err != nil { + errs.Append(errors.Wrap(err, "MergeVars")) } crdTc, err := config.NewFactory(kt.ldr).LoadCRDs(kt.kustomization.Crds) if err != nil { errs.Append(errors.Wrap(err, "LoadCRDs")) } - kt.tConfig, err = kt.tConfig.Merge(crdTc) + err = ra.MergeConfig(crdTc) if err != nil { errs.Append(errors.Wrap(err, "merge CRDs")) } @@ -206,11 +211,10 @@ func (kt *KustTarget) loadCustomizedResMap() (resmap.ResMap, error) { if err != nil { errs.Append(errors.Wrap(err, "generateConfigMapsAndSecrets")) } - result, err = resmap.MergeWithOverride(result, resMap) + err = ra.MergeResourcesWithOverride(resMap) if err != nil { return nil, err } - patches, err := kt.rFactory.RF().SliceFromPatches( kt.ldr, kt.kustomization.PatchesStrategicMerge) if err != nil { @@ -219,30 +223,15 @@ func (kt *KustTarget) loadCustomizedResMap() (resmap.ResMap, error) { if len(errs.Get()) > 0 { return nil, errs } - - var r []transformers.Transformer - t, err := kt.newTransformer(patches) + t, err := kt.newTransformer(patches, ra.tConfig) if err != nil { return nil, err } - r = append(r, t) - t, err = patchtransformer.NewPatchJson6902Factory(kt.ldr). - MakePatchJson6902Transformer(kt.kustomization.PatchesJson6902) + err = ra.Transform(t) if err != nil { return nil, err } - r = append(r, t) - t, err = transformers.NewImageTagTransformer(kt.kustomization.ImageTags) - if err != nil { - return nil, err - } - r = append(r, t) - - err = transformers.NewMultiTransformer(r).Transform(result) - if err != nil { - return nil, err - } - return result, nil + return ra, nil } func (kt *KustTarget) generateConfigMapsAndSecrets( @@ -261,75 +250,48 @@ func (kt *KustTarget) generateConfigMapsAndSecrets( return resmap.MergeWithErrorOnIdCollision(cms, secrets) } -// Gets Bases and Resources as advertised. -func (kt *KustTarget) loadResMapFromBasesAndResources() (resmap.ResMap, error) { - bases, errs := kt.loadCustomizedBases() - resources, err := kt.rFactory.FromFiles( - kt.ldr, kt.kustomization.Resources) - if err != nil { - errs.Append(errors.Wrap(err, "rawResources failed to read Resources")) - } - if len(errs.Get()) > 0 { - return nil, errs - } - return resmap.MergeWithErrorOnIdCollision(resources, bases) -} +// accumulateBases returns a new ResAccumulator +// holding customized resources and the data/rules +// used to customized them from only the _bases_ +// of this KustTarget. +func (kt *KustTarget) accumulateBases() ( + ra *ResAccumulator, errs *interror.KustomizationErrors) { + errs = &interror.KustomizationErrors{} + ra = MakeEmptyAccumulator() -// Loop through the Bases of this kustomization recursively loading resources. -// Combine into one ResMap, demanding unique Ids for each resource. -func (kt *KustTarget) loadCustomizedBases() ( - resmap.ResMap, *interror.KustomizationErrors) { - var list []resmap.ResMap - errs := &interror.KustomizationErrors{} for _, path := range kt.kustomization.Bases { ldr, err := kt.ldr.New(path) if err != nil { errs.Append(errors.Wrap(err, "couldn't make loader for "+path)) continue } - target, err := NewKustTarget( - ldr, kt.fSys, - kt.rFactory, kt.tFactory) - if err != nil { - errs.Append(errors.Wrap(err, "couldn't make target for "+path)) - continue - } - resMap, err := target.loadCustomizedResMap() - if err != nil { - errs.Append(errors.Wrap(err, "SemiResources")) - continue - } - ldr.Cleanup() - list = append(list, resMap) - } - result, err := resmap.MergeWithErrorOnIdCollision(list...) - if err != nil { - errs.Append(errors.Wrap(err, "Merge failed")) - } - return result, errs -} - -func (kt *KustTarget) loadBasesAsKustTargets() ([]*KustTarget, error) { - var result []*KustTarget - for _, path := range kt.kustomization.Bases { - ldr, err := kt.ldr.New(path) - if err != nil { - return nil, err - } - target, err := NewKustTarget( + subKt, err := NewKustTarget( ldr, kt.fSys, kt.rFactory, kt.tFactory) if err != nil { - return nil, err + errs.Append(errors.Wrap(err, "couldn't make target for "+path)) + ldr.Cleanup() + continue } - result = append(result, target) + subRa, err := subKt.accumulateTarget() + if err != nil { + errs.Append(errors.Wrap(err, "accumulateTarget")) + ldr.Cleanup() + continue + } + err = ra.MergeAccumulator(subRa) + if err != nil { + errs.Append(errors.Wrap(err, path)) + } + ldr.Cleanup() } - return result, nil + return ra, errs } // newTransformer makes a Transformer that does a collection // of object transformations. func (kt *KustTarget) newTransformer( - patches []*resource.Resource) (transformers.Transformer, error) { + patches []*resource.Resource, tConfig *config.TransformerConfig) ( + transformers.Transformer, error) { var r []transformers.Transformer t, err := kt.tFactory.MakePatchTransformer(patches, kt.rFactory.RF()) if err != nil { @@ -337,24 +299,35 @@ func (kt *KustTarget) newTransformer( } r = append(r, t) r = append(r, transformers.NewNamespaceTransformer( - string(kt.kustomization.Namespace), kt.tConfig.NameSpace)) + string(kt.kustomization.Namespace), tConfig.NameSpace)) t, err = transformers.NewNamePrefixSuffixTransformer( string(kt.kustomization.NamePrefix), string(kt.kustomization.NameSuffix), - kt.tConfig.NamePrefix, + tConfig.NamePrefix, ) if err != nil { return nil, err } r = append(r, t) t, err = transformers.NewLabelsMapTransformer( - kt.kustomization.CommonLabels, kt.tConfig.CommonLabels) + kt.kustomization.CommonLabels, tConfig.CommonLabels) if err != nil { return nil, err } r = append(r, t) t, err = transformers.NewAnnotationsMapTransformer( - kt.kustomization.CommonAnnotations, kt.tConfig.CommonAnnotations) + kt.kustomization.CommonAnnotations, tConfig.CommonAnnotations) + if err != nil { + return nil, err + } + r = append(r, t) + t, err = patchtransformer.NewPatchJson6902Factory(kt.ldr). + MakePatchJson6902Transformer(kt.kustomization.PatchesJson6902) + if err != nil { + return nil, err + } + r = append(r, t) + t, err = transformers.NewImageTagTransformer(kt.kustomization.ImageTags) if err != nil { return nil, err } @@ -362,78 +335,6 @@ func (kt *KustTarget) newTransformer( return transformers.NewMultiTransformer(r), nil } -// resolveVars returns a map of Var names to their final values. -// The values are strings intended for substitution wherever -// the $(var.Name) occurs. -func (kt *KustTarget) resolveVars( - m resmap.ResMap, vars map[string]types.Var) (map[string]string, error) { - result := map[string]string{} - for _, v := range vars { - id := resid.NewResId(v.ObjRef.GVK(), v.ObjRef.Name) - if r, found := m.DemandOneMatchForId(id); found { - s, err := r.GetFieldValue(v.FieldRef.FieldPath) - if err != nil { - return nil, fmt.Errorf("field path err for var: %+v", v) - } - result[v.Name] = s - } else { - log.Printf("couldn't resolve var: %v", v) - } - } - return result, nil -} - -type ErrVarCollision struct { - name string - path string -} - -func (e ErrVarCollision) Error() string { - return fmt.Sprintf( - "var %s in %s defined in some other kustomization", - e.name, e.path) -} - -// getAllVars returns a map of Var names to Var instances, pulled from -// this kustomization and the kustomizations it depends on. -// An error is returned on Var name collision. -func (kt *KustTarget) getAllVars() (map[string]types.Var, error) { - result, err := kt.getVarsFromBases() - if err != nil { - return nil, err - } - for _, v := range kt.kustomization.Vars { - v.Defaulting() - if _, oops := result[v.Name]; oops { - return nil, ErrVarCollision{v.Name, kt.ldr.Root()} - } - result[v.Name] = v - } - return result, nil -} - -func (kt *KustTarget) getVarsFromBases() (map[string]types.Var, error) { - targets, err := kt.loadBasesAsKustTargets() - if err != nil { - return nil, err - } - result := make(map[string]types.Var) - for _, subKt := range targets { - vars, err := subKt.getAllVars() - if err != nil { - return nil, err - } - subKt.ldr.Cleanup() - for k, v := range vars { - if _, oops := result[k]; oops { - return nil, ErrVarCollision{v.Name, subKt.ldr.Root()} - } - result[k] = v - } - } - return result, nil -} - func loadKustFile(ldr ifc.Loader) ([]byte, error) { for _, kf := range []string{ constants.KustomizationFileName, diff --git a/pkg/target/kusttarget_test.go b/pkg/target/kusttarget_test.go index ae4eec49d..b23dafef1 100644 --- a/pkg/target/kusttarget_test.go +++ b/pkg/target/kusttarget_test.go @@ -18,8 +18,8 @@ package target import ( "encoding/base64" + "fmt" "reflect" - "sort" "strings" "testing" @@ -346,28 +346,21 @@ vars: name: heron apiVersion: v300 `) - vars, err := th.makeKustTarget().getAllVars() + ra, err := th.makeKustTarget().accumulateTarget() if err != nil { t.Fatalf("Err: %v", err) } + vars := ra.varSet.Set() if len(vars) != 2 { t.Fatalf("unexpected size %d", len(vars)) } - for i, k := range sortedKeys(vars)[:2] { - if !reflect.DeepEqual(vars[k], someVars[i]) { - t.Fatalf("unexpected var[%d]:\n %v\n %v", i, vars[k], someVars[i]) + for i := range vars[:2] { + if !reflect.DeepEqual(vars[i], someVars[i]) { + t.Fatalf("unexpected var[%d]:\n %v\n %v", i, vars[i], someVars[i]) } } } -func sortedKeys(m map[string]types.Var) (result []string) { - for k := range m { - result = append(result, k) - } - sort.Strings(result) - return -} - func TestGetAllVarsNested(t *testing.T) { th := NewKustTestHarness(t, "/app/overlays/o2") th.writeK("/app/base", ` @@ -403,16 +396,20 @@ vars: bases: - ../o1 `) - vars, err := th.makeKustTarget().getAllVars() + ra, err := th.makeKustTarget().accumulateTarget() if err != nil { t.Fatalf("Err: %v", err) } + vars := ra.varSet.Set() if len(vars) != 4 { - t.Fatalf("unexpected size %d", len(vars)) + for i, v := range vars { + fmt.Printf("%v: %v\n", i, v) + } + t.Fatalf("expected 4 vars, got %d", len(vars)) } - for i, k := range sortedKeys(vars) { - if !reflect.DeepEqual(vars[k], someVars[i]) { - t.Fatalf("unexpected var[%d]:\n %v\n %v", i, vars[k], someVars[i]) + for i := range vars { + if !reflect.DeepEqual(vars[i], someVars[i]) { + t.Fatalf("unexpected var[%d]:\n %v\n %v", i, vars[i], someVars[i]) } } } @@ -452,11 +449,12 @@ vars: bases: - ../o1 `) - _, err := th.makeKustTarget().getAllVars() + _, err := th.makeKustTarget().accumulateTarget() if err == nil { t.Fatalf("expected var collision") } - if _, ok := err.(ErrVarCollision); !ok { + if !strings.Contains(err.Error(), + "var AWARD already encountered") { t.Fatalf("unexpected error: %v", err) } } diff --git a/pkg/target/resaccumulator.go b/pkg/target/resaccumulator.go new file mode 100644 index 000000000..41e448b8d --- /dev/null +++ b/pkg/target/resaccumulator.go @@ -0,0 +1,134 @@ +/* +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 ( + "fmt" + "log" + "sigs.k8s.io/kustomize/pkg/resid" + "sigs.k8s.io/kustomize/pkg/resmap" + "sigs.k8s.io/kustomize/pkg/transformers" + "sigs.k8s.io/kustomize/pkg/transformers/config" + "sigs.k8s.io/kustomize/pkg/types" +) + +// ResAccumulator accumulates resources and the rules +// used to customize those resources. +// TODO(monopole): Move to "accumulator" package and make members private. +// This will make a better separation between KustTarget, which should +// be mainly concerned with data loading, and this class, which could +// become the home of all transformation data and logic. +type ResAccumulator struct { + resMap resmap.ResMap + tConfig *config.TransformerConfig + varSet types.VarSet +} + +func MakeEmptyAccumulator() *ResAccumulator { + ra := &ResAccumulator{} + ra.resMap = make(resmap.ResMap) + ra.tConfig = &config.TransformerConfig{} + ra.varSet = types.VarSet{} + return ra +} + +// ResMap returns a copy of the internal resMap. +func (ra *ResAccumulator) ResMap() resmap.ResMap { + result := make(resmap.ResMap) + for k, v := range ra.resMap { + result[k] = v + } + return result +} + +func (ra *ResAccumulator) MergeResourcesWithErrorOnIdCollision( + resources resmap.ResMap) (err error) { + ra.resMap, err = resmap.MergeWithErrorOnIdCollision( + resources, ra.resMap) + return err +} + +func (ra *ResAccumulator) MergeResourcesWithOverride( + resources resmap.ResMap) (err error) { + ra.resMap, err = resmap.MergeWithOverride( + ra.resMap, resources) + return err +} + +func (ra *ResAccumulator) MergeConfig( + tConfig *config.TransformerConfig) (err error) { + ra.tConfig, err = ra.tConfig.Merge(tConfig) + return err +} + +func (ra *ResAccumulator) MergeVars(incoming []types.Var) error { + return ra.varSet.MergeSlice(incoming) +} + +func (ra *ResAccumulator) MergeAccumulator(other *ResAccumulator) (err error) { + err = ra.MergeResourcesWithErrorOnIdCollision(other.resMap) + if err != nil { + return err + } + err = ra.MergeConfig(other.tConfig) + if err != nil { + return err + } + return ra.varSet.MergeSet(&other.varSet) +} + +// makeVarReplacementMap returns a map of Var names to +// their final values. The values are strings intended +// for substitution wherever the $(var.Name) occurs. +func (ra *ResAccumulator) makeVarReplacementMap() (map[string]string, error) { + result := map[string]string{} + for _, v := range ra.varSet.Set() { + id := resid.NewResId(v.ObjRef.GVK(), v.ObjRef.Name) + if r, found := ra.resMap.DemandOneMatchForId(id); found { + s, err := r.GetFieldValue(v.FieldRef.FieldPath) + if err != nil { + return nil, fmt.Errorf("field path err for var: %+v", v) + } + result[v.Name] = s + } else { + // Should this be an error? + log.Printf("var %v defined but not used", v) + } + } + return result, nil +} + +func (ra *ResAccumulator) Transform(t transformers.Transformer) error { + return t.Transform(ra.resMap) +} + +func (ra *ResAccumulator) ResolveVars() error { + replacementMap, err := ra.makeVarReplacementMap() + if err != nil { + return err + } + return ra.Transform(transformers.NewRefVarTransformer( + replacementMap, ra.tConfig.VarReference)) +} + +func (ra *ResAccumulator) FixBackReferences() (err error) { + if ra.tConfig.NameReference == nil { + return nil + } + return ra.Transform(transformers.NewNameReferenceTransformer( + ra.tConfig.NameReference)) +} diff --git a/pkg/target/resaccumulator_test.go b/pkg/target/resaccumulator_test.go new file mode 100644 index 000000000..0d7c7036f --- /dev/null +++ b/pkg/target/resaccumulator_test.go @@ -0,0 +1,132 @@ +/* +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 ( + "strings" + "testing" + + "sigs.k8s.io/kustomize/k8sdeps/kunstruct" + "sigs.k8s.io/kustomize/pkg/gvk" + "sigs.k8s.io/kustomize/pkg/resid" + "sigs.k8s.io/kustomize/pkg/resmap" + "sigs.k8s.io/kustomize/pkg/resource" + "sigs.k8s.io/kustomize/pkg/transformers/config" + "sigs.k8s.io/kustomize/pkg/types" +) + +func TestResolveVars(t *testing.T) { + ra := MakeEmptyAccumulator() + err := ra.MergeConfig(config.NewFactory(nil).DefaultConfig()) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + rf := resource.NewFactory( + kunstruct.NewKunstructuredFactoryImpl()) + ra.MergeResourcesWithErrorOnIdCollision(resmap.ResMap{ + resid.NewResId( + gvk.Gvk{Group: "apps", Version: "v1", Kind: "Deployment"}, + "deploy1"): rf.FromMap( + map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "deploy1", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "command": []interface{}{ + "myserver", + "--somebackendService $(FOO)", + "--yetAnother $(BAR)", + }, + }, + }, + }, + }, + }, + }), + resid.NewResId( + gvk.Gvk{Version: "v1", Kind: "Service"}, + "backendOne"): rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "Service", + "metadata": map[string]interface{}{ + "name": "backendOne", + }, + }), + resid.NewResId( + gvk.Gvk{Version: "v1", Kind: "Service"}, + "backendTwo"): rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "Service", + "metadata": map[string]interface{}{ + "name": "backendTwo", + }, + }), + }) + err = ra.MergeVars([]types.Var{ + { + Name: "FOO", + ObjRef: types.Target{ + Gvk: gvk.Gvk{Version: "v1", Kind: "Service"}, + Name: "backendOne"}, + }, { + Name: "BAR", + ObjRef: types.Target{ + Gvk: gvk.Gvk{Version: "v1", Kind: "Service"}, + Name: "backendTwo"}, + }, + }) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + err = ra.ResolveVars() + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + c := getCommand(find("deploy1", ra.ResMap())) + if c != "myserver --somebackendService backendOne --yetAnother backendTwo" { + t.Fatalf("unexpected command: %s", c) + } +} + +func find(name string, resMap resmap.ResMap) *resource.Resource { + for k, v := range resMap { + if k.Name() == name { + return v + } + } + return nil +} + +// Assumes arg is a deployment, returns the command of first container. +func getCommand(r *resource.Resource) string { + var m map[string]interface{} + var c []interface{} + m, _ = r.Map()["spec"].(map[string]interface{}) + m, _ = m["template"].(map[string]interface{}) + m, _ = m["spec"].(map[string]interface{}) + c, _ = m["containers"].([]interface{}) + m, _ = c[0].(map[string]interface{}) + return strings.Join(m["command"].([]string), " ") +} diff --git a/pkg/types/var.go b/pkg/types/var.go index cda2c2f55..4be30078b 100644 --- a/pkg/types/var.go +++ b/pkg/types/var.go @@ -17,11 +17,15 @@ limitations under the License. package types import ( + "fmt" + "sort" "strings" "sigs.k8s.io/kustomize/pkg/gvk" ) +const defaultFieldPath = "metadata.name" + // Var represents a variable whose value will be sourced // from a field in a Kubernetes object. type Var struct { @@ -38,7 +42,7 @@ type Var struct { // FieldRef refers to the field of the object referred to by // ObjRef whose value will be extracted for use in // replacing $(FOO). - // If unspecified, this defaults to fieldPath: metadata.name + // If unspecified, this defaults to fieldPath: $defaultFieldPath FieldRef FieldSelector `json:"fieldref,omitempty" yaml:"fieldref,omitempty"` } @@ -52,20 +56,76 @@ type Target struct { Name string `json:"name" yaml:"name"` } -// FieldSelector contains the fieldPath to an object field, such as metadata.name +// FieldSelector contains the fieldPath to an object field. // This struct is added to keep the backward compatibility of using ObjectFieldSelector // for Var.FieldRef type FieldSelector struct { FieldPath string `json:"fieldPath,omitempty" yaml:"fieldPath,omitempty"` } -// Defaulting sets reference to field used by default. -func (v *Var) Defaulting() { +// defaulting sets reference to field used by default. +func (v *Var) defaulting() { if v.FieldRef.FieldPath == "" { - v.FieldRef.FieldPath = "metadata.name" + v.FieldRef.FieldPath = defaultFieldPath } } +// VarSet is a slice of Vars where no var.Name is repeated. +type VarSet struct { + set []Var +} + +// Set returns the var set. +func (vs *VarSet) Set() []Var { + return vs.set +} + +// MergeSet absorbs other vars with error on name collision. +func (vs *VarSet) MergeSet(incoming *VarSet) error { + return vs.MergeSlice(incoming.set) +} + +// MergeSlice absorbs other vars with error on name collision. +// Empty fields in incoming vars are defaulted. +func (vs *VarSet) MergeSlice(incoming []Var) error { + for _, v := range incoming { + if vs.Contains(v) { + return fmt.Errorf( + "var %s already encountered", v.Name) + } + v.defaulting() + vs.insert(v) + } + return nil +} + +func (vs *VarSet) insert(v Var) { + index := sort.Search( + len(vs.set), + func(i int) bool { return vs.set[i].Name > v.Name }) + // make room + vs.set = append(vs.set, Var{}) + // shift right at index. + // copy will not increase size of destination. + copy(vs.set[index+1:], vs.set[index:]) + vs.set[index] = v +} + +// Contains is true if the set has the other var. +func (vs *VarSet) Contains(other Var) bool { + return vs.Get(other.Name) != nil +} + +// Get returns the var with the given name, else nil. +func (vs *VarSet) Get(name string) *Var { + for _, v := range vs.set { + if v.Name == name { + return &v + } + } + return nil +} + // GVK returns the Gvk object in Target func (t *Target) GVK() gvk.Gvk { if t.APIVersion == "" { diff --git a/pkg/types/var_test.go b/pkg/types/var_test.go index ee2b7205a..f48c5e1cf 100644 --- a/pkg/types/var_test.go +++ b/pkg/types/var_test.go @@ -17,10 +17,12 @@ limitations under the License. package types import ( - "gopkg.in/yaml.v2" "reflect" - "sigs.k8s.io/kustomize/pkg/gvk" + "strings" "testing" + + "gopkg.in/yaml.v2" + "sigs.k8s.io/kustomize/pkg/gvk" ) func TestGVK(t *testing.T) { @@ -79,8 +81,73 @@ func TestDefaulting(t *testing.T) { Name: "my-secret", }, } - v.Defaulting() - if v.FieldRef.FieldPath != "metadata.name" { - t.Fatalf("var defaulting doesn't behave correctly.\n expected metadata.name, but got %v", v.FieldRef.FieldPath) + v.defaulting() + if v.FieldRef.FieldPath != defaultFieldPath { + t.Fatalf("expected %s, got %v", + defaultFieldPath, v.FieldRef.FieldPath) + } +} + +func TestVarSet(t *testing.T) { + set := &VarSet{} + vars := []Var{ + { + Name: "SHELLVARS", + ObjRef: Target{ + APIVersion: "v7", + Gvk: gvk.Gvk{Kind: "ConfigMap"}, + Name: "bash"}, + }, + { + Name: "BACKEND", + ObjRef: Target{ + APIVersion: "v7", + Gvk: gvk.Gvk{Kind: "Deployment"}, + Name: "myTiredBackend"}, + }, + { + Name: "AWARD", + ObjRef: Target{ + APIVersion: "v7", + Gvk: gvk.Gvk{Kind: "Service"}, + Name: "nobelPrize"}, + FieldRef: FieldSelector{FieldPath: "some.arbitrary.path"}, + }, + } + err := set.MergeSlice(vars) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + for _, v := range vars { + if !set.Contains(v) { + t.Fatalf("set %v should contain var %v", set.Set(), v) + } + } + set2 := &VarSet{} + err = set2.MergeSet(set) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + err = set2.MergeSlice(vars) + if err == nil { + t.Fatalf("expected err") + } + if !strings.Contains(err.Error(), "var SHELLVARS already encountered") { + t.Fatalf("unexpected err: %v", err) + } + v := set2.Get("BACKEND") + if v == nil { + t.Fatalf("expected var") + } + // Confirm defaulting. + if v.FieldRef.FieldPath != defaultFieldPath { + t.Fatalf("unexpected field path: %v", v.FieldRef.FieldPath) + } + // Confirm sorting. + names := set2.Set() + if names[0].Name != "AWARD" || + names[1].Name != "BACKEND" || + names[2].Name != "SHELLVARS" { + t.Fatalf("unexpected order in : %v", names) } }