bugfix/583-fix_list_buckets_vhs_and_fix_bucket_name_conflicts_error_support_v31.0 #584

14 changed files with 167 additions and 37 deletions

View file

@ -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
[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

View file

@ -1 +1 @@
v0.31.2
v0.31.3

View file

@ -1822,6 +1822,10 @@ func TransformToS3Error(err error) error {
return GetAPIError(ErrGatewayTimeout)
}
if errors.Is(err, frostfs.ErrGlobalDomainIsAlreadyTaken) {
return GetAPIError(ErrBucketAlreadyExists)
}
return GetAPIError(ErrInternalError)
}

View file

@ -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.

View file

@ -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, ".")

View file

@ -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.<wildcard>.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)

View file

@ -30,6 +30,9 @@ const (
QueryMaxKeys = "max-keys"
QueryMarker = "marker"
QueryEncodingType = "encoding-type"
QueryMaxBuckets = "max-buckets"
QueryContinuationToken = "continuation-token"
QueryBucketRegion = "bucket-region"
amzTagging = "x-amz-tagging"
unmatchedBucketOperation = "UnmatchedBucketOperation"

View file

@ -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()

View file

@ -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
}
}

View file

@ -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)

View file

@ -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 {

View file

@ -21,6 +21,8 @@ func TestValidateDomains(t *testing.T) {
"s3dev.fro<stfs.devenv",
"<wildcard>.dev.<wildcard>.frostfs.devenv",
"<wildcard>.dev.<wildc>ard>.frostfs.devenv",
"s3dev.frostfs.devenv:8888",
"<wildcard>.frostfs.devenv:443",
}
expectedDomains := []string{
"s3dev.frostfs.devenv",

View file

@ -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)
}

View file

@ -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"
)