Skip to content

Commit ccb3bd3

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #5388 from camilamacedo86/helm-scope
🐛 (helm/v2-alpha): Escape existing Go template syntax in generated Helm charts
2 parents 8d8c1a8 + daf921e commit ccb3bd3

File tree

3 files changed

+367
-0
lines changed

3 files changed

+367
-0
lines changed

‎pkg/plugins/optional/helm/v2alpha/scaffolds/extras_integration_test.go‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -976,5 +976,133 @@ spec:
976976
}
977977
Expect(externalSAFound).To(BeTrue(), "external ServiceAccount file should exist")
978978
})
979+
980+
It("should escape existing Go template syntax in CRD samples", func() {
981+
// Test a CRD with Go template syntax in default values.
982+
// Real-world example: gitops-promoter's ChangeTransferPolicy CRD has templates
983+
// in pullRequest.template fields that should be preserved as literal text.
984+
kustomizeYAML := `---
985+
apiVersion: v1
986+
kind: Namespace
987+
metadata:
988+
labels:
989+
app.kubernetes.io/managed-by: kustomize
990+
app.kubernetes.io/name: test-project
991+
name: test-project-system
992+
---
993+
apiVersion: apiextensions.k8s.io/v1
994+
kind: CustomResourceDefinition
995+
metadata:
996+
name: changetransferpolicies.promoter.argoproj.io
997+
namespace: test-project-system
998+
labels:
999+
app.kubernetes.io/managed-by: kustomize
1000+
app.kubernetes.io/name: test-project
1001+
spec:
1002+
group: promoter.argoproj.io
1003+
names:
1004+
kind: ChangeTransferPolicy
1005+
listKind: ChangeTransferPolicyList
1006+
plural: changetransferpolicies
1007+
singular: changetransferpolicy
1008+
scope: Namespaced
1009+
versions:
1010+
- name: v1alpha1
1011+
served: true
1012+
storage: true
1013+
schema:
1014+
openAPIV3Schema:
1015+
description: ChangeTransferPolicy is the Schema for the changetransferpolicies API
1016+
properties:
1017+
spec:
1018+
properties:
1019+
activeBranch:
1020+
type: string
1021+
pullRequest:
1022+
properties:
1023+
template:
1024+
properties:
1025+
description:
1026+
default: "Promoting {{ .ChangeTransferPolicy.Spec.ActiveBranch }}"
1027+
type: string
1028+
title:
1029+
default: "Promote {{ trunc 5 .ChangeTransferPolicy.Status.Proposed.Dry.Sha }}"
1030+
type: string
1031+
type: object
1032+
type: object
1033+
type: object
1034+
type: object
1035+
---
1036+
apiVersion: apps/v1
1037+
kind: Deployment
1038+
metadata:
1039+
name: test-project-controller-manager
1040+
namespace: test-project-system
1041+
labels:
1042+
app.kubernetes.io/managed-by: kustomize
1043+
app.kubernetes.io/name: test-project
1044+
spec:
1045+
replicas: 1
1046+
selector:
1047+
matchLabels:
1048+
control-plane: controller-manager
1049+
template:
1050+
metadata:
1051+
labels:
1052+
control-plane: controller-manager
1053+
spec:
1054+
serviceAccountName: test-project-controller-manager
1055+
containers:
1056+
- name: manager
1057+
image: controller:latest
1058+
args:
1059+
- --metrics-bind-address=:8443
1060+
- --health-probe-bind-address=:8081
1061+
`
1062+
1063+
kustomizeFile := filepath.Join(tmpDir, "install.yaml")
1064+
err := os.WriteFile(kustomizeFile, []byte(kustomizeYAML), 0o600)
1065+
Expect(err).NotTo(HaveOccurred())
1066+
1067+
parser := kustomize.NewParser(kustomizeFile)
1068+
resources, err := parser.Parse()
1069+
Expect(err).NotTo(HaveOccurred())
1070+
1071+
converter := kustomize.NewChartConverter(resources, "test-project", "test-project", "dist")
1072+
err = converter.WriteChartFiles(fs)
1073+
Expect(err).NotTo(HaveOccurred())
1074+
1075+
By("verifying CRD file has escaped Go template syntax")
1076+
crdDir := filepath.Join("dist", "chart", "templates", "crd")
1077+
crdPath := filepath.Join(crdDir, "changetransferpolicies.promoter.argoproj.io.yaml")
1078+
exists, err := afero.Exists(fs.FS, crdPath)
1079+
Expect(err).NotTo(HaveOccurred())
1080+
Expect(exists).To(BeTrue(), "CRD file should exist")
1081+
1082+
crdContent, err := afero.ReadFile(fs.FS, crdPath)
1083+
Expect(err).NotTo(HaveOccurred())
1084+
crdStr := string(crdContent)
1085+
1086+
// Existing Go template syntax should be escaped to prevent Helm from parsing it
1087+
Expect(crdStr).To(ContainSubstring(`{{ "{{ .ChangeTransferPolicy.Spec.ActiveBranch }}" }}`),
1088+
"existing template syntax should be escaped")
1089+
Expect(crdStr).To(ContainSubstring(`{{ "{{ trunc 5 .ChangeTransferPolicy.Status.Proposed.Dry.Sha }}" }}`),
1090+
"template functions should be escaped")
1091+
1092+
// Verify we don't have unescaped template syntax that would break Helm rendering
1093+
// We check that all ChangeTransferPolicy references are properly wrapped in escaped strings
1094+
// Pattern checks for: default: "...<text>{{ .ChangeTransferPolicy" (not escaped)
1095+
// The properly escaped version is: default: "...{{ "{{ .ChangeTransferPolicy..." }}"
1096+
Expect(crdStr).NotTo(MatchRegexp(`default:\s+"[^{]*\{\{\s*\.ChangeTransferPolicy`),
1097+
"unescaped Go templates should not exist in default values")
1098+
1099+
// Helm templates we add should still work (not escaped)
1100+
Expect(crdStr).To(ContainSubstring("{{- if .Values.crd.enable }}"),
1101+
"Helm conditional should be present and NOT escaped")
1102+
Expect(crdStr).To(ContainSubstring("namespace: {{ .Release.Namespace }}"),
1103+
"Helm namespace template should be present and NOT escaped")
1104+
Expect(crdStr).To(ContainSubstring(`app.kubernetes.io/name: {{ include "test-project.name" . }}`),
1105+
"Helm label template should be present and NOT escaped")
1106+
})
9791107
})
9801108
})

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

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,11 @@ func (t *HelmTemplater) resourceNameTemplate(suffix string) string {
7171

7272
// ApplyHelmSubstitutions converts YAML content to use Helm template syntax
7373
func (t *HelmTemplater) ApplyHelmSubstitutions(yamlContent string, resource *unstructured.Unstructured) string {
74+
// Escape existing Go template syntax ({{ }}) FIRST before adding Helm templates.
75+
// Resources from install.yaml may contain templates that should be preserved as literal text.
76+
// For example: CRD default values, ConfigMap data, Secret URLs, annotations, etc.
77+
yamlContent = t.escapeExistingTemplateSyntax(yamlContent)
78+
7479
// Apply conditional wrappers first
7580
yamlContent = t.addConditionalWrappers(yamlContent, resource)
7681

@@ -116,6 +121,70 @@ func (t *HelmTemplater) ApplyHelmSubstitutions(yamlContent string, resource *uns
116121
return yamlContent
117122
}
118123

124+
// escapeExistingTemplateSyntax escapes Go template syntax ({{ }}) in YAML to prevent
125+
// Helm from parsing them. Converts existing templates to literal strings that Helm outputs as-is.
126+
//
127+
// Why this is needed:
128+
// Resources from install.yaml may contain {{ }} in string fields that are NOT Helm templates.
129+
// Without escaping, Helm will try to evaluate them and fail. For example:
130+
//
131+
// CRD default: "Branch: {{ .Spec.Branch }}" -> ERROR: .Spec undefined
132+
//
133+
// How it works:
134+
// Wraps non-Helm templates in string literals so Helm outputs them unchanged:
135+
//
136+
// {{ .Field }} -> {{ "{{ .Field }}" }}
137+
//
138+
// When Helm renders this, it outputs the literal string: {{ .Field }}
139+
//
140+
// Smart detection:
141+
// Only escapes templates that DON'T start with Helm keywords:
142+
// - .Release, .Values, .Chart (Helm built-ins)
143+
// - include, if, with, range, toYaml (Helm functions)
144+
//
145+
// This means our Helm templates work normally while existing templates are preserved.
146+
func (t *HelmTemplater) escapeExistingTemplateSyntax(yamlContent string) string {
147+
// Find all {{ ... }} patterns (non-greedy for multiple on same line)
148+
templatePattern := regexp.MustCompile(`\{\{(.*?)\}\}`)
149+
150+
yamlContent = templatePattern.ReplaceAllStringFunc(yamlContent, func(match string) string {
151+
// Extract content between {{ and }}
152+
content := strings.TrimPrefix(match, "{{")
153+
content = strings.TrimSuffix(content, "}}")
154+
trimmedContent := strings.TrimSpace(content)
155+
156+
// Check if this is a Helm template (starts with Helm keyword)
157+
helmPatterns := []string{
158+
"include ", "- include ",
159+
".Release.", "- .Release.",
160+
".Values.", "- .Values.",
161+
".Chart.", "- .Chart.",
162+
"toYaml ", "- toYaml ",
163+
"if ", "- if ",
164+
"end ", "- end ",
165+
"with ", "- with ",
166+
"range ", "- range ",
167+
"else", "- else",
168+
}
169+
170+
// If it's a Helm template, keep it as-is
171+
for _, pattern := range helmPatterns {
172+
if strings.HasPrefix(trimmedContent, pattern) {
173+
return match
174+
}
175+
}
176+
177+
// Otherwise, escape it to preserve as literal text
178+
// Escape any quotes inside the template content
179+
escapedContent := strings.ReplaceAll(content, `"`, `\"`)
180+
181+
// Wrap in Helm string literal: {{ "{{...}}" }}
182+
return `{{ "{{` + escapedContent + `}}" }}`
183+
})
184+
185+
return yamlContent
186+
}
187+
119188
// substituteProjectNames keeps original YAML as much as possible - only add Helm templating
120189
func (t *HelmTemplater) substituteProjectNames(yamlContent string, _ *unstructured.Unstructured) string {
121190
return yamlContent

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

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,176 @@ spec:
714714
})
715715
})
716716

717+
Context("existing Go template syntax escaping", func() {
718+
It("should escape existing Go template syntax in CRD samples", func() {
719+
crdResource := &unstructured.Unstructured{}
720+
crdResource.SetAPIVersion("apiextensions.k8s.io/v1")
721+
crdResource.SetKind("CustomResourceDefinition")
722+
crdResource.SetName("changetransferpolicies.promoter.argoproj.io")
723+
724+
content := `apiVersion: apiextensions.k8s.io/v1
725+
kind: CustomResourceDefinition
726+
metadata:
727+
name: changetransferpolicies.promoter.argoproj.io
728+
spec:
729+
names:
730+
kind: ChangeTransferPolicy
731+
versions:
732+
- name: v1alpha1
733+
schema:
734+
openAPIV3Schema:
735+
properties:
736+
spec:
737+
properties:
738+
pullRequest:
739+
properties:
740+
template:
741+
properties:
742+
description:
743+
default: "Promoting {{ .ChangeTransferPolicy.Spec.ActiveBranch }}"
744+
type: string
745+
title:
746+
default: "Promote {{ trunc 5 .ChangeTransferPolicy.Status.Proposed.Dry.Sha }}"
747+
type: string`
748+
749+
result := templater.ApplyHelmSubstitutions(content, crdResource)
750+
751+
// Existing {{ }} should be escaped to {{ "{{ ... }}" }}
752+
Expect(result).To(ContainSubstring(`{{ "{{ .ChangeTransferPolicy.Spec.ActiveBranch }}" }}`),
753+
"existing template syntax should be escaped")
754+
Expect(result).To(ContainSubstring(`{{ "{{ trunc 5 .ChangeTransferPolicy.Status.Proposed.Dry.Sha }}" }}`),
755+
"function calls in templates should be escaped")
756+
757+
// Should NOT have unescaped Go template syntax (which would break Helm)
758+
// We check that all ChangeTransferPolicy references are properly wrapped
759+
// Pattern checks for: default: "...<text>{{ .ChangeTransferPolicy" (not escaped)
760+
// The properly escaped version is: default: "...{{ "{{ .ChangeTransferPolicy..." }}"
761+
Expect(result).NotTo(MatchRegexp(`default:\s+"[^{]*\{\{\s*\.ChangeTransferPolicy`),
762+
"unescaped Go templates should not exist in default values")
763+
})
764+
765+
It("should escape multiple template expressions on the same line", func() {
766+
crdResource := &unstructured.Unstructured{}
767+
crdResource.SetAPIVersion("apiextensions.k8s.io/v1")
768+
crdResource.SetKind("CustomResourceDefinition")
769+
crdResource.SetName("policies.example.com")
770+
771+
content := `apiVersion: apiextensions.k8s.io/v1
772+
kind: CustomResourceDefinition
773+
spec:
774+
versions:
775+
- schema:
776+
openAPIV3Schema:
777+
properties:
778+
spec:
779+
properties:
780+
message:
781+
default: "From {{ .Source.Branch }} to {{ .Target.Branch }}"`
782+
783+
result := templater.ApplyHelmSubstitutions(content, crdResource)
784+
785+
// Both templates should be escaped (applies to all resources)
786+
Expect(result).To(ContainSubstring(`{{ "{{ .Source.Branch }}" }}`))
787+
Expect(result).To(ContainSubstring(`{{ "{{ .Target.Branch }}" }}`))
788+
})
789+
790+
It("should escape templates with special characters", func() {
791+
crdResource := &unstructured.Unstructured{}
792+
crdResource.SetAPIVersion("apiextensions.k8s.io/v1")
793+
crdResource.SetKind("CustomResourceDefinition")
794+
crdResource.SetName("configs.example.com")
795+
796+
content := `apiVersion: apiextensions.k8s.io/v1
797+
kind: CustomResourceDefinition
798+
spec:
799+
versions:
800+
- schema:
801+
openAPIV3Schema:
802+
properties:
803+
spec:
804+
properties:
805+
value:
806+
default: "Value: {{ .Config.Key-With-Dashes }}"`
807+
808+
result := templater.ApplyHelmSubstitutions(content, crdResource)
809+
810+
Expect(result).To(ContainSubstring(`{{ "{{ .Config.Key-With-Dashes }}" }}`))
811+
})
812+
813+
It("should handle template syntax with quotes correctly", func() {
814+
crdResource := &unstructured.Unstructured{}
815+
crdResource.SetAPIVersion("apiextensions.k8s.io/v1")
816+
crdResource.SetKind("CustomResourceDefinition")
817+
crdResource.SetName("messages.example.com")
818+
819+
content := `apiVersion: apiextensions.k8s.io/v1
820+
kind: CustomResourceDefinition
821+
spec:
822+
versions:
823+
- schema:
824+
openAPIV3Schema:
825+
properties:
826+
spec:
827+
properties:
828+
template:
829+
default: '{{ .Config.Message "default" }}'`
830+
831+
result := templater.ApplyHelmSubstitutions(content, crdResource)
832+
833+
// Quotes inside templates should be escaped
834+
Expect(result).To(ContainSubstring(`{{ "{{ .Config.Message \"default\" }}" }}`))
835+
})
836+
837+
It("should escape templates in ConfigMaps and other non-CRD resources", func() {
838+
configMapResource := &unstructured.Unstructured{}
839+
configMapResource.SetAPIVersion("v1")
840+
configMapResource.SetKind("ConfigMap")
841+
configMapResource.SetName("template-config")
842+
configMapResource.SetNamespace("test-project-system")
843+
844+
// ANY resource can have Go template syntax that needs escaping
845+
// Examples: ConfigMaps with notification templates, Secrets with webhook URLs,
846+
// Deployment annotations with CI/CD metadata, etc.
847+
content := `apiVersion: v1
848+
kind: ConfigMap
849+
metadata:
850+
name: template-config
851+
namespace: test-project-system
852+
labels:
853+
app.kubernetes.io/name: test-project
854+
data:
855+
notification: "Deployed from {{ .Source.Branch }} to {{ .Target.Branch }}"`
856+
857+
result := templater.ApplyHelmSubstitutions(content, configMapResource)
858+
859+
// Existing templates should be escaped (applies to ALL resources, not just CRDs)
860+
Expect(result).To(ContainSubstring(`{{ "{{ .Source.Branch }}" }}`))
861+
Expect(result).To(ContainSubstring(`{{ "{{ .Target.Branch }}" }}`))
862+
863+
// Helm templates should still be added normally
864+
Expect(result).To(ContainSubstring("namespace: {{ .Release.Namespace }}"))
865+
Expect(result).To(ContainSubstring(`app.kubernetes.io/name: {{ include "test-project.name" . }}`))
866+
})
867+
868+
It("should handle content without any templates", func() {
869+
configMapResource := &unstructured.Unstructured{}
870+
configMapResource.SetAPIVersion("v1")
871+
configMapResource.SetKind("ConfigMap")
872+
configMapResource.SetName("no-template")
873+
874+
content := `apiVersion: v1
875+
kind: ConfigMap
876+
data:
877+
message: "No templates here"`
878+
879+
result := templater.ApplyHelmSubstitutions(content, configMapResource)
880+
881+
// Should not add any escaping
882+
Expect(result).To(ContainSubstring(`message: "No templates here"`))
883+
Expect(result).NotTo(ContainSubstring(`{{ "{{`))
884+
})
885+
})
886+
717887
Context("edge cases", func() {
718888
It("should handle empty content", func() {
719889
testResource := &unstructured.Unstructured{}

0 commit comments

Comments
 (0)