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 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/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/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) 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/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) } 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" )