Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

0.13: [Bug]: deploy actions resolve template strings in manifests in a way that breaks for multi-line strings #5266

Open
stefreak opened this issue Oct 18, 2023 · 1 comment · Fixed by #5270
Open

0.13: [Bug]: deploy actions resolve template strings in manifests in a way that breaks for multi-line strings #5266

stefreak opened this issue Oct 18, 2023 · 1 comment · Fixed by #5270
Labels
0.13 0.14 bug priority:high High priority issue or feature

Comments

@stefreak
Copy link
Member

stefreak commented Oct 18, 2023 •
edited
Loading

Garden Bonsai (0.13) Bug

Current Behavior

Error cannot parse yaml with multiline string in variable templated to a manifest file

Expected behavior

Should work.

Reproducible example

manifest:

apiVersion: v1
kind: ConfigMap
metadata:
  name: test-configmap
data:
  hello: ${var.multiline-var}

deploy action:

kind: Deploy
type: kubernetes
name: multiline-var-in-manifest-deploy
spec:
  files: [ "deployment.yml" ]
variables:
  multiline-var: |
    this is a
    multiline
    variable
  foo: bar

Workaround

Works with inline manifest

Suggested solution(s)

Resolve variables after parsing

Additional context

Your environment

  • OS: macos
  • How I'm running Kubernetes: orbstack

garden version latest main
@stefreak stefreak added the 0.13 label Oct 18, 2023
@vvagaytsev vvagaytsev added the bug label Oct 18, 2023
@shumailxyz shumailxyz mentioned this issue Oct 19, 2023
Merged
@stefreak stefreak mentioned this issue Oct 20, 2023
Merged
@stefreak stefreak reopened this Oct 23, 2023
@alexkuretz
Copy link

I recently ran into this and had to b64 encode some jwt tokens, would be nice to get this fixed

stefreak added a commit that referenced this issue Dec 12, 2024
@stefreak
fix(template): establish backwards bug-compatibility for kubernetes m…
37767a2 
…anifest files

Template strings in Kubernetes manifest files are partially resolved for legacy reasons. The partial resolve behaviour has been changed to improve sanity in #6685 but for kubernetes manifest files, we need to have a version of the old behaviour.

The expected legacy behaviour looks like this:
- If template strings reference variables that do not exist there is no error
- If a template string contains multiple template expressions, each expression can be partially resolved.
- Template expressions are evaluated before parsing yaml, which means that valid yaml with template expressions can be resolved to invalid yaml (e.g. if variable values contain special characters)

This legacy behaviour can lead to quite some surprises and UX problems, for example #5266

I would suggest that we remove this functionality in 0.14.
@stefreak stefreak mentioned this issue Dec 12, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Dec 12, 2024
@stefreak
fix(template): establish backwards bug-compatibility for kubernetes m…
424b392 
…anifest files (#6713)

* fix(template): establish backwards bug-compatibility for kubernetes manifest files

Template strings in Kubernetes manifest files are partially resolved for legacy reasons. The partial resolve behaviour has been changed to improve sanity in #6685 but for kubernetes manifest files, we need to have a version of the old behaviour.

The expected legacy behaviour looks like this:
- If template strings reference variables that do not exist there is no error
- If a template string contains multiple template expressions, each expression can be partially resolved.
- Template expressions are evaluated before parsing yaml, which means that valid yaml with template expressions can be resolved to invalid yaml (e.g. if variable values contain special characters)

This legacy behaviour can lead to quite some surprises and UX problems, for example #5266

I would suggest that we remove this functionality in 0.14.

* test: fix running tests on macos

this commit fixes the "cannot find dockerfile" error that plagued us on
macos

* test: add integ tests for legacyAllowPartial behaviour in kubernetes
manifests
@stefreak stefreak added 0.14 priority:high High priority issue or feature labels Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
0.13 0.14 bug priority:high High priority issue or feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: resolve multiline variables in kubernetes manifests correctly garden-io/garden
5 participants
@mkhq @stefreak @shumailxyz @vvagaytsev @alexkuretz