Skip to content

test(storage): cover GCS HTTPS OpenDAL TLS regression#6479

Open
palkakrzysiek wants to merge 1 commit into
quickwit-oss:fix/gcs-opendal-tlsfrom
palkakrzysiek:codex/fix-gcs-opendal-tls
Open

test(storage): cover GCS HTTPS OpenDAL TLS regression#6479
palkakrzysiek wants to merge 1 commit into
quickwit-oss:fix/gcs-opendal-tlsfrom
palkakrzysiek:codex/fix-gcs-opendal-tls

Conversation

@palkakrzysiek
Copy link
Copy Markdown

@palkakrzysiek palkakrzysiek commented May 30, 2026

Description

Draft suggestion for #6478 / #6477.

This PR is intentionally based on quickwit-oss:fix/gcs-opendal-tls and leaves the dependency fix mechanism from #6478 untouched:

opendal = { version = "0.56", default-features = false, features = ["reqwest-rustls-tls"] }

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:

  • it starts a local TLS server on an ephemeral 127.0.0.1 port;
  • the server certificate has SANs for localhost and 127.0.0.1 and is valid from 2020 to 3020;
  • the test client trusts only the embedded test CA, so certificate verification stays enabled;
  • the OpenDAL GCS backend is pointed at that HTTPS endpoint;
  • Quickwit builds OpendalStorage through a test-only GCS constructor that shares the production initializer and only injects the local-CA HTTP client;
  • the assertion uses Quickwit's production Storage::get_slice implementation and verifies the returned bytes;
  • the server validates that the request is the expected GCS object read path with the expected Range header.

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_FILE is 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?

  • Positive check: cargo test --locked -p quickwit-storage --features gcs --lib test_gcs_storage_get_slice_over_https_with_verified_tls
  • Existing GCS unit tests: cargo test --locked -p quickwit-storage --features gcs --lib opendal_storage::google_cloud_storage::tests
  • Build check: cargo check --locked -p quickwit-storage --features gcs
  • Format/check whitespace: rustfmt --edition 2024 --check quickwit-storage/src/opendal_storage/base.rs quickwit-storage/src/opendal_storage/google_cloud_storage.rs && git diff --check
  • Feature check: cargo tree --locked -e features -i reqwest@0.13.3 -p quickwit-storage --features gcs shows opendal feature "reqwest-rustls-tls" enabling reqwest feature "rustls".
  • Negative check: temporarily changed the base branch dependency back to opendal = { version = "0.56", default-features = false } and ran cargo test --offline -p quickwit-storage --features gcs --lib test_gcs_storage_get_slice_over_https_with_verified_tls; it failed because reqwest_013::Certificate and ClientBuilder::tls_certs_only are gated behind reqwest's TLS feature.

@palkakrzysiek palkakrzysiek requested review from a team as code owners May 30, 2026 12:39
@palkakrzysiek
Copy link
Copy Markdown
Author

minimal change in #6478 should be good enough for now

@palkakrzysiek palkakrzysiek reopened this May 30, 2026
@palkakrzysiek palkakrzysiek marked this pull request as draft May 30, 2026 13:15
@palkakrzysiek palkakrzysiek force-pushed the codex/fix-gcs-opendal-tls branch from be414ae to d9a02a6 Compare May 30, 2026 13:22
@palkakrzysiek palkakrzysiek changed the title fix(storage): restore TLS for GCS OpenDAL backend test(storage): cover GCS HTTPS OpenDAL TLS regression May 30, 2026
@palkakrzysiek palkakrzysiek changed the base branch from main to fix/gcs-opendal-tls May 30, 2026 13:22
@palkakrzysiek palkakrzysiek force-pushed the codex/fix-gcs-opendal-tls branch 5 times, most recently from 8160d4e to 2b4e8a8 Compare May 30, 2026 14:18
@palkakrzysiek palkakrzysiek force-pushed the codex/fix-gcs-opendal-tls branch from 2b4e8a8 to f7329bb Compare May 30, 2026 14:20
@palkakrzysiek palkakrzysiek marked this pull request as ready for review May 30, 2026 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GCS split uploads fail with invalid URL, scheme is not http since opendal 0.56 (reqwest built without TLS)pr

1 participant