search

aws-cloudformation / rain

A development workflow tool for working with AWS CloudFormation.
Apache License 2.0
771 stars 69 forks source link

Fixed template packaging to resolve anchors #265

Closed ericzbeard closed 7 months ago

ericzbeard commented 7 months ago

Fixes #257

Instead of using just the fmt command, first pkg and then pipe into fmt.

Going to leave this a draft for now. All tests are passing but there are some templates with anchors that will cause fmt to fail, which means you would have to package first. That may be unexpected for some users, although the anchors would fail when you send them to CloudFormation, so maybe it doesn't matter. The issue is due to default sorting, which puts the anchors out of order and the go yaml parser can't resolve them. Maybe that's ok? If you want to format a template with anchors you can add --unsorted and it will work.

@hariprakash-j, @khmoryz, @null93, @iainelder @j5nb4l

null93 commented 7 months ago

IMO I think that fmt should try not to transform the template as much as possible. If I am using yaml anchors, I would expect those anchors to still be there after formatting it (with sorted properties).

If I am trying to translate the template to JSON and I am using yaml anchors, I would expect it to fail since JSON does not have such features. That being said, maybe it only resolves the anchors when using fmt and translating to JSON?

Since most people use fmt to "prettify" and standardize their source templates, I think it might have an undesired effect if the fmt command resolves all the anchors and copies the values across the template.

For this reason, I would have to agree with you. I think if you are trying to resolve those anchors, you should just process it through pkg.

Looking through the code, everything looks good. Let me know if you want me to compile this PR and test it out myself.

j5nb4l commented 7 months ago

I tend to agree with @null93. To be perfectly frank, the sorting behavior was unexpected to me in the first place. Rain was probably the first formatter I've used that rearranged the structure of the file. In my opinion, it would be better for that feature be turned off by default if it will improve the support for anchors.