Skip to content

Conversation

@jfuss
Copy link
Contributor

@jfuss jfuss commented Aug 14, 2020

Issue #, if available:
aws/aws-sam-cli#442

Description of changes:
This is the first part in solving aws/aws-sam-cli#442. Specifically, SAM uses boto to get a region and then pick the associated partition it lives within. This is mainly for two things (that I can tell):

  1. Get the partition so it can be added to a generated arn in the generated template
  2. Add pseudo values as Parameters so SAM can do some basic resolving.

For the first, we shouldn't need to explicitly put the partition into the template anywhere but instead use the CloudFormation pseudo value when writing the template. I attempted to do this initially but required changing every single output.json test file we have and the pr started to really grow. So I would like to handle that outside of this PR and get the bug addressed in SAM CLI first, and then come back and see how we can do somethings better.

For the second, we still need to find the partition and since there is no way to do this in the sdk, basing it on the region is the way to go.

This change adds an optional parameter into the transform call (effectively the 'public' api to translate the template). This optional parameter is currently only used to get the region but one could image this would be something SAM might be able to use in the future to be able to create any AWS Clients through Boto.

This will help solve the above issue because currently SAM creates a new session and has no way to know what profile or region was passed into SAM CLI. Because of this, locally customers see command failures with stacktraces from SAM for cases that should work. If approved, I will update SAM CLI to pass in a Boto Session which will end up resolving the issue linked above.

Description of how you validated changes:
Added unit tests and since this is an optional parameter, it is backwards compatible.

Checklist:

  • Write/update tests
  • make pr passes
  • Update documentation
  • Verify transformed template deploys and application functions as expected

Examples?

Please reach out in the comments, if you want to add an example. Examples will be
added to sam init through https://github.com/awslabs/aws-sam-cli-app-templates/

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@c2tarun c2tarun left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@mgrandis mgrandis left a comment

Choose a reason for hiding this comment

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

@jfuss Is this still relevant?
If so, can we move forward and merge it?
This would help with the associated issue.
We would then also have a clearer error message.

@jfuss jfuss force-pushed the optional-botosession branch from a69eb27 to 4d4ca1f Compare February 9, 2021 18:41
@jfuss
Copy link
Contributor Author

jfuss commented Feb 9, 2021

@mgrandis Yes this is still relevant, just slipped off my radar. I updated the PR but I remember there being some unit tests failures. I am going to see what travis reports and handle the issues that way.

@mgrandis mgrandis merged commit 16aefe7 into aws:develop Feb 10, 2021
mgrandis pushed a commit to mgrandis/serverless-application-model that referenced this pull request Mar 2, 2021
Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com>
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.

3 participants