test(storage): cover GCS HTTPS OpenDAL TLS regression#6479
Open
palkakrzysiek wants to merge 1 commit into
Open
test(storage): cover GCS HTTPS OpenDAL TLS regression#6479palkakrzysiek wants to merge 1 commit into
palkakrzysiek wants to merge 1 commit into
Conversation
Author
|
minimal change in #6478 should be good enough for now |
be414ae to
d9a02a6
Compare
8160d4e to
2b4e8a8
Compare
2b4e8a8 to
f7329bb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Draft suggestion for #6478 / #6477.
This PR is intentionally based on
quickwit-oss:fix/gcs-opendal-tlsand leaves the dependency fix mechanism from #6478 untouched:The purpose of this branch is to add regression coverage proving that Quickwit's GCS storage path can perform a verified HTTPS object read when the workspace-level OpenDAL TLS feature is enabled.
The test is self-contained and does not depend on public GCS, internet access, credentials, Docker, or the host root store:
127.0.0.1port;localhostand127.0.0.1and is valid from 2020 to 3020;OpendalStoragethrough a test-only GCS constructor that shares the production initializer and only injects the local-CA HTTP client;Storage::get_sliceimplementation and verifies the returned bytes;Rangeheader.The custom reqwest client is only there to install the local test CA. I checked whether a process-global CA file could remove that plumbing, but reqwest 0.13's rustls path uses
rustls-platform-verifier:SSL_CERT_FILEis honored by the Unix/WebPKI verifier, but not by the Apple platform verifier used on macOS. Relying on a globally installed OS certificate would make the test machine-dependent, so the local CA stays explicit.If fmassot wants to merge the regression coverage, this can be used as a direct suggestion/stacked PR. If not, the dependency fix in #6478 remains independent.
How was this PR tested?
cargo test --locked -p quickwit-storage --features gcs --lib test_gcs_storage_get_slice_over_https_with_verified_tlscargo test --locked -p quickwit-storage --features gcs --lib opendal_storage::google_cloud_storage::testscargo check --locked -p quickwit-storage --features gcsrustfmt --edition 2024 --check quickwit-storage/src/opendal_storage/base.rs quickwit-storage/src/opendal_storage/google_cloud_storage.rs && git diff --checkcargo tree --locked -e features -i reqwest@0.13.3 -p quickwit-storage --features gcsshowsopendal feature "reqwest-rustls-tls"enablingreqwest feature "rustls".opendal = { version = "0.56", default-features = false }and rancargo test --offline -p quickwit-storage --features gcs --lib test_gcs_storage_get_slice_over_https_with_verified_tls; it failed becausereqwest_013::CertificateandClientBuilder::tls_certs_onlyare gated behind reqwest's TLS feature.