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

feat(api): Enable parsing of YAML anchors and aliases #1427

Merged
merged 3 commits into from
Aug 13, 2020

Conversation

luispollo
Copy link
Contributor

Fixes #1398.

Cc: @nimakaviani

@luispollo luispollo requested a review from robfletcher August 12, 2020 21:51
resources:
- kind: ec2/security-group@v1
spec:
<< : *secgrp
Copy link
Contributor Author

@luispollo luispollo Aug 12, 2020

Choose a reason for hiding this comment

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

@nimakaviani Note that this is slightly different than your example because we do additional validation of the delivery config and verify that resource IDs (which derive from the moniker field) are unique. IMO this is a good-enough compromise, but wanted to give you a heads-up.

Copy link
Member

Choose a reason for hiding this comment

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

looks great thanks!

agreed. parsing is independent of the validation. As long as parsing works as expected, plugins can to decide at what level to use anchors and aliases and how to do the validations. so good enough IMO too. thanks again.

@luispollo luispollo force-pushed the two-pass-yaml-parsing branch from e55b4bb to 7643beb Compare August 12, 2020 22:37
@luispollo luispollo force-pushed the two-pass-yaml-parsing branch from 7643beb to 8f543ee Compare August 12, 2020 23:12
@robfletcher
Copy link
Contributor

I'm kind of stumped as to why this doesn't work out of the box given Jackson's YAML module uses SnakeYAML under the hood. Have we raised an issue on that repo?

@robfletcher
Copy link
Contributor

This seems relevant: FasterXML/jackson-dataformats-text#98

@luispollo
Copy link
Contributor Author

luispollo commented Aug 13, 2020

This seems relevant: FasterXML/jackson-dataformats-text#98

Yup. I referenced that exact same link in #1398 (comment) 🙂

@luispollo luispollo added the ready to merge Approved and ready for merge label Aug 13, 2020
@mergify mergify bot merged commit 271f1b4 into spinnaker:master Aug 13, 2020
@mergify mergify bot added the auto merged label Aug 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto merged ready to merge Approved and ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jackson doesn't properly parse anchors and aliases in the managed delivery manifest
3 participants