chore(bootstrap): split file/image publishing roles #8403
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For security purposes, we decided that it would be lower risk to assume a different role when we publish S3 assets and when we publish ECR assets. The reason is that ECR publishers execute `docker build` which can potentially execute 3rd party code (via a base docker image). This change modifies the conventional name for the publishing roles as well as adds a set of properties to the `DefaultStackSynthesizer` to allow customization as needed. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
eladb
approved these changes
Jun 7, 2020
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
eladb
pushed a commit
that referenced
this pull request
Jun 7, 2020
The PR #8403 changed the "IAM stack" to use the default environment and forgot to update the expected output (which now does not contain a token for the URL suffix).
mergify bot
pushed a commit
that referenced
this pull request
Jun 7, 2020
The PR #8403 changed the "IAM stack" to use the default environment and forgot to update the expected output (which now does not contain a token for the URL suffix). ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
eladb
pushed a commit
that referenced
this pull request
Jun 7, 2020
The PR #8403 changed the "IAM stack" to use the default environment and forgot to update the expected output (which now does not contain a token for the URL suffix).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For security purposes, we decided that it would be lower risk to assume a different role when we publish S3 assets and when we publish ECR assets. The reason is that ECR publishers execute
docker buildwhich can potentially execute 3rd party code (via a base docker image).This change modifies the conventional name for the publishing roles as well as adds a set of properties to the
DefaultStackSynthesizerto allow customization as needed.This is a resubmission of #8319. That one was failing backwards regression tests... and for good reason! However in this case, the regression was intended (and deemed acceptable since we haven't officially "released" the feature we're breaking yet).
Unfortunately the mechanism to skip integration tests during the regression tests has been broken recently, so had to be reintroduced here.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license