Skip to content

add host-style buckets support#206

Merged
arttor merged 4 commits into
clyso:mainfrom
askarsabyrov:main
May 26, 2026
Merged

add host-style buckets support#206
arttor merged 4 commits into
clyso:mainfrom
askarsabyrov:main

Conversation

@askarsabyrov

@askarsabyrov askarsabyrov commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Pull Request

Description

Resolves #208
This PR adds support for S3 virtual-host-style requests in the proxy while keeping existing path-style behavior unchanged.

It introduces host-aware bucket/object parsing in the S3 request parser and wires proxy middleware/auth to use the configured proxy.virtual_hostname value. With this change, requests like PUT / on bucket.<virtual_hostname> are correctly recognized as bucket operations (e.g. CreateBucket) instead of falling through to UndefinedMethod.

Key updates:

  • Add host-aware parsing path in pkg/s3/req_parser.go (ParseReqForHost, ParseBucketAndObjectForHost).
  • Use host-aware parser in proxy S3 middleware.
  • Update signature-v2 canonical resource handling for virtual-host-style requests.
  • Rewrite forwarded requests to backend path-style when incoming request is virtual-host-style.
  • Add virtual_hostname to proxy/standalone config wiring and pass it through startup paths.
  • Add/adjust unit tests for parser, auth resource canonicalization, and forward-path rewriting.

Checklist

Note: By submitting this PR, you agree to license your contributions under the Apache 2.0 License and follow our Code of Conduct.

@arttor arttor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for submitting the issue and PR!

can you please rebase onto current main - otherwise CI check will not pass.

what do you think about the following approch:

  • reuse address config for detecting host bucket
  • detect bucket from r.Host and rewrite r.URL.Path to path-style once in S3Middleware.
  • auth middleware check runs before req-parse, so SigV4/SigV2 verify the original wire form; the rewrite is invisible to both and there is no need to pass Get/SetVirtualHostStyle to ctx.

aloso, do you have any strategy for e2e testing? not sure that it will be easy to test it on CI with e2e test? have you tested your branch manually?

Comment thread service/proxy/config.yaml Outdated
Comment thread pkg/config/loader.go Outdated
Comment thread service/proxy/config_test.go Outdated
Comment thread service/proxy/config.go Outdated
Comment thread pkg/s3/req_parser_test.go
Comment thread service/proxy/auth/signature_v2_test.go
@askarsabyrov

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback.

Changes made:

  • rebased onto current main
  • removed separate virtual_hostname / VirtualHostname config
  • reused existing proxy address for virtual-host bucket detection
  • reverted the config loader change, since it is no longer needed
  • moved virtual-host path rewrite into S3Middleware, after auth and before routing/forwarding

All tests passed.

@arttor arttor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for updates but there are some comments and questions not adressed from the previous review round.

Comment thread service/standalone/config.yaml Outdated
@askarsabyrov

Copy link
Copy Markdown
Contributor Author

@arttor, I've addressed all the comments and resolved the discussions. Please let me know if I missed anything.

@askarsabyrov

askarsabyrov commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

Tested the branch locally
haproxy(in front of chorus) logs:

May 21 10:34:49  awesomebucket.archive.pscloud.io 10.216.232.112:57074 http_frontend chorus-proxy "HEAD /chorus.yaml HTTP/1.1" 200 458 33
May 21 10:34:50  awesomebucket.archive.pscloud.io 10.216.232.112:57074 http_frontend chorus-proxy "GET /chorus.yaml HTTP/1.1" 200 442 59

chorus debug logs:

proxy-1    | 2026-05-21T10:39:09Z DBG ../../build/service/proxy/router/server.go:125 > proxy: new request received app=proxy app_id=d87e1hmhpios738bij8g bucket=awesomebucket flow=event http_method=GET http_path=/awesomebucket/ http_query=location method=GetBucketLocation stor_type=S3 user=10390
proxy-1    | 2026-05-21T10:39:09Z DBG ../../build/pkg/ratelimit/service.go:120 > rate limit disabled app=proxy app_id=d87e1hmhpios738bij8g bucket=awesomebucket flow=event http_method=GET http_path=/awesomebucket/ http_query=location method=GetBucketLocation stor_name=object stor_type=S3 user=10390
proxy-1    | 2026-05-21T10:39:09Z DBG ../../build/service/proxy/router/server.go:125 > proxy: new request received app=proxy app_id=d87e1hmhpios738bij8g bucket=awesomebucket flow=event http_method=HEAD http_path=/chorus.yaml http_query= method=HeadObject obj_name=chorus.yaml stor_type=S3 user=10390
proxy-1    | 2026-05-21T10:39:09Z DBG ../../build/pkg/ratelimit/service.go:120 > rate limit disabled app=proxy app_id=d87e1hmhpios738bij8g bucket=awesomebucket flow=event http_method=HEAD http_path=/chorus.yaml http_query= method=HeadObject obj_name=chorus.yaml stor_name=object stor_type=S3 user=10390

Comment thread service/standalone/config.go Outdated
Comment thread service/proxy/auth/signature_v2.go Outdated
Comment thread pkg/config/loader.go
Comment thread service/proxy/config_test.go Outdated
@askarsabyrov askarsabyrov requested a review from arttor May 22, 2026 06:46
@arttor

arttor commented May 22, 2026

Copy link
Copy Markdown
Collaborator

ci check failed. can you please update your branch? main has upgraded dependencies. you can also run make test locally to make sure that ci will pass.

Signed-off-by: Askar Sabyrov <kyrkaz@gmail.com>
Signed-off-by: Askar Sabyrov <kyrkaz@gmail.com>
Signed-off-by: Askar Sabyrov <kyrkaz@gmail.com>
Signed-off-by: Askar Sabyrov <kyrkaz@gmail.com>
@askarsabyrov

askarsabyrov commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@arttor Rebased with upstream, but it didn’t resolve the CI govulncheck failure. The new failures were coming from golang.org/x/crypto@v0.50.0 advisories fixed in v0.52.0, so I updated golang.org/x/crypto to v0.52.0 and refreshed related golang.org/x/* dependencies in the root module and tool modules.
After i got:

Your code is affected by 2 vulnerabilities from 1 module.
This scan also found 1 vulnerability in packages you import and 7
vulnerabilities in modules you require, but your code doesn't appear to call
these vulnerabilities.
Use '-show verbose' for more details.

>>> All reported vulnerabilities are allowlisted in .govulncheck-ignore.yaml — PASS

@arttor arttor merged commit 315e9e8 into clyso:main May 26, 2026
2 checks passed
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.

[FEATURE] add support for virtual-host-style buckets

2 participants