From 4454edc14cd734fc1a578c97b1b3da178db2eb45 Mon Sep 17 00:00:00 2001 From: Paul Martin Date: Wed, 4 Dec 2019 18:30:39 -0800 Subject: [PATCH 1/9] Implement "kind: KustomizationPatch" to support composition of Kustomize files --- api/internal/target/kusttarget.go | 42 ++-- api/krusty/kustomizationpatch_test.go | 334 ++++++++++++++++++++++++++ api/types/kustomization.go | 11 +- 3 files changed, 367 insertions(+), 20 deletions(-) create mode 100644 api/krusty/kustomizationpatch_test.go diff --git a/api/internal/target/kusttarget.go b/api/internal/target/kusttarget.go index f99cd0f3b..257d08922 100644 --- a/api/internal/target/kusttarget.go +++ b/api/internal/target/kusttarget.go @@ -155,8 +155,12 @@ func (kt *KustTarget) addHashesToNames( // not yet fixed. func (kt *KustTarget) AccumulateTarget() ( ra *accumulator.ResAccumulator, err error) { - ra = accumulator.MakeEmptyAccumulator() - err = kt.accumulateResources(ra, kt.kustomization.Resources) + return kt.accumulateTarget(accumulator.MakeEmptyAccumulator()) +} + +func (kt *KustTarget) accumulateTarget(ra *accumulator.ResAccumulator) ( + resRa *accumulator.ResAccumulator, err error) { + ra, err = kt.accumulateResources(ra, kt.kustomization.Resources) if err != nil { return nil, errors.Wrap(err, "accumulating resources") } @@ -224,7 +228,7 @@ func (kt *KustTarget) runGenerators( func (kt *KustTarget) configureExternalGenerators() ([]resmap.Generator, error) { ra := accumulator.MakeEmptyAccumulator() - err := kt.accumulateResources(ra, kt.kustomization.Generators) + ra, err := kt.accumulateResources(ra, kt.kustomization.Generators) if err != nil { return nil, err } @@ -250,7 +254,7 @@ func (kt *KustTarget) runTransformers(ra *accumulator.ResAccumulator) error { func (kt *KustTarget) configureExternalTransformers() ([]resmap.Transformer, error) { ra := accumulator.MakeEmptyAccumulator() - err := kt.accumulateResources(ra, kt.kustomization.Transformers) + ra, err := kt.accumulateResources(ra, kt.kustomization.Transformers) if err != nil { return nil, err } @@ -260,44 +264,52 @@ func (kt *KustTarget) configureExternalTransformers() ([]resmap.Transformer, err // accumulateResources fills the given resourceAccumulator // with resources read from the given list of paths. func (kt *KustTarget) accumulateResources( - ra *accumulator.ResAccumulator, paths []string) error { + ra *accumulator.ResAccumulator, paths []string) (*accumulator.ResAccumulator, error) { for _, path := range paths { // try loading resource as file then as base (directory or git repository) if errF := kt.accumulateFile(ra, path); errF != nil { ldr, errL := kt.ldr.New(path) if errL != nil { - return fmt.Errorf("accumulateFile %q, loader.New %q", errF, errL) + return nil, fmt.Errorf("accumulateFile %q, loader.New %q", errF, errL) } - errD := kt.accumulateDirectory(ra, ldr) + var errD error + ra, errD = kt.accumulateDirectory(ra, ldr) if errD != nil { - return fmt.Errorf("accumulateFile %q, accumulateDirector: %q", errF, errD) + return nil, fmt.Errorf("accumulateFile %q, accumulateDirector: %q", errF, errD) } } } - return nil + return ra, nil } func (kt *KustTarget) accumulateDirectory( - ra *accumulator.ResAccumulator, ldr ifc.Loader) error { + ra *accumulator.ResAccumulator, ldr ifc.Loader) (*accumulator.ResAccumulator, error) { defer ldr.Cleanup() subKt := NewKustTarget( ldr, kt.validator, kt.rFactory, kt.tFactory, kt.pLdr) err := subKt.Load() if err != nil { - return errors.Wrapf( + return nil, errors.Wrapf( err, "couldn't make target for path '%s'", ldr.Root()) } - subRa, err := subKt.AccumulateTarget() + + var subRa *accumulator.ResAccumulator + if subKt.kustomization.Kind == types.KustomizationPatchKind { + subRa, err = subKt.accumulateTarget(ra) + ra = accumulator.MakeEmptyAccumulator() + } else { + subRa, err = subKt.AccumulateTarget() + } if err != nil { - return errors.Wrapf( + return nil, errors.Wrapf( err, "recursed accumulation of path '%s'", ldr.Root()) } err = ra.MergeAccumulator(subRa) if err != nil { - return errors.Wrapf( + return nil, errors.Wrapf( err, "recursed merging from path '%s'", ldr.Root()) } - return nil + return ra, nil } func (kt *KustTarget) accumulateFile( diff --git a/api/krusty/kustomizationpatch_test.go b/api/krusty/kustomizationpatch_test.go new file mode 100644 index 000000000..362115b40 --- /dev/null +++ b/api/krusty/kustomizationpatch_test.go @@ -0,0 +1,334 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package krusty_test + +import ( + "testing" + + kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" +) + +// Test kustomization.yaml files with kind: KustomizationPatch +func writeKustomizationPatchBase(th kusttest_test.Harness) { + th.WriteK("/app/base", ` +resources: +- deploy.yaml +configMapGenerator: +- name: my-configmap + literals: + - testValue=1 + - otherValue=10 +`) + th.WriteF("/app/base/deploy.yaml", ` +apiVersion: v1 +kind: Deployment +metadata: + name: storefront +spec: + replicas: 1 +`) +} + +func writeKustomizationPatchPatch(th kusttest_test.Harness) { + th.WriteF("/app/patch/kustomization.yaml", ` +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: KustomizationPatch +namePrefix: patched- +replicas: +- name: storefront + count: 3 +resources: + - stub.yaml +configMapGenerator: +- name: my-configmap + behavior: merge + literals: + - testValue=2 + - patchValue=5 +`) + th.WriteF("/app/patch/stub.yaml", ` +apiVersion: v1 +kind: Deployment +metadata: + name: stub +spec: + replicas: 1 +`) +} + +func writeKustomizationPatchProd(th kusttest_test.Harness) { + th.WriteK("/app/prod", ` +resources: +- ../base +- ../patch +- db +`) + th.WriteF("/app/prod/db", ` +apiVersion: v1 +kind: Deployment +metadata: + name: db +spec: + type: Logical +`) +} + +// KustomizationPatch are inserted into the resource hierarchy as the parent of those +// resources that come before it in the resources list of the parent Kustomization. +func TestBasicKustomizationPatch(t *testing.T) { + th := kusttest_test.MakeHarness(t) + writeKustomizationPatchBase(th) + writeKustomizationPatchPatch(th) + writeKustomizationPatchProd(th) + m := th.Run("/app/prod", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: v1 +kind: Deployment +metadata: + name: patched-storefront +spec: + replicas: 3 +--- +apiVersion: v1 +data: + otherValue: "10" + patchValue: "5" + testValue: "2" +kind: ConfigMap +metadata: + annotations: {} + labels: {} + name: patched-my-configmap-5g55h28cdh +--- +apiVersion: v1 +kind: Deployment +metadata: + name: patched-stub +spec: + replicas: 1 +--- +apiVersion: v1 +kind: Deployment +metadata: + name: db +spec: + type: Logical +`) +} + +func TestMultipleKustomizationPatches(t *testing.T) { + th := kusttest_test.MakeHarness(t) + writeKustomizationPatchBase(th) + writeKustomizationPatchPatch(th) + writeKustomizationPatchProd(th) + th.WriteF("/app/additionalpatch/kustomization.yaml", ` +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: KustomizationPatch +configMapGenerator: +- name: my-configmap + behavior: merge + literals: + - otherValue=9 +`) + th.WriteK("/app/prod", ` +resources: +- ../base +- ../patch +- ../additionalpatch +- db +`) + m := th.Run("/app/prod", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: v1 +kind: Deployment +metadata: + name: patched-storefront +spec: + replicas: 3 +--- +apiVersion: v1 +data: + otherValue: "9" + patchValue: "5" + testValue: "2" +kind: ConfigMap +metadata: + annotations: {} + labels: {} + name: patched-my-configmap-9fddc87cdk +--- +apiVersion: v1 +kind: Deployment +metadata: + name: patched-stub +spec: + replicas: 1 +--- +apiVersion: v1 +kind: Deployment +metadata: + name: db +spec: + type: Logical +`) +} + +func TestNestedKustomizationPatches(t *testing.T) { + th := kusttest_test.MakeHarness(t) + writeKustomizationPatchBase(th) + writeKustomizationPatchPatch(th) + writeKustomizationPatchProd(th) + th.WriteF("/app/additionalpatch/kustomization.yaml", ` +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: KustomizationPatch +resources: +- ../patch +configMapGenerator: +- name: my-configmap + behavior: merge + literals: + - otherValue=9 +`) + th.WriteK("/app/prod", ` +resources: +- ../base +- ../additionalpatch +- db +`) + m := th.Run("/app/prod", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: v1 +kind: Deployment +metadata: + name: patched-storefront +spec: + replicas: 3 +--- +apiVersion: v1 +data: + otherValue: "9" + patchValue: "5" + testValue: "2" +kind: ConfigMap +metadata: + annotations: {} + labels: {} + name: patched-my-configmap-9fddc87cdk +--- +apiVersion: v1 +kind: Deployment +metadata: + name: patched-stub +spec: + replicas: 1 +--- +apiVersion: v1 +kind: Deployment +metadata: + name: db +spec: + type: Logical +`) +} + +// If a patch sets a name prefix on a base, then that base can also be separately included +// without being affected by the patch in another branch of the resource tree +func TestBasicKustomizationPatchWithRepeatedBase(t *testing.T) { + th := kusttest_test.MakeHarness(t) + writeKustomizationPatchBase(th) + writeKustomizationPatchPatch(th) + writeKustomizationPatchProd(th) + th.WriteK("/app/repeated", ` +resources: +- ../base +- ../prod +`) + m := th.Run("/app/repeated", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: v1 +kind: Deployment +metadata: + name: storefront +spec: + replicas: 1 +--- +apiVersion: v1 +data: + otherValue: "10" + testValue: "1" +kind: ConfigMap +metadata: + name: my-configmap-7k9t4h74f8 +--- +apiVersion: v1 +kind: Deployment +metadata: + name: patched-storefront +spec: + replicas: 3 +--- +apiVersion: v1 +data: + otherValue: "10" + patchValue: "5" + testValue: "2" +kind: ConfigMap +metadata: + annotations: {} + labels: {} + name: patched-my-configmap-5g55h28cdh +--- +apiVersion: v1 +kind: Deployment +metadata: + name: patched-stub +spec: + replicas: 1 +--- +apiVersion: v1 +kind: Deployment +metadata: + name: db +spec: + type: Logical +`) +} + +func TestApplyingKustomizationPatchDirectlySameAsKustomization(t *testing.T) { + th := kusttest_test.MakeHarness(t) + writeKustomizationPatchBase(th) + writeKustomizationPatchPatch(th) + th.WriteF("/app/solopatch/kustomization.yaml", ` +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: KustomizationPatch +resources: +- ../base +configMapGenerator: +- name: my-configmap + behavior: merge + literals: + - patchValue=5 + - testValue=2 +`) + m := th.Run("/app/solopatch", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: v1 +kind: Deployment +metadata: + name: storefront +spec: + replicas: 1 +--- +apiVersion: v1 +data: + otherValue: "10" + patchValue: "5" + testValue: "2" +kind: ConfigMap +metadata: + annotations: {} + labels: {} + name: my-configmap-t86ktk6tdk +`) +} diff --git a/api/types/kustomization.go b/api/types/kustomization.go index 4be803f53..135e3f762 100644 --- a/api/types/kustomization.go +++ b/api/types/kustomization.go @@ -4,9 +4,10 @@ package types const ( - KustomizationVersion = "kustomize.config.k8s.io/v1beta1" - KustomizationKind = "Kustomization" - MetadataNamespacePath = "metadata/namespace" + KustomizationVersion = "kustomize.config.k8s.io/v1beta1" + KustomizationKind = "Kustomization" + KustomizationPatchKind = "KustomizationPatch" + MetadataNamespacePath = "metadata/namespace" ) // Kustomization holds the information needed to generate customized k8s api resources. @@ -143,8 +144,8 @@ func (k *Kustomization) EnforceFields() []string { if k.APIVersion != "" && k.APIVersion != KustomizationVersion { errs = append(errs, "apiVersion should be "+KustomizationVersion) } - if k.Kind != "" && k.Kind != KustomizationKind { - errs = append(errs, "kind should be "+KustomizationKind) + if k.Kind != "" && k.Kind != KustomizationKind && k.Kind != KustomizationPatchKind { + errs = append(errs, "kind should be "+KustomizationKind+" or "+KustomizationPatchKind) } return errs } From 1079d8604cc2d4505bec675d40950ff573587821 Mon Sep 17 00:00:00 2001 From: Paul Martin Date: Thu, 14 May 2020 18:00:10 +0100 Subject: [PATCH 2/9] Renamed `KustomizationPatch` to `Component` --- api/internal/target/kusttarget.go | 2 +- ...izationpatch_test.go => component_test.go} | 56 +++++++++---------- api/types/kustomization.go | 12 ++-- 3 files changed, 35 insertions(+), 35 deletions(-) rename api/krusty/{kustomizationpatch_test.go => component_test.go} (80%) diff --git a/api/internal/target/kusttarget.go b/api/internal/target/kusttarget.go index 257d08922..766351951 100644 --- a/api/internal/target/kusttarget.go +++ b/api/internal/target/kusttarget.go @@ -294,7 +294,7 @@ func (kt *KustTarget) accumulateDirectory( } var subRa *accumulator.ResAccumulator - if subKt.kustomization.Kind == types.KustomizationPatchKind { + if subKt.kustomization.Kind == types.ComponentKind { subRa, err = subKt.accumulateTarget(ra) ra = accumulator.MakeEmptyAccumulator() } else { diff --git a/api/krusty/kustomizationpatch_test.go b/api/krusty/component_test.go similarity index 80% rename from api/krusty/kustomizationpatch_test.go rename to api/krusty/component_test.go index 362115b40..28ae4fc58 100644 --- a/api/krusty/kustomizationpatch_test.go +++ b/api/krusty/component_test.go @@ -9,8 +9,8 @@ import ( kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" ) -// Test kustomization.yaml files with kind: KustomizationPatch -func writeKustomizationPatchBase(th kusttest_test.Harness) { +// Test kustomization.yaml files with kind: Component +func writeComponentBase(th kusttest_test.Harness) { th.WriteK("/app/base", ` resources: - deploy.yaml @@ -30,10 +30,10 @@ spec: `) } -func writeKustomizationPatchPatch(th kusttest_test.Harness) { +func writeComponentPatch(th kusttest_test.Harness) { th.WriteF("/app/patch/kustomization.yaml", ` apiVersion: kustomize.config.k8s.io/v1beta1 -kind: KustomizationPatch +kind: Component namePrefix: patched- replicas: - name: storefront @@ -57,7 +57,7 @@ spec: `) } -func writeKustomizationPatchProd(th kusttest_test.Harness) { +func writeComponentProd(th kusttest_test.Harness) { th.WriteK("/app/prod", ` resources: - ../base @@ -74,13 +74,13 @@ spec: `) } -// KustomizationPatch are inserted into the resource hierarchy as the parent of those +// Components are inserted into the resource hierarchy as the parent of those // resources that come before it in the resources list of the parent Kustomization. -func TestBasicKustomizationPatch(t *testing.T) { +func TestBasicComponent(t *testing.T) { th := kusttest_test.MakeHarness(t) - writeKustomizationPatchBase(th) - writeKustomizationPatchPatch(th) - writeKustomizationPatchProd(th) + writeComponentBase(th) + writeComponentPatch(th) + writeComponentProd(th) m := th.Run("/app/prod", th.MakeDefaultOptions()) th.AssertActualEqualsExpected(m, ` apiVersion: v1 @@ -117,14 +117,14 @@ spec: `) } -func TestMultipleKustomizationPatches(t *testing.T) { +func TestMultipleComponentes(t *testing.T) { th := kusttest_test.MakeHarness(t) - writeKustomizationPatchBase(th) - writeKustomizationPatchPatch(th) - writeKustomizationPatchProd(th) + writeComponentBase(th) + writeComponentPatch(th) + writeComponentProd(th) th.WriteF("/app/additionalpatch/kustomization.yaml", ` apiVersion: kustomize.config.k8s.io/v1beta1 -kind: KustomizationPatch +kind: Component configMapGenerator: - name: my-configmap behavior: merge @@ -174,14 +174,14 @@ spec: `) } -func TestNestedKustomizationPatches(t *testing.T) { +func TestNestedComponents(t *testing.T) { th := kusttest_test.MakeHarness(t) - writeKustomizationPatchBase(th) - writeKustomizationPatchPatch(th) - writeKustomizationPatchProd(th) + writeComponentBase(th) + writeComponentPatch(th) + writeComponentProd(th) th.WriteF("/app/additionalpatch/kustomization.yaml", ` apiVersion: kustomize.config.k8s.io/v1beta1 -kind: KustomizationPatch +kind: Component resources: - ../patch configMapGenerator: @@ -234,11 +234,11 @@ spec: // If a patch sets a name prefix on a base, then that base can also be separately included // without being affected by the patch in another branch of the resource tree -func TestBasicKustomizationPatchWithRepeatedBase(t *testing.T) { +func TestBasicComponentWithRepeatedBase(t *testing.T) { th := kusttest_test.MakeHarness(t) - writeKustomizationPatchBase(th) - writeKustomizationPatchPatch(th) - writeKustomizationPatchProd(th) + writeComponentBase(th) + writeComponentPatch(th) + writeComponentProd(th) th.WriteK("/app/repeated", ` resources: - ../base @@ -295,13 +295,13 @@ spec: `) } -func TestApplyingKustomizationPatchDirectlySameAsKustomization(t *testing.T) { +func TestApplyingComponentDirectlySameAsKustomization(t *testing.T) { th := kusttest_test.MakeHarness(t) - writeKustomizationPatchBase(th) - writeKustomizationPatchPatch(th) + writeComponentBase(th) + writeComponentPatch(th) th.WriteF("/app/solopatch/kustomization.yaml", ` apiVersion: kustomize.config.k8s.io/v1beta1 -kind: KustomizationPatch +kind: Component resources: - ../base configMapGenerator: diff --git a/api/types/kustomization.go b/api/types/kustomization.go index 135e3f762..89dc34e12 100644 --- a/api/types/kustomization.go +++ b/api/types/kustomization.go @@ -4,10 +4,10 @@ package types const ( - KustomizationVersion = "kustomize.config.k8s.io/v1beta1" - KustomizationKind = "Kustomization" - KustomizationPatchKind = "KustomizationPatch" - MetadataNamespacePath = "metadata/namespace" + KustomizationVersion = "kustomize.config.k8s.io/v1beta1" + KustomizationKind = "Kustomization" + ComponentKind = "Component" + MetadataNamespacePath = "metadata/namespace" ) // Kustomization holds the information needed to generate customized k8s api resources. @@ -144,8 +144,8 @@ func (k *Kustomization) EnforceFields() []string { if k.APIVersion != "" && k.APIVersion != KustomizationVersion { errs = append(errs, "apiVersion should be "+KustomizationVersion) } - if k.Kind != "" && k.Kind != KustomizationKind && k.Kind != KustomizationPatchKind { - errs = append(errs, "kind should be "+KustomizationKind+" or "+KustomizationPatchKind) + if k.Kind != "" && k.Kind != KustomizationKind && k.Kind != ComponentKind { + errs = append(errs, "kind should be "+KustomizationKind+" or "+ComponentKind) } return errs } From fdfb58cc3e736d332b43d4baf6c2b5a667c4deb2 Mon Sep 17 00:00:00 2001 From: Paul Martin Date: Thu, 14 May 2020 21:01:02 +0100 Subject: [PATCH 3/9] Require version `kustomize.config.k8s.io/v1alpha1` for `Component` --- api/krusty/component_test.go | 73 +++++++++++++++++++++++++++++++++--- api/types/kustomization.go | 21 ++++++++--- 2 files changed, 83 insertions(+), 11 deletions(-) diff --git a/api/krusty/component_test.go b/api/krusty/component_test.go index 28ae4fc58..022f9f4aa 100644 --- a/api/krusty/component_test.go +++ b/api/krusty/component_test.go @@ -4,6 +4,7 @@ package krusty_test import ( + "strings" "testing" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" @@ -32,7 +33,7 @@ spec: func writeComponentPatch(th kusttest_test.Harness) { th.WriteF("/app/patch/kustomization.yaml", ` -apiVersion: kustomize.config.k8s.io/v1beta1 +apiVersion: kustomize.config.k8s.io/v1alpha1 kind: Component namePrefix: patched- replicas: @@ -117,13 +118,13 @@ spec: `) } -func TestMultipleComponentes(t *testing.T) { +func TestMultipleComponents(t *testing.T) { th := kusttest_test.MakeHarness(t) writeComponentBase(th) writeComponentPatch(th) writeComponentProd(th) th.WriteF("/app/additionalpatch/kustomization.yaml", ` -apiVersion: kustomize.config.k8s.io/v1beta1 +apiVersion: kustomize.config.k8s.io/v1alpha1 kind: Component configMapGenerator: - name: my-configmap @@ -180,7 +181,7 @@ func TestNestedComponents(t *testing.T) { writeComponentPatch(th) writeComponentProd(th) th.WriteF("/app/additionalpatch/kustomization.yaml", ` -apiVersion: kustomize.config.k8s.io/v1beta1 +apiVersion: kustomize.config.k8s.io/v1alpha1 kind: Component resources: - ../patch @@ -300,7 +301,7 @@ func TestApplyingComponentDirectlySameAsKustomization(t *testing.T) { writeComponentBase(th) writeComponentPatch(th) th.WriteF("/app/solopatch/kustomization.yaml", ` -apiVersion: kustomize.config.k8s.io/v1beta1 +apiVersion: kustomize.config.k8s.io/v1alpha1 kind: Component resources: - ../base @@ -332,3 +333,65 @@ metadata: name: my-configmap-t86ktk6tdk `) } + +func TestMissingOptionalComponentApiVersion(t *testing.T) { + th := kusttest_test.MakeHarness(t) + writeComponentBase(th) + writeComponentProd(th) + th.WriteF("/app/patch/kustomization.yaml", ` +kind: Component +configMapGenerator: +- name: my-configmap + behavior: merge + literals: + - otherValue=9 +`) + + m := th.Run("/app/prod", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: v1 +kind: Deployment +metadata: + name: storefront +spec: + replicas: 1 +--- +apiVersion: v1 +data: + otherValue: "9" + testValue: "1" +kind: ConfigMap +metadata: + annotations: {} + labels: {} + name: my-configmap-72cfg2mg5d +--- +apiVersion: v1 +kind: Deployment +metadata: + name: db +spec: + type: Logical +`) +} + +func TestInvalidComponentApiVersion(t *testing.T) { + th := kusttest_test.MakeHarness(t) + writeComponentBase(th) + writeComponentProd(th) + th.WriteF("/app/patch/kustomization.yaml", ` +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Component +configMapGenerator: +- name: my-configmap + behavior: merge + literals: + - otherValue=9 +`) + err := th.RunWithErr("/app/prod", th.MakeDefaultOptions()) + if !strings.Contains( + err.Error(), + "apiVersion should be kustomize.config.k8s.io/v1alpha1") { + t.Fatalf("unexpected error: %s", err) + } +} diff --git a/api/types/kustomization.go b/api/types/kustomization.go index 89dc34e12..eb32d906a 100644 --- a/api/types/kustomization.go +++ b/api/types/kustomization.go @@ -6,6 +6,7 @@ package types const ( KustomizationVersion = "kustomize.config.k8s.io/v1beta1" KustomizationKind = "Kustomization" + ComponentVersion = "kustomize.config.k8s.io/v1alpha1" ComponentKind = "Component" MetadataNamespacePath = "metadata/namespace" ) @@ -129,23 +130,31 @@ type Kustomization struct { // moving content of deprecated fields to newer // fields. func (k *Kustomization) FixKustomizationPostUnmarshalling() { - if k.APIVersion == "" { - k.APIVersion = KustomizationVersion - } if k.Kind == "" { k.Kind = KustomizationKind } + if k.APIVersion == "" { + if k.Kind == ComponentKind { + k.APIVersion = ComponentVersion + } else { + k.APIVersion = KustomizationVersion + } + } k.Resources = append(k.Resources, k.Bases...) k.Bases = nil } func (k *Kustomization) EnforceFields() []string { var errs []string - if k.APIVersion != "" && k.APIVersion != KustomizationVersion { - errs = append(errs, "apiVersion should be "+KustomizationVersion) - } if k.Kind != "" && k.Kind != KustomizationKind && k.Kind != ComponentKind { errs = append(errs, "kind should be "+KustomizationKind+" or "+ComponentKind) } + requiredVersion := KustomizationVersion + if k.Kind == ComponentKind { + requiredVersion = ComponentVersion + } + if k.APIVersion != "" && k.APIVersion != requiredVersion { + errs = append(errs, "apiVersion should be "+requiredVersion) + } return errs } From c5c53011da134543c604d0d8047d22cce1a0aa12 Mon Sep 17 00:00:00 2001 From: Paul Martin Date: Thu, 14 May 2020 19:16:04 +0100 Subject: [PATCH 4/9] PoC to add `Component`s to a separate `components` list (instead of `resources`) If this idea is preferred, the code could really do with a refactor (probably using closures to control the behaviour of accumulate directory) --- api/internal/target/kusttarget.go | 34 ++++++++++++- api/krusty/component_test.go | 84 +++++++++++++++++++++++-------- api/types/kustomization.go | 6 ++- 3 files changed, 101 insertions(+), 23 deletions(-) diff --git a/api/internal/target/kusttarget.go b/api/internal/target/kusttarget.go index 766351951..a51299a4c 100644 --- a/api/internal/target/kusttarget.go +++ b/api/internal/target/kusttarget.go @@ -164,6 +164,10 @@ func (kt *KustTarget) accumulateTarget(ra *accumulator.ResAccumulator) ( if err != nil { return nil, errors.Wrap(err, "accumulating resources") } + ra, err = kt.accumulateComponents(ra, kt.kustomization.Components) + if err != nil { + return nil, errors.Wrap(err, "accumulating components") + } tConfig, err := builtinconfig.MakeTransformerConfig( kt.ldr, kt.kustomization.Configurations) if err != nil { @@ -273,7 +277,7 @@ func (kt *KustTarget) accumulateResources( return nil, fmt.Errorf("accumulateFile %q, loader.New %q", errF, errL) } var errD error - ra, errD = kt.accumulateDirectory(ra, ldr) + ra, errD = kt.accumulateDirectory(ra, ldr, false) if errD != nil { return nil, fmt.Errorf("accumulateFile %q, accumulateDirector: %q", errF, errD) } @@ -282,8 +286,27 @@ func (kt *KustTarget) accumulateResources( return ra, nil } +// accumulateResources fills the given resourceAccumulator +// with resources read from the given list of paths. +func (kt *KustTarget) accumulateComponents( + ra *accumulator.ResAccumulator, paths []string) (*accumulator.ResAccumulator, error) { + for _, path := range paths { + // Components always refer to directories + ldr, errL := kt.ldr.New(path) + if errL != nil { + return nil, fmt.Errorf("loader.New %q", errL) + } + var errD error + ra, errD = kt.accumulateDirectory(ra, ldr, true) + if errD != nil { + return nil, fmt.Errorf("accumulateDirectory: %q", errD) + } + } + return ra, nil +} + func (kt *KustTarget) accumulateDirectory( - ra *accumulator.ResAccumulator, ldr ifc.Loader) (*accumulator.ResAccumulator, error) { + ra *accumulator.ResAccumulator, ldr ifc.Loader, isComponent bool) (*accumulator.ResAccumulator, error) { defer ldr.Cleanup() subKt := NewKustTarget( ldr, kt.validator, kt.rFactory, kt.tFactory, kt.pLdr) @@ -292,6 +315,13 @@ func (kt *KustTarget) accumulateDirectory( return nil, errors.Wrapf( err, "couldn't make target for path '%s'", ldr.Root()) } + if isComponent && subKt.kustomization.Kind != types.ComponentKind { + return nil, fmt.Errorf( + "expected kind '%s' for path '%s' but got '%s'", types.ComponentKind, ldr.Root(), subKt.kustomization.Kind) + } else if !isComponent && subKt.kustomization.Kind == types.ComponentKind { + return nil, fmt.Errorf( + "expected kind != '%s' for path '%s'", types.ComponentKind, ldr.Root()) + } var subRa *accumulator.ResAccumulator if subKt.kustomization.Kind == types.ComponentKind { diff --git a/api/krusty/component_test.go b/api/krusty/component_test.go index 022f9f4aa..5ab861fe3 100644 --- a/api/krusty/component_test.go +++ b/api/krusty/component_test.go @@ -62,8 +62,10 @@ func writeComponentProd(th kusttest_test.Harness) { th.WriteK("/app/prod", ` resources: - ../base -- ../patch - db + +components: +- ../patch `) th.WriteF("/app/prod/db", ` apiVersion: v1 @@ -105,16 +107,16 @@ metadata: apiVersion: v1 kind: Deployment metadata: - name: patched-stub + name: patched-db spec: - replicas: 1 + type: Logical --- apiVersion: v1 kind: Deployment metadata: - name: db + name: patched-stub spec: - type: Logical + replicas: 1 `) } @@ -135,9 +137,11 @@ configMapGenerator: th.WriteK("/app/prod", ` resources: - ../base +- db + +components: - ../patch - ../additionalpatch -- db `) m := th.Run("/app/prod", th.MakeDefaultOptions()) th.AssertActualEqualsExpected(m, ` @@ -162,16 +166,16 @@ metadata: apiVersion: v1 kind: Deployment metadata: - name: patched-stub + name: patched-db spec: - replicas: 1 + type: Logical --- apiVersion: v1 kind: Deployment metadata: - name: db + name: patched-stub spec: - type: Logical + replicas: 1 `) } @@ -183,7 +187,7 @@ func TestNestedComponents(t *testing.T) { th.WriteF("/app/additionalpatch/kustomization.yaml", ` apiVersion: kustomize.config.k8s.io/v1alpha1 kind: Component -resources: +components: - ../patch configMapGenerator: - name: my-configmap @@ -194,8 +198,10 @@ configMapGenerator: th.WriteK("/app/prod", ` resources: - ../base -- ../additionalpatch - db + +components: +- ../additionalpatch `) m := th.Run("/app/prod", th.MakeDefaultOptions()) th.AssertActualEqualsExpected(m, ` @@ -220,16 +226,16 @@ metadata: apiVersion: v1 kind: Deployment metadata: - name: patched-stub + name: patched-db spec: - replicas: 1 + type: Logical --- apiVersion: v1 kind: Deployment metadata: - name: db + name: patched-stub spec: - type: Logical + replicas: 1 `) } @@ -283,16 +289,16 @@ metadata: apiVersion: v1 kind: Deployment metadata: - name: patched-stub + name: patched-db spec: - replicas: 1 + type: Logical --- apiVersion: v1 kind: Deployment metadata: - name: db + name: patched-stub spec: - type: Logical + replicas: 1 `) } @@ -334,6 +340,44 @@ metadata: `) } +func TestComponentsCannotBeAddedToResources(t *testing.T) { + th := kusttest_test.MakeHarness(t) + writeComponentBase(th) + writeComponentPatch(th) + th.WriteF("/app/custinres/kustomization.yaml", ` +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base +- ../patch +`) + err := th.RunWithErr("/app/custinres", th.MakeDefaultOptions()) + if !strings.Contains( + err.Error(), + "expected kind != 'Component' for path '/app/patch'") { + t.Fatalf("unexpected error: %s", err) + } +} + +func TestResourcesCannotBeAddedToComponents(t *testing.T) { + th := kusttest_test.MakeHarness(t) + writeComponentBase(th) + writeComponentPatch(th) + th.WriteF("/app/resincust/kustomization.yaml", ` +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +components: +- ../base +- ../patch +`) + err := th.RunWithErr("/app/resincust", th.MakeDefaultOptions()) + if !strings.Contains( + err.Error(), + "accumulating components: accumulateDirectory: \"expected kind 'Component' for path '/app/base' but got 'Kustomization'") { + t.Fatalf("unexpected error: %s", err) + } +} + func TestMissingOptionalComponentApiVersion(t *testing.T) { th := kusttest_test.MakeHarness(t) writeComponentBase(th) diff --git a/api/types/kustomization.go b/api/types/kustomization.go index eb32d906a..f86edb4bf 100644 --- a/api/types/kustomization.go +++ b/api/types/kustomization.go @@ -75,10 +75,14 @@ type Kustomization struct { // // Resources specifies relative paths to files holding YAML representations - // of kubernetes API objects, or specifcations of other kustomizations + // of kubernetes API objects, or specifications of other kustomizations // via relative paths, absolute paths, or URLs. Resources []string `json:"resources,omitempty" yaml:"resources,omitempty"` + // Components specifies relative paths to specifications of other Components + // via relative paths, absolute paths, or URLs. + Components []string `json:"components,omitempty" yaml:"components,omitempty"` + // Crds specifies relative paths to Custom Resource Definition files. // This allows custom resources to be recognized as operands, making // it possible to add them to the Resources list. From ab9d4c7e7b035e0a1efa14b23c0356225a4e955b Mon Sep 17 00:00:00 2001 From: Paul Martin Date: Tue, 19 May 2020 19:26:18 +0100 Subject: [PATCH 5/9] Added test to ensure that individusal files cannot be added to the components list. --- api/krusty/component_test.go | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/api/krusty/component_test.go b/api/krusty/component_test.go index 5ab861fe3..bc1d1be82 100644 --- a/api/krusty/component_test.go +++ b/api/krusty/component_test.go @@ -359,18 +359,18 @@ resources: } } -func TestResourcesCannotBeAddedToComponents(t *testing.T) { +func TestKustomizationsCannotBeAddedToComponents(t *testing.T) { th := kusttest_test.MakeHarness(t) writeComponentBase(th) writeComponentPatch(th) - th.WriteF("/app/resincust/kustomization.yaml", ` + th.WriteF("/app/kustincomponents/kustomization.yaml", ` apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization components: - ../base - ../patch `) - err := th.RunWithErr("/app/resincust", th.MakeDefaultOptions()) + err := th.RunWithErr("/app/kustincomponents", th.MakeDefaultOptions()) if !strings.Contains( err.Error(), "accumulating components: accumulateDirectory: \"expected kind 'Component' for path '/app/base' but got 'Kustomization'") { @@ -378,6 +378,34 @@ components: } } +func TestFilesCannotBeAddedToComponentsList(t *testing.T) { + th := kusttest_test.MakeHarness(t) + writeComponentBase(th) + + th.WriteF("/app/filesincomponents/stub.yaml", ` +apiVersion: v1 +kind: Deployment +metadata: + name: stub +spec: + replicas: 1 +`) + + th.WriteF("/app/filesincomponents/kustomization.yaml", ` +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +components: +- stub.yaml +- ../patch +`) + err := th.RunWithErr("/app/filesincomponents", th.MakeDefaultOptions()) + if !strings.Contains( + err.Error(), + "'/app/filesincomponents/stub.yaml' must be a directory to be a root") { + t.Fatalf("unexpected error: %s", err) + } +} + func TestMissingOptionalComponentApiVersion(t *testing.T) { th := kusttest_test.MakeHarness(t) writeComponentBase(th) From 34a442bbef1df0e2f839fee9d651a3d1860f2c6d Mon Sep 17 00:00:00 2001 From: Paul Martin Date: Thu, 28 May 2020 19:50:00 +0100 Subject: [PATCH 6/9] Additional tests around reusing the same bases and components --- api/internal/target/kusttarget.go | 7 ++++++- api/krusty/component_test.go | 2 +- api/types/kustomization.go | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/api/internal/target/kusttarget.go b/api/internal/target/kusttarget.go index a51299a4c..1e39d925f 100644 --- a/api/internal/target/kusttarget.go +++ b/api/internal/target/kusttarget.go @@ -158,6 +158,8 @@ func (kt *KustTarget) AccumulateTarget() ( return kt.accumulateTarget(accumulator.MakeEmptyAccumulator()) } +// ra should be empty when this KustTarget is a Kustomization, or the ra of the parent if this KustTarget is a Component +// (or empty if the Component does not have a parent). func (kt *KustTarget) accumulateTarget(ra *accumulator.ResAccumulator) ( resRa *accumulator.ResAccumulator, err error) { ra, err = kt.accumulateResources(ra, kt.kustomization.Resources) @@ -324,10 +326,13 @@ func (kt *KustTarget) accumulateDirectory( } var subRa *accumulator.ResAccumulator - if subKt.kustomization.Kind == types.ComponentKind { + if isComponent { + // Components don't create a new accumulator: the kustomization directives are added to the current accumulator subRa, err = subKt.accumulateTarget(ra) ra = accumulator.MakeEmptyAccumulator() } else { + // Child Kustomizations create a new accumulator which resolves their kustomization directives, which will later + // be merged into the current accumulator. subRa, err = subKt.AccumulateTarget() } if err != nil { diff --git a/api/krusty/component_test.go b/api/krusty/component_test.go index bc1d1be82..7681e3962 100644 --- a/api/krusty/component_test.go +++ b/api/krusty/component_test.go @@ -463,7 +463,7 @@ configMapGenerator: err := th.RunWithErr("/app/prod", th.MakeDefaultOptions()) if !strings.Contains( err.Error(), - "apiVersion should be kustomize.config.k8s.io/v1alpha1") { + "apiVersion for Component should be kustomize.config.k8s.io/v1alpha1") { t.Fatalf("unexpected error: %s", err) } } diff --git a/api/types/kustomization.go b/api/types/kustomization.go index f86edb4bf..4a2c42d36 100644 --- a/api/types/kustomization.go +++ b/api/types/kustomization.go @@ -158,7 +158,7 @@ func (k *Kustomization) EnforceFields() []string { requiredVersion = ComponentVersion } if k.APIVersion != "" && k.APIVersion != requiredVersion { - errs = append(errs, "apiVersion should be "+requiredVersion) + errs = append(errs, "apiVersion for "+k.Kind+" should be "+requiredVersion) } return errs } From 93b0b1b0b168f2889d2c39d9f7538c199e105f25 Mon Sep 17 00:00:00 2001 From: Paul Martin Date: Thu, 28 May 2020 21:05:13 +0100 Subject: [PATCH 7/9] Refactored Compontent tests to be table-driven --- api/krusty/component_test.go | 342 +++++++++++++++--------------- api/testutils/kusttest/harness.go | 10 + 2 files changed, 186 insertions(+), 166 deletions(-) diff --git a/api/krusty/component_test.go b/api/krusty/component_test.go index 7681e3962..61ccdd518 100644 --- a/api/krusty/component_test.go +++ b/api/krusty/component_test.go @@ -1,4 +1,4 @@ -// Copyright 2019 The Kubernetes Authors. +// Copyright 2020 The Kubernetes Authors. // SPDX-License-Identifier: Apache-2.0 package krusty_test @@ -7,11 +7,31 @@ import ( "strings" "testing" + "sigs.k8s.io/kustomize/api/konfig" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" ) -// Test kustomization.yaml files with kind: Component -func writeComponentBase(th kusttest_test.Harness) { +type FileGen func(kusttest_test.Harness) + +func writeC(path string, content string) FileGen { + return func(th kusttest_test.Harness) { + th.WriteC(path, content) + } +} + +func writeF(path string, content string) FileGen { + return func(th kusttest_test.Harness) { + th.WriteF(path, content) + } +} + +func writeK(path string, content string) FileGen { + return func(th kusttest_test.Harness) { + th.WriteK(path, content) + } +} + +func writeTestBase(th kusttest_test.Harness) { th.WriteK("/app/base", ` resources: - deploy.yaml @@ -31,10 +51,8 @@ spec: `) } -func writeComponentPatch(th kusttest_test.Harness) { - th.WriteF("/app/patch/kustomization.yaml", ` -apiVersion: kustomize.config.k8s.io/v1alpha1 -kind: Component +func writeTestComponent(th kusttest_test.Harness) { + th.WriteC("/app/patch", ` namePrefix: patched- replicas: - name: storefront @@ -58,7 +76,7 @@ spec: `) } -func writeComponentProd(th kusttest_test.Harness) { +func writeOverlayProd(th kusttest_test.Harness) { th.WriteK("/app/prod", ` resources: - ../base @@ -67,6 +85,10 @@ resources: components: - ../patch `) + writeDB(th) +} + +func writeDB(th kusttest_test.Harness) { th.WriteF("/app/prod/db", ` apiVersion: v1 kind: Deployment @@ -77,15 +99,18 @@ spec: `) } -// Components are inserted into the resource hierarchy as the parent of those -// resources that come before it in the resources list of the parent Kustomization. -func TestBasicComponent(t *testing.T) { - th := kusttest_test.MakeHarness(t) - writeComponentBase(th) - writeComponentPatch(th) - writeComponentProd(th) - m := th.Run("/app/prod", th.MakeDefaultOptions()) - th.AssertActualEqualsExpected(m, ` +func TestComponent(t *testing.T) { + testCases := map[string]struct { + input []FileGen + runPath string + expectedOutput string + }{ + // Components are inserted into the resource hierarchy as the parent of those + // resources that come before it in the resources list of the parent Kustomization. + "basic-component": { + input: []FileGen{writeTestBase, writeTestComponent, writeOverlayProd}, + runPath: "/app/prod", + expectedOutput: ` apiVersion: v1 kind: Deployment metadata: @@ -117,24 +142,18 @@ metadata: name: patched-stub spec: replicas: 1 -`) -} - -func TestMultipleComponents(t *testing.T) { - th := kusttest_test.MakeHarness(t) - writeComponentBase(th) - writeComponentPatch(th) - writeComponentProd(th) - th.WriteF("/app/additionalpatch/kustomization.yaml", ` -apiVersion: kustomize.config.k8s.io/v1alpha1 -kind: Component +`, + }, + "multiple-components": { + input: []FileGen{writeTestBase, writeTestComponent, writeDB, + writeC("/app/additionalpatch", ` configMapGenerator: - name: my-configmap behavior: merge literals: - otherValue=9 -`) - th.WriteK("/app/prod", ` +`), + writeK("/app/prod", ` resources: - ../base - db @@ -142,9 +161,10 @@ resources: components: - ../patch - ../additionalpatch -`) - m := th.Run("/app/prod", th.MakeDefaultOptions()) - th.AssertActualEqualsExpected(m, ` +`), + }, + runPath: "/app/prod", + expectedOutput: ` apiVersion: v1 kind: Deployment metadata: @@ -176,17 +196,11 @@ metadata: name: patched-stub spec: replicas: 1 -`) -} - -func TestNestedComponents(t *testing.T) { - th := kusttest_test.MakeHarness(t) - writeComponentBase(th) - writeComponentPatch(th) - writeComponentProd(th) - th.WriteF("/app/additionalpatch/kustomization.yaml", ` -apiVersion: kustomize.config.k8s.io/v1alpha1 -kind: Component +`, + }, + "nested-components": { + input: []FileGen{writeTestBase, writeTestComponent, writeDB, + writeC("/app/additionalpatch", ` components: - ../patch configMapGenerator: @@ -194,17 +208,18 @@ configMapGenerator: behavior: merge literals: - otherValue=9 -`) - th.WriteK("/app/prod", ` +`), + writeK("/app/prod", ` resources: - ../base - db components: - ../additionalpatch -`) - m := th.Run("/app/prod", th.MakeDefaultOptions()) - th.AssertActualEqualsExpected(m, ` +`), + }, + runPath: "/app/prod", + expectedOutput: ` apiVersion: v1 kind: Deployment metadata: @@ -236,23 +251,20 @@ metadata: name: patched-stub spec: replicas: 1 -`) -} - -// If a patch sets a name prefix on a base, then that base can also be separately included -// without being affected by the patch in another branch of the resource tree -func TestBasicComponentWithRepeatedBase(t *testing.T) { - th := kusttest_test.MakeHarness(t) - writeComponentBase(th) - writeComponentPatch(th) - writeComponentProd(th) - th.WriteK("/app/repeated", ` +`, + }, + // If a patch sets a name prefix on a base, then that base can also be separately included + // without being affected by the patch in another branch of the resource tree + "basic-component-with-repeated-base": { + input: []FileGen{writeTestBase, writeTestComponent, writeOverlayProd, + writeK("/app/repeated", ` resources: - ../base - ../prod -`) - m := th.Run("/app/repeated", th.MakeDefaultOptions()) - th.AssertActualEqualsExpected(m, ` +`), + }, + runPath: "/app/repeated", + expectedOutput: ` apiVersion: v1 kind: Deployment metadata: @@ -299,16 +311,11 @@ metadata: name: patched-stub spec: replicas: 1 -`) -} - -func TestApplyingComponentDirectlySameAsKustomization(t *testing.T) { - th := kusttest_test.MakeHarness(t) - writeComponentBase(th) - writeComponentPatch(th) - th.WriteF("/app/solopatch/kustomization.yaml", ` -apiVersion: kustomize.config.k8s.io/v1alpha1 -kind: Component +`, + }, + "applying-component-directly-should-be-same-as-kustomization": { + input: []FileGen{writeTestBase, writeTestComponent, + writeC("/app/direct-component", ` resources: - ../base configMapGenerator: @@ -317,9 +324,10 @@ configMapGenerator: literals: - patchValue=5 - testValue=2 -`) - m := th.Run("/app/solopatch", th.MakeDefaultOptions()) - th.AssertActualEqualsExpected(m, ` +`), + }, + runPath: "/app/direct-component", + expectedOutput: ` apiVersion: v1 kind: Deployment metadata: @@ -337,90 +345,21 @@ metadata: annotations: {} labels: {} name: my-configmap-t86ktk6tdk -`) -} - -func TestComponentsCannotBeAddedToResources(t *testing.T) { - th := kusttest_test.MakeHarness(t) - writeComponentBase(th) - writeComponentPatch(th) - th.WriteF("/app/custinres/kustomization.yaml", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization -resources: -- ../base -- ../patch -`) - err := th.RunWithErr("/app/custinres", th.MakeDefaultOptions()) - if !strings.Contains( - err.Error(), - "expected kind != 'Component' for path '/app/patch'") { - t.Fatalf("unexpected error: %s", err) - } -} - -func TestKustomizationsCannotBeAddedToComponents(t *testing.T) { - th := kusttest_test.MakeHarness(t) - writeComponentBase(th) - writeComponentPatch(th) - th.WriteF("/app/kustincomponents/kustomization.yaml", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization -components: -- ../base -- ../patch -`) - err := th.RunWithErr("/app/kustincomponents", th.MakeDefaultOptions()) - if !strings.Contains( - err.Error(), - "accumulating components: accumulateDirectory: \"expected kind 'Component' for path '/app/base' but got 'Kustomization'") { - t.Fatalf("unexpected error: %s", err) - } -} - -func TestFilesCannotBeAddedToComponentsList(t *testing.T) { - th := kusttest_test.MakeHarness(t) - writeComponentBase(th) - - th.WriteF("/app/filesincomponents/stub.yaml", ` -apiVersion: v1 -kind: Deployment -metadata: - name: stub -spec: - replicas: 1 -`) - - th.WriteF("/app/filesincomponents/kustomization.yaml", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization -components: -- stub.yaml -- ../patch -`) - err := th.RunWithErr("/app/filesincomponents", th.MakeDefaultOptions()) - if !strings.Contains( - err.Error(), - "'/app/filesincomponents/stub.yaml' must be a directory to be a root") { - t.Fatalf("unexpected error: %s", err) - } -} - -func TestMissingOptionalComponentApiVersion(t *testing.T) { - th := kusttest_test.MakeHarness(t) - writeComponentBase(th) - writeComponentProd(th) - th.WriteF("/app/patch/kustomization.yaml", ` +`, + }, + "missing-optional-component-api-version": { + input: []FileGen{writeTestBase, writeOverlayProd, + writeF("/app/patch/"+konfig.DefaultKustomizationFileName(), ` kind: Component configMapGenerator: - name: my-configmap behavior: merge literals: - otherValue=9 -`) - - m := th.Run("/app/prod", th.MakeDefaultOptions()) - th.AssertActualEqualsExpected(m, ` +`), + }, + runPath: "/app/prod", + expectedOutput: ` apiVersion: v1 kind: Deployment metadata: @@ -444,14 +383,73 @@ metadata: name: db spec: type: Logical -`) +`, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + th := kusttest_test.MakeHarness(t) + for _, f := range tc.input { + f(th) + } + m := th.Run(tc.runPath, th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, tc.expectedOutput) + }) + } } -func TestInvalidComponentApiVersion(t *testing.T) { - th := kusttest_test.MakeHarness(t) - writeComponentBase(th) - writeComponentProd(th) - th.WriteF("/app/patch/kustomization.yaml", ` +func TestComponentErrors(t *testing.T) { + testCases := map[string]struct { + input []FileGen + runPath string + expectedError string + }{ + "components-cannot-be-added-to-resources": { + input: []FileGen{writeTestBase, writeTestComponent, + writeK("/app/compinres", ` +resources: +- ../base +- ../patch +`), + }, + runPath: "app/compinres", + expectedError: "expected kind != 'Component' for path '/app/patch'", + }, + "kustomizations-cannot-be-added-to-components": { + input: []FileGen{writeTestBase, writeTestComponent, + writeK("/app/kustincomponents", ` +components: +- ../base +- ../patch +`), + }, + runPath: "/app/kustincomponents", + expectedError: "accumulating components: accumulateDirectory: \"expected kind 'Component' for path " + + "'/app/base' but got 'Kustomization'", + }, + "files-cannot-be-added-to-components-list": { + input: []FileGen{writeTestBase, + writeF("/app/filesincomponents/stub.yaml", ` +apiVersion: v1 +kind: Deployment +metadata: + name: stub +spec: + replicas: 1 +`), + writeK("/app/filesincomponents", ` +components: +- stub.yaml +- ../patch +`), + }, + runPath: "/app/filesincomponents", + expectedError: "'/app/filesincomponents/stub.yaml' must be a directory to be a root", + }, + "invalid-component-api-version": { + input: []FileGen{writeTestBase, writeOverlayProd, + writeF("/app/patch/"+konfig.DefaultKustomizationFileName(), ` apiVersion: kustomize.config.k8s.io/v1beta1 kind: Component configMapGenerator: @@ -459,11 +457,23 @@ configMapGenerator: behavior: merge literals: - otherValue=9 -`) - err := th.RunWithErr("/app/prod", th.MakeDefaultOptions()) - if !strings.Contains( - err.Error(), - "apiVersion for Component should be kustomize.config.k8s.io/v1alpha1") { - t.Fatalf("unexpected error: %s", err) +`), + }, + runPath: "/app/prod", + expectedError: "apiVersion for Component should be kustomize.config.k8s.io/v1alpha1", + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + th := kusttest_test.MakeHarness(t) + for _, f := range tc.input { + f(th) + } + err := th.RunWithErr(tc.runPath, th.MakeDefaultOptions()) + if err == nil || !strings.Contains(err.Error(), tc.expectedError) { + t.Fatalf("unexpected error: %s", err) + } + }) } } diff --git a/api/testutils/kusttest/harness.go b/api/testutils/kusttest/harness.go index b35969670..d79e40e82 100644 --- a/api/testutils/kusttest/harness.go +++ b/api/testutils/kusttest/harness.go @@ -52,6 +52,16 @@ kind: Kustomization `+content)) } +func (th Harness) WriteC(path string, content string) { + th.fSys.WriteFile( + filepath.Join( + path, + konfig.DefaultKustomizationFileName()), []byte(` +apiVersion: kustomize.config.k8s.io/v1alpha1 +kind: Component +`+content)) +} + func (th Harness) WriteF(path string, content string) { th.fSys.WriteFile(path, []byte(content)) } From ca8e00fc158aec9ba681332f505272f22432a1c1 Mon Sep 17 00:00:00 2001 From: Paul Martin Date: Thu, 28 May 2020 21:43:21 +0100 Subject: [PATCH 8/9] Replace 'patch' with 'comp' in test data. --- api/krusty/component_test.go | 84 ++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/api/krusty/component_test.go b/api/krusty/component_test.go index 61ccdd518..28538dcb3 100644 --- a/api/krusty/component_test.go +++ b/api/krusty/component_test.go @@ -52,8 +52,8 @@ spec: } func writeTestComponent(th kusttest_test.Harness) { - th.WriteC("/app/patch", ` -namePrefix: patched- + th.WriteC("/app/comp", ` +namePrefix: comp- replicas: - name: storefront count: 3 @@ -64,9 +64,9 @@ configMapGenerator: behavior: merge literals: - testValue=2 - - patchValue=5 + - compValue=5 `) - th.WriteF("/app/patch/stub.yaml", ` + th.WriteF("/app/comp/stub.yaml", ` apiVersion: v1 kind: Deployment metadata: @@ -83,7 +83,7 @@ resources: - db components: -- ../patch +- ../comp `) writeDB(th) } @@ -114,39 +114,39 @@ func TestComponent(t *testing.T) { apiVersion: v1 kind: Deployment metadata: - name: patched-storefront + name: comp-storefront spec: replicas: 3 --- apiVersion: v1 data: + compValue: "5" otherValue: "10" - patchValue: "5" testValue: "2" kind: ConfigMap metadata: annotations: {} labels: {} - name: patched-my-configmap-5g55h28cdh + name: comp-my-configmap-ct5bgtbccd --- apiVersion: v1 kind: Deployment metadata: - name: patched-db + name: comp-db spec: type: Logical --- apiVersion: v1 kind: Deployment metadata: - name: patched-stub + name: comp-stub spec: replicas: 1 `, }, "multiple-components": { input: []FileGen{writeTestBase, writeTestComponent, writeDB, - writeC("/app/additionalpatch", ` + writeC("/app/additionalcomp", ` configMapGenerator: - name: my-configmap behavior: merge @@ -159,8 +159,8 @@ resources: - db components: -- ../patch -- ../additionalpatch +- ../comp +- ../additionalcomp `), }, runPath: "/app/prod", @@ -168,41 +168,41 @@ components: apiVersion: v1 kind: Deployment metadata: - name: patched-storefront + name: comp-storefront spec: replicas: 3 --- apiVersion: v1 data: + compValue: "5" otherValue: "9" - patchValue: "5" testValue: "2" kind: ConfigMap metadata: annotations: {} labels: {} - name: patched-my-configmap-9fddc87cdk + name: comp-my-configmap-dgf97tmg6h --- apiVersion: v1 kind: Deployment metadata: - name: patched-db + name: comp-db spec: type: Logical --- apiVersion: v1 kind: Deployment metadata: - name: patched-stub + name: comp-stub spec: replicas: 1 `, }, "nested-components": { input: []FileGen{writeTestBase, writeTestComponent, writeDB, - writeC("/app/additionalpatch", ` + writeC("/app/additionalcomp", ` components: -- ../patch +- ../comp configMapGenerator: - name: my-configmap behavior: merge @@ -215,7 +215,7 @@ resources: - db components: -- ../additionalpatch +- ../additionalcomp `), }, runPath: "/app/prod", @@ -223,38 +223,38 @@ components: apiVersion: v1 kind: Deployment metadata: - name: patched-storefront + name: comp-storefront spec: replicas: 3 --- apiVersion: v1 data: + compValue: "5" otherValue: "9" - patchValue: "5" testValue: "2" kind: ConfigMap metadata: annotations: {} labels: {} - name: patched-my-configmap-9fddc87cdk + name: comp-my-configmap-dgf97tmg6h --- apiVersion: v1 kind: Deployment metadata: - name: patched-db + name: comp-db spec: type: Logical --- apiVersion: v1 kind: Deployment metadata: - name: patched-stub + name: comp-stub spec: replicas: 1 `, }, - // If a patch sets a name prefix on a base, then that base can also be separately included - // without being affected by the patch in another branch of the resource tree + // If a component sets a name prefix on a base, then that base can also be separately included + // without being affected by the component in another branch of the resource tree "basic-component-with-repeated-base": { input: []FileGen{writeTestBase, writeTestComponent, writeOverlayProd, writeK("/app/repeated", ` @@ -283,32 +283,32 @@ metadata: apiVersion: v1 kind: Deployment metadata: - name: patched-storefront + name: comp-storefront spec: replicas: 3 --- apiVersion: v1 data: + compValue: "5" otherValue: "10" - patchValue: "5" testValue: "2" kind: ConfigMap metadata: annotations: {} labels: {} - name: patched-my-configmap-5g55h28cdh + name: comp-my-configmap-ct5bgtbccd --- apiVersion: v1 kind: Deployment metadata: - name: patched-db + name: comp-db spec: type: Logical --- apiVersion: v1 kind: Deployment metadata: - name: patched-stub + name: comp-stub spec: replicas: 1 `, @@ -322,7 +322,7 @@ configMapGenerator: - name: my-configmap behavior: merge literals: - - patchValue=5 + - compValue=5 - testValue=2 `), }, @@ -337,19 +337,19 @@ spec: --- apiVersion: v1 data: + compValue: "5" otherValue: "10" - patchValue: "5" testValue: "2" kind: ConfigMap metadata: annotations: {} labels: {} - name: my-configmap-t86ktk6tdk + name: my-configmap-96dt22k28h `, }, "missing-optional-component-api-version": { input: []FileGen{writeTestBase, writeOverlayProd, - writeF("/app/patch/"+konfig.DefaultKustomizationFileName(), ` + writeF("/app/comp/"+konfig.DefaultKustomizationFileName(), ` kind: Component configMapGenerator: - name: my-configmap @@ -410,18 +410,18 @@ func TestComponentErrors(t *testing.T) { writeK("/app/compinres", ` resources: - ../base -- ../patch +- ../comp `), }, runPath: "app/compinres", - expectedError: "expected kind != 'Component' for path '/app/patch'", + expectedError: "expected kind != 'Component' for path '/app/comp'", }, "kustomizations-cannot-be-added-to-components": { input: []FileGen{writeTestBase, writeTestComponent, writeK("/app/kustincomponents", ` components: - ../base -- ../patch +- ../comp `), }, runPath: "/app/kustincomponents", @@ -441,7 +441,7 @@ spec: writeK("/app/filesincomponents", ` components: - stub.yaml -- ../patch +- ../comp `), }, runPath: "/app/filesincomponents", @@ -449,7 +449,7 @@ components: }, "invalid-component-api-version": { input: []FileGen{writeTestBase, writeOverlayProd, - writeF("/app/patch/"+konfig.DefaultKustomizationFileName(), ` + writeF("/app/comp/"+konfig.DefaultKustomizationFileName(), ` apiVersion: kustomize.config.k8s.io/v1beta1 kind: Component configMapGenerator: From 98cb7fc8cd01b12976ad00b7abc8e1710ccc633e Mon Sep 17 00:00:00 2001 From: Paul Martin Date: Thu, 28 May 2020 22:42:52 +0100 Subject: [PATCH 9/9] Additional tests around reusing the same bases and components --- api/krusty/component_test.go | 197 ++++++++++++++++++++++++++++++++++- 1 file changed, 194 insertions(+), 3 deletions(-) diff --git a/api/krusty/component_test.go b/api/krusty/component_test.go index 28538dcb3..da9612ec4 100644 --- a/api/krusty/component_test.go +++ b/api/krusty/component_test.go @@ -4,6 +4,7 @@ package krusty_test import ( + "fmt" "strings" "testing" @@ -89,14 +90,18 @@ components: } func writeDB(th kusttest_test.Harness) { - th.WriteF("/app/prod/db", ` + deployment("db", "/app/prod/db")(th) +} + +func deployment(name string, path string) FileGen { + return writeF(path, fmt.Sprintf(` apiVersion: v1 kind: Deployment metadata: - name: db + name: %s spec: type: Logical -`) +`, name)) } func TestComponent(t *testing.T) { @@ -383,6 +388,119 @@ metadata: name: db spec: type: Logical +`, + }, + // See how nameSuffix "-b" is also added to the resources included by "comp-a" because they are in the + // accumulator when "comp-b" is accumulated. In practice we could use simple Kustomizations for this example. + "components-can-add-the-same-base-if-the-first-renames-resources": { + input: []FileGen{writeTestBase, + deployment("proxy", "/app/comp-a/proxy.yaml"), + writeC("/app/comp-a", ` +resources: +- ../base + +nameSuffix: "-a" +`), + writeC("/app/comp-b", ` +resources: +- ../base + +nameSuffix: "-b" +`), + writeK("/app/prod", ` +components: +- ../comp-a +- ../comp-b`), + }, + runPath: "/app/prod", + expectedOutput: ` +apiVersion: v1 +kind: Deployment +metadata: + name: storefront-a-b +spec: + replicas: 1 +--- +apiVersion: v1 +data: + otherValue: "10" + testValue: "1" +kind: ConfigMap +metadata: + name: my-configmap-a-b-tfb7c5t69m +--- +apiVersion: v1 +kind: Deployment +metadata: + name: storefront-b +spec: + replicas: 1 +--- +apiVersion: v1 +data: + otherValue: "10" + testValue: "1" +kind: ConfigMap +metadata: + name: my-configmap-b-8h7b8862bb +`, + }, + + "multiple-bases-can-add-the-same-component-if-it-doesn-not-define-named-entities": { + input: []FileGen{ + writeC("/app/comp", ` +namespace: prod +`), + writeK("/app/base-a", ` +resources: +- proxy.yaml + +components: +- ../comp +`), + deployment("proxy-a", "/app/base-a/proxy.yaml"), + writeK("/app/base-b", ` +resources: +- proxy.yaml + +components: +- ../comp +`), + deployment("proxy-b", "/app/base-b/proxy.yaml"), + writeK("/app/prod", ` +resources: +- proxy.yaml +- ../base-a +- ../base-b +`), + deployment("proxy-prod", "/app/prod/proxy.yaml"), + }, + runPath: "/app/prod", + // Note that the namepsace has not been applied to proxy-prod because it was not in scope when the + // component was applied + expectedOutput: ` +apiVersion: v1 +kind: Deployment +metadata: + name: proxy-prod +spec: + type: Logical +--- +apiVersion: v1 +kind: Deployment +metadata: + name: proxy-a + namespace: prod +spec: + type: Logical +--- +apiVersion: v1 +kind: Deployment +metadata: + name: proxy-b + namespace: prod +spec: + type: Logical `, }, } @@ -462,6 +580,79 @@ configMapGenerator: runPath: "/app/prod", expectedError: "apiVersion for Component should be kustomize.config.k8s.io/v1alpha1", }, + "components-cannot-add-the-same-resource": { + input: []FileGen{writeTestBase, + writeC("/app/comp-a", ` +resources: +- proxy.yaml +`), + deployment("proxy", "/app/comp-a/proxy.yaml"), + writeC("/app/comp-b", ` +resources: +- proxy.yaml +`), + deployment("proxy", "/app/comp-b/proxy.yaml"), + writeK("/app/prod", ` +resources: +- ../base + +components: +- ../comp-a +- ../comp-b`), + }, + runPath: "/app/prod", + expectedError: "may not add resource with an already registered id: ~G_v1_Deployment|~X|proxy", + }, + "components-cannot-add-the-same-base": { + input: []FileGen{writeTestBase, + deployment("proxy", "/app/comp-a/proxy.yaml"), + writeC("/app/comp-a", ` +resources: +- ../base +`), + writeC("/app/comp-b", ` +resources: +- ../base +`), + writeK("/app/prod", ` +components: +- ../comp-a +- ../comp-b`), + }, + runPath: "/app/prod", + expectedError: "may not add resource with an already registered id: ~G_v1_Deployment|~X|storefront", + }, + "components-cannot-add-bases-containing-the-same-resource": { + input: []FileGen{writeTestBase, + writeC("/app/comp-a", ` +resources: +- ../base-a +`), + writeK("/app/base-a", ` +resources: +- proxy.yaml +`), + deployment("proxy", "/app/base-a/proxy.yaml"), + writeC("/app/comp-b", ` +resources: +- ../base-b +`), + writeK("/app/base-b", ` +resources: +- proxy.yaml +`), + deployment("proxy", "/app/base-b/proxy.yaml"), + writeK("/app/prod", ` +resources: +- ../base + +components: +- ../comp-a +- ../comp-b`), + }, + runPath: "/app/prod", + expectedError: "may not add resource with an already registered id: ~G_v1_Deployment|~X|proxy", + }, } for tn, tc := range testCases {