-
Notifications
You must be signed in to change notification settings - Fork 167
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: add --force-yaml-string-quotation forcing string quotation for rendered YAML #731
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Matthias Riegler <me@xvzf.tech>
fixes grafana#687 Signed-off-by: Matthias Riegler <me@xvzf.tech>
Signed-off-by: Matthias Riegler <me@xvzf.tech>
d02b8ea
to
a1a9ecf
Compare
@@ -9,9 +9,19 @@ import ( | |||
"github.com/Masterminds/sprig/v3" | |||
"github.com/pkg/errors" | |||
"github.com/stretchr/objx" | |||
yaml "gopkg.in/yaml.v2" | |||
yaml "gopkg.in/yaml.v3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's going to be a blocker for us 😞. v3 has major formatting differences from v2, and there's no way to configure it (see #607 (comment)). For anyone using GitOps, this upgrade is very troublesome because it means that most Kubernetes resources will be redeployed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Damn, didn't think about this, thought this had been resolved already. Do you consider switching to the fork the kustomize folks are using right now? With the v2 API, we cannot have such functionalities
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we can maintain the same indentations as we currently have, and it supports new features such as this, then I think it'd be a good option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So - the short version of this is that we cannot, I'll check if I can come up with another way, maybe a feature flag like --use-yaml-v3
?
This PR adds the
--force-yaml-string-quotation
which patches theyaml.Node
object and adds a custom formatting option to inject quotes around strings.Fixes #687