Skip to content

Commit b4592f9

Browse files
author
Gaal
committed
chartutil: add Literal mode to ValuesReference
When a HelmRelease consumer references a ConfigMap or Secret via valuesFrom with a targetPath set, the referenced value is currently fed to Helm's strvals.ParseInto parser — the same parser as `helm --set`. That treats commas, square brackets, braces and equal signs in the value as syntactic metacharacters, so arbitrary file content (Spring application.yml with flow sequences, HOCON application.conf, JSON, multi-line strings with trailing commas, …) is misparsed or causes hard errors like `error parsing index: strconv.Atoi: parsing " \"foo\", \"bar\" "`. The existing single/double-quote workaround (introduced in helm-controller #298) is impractical for delivering raw config files from kustomize configMapGenerator / secretGenerator: the generated CM data is the file content as-is, with no opportunity to wrap it. This commit adds a new boolean field `Literal` to meta.ValuesReference. When set together with TargetPath, ChartValuesFromReferences calls a new ReplacePathLiteralValue helper that pre-escapes strvals metacharacters in the value before delegating to strvals.ParseIntoString. The result is a verbatim value preserved at the target path, while the path itself still honours `\.` escapes (which the alternative helm strvals.ParseLiteralInto does not). Has no effect when TargetPath is empty (root YAML merge is unchanged). Default Literal=false preserves backward compatibility with all existing HelmReleases. Resolves fluxcd/helm-controller#1317, addresses parts of #460, #853, flux2#1756. The CRD schema bump and consumer wiring in helm-controller is a separate follow-up PR. Signed-off-by: Gaal <g.gaal@salmon.group>
1 parent 2b9f4c5 commit b4592f9

3 files changed

Lines changed: 250 additions & 1 deletion

File tree

apis/meta/reference_types.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,17 @@ type ValuesReference struct {
218218
// transient error will still result in a reconciliation failure.
219219
// +optional
220220
Optional bool `json:"optional,omitempty"`
221+
222+
// Literal marks this ValuesReference as a literal value. When set in
223+
// combination with TargetPath, the referenced value is merged at the target
224+
// path without interpreting Helm's `--set` syntax (commas, brackets, dots,
225+
// equal signs, etc.), mirroring the behavior of `helm --set-literal`. This
226+
// is the only safe way to inject arbitrary file content (config files, JSON
227+
// blobs, multi-line strings containing special characters) through
228+
// `valuesFrom`. Has no effect when TargetPath is empty: in that mode the
229+
// referenced value is always YAML-merged at the root.
230+
// +optional
231+
Literal bool `json:"literal,omitempty"`
221232
}
222233

223234
// GetValuesKey returns the defined ValuesKey, or the default ('values.yaml').

chartutil/values.go

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,11 @@ func ChartValuesFromReferences(ctx context.Context, log logr.Logger, client kube
236236
// TODO(hidde): this is a bit of hack, as it mimics the way the option string is passed
237237
// to Helm from a CLI perspective. Given the parser is however not publicly accessible
238238
// while it contains all logic around parsing the target path, it is a fair trade-off.
239-
if err := ReplacePathValue(result, ref.TargetPath, string(valuesData)); err != nil {
239+
merger := ReplacePathValue
240+
if ref.Literal {
241+
merger = ReplacePathLiteralValue
242+
}
243+
if err := merger(result, ref.TargetPath, string(valuesData)); err != nil {
240244
return nil, NewErrValuesReference(namespacedName, ref, ErrValueMerge, err)
241245
}
242246
continue
@@ -269,3 +273,29 @@ func ReplacePathValue(values common.Values, path string, value string) error {
269273
value = path + "=" + value
270274
return strvals.ParseInto(value, values)
271275
}
276+
277+
// ReplacePathLiteralValue replaces the value at the dot notation path with the
278+
// given value, treating the value as a literal string. The value is consumed
279+
// verbatim: commas, brackets, braces, equal signs and backslashes that `--set`
280+
// would interpret as syntax are preserved as part of the value. This is the
281+
// only safe way to inject arbitrary file content (config files, JSON blobs,
282+
// multi-line strings containing special characters) at a target path.
283+
//
284+
// Mirrors the behavior of `helm --set-literal` for the value, while keeping
285+
// `\.` escape support in the path (which `helm --set-literal` itself does
286+
// not). Implemented by pre-escaping strvals metacharacters in the value and
287+
// then delegating to strvals.ParseIntoString — that combination yields a
288+
// verbatim value AND escape-aware path parsing.
289+
func ReplacePathLiteralValue(values common.Values, path string, value string) error {
290+
// Order matters: backslash must be escaped first so subsequent escapes
291+
// don't get re-escaped.
292+
escaper := strings.NewReplacer(
293+
`\`, `\\`,
294+
`,`, `\,`,
295+
`[`, `\[`,
296+
`]`, `\]`,
297+
`{`, `\{`,
298+
`}`, `\}`,
299+
)
300+
return strvals.ParseIntoString(path+"="+escaper.Replace(value), values)
301+
}

chartutil/values_test.go

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,94 @@ invalid`,
279279
},
280280
wantErr: true,
281281
},
282+
{
283+
// Documents the bug that Literal exists to work around: a value
284+
// containing helm-set metacharacters (here: a comma inside a YAML
285+
// flow sequence) is interpreted by strvals.ParseInto and corrupts
286+
// the merged value. Same input passes through cleanly in the
287+
// "literal mode" test cases below.
288+
name: "non-literal target path mangles value with commas",
289+
resources: []runtime.Object{
290+
mockConfigMap("values", map[string]string{
291+
"application.yml": `endpoints: [a,b,c]`,
292+
}),
293+
},
294+
references: []meta.ValuesReference{
295+
{
296+
Kind: kindConfigMap,
297+
Name: "values",
298+
ValuesKey: "application.yml",
299+
TargetPath: `externalConfig.application\.yml.content`,
300+
},
301+
},
302+
wantErr: true,
303+
},
304+
{
305+
name: "literal target path preserves helm-set metacharacters",
306+
resources: []runtime.Object{
307+
mockConfigMap("values", map[string]string{
308+
"application.yml": "server:\n port: 8080\nmanagement:\n endpoints:\n web:\n exposure:\n include: [ \"prometheus\", \"health\" ]\n",
309+
}),
310+
},
311+
references: []meta.ValuesReference{
312+
{
313+
Kind: kindConfigMap,
314+
Name: "values",
315+
ValuesKey: "application.yml",
316+
TargetPath: `externalConfig.application\.yml.content`,
317+
Literal: true,
318+
},
319+
},
320+
want: common.Values{
321+
"externalConfig": map[string]interface{}{
322+
"application.yml": map[string]interface{}{
323+
"content": "server:\n port: 8080\nmanagement:\n endpoints:\n web:\n exposure:\n include: [ \"prometheus\", \"health\" ]\n",
324+
},
325+
},
326+
},
327+
},
328+
{
329+
name: "literal target path preserves multi-line HOCON with equals and braces",
330+
resources: []runtime.Object{
331+
mockSecret("values", map[string][]byte{
332+
"application.conf": []byte("kafka {\n bootstrap = \"b-1:9098,b-2:9098\"\n group = \"my.service\"\n}\n"),
333+
}),
334+
},
335+
references: []meta.ValuesReference{
336+
{
337+
Kind: kindSecret,
338+
Name: "values",
339+
ValuesKey: "application.conf",
340+
TargetPath: `externalConfig.application\.conf.content`,
341+
Literal: true,
342+
},
343+
},
344+
want: common.Values{
345+
"externalConfig": map[string]interface{}{
346+
"application.conf": map[string]interface{}{
347+
"content": "kafka {\n bootstrap = \"b-1:9098,b-2:9098\"\n group = \"my.service\"\n}\n",
348+
},
349+
},
350+
},
351+
},
352+
{
353+
name: "literal flag without targetPath is ignored (root YAML merge)",
354+
resources: []runtime.Object{
355+
mockConfigMap("values", map[string]string{
356+
"values.yaml": "flat: value\n",
357+
}),
358+
},
359+
references: []meta.ValuesReference{
360+
{
361+
Kind: kindConfigMap,
362+
Name: "values",
363+
Literal: true,
364+
},
365+
},
366+
want: common.Values{
367+
"flat": "value",
368+
},
369+
},
282370
}
283371

284372
for _, tt := range tests {
@@ -406,6 +494,126 @@ func TestReplacePathValue(t *testing.T) {
406494
}
407495
}
408496

497+
// TestReplacePathLiteralValue covers the helm `--set-literal` equivalent:
498+
// the value is consumed verbatim, with metacharacters preserved.
499+
func TestReplacePathLiteralValue(t *testing.T) {
500+
tests := []struct {
501+
name string
502+
value []byte
503+
path string
504+
want map[string]interface{}
505+
wantErr bool
506+
}{
507+
{
508+
name: "simple string",
509+
value: []byte("value"),
510+
path: "outer.inner",
511+
want: map[string]interface{}{
512+
"outer": map[string]interface{}{
513+
"inner": "value",
514+
},
515+
},
516+
},
517+
{
518+
name: "value with commas is preserved",
519+
value: []byte("a,b,c"),
520+
path: "name",
521+
want: map[string]interface{}{
522+
"name": "a,b,c",
523+
},
524+
},
525+
{
526+
name: "value with braces is preserved (not parsed as inline list)",
527+
value: []byte("{a,b,c}"),
528+
path: "name",
529+
want: map[string]interface{}{
530+
"name": "{a,b,c}",
531+
},
532+
},
533+
{
534+
name: "value with equals signs is preserved",
535+
value: []byte("foo=bar=baz"),
536+
path: "name",
537+
want: map[string]interface{}{
538+
"name": "foo=bar=baz",
539+
},
540+
},
541+
{
542+
name: "value with brackets is preserved (not parsed as array index)",
543+
value: []byte("endpoints: [a, b, c]"),
544+
path: "config",
545+
want: map[string]interface{}{
546+
"config": "endpoints: [a, b, c]",
547+
},
548+
},
549+
{
550+
name: "multi-line YAML content is preserved verbatim",
551+
value: []byte("server:\n port: 8080\nmanagement:\n endpoints:\n web:\n exposure:\n include: [ \"prometheus\", \"health\" ]\n"),
552+
path: `externalConfig.application\.yml.content`,
553+
want: map[string]interface{}{
554+
"externalConfig": map[string]interface{}{
555+
"application.yml": map[string]interface{}{
556+
"content": "server:\n port: 8080\nmanagement:\n endpoints:\n web:\n exposure:\n include: [ \"prometheus\", \"health\" ]\n",
557+
},
558+
},
559+
},
560+
},
561+
{
562+
name: "HOCON content with equals and quoted strings is preserved",
563+
value: []byte("kafka {\n bootstrap = \"b-1:9098,b-2:9098\"\n group = \"svc.consumer\"\n}\n"),
564+
path: `externalConfig.application\.conf.content`,
565+
want: map[string]interface{}{
566+
"externalConfig": map[string]interface{}{
567+
"application.conf": map[string]interface{}{
568+
"content": "kafka {\n bootstrap = \"b-1:9098,b-2:9098\"\n group = \"svc.consumer\"\n}\n",
569+
},
570+
},
571+
},
572+
},
573+
{
574+
name: "JSON string is preserved verbatim (no escape needed)",
575+
value: []byte(`["a","b","c"]`),
576+
path: "subnet_ids",
577+
want: map[string]interface{}{
578+
"subnet_ids": `["a","b","c"]`,
579+
},
580+
},
581+
{
582+
name: "boolean-like string stays a string",
583+
value: []byte("true"),
584+
path: "feature.enabled",
585+
want: map[string]interface{}{
586+
"feature": map[string]interface{}{
587+
"enabled": "true",
588+
},
589+
},
590+
},
591+
{
592+
name: "dot escape in path still works",
593+
value: []byte("master"),
594+
path: `nodeSelector.kubernetes\.io/role`,
595+
want: map[string]interface{}{
596+
"nodeSelector": map[string]interface{}{
597+
"kubernetes.io/role": "master",
598+
},
599+
},
600+
},
601+
}
602+
for _, tt := range tests {
603+
t.Run(tt.name, func(t *testing.T) {
604+
g := NewWithT(t)
605+
values := map[string]interface{}{}
606+
err := ReplacePathLiteralValue(values, tt.path, string(tt.value))
607+
if tt.wantErr {
608+
g.Expect(err).To(HaveOccurred())
609+
return
610+
}
611+
g.Expect(err).ToNot(HaveOccurred())
612+
g.Expect(values).To(Equal(tt.want))
613+
})
614+
}
615+
}
616+
409617
func mockSecret(name string, data map[string][]byte) *corev1.Secret {
410618
return &corev1.Secret{
411619
TypeMeta: metav1.TypeMeta{

0 commit comments

Comments
 (0)