From c1f3110dcc1ff0b742de1857d5ea99c96a79c06e Mon Sep 17 00:00:00 2001 From: Roman Loginov Date: Mon, 16 Dec 2024 10:49:39 +0300 Subject: [PATCH 1/4] [#584] Return BucketAlreadyExists when global domain taken A situation may occur when the global domain is already occupied when creating the container. The error comes from the nns smart contract. This error actually means that the bucket has already been created. Signed-off-by: Roman Loginov --- api/errors/errors.go | 4 ++++ api/layer/frostfs/frostfs.go | 3 +++ internal/frostfs/frostfs.go | 5 +++++ 3 files changed, 12 insertions(+) diff --git a/api/errors/errors.go b/api/errors/errors.go index 775add32..7ce7c5fe 100644 --- a/api/errors/errors.go +++ b/api/errors/errors.go @@ -1822,6 +1822,10 @@ func TransformToS3Error(err error) error { return GetAPIError(ErrGatewayTimeout) } + if errors.Is(err, frostfs.ErrGlobalDomainIsAlreadyTaken) { + return GetAPIError(ErrBucketAlreadyExists) + } + return GetAPIError(ErrInternalError) } diff --git a/api/layer/frostfs/frostfs.go b/api/layer/frostfs/frostfs.go index f813d0b8..847ebd39 100644 --- a/api/layer/frostfs/frostfs.go +++ b/api/layer/frostfs/frostfs.go @@ -233,6 +233,9 @@ var ( // ErrGatewayTimeout is returned from FrostFS in case of timeout, deadline exceeded etc. ErrGatewayTimeout = errors.New("gateway timeout") + + // ErrGlobalDomainIsAlreadyTaken is returned from FrostFS in case of global domain is already taken. + ErrGlobalDomainIsAlreadyTaken = errors.New("global domain is already taken") ) // FrostFS represents virtual connection to FrostFS network. diff --git a/internal/frostfs/frostfs.go b/internal/frostfs/frostfs.go index f89b94be..7298d97f 100644 --- a/internal/frostfs/frostfs.go +++ b/internal/frostfs/frostfs.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "strconv" + "strings" "time" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer/frostfs" @@ -477,6 +478,10 @@ func handleObjectError(msg string, err error) error { return fmt.Errorf("%s: %w: %s", msg, frostfs.ErrGatewayTimeout, err.Error()) } + if strings.Contains(err.Error(), "global domain is already taken") { + return fmt.Errorf("%s: %w: %s", msg, frostfs.ErrGlobalDomainIsAlreadyTaken, err.Error()) + } + return fmt.Errorf("%s: %w", msg, err) } -- 2.45.2 From a990fb13d1a928399fa26f5e23ad519d6ae27367 Mon Sep 17 00:00:00 2001 From: Roman Loginov Date: Mon, 16 Dec 2024 10:52:16 +0300 Subject: [PATCH 2/4] [#583] Fix list-buckets vhs routing The problem is that with VHS requests, the list-buckets operation does not work because the request is filtered on list-objects-v1. Since list-buckets can also have query parameters, in the end it is necessary to distinguish list-buckets from list-objects-v1 only by the presence of the bucket name in the URL (provided that the request is in VHS style). Signed-off-by: Roman Loginov --- api/middleware/policy.go | 17 ++++++---- api/router.go | 63 +++++++++++++++++++++-------------- api/router_filter.go | 9 ++--- api/router_test.go | 72 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 126 insertions(+), 35 deletions(-) diff --git a/api/middleware/policy.go b/api/middleware/policy.go index 9e36f0be..982fb3a5 100644 --- a/api/middleware/policy.go +++ b/api/middleware/policy.go @@ -24,13 +24,16 @@ import ( ) const ( - QueryVersionID = "versionId" - QueryPrefix = "prefix" - QueryDelimiter = "delimiter" - QueryMaxKeys = "max-keys" - QueryMarker = "marker" - QueryEncodingType = "encoding-type" - amzTagging = "x-amz-tagging" + QueryVersionID = "versionId" + QueryPrefix = "prefix" + QueryDelimiter = "delimiter" + QueryMaxKeys = "max-keys" + QueryMarker = "marker" + QueryEncodingType = "encoding-type" + QueryMaxBuckets = "max-buckets" + QueryContinuationToken = "continuation-token" + QueryBucketRegion = "bucket-region" + amzTagging = "x-amz-tagging" unmatchedBucketOperation = "UnmatchedBucketOperation" ) diff --git a/api/router.go b/api/router.go index 765bec30..49f348fd 100644 --- a/api/router.go +++ b/api/router.go @@ -159,7 +159,7 @@ func NewRouter(cfg Config) *chi.Mux { defaultRouter.Get("/", named(s3middleware.ListBucketsOperation, cfg.Handler.ListBucketsHandler)) attachErrorHandler(defaultRouter) - vhsRouter := bucketRouter(cfg.Handler) + vhsRouter := newDomainRouter(cfg.Handler) router := newGlobalRouter(defaultRouter, vhsRouter) api.Mount("/", router) @@ -169,12 +169,43 @@ func NewRouter(cfg Config) *chi.Mux { return api } -type globalRouter struct { - pathStyleRouter chi.Router - vhsRouter chi.Router +type domainRouter struct { + bucketRouter chi.Router + defaultRouter chi.Router } -func newGlobalRouter(pathStyleRouter, vhsRouter chi.Router) *globalRouter { +func newDomainRouter(handler Handler) *domainRouter { + defaultRouter := chi.NewRouter() + defaultRouter.Group(func(r chi.Router) { + r.Method(http.MethodGet, "/", NewHandlerFilter(). + Add(NewFilter(). + AllowedQueries(s3middleware.QueryMaxBuckets, s3middleware.QueryPrefix, + s3middleware.QueryContinuationToken, s3middleware.QueryBucketRegion). + Handler(named(s3middleware.ListBucketsOperation, handler.ListBucketsHandler))). + DefaultHandler(notSupportedHandler())) + }) + attachErrorHandler(defaultRouter) + + return &domainRouter{ + bucketRouter: bucketRouter(handler), + defaultRouter: defaultRouter, + } +} + +func (g *domainRouter) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if reqInfo := s3middleware.GetReqInfo(r.Context()); reqInfo.BucketName != "" { + g.bucketRouter.ServeHTTP(w, r) + } else { + g.defaultRouter.ServeHTTP(w, r) + } +} + +type globalRouter struct { + pathStyleRouter chi.Router + vhsRouter *domainRouter +} + +func newGlobalRouter(pathStyleRouter chi.Router, vhsRouter *domainRouter) *globalRouter { return &globalRouter{ pathStyleRouter: pathStyleRouter, vhsRouter: vhsRouter, @@ -182,12 +213,11 @@ func newGlobalRouter(pathStyleRouter, vhsRouter chi.Router) *globalRouter { } func (g *globalRouter) ServeHTTP(w http.ResponseWriter, r *http.Request) { - router := g.pathStyleRouter if reqInfo := s3middleware.GetReqInfo(r.Context()); reqInfo.RequestVHSEnabled { - router = g.vhsRouter + g.vhsRouter.ServeHTTP(w, r) + } else { + g.pathStyleRouter.ServeHTTP(w, r) } - - router.ServeHTTP(w, r) } func named(name string, handlerFunc http.HandlerFunc) http.HandlerFunc { @@ -338,9 +368,6 @@ func bucketRouter(h Handler) chi.Router { AllowedQueries(s3middleware.QueryDelimiter, s3middleware.QueryMaxKeys, s3middleware.QueryPrefix, s3middleware.QueryMarker, s3middleware.QueryEncodingType). Handler(named(s3middleware.ListObjectsV1Operation, h.ListObjectsV1Handler))). - Add(NewFilter(). - NoQueries(). - Handler(listWrapper(h))). DefaultHandler(notSupportedHandler())) }) @@ -422,18 +449,6 @@ func bucketRouter(h Handler) chi.Router { return bktRouter } -func listWrapper(h Handler) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - if reqInfo := s3middleware.GetReqInfo(r.Context()); reqInfo.BucketName == "" { - reqInfo.API = s3middleware.ListBucketsOperation - h.ListBucketsHandler(w, r) - } else { - reqInfo.API = s3middleware.ListObjectsV1Operation - h.ListObjectsV1Handler(w, r) - } - } -} - func objectRouter(h Handler) chi.Router { objRouter := chi.NewRouter() diff --git a/api/router_filter.go b/api/router_filter.go index 7742ce54..06282bed 100644 --- a/api/router_filter.go +++ b/api/router_filter.go @@ -138,13 +138,14 @@ func (hf *HandlerFilters) ServeHTTP(w http.ResponseWriter, r *http.Request) { } func (hf *HandlerFilters) match(r *http.Request) http.Handler { + queries := r.URL.Query() + LOOP: for _, filter := range hf.filters { - if filter.noQueries && len(r.URL.Query()) > 0 { + if filter.noQueries && len(queries) > 0 { continue } if len(filter.allowedQueries) > 0 { - queries := r.URL.Query() for key := range queries { if _, ok := filter.allowedQueries[key]; !ok { continue LOOP @@ -158,8 +159,8 @@ LOOP: } } for _, query := range filter.queries { - queryVal := r.URL.Query().Get(query.Key) - if !r.URL.Query().Has(query.Key) || query.Value != "" && query.Value != queryVal { + queryVal := queries.Get(query.Key) + if !queries.Has(query.Key) || query.Value != "" && query.Value != queryVal { continue LOOP } } diff --git a/api/router_test.go b/api/router_test.go index 18fe3369..981b0244 100644 --- a/api/router_test.go +++ b/api/router_test.go @@ -903,6 +903,78 @@ func TestRouterListObjectsV2Domains(t *testing.T) { require.Equal(t, s3middleware.ListObjectsV2Operation, resp.Method) } +func TestRouterListingVHS(t *testing.T) { + baseDomain := "domain.com" + baseDomainWithBkt := "bucket.domain.com" + chiRouter := prepareRouter(t, enableVHSDomains(baseDomain)) + chiRouter.handler.buckets["bucket"] = &data.BucketInfo{} + + for _, tc := range []struct { + name string + host string + queries string + expectedOperation string + notSupported bool + }{ + { + name: "list-object-v1 without query params", + host: baseDomainWithBkt, + expectedOperation: s3middleware.ListObjectsV1Operation, + }, + { + name: "list-buckets without query params", + host: baseDomain, + expectedOperation: s3middleware.ListBucketsOperation, + }, + { + name: "list-objects-v1 with prefix param", + host: baseDomainWithBkt, + queries: func() string { + query := make(url.Values) + query.Set(s3middleware.QueryPrefix, "prefix") + return query.Encode() + }(), + expectedOperation: s3middleware.ListObjectsV1Operation, + }, + { + name: "list-buckets with prefix param", + host: baseDomain, + queries: func() string { + query := make(url.Values) + query.Set(s3middleware.QueryPrefix, "prefix") + return query.Encode() + }(), + expectedOperation: s3middleware.ListBucketsOperation, + }, + { + name: "not supported operation", + host: baseDomain, + queries: func() string { + query := make(url.Values) + query.Set("invalid", "invalid") + return query.Encode() + }(), + notSupported: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodGet, "/", nil) + r.URL.RawQuery = tc.queries + r.Host = tc.host + chiRouter.ServeHTTP(w, r) + + if tc.notSupported { + assertAPIError(t, w, apierr.ErrNotSupported) + return + } + + resp := readResponse(t, w) + require.Equal(t, tc.expectedOperation, resp.Method) + }) + } +} + func enableVHSDomains(domains ...string) option { return func(cfg *Config) { setting := cfg.MiddlewareSettings.(*middlewareSettingsMock) -- 2.45.2 From fbdd83e574b8b3ece9423f71fa5a5a004bce80bd Mon Sep 17 00:00:00 2001 From: Roman Loginov Date: Tue, 17 Dec 2024 12:27:55 +0300 Subject: [PATCH 3/4] [#586] Skip port when matching listen domains We may have a situation where the domain can be specified in the config without a port, and the host in the header will be with a port. As a result, the host will not match. Now the port is not taken into account when checking for a match. Signed-off-by: Roman Loginov --- api/middleware/address_style.go | 4 ++++ api/middleware/address_style_test.go | 7 +++++++ cmd/s3-gw/app_settings.go | 5 +++++ cmd/s3-gw/validate_test.go | 2 ++ internal/logs/logs.go | 1 + 5 files changed, 19 insertions(+) diff --git a/api/middleware/address_style.go b/api/middleware/address_style.go index dd5efb43..6e82ae68 100644 --- a/api/middleware/address_style.go +++ b/api/middleware/address_style.go @@ -122,6 +122,10 @@ func preparePathStyleAddress(reqInfo *ReqInfo, r *http.Request, reqLogger *zap.L } func checkDomain(host string, domains []string) (bktName string, match bool) { + if pos := strings.Index(host, ":"); pos != -1 { + host = host[:pos] + } + partsHost := strings.Split(host, ".") for _, pattern := range domains { partsPattern := strings.Split(pattern, ".") diff --git a/api/middleware/address_style_test.go b/api/middleware/address_style_test.go index 09e2d542..78924f9c 100644 --- a/api/middleware/address_style_test.go +++ b/api/middleware/address_style_test.go @@ -409,6 +409,13 @@ func TestCheckDomains(t *testing.T) { requestURL: "bktA.bktB.s3.kapusta.domain.com", expectedMatch: false, }, + { + name: "valid url with bktName and namespace (wildcard after protocol infix) with port", + domains: []string{"s3..domain.com"}, + requestURL: "bktA.s3.kapusta.domain.com:8884", + expectedBktName: "bktA", + expectedMatch: true, + }, } { t.Run(tc.name, func(t *testing.T) { bktName, match := checkDomain(tc.requestURL, tc.domains) diff --git a/cmd/s3-gw/app_settings.go b/cmd/s3-gw/app_settings.go index 691e2319..abe5f3f8 100644 --- a/cmd/s3-gw/app_settings.go +++ b/cmd/s3-gw/app_settings.go @@ -1210,6 +1210,11 @@ func validateDomains(domains []string, log *zap.Logger) []string { validDomains := make([]string, 0, len(domains)) LOOP: for _, domain := range domains { + if strings.Contains(domain, ":") { + log.Warn(logs.WarnDomainContainsPort, zap.String("domain", domain)) + continue + } + domainParts := strings.Split(domain, ".") for _, part := range domainParts { if strings.ContainsAny(part, "<>") && part != wildcardPlaceholder { diff --git a/cmd/s3-gw/validate_test.go b/cmd/s3-gw/validate_test.go index fe88228e..95ef30dc 100644 --- a/cmd/s3-gw/validate_test.go +++ b/cmd/s3-gw/validate_test.go @@ -21,6 +21,8 @@ func TestValidateDomains(t *testing.T) { "s3dev.fro.dev..frostfs.devenv", ".dev.ard>.frostfs.devenv", + "s3dev.frostfs.devenv:8888", + ".frostfs.devenv:443", } expectedDomains := []string{ "s3dev.frostfs.devenv", diff --git a/internal/logs/logs.go b/internal/logs/logs.go index f8047a2d..17e78bb2 100644 --- a/internal/logs/logs.go +++ b/internal/logs/logs.go @@ -178,4 +178,5 @@ const ( MultinetDialSuccess = "multinet dial successful" MultinetDialFail = "multinet dial failed" FailedToParseHTTPTime = "failed to parse http time, header is ignored" + WarnDomainContainsPort = "the domain contains a port, domain skipped" ) -- 2.45.2 From e721ac82f720240b54d82277c05c6e4f4e216151 Mon Sep 17 00:00:00 2001 From: Roman Loginov Date: Tue, 17 Dec 2024 12:29:30 +0300 Subject: [PATCH 4/4] Release v0.31.3 Signed-off-by: Roman Loginov --- CHANGELOG.md | 10 +++++++++- VERSION | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2740b79..20ad952f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ This document outlines major changes between releases. ## [Unreleased] +## [0.31.3] - 2024-12-17 + +### Fixed +- Return BucketAlreadyExists when global domain taken (#584) +- Fix list-buckets vhs routing (#583) +- Skip port when matching listen domains (#586) + ## [0.31.2] - 2024-12-13 ### Fixed @@ -361,4 +368,5 @@ To see CHANGELOG for older versions, refer to https://github.com/nspcc-dev/neofs [0.31.0]: https://git.frostfs.info/TrueCloudLab/frostfs-s3-gw/compare/v0.30.9...v0.31.0 [0.31.1]: https://git.frostfs.info/TrueCloudLab/frostfs-s3-gw/compare/v0.31.0...v0.31.1 [0.31.2]: https://git.frostfs.info/TrueCloudLab/frostfs-s3-gw/compare/v0.31.1...v0.31.2 -[Unreleased]: https://git.frostfs.info/TrueCloudLab/frostfs-s3-gw/compare/v0.31.2...master \ No newline at end of file +[0.31.3]: https://git.frostfs.info/TrueCloudLab/frostfs-s3-gw/compare/v0.31.2...v0.31.3 +[Unreleased]: https://git.frostfs.info/TrueCloudLab/frostfs-s3-gw/compare/v0.31.3...master \ No newline at end of file diff --git a/VERSION b/VERSION index c11797e1..d5f263f4 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -v0.31.2 +v0.31.3 -- 2.45.2