[improve][offload] Support credentials from offload policies for S3 and Aliyun OSS drivers

[KannarFr]

Motivation

The S3 and aliyun-oss offload drivers resolve credentials exclusively from the ACCESS_KEY_ID/ACCESS_KEY_SECRET (or ALIYUN_OSS_ACCESS_KEY_ID/ALIYUN_OSS_ACCESS_KEY_SECRET) environment variables, and throw if they are absent.

However, OffloadPoliciesImpl carries the s3ManagedLedgerOffloadCredentialId/s3ManagedLedgerOffloadCredentialSecret properties for every S3-compatible driver — set either in broker.conf or per namespace/topic via set-offload-policies — and S3_CREDENTIAL_BUILDER silently ignores them. Credentials configured through offload policies therefore never take effect for these drivers, and multi-tenant setups where each namespace offloads to its own bucket with its own credentials cannot work: environment variables are global to the broker process.

The aws-s3 driver already resolves these same configuration properties first (in AWS_CREDENTIAL_BUILDER); this change aligns the S3 and aliyun-oss drivers with that behavior.

Modifications

In JCloudBlobStoreProvider.S3_CREDENTIAL_BUILDER (shared by the S3 and ALIYUN_OSS providers):

• Return early if credentials are already set on the configuration.
• If both s3ManagedLedgerOffloadCredentialId and s3ManagedLedgerOffloadCredentialSecret are present and non-blank in the tiered storage configuration, use them (they take precedence over the environment variables).
• If exactly one of the pair is supplied, throw an IllegalArgumentException naming both keys, instead of silently ignoring the partial pair and failing later with a misleading env-var error.
• Otherwise, fall back to the environment variables exactly as before (same lookup order, same error messages).

Blank/whitespace-only values are treated as absent rather than being accepted as credentials.

Verifying this change

This change added tests and can be verified as follows:

• Added s3CredentialsFromConfigTest and aliyunOssCredentialsFromConfigTest verifying that a credential pair supplied via the configuration is used to build the provider credentials for the S3 and aliyun-oss drivers.
• Added s3PartialCredentialsFromConfigTest verifying that supplying only one of the two keys fails fast with a clear message.
• Existing JCloudBlobStoreProviderTest validation tests are unaffected (13/13 pass).

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

• Dependencies (add or upgrade a dependency)
• The public API
• The schema
• The default values of configurations
• The threading model
• The binary protocol
• The REST endpoints
• The admin CLI options
• The metrics
• Anything that affects deployment

Behavior change for S3/aliyun-oss deployments that have s3ManagedLedgerOffloadCredentialId/Secret set in their configuration while relying on environment variables: those configuration values were previously inert for these drivers and now take precedence. Deployments without these configuration keys keep the exact previous behavior. Additionally, a partial credential pair in the configuration now fails fast at credential-building time instead of being silently ignored.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Tu1LLcX9KGPhnZygeWRZXQ

[lhotari]

Reviewed with AI assistance: Claude Code (Claude Fable 5) combined with an OpenAI Codex (gpt-5.6-sol) review of the branch diff.

Overall: The implementation matches the stated intent and looks safe to merge. S3_CREDENTIAL_BUILDER (shared by S3 and aliyun-oss) now gives config-provided credentials the same precedence AWS_CREDENTIAL_BUILDER has always had for aws-s3, and the env-var fallback path is untouched. Verified the wiring: OffloadPoliciesImpl.toProperties() copies the s3-prefixed credential fields by name, so they do land in the TieredStorageConfiguration map for these drivers. Ran JCloudBlobStoreProviderTest locally: 13/13 pass. Codex's verdict: "The change correctly prioritizes complete configured credentials for S3-compatible drivers, rejects incomplete pairs, and preserves the existing environment-variable fallback. The added tests cover both affected drivers and partial configuration."

Two observations, neither blocking:

1. CLI gap for aliyun-oss (pre-existing): OffloadPoliciesImpl.create(driver, …) — the path used by pulsar-admin namespaces/topics set-offload-policies — only copies credentialId/credentialSecret into the s3-prefixed fields when the driver is S3 or aws-s3. For -d aliyun-oss the CLI silently drops the credentials before they reach the offloader, so this fix is only reachable for Aliyun via broker.conf or a policies object set through the admin API. Worth a follow-up extending the driver check in create() (and isS3Driver()). The new code comment ("…carry credentials under the s3-prefixed keys for every S3-compatible driver") slightly overstates the current CLI behavior.

2. Behavior change worth a release note: (a) deployments that set both credential keys in config while relying on env vars now use the config values; (b) a partial pair now fails fast with IllegalArgumentException instead of silently falling through to env vars. Both are disclosed in the PR description; flagging so it gets picked up for release notes, since operators with stale/partial keys in broker.conf would see a new failure at credential-build time.

Also checked: the static Credentials supplier is appropriate for fixed config values (matches the existing env-var path), the exception message names the keys but never the values, and the new tests are deterministic regardless of ACCESS_KEY_ID/ACCESS_KEY_SECRET being set in the environment.

[KannarFr]

@lhotari thanks for the review! Addressed observation #1 in 164fb6f: OffloadPoliciesImpl.create(driver, …) (the set-offload-policies CLI path) now copies credentialId/credentialSecret into the s3-prefixed keys for aliyun-oss too — extended the driver check in both create() and isS3Driver() since aliyun-oss is S3-compatible (shares S3_CREDENTIAL_BUILDER and reads the s3ManagedLedgerOffload* keys). This also makes the code comment you flagged accurate and lets bucketValid() check the s3-prefixed bucket for aliyun-oss. Added testAliyunOssConfiguration covering it; checkstyle + OffloadPoliciesTest pass locally.

For observation #2, the behavior change is already noted in the PR description — flagging here so it gets picked up for release notes.