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

feat: Add private bucket option #58

Merged
merged 7 commits into from
May 28, 2024
Merged

feat: Add private bucket option #58

merged 7 commits into from
May 28, 2024

Conversation

GregdTd
Copy link
Contributor

@GregdTd GregdTd commented May 27, 2024

No description provided.

…d files into this folder). Moving methods to generate name in helper files. Prettier also applied some changes here and there.
@codecov-commenter
Copy link

codecov-commenter commented May 27, 2024

Codecov Report

Attention: Patch coverage is 74.19355% with 24 lines in your changes are missing coverage. Please review.

Project coverage is 83.14%. Comparing base (6031af3) to head (e591dc5).

Files Patch % Lines
src/s3.ts 21.73% 18 Missing ⚠️
src/cloudfront/index.ts 88.46% 3 Missing ⚠️
src/cloudfront/origin-access.ts 92.10% 3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #58      +/-   ##
==========================================
- Coverage   85.86%   83.14%   -2.72%     
==========================================
  Files          12       13       +1     
  Lines         375      451      +76     
  Branches       78       85       +7     
==========================================
+ Hits          322      375      +53     
- Misses         53       76      +23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines 101 to 103
if (isOACAlreadyAssociated) {
return true;
}

Choose a reason for hiding this comment

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

It looks like you could directly return isOACAlreadyAssociated

Comment on lines 110 to 114
if (isS3WebsiteAlreadyAssociated) {
return true;
}
}
return false;

Choose a reason for hiding this comment

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

Same here and you wouldn't need to do the late return

src/s3.ts Outdated
@@ -95,6 +95,20 @@ export const tagBucket = async (bucketName: string) => {
.promise();
};

export const removeBucketWebsite = (bucketName: string) => {
logger.info(
`[S3] 🔏 Ensure bucket "${bucketName}" is not a static website hoisting`

Choose a reason for hiding this comment

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

hoisting or hosting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hosting of course

@GregdTd GregdTd force-pushed the add-private-bucket-option branch from 51d7529 to f482763 Compare May 28, 2024 10:01
@GregdTd GregdTd force-pushed the add-private-bucket-option branch from cf5a3f8 to f87f8a8 Compare May 28, 2024 15:39
…s Control that will be fetched or created for the occasion.

The distribution can be updated in both way, either by adding an OAC or on the contrary removing an OAC and add an s3-website-hoisting url.
@GregdTd GregdTd force-pushed the add-private-bucket-option branch from f87f8a8 to e591dc5 Compare May 28, 2024 15:40
@GregdTd GregdTd merged commit 6fa6533 into main May 28, 2024
1 check passed
@LaliloCI
Copy link

🎉 This PR is included in version 3.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants