Skip to content

Commit 8238663

Browse files
review
1 parent c498000 commit 8238663

File tree

4 files changed

+115
-7
lines changed

4 files changed

+115
-7
lines changed

docs/book/src/cronjob-tutorial/testdata/project/dist/chart/templates/prometheus/controller-manager-metrics-monitor.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ spec:
2929
keySecret:
3030
key: tls.key
3131
name: metrics-server-cert
32-
serverName: {{ include "project.resourceName" (dict "suffix" "controller-manager-metrics-service" "context" $) }}.{{ .Release.Namespace }}.svc
32+
serverName: {{ include "project.resourceName" (dict "suffix" "controller-manager-metrics-service" "context" $) }}.project-system.svc
3333
selector:
3434
matchLabels:
3535
app.kubernetes.io/name: {{ include "project.name" . }}

docs/book/src/multiversion-tutorial/testdata/project/dist/chart/templates/prometheus/controller-manager-metrics-monitor.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ spec:
2929
keySecret:
3030
key: tls.key
3131
name: metrics-server-cert
32-
serverName: {{ include "project.resourceName" (dict "suffix" "controller-manager-metrics-service" "context" $) }}.{{ .Release.Namespace }}.svc
32+
serverName: {{ include "project.resourceName" (dict "suffix" "controller-manager-metrics-service" "context" $) }}.project-system.svc
3333
selector:
3434
matchLabels:
3535
app.kubernetes.io/name: {{ include "project.name" . }}

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

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,16 +122,23 @@ func (t *HelmTemplater) substituteProjectNames(yamlContent string, _ *unstructur
122122
}
123123

124124
// substituteNamespace replaces manager namespace references with {{ .Release.Namespace }}
125-
// while preserving cross-namespace references (e.g., infrastructure, production)
125+
// while preserving cross-namespace references (e.g., infrastructure, production).
126+
// Uses context-aware replacement to avoid breaking resource names that contain the namespace as a substring.
126127
func (t *HelmTemplater) substituteNamespace(yamlContent string, resource *unstructured.Unstructured) string {
127128
managerNamespace := t.managerNamespace
128129
namespaceTemplate := "{{ .Release.Namespace }}"
129130

130-
// Replace only the manager namespace with template variable.
131-
// Cross-namespace values (infrastructure, production, etc.) are naturally preserved
132-
// because they don't match the manager namespace string.
133-
yamlContent = strings.ReplaceAll(yamlContent, managerNamespace, namespaceTemplate)
131+
// 1. Replace namespace in YAML field contexts (namespace: <value>)
132+
// This prevents bugs where namespace "user" breaks resource name "users" → "{{ .Release.Namespace }}s"
133+
namespaceFieldPattern := regexp.MustCompile(`(?m)^(\s*)namespace:\s+` + regexp.QuoteMeta(managerNamespace) + `\s*$`)
134+
yamlContent = namespaceFieldPattern.ReplaceAllString(yamlContent, "${1}namespace: "+namespaceTemplate)
134135

136+
// 2. Replace namespace in annotation values (e.g., cert-manager.io/inject-ca-from: <namespace>/resource)
137+
// Pattern: <namespace>/ in annotation context
138+
annotationNamespacePattern := regexp.MustCompile(regexp.QuoteMeta(managerNamespace) + `/`)
139+
yamlContent = annotationNamespacePattern.ReplaceAllString(yamlContent, namespaceTemplate+"/")
140+
141+
// 3. Replace namespace in DNS names (handled in certificate-specific function)
135142
if resource.GetKind() == kindCertificate {
136143
yamlContent = t.substituteCertificateDNSNames(yamlContent, resource)
137144
}
@@ -166,6 +173,12 @@ func (t *HelmTemplater) substituteCertificateDNSNames(yamlContent string, resour
166173
yamlContent = strings.ReplaceAll(yamlContent, hardcodedWebhookServiceShort, t.resourceNameTemplate("webhook-service"))
167174
}
168175

176+
// Replace namespace in DNS names (e.g., .project-system.svc → .{{ .Release.Namespace }}.svc)
177+
// This handles hardcoded namespace values in service DNS names
178+
namespaceInDNS := "." + t.managerNamespace + ".svc"
179+
templatedNamespaceInDNS := ".{{ .Release.Namespace }}.svc"
180+
yamlContent = strings.ReplaceAll(yamlContent, namespaceInDNS, templatedNamespaceInDNS)
181+
169182
return yamlContent
170183
}
171184

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

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -950,6 +950,101 @@ subjects:
950950
// RoleBinding metadata namespace should be preserved
951951
Expect(result).To(ContainSubstring("namespace: infrastructure"))
952952
})
953+
954+
It("should NOT replace namespace substring in resource names (regression test for issue #5354)", func() {
955+
// This test validates that namespace replacement is field-aware,
956+
// not a blind string replacement that breaks resource names
957+
roleResource := &unstructured.Unstructured{}
958+
roleResource.SetAPIVersion("rbac.authorization.k8s.io/v1")
959+
roleResource.SetKind("ClusterRole")
960+
roleResource.SetName("manager-role")
961+
962+
// Scenario: manager namespace is "user", CRD resource is "users"
963+
customTemplater := &HelmTemplater{
964+
detectedPrefix: "test-project",
965+
chartName: "test-project",
966+
managerNamespace: "user", // Short namespace that appears as substring
967+
}
968+
969+
content := `apiVersion: rbac.authorization.k8s.io/v1
970+
kind: ClusterRole
971+
metadata:
972+
name: test-project-manager-role
973+
rules:
974+
- apiGroups:
975+
- identity.example.com
976+
resources:
977+
- users
978+
- users/finalizers
979+
- users/status
980+
verbs:
981+
- get
982+
- list
983+
- watch
984+
- create
985+
- update
986+
- patch
987+
- delete`
988+
989+
result := customTemplater.ApplyHelmSubstitutions(content, roleResource)
990+
991+
// Critical: resource name "users" must NOT be replaced with "{{ .Release.Namespace }}s"
992+
Expect(result).To(ContainSubstring("- users"),
993+
"resource name 'users' should remain unchanged")
994+
Expect(result).To(ContainSubstring("- users/finalizers"),
995+
"resource name 'users/finalizers' should remain unchanged")
996+
Expect(result).To(ContainSubstring("- users/status"),
997+
"resource name 'users/status' should remain unchanged")
998+
999+
// Ensure we didn't create templated resource names
1000+
Expect(result).NotTo(ContainSubstring("- {{ .Release.Namespace }}s"),
1001+
"must NOT replace 'user' substring in resource names")
1002+
Expect(result).NotTo(MatchRegexp(`resources:\s*-\s*\{\{.*\}\}`),
1003+
"resource names must never be templated")
1004+
})
1005+
1006+
It("should handle edge case where namespace is substring of multiple fields", func() {
1007+
// Test more edge cases: namespace "app" appears in "applications", "apps", etc.
1008+
customTemplater := &HelmTemplater{
1009+
detectedPrefix: "test-project",
1010+
chartName: "test-project",
1011+
managerNamespace: "app",
1012+
}
1013+
1014+
roleBindingResource := &unstructured.Unstructured{}
1015+
roleBindingResource.SetAPIVersion("rbac.authorization.k8s.io/v1")
1016+
roleBindingResource.SetKind("RoleBinding")
1017+
roleBindingResource.SetName("manager-rolebinding")
1018+
1019+
content := `apiVersion: rbac.authorization.k8s.io/v1
1020+
kind: RoleBinding
1021+
metadata:
1022+
name: test-project-manager-rolebinding
1023+
namespace: app
1024+
roleRef:
1025+
apiGroup: rbac.authorization.k8s.io
1026+
kind: ClusterRole
1027+
name: test-project-manager-role
1028+
subjects:
1029+
- kind: ServiceAccount
1030+
name: test-project-controller-manager
1031+
namespace: app`
1032+
1033+
result := customTemplater.ApplyHelmSubstitutions(content, roleBindingResource)
1034+
1035+
// Namespace fields should be templated
1036+
Expect(result).To(ContainSubstring("namespace: {{ .Release.Namespace }}"),
1037+
"namespace fields should be templated")
1038+
1039+
// Verify we have exactly 2 namespace template substitutions (metadata + subject)
1040+
namespaceTemplateCount := strings.Count(result, "namespace: {{ .Release.Namespace }}")
1041+
Expect(namespaceTemplateCount).To(Equal(2),
1042+
"should have exactly 2 namespace field replacements")
1043+
1044+
// Verify apiGroup field is NOT affected (contains "app" in "rbac.authorization.k8s.io")
1045+
Expect(result).To(ContainSubstring("apiGroup: rbac.authorization.k8s.io"),
1046+
"apiGroup should not be affected by namespace replacement")
1047+
})
9531048
})
9541049

9551050
Context("templatePorts", func() {

0 commit comments

Comments
 (0)