kustomize-controller icon indicating copy to clipboard operation
kustomize-controller copied to clipboard

Allow variable substitution of plain string value without line feed removal

Open gberche-orange opened this issue 2 years ago • 6 comments

The variable substitution documented at https://fluxcd.io/docs/components/kustomize/kustomization/#variable-substitution systematically perform line feed removal in values, see

https://github.com/fluxcd/kustomize-controller/blob/8d61ff76d327ec76381f5f7fe3e9f0b722d75cf7/controllers/kustomization_varsub.go#L48-L55

https://pkg.go.dev/strings#Replace

func Replace(s, old, new string, n int) string

Replace returns a copy of the string s with the first n non-overlapping instances of old replaced by new. If old is empty, it matches at the beginning of the string and after each UTF-8 sequence, yielding up to k+1 replacements for a k-rune string. If n < 0, there is no limit on the number of replacements.

and related rationale during PR at https://github.com/fluxcd/kustomize-controller/pull/275/files#r576788053

As a result, this prevents using variable substitution to construct linefeed-sensitive string fields from files loaded with config generator.

See discussion for the related use-case at https://github.com/fluxcd/flux2/discussions/2211

An opt-in, in the form of a label/annotation on certain resources such as kustomize.toolkit.fluxcd.io/substitute-linefeed-removal: disabled could enable such use-case.

gberche-orange avatar Dec 13 '21 15:12 gberche-orange

Thank you for this proposal! It's very well thought-out, and already addresses many of the implementation questions we had as we were reviewing the idea at the Flux Bug Scrub today.

In general what I tell people when they ask for variable substitution to be more versatile than it is now, is that there are no guard rails and variable substitution is working on your YAML document as a plain text stream of bytes. Which means that it's very easy to break the YAML formatting, and so we strongly discourage doing anything that makes using the substitution feature more likely to ruin the formatting of the YAML.

Have you been able to use this adjusted version in your environment, and if so how do you find it working out for you? Are you able to write reasonable multiline strings into YAML substitutions now (and do they come out as you'd hopefully expect in the resulting applied resources?)

The alternative I usually recommend for users who look for this support is patching, as kustomize CLI accepts patches from either inline sources or separate files, you can write a patchesStrategicMerge that updates the field with a multiline string, and it can even be validated for correctness as YAML (independently of the resources that it patches), or marked up with colorization by your editor to show when your YAML formatting is correctly invoking the scalar block, or other options.

kingdonb avatar Feb 09 '22 21:02 kingdonb

Thanks @kingdonb for considering this proposal

Have you been able to use this adjusted version in your environment, and if so how do you find it working out for you? Are you able to write reasonable multiline strings into YAML substitutions now (and do they come out as you'd hopefully expect in the resulting applied resources?)

Do you mean that the proposal has been implemented somewhere and that I way test it ? If so, can you please clarify which flux version is bringing it as I could not yet easily spot it in the https://github.com/fluxcd/kustomize-controller/blob/main/CHANGELOG.md and I don't see a related change on the master branch code at

https://github.com/fluxcd/kustomize-controller/blob/34e2da27fa97fbc12565302892b497f1824c44c4/controllers/kustomization_varsub.go#L44-L65

The alternative I usually recommend for users who look for this support is patching, as kustomize CLI accepts patches from either inline sources or separate files, you can write a patchesStrategicMerge that updates the field with a multiline string, and it can even be validated for correctness as YAML (independently of the resources that it patches), or marked up with colorization by your editor to show when your YAML formatting is correctly invoking the scalar block, or other options.

In my specific use-case, detailed in https://github.com/fluxcd/flux2/discussions/2211 this is a multiline string which needs to be constructed within a yaml structure, see extract below.

apiVersion: osb.servicefabrik.io/v1alpha1
kind: SFPlan
metadata:
  name: &id 39d7d4c8-6fe2-4c2a-a5ca-b826937d5a88
spec:
  templates:
  - action: status
    type: gotemplate
    content: |
      ${status-bind.gotemplate} 
      ${status-unbind.gotemplate} 
      ${status-provision.gotemplate} 
      ${status-unprovision.gotemplate} 

The Json patch specificiation however does not allow for manipulating a existing string other than its full replacement, as far as I can read from https://datatracker.ietf.org/doc/html/rfc6902 specificially the add operation only applies on arrays and objects

gberche-orange avatar Feb 11 '22 07:02 gberche-orange

Do you mean that the proposal has been implemented somewhere and that I way test it ?

No, I'm afraid that's not what I meant... I saw that you had identified the source of the newlines in your research, and I wondered if you had already attempted this, because I was speculating internally about what else can go wrong if that line of code which strips newlines is simply blocked from stripping them.

My suspicion is that a one-line change will not provide satisfactory results even if you know to use the annotation, because the text that you add with your variable substitution will have newlines, but it will not be easy for Flux to know the indent level that it should be writing those new lines at without backtracking and reading carefully the string that it is substituting into.

So your multiline data which is intended to be inserted into a key-value pair that is indented to (say) 8 spaces will have to remember to indent subsequent lines by an additional 10 spaces to conform with the expected spacing in the YAML target file, it's going to have to look something like this:

data:
  the_value_is_multiline: |-
    This string is a multi-line, which will be indented at the normal level by substituting
              and this is its second line, which carefully includes enough spaces to not break the formatting of the target file.

Please allow me some time to review the new information you provided in the discussion, as it will help me to better understand exactly how you're trying to solve this use case, and if there is a better option that I can suggest to make the solution more apparent. I'm not sure I've really understood your use case after reading both threads.

In general the position of the Flux team has been that the variable substitution feature based on envsubst is reaching for the bottom of the barrel when it comes to ways to generate and manipulate manifests. There are a few "features of last resort" and "alternatives we'd suggest instead" – and environment substitution was never meant to work with multi-line strings. A structured patch will provide better guarantees that some errant spacing will not break the spell and cause Flux to seize up with a validation failure until someone corrects the error.

If you need to patch a multi-line scalar block value into another resource (whether it's for composition or to remain DRY, or any other reason) I would suggest using Kustomize and patchesStrategicMerge in the kustomization.yaml to make those substitutions, because Kustomize will not perform the patch as a simple string manipulation. It will line up matching fields between the target and the patch, and overwrite them with whatever structured data was in the patch.

The problem remains though, that a single scalar block value with multi-line strings spanning a scalar block value... cannot be "composed" in the way I think you meant with any of these tools. envsubst with Flux will balk at multi-line strings, and patchesStrategicMerge cannot perform merges inside of a scalar block. So there must be a good solution for this, ...but perhaps not in any of the places where we are currently looking. If you need to put several multiline strings together in the space of a single scalar block, then I'm afraid patchesStrategicMerge will not be the tool to help solve this either.

kingdonb avatar Feb 11 '22 21:02 kingdonb

To be clear I am trying to talk you out of this feature as I have understood your request! I have imagined a few substandard implementations that I would not like to see in Flux, this might simply be a problem that I may not have the imagination required to conceive of a good solution to this type of issue in general. Or maybe I did not understand the problem at all.

I looked at it for a while though, and I think it makes sense now. This is clear enough:

    content: |
      ${status-bind.gotemplate} 
      ${status-unbind.gotemplate} 
      ${status-provision.gotemplate} 
      ${status-unprovision.gotemplate} 

You'd like to be able to take the content from these four files, located in a configmap (placed there by configmapgenerator) and substitute them into the target YAML with a Flux Kustomization. That's a really straightforward request now, at least from an end-user perspective, if I finally have it right. (*I do not think it will be straightforward to implement though.)

I looked up this guide to different Scalar block string construction methods in YAML, and I noticed that none of them provides this "indent everything N more indent levels" behavior that would be needed to make indentation work well, like the indent template in Go which you see all over the place in every Helm chart that use go templates under the hood. (They solve a similar problem.) https://stackoverflow.com/questions/3790454/how-do-i-break-a-string-in-yaml-over-multiple-lines

So to provide the right level of indentation, there might be some support provided by the environment: I see YAML has a block chomping and a block indenting indicator, these are both at first glance not going to be helpful at all. What we would need from YAML to make this work reasonably is a negative block indenting indicator. (This question is very deep!) I don't think it's possible for YAML to implement a negative block indenting indicator, the thing I notice about this chart is that every different scalar literal still requires the string block to be indented at least as far as the correct level, so the end of the block can still be parser-located by consuming whitespace until it's not present at the right depth level anymore.)

At first reading your issue report, I almost even thought you were trying to invoke Go Templates through string substitution, which is obviously not going to work. I think I got the whole idea now though, and can understand what you're after here.

kingdonb avatar Feb 11 '22 21:02 kingdonb

thanks a lot @kingdonb for your deep study of my request as associated use-case.

You're absolutely right that in addition to configuring line feed trimming, indentation within the included variables content (e.g. ${status-bind.gotemplate}) is touchy and would require care to be authored in a friendly manner without error-prone manual indentation.

This would indeed require testing the yaml features along with an annotation kustomize.toolkit.fluxcd.io/substitute-linefeed-removal: disabled, this tool might help https://yaml-multiline.info/

gberche-orange avatar Feb 14 '22 08:02 gberche-orange

So your multiline data which is intended to be inserted into a key-value pair that is indented to (say) 8 spaces will have to remember to indent subsequent lines by an additional 10 spaces to conform with the expected spacing in the YAML target file, it's going to have to look something like this:

Helm has a great way of handling this:

{{ .multi-line-value | indent 8 }}

Would love to see something like this with Flux.

laszlocph avatar Oct 11 '23 09:10 laszlocph