Skip to content

Commit

Permalink
Fix Helm Chart resource lookup key handling for objects in default na…
Browse files Browse the repository at this point in the history
…mespace (#2655)

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:
helm/helm#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
  • Loading branch information
rquitales committed Dec 15, 2023
1 parent b46d658 commit a48fe95
Show file tree
Hide file tree
Showing 15 changed files with 345 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
16 changes: 15 additions & 1 deletion provider/pkg/gen/_go-templates/helm/v3/chart.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
16 changes: 15 additions & 1 deletion sdk/go/kubernetes/helm/v3/chart.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
17 changes: 17 additions & 0 deletions tests/sdk/go/go_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
3 changes: 3 additions & 0 deletions tests/sdk/go/helm-get-default-namespace/step1/Pulumi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
name: go_helm_local_kubernetes
description: Test Kubernetes Helm package with local Chart.
runtime: go
Original file line number Diff line number Diff line change
@@ -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/
Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
@@ -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 }}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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: ""
35 changes: 35 additions & 0 deletions tests/sdk/go/helm-get-default-namespace/step1/main.go
Original file line number Diff line number Diff line change
@@ -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
})
}
3 changes: 3 additions & 0 deletions tests/sdk/go/helm-get-default-namespace/step2/Pulumi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
name: go_helm_local_kubernetes
description: Test Kubernetes Helm package with local Chart.
runtime: go
Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
@@ -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
35 changes: 35 additions & 0 deletions tests/sdk/go/helm-get-default-namespace/step2/main.go
Original file line number Diff line number Diff line change
@@ -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
})
}

0 comments on commit a48fe95

Please sign in to comment.