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

Create S3 bucket for storing CloudFormation templates #2

Merged
merged 5 commits into from
Apr 15, 2021

Conversation

BrunoGrandePhD
Copy link
Contributor

For now, the only use case is for storing the templates from the aws-genomics-workflows-library repository.

@zaro0508: Is there a reason why you deploy templates to an S3 bucket instead of downloading from GitHub directly in the sceptre pre-hooks? If you think the S3 deployment is essential, then your review of this PR would be appreciated.

@BrunoGrandePhD BrunoGrandePhD requested a review from tthyer April 13, 2021 23:11
@zaro0508
Copy link
Contributor

zaro0508 commented Apr 14, 2021

@zaro0508: Is there a reason why you deploy templates to an S3 bucket instead of downloading from GitHub directly in the sceptre pre-hooks? If you think the S3 deployment is essential, then your review of this PR would be appreciated.

Nope, you don't need to download from S3. You can download directly from raw github file references when using sceptre. Cases where you may want to put templates in S3 is when you use a cloudformation type that only supports S3 references. For example lambda packages, includes of template snippets, and many other things that i can't recall ATM.

The question here is whether you want your own cloudformation repo or do you just want to use the already existing Sage CFN repo? https://bootstrap-awss3cloudformationbucket-19qromfd235z9.s3.amazonaws.com/index.html

Copy link
Contributor

@tthyer tthyer left a comment

Choose a reason for hiding this comment

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

@BrunoGrandePhD I think I prefer just using a raw github url

@BrunoGrandePhD BrunoGrandePhD force-pushed the bgrande/add-templates-bucket branch from a57b5f7 to 420a40d Compare April 15, 2021 13:33
@BrunoGrandePhD
Copy link
Contributor Author

BrunoGrandePhD commented Apr 15, 2021

I've updated this PR to use the raw GitHub URL pinned to a version of the repository. This PR will also serve to test the aws-deploy CI workflow that was just merged. At the moment, the CI is failing because sceptre returns exit code 1 as long as the prod stack group is empty.

Edit: I realize that templates using the S3 URL are essentially using the tip of master instead of a pinned version. I suppose my rationale is that I would like to control when I increment the version of aws-infra so that a push to prod doesn't involve multiple changes (e.g. one from this repo and another from a remote template in aws-infra). Is this being overly cautious?

/cc @tthyer @zaro0508

@BrunoGrandePhD
Copy link
Contributor Author

The question here is whether you want your own cloudformation repo or do you just want to use the already existing Sage CFN repo? https://bootstrap-awss3cloudformationbucket-19qromfd235z9.s3.amazonaws.com/index.html

@zaro0508: I think our own bucket would be ideal because I want to leverage the existing Travis CI deployment workflow in the aws-genomics-workflows-library repository and I would like to keep that separate from the existing CFN S3 bucket.

config/config.yaml Outdated Show resolved Hide resolved
config/prod/s3-cfn-templates.yaml Outdated Show resolved Hide resolved
@BrunoGrandePhD
Copy link
Contributor Author

@zaro0508: I switched to wget. Thanks for the tip!

@BrunoGrandePhD
Copy link
Contributor Author

@tthyer: I switched to the GitHub URL. I'll still wait for your approval before merging.

@BrunoGrandePhD BrunoGrandePhD merged commit 1beb1dc into prod Apr 15, 2021
@BrunoGrandePhD BrunoGrandePhD deleted the bgrande/add-templates-bucket branch April 15, 2021 20:20
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