Add support VHS #446
No reviewers
TrueCloudLab/storage-services-developers
Labels
No labels
P0
P1
P2
P3
good first issue
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/frostfs-s3-gw#446
Loading…
Reference in a new issue
No description provided.
Delete branch "r.loginov/frostfs-s3-gw:feature/vhs"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
f6ee43a650
tofe34d400f8
@ -42,6 +42,13 @@ server:
# Domains to be able to use virtual-hosted-style access to bucket.
listen_domains:
In theory, it may be that the user enters an incorrect domain, for example:
s3dev<invalid-placeholder>.frostfs.devenv
s3dev.<wildcard.frostfs.devenv
s3dev.<wild.card>.frostfs.devenv
Do you think we should check the validity of these placeholders, or is it the user's responsibility?
I think we should.
@ -0,0 +83,4 @@
}
}
func checkDomain(requestURL string, domains []string) (bktName string, match bool) {
Isn't it better to rename
requestURL
tohost
?@ -0,0 +101,4 @@
}
var patternIndex, hostIndex int
for i := 0; i < len(pattern); i++ {
Maybe the following be more readable (if not just ignore this comment):
Yes. I added another condition to switch in order to correctly handle the case when our domain and pattern contain the same number of parts and the first part does not match (an example of a test for this).
It seems we can use
then.
@ -232,4 +201,0 @@
// we have to do this because of
// https://github.com/go-chi/chi/issues/641
// https://github.com/go-chi/chi/issues/642
if obj, err := url.PathUnescape(reqInfo.ObjectName); err != nil {
Do we the same in
preparePathStyleAddress
or we shouldn't ?I could not reproduce the problem described in these articles, but the search itself has not yet been closed and I do not see a solution to this problem there. So I added this to
preparePathStyleAddress
.@ -156,3 +154,2 @@
defaultRouter := chi.NewRouter()
defaultRouter.Mount(fmt.Sprintf("/{%s}", s3middleware.BucketURLPrm), bucketRouter(cfg.Handler, cfg.Log))
defaultRouter.Get("/", named("ListBuckets", cfg.Handler.ListBucketsHandler))
defaultRouter.Mount(fmt.Sprintf("/{%s}", s3middleware.BucketURLPrm), bucketRouter(cfg.Handler))
I suppose now we can write:
You mean:
defaultRouter.Mount("/{bucket}", bucketRouter(cfg.Handler))
?Yes, sure. I've just meant that
s3middleware.BucketURLPrm
isn't used for getting value, so we can drop separate const for this.@ -164,3 +159,1 @@
hr.Map(domain, bucketRouter(cfg.Handler, cfg.Log))
}
api.Mount("/", hr)
vhsRouter := bucketRouter(cfg.Handler)
Should we also write
attachErrorHandler(vhsRouter)
?No, you shouldn't, because
bucketRouter
already has it.@ -181,0 +203,4 @@
cfg.Handler.ListBucketsHandler(w, r)
} else {
reqInfo.API = s3middleware.ListObjectsV1Operation
cfg.Handler.ListObjectsV1Handler(w, r)
Should we check query params to determine which version of listing is used?
If I understand correctly it can be one of
ListObjectsV1
ListObjectsV2
ListMultipartUploads
ListObjectVersins
Oh, that's right. There may even be not only a listing, but almost all bucket operations for a GET request. It seems the shortest solution seems to be to call the
listWrapper
function in the default bucket handler for GET requests. At the same time, it will not break the routing logic with path-style addressing.Let's add test:
@ -248,0 +260,4 @@
defer s.mu.Unlock()
s.domains = fetchDomains(v)
s.vhsEnabled = v.GetBool(cfgVHSEnabled)
Let's keep critical section as small as possible.
@ -670,0 +703,4 @@
}
if len(vhsNamespacesEnabled) > 0 {
return vhsNamespacesEnabled
It seems we can unconditionally return
vhsNamespaceEnabled
fe34d400f8
tobe9944ba37
be9944ba37
to94070b107b
r.loginov referenced this pull request2024-08-01 14:38:04 +00:00
Add support VHSto Add support VHS