search

aws / aws-cli

Universal Command Line Interface for Amazon Web Services
Other
15.1k stars 4.01k forks source link

allow aws cloudformation package --use-json to affect artifacts uploaded to s3 as well #3726

Open tjstansell opened 5 years ago

tjstansell commented 5 years ago

Due to issue #2934, we constantly run into yaml issues with Mappings that reference accounts that start with a zero. As a workaround to that yaml bug, we were going to just use --use-json instead. Unfortunately, when aws cloudformation package is uploading artifacts from referenced AWS::CloudFormation resources, it's hardcoded to use the yaml format, which triggers this leading zero bug. If you specify, --use-json when packaging a template, that setting should propagate to the cloudformation artifacts it uploads as well.

Specifically, the following line is what I believe needs to support json as well:

https://github.com/aws/aws-cli/blob/develop/awscli/customizations/cloudformation/artifact_exporter.py#L408

justnance commented 5 years ago

@tjstansell - Thank you for reaching out. I have labeled this issue as a feature request pending further review.

@sanathkr What are your thoughts?

tjstansell commented 5 years ago

Any thoughts on if this is something that might get added? Thanks.

tjstansell commented 5 years ago

@justnance @sanathkr ping?

tjstansell commented 5 years ago

@justnance @sanathkr any way we can make some progress here?

justnance commented 5 years ago

@tjstansell - Thanks for updating this issue and your patience. An internal ticket has been generated and your account team has been notified. The CLI does not control this behavior and only echos what is returned by the service. As such, your request has been escalated to our service team and is still under review. In general, we do not provide further information such as if the feature request will be granted or an ETAs of completion. This GitHub issue is assigned to me and I will provide updates as soon as there is more information.

tjstansell commented 5 years ago

Revisiting this because it's lingered for so long. The CLI does control this behavior. PR #3727 linked back in November addresses it. Has anyone even looked at that PR? This is so frustrating.

kdaily commented 3 years ago

Hi @tjstansell, I apologize for the delay. I'm looking into this and will check on the related PR that was approved by @sanathkr.

kdaily commented 3 years ago

Hi @tjstansell,

Thanks for your patience. I understand your frustration with the delay and your need for this feature. After reviewing this with the developer team, we would like to make some changes to the implementation. If you are still interested after all this time, we would be appreciative of your efforts. If you need more assistance in determining where to alter the code, I can also help. If you would rather not, I can make the changes and push them back to your branch and get this merged in.

We’d like to see some more future-proofing of the internal interface. Instead of plumbing the use-json parameter all the way through, you would use a lightweight 'formatter' class and a lookup or dispatcher to provide the appropriate behavior. The default formatter would be YAML, and a separate JSON formatter could be supplied. This will allow the formats to evolve without relying on the specific parameter name of the command line argument.

tjstansell commented 3 years ago

Feel free to take this over. We have long since worked around this. While it would be nice to get this working I have little vested interest anymore.