From a48fe95d27827dd64fe9032db2be0befb81171c1 Mon Sep 17 00:00:00 2001 From: Ramon Quitales Date: Tue, 12 Dec 2023 09:10:46 -0800 Subject: [PATCH] Fix Helm Chart resource lookup key handling for objects in default namespace (#2655) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This pull request modifies the `GetResource` Go method for the Helm Chart resource. Previously, when a resource was deployed to the default namespace, the `default` namespace was always omitted from the resource lookup key. However, in cases where the namespace is explicitly defined in the Helm chart template, the `default` namespace was inadvertently excluded from the resource lookup key despite it being needed. This behavior in Helm is documented in the upstream issue: https://github.com/helm/helm/issues/3553. **Changes Made:** - Implemented a fallback mechanism to include the `default` namespace in the resource lookup key when necessary for the `GetResource` method. **Test Added:** - Added a test (`ChartGetResource`) to verify the `GetResource` method for both types of Helm charts—those with the explicitly defined default namespace and those without. **Verification:** - Verified that the modified `GetResource` method successfully handles scenarios where the namespace is explicitly defined in the Helm chart template. - Confirmed that the added test covers both types of Helm charts and fails when the fallback logic is reverted. Fixes: #2638 --- CHANGELOG.md | 1 + .../pkg/gen/_go-templates/helm/v3/chart.go | 16 ++++- sdk/go/kubernetes/helm/v3/chart.go | 16 ++++- tests/sdk/go/go_test.go | 17 +++++ .../step1/Pulumi.yaml | 3 + .../step1/local-chart/.helmignore | 23 +++++++ .../step1/local-chart/Chart.yaml | 24 +++++++ .../step1/local-chart/templates/_helpers.tpl | 62 +++++++++++++++++++ .../local-chart/templates/deployment.yaml | 36 +++++++++++ .../step1/local-chart/values.yaml | 15 +++++ .../helm-get-default-namespace/step1/main.go | 35 +++++++++++ .../step2/Pulumi.yaml | 3 + .../step2/local-chart/Chart.yaml | 24 +++++++ .../local-chart/templates/deployment.yaml | 37 +++++++++++ .../helm-get-default-namespace/step2/main.go | 35 +++++++++++ 15 files changed, 345 insertions(+), 2 deletions(-) create mode 100644 tests/sdk/go/helm-get-default-namespace/step1/Pulumi.yaml create mode 100644 tests/sdk/go/helm-get-default-namespace/step1/local-chart/.helmignore create mode 100644 tests/sdk/go/helm-get-default-namespace/step1/local-chart/Chart.yaml create mode 100644 tests/sdk/go/helm-get-default-namespace/step1/local-chart/templates/_helpers.tpl create mode 100644 tests/sdk/go/helm-get-default-namespace/step1/local-chart/templates/deployment.yaml create mode 100644 tests/sdk/go/helm-get-default-namespace/step1/local-chart/values.yaml create mode 100644 tests/sdk/go/helm-get-default-namespace/step1/main.go create mode 100644 tests/sdk/go/helm-get-default-namespace/step2/Pulumi.yaml create mode 100644 tests/sdk/go/helm-get-default-namespace/step2/local-chart/Chart.yaml create mode 100644 tests/sdk/go/helm-get-default-namespace/step2/local-chart/templates/deployment.yaml create mode 100644 tests/sdk/go/helm-get-default-namespace/step2/main.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 6659a6418e..471b344fe9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## Unreleased - Fix: Refine URN lookup by using its core type for more accurate resource identification (https://github.com/pulumi/pulumi-kubernetes/issues/2719) +- Fix Helm Chart resource lookup key handling for objects in default namespace (https://github.com/pulumi/pulumi-kubernetes/pull/2655) ## 4.5.5 (November 28, 2023) - Fix: Make the invoke calls for Helm charts and YAML config resilient to the value being None or an empty dict (https://github.com/pulumi/pulumi-kubernetes/pull/2665) diff --git a/provider/pkg/gen/_go-templates/helm/v3/chart.go b/provider/pkg/gen/_go-templates/helm/v3/chart.go index 3bf3065005..91cac28fad 100644 --- a/provider/pkg/gen/_go-templates/helm/v3/chart.go +++ b/provider/pkg/gen/_go-templates/helm/v3/chart.go @@ -351,9 +351,23 @@ func (c *Chart) GetResource(gvk, name, namespace string) pulumi.AnyOutput { if len(namespace) > 0 && namespace != "default" { id = fmt.Sprintf("%s/%s", namespace, name) } + key := fmt.Sprintf("%s::%s", gvk, id) + return c.Resources.ApplyT(func(x interface{}) interface{} { resources := x.(map[string]pulumi.Resource) - return resources[key] + if r, ok := resources[key]; ok { + return r + } + + // Some resources in the default namespace do not adhere to the name only pattern, so we need to fallback + // to searching for the resource by namespace and name. This occurs when the Helm template manifest explicitly + // sets the namespace field on the resource. + if namespace == "default" || namespace == "" { + return resources[fmt.Sprintf("%s::default/%s", gvk, name)] + } + + return nil + }).(pulumi.AnyOutput) } diff --git a/sdk/go/kubernetes/helm/v3/chart.go b/sdk/go/kubernetes/helm/v3/chart.go index 3bf3065005..91cac28fad 100644 --- a/sdk/go/kubernetes/helm/v3/chart.go +++ b/sdk/go/kubernetes/helm/v3/chart.go @@ -351,9 +351,23 @@ func (c *Chart) GetResource(gvk, name, namespace string) pulumi.AnyOutput { if len(namespace) > 0 && namespace != "default" { id = fmt.Sprintf("%s/%s", namespace, name) } + key := fmt.Sprintf("%s::%s", gvk, id) + return c.Resources.ApplyT(func(x interface{}) interface{} { resources := x.(map[string]pulumi.Resource) - return resources[key] + if r, ok := resources[key]; ok { + return r + } + + // Some resources in the default namespace do not adhere to the name only pattern, so we need to fallback + // to searching for the resource by namespace and name. This occurs when the Helm template manifest explicitly + // sets the namespace field on the resource. + if namespace == "default" || namespace == "" { + return resources[fmt.Sprintf("%s::default/%s", gvk, name)] + } + + return nil + }).(pulumi.AnyOutput) } diff --git a/tests/sdk/go/go_test.go b/tests/sdk/go/go_test.go index 88353097dc..28b06f85fc 100644 --- a/tests/sdk/go/go_test.go +++ b/tests/sdk/go/go_test.go @@ -749,4 +749,21 @@ func TestGo(t *testing.T) { }) integration.ProgramTest(t, &test) }) + + // Test to ensure that we can get a resource from the default namespace. This uses the wordpress chart as it requires the + // default namespace to be present in the GVK get request. + t.Run("ChartGetResource", func(t *testing.T) { + options := baseOptions.With(integration.ProgramTestOptions{ + Dir: filepath.Join(cwd, "helm-get-default-namespace", "step1"), + Quick: true, + EditDirs: []integration.EditDir{ + { + Dir: filepath.Join(cwd, "helm-get-default-namespace", "step2"), + Additive: true, + ExpectNoChanges: false, + }, + }, + }) + integration.ProgramTest(t, &options) + }) } diff --git a/tests/sdk/go/helm-get-default-namespace/step1/Pulumi.yaml b/tests/sdk/go/helm-get-default-namespace/step1/Pulumi.yaml new file mode 100644 index 0000000000..6d405395f4 --- /dev/null +++ b/tests/sdk/go/helm-get-default-namespace/step1/Pulumi.yaml @@ -0,0 +1,3 @@ +name: go_helm_local_kubernetes +description: Test Kubernetes Helm package with local Chart. +runtime: go diff --git a/tests/sdk/go/helm-get-default-namespace/step1/local-chart/.helmignore b/tests/sdk/go/helm-get-default-namespace/step1/local-chart/.helmignore new file mode 100644 index 0000000000..691fa13d6a --- /dev/null +++ b/tests/sdk/go/helm-get-default-namespace/step1/local-chart/.helmignore @@ -0,0 +1,23 @@ +# Patterns to ignore when building packages. +# This supports shell glob matching, relative path matching, and +# negation (prefixed with !). Only one pattern per line. +.DS_Store +# Common VCS dirs +.git/ +.gitignore +.bzr/ +.bzrignore +.hg/ +.hgignore +.svn/ +# Common backup files +*.swp +*.bak +*.tmp +*.orig +*~ +# Various IDEs +.project +.idea/ +*.tmproj +.vscode/ \ No newline at end of file diff --git a/tests/sdk/go/helm-get-default-namespace/step1/local-chart/Chart.yaml b/tests/sdk/go/helm-get-default-namespace/step1/local-chart/Chart.yaml new file mode 100644 index 0000000000..d1f3453973 --- /dev/null +++ b/tests/sdk/go/helm-get-default-namespace/step1/local-chart/Chart.yaml @@ -0,0 +1,24 @@ +apiVersion: v2 +name: local-test-chart +description: A Helm chart for Kubernetes + +# A chart can be either an 'application' or a 'library' chart. +# +# Application charts are a collection of templates that can be packaged into versioned archives +# to be deployed. +# +# Library charts provide useful utilities or functions for the chart developer. They're included as +# a dependency of application charts to inject those utilities and functions into the rendering +# pipeline. Library charts do not define any templates and therefore cannot be deployed. +type: application + +# This is the chart version. This version number should be incremented each time you make changes +# to the chart and its templates, including the app version. +# Versions are expected to follow Semantic Versioning (https://semver.org/) +version: 0.1.0 + +# This is the version number of the application being deployed. This version number should be +# incremented each time you make changes to the application. Versions are not expected to +# follow Semantic Versioning. They should reflect the version the application is using. +# It is recommended to use it with quotes. +appVersion: "1.16.0" \ No newline at end of file diff --git a/tests/sdk/go/helm-get-default-namespace/step1/local-chart/templates/_helpers.tpl b/tests/sdk/go/helm-get-default-namespace/step1/local-chart/templates/_helpers.tpl new file mode 100644 index 0000000000..14eb2b765b --- /dev/null +++ b/tests/sdk/go/helm-get-default-namespace/step1/local-chart/templates/_helpers.tpl @@ -0,0 +1,62 @@ +{{/* +Expand the name of the chart. +*/}} +{{- define "local-test-chart.name" -}} +{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }} +{{- end }} + +{{/* +Create a default fully qualified app name. +We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec). +If release name contains chart name it will be used as a full name. +*/}} +{{- define "local-test-chart.fullname" -}} +{{- if .Values.fullnameOverride }} +{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }} +{{- else }} +{{- $name := default .Chart.Name .Values.nameOverride }} +{{- if contains $name .Release.Name }} +{{- .Release.Name | trunc 63 | trimSuffix "-" }} +{{- else }} +{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }} +{{- end }} +{{- end }} +{{- end }} + +{{/* +Create chart name and version as used by the chart label. +*/}} +{{- define "local-test-chart.chart" -}} +{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" }} +{{- end }} + +{{/* +Common labels +*/}} +{{- define "local-test-chart.labels" -}} +helm.sh/chart: {{ include "local-test-chart.chart" . }} +{{ include "local-test-chart.selectorLabels" . }} +{{- if .Chart.AppVersion }} +app.kubernetes.io/version: {{ .Chart.AppVersion | quote }} +{{- end }} +app.kubernetes.io/managed-by: {{ .Release.Service }} +{{- end }} + +{{/* +Selector labels +*/}} +{{- define "local-test-chart.selectorLabels" -}} +app.kubernetes.io/name: {{ include "local-test-chart.name" . }} +app.kubernetes.io/instance: {{ .Release.Name }} +{{- end }} + +{{/* +Create the name of the service account to use +*/}} +{{- define "local-test-chart.serviceAccountName" -}} +{{- if .Values.serviceAccount.create }} +{{- default (include "local-test-chart.fullname" .) .Values.serviceAccount.name }} +{{- else }} +{{- default "default" .Values.serviceAccount.name }} +{{- end }} +{{- end }} \ No newline at end of file diff --git a/tests/sdk/go/helm-get-default-namespace/step1/local-chart/templates/deployment.yaml b/tests/sdk/go/helm-get-default-namespace/step1/local-chart/templates/deployment.yaml new file mode 100644 index 0000000000..335c58309a --- /dev/null +++ b/tests/sdk/go/helm-get-default-namespace/step1/local-chart/templates/deployment.yaml @@ -0,0 +1,36 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: {{ include "local-test-chart.fullname" . }} + labels: + {{- include "local-test-chart.labels" . | nindent 4 }} +spec: + replicas: {{ .Values.replicaCount }} + selector: + matchLabels: + {{- include "local-test-chart.selectorLabels" . | nindent 6 }} + template: + metadata: + labels: + {{- include "local-test-chart.selectorLabels" . | nindent 8 }} + spec: + {{- with .Values.imagePullSecrets }} + imagePullSecrets: + {{- toYaml . | nindent 8 }} + {{- end }} + containers: + - name: {{ .Chart.Name }} + image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}" + imagePullPolicy: {{ .Values.image.pullPolicy }} + ports: + - name: http + containerPort: 80 + protocol: TCP + livenessProbe: + httpGet: + path: / + port: http + readinessProbe: + httpGet: + path: / + port: http \ No newline at end of file diff --git a/tests/sdk/go/helm-get-default-namespace/step1/local-chart/values.yaml b/tests/sdk/go/helm-get-default-namespace/step1/local-chart/values.yaml new file mode 100644 index 0000000000..1b3715e789 --- /dev/null +++ b/tests/sdk/go/helm-get-default-namespace/step1/local-chart/values.yaml @@ -0,0 +1,15 @@ +# Default values for local-test-chart. +# This is a YAML-formatted file. +# Declare variables to be passed into your templates. + +replicaCount: 1 + +image: + repository: nginx + pullPolicy: IfNotPresent + # Overrides the image tag whose default is the chart appVersion. + tag: "" + +imagePullSecrets: [] +nameOverride: "" +fullnameOverride: "" \ No newline at end of file diff --git a/tests/sdk/go/helm-get-default-namespace/step1/main.go b/tests/sdk/go/helm-get-default-namespace/step1/main.go new file mode 100644 index 0000000000..f1f4a48b03 --- /dev/null +++ b/tests/sdk/go/helm-get-default-namespace/step1/main.go @@ -0,0 +1,35 @@ +package main + +import ( + appsv1 "github.com/pulumi/pulumi-kubernetes/sdk/v4/go/kubernetes/apps/v1" + "github.com/pulumi/pulumi-kubernetes/sdk/v4/go/kubernetes/helm/v3" + "github.com/pulumi/pulumi/sdk/v3/go/pulumi" +) + +func main() { + pulumi.Run(func(ctx *pulumi.Context) error { + chart, err := helm.NewChart(ctx, "test-get-default-object", helm.ChartArgs{ + Path: pulumi.String("local-chart"), + Version: pulumi.String("0.1.0"), + }) + if err != nil { + return err + } + + // // Get the deployment spec from the chart, with implicit default namespace. + _ = chart.GetResource("apps/v1/Deployment", "test-get-default-object-local-test-chart", ""). + ApplyT(func(r any) (any, error) { + dep := r.(*appsv1.Deployment) + return dep.Spec, nil + }) + + // Get the deployment spec from the chart, with explicit default namespace. + _ = chart.GetResource("apps/v1/Deployment", "test-get-default-object-local-test-chart", "default"). + ApplyT(func(r any) (any, error) { + dep := r.(*appsv1.Deployment) + return dep.Spec, nil + }) + + return nil + }) +} diff --git a/tests/sdk/go/helm-get-default-namespace/step2/Pulumi.yaml b/tests/sdk/go/helm-get-default-namespace/step2/Pulumi.yaml new file mode 100644 index 0000000000..6d405395f4 --- /dev/null +++ b/tests/sdk/go/helm-get-default-namespace/step2/Pulumi.yaml @@ -0,0 +1,3 @@ +name: go_helm_local_kubernetes +description: Test Kubernetes Helm package with local Chart. +runtime: go diff --git a/tests/sdk/go/helm-get-default-namespace/step2/local-chart/Chart.yaml b/tests/sdk/go/helm-get-default-namespace/step2/local-chart/Chart.yaml new file mode 100644 index 0000000000..df149f51a8 --- /dev/null +++ b/tests/sdk/go/helm-get-default-namespace/step2/local-chart/Chart.yaml @@ -0,0 +1,24 @@ +apiVersion: v2 +name: local-test-chart +description: A Helm chart for Kubernetes + +# A chart can be either an 'application' or a 'library' chart. +# +# Application charts are a collection of templates that can be packaged into versioned archives +# to be deployed. +# +# Library charts provide useful utilities or functions for the chart developer. They're included as +# a dependency of application charts to inject those utilities and functions into the rendering +# pipeline. Library charts do not define any templates and therefore cannot be deployed. +type: application + +# This is the chart version. This version number should be incremented each time you make changes +# to the chart and its templates, including the app version. +# Versions are expected to follow Semantic Versioning (https://semver.org/) +version: 0.1.1 + +# This is the version number of the application being deployed. This version number should be +# incremented each time you make changes to the application. Versions are not expected to +# follow Semantic Versioning. They should reflect the version the application is using. +# It is recommended to use it with quotes. +appVersion: "1.16.0" \ No newline at end of file diff --git a/tests/sdk/go/helm-get-default-namespace/step2/local-chart/templates/deployment.yaml b/tests/sdk/go/helm-get-default-namespace/step2/local-chart/templates/deployment.yaml new file mode 100644 index 0000000000..4a9c49b08e --- /dev/null +++ b/tests/sdk/go/helm-get-default-namespace/step2/local-chart/templates/deployment.yaml @@ -0,0 +1,37 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: {{ include "local-test-chart.fullname" . }}-new + namespace: {{ .Release.Namespace }} # This is what causes the difference when needing to specify a namespace or not during templating. + labels: + {{- include "local-test-chart.labels" . | nindent 4 }} +spec: + replicas: {{ .Values.replicaCount }} + selector: + matchLabels: + {{- include "local-test-chart.selectorLabels" . | nindent 6 }} + template: + metadata: + labels: + {{- include "local-test-chart.selectorLabels" . | nindent 8 }} + spec: + {{- with .Values.imagePullSecrets }} + imagePullSecrets: + {{- toYaml . | nindent 8 }} + {{- end }} + containers: + - name: {{ .Chart.Name }} + image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}" + imagePullPolicy: {{ .Values.image.pullPolicy }} + ports: + - name: http + containerPort: 80 + protocol: TCP + livenessProbe: + httpGet: + path: / + port: http + readinessProbe: + httpGet: + path: / + port: http \ No newline at end of file diff --git a/tests/sdk/go/helm-get-default-namespace/step2/main.go b/tests/sdk/go/helm-get-default-namespace/step2/main.go new file mode 100644 index 0000000000..79a87bb94f --- /dev/null +++ b/tests/sdk/go/helm-get-default-namespace/step2/main.go @@ -0,0 +1,35 @@ +package main + +import ( + appsv1 "github.com/pulumi/pulumi-kubernetes/sdk/v4/go/kubernetes/apps/v1" + "github.com/pulumi/pulumi-kubernetes/sdk/v4/go/kubernetes/helm/v3" + "github.com/pulumi/pulumi/sdk/v3/go/pulumi" +) + +func main() { + pulumi.Run(func(ctx *pulumi.Context) error { + chart, err := helm.NewChart(ctx, "test-get-default-object", helm.ChartArgs{ + Path: pulumi.String("local-chart"), + Version: pulumi.String("0.1.1"), // Trigger a Helm upgrade. This version contains the explicit default namespace for the resource manifest. + }) + if err != nil { + return err + } + + // Get the deployment spec from the chart, with implicit default namespace. + _ = chart.GetResource("apps/v1/Deployment", "test-get-default-object-local-test-chart-new", ""). + ApplyT(func(r any) (any, error) { + dep := r.(*appsv1.Deployment) + return dep.Spec, nil + }) + + // Get the deployment spec from the chart, with explicit default namespace. + _ = chart.GetResource("apps/v1/Deployment", "test-get-default-object-local-test-chart-new", "default"). + ApplyT(func(r any) (any, error) { + dep := r.(*appsv1.Deployment) + return dep.Spec, nil + }) + + return nil + }) +}