[papaharry]

Description

• Adds a storage.s3.buckets map, keyed by bare bucket name, so a single Quickwit cluster can address multiple S3-compatible backends (separate credentials, region, endpoint, checksum settings, flavor and flags per bucket).
• Each entry is a full S3StorageConfig (same fields as the primary s3: block, no inheritance from it). Nested buckets are rejected.
• When an s3://<bucket>/... URI is resolved, an exact bucket-name match uses that entry; any unlisted bucket falls back to the primary s3: block.
• URIs stay canonical s3://bucket/path. Routing is decided at runtime from each node's own config, so nothing new is persisted in the metastore / index metadata. Every node that touches a bucket (indexers, searchers, janitor, metastore) must declare the same entry.
• Bucket entries are self-contained: QW_S3_ENDPOINT and QW_S3_FORCE_PATH_STYLE_ACCESS apply to the primary backend only. This replaces the process-wide OnceLock cache in force_path_style_access(), which would otherwise have leaked the first backend's value into every other backend.
• Caveat: because routing is by bucket name, a bucket name maps to exactly one backend.

Changes:

• quickwit-config/src/storage_config.rs: buckets: BTreeMap<String, S3StorageConfig> and a non-serialized is_bucket_config flag on S3StorageConfig; bucket_configs() yields each entry with is_bucket_config set; apply_flavor() and redact() recurse into bucket entries; Debug includes buckets; endpoint() / force_path_style_access() skip the env overrides for bucket configs. StorageConfigs::validate() rejects empty keys, keys containing / (e.g. s3://bucket or bucket/prefix), and nested buckets.
• quickwit-storage/src/object_storage/s3_compatible_storage_resolver.rs: the factory now holds a default S3Backend plus one S3Backend per bucket entry, each with its own lazily built, cached S3Client. backend_for_uri() picks the backend via parse_s3_uri.
• docs/configuration/storage-config.md: buckets row in the S3 properties table, QW_S3_FORCE_PATH_STYLE_ACCESS env var documented, and a new "Per-bucket S3 configuration" section with a YAML example and notes on env overrides, per-node config, YAML quoting of numeric/boolean bucket names, and bucket-name uniqueness.

How was this PR tested?

Unit tests in quickwit-config (storage_config::tests):

• test_storage_s3_buckets_serde: YAML deserialization and bucket_configs()
• test_storage_s3_buckets_reject_nested_buckets
• test_storage_s3_buckets_reject_unmatchable_bucket_keys: empty, s3://... and .../prefix keys
• test_storage_s3_buckets_redact: secrets redacted in Debug and redact()
• test_storage_s3_buckets_apply_flavor: flavor and deprecated disable_checksums expand per bucket
• test_storage_s3_bucket_config_uses_own_endpoint: env overrides hit the primary backend only

Unit test in quickwit-storage (s3_compatible_storage_resolver::tests):

• test_s3_factory_routes_by_exact_bucket_name: exact match, bucket-only URI, dotted bucket names, near-miss and unlisted buckets fall back to primary

End-to-end through the REST stack (ingest -> split -> upload -> publish -> search) against an in-cluster SeaweedFS backend and an external OVH backend.

This fork has been running on my staging environment for a couple of weeks and everything is working as expected. The indexes are created and the source config also works as intended.

[chatgpt-codex-connector]

💡 Codex Review

https://github.com/quickwit-oss/quickwit/blob/c7b3a9b51aebfb12cf7d40470417b5e9d1a8203c/quickwit-common/src/uri.rs#L305-L309
Fix Uri path slicing for named S3 schemes

When s3+<name> is preserved here, Uri::path() still strips only self.protocol.as_str().len() + 3 bytes (s3://), so s3+alt://bucket/file is treated as the path t://bucket/file. Any caller that uses parent() before resolving, such as quickwit_storage::load_file or object URI sources via dir_and_filename, will produce s3://t://bucket and route to the wrong primary bucket/backend instead of the named backend.

---

https://github.com/quickwit-oss/quickwit/blob/c7b3a9b51aebfb12cf7d40470417b5e9d1a8203c/quickwit-storage/src/object_storage/s3_compatible_storage_resolver.rs#L83
Keep path-style access per named backend

When different S3 backends require different force_path_style_access values, the first client created wins because create_s3_client() calls S3StorageConfig::force_path_style_access(), which caches the value in a process-wide OnceLock. If a plain s3:// client with the default false is resolved before s3+minio:// with true, this named client is still built with false, so path-style-only endpoints fail despite the named config setting.

---

https://github.com/quickwit-oss/quickwit/blob/c7b3a9b51aebfb12cf7d40470417b5e9d1a8203c/quickwit-config/src/storage_config.rs#L406
Apply flavor overrides to named S3 backends

When a named backend is configured with flavor: minio, garage, gcs, or digital_ocean, this projection only copies the flavor into a fresh S3StorageConfig; the only code that expands flavors is StorageConfigs::apply_flavors() on the top-level S3 config, and it never visits named. As a result named backends miss the expected region/path-style/checksum/delete overrides and can fail even though the same flavor works on the primary S3 block.

---

https://github.com/quickwit-oss/quickwit/blob/c7b3a9b51aebfb12cf7d40470417b5e9d1a8203c/quickwit-storage/src/object_storage/s3_compatible_storage_resolver.rs#L83
Bypass the global S3 endpoint for named backends

When QW_S3_ENDPOINT is set for the primary S3 backend, this new named-backend path passes the projected config into create_s3_client(), whose S3StorageConfig::endpoint() always prefers that env var over the struct endpoint. In that environment every s3+<name>:// backend ignores storage.s3.named.<name>.endpoint and connects to the global endpoint, so multi-backend clusters can route named indexes to the wrong service.

---

https://github.com/quickwit-oss/quickwit/blob/c7b3a9b51aebfb12cf7d40470417b5e9d1a8203c/quickwit-config/src/storage_config.rs#L392-L393
Preserve the multi-delete alias for named S3 configs

Because NamedS3StorageConfig denies unknown fields, this field no longer accepts the disable_multi_object_delete_requests alias that the primary S3StorageConfig accepts. If an existing S3-compatible config using that key is moved under storage.s3.named.<name> as documented as the same S3 fields, node config parsing fails at startup even though the same option works for plain s3://.

---

https://github.com/quickwit-oss/quickwit/blob/c7b3a9b51aebfb12cf7d40470417b5e9d1a8203c/quickwit-config/src/storage_config.rs#L396-L398
Accept stalled-stream toggles on named S3 configs

Named backends are documented as accepting the same fields as the primary S3 block, but this struct stops after checksum_algorithm and denies unknown fields, so configs that need disable_stalled_stream_protection_upload or disable_stalled_stream_protection_download for a particular S3-compatible endpoint now fail to parse when moved under storage.s3.named.<name>. Even if parsing were allowed, as_s3_config() currently hard-codes those projected flags to false, so the named backend could not disable the protection independently.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9ebf5acfde

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[guilload]

I'm not fond of embedding the "profile" in the storage which eventually gets persisted in the metastore. Can we consider an alternative design with per bucket profiles:

storage:
  s3:
    region: us-east-1            # base defaults for any unlisted bucket
    profiles:
      logs-bucket-eu:            # the bucket name IS the key
        endpoint: https://s3.eu-west-3.amazonaws.com
        region: eu-west-3
        access_key_id: ${SECONDARY_S3_ACCESS_KEY_ID}
        secret_access_key: ${SECONDARY_S3_SECRET_ACCESS_KEY}
      seaweed-logs:
        endpoint: http://seaweedfs-s3:8333
        force_path_style_access: true

[papaharry]

Thanks for that. Yeah, makes sense and will make it clearer too. I'll update it (and test on my environment) and will tag you in for another review when I think it's ready again if that's ok (will be a few days before I update it and test it)

[guilload]

Sounds good. Thanks for the PR, it's a useful feature.

[papaharry]

Hi @guilload, the change has been done (example)
I've been running it locally and works as expected. Is this what you were suggesting?

[dayaffe]

Thanks for submitting this PR and for switching to bucket-keyed profiles as requested! I don't see any remaining blockers. Please update the PR description though as it still describes the old s3+:// approach.

LGTM

[papaharry]

Thank you, description updated.

[dayaffe]

Hi @papaharry Thanks again! After discussing internally we decided we actually wanted to change profiles back to buckets and reduce some duplication. Have a look at defensedotcom#6 which includes suggested changes. If you merge that it should update here!

[papaharry]

Thanks for getting back to me @dayaffe. PR has been updated (along with description and title updates).
Let me know if more changes are required and I'll have a look

[dayaffe]

@codex review

[guilload]

Approved but address my nits.

[guilload]

Suggested change

	&& let Ok(endpoint) = env::var("QW_S3_ENDPOINT")
	&& let Ok(endpoint) = get_bool_from_env("QW_S3_ENDPOINT")

[guilload]

:/

Let's introduce some test-only methods on S3StorageConfig if need be so we don't have to do that.

[guilload]

Suggested change

	default_backend: S3Backend,
	default_backend: LazyS3Client,

StorageBackend already exists in quickwit-storage

[guilload]

"bare" is weird and I don't think it's even needed:

"The map key is the bucket name, e.g. my-bucket, not s3://my-bucket or my-bucket/prefix; such keys are rejected at startup."

[guilload]

Suggested change

	"S3 bucket config key `{bucket}` must be a bare bucket name, without `s3://` \
	or `/`",
	"invalid S3 bucket config key `{bucket}`: expected a bucket name (e.g. `my-bucket`), \
	not a URI or path",

[guilload]

Suggested change

	ensure!(
	bucket_config.buckets.is_empty(),
	"S3 bucket config `{bucket}` cannot define nested `buckets`",
	);
	ensure!(!bucket.is_empty(), "S3 bucket config key must not be empty");
	ensure!(
	!bucket.contains('/'),
	"invalid S3 bucket config key `{bucket}`: expected a bucket name (e.g. `my-bucket`), \
	not a URI or path",
	);