Skip to content

Commit 3bc7306

Browse files
fix(helm/v2-alpha): Fix chart generation and template rendering issues
This commit resolves multiple issues discovered during Helm chart testing that prevented charts from rendering correctly in certain configurations: - Fix template context in certificate DNS names: metrics certificates now use root context ($) instead of current context (.), preventing rendering failures in nested template scopes - Fix service categorization logic: changed from substring matching to suffix matching to prevent false positives when chart names contain "webhook" or "metrics" keywords - Fix ServiceMonitor YAML indentation: corrected tls.key field alignment to prevent YAML parsing errors - Enable tracking of dist/chart files in testdata projects for better chart verification in version control - Add integration tests to validate chart consistency across different naming configurations (Chart.yaml name vs project name, custom kustomize namePrefix) These fixes ensure Helm charts render correctly regardless of project naming conventions and improve overall chart generation reliability. Generate-by: Cursor
1 parent b953abb commit 3bc7306

File tree

11 files changed

+77
-16
lines changed

11 files changed

+77
-16
lines changed

.github/workflows/test-helm-samples.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ jobs:
158158
go mod init test-helm-no-webhooks
159159
kubebuilder init
160160
kubebuilder create api --group example.com --version v1 --kind App --controller=true --resource=true
161-
kubebuilder edit --plugins=helm.kubebuilder.io/v1-alpha
161+
kubebuilder edit --plugins=helm.kubebuilder.io/v2-alpha
162162
163163
- name: Build and load Docker image
164164
working-directory: test-helm-no-webhooks

.gitignore

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ docs/book/src/docs
1616
# skip bin dirs
1717
**/bin
1818
**/testbin
19-
**/dist
19+
20+
# skip GoReleaser dist directory (root level only, not testdata)
21+
/dist
2022

2123
# skip .out files (coverage tests)
2224
*.out

docs/book/src/cronjob-tutorial/testdata/project/dist/chart/templates/cert-manager/metrics-certs.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ metadata:
1111
namespace: {{ .Release.Namespace }}
1212
spec:
1313
dnsNames:
14-
- {{ include "project.resourceName" (dict "suffix" "controller-manager-metrics-service" "context" .) }}.{{ .Release.Namespace }}.svc
15-
- {{ include "project.resourceName" (dict "suffix" "controller-manager-metrics-service" "context" .) }}.{{ .Release.Namespace }}.svc.cluster.local
14+
- {{ include "project.resourceName" (dict "suffix" "controller-manager-metrics-service" "context" $) }}.{{ .Release.Namespace }}.svc
15+
- {{ include "project.resourceName" (dict "suffix" "controller-manager-metrics-service" "context" $) }}.{{ .Release.Namespace }}.svc.cluster.local
1616
issuerRef:
1717
kind: Issuer
1818
name: {{ include "project.resourceName" (dict "suffix" "selfsigned-issuer" "context" $) }}

docs/book/src/getting-started/testdata/project/dist/chart/templates/monitoring/servicemonitor.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ spec:
3131
key: tls.crt
3232
keySecret:
3333
name: metrics-server-cert
34-
key: tls.key
34+
key: tls.key
3535
{{- else }}
3636
# Development/Test mode (insecure configuration)
3737
insecureSkipVerify: true

docs/book/src/multiversion-tutorial/testdata/project/dist/chart/templates/cert-manager/metrics-certs.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ metadata:
1111
namespace: {{ .Release.Namespace }}
1212
spec:
1313
dnsNames:
14-
- {{ include "project.resourceName" (dict "suffix" "controller-manager-metrics-service" "context" .) }}.{{ .Release.Namespace }}.svc
15-
- {{ include "project.resourceName" (dict "suffix" "controller-manager-metrics-service" "context" .) }}.{{ .Release.Namespace }}.svc.cluster.local
14+
- {{ include "project.resourceName" (dict "suffix" "controller-manager-metrics-service" "context" $) }}.{{ .Release.Namespace }}.svc
15+
- {{ include "project.resourceName" (dict "suffix" "controller-manager-metrics-service" "context" $) }}.{{ .Release.Namespace }}.svc.cluster.local
1616
issuerRef:
1717
kind: Issuer
1818
name: {{ include "project.resourceName" (dict "suffix" "selfsigned-issuer" "context" $) }}

pkg/plugins/optional/helm/v2alpha/scaffolds/internal/kustomize/helm_templater.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,10 +258,10 @@ func (t *HelmTemplater) substituteCertificateDNSNames(yamlContent string, resour
258258
// Metrics certificates should point to metrics service
259259
// Use chart-specific resourceName helper for consistent naming with 63-char safety
260260
metricsServiceTemplate := "{{ include \"" + t.chartName + ".resourceName\" " +
261-
"(dict \"suffix\" \"controller-manager-metrics-service\" \"context\" .) }}"
262-
metricsServiceFQDN := metricsServiceTemplate + ".{{ include \"" + t.chartName + ".namespaceName\" . }}.svc"
261+
"(dict \"suffix\" \"controller-manager-metrics-service\" \"context\" $) }}"
262+
metricsServiceFQDN := metricsServiceTemplate + ".{{ include \"" + t.chartName + ".namespaceName\" $ }}.svc"
263263
metricsServiceFQDNCluster := metricsServiceTemplate +
264-
".{{ include \"" + t.chartName + ".namespaceName\" . }}.svc.cluster.local"
264+
".{{ include \"" + t.chartName + ".namespaceName\" $ }}.svc.cluster.local"
265265

266266
// Replace placeholders
267267
yamlContent = strings.ReplaceAll(yamlContent, "SERVICE_NAME.SERVICE_NAMESPACE.svc", metricsServiceFQDN)

pkg/plugins/optional/helm/v2alpha/scaffolds/internal/kustomize/resource_organizer.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,15 +163,28 @@ func (o *ResourceOrganizer) collectPrometheusResources() []*unstructured.Unstruc
163163
}
164164

165165
// isWebhookService determines if a service is webhook-related
166+
// Verifies KIND is "Service" and name ends with "webhook-service" suffix
166167
func (o *ResourceOrganizer) isWebhookService(service *unstructured.Unstructured) bool {
168+
// Only match resources with KIND "Service" (excludes ServiceAccount, ServiceMonitor, etc.)
169+
if service.GetKind() != kindService {
170+
return false
171+
}
167172
serviceName := service.GetName()
168-
return strings.Contains(serviceName, "webhook")
173+
// Use suffix matching to avoid false positives when project names contain "webhook"
174+
// e.g., project "test-helm-no-webhooks" should not match webhook services
175+
return strings.HasSuffix(serviceName, "webhook-service")
169176
}
170177

171178
// isMetricsService determines if a service is metrics-related
179+
// Verifies KIND is "Service" and name ends with "metrics-service" suffix
172180
func (o *ResourceOrganizer) isMetricsService(service *unstructured.Unstructured) bool {
181+
// Only match resources with KIND "Service" (excludes ServiceAccount, ServiceMonitor, etc.)
182+
if service.GetKind() != kindService {
183+
return false
184+
}
173185
serviceName := service.GetName()
174-
return strings.Contains(serviceName, "metrics")
186+
// Use suffix matching to avoid false positives when project names contain "metrics"
187+
return strings.HasSuffix(serviceName, "metrics-service")
175188
}
176189

177190
// collectExtrasResources gathers uncategorized resources that don't fit standard categories

pkg/plugins/optional/helm/v2alpha/scaffolds/internal/templates/chart-templates/servicemonitor.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ spec:
9797
key: tls.crt
9898
keySecret:
9999
name: metrics-server-cert
100-
key: tls.key
100+
key: tls.key
101101
{{ "{{- else }}" }}
102102
# Development/Test mode (insecure configuration)
103103
insecureSkipVerify: true

test/e2e/helm/plugin_cluster_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,52 @@ var _ = Describe("kubebuilder", func() {
128128
By("deploying with fullnameOverride - all resources use the custom name")
129129
runHelm(kbc, true, true, false, "custom-operator")
130130
})
131+
132+
It("should work correctly when kustomize uses custom namePrefix", func() {
133+
By("generating a full-featured project with webhooks and metrics")
134+
generateHelmProject(kbc)
135+
136+
By("installing Helm")
137+
Expect(kbc.InstallHelm()).To(Succeed())
138+
139+
By("adding custom namePrefix to config/default/kustomization.yaml")
140+
kustomizationPath := filepath.Join(kbc.Dir, "config", "default", "kustomization.yaml")
141+
content, err := os.ReadFile(kustomizationPath)
142+
Expect(err).NotTo(HaveOccurred())
143+
144+
// Prepend custom namePrefix to the kustomization file
145+
modifiedContent := "namePrefix: custom-prefix-\n" + string(content)
146+
err = os.WriteFile(kustomizationPath, []byte(modifiedContent), 0o644)
147+
Expect(err).NotTo(HaveOccurred())
148+
149+
By("building installer with custom namePrefix")
150+
err = kbc.Make("build-installer")
151+
Expect(err).NotTo(HaveOccurred())
152+
153+
By("generating helm chart with custom namePrefix in kustomize")
154+
err = kbc.EditHelmPlugin()
155+
Expect(err).NotTo(HaveOccurred())
156+
157+
By("verifying helm chart uses project name helpers, not kustomize prefix")
158+
chartPath := filepath.Join(kbc.Dir, "dist", "chart")
159+
projectName := "e2e-" + kbc.TestSuffix
160+
161+
// Check _helpers.tpl uses project name
162+
helpersContent, err := os.ReadFile(filepath.Join(chartPath, "templates", "_helpers.tpl"))
163+
Expect(err).NotTo(HaveOccurred())
164+
Expect(string(helpersContent)).To(ContainSubstring(`define "` + projectName + `.name"`))
165+
Expect(string(helpersContent)).To(ContainSubstring(`define "` + projectName + `.resourceName"`))
166+
167+
// Verify templates use project name helpers, not kustomize prefix
168+
managerContent, err := os.ReadFile(filepath.Join(chartPath, "templates", "manager", "manager.yaml"))
169+
Expect(err).NotTo(HaveOccurred())
170+
Expect(string(managerContent)).To(ContainSubstring(`include "` + projectName))
171+
Expect(string(managerContent)).NotTo(ContainSubstring(`custom-prefix`),
172+
"Manager template should not contain kustomize prefix")
173+
174+
By("deploying with custom kustomize namePrefix - helm chart should still work")
175+
runHelm(kbc, true, true, false, "")
176+
})
131177
})
132178
})
133179

testdata/project-v4-with-plugins/dist/chart/templates/cert-manager/metrics-certs.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ metadata:
1111
namespace: {{ .Release.Namespace }}
1212
spec:
1313
dnsNames:
14-
- {{ include "project-v4-with-plugins.resourceName" (dict "suffix" "controller-manager-metrics-service" "context" .) }}.{{ include "project-v4-with-plugins.namespaceName" . }}.svc
15-
- {{ include "project-v4-with-plugins.resourceName" (dict "suffix" "controller-manager-metrics-service" "context" .) }}.{{ include "project-v4-with-plugins.namespaceName" . }}.svc.cluster.local
14+
- {{ include "project-v4-with-plugins.resourceName" (dict "suffix" "controller-manager-metrics-service" "context" $) }}.{{ include "project-v4-with-plugins.namespaceName" $ }}.svc
15+
- {{ include "project-v4-with-plugins.resourceName" (dict "suffix" "controller-manager-metrics-service" "context" $) }}.{{ include "project-v4-with-plugins.namespaceName" $ }}.svc.cluster.local
1616
issuerRef:
1717
kind: Issuer
1818
name: {{ include "project-v4-with-plugins.resourceName" (dict "suffix" "selfsigned-issuer" "context" $) }}

0 commit comments

Comments
 (0)