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

aws-servicecatalog: ProductStack fails with assetBucket that requires encryption #26302

Closed
wolanlu opened this issue Jul 10, 2023 · 3 comments · Fixed by #26303
Closed

aws-servicecatalog: ProductStack fails with assetBucket that requires encryption #26302

wolanlu opened this issue Jul 10, 2023 · 3 comments · Fixed by #26303
Labels
@aws-cdk/aws-servicecatalog Related to AWS Service Catalog bug This issue is a bug. effort/medium Medium work item – several days of effort p2

Comments

@wolanlu
Copy link
Contributor

wolanlu commented Jul 10, 2023

Describe the bug

Clients account setup strictly required that every bucket is encrypted. It also forces Bucket Policy to deny any unencrypted PutObject actions. When suppling ProductStack with such a Bucket as assetBucket the deployment fails on Custom Resource Lambda not being able to synch asset files into assetBucket (Access Denied).

Expected Behavior

I would expect that assets will be encrypted when the asset bucket has encryption enabled.

Current Behavior

Under the hood ProductStack uses BucketDeployment construct without encryption which results in AccessDenied. I does not considers Bucket encryption settings.

Reproduction Steps

Create a ProductStack that includes a Lambda (to use assets). Provide as assetBucket a Bucket with SSE_KMS encryption enabled and BucketPolicy configured to deny any unencrypted PutObjects.

Possible Solution

I've created a PR with a possible solution in which user would provide additional property which would turn on respectful options on BucketDeployment to enable encryption. This could also be implicitly learnt from assetBucket configuration but I feel it better to leave this decision to the user.

Additional Information/Context

No response

CDK CLI Version

2.87.0

Framework Version

No response

Node.js Version

18.16.1

OS

MacOS

Language

Typescript

Language Version

No response

Other information

No response
@wolanlu wolanlu added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 10, 2023
@github-actions github-actions bot added the @aws-cdk/aws-servicecatalog Related to AWS Service Catalog label Jul 10, 2023
@wolanlu wolanlu mentioned this issue Jul 10, 2023
Merged
@wolanlu
Copy link
Contributor Author

wolanlu commented Jul 10, 2023

A possible solution: #26303

@pahud
Copy link
Contributor

pahud commented Jul 10, 2023

Yes it makes sense to me. Thank you for your PR.

@pahud pahud added p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jul 10, 2023
@mergify mergify bot closed this as completed in #26303 Jul 11, 2023
mergify bot pushed a commit that referenced this issue Jul 11, 2023
@wolanlu
fix(servicecatalog): product stack asset bucket with forced encryption (
cb5bef5 
#26303)

Clients account setup strictly required that every bucket is encrypted. It also forces Bucket Policy to deny any unencrypted PutObject actions. When suppling ProductStack with such a Bucket as assetBucket the deployment fails on Custom Resource Lambda not being able to synch asset files into assetBucket (Access Denied).

Expected Behavior
I would expect that assets will be encrypted when the asset bucket has encryption enabled.

Current Behavior
Under the hood ProductStack uses BucketDeployment construct without encryption which results in AccessDenied. I does not considers Bucket encryption settings.

Possible Solution
This PR is a possible solution in which user would provide additional property `serverSideEncryption` along with encryption key `serverSideEncryptionAwsKmsKeyId` which would turn on respectful options on BucketDeployment to enable encryption. This could also be implicitly learnt from assetBucket configuration but I feel it better to leave this decision to the user - but I am open to change implementation if you thing this is better approach.

Closes #26302.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

bmoffatt pushed a commit to bmoffatt/aws-cdk that referenced this issue Jul 29, 2023
@wolanlu @bmoffatt
fix(servicecatalog): product stack asset bucket with forced encryption (
0e698ba 
aws#26303)

Clients account setup strictly required that every bucket is encrypted. It also forces Bucket Policy to deny any unencrypted PutObject actions. When suppling ProductStack with such a Bucket as assetBucket the deployment fails on Custom Resource Lambda not being able to synch asset files into assetBucket (Access Denied).

Expected Behavior
I would expect that assets will be encrypted when the asset bucket has encryption enabled.

Current Behavior
Under the hood ProductStack uses BucketDeployment construct without encryption which results in AccessDenied. I does not considers Bucket encryption settings.

Possible Solution
This PR is a possible solution in which user would provide additional property `serverSideEncryption` along with encryption key `serverSideEncryptionAwsKmsKeyId` which would turn on respectful options on BucketDeployment to enable encryption. This could also be implicitly learnt from assetBucket configuration but I feel it better to leave this decision to the user - but I am open to change implementation if you thing this is better approach.

Closes aws#26302.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions github-actions bot mentioned this issue Aug 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
@aws-cdk/aws-servicecatalog Related to AWS Service Catalog bug This issue is a bug. effort/medium Medium work item – several days of effort p2
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(servicecatalog): product stack asset bucket with forced encryption wolanlu/aws-cdk
2 participants
@pahud @wolanlu