diff --git a/api/internal/target/kusttarget.go b/api/internal/target/kusttarget.go index 36501a976..4cade158d 100644 --- a/api/internal/target/kusttarget.go +++ b/api/internal/target/kusttarget.go @@ -166,11 +166,21 @@ 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()) +} + +// 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) 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 { @@ -235,7 +245,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 } @@ -261,7 +271,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 } @@ -271,44 +281,81 @@ 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, false) 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 +} + +// 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) 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) 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() + 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 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 { - 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/component_test.go b/api/krusty/component_test.go new file mode 100644 index 000000000..da9612ec4 --- /dev/null +++ b/api/krusty/component_test.go @@ -0,0 +1,670 @@ +// Copyright 2020 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package krusty_test + +import ( + "fmt" + "strings" + "testing" + + "sigs.k8s.io/kustomize/api/konfig" + kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" +) + +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 +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 writeTestComponent(th kusttest_test.Harness) { + th.WriteC("/app/comp", ` +namePrefix: comp- +replicas: +- name: storefront + count: 3 +resources: + - stub.yaml +configMapGenerator: +- name: my-configmap + behavior: merge + literals: + - testValue=2 + - compValue=5 +`) + th.WriteF("/app/comp/stub.yaml", ` +apiVersion: v1 +kind: Deployment +metadata: + name: stub +spec: + replicas: 1 +`) +} + +func writeOverlayProd(th kusttest_test.Harness) { + th.WriteK("/app/prod", ` +resources: +- ../base +- db + +components: +- ../comp +`) + writeDB(th) +} + +func writeDB(th kusttest_test.Harness) { + deployment("db", "/app/prod/db")(th) +} + +func deployment(name string, path string) FileGen { + return writeF(path, fmt.Sprintf(` +apiVersion: v1 +kind: Deployment +metadata: + name: %s +spec: + type: Logical +`, name)) +} + +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: + name: comp-storefront +spec: + replicas: 3 +--- +apiVersion: v1 +data: + compValue: "5" + otherValue: "10" + testValue: "2" +kind: ConfigMap +metadata: + annotations: {} + labels: {} + name: comp-my-configmap-ct5bgtbccd +--- +apiVersion: v1 +kind: Deployment +metadata: + name: comp-db +spec: + type: Logical +--- +apiVersion: v1 +kind: Deployment +metadata: + name: comp-stub +spec: + replicas: 1 +`, + }, + "multiple-components": { + input: []FileGen{writeTestBase, writeTestComponent, writeDB, + writeC("/app/additionalcomp", ` +configMapGenerator: +- name: my-configmap + behavior: merge + literals: + - otherValue=9 +`), + writeK("/app/prod", ` +resources: +- ../base +- db + +components: +- ../comp +- ../additionalcomp +`), + }, + runPath: "/app/prod", + expectedOutput: ` +apiVersion: v1 +kind: Deployment +metadata: + name: comp-storefront +spec: + replicas: 3 +--- +apiVersion: v1 +data: + compValue: "5" + otherValue: "9" + testValue: "2" +kind: ConfigMap +metadata: + annotations: {} + labels: {} + name: comp-my-configmap-dgf97tmg6h +--- +apiVersion: v1 +kind: Deployment +metadata: + name: comp-db +spec: + type: Logical +--- +apiVersion: v1 +kind: Deployment +metadata: + name: comp-stub +spec: + replicas: 1 +`, + }, + "nested-components": { + input: []FileGen{writeTestBase, writeTestComponent, writeDB, + writeC("/app/additionalcomp", ` +components: +- ../comp +configMapGenerator: +- name: my-configmap + behavior: merge + literals: + - otherValue=9 +`), + writeK("/app/prod", ` +resources: +- ../base +- db + +components: +- ../additionalcomp +`), + }, + runPath: "/app/prod", + expectedOutput: ` +apiVersion: v1 +kind: Deployment +metadata: + name: comp-storefront +spec: + replicas: 3 +--- +apiVersion: v1 +data: + compValue: "5" + otherValue: "9" + testValue: "2" +kind: ConfigMap +metadata: + annotations: {} + labels: {} + name: comp-my-configmap-dgf97tmg6h +--- +apiVersion: v1 +kind: Deployment +metadata: + name: comp-db +spec: + type: Logical +--- +apiVersion: v1 +kind: Deployment +metadata: + name: comp-stub +spec: + replicas: 1 +`, + }, + // 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", ` +resources: +- ../base +- ../prod +`), + }, + runPath: "/app/repeated", + expectedOutput: ` +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: comp-storefront +spec: + replicas: 3 +--- +apiVersion: v1 +data: + compValue: "5" + otherValue: "10" + testValue: "2" +kind: ConfigMap +metadata: + annotations: {} + labels: {} + name: comp-my-configmap-ct5bgtbccd +--- +apiVersion: v1 +kind: Deployment +metadata: + name: comp-db +spec: + type: Logical +--- +apiVersion: v1 +kind: Deployment +metadata: + name: comp-stub +spec: + replicas: 1 +`, + }, + "applying-component-directly-should-be-same-as-kustomization": { + input: []FileGen{writeTestBase, writeTestComponent, + writeC("/app/direct-component", ` +resources: +- ../base +configMapGenerator: +- name: my-configmap + behavior: merge + literals: + - compValue=5 + - testValue=2 +`), + }, + runPath: "/app/direct-component", + expectedOutput: ` +apiVersion: v1 +kind: Deployment +metadata: + name: storefront +spec: + replicas: 1 +--- +apiVersion: v1 +data: + compValue: "5" + otherValue: "10" + testValue: "2" +kind: ConfigMap +metadata: + annotations: {} + labels: {} + name: my-configmap-96dt22k28h +`, + }, + "missing-optional-component-api-version": { + input: []FileGen{writeTestBase, writeOverlayProd, + writeF("/app/comp/"+konfig.DefaultKustomizationFileName(), ` +kind: Component +configMapGenerator: +- name: my-configmap + behavior: merge + literals: + - otherValue=9 +`), + }, + runPath: "/app/prod", + expectedOutput: ` +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 +`, + }, + // 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 +`, + }, + } + + 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 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 +- ../comp +`), + }, + runPath: "app/compinres", + expectedError: "expected kind != 'Component' for path '/app/comp'", + }, + "kustomizations-cannot-be-added-to-components": { + input: []FileGen{writeTestBase, writeTestComponent, + writeK("/app/kustincomponents", ` +components: +- ../base +- ../comp +`), + }, + 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 +- ../comp +`), + }, + 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/comp/"+konfig.DefaultKustomizationFileName(), ` +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Component +configMapGenerator: +- name: my-configmap + behavior: merge + literals: + - otherValue=9 +`), + }, + 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 { + 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)) } diff --git a/api/types/kustomization.go b/api/types/kustomization.go index 4be803f53..4a2c42d36 100644 --- a/api/types/kustomization.go +++ b/api/types/kustomization.go @@ -6,6 +6,8 @@ package types const ( KustomizationVersion = "kustomize.config.k8s.io/v1beta1" KustomizationKind = "Kustomization" + ComponentVersion = "kustomize.config.k8s.io/v1alpha1" + ComponentKind = "Component" MetadataNamespacePath = "metadata/namespace" ) @@ -73,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. @@ -128,23 +134,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) } - if k.Kind != "" && k.Kind != KustomizationKind { - errs = append(errs, "kind should be "+KustomizationKind) + requiredVersion := KustomizationVersion + if k.Kind == ComponentKind { + requiredVersion = ComponentVersion + } + if k.APIVersion != "" && k.APIVersion != requiredVersion { + errs = append(errs, "apiVersion for "+k.Kind+" should be "+requiredVersion) } return errs }