New S3 backend #1086

alessandrod · 2022-01-04T00:35:08Z

The backend uses the official AWS SDK for Rust.

Supersedes #869 (including readonly mode with SCCACHE_S3_NO_CREDENTIALS), and fixes #632 #281 and #217

The new backend is largely compatible with the old one in terms of conf settings, except that it does not support SCCACHE_S3_USE_SSL=0. SSL is always required at the moment.

glandium · 2022-01-04T00:43:35Z

This doesn't address #869 (comment) AFAICT.

alessandrod · 2022-01-04T01:01:06Z

This doesn't address #869 (comment) AFAICT.

Oh oops my bad, I completely missed that. I'll look into it!

codecov-commenter · 2022-01-14T10:43:55Z

Codecov Report

Merging #1086 (788e4db) into main (586c308) will increase coverage by 3.75%.
The diff coverage is 3.84%.

@@            Coverage Diff             @@
##             main    #1086      +/-   ##
==========================================
+ Coverage   55.72%   59.47%   +3.75%     
==========================================
  Files          47       45       -2     
  Lines       11809    11774      -35     
  Branches     2099     2106       +7     
==========================================
+ Hits         6580     7003     +423     
+ Misses       3915     3404     -511     
- Partials     1314     1367      +53

Impacted Files	Coverage Δ
src/cache/cache.rs	`35.00% <0.00%> (-0.39%)`	⬇️
src/cache/s3.rs	`0.00% <0.00%> (ø)`
src/lib.rs	`44.44% <ø> (ø)`
src/config.rs	`57.99% <27.27%> (+1.91%)`	⬆️
src/dist/pkg.rs	`29.54% <0.00%> (-26.02%)`	⬇️
tests/sccache_cargo.rs	`84.05% <0.00%> (-15.95%)`	⬇️
src/compiler/c.rs	`64.13% <0.00%> (-10.05%)`	⬇️
src/dist/cache.rs	`53.17% <0.00%> (-0.43%)`	⬇️
src/compiler/msvc.rs	`65.46% <0.00%> (+0.15%)`	⬆️
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 586c308...788e4db. Read the comment docs.

sylvestre · 2022-01-28T07:39:57Z

@alessandrod do you have an update? thanks

mitchhentges

Hey, would you mind rebasing this off main?
Sorry for the delay in reviewing this so far, but I'll dig into it if you're able to update it :)

jschwe · 2022-02-15T08:29:53Z

From the AWS sdk github:

The SDK currently requires a minimum of Rust 1.54, and is not guaranteed to build on compiler versions earlier than that. While we are still in alpha, we will be keeping the minimum compiler version two releases behind the latest stable release where possible (so if the latest stable were 1.55, we would be on 1.53). However, we are not making any guarantees around this at present. Increases in minimum required Rust version will be called out in the Release Notes for new releases of the SDK.

Since you wanted to not bump the MSRV for the upcoming Release, I guess this PR will have to wait until after 0.2.16 anyway?

mitchhentges · 2022-02-15T15:50:07Z

Since you wanted to not bump the MSRV for the upcoming Release, I guess this PR will have to wait until after 0.2.16 anyway?

Hmm, I suppose you're right - I assumed that there's be an older version of the AWS SDK that would be compatible with Rust 1.48, but looks like that's not the case: the oldest published version I could find (0.2.0) still required at least Rust 1.54.

alessandrod · 2022-03-01T09:25:06Z

Rebased, sorry for the delay 😊

mitchhentges · 2022-03-02T18:44:51Z

Looks like we have some CI failures:

error: failed to parse manifest at `/home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/time-0.3.7/Cargo.toml`

Caused by:
  feature `resolver` is required

  this Cargo does not support nightly features, but if you
  switch to nightly channel you can add
  `cargo-features = ["resolver"]` to enable this feature

I'm guessing that this will be blocked on #1121 anyways, which is itself blocked on Milestone 3.

alessandrod · 2022-03-03T02:42:14Z

Yep, it looks like resolver was added in cargo 1.51

jrcichra · 2022-04-17T22:39:22Z

Just wanted to say thanks @alessandrod for this PR. The main branch wasn't working with my Minio cluster (getting 404 Not Found ) errors. Your PR solved those issues and works great for me 💯

GZGavinZhao · 2022-05-16T22:23:23Z

I have resolved conflicts and made a PR to the author's branch. The struggle for a new S3 backend will not be abandoned :P

The backend uses the official AWS SDK for Rust. It supports SCCACHE_BUCKET, SCCACHE_REGION and SCCACHE_ENDPOINT like the old backend. It does not support SCCACHE_S3_USE_SSL. The backend adds support for a new SCCACHE_S3_NO_CREDENTIALS key that can be set to implement a readonly cache that does not require AWS credentials at all. This is useful to speed up public pull request builds that typically can't access AWS credentials for security reasons.

alessandrod · 2022-05-26T08:54:38Z

I've rebased this, sorry for the delay! Someone kick tests pls.

GZGavinZhao · 2022-05-27T00:46:51Z

Is there a particular reason why the aws dependencies are on version 0.3? I tested with 0.12 and it works fine on my machine.

woss · 2022-07-22T10:29:25Z

I started using this PR instead of the one in the main. Needed to compile it so here is the link and the yaml anchor i am using for gitlab. I hope all of you will find it useful:

.with-sccache-new-s3:
  before_script:
    # stop the sccache server just in case so it can pick up the env vars, it will start again
    - sccache --stop-server
    - wget -q https://ipfs.anagolay.network/ipfs/bafybeidovvudc7jcc3ulkz2t4pew4fa5ylnzoaf67b67kc3yderuavx3hy -O /usr/local/bin/sccache
    - chmod +x /usr/local/bin/sccache
    - sccache --version
    - sccache --show-stats

drahnr · 2022-07-22T10:47:15Z

src/cache/s3.rs

+            RegionProviderChain::first_try(region.map(|r| Region::new(r.to_owned())))
+                .or_default_provider();
+
+        let shared_config = aws_config::from_env().region(region_provider).load().await;


We should not pull in env vars here, but rather use the arguments and the arguments alone. Everything else should be part of the config file. There are two reasons, mainly maintainability, reproducibility of tests.

i'd like to chip in. why not pull envs in? the whole AWS compatible cli works the same way. and passing the AWS_ACCESS_SECRET_KEY as an argument sounds terribly risky

I am not saying to remove the functionality, but with clap there is an easier way to provide the information in one place. If env is pulled in from allover the place, that's not maintainable.

The clap approach of centrally pulling in AWS credentials from related arguments would lead to less maintainability in this case.

The aws_config crate pulls credentials from different sources with their corresponding precedences.

If the access key id and secret access key method-usage is hard-coded, authentication options such as a CLI credentials file, IAM roles for ECS tasks and IAM roles for service accounts become unusable right from the get-go.

Also, future CLI authentication method integrations, e.g., IAM roles anywhere, would have to be added manually instead of just updating the aws_config crate.

drahnr

LGTM, there is one nit that must be resolved, otherwise looks good to me. Going to test on a rather old minio cluster before merging. Could you also bump the dependencies to their latest versions?

Does this require openssl under the hood or is there an option to use rustls instead, if so that should be the default.

Thank you for your work and endurance on this!

ajschmidt8 · 2022-11-09T19:19:00Z

bump! This is so close to being merged.

@alessandrod, can you bump the dependencies as per the request in the previous comment above?

messense · 2022-11-14T05:48:25Z

Opened a rebased version of this PR in #1403, hopefully it will speed things up.

sylvestre · 2022-11-14T17:05:56Z

merged here:
#1403

sylvestre · 2022-11-14T17:06:30Z

thanks for the work!

mitchhentges suggested changes Feb 14, 2022

View reviewed changes

mitchhentges added the awaiting response label Feb 14, 2022

mitchhentges added blocked and removed awaiting response labels Feb 17, 2022

mitchhentges mentioned this pull request Feb 17, 2022

Rusoto for S3 communication (with support for public buckets). #869

Closed

alessandrod force-pushed the new-s3 branch from aee4e08 to 788e4db Compare March 1, 2022 09:24

alessandrod requested a review from mitchhentges March 1, 2022 09:31

SomeoneToIgnore mentioned this pull request Mar 7, 2022

Docker image build time on main is huge neondatabase/neon#1113

Closed

mitchhentges removed the blocked label May 5, 2022

mitchhentges removed their request for review May 5, 2022 13:33

GZGavinZhao mentioned this pull request May 25, 2022

Aliyun OSS Incompatibility #1167

Open

alessandrod force-pushed the new-s3 branch from 788e4db to 2612ad7 Compare May 26, 2022 08:52

alessandrod mentioned this pull request Jul 13, 2022

chore: use sccache solana-labs/solana#26376

Merged

2 tasks

drahnr reviewed Jul 22, 2022

View reviewed changes

drahnr approved these changes Jul 22, 2022

View reviewed changes

drahnr mentioned this pull request Oct 6, 2022

S3/Minio does not work: invalid Zip archive #1345

Closed

driazati mentioned this pull request Nov 2, 2022

Add SCCACHE_S3_RW_MODE to enable read-only S3 cache #1380

Closed

Xuanwo mentioned this pull request Nov 9, 2022

ci: Try to use sccache databendlabs/databend#8709

Merged

messense mentioned this pull request Nov 14, 2022

New S3 backend - rebased #1403

Merged

sylvestre closed this Nov 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New S3 backend #1086

New S3 backend #1086

alessandrod commented Jan 4, 2022

glandium commented Jan 4, 2022

alessandrod commented Jan 4, 2022

codecov-commenter commented Jan 14, 2022 •

edited

Loading

sylvestre commented Jan 28, 2022

mitchhentges left a comment

jschwe commented Feb 15, 2022

mitchhentges commented Feb 15, 2022

alessandrod commented Mar 1, 2022

mitchhentges commented Mar 2, 2022

alessandrod commented Mar 3, 2022

jrcichra commented Apr 17, 2022

GZGavinZhao commented May 16, 2022

alessandrod commented May 26, 2022

GZGavinZhao commented May 27, 2022

woss commented Jul 22, 2022 •

edited

Loading

drahnr Jul 22, 2022

woss Jul 22, 2022

drahnr Aug 4, 2022

ThomasK33 Aug 4, 2022

drahnr left a comment •

edited

Loading

ajschmidt8 commented Nov 9, 2022

messense commented Nov 14, 2022

sylvestre commented Nov 14, 2022

sylvestre commented Nov 14, 2022

New S3 backend #1086

New S3 backend #1086

Conversation

alessandrod commented Jan 4, 2022

glandium commented Jan 4, 2022

alessandrod commented Jan 4, 2022

codecov-commenter commented Jan 14, 2022 • edited Loading

Codecov Report

sylvestre commented Jan 28, 2022

mitchhentges left a comment

Choose a reason for hiding this comment

jschwe commented Feb 15, 2022

mitchhentges commented Feb 15, 2022

alessandrod commented Mar 1, 2022

mitchhentges commented Mar 2, 2022

alessandrod commented Mar 3, 2022

jrcichra commented Apr 17, 2022

GZGavinZhao commented May 16, 2022

alessandrod commented May 26, 2022

GZGavinZhao commented May 27, 2022

woss commented Jul 22, 2022 • edited Loading

drahnr Jul 22, 2022

Choose a reason for hiding this comment

woss Jul 22, 2022

Choose a reason for hiding this comment

drahnr Aug 4, 2022

Choose a reason for hiding this comment

ThomasK33 Aug 4, 2022

Choose a reason for hiding this comment

drahnr left a comment • edited Loading

Choose a reason for hiding this comment

ajschmidt8 commented Nov 9, 2022

messense commented Nov 14, 2022

sylvestre commented Nov 14, 2022

sylvestre commented Nov 14, 2022

codecov-commenter commented Jan 14, 2022 •

edited

Loading

woss commented Jul 22, 2022 •

edited

Loading

drahnr left a comment •

edited

Loading