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.

Sign up for GitHub

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

Jump to bottom

fix(core): DefaultStackSynthesizer supports object prefix for s3 assets #11327

Merged
merged 8 commits into from
Nov 19, 2020
Merged

fix(core): DefaultStackSynthesizer supports object prefix for s3 assets #11327

merged 8 commits into from
Nov 19, 2020

Conversation

naveenkoppula
Copy link
Contributor

@naveenkoppula naveenkoppula commented Nov 6, 2020 •
edited by rix0rrr
Loading

Allow configuring a prefix for the storage location in the file asset bucket.

Specifically intended to support prefixing with the account ID, so that a single asset bucket can be shared between multiple accounts (given appropriate bucket permissions, of course).

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

@gitpod-io
Copy link

gitpod-io bot commented Nov 6, 2020 •
edited
Loading

@naveenkoppula naveenkoppula changed the title Adding prefix in DefaultStackSynthesizer for s3 assets fix(synth): Adding prefix in DefaultStackSynthesizer for s3 assets Nov 6, 2020
@mergify
Copy link
Contributor

mergify bot commented Nov 6, 2020

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

rix0rrr
rix0rrr previously requested changes Nov 9, 2020
View reviewed changes
Copy link
Contributor

@rix0rrr rix0rrr left a comment

Choose a reason for hiding this comment

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

Thanks for the submission! Needs a test

packages/@aws-cdk/core/lib/stack-synthesizers/default-synthesizer.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/core/lib/stack-synthesizers/default-synthesizer.ts Outdated Show resolved Hide resolved
@mergify mergify bot dismissed rix0rrr’s stale review November 11, 2020 11:03

Pull request has been modified.

rix0rrr
rix0rrr previously requested changes Nov 12, 2020
View reviewed changes
Copy link
Contributor

@rix0rrr rix0rrr left a comment

Choose a reason for hiding this comment

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

Looks good! Still needs a test though...

@mergify mergify bot dismissed rix0rrr’s stale review November 17, 2020 05:05

Pull request has been modified.

@Brian969
Copy link

Hey! What's left to getting this through the production CDK release process?
We are trying to plan when we can incorporate the feature in this CDK Open Source project:
https://github.com/aws-samples/aws-secure-environment-accelerator
as we have multiple production customers waiting on an enhancement dependent on this CDK change.
Thanks!

@rix0rrr rix0rrr changed the title fix(synth): Adding prefix in DefaultStackSynthesizer for s3 assets fix(core): DefaultStackSynthesizer supports object prefix for s3 assets Nov 19, 2020
@mergify
Copy link
Contributor

mergify bot commented Nov 19, 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-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: da86bb4
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Nov 19, 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).

@mergify mergify bot merged commit 1b5f218 into aws:master Nov 19, 2020
@Brian969 Brian969 mentioned this pull request Nov 20, 2020
Closed
@naveenkoppula naveenkoppula mentioned this pull request Dec 3, 2020
Merged
mergify bot pushed a commit that referenced this pull request Jan 6, 2021
@naveenkoppula
fix(core): DefaultStackSynthesizer bucket prefix missing for template…
50a3d3a 
… assets (#11855)

Related to #10710, #11327 @rix0rrr 

Adding bucketPrefix to "stackTemplateAssetObjectUrl"

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rix0rrr rix0rrr rix0rrr approved these changes

Assignees

@rix0rrr rix0rrr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@naveenkoppula @Brian969 @aws-cdk-automation @rix0rrr