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.

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

fix: prevent crash due to unresolved alias in yaml #5215

Merged
merged 4 commits into from
Oct 10, 2023
Merged

fix: prevent crash due to unresolved alias in yaml #5215

merged 4 commits into from
Oct 10, 2023

Conversation

stefreak
Copy link
Member

@stefreak stefreak commented Oct 9, 2023

What this PR does / why we need it:

  • chore(deps): update yaml to 2.3.2
  • fix: prevent crash due to unresolved alias in yaml
  • improvement: use loadAndValidateYaml for both kubernetes manifests and garden configs
  • test: add tests for loadAndValidateYaml
  • improvement: eliminate explicit any in loadAndValidateYaml

Which issue(s) this PR fixes:

Fixes #5186

Special notes for your reviewer:
This is a workaround for possible upstream bug described in eemeli/yaml#497

Manual test looks like this:

% gdev deploy  
Deploy 🚀 

Garden v0.13 (Bonsai) is a major release with significant changes. Please help us improve it by reporting any issues/bugs here:
https://go.garden.io/report-bonsai
→ Run garden util hide-warning 0.13-bonsai to disable this warning.
ℹ garden               → Running in Garden environment local.default
Could not parse frontend.garden.yml in directory /Users/steffen/work/github/garden/examples/demo-project/frontend as valid YAML: YAMLException: unidentified alias "foobar" (2:14)

 1 | kind: Build
 2 | name: *foobar
------------------^
 3 | description: Frontend service container image
 4 | type: container

and for a kubernetes manifest:

% gdev deploy redis
Deploy 🚀

Garden v0.13 (Bonsai) is a major release with significant changes. Please help us improve it by reporting any issues/bugs here:
https://go.garden.io/report-bonsai
→ Run garden util hide-warning 0.13-bonsai to disable this warning.
ℹ garden               → Running in Garden environment local.default
ℹ cloud-dashboard      → View command results at: https://app.garden.io/go/command/RWwxXGFhUl

ℹ providers            → Getting status...
✔ providers            → Cached (took 0.8 sec)
ℹ providers            → Run with --force-refresh to force a refresh of provider statuses.
ℹ graph                → Resolving actions and modules...
✔ graph                → Done (took 0 sec)
ℹ deploy.redis         → missing
ℹ deploy.redis         → Deploying version v-96dc07041c...
✖ deploy.redis         → Failed (took 0 sec)
✖ deploy.redis         → Failed processing Deploy type=kubernetes name=redis (took 0.06 sec). This is what happened:

────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Could not parse redis.yml in directory /Users/steffen/work/github/garden/examples/kubernetes-deploy/redis (specified in Deploy type=kubernetes name=redis) as valid YAML: YAMLException: unidentified alias "bar" (4:10)

 1 | ---
 2 | # Source: redis/templates/secret.yaml
 3 | apiVersion: v1
 4 | foo: *bar
--------------^
 5 | kind: Secret
 6 | metadata:
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
1 deploy action(s) failed!

See .garden/error.log for detailed error message

@@ -591,3 +593,136 @@ describe("findProjectConfig", async () => {
})
})
})

describe("loadAndValidateYaml", () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great tests!

Copy link
Collaborator

@eysi09 eysi09 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me!

Thanks for adding the tests, great work!

@stefreak stefreak added this pull request to the merge queue Oct 10, 2023
Merged via the queue into main with commit 1ceb355 Oct 10, 2023
42 checks passed
@stefreak stefreak deleted the fix-5186 branch October 10, 2023 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash: Unresolved alias (the anchor must be set before the alias)
2 participants