Skip to content

feat(s3): add server side encryption #1847

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
Closed
wants to merge 0 commits into from
Closed

feat(s3): add server side encryption #1847

wants to merge 0 commits into from

Conversation

nebnes
Copy link
Contributor

@nebnes nebnes commented Jul 27, 2023 •
edited
Loading

Add server side encryption with s3

via env
SCCACHE_S3_SERVER_SIDE_ENCRYPTION=true

@nebnes nebnes force-pushed the feat/s3-server-side-encryption branch from a70c09d to a984373 Compare July 27, 2023 10:44
@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2023 •
edited
Loading

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (92038f4) 29.81% compared to head (a5e578f) 29.73%.
Report is 2 commits behind head on main.

❗ Current head a5e578f differs from pull request most recent head e54eb3f. Consider uploading reports for the commit e54eb3f to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1847      +/-   ##
==========================================
- Coverage   29.81%   29.73%   -0.08%     
==========================================
  Files          49       49              
  Lines       17556    17539      -17     
  Branches     8442     8447       +5     
==========================================
- Hits         5235     5216      -19     
+ Misses       7217     7214       -3     
- Partials     5104     5109       +5
Files Coverage Δ
src/cache/cache.rs 38.74% <0.00%> (+0.22%) ⬆️
src/cache/s3.rs 52.54% <0.00%> (-1.85%) ⬇️
src/config.rs 29.35% <18.18%> (-0.30%) ⬇️

... and 13 files with indirect coverage changes

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

@sylvestre sylvestre force-pushed the feat/s3-server-side-encryption branch from a984373 to f4f5150 Compare September 7, 2023 06:47
sylvestre
sylvestre reviewed Sep 26, 2023
View reviewed changes
docs/S3.md Outdated

If your endpoint requires HTTPS/TLS, set `SCCACHE_S3_USE_SSL=true`. If you don't need a secure network layer, HTTP (`SCCACHE_S3_USE_SSL=false`) might be better for performance.

Enable server side encryption with `SCCACHE_S3_SERVER_SIDE_ENCRYPTION=true`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be possible to add a link to the Doc to explain what is this encryption? Thanks

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe this docs will be helpful: https://opendal.apache.org/docs/services/s3#server-side-encryption

@nebnes nebnes force-pushed the feat/s3-server-side-encryption branch from a5e578f to e54eb3f Compare September 26, 2023 08:09
sylvestre
sylvestre reviewed Sep 26, 2023
View reviewed changes
docs/S3.md Outdated
If your endpoint requires HTTPS/TLS, set `SCCACHE_S3_USE_SSL=true`. If you don't need a secure network layer, HTTP (`SCCACHE_S3_USE_SSL=false`) might be better for performance.

Enable server side encryption with s3 managed key (SSE-S3), set `SCCACHE_S3_SERVER_SIDE_ENCRYPTION=true` .
More details about encryption [here](https://opendal.apache.org/docs/services/s3/#server-side-encryption) and documentation [here](https://docs.rs/opendal/latest/opendal/services/struct.S3.html#method.server_side_encryption_with_s3_key).
Copy link
Collaborator

Choose a reason for hiding this comment

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

please add the link on "encryption" and "documentation"
"here" is pretty bad for accessibility / screen reader. thanks

@nebnes nebnes force-pushed the feat/s3-server-side-encryption branch from e54eb3f to bf0020e Compare September 26, 2023 08:18
@nebnes nebnes closed this Oct 19, 2023
@nebnes nebnes force-pushed the feat/s3-server-side-encryption branch from bf0020e to 46f47cd Compare October 19, 2023 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sylvestre sylvestre sylvestre left review comments

@Xuanwo Xuanwo Xuanwo approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nebnes @codecov-commenter @sylvestre @Xuanwo