Skip to content

Commit 899cfec

Browse files
authored
Merge pull request #5422 from camilamacedo86/helm-fix-issues
🐛 (helm/v2-alpha): Fix chart generation and template rendering issues
2 parents b953abb + 3bc7306 commit 899cfec

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)