test, docs: cover configurable S3 addressing style by gavishtea · Pull Request #8991 · triton-inference-server/server

gavishtea · 2026-09-29T20:39:46Z

What does the PR do?

Add tests and documentation for the configurable S3 addressing-style feature (triton-inference-server/core: S3_USE_VIRTUAL_ADDRESSING env var and use_virtual_addressing credential-file field), which unblocks
S3-compatible stores that require virtual-hosted-style addressing (e.g. Tencent COS, Alibaba OSS, Baidu bcebos - see #6645).

Tests (qa/L0_storage_S3_local):

mock_s3_addressing_service.py: mock S3 endpoint that classifies each request as path-style vs virtual-hosted and asserts the expected style.
test.sh: three cases - default (path-style preserved), env var enables virtual-hosted, and JSON credential field enables virtual-hosted.

Docs (docs/user_guide/model_repository.md):

Document S3_USE_VIRTUAL_ADDRESSING and the use_virtual_addressing credential-file field, the default path-style behavior, and the DNS-compatible bucket-name constraint.

Checklist

Commit Type:

Check the conventional commit type
box here and add the label to the github PR.

Related PRs:

feat: make S3 addressing style configurable core#530 — the core implementation this PR tests and documents.

Where should the reviewer start?

qa/L0_storage_S3_local/mock_s3_addressing_service.py — the mock S3 endpoint
that classifies each request as path-style (Host: <endpoint>) vs
virtual-hosted (Host: <bucket>.<endpoint>) and exits non-zero on mismatch.
qa/L0_storage_S3_local/test.sh — the new "S3 addressing style tests" section
wiring the three cases.
docs/user_guide/model_repository.md — the new S3_USE_VIRTUAL_ADDRESSING /
use_virtual_addressing documentation.

Test plan:

qa/L0_storage_S3_local exercises the core flag via a mock S3 endpoint that
asserts the addressing style Triton emits:

No setting → path-style request (default behavior preserved).
S3_USE_VIRTUAL_ADDRESSING=true → virtual-hosted request.
"use_virtual_addressing": true in a TRITON_CLOUD_CREDENTIAL_PATH JSON file
→ virtual-hosted request.

The mock's classification logic was verified independently (crafted Host
headers: virtual + expect-virtual → pass; path + expect-virtual → fail;
path + expect-path → pass). bash -n test.sh and py_compile on the mock pass,
and pre-commit run --all passes on all changed files.

Caveats:

Tests live under qa/L0_storage_S3_local (the MinIO-based local suite), not
qa/L0_storage_S3 (which targets real AWS S3), since the mock validates
addressing style without requiring a live S3-compatible endpoint. The AWS SDK
only applies virtual-hosted addressing for DNS-compatible bucket names
(all-lowercase, no dots); this constraint is documented in the model-repository
guide. This PR is documentation + tests only and depends on the companion core
PR for the actual functionality.

Background

S3-compatible stores such as Tencent COS (buckets created after 2024-01-01),
Alibaba OSS, and Baidu bcebos require virtual-hosted-style addressing, which
Triton's previously hardcoded path-style S3 client could not produce
(surfacing as Unable to create S3 filesystem client, see #6645). The companion
core PR makes the addressing style configurable; this PR adds the tests and user
documentation for it.

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

Relates to GitHub issue: [Question] Internal: Unable to create S3 filesystem client. Check account credentials. #6645

Add tests and documentation for the configurable S3 addressing-style feature (triton-inference-server/core: S3_USE_VIRTUAL_ADDRESSING env var and use_virtual_addressing credential-file field), which unblocks S3-compatible stores that require virtual-hosted-style addressing (e.g. Tencent COS, Alibaba OSS, Baidu bcebos - see triton-inference-server#6645). Tests (qa/L0_storage_S3_local): - mock_s3_addressing_service.py: mock S3 endpoint that classifies each request as path-style vs virtual-hosted and asserts the expected style. - test.sh: three cases - default (path-style preserved), env var enables virtual-hosted, and JSON credential field enables virtual-hosted. Docs (docs/user_guide/model_repository.md): - Document S3_USE_VIRTUAL_ADDRESSING and the use_virtual_addressing credential-file field, the default path-style behavior, and the DNS-compatible bucket-name constraint. Signed-off-by: gavishtea <gavishtea@users.noreply.github.com>

greptile-apps · 2026-09-29T20:43:54Z

Confidence Score: 5/5

[Low risk] Adds test coverage and documentation for S3 addressing modes.

The PR appears safe to merge; no outstanding or new findings were identified.

Summary

This PR documents configurable S3 addressing and adds a mock-backed test suite for path-style and virtual-hosted-style requests.

Covers the default, environment-variable, credential-file, and credential-file fallback cases.
Documents the setting, precedence, and bucket-name constraint.

_{Reviews (5) · Last reviewed commit: "Merge branch 'main' into configurable-s3..."}

Add a regression case to qa/L0_storage_S3_local for the scenario where a credential file is present without the use_virtual_addressing field but S3_USE_VIRTUAL_ADDRESSING=true is set; the resulting requests must be virtual-hosted-style. Document that the credential-file field takes precedence and the environment variable is used as a fallback when the field is omitted. Signed-off-by: gavishtea <gavishtea@users.noreply.github.com>

The virtual-hosted-style cases cause the S3 client to connect to <bucket>.localhost, but whether *.localhost resolves to loopback is platform/resolver dependent. If the QA runner does not resolve dummy-bucket.localhost, those requests never reach the mock (which listened only on localhost) and the test times out without validating the addressing style Triton actually chose. Map <bucket>.localhost to 127.0.0.1 in /etc/hosts for the duration of the addressing tests (added only if absent, removed afterward) and bind the mock to all interfaces, so the bucket-prefixed hostname reliably reaches the mock regardless of the runner's resolver behavior. Signed-off-by: gavishtea <gavishtea@users.noreply.github.com>

Address review feedback on the S3 addressing-style tests: - Failed wait no longer skips reporting/cleanup: with set -e active, a failing mock (exit 1) made 'wait ${mock_pid}' abort the script before the failure was reported and before the credential file / hosts mapping were cleaned up. The wait status is now captured under 'set +e' so the case is reported and the script proceeds to cleanup. - /etc/hosts mapping and generated credential files are now removed via an EXIT trap, so they are cleaned up even when a case fails and set -e aborts the run (previously the mapping could survive and leak into later tests). - Each addressing case uses a distinct log-file label (default_path, envvar_virtual, credfile_virtual, credfile_envvar_virtual) so the three virtual-hosted cases no longer overwrite each other's mock/server logs. Signed-off-by: gavishtea <gavishtea@users.noreply.github.com>

whoisj · 2026-10-01T22:31:57Z

@gavishtea, thank you for your contribution. Please see my comment on the core change. Thank you.

gavishtea · 2026-10-01T23:02:00Z

@whoisj Thank you! I addressed the comment on the core change, please let me know if there's anything else I can do!

Copilot

Copilot review overview

🟡 Changes recommended

The mock can falsely classify malformed requests, and credential precedence lacks coverage.

Review effort: Balanced
Findings: 2

Open (2)

Validate both host and path for path-style classification · New
Test JSON setting precedence over environment fallback · New

What changed in this PR

Adds documentation and QA coverage for configurable S3 addressing styles.

Changes:

Adds an S3 addressing-style mock service.
Tests environment-variable and credential-file configuration.
Documents configuration, defaults, precedence, and bucket constraints.

File	Description
`qa/L0_storage_S3_local/test.sh`	Adds addressing-style test cases and cleanup.
`qa/L0_storage_S3_local/mock_s3_addressing_service.py`	Classifies mock S3 requests by addressing style.
`docs/user_guide/model_repository.md`	Documents virtual-hosted S3 configuration.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+                host = self.headers.get("host", "").lower()
+                if host.startswith(bucket_name.lower() + "."):
+                    results["virtual_hosted_count"] += 1
+                else:
+                    results["path_style_count"] += 1


+unset S3_USE_VIRTUAL_ADDRESSING
+unset TRITON_CLOUD_CREDENTIAL_PATH
+rm -f ${CRED_FILE_NOFLAG}


greptile-apps Bot reviewed Sep 29, 2026

View reviewed changes

Comment thread qa/L0_storage_S3_local/test.sh

Comment thread qa/L0_storage_S3_local/test.sh Outdated

Comment thread qa/L0_storage_S3_local/test.sh Outdated

gavishtea mentioned this pull request Sep 29, 2026

feat: make S3 addressing style configurable triton-inference-server/core#530

Open

9 of 22 tasks

gavishtea added 2 commits September 29, 2026 14:09

greptile-apps Bot reviewed Sep 29, 2026

View reviewed changes

Comment thread qa/L0_storage_S3_local/test.sh Outdated

Merge branch 'main' into configurable-s3-addressing

0fff3ab

mc-nv requested review from mc-nv and whoisj and a balanced review from Copilot October 1, 2026 23:42

Copilot started reviewing on behalf of mc-nv October 1, 2026 23:43 View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

Navigation Menu

test, docs: cover configurable S3 addressing style - #8991

test, docs: cover configurable S3 addressing style#8991
gavishtea wants to merge 5 commits into
triton-inference-server:mainfrom
gavishtea:configurable-s3-addressing

gavishtea commented Sep 29, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Sep 29, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

whoisj commented Oct 1, 2026

Uh oh!

gavishtea commented Oct 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

Conversation

gavishtea commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does the PR do?

Checklist

Commit Type:

Related PRs:

Where should the reviewer start?

Test plan:

Caveats:

Background

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

Uh oh!

greptile-apps Bot commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

whoisj commented Oct 1, 2026

Uh oh!

gavishtea commented Oct 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

gavishtea commented Sep 29, 2026 •

edited

Loading

greptile-apps Bot commented Sep 29, 2026 •

edited

Loading