diff --git a/api/internal/builtins/HelmChartInflationGenerator.go b/api/internal/builtins/HelmChartInflationGenerator.go index aed01081a2..e50c824ea5 100644 --- a/api/internal/builtins/HelmChartInflationGenerator.go +++ b/api/internal/builtins/HelmChartInflationGenerator.go @@ -363,6 +363,11 @@ func (p *HelmChartInflationGeneratorPlugin) markHelmGeneratedResources(rm resmap if err := r.RNode.PipeE(kyaml.SetAnnotation(konfig.HelmGeneratedAnnotation, "true")); err != nil { return fmt.Errorf("failed to set helm annotation: %w", err) } + if p.Namespace != "" { + if err := r.RNode.PipeE(kyaml.SetAnnotation(konfig.HelmChartNamespaceAnnotation, p.Namespace)); err != nil { + return fmt.Errorf("failed to set helm namespace annotation: %w", err) + } + } } return nil } diff --git a/api/internal/builtins/NamespaceTransformer.go b/api/internal/builtins/NamespaceTransformer.go index 3b98195b3f..303e2ad707 100644 --- a/api/internal/builtins/NamespaceTransformer.go +++ b/api/internal/builtins/NamespaceTransformer.go @@ -52,16 +52,21 @@ func (p *NamespaceTransformerPlugin) Transform(m resmap.ResMap) error { // Don't mutate empty objects? continue } - if annotations := r.GetAnnotations(konfig.HelmGeneratedAnnotation); annotations[konfig.HelmGeneratedAnnotation] == "true" { - // Don't apply namespace on Helm generated manifest. Helm should take care of it. - continue + transformedNamespace := p.Namespace + unsetOnly := p.UnsetOnly + if annotations := r.GetAnnotations(); annotations[konfig.HelmGeneratedAnnotation] == "true" { + // Preserve namespaces emitted by Helm, but still fill any missing namespace fields. + unsetOnly = true + if helmNamespace := annotations[konfig.HelmChartNamespaceAnnotation]; helmNamespace != "" { + transformedNamespace = helmNamespace + } } r.StorePreviousId() if err := r.ApplyFilter(namespace.Filter{ - Namespace: p.Namespace, + Namespace: transformedNamespace, FsSlice: p.FieldSpecs, SetRoleBindingSubjects: p.SetRoleBindingSubjects, - UnsetOnly: p.UnsetOnly, + UnsetOnly: unsetOnly, }); err != nil { return err } diff --git a/api/konfig/general.go b/api/konfig/general.go index c661883883..b2728ac692 100644 --- a/api/konfig/general.go +++ b/api/konfig/general.go @@ -49,4 +49,7 @@ const ( // Annotation key for marking helm-generated resources to skip namespace transformation HelmGeneratedAnnotation = ConfigAnnoDomain + "/helm-generated" + + // Annotation key for preserving the effective Helm chart namespace during build-time transforms. + HelmChartNamespaceAnnotation = ConfigAnnoDomain + "/helm-chart-namespace" ) diff --git a/api/krusty/helmchartinflationgenerator_test.go b/api/krusty/helmchartinflationgenerator_test.go index 2cc4b4acc0..386bfa03b3 100644 --- a/api/krusty/helmchartinflationgenerator_test.go +++ b/api/krusty/helmchartinflationgenerator_test.go @@ -1179,6 +1179,52 @@ metadata: `) } +func TestHelmChartDifferentNamespacesMissingResourceNamespace(t *testing.T) { + th := kusttest_test.MakeEnhancedHarnessWithTmpRoot(t) + defer th.Reset() + if err := th.ErrIfNoHelm(); err != nil { + t.Skip("skipping: " + err.Error()) + } + + chartDir := filepath.Join(th.GetRoot(), "charts", "service") + require.NoError(t, th.GetFSys().MkdirAll(filepath.Join(chartDir, "templates"))) + th.WriteF(filepath.Join(chartDir, "Chart.yaml"), ` +apiVersion: v2 +name: service +type: application +version: 1.0.0 +`) + th.WriteF(filepath.Join(chartDir, "values.yaml"), ``) + th.WriteF(filepath.Join(chartDir, "templates", "service.yaml"), ` +apiVersion: v1 +kind: Service +metadata: + name: test-service + annotations: + release-namespace: {{ .Release.Namespace }} +`) + + th.WriteK(th.GetRoot(), ` +helmGlobals: + chartHome: ./charts +namespace: transformer-ns +helmCharts: + - name: service + releaseName: service + namespace: helm-ns +`) + + m := th.Run(th.GetRoot(), th.MakeOptionsPluginsEnabled()) + th.AssertActualEqualsExpected(m, `apiVersion: v1 +kind: Service +metadata: + annotations: + release-namespace: helm-ns + name: test-service + namespace: helm-ns +`) +} + func TestHelmChartSameNamespace(t *testing.T) { th := kusttest_test.MakeEnhancedHarnessWithTmpRoot(t) defer th.Reset() diff --git a/api/resource/resource.go b/api/resource/resource.go index dc6995799b..ae7bdfdb44 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -50,6 +50,7 @@ var BuildAnnotations = []string{ kioutil.LegacyIdAnnotation, konfig.HelmGeneratedAnnotation, + konfig.HelmChartNamespaceAnnotation, } func (r *Resource) ResetRNode(incoming *Resource) { @@ -160,7 +161,9 @@ func (r *Resource) DeepCopy() *Resource { // the resource. // TODO: move to RNode, use GetMeta to improve performance. // TODO: make a version of mergeStringMaps that is build-annotation aware -// to avoid repeatedly setting refby and genargs annotations +// +// to avoid repeatedly setting refby and genargs annotations +// // Must remove the kustomize bit at the end. func (r *Resource) CopyMergeMetaDataFieldsFrom(other *Resource) error { if err := r.SetLabels( diff --git a/examples/README.md b/examples/README.md index a4feae368e..1058249b87 100644 --- a/examples/README.md +++ b/examples/README.md @@ -42,6 +42,8 @@ Advanced Usage * [last mile helm](chart.md) - Make last mile modifications to a helm chart. + * [helm namespace inheritance](helmNamespace/README.md) - Verify that an overlay namespace fills unset namespaces in Helm output without overwriting explicit chart namespaces. + * [secret generation](secretGeneratorPlugin.md) - Generating secrets from a plugin. * [remote sources](goGetterGeneratorPlugin.md) - Generating from remote sources. diff --git a/examples/helmNamespace/README.md b/examples/helmNamespace/README.md new file mode 100644 index 0000000000..0533735f7f --- /dev/null +++ b/examples/helmNamespace/README.md @@ -0,0 +1,78 @@ +# Helm Namespace Example + +This example exercises namespace transformation on resources generated from a local Helm chart. + +The example kustomization sets a namespace for Helm-generated resources that do not already declare one, while preserving any namespace explicitly emitted by the chart. + +## Build the example + +This example defines the `helm` command as: + + +```sh +helmCommand=${MYGOBIN:-~/go/bin}/helmV3 +``` + +Use the checked-in example: + + +```sh +EXAMPLE_HOME=examples/helmNamespace +``` + +Build the example with Helm enabled: + + +```sh +output=$(kustomize build \ + --enable-helm \ + --helm-command "$helmCommand" \ + "$EXAMPLE_HOME") +printf '%s\n' "$output" +``` + +## Helm Chart with Namespace in `helmChart.namespace` + +The Service is emitted by the chart without a namespace, so the example namespace should be applied: + + +```sh +printf '%s\n' "$output" | grep -A4 'name: test-a-service' | grep 'namespace: chart-ns' +``` + +The ConfigMap is emitted by the chart with an explicit namespace, so that value should be preserved: + + +```sh +printf '%s\n' "$output" | grep -A4 'name: test-a-config' | grep 'namespace: chart-owned-ns' +``` + +The Secret is emitted by the chart with an release namespace, so that value should be preserved: + + +```sh +printf '%s\n' "$output" | grep -A4 'name: test-a-secret' | grep 'namespace: chart-ns' +``` + +## Helm Chart without Namespace in `helmChart.namespace` + +The Service is emitted by the chart without a namespace, so the example namespace should be applied: + + +```sh +printf '%s\n' "$output" | grep -A4 'name: test-b-service' | grep 'namespace: top-level-ns' +``` + +The ConfigMap is emitted by the chart with an explicit namespace, so that value should be preserved: + + +```sh +printf '%s\n' "$output" | grep -A4 'name: test-b-config' | grep 'namespace: chart-owned-ns' +``` + +The Secret is emitted by the chart with an release namespace, so that value should be preserved: + + +```sh +printf '%s\n' "$output" | grep -A4 'name: test-b-secret' | grep 'namespace: top-level-ns' +``` diff --git a/examples/helmNamespace/charts/namespace-fill/Chart.yaml b/examples/helmNamespace/charts/namespace-fill/Chart.yaml new file mode 100644 index 0000000000..64222aa72b --- /dev/null +++ b/examples/helmNamespace/charts/namespace-fill/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v2 +name: namespace-fill +version: 0.1.0 +type: application diff --git a/examples/helmNamespace/charts/namespace-fill/templates/configmap.yaml b/examples/helmNamespace/charts/namespace-fill/templates/configmap.yaml new file mode 100644 index 0000000000..dcdf491720 --- /dev/null +++ b/examples/helmNamespace/charts/namespace-fill/templates/configmap.yaml @@ -0,0 +1,7 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: {{ .Release.Name }}-config + namespace: chart-owned-ns +data: + owner: helm-chart diff --git a/examples/helmNamespace/charts/namespace-fill/templates/secrets.yaml b/examples/helmNamespace/charts/namespace-fill/templates/secrets.yaml new file mode 100644 index 0000000000..15a55612e8 --- /dev/null +++ b/examples/helmNamespace/charts/namespace-fill/templates/secrets.yaml @@ -0,0 +1,7 @@ +apiVersion: v1 +kind: Secret +metadata: + name: {{ .Release.Name }}-secret + namespace: {{ .Release.Namespace }} +data: + .secret-file: dmFsdWUtMg0KDQo= diff --git a/examples/helmNamespace/charts/namespace-fill/templates/service.yaml b/examples/helmNamespace/charts/namespace-fill/templates/service.yaml new file mode 100644 index 0000000000..9205b8ff59 --- /dev/null +++ b/examples/helmNamespace/charts/namespace-fill/templates/service.yaml @@ -0,0 +1,10 @@ +apiVersion: v1 +kind: Service +metadata: + name: {{ .Release.Name }}-service +spec: + selector: + app: app-service + ports: + - port: 80 + targetPort: 8080 diff --git a/examples/helmNamespace/charts/namespace-fill/values.yaml b/examples/helmNamespace/charts/namespace-fill/values.yaml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/examples/helmNamespace/kustomization.yaml b/examples/helmNamespace/kustomization.yaml new file mode 100644 index 0000000000..8a115fdfa2 --- /dev/null +++ b/examples/helmNamespace/kustomization.yaml @@ -0,0 +1,16 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +namespace: top-level-ns + +helmGlobals: + chartHome: charts + +helmCharts: +# test a contains a namespace +- name: namespace-fill + releaseName: test-a + namespace: chart-ns +# test b inherits the namespace from top level +- name: namespace-fill + releaseName: test-b diff --git a/hack/testExamplesAgainstKustomize.sh b/hack/testExamplesAgainstKustomize.sh index f1a85ad906..403132788e 100755 --- a/hack/testExamplesAgainstKustomize.sh +++ b/hack/testExamplesAgainstKustomize.sh @@ -39,6 +39,7 @@ if onLinuxAndNotOnRemoteCI; then echo "On linux, and not on remote CI. Running helm tests." make $MYGOBIN/helmV3 mdrip --mode test --label testHelm examples/chart.md + mdrip --mode test --label testHelm examples/helmNamespace/README.md else echo "Skipping helm tests against $version." echo "Helm chart inflator has new features (includeCRD) only in HEAD." diff --git a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go index 105bf9b94e..d3b6708429 100644 --- a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go +++ b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go @@ -371,6 +371,11 @@ func (p *plugin) markHelmGeneratedResources(rm resmap.ResMap) error { if err := r.RNode.PipeE(kyaml.SetAnnotation(konfig.HelmGeneratedAnnotation, "true")); err != nil { return fmt.Errorf("failed to set helm annotation: %w", err) } + if p.Namespace != "" { + if err := r.RNode.PipeE(kyaml.SetAnnotation(konfig.HelmChartNamespaceAnnotation, p.Namespace)); err != nil { + return fmt.Errorf("failed to set helm namespace annotation: %w", err) + } + } } return nil } diff --git a/plugin/builtin/namespacetransformer/NamespaceTransformer.go b/plugin/builtin/namespacetransformer/NamespaceTransformer.go index b59c8aa463..5d7daae4b9 100644 --- a/plugin/builtin/namespacetransformer/NamespaceTransformer.go +++ b/plugin/builtin/namespacetransformer/NamespaceTransformer.go @@ -57,16 +57,21 @@ func (p *plugin) Transform(m resmap.ResMap) error { // Don't mutate empty objects? continue } - if annotations := r.GetAnnotations(konfig.HelmGeneratedAnnotation); annotations[konfig.HelmGeneratedAnnotation] == "true" { - // Don't apply namespace on Helm generated manifest. Helm should take care of it. - continue + transformedNamespace := p.Namespace + unsetOnly := p.UnsetOnly + if annotations := r.GetAnnotations(); annotations[konfig.HelmGeneratedAnnotation] == "true" { + // Preserve namespaces emitted by Helm, but still fill any missing namespace fields. + unsetOnly = true + if helmNamespace := annotations[konfig.HelmChartNamespaceAnnotation]; helmNamespace != "" { + transformedNamespace = helmNamespace + } } r.StorePreviousId() if err := r.ApplyFilter(namespace.Filter{ - Namespace: p.Namespace, + Namespace: transformedNamespace, FsSlice: p.FieldSpecs, SetRoleBindingSubjects: p.SetRoleBindingSubjects, - UnsetOnly: p.UnsetOnly, + UnsetOnly: unsetOnly, }); err != nil { return err } diff --git a/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go b/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go index bddfa372ad..3b1b049198 100644 --- a/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go +++ b/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go @@ -768,7 +768,7 @@ subjects: }) } -func TestNamespaceTransformer_SkipHelmOrigin(t *testing.T) { +func TestNamespaceTransformer_HelmOriginPreservesExistingNamespace(t *testing.T) { th := kusttest_test.MakeEnhancedHarness(t). PrepBuiltin("NamespaceTransformer") defer th.Reset() @@ -804,3 +804,76 @@ metadata: namespace: helm-ns `) } + +func TestNamespaceTransformer_HelmOriginSetsMissingNamespace(t *testing.T) { + th := kusttest_test.MakeEnhancedHarness(t). + PrepBuiltin("NamespaceTransformer") + defer th.Reset() + + rmF := resmap.NewFactory(provider.NewDefaultDepProvider().GetResourceFactory()) + rm, err := rmF.NewResMapFromBytes([]byte(`apiVersion: v1 +kind: Service +metadata: + name: svc + annotations: + this-should-be-keept: "true" +`)) + require.NoError(t, err) + r := rm.Resources()[0] + require.NoError(t, r.RNode.PipeE(kyaml.SetAnnotation(konfig.HelmGeneratedAnnotation, "true"))) + + rm, err = th.RunTransformerFromResMap(` +apiVersion: builtin +kind: NamespaceTransformer +metadata: + name: notImportantHere + namespace: test +`+defaultFieldSpecs, rm) + require.NoError(t, err) + require.NoError(t, rm.RemoveOriginAnnotations()) + th.AssertActualEqualsExpectedNoIdAnnotations(rm, `apiVersion: v1 +kind: Service +metadata: + annotations: + this-should-be-keept: "true" + name: svc + namespace: test +`) +} + +func TestNamespaceTransformer_HelmChartNamespaceTakesPriorityForMissingNamespace(t *testing.T) { + th := kusttest_test.MakeEnhancedHarness(t). + PrepBuiltin("NamespaceTransformer") + defer th.Reset() + + rmF := resmap.NewFactory(provider.NewDefaultDepProvider().GetResourceFactory()) + rm, err := rmF.NewResMapFromBytes([]byte(`apiVersion: v1 +kind: Service +metadata: + name: svc + annotations: + this-should-be-keept: "true" +`)) + require.NoError(t, err) + r := rm.Resources()[0] + require.NoError(t, r.RNode.PipeE(kyaml.SetAnnotation(konfig.HelmGeneratedAnnotation, "true"))) + require.NoError(t, r.RNode.PipeE(kyaml.SetAnnotation(konfig.HelmChartNamespaceAnnotation, "helm-ns"))) + + rm, err = th.RunTransformerFromResMap(` +apiVersion: builtin +kind: NamespaceTransformer +metadata: + name: notImportantHere + namespace: top-level-ns +`+defaultFieldSpecs, rm) + require.NoError(t, err) + require.NoError(t, rm.RemoveOriginAnnotations()) + th.AssertActualEqualsExpectedNoIdAnnotations(rm, `apiVersion: v1 +kind: Service +metadata: + annotations: + this-should-be-keept: "true" + name: svc + namespace: helm-ns +`) +}