From ff1ec56d24f4402f84e99d86fc229b2220675dc5 Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Thu, 23 Nov 2023 10:44:50 +0300 Subject: [PATCH] [#260] Use namespace as domain when create bucket Signed-off-by: Denis Kirillov --- api/cache/buckets.go | 15 +++++++++------ api/cache/cache_test.go | 6 +++--- api/handler/put.go | 3 ++- api/handler/put_test.go | 20 ++++++++++++++++++++ api/layer/cache.go | 10 +++++----- api/layer/container.go | 16 ++++++++++++---- api/layer/frostfs.go | 3 +++ api/layer/frostfs_mock.go | 10 ++++++++++ api/layer/layer.go | 6 +++--- api/resolver/resolver.go | 19 ++++++++----------- cmd/s3-gw/app.go | 15 +++++++++++---- internal/frostfs/frostfs.go | 1 + 12 files changed, 87 insertions(+), 37 deletions(-) diff --git a/api/cache/buckets.go b/api/cache/buckets.go index 0a90ae3aa..a8a6af83d 100644 --- a/api/cache/buckets.go +++ b/api/cache/buckets.go @@ -40,8 +40,7 @@ func NewBucketCache(config *Config) *BucketCache { // Get returns a cached object. func (o *BucketCache) Get(ns, bktName string) *data.BucketInfo { - key := ns + "/" + bktName - entry, err := o.cache.Get(key) + entry, err := o.cache.Get(formKey(ns, bktName)) if err != nil { return nil } @@ -57,11 +56,15 @@ func (o *BucketCache) Get(ns, bktName string) *data.BucketInfo { } // Put puts an object to cache. -func (o *BucketCache) Put(ns string, bkt *data.BucketInfo) error { - return o.cache.Set(ns+"/"+bkt.Name, bkt) +func (o *BucketCache) Put(bkt *data.BucketInfo) error { + return o.cache.Set(formKey(bkt.Zone, bkt.Name), bkt) } // Delete deletes an object from cache. -func (o *BucketCache) Delete(ns, bktName string) bool { - return o.cache.Remove(ns + "/" + bktName) +func (o *BucketCache) Delete(bkt *data.BucketInfo) bool { + return o.cache.Remove(formKey(bkt.Zone, bkt.Name)) +} + +func formKey(ns, name string) string { + return name + "." + ns } diff --git a/api/cache/cache_test.go b/api/cache/cache_test.go index e8374a979..1d89a36a5 100644 --- a/api/cache/cache_test.go +++ b/api/cache/cache_test.go @@ -36,15 +36,15 @@ func TestBucketsCacheType(t *testing.T) { bktInfo := &data.BucketInfo{Name: "bucket"} - err := cache.Put("", bktInfo) + err := cache.Put(bktInfo) require.NoError(t, err) val := cache.Get("", bktInfo.Name) require.Equal(t, bktInfo, val) require.Equal(t, 0, observedLog.Len()) - err = cache.cache.Set("/"+bktInfo.Name, "tmp") + err = cache.cache.Set(bktInfo.Name+"."+bktInfo.Zone, "tmp") require.NoError(t, err) - assertInvalidCacheEntry(t, cache.Get("", bktInfo.Name), observedLog) + assertInvalidCacheEntry(t, cache.Get(bktInfo.Zone, bktInfo.Name), observedLog) } func TestObjectNamesCacheType(t *testing.T) { diff --git a/api/handler/put.go b/api/handler/put.go index 3158a8785..14e27a79c 100644 --- a/api/handler/put.go +++ b/api/handler/put.go @@ -748,7 +748,8 @@ func (h *handler) CreateBucketHandler(w http.ResponseWriter, r *http.Request) { ctx := r.Context() reqInfo := middleware.GetReqInfo(ctx) p := &layer.CreateBucketParams{ - Name: reqInfo.BucketName, + Name: reqInfo.BucketName, + Namespace: reqInfo.Namespace, } if err := checkBucketName(reqInfo.BucketName); err != nil { diff --git a/api/handler/put_test.go b/api/handler/put_test.go index 345242e6f..d519de5e3 100644 --- a/api/handler/put_test.go +++ b/api/handler/put_test.go @@ -380,6 +380,26 @@ func TestCreateBucket(t *testing.T) { createBucketAssertS3Error(hc, bktName, box2, s3errors.ErrBucketAlreadyExists) } +func TestCreateNamespacedBucket(t *testing.T) { + hc := prepareHandlerContext(t) + bktName := "bkt-name" + namespace := "yabloko" + + box, _ := createAccessBox(t) + w, r := prepareTestRequest(hc, bktName, "", nil) + ctx := middleware.SetBoxData(r.Context(), box) + reqInfo := middleware.GetReqInfo(ctx) + reqInfo.Namespace = namespace + r = r.WithContext(middleware.SetReqInfo(ctx, reqInfo)) + hc.Handler().CreateBucketHandler(w, r) + assertStatus(t, w, http.StatusOK) + + bktInfo, err := hc.Layer().GetBucketInfo(middleware.SetReqInfo(hc.Context(), reqInfo), bktName) + require.NoError(t, err) + + require.Equal(t, namespace+".ns", bktInfo.Zone) +} + func TestPutObjectClientCut(t *testing.T) { hc := prepareHandlerContext(t) bktName, objName1, objName2 := "bkt-name", "obj-name1", "obj-name2" diff --git a/api/layer/cache.go b/api/layer/cache.go index 44ad1abb4..87381b335 100644 --- a/api/layer/cache.go +++ b/api/layer/cache.go @@ -60,18 +60,18 @@ func (c *Cache) GetBucket(ns, name string) *data.BucketInfo { return c.bucketCache.Get(ns, name) } -func (c *Cache) PutBucket(ns string, bktInfo *data.BucketInfo) { - if err := c.bucketCache.Put(ns, bktInfo); err != nil { +func (c *Cache) PutBucket(bktInfo *data.BucketInfo) { + if err := c.bucketCache.Put(bktInfo); err != nil { c.logger.Warn(logs.CouldntPutBucketInfoIntoCache, - zap.String("namespace", ns), + zap.String("zone", bktInfo.Zone), zap.String("bucket name", bktInfo.Name), zap.Stringer("bucket cid", bktInfo.CID), zap.Error(err)) } } -func (c *Cache) DeleteBucket(ns, name string) { - c.bucketCache.Delete(ns, name) +func (c *Cache) DeleteBucket(bktInfo *data.BucketInfo) { + c.bucketCache.Delete(bktInfo) } func (c *Cache) CleanListCacheEntriesContainingObject(objectName string, cnrID cid.ID) { diff --git a/api/layer/container.go b/api/layer/container.go index 3ca0a816a..e5085a310 100644 --- a/api/layer/container.go +++ b/api/layer/container.go @@ -5,7 +5,6 @@ import ( "fmt" "strconv" - v2container "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/container" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/data" s3errors "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors" @@ -75,7 +74,12 @@ func (n *layer) containerInfo(ctx context.Context, idCnr cid.ID) (*data.BucketIn } } - n.cache.PutBucket(reqInfo.Namespace, info) + zone, _ := n.features.FormContainerZone(reqInfo.Namespace) + if zone != info.Zone { + return nil, fmt.Errorf("ns '%s' and zone '%s' are mismatched", zone, info.Zone) + } + + n.cache.PutBucket(info) return info, nil } @@ -105,9 +109,12 @@ func (n *layer) createContainer(ctx context.Context, p *CreateBucketParams) (*da if p.LocationConstraint == "" { p.LocationConstraint = api.DefaultLocationConstraint // s3tests_boto3.functional.test_s3:test_bucket_get_location } + + zone, _ := n.features.FormContainerZone(p.Namespace) + bktInfo := &data.BucketInfo{ Name: p.Name, - Zone: v2container.SysAttributeZoneDefault, + Zone: zone, Owner: n.BearerOwner(ctx), Created: TimeNow(ctx), LocationConstraint: p.LocationConstraint, @@ -130,6 +137,7 @@ func (n *layer) createContainer(ctx context.Context, p *CreateBucketParams) (*da Creator: bktInfo.Owner, Policy: p.Policy, Name: p.Name, + Zone: zone, SessionToken: p.SessionContainerCreation, CreationTime: bktInfo.Created, AdditionalAttributes: attributes, @@ -145,7 +153,7 @@ func (n *layer) createContainer(ctx context.Context, p *CreateBucketParams) (*da return nil, fmt.Errorf("set container eacl: %w", err) } - n.cache.PutBucket(bktInfo.Zone, bktInfo) + n.cache.PutBucket(bktInfo) return bktInfo, nil } diff --git a/api/layer/frostfs.go b/api/layer/frostfs.go index 0f3388f94..b1120dc96 100644 --- a/api/layer/frostfs.go +++ b/api/layer/frostfs.go @@ -30,6 +30,9 @@ type PrmContainerCreate struct { // Name for the container. Name string + // Zone for container registration. + Zone string + // CreationTime value for Timestamp attribute CreationTime time.Time diff --git a/api/layer/frostfs_mock.go b/api/layer/frostfs_mock.go index 14b17942f..58db92f57 100644 --- a/api/layer/frostfs_mock.go +++ b/api/layer/frostfs_mock.go @@ -10,6 +10,7 @@ import ( "io" "time" + v2container "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/container" objectv2 "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/object" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer" @@ -50,6 +51,14 @@ func (k *FeatureSettingsMock) SetMD5Enabled(md5Enabled bool) { k.md5Enabled = md5Enabled } +func (k *FeatureSettingsMock) FormContainerZone(ns string) (zone string, isDefault bool) { + if ns == "" { + return v2container.SysAttributeZoneDefault, true + } + + return ns + ".ns", false +} + type TestFrostFS struct { FrostFS @@ -143,6 +152,7 @@ func (t *TestFrostFS) CreateContainer(_ context.Context, prm PrmContainerCreate) if prm.Name != "" { var d container.Domain d.SetName(prm.Name) + d.SetZone(prm.Zone) container.WriteDomain(&cnr, d) container.SetName(&cnr, prm.Name) diff --git a/api/layer/layer.go b/api/layer/layer.go index 79af9f074..d79dca940 100644 --- a/api/layer/layer.go +++ b/api/layer/layer.go @@ -51,6 +51,7 @@ type ( ClientCut() bool BufferMaxSizeForPut() uint64 MD5Enabled() bool + FormContainerZone(ns string) (zone string, isDefault bool) } layer struct { @@ -172,6 +173,7 @@ type ( // CreateBucketParams stores bucket create request parameters. CreateBucketParams struct { Name string + Namespace string Policy netmap.PlacementPolicy EACL *eacl.Table SessionContainerCreation *session.Container @@ -816,8 +818,6 @@ func (n *layer) DeleteBucket(ctx context.Context, p *DeleteBucketParams) error { return errors.GetAPIError(errors.ErrBucketNotEmpty) } - reqInfo := middleware.GetReqInfo(ctx) - - n.cache.DeleteBucket(reqInfo.Namespace, p.BktInfo.Name) + n.cache.DeleteBucket(p.BktInfo) return n.frostFS.DeleteContainer(ctx, p.BktInfo.CID, p.SessionToken) } diff --git a/api/resolver/resolver.go b/api/resolver/resolver.go index fde29b9e2..c394be714 100644 --- a/api/resolver/resolver.go +++ b/api/resolver/resolver.go @@ -10,7 +10,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container" cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/ns" - "golang.org/x/exp/slices" ) const ( @@ -18,8 +17,6 @@ const ( DNSResolver = "dns" ) -const nsDomain = ".ns" - // ErrNoResolvers returns when trying to resolve container without any resolver. var ErrNoResolvers = errors.New("no resolvers") @@ -33,7 +30,7 @@ type FrostFS interface { } type Settings interface { - DefaultNamespaces() []string + FormContainerZone(ns string) (zone string, isDefault bool) } type Config struct { @@ -176,15 +173,16 @@ func NewDNSResolver(frostFS FrostFS, settings Settings) (*Resolver, error) { resolveFunc := func(ctx context.Context, name string) (cid.ID, error) { var err error reqInfo := middleware.GetReqInfo(ctx) - domain := reqInfo.Namespace + nsDomain - if slices.Contains(settings.DefaultNamespaces(), domain) { - domain, err = frostFS.SystemDNS(ctx) + + zone, isDefault := settings.FormContainerZone(reqInfo.Namespace) + if isDefault { + zone, err = frostFS.SystemDNS(ctx) if err != nil { return cid.ID{}, fmt.Errorf("read system DNS parameter of the FrostFS: %w", err) } } - domain = name + "." + domain + domain := name + "." + zone cnrID, err := dns.ResolveContainerName(domain) if err != nil { return cid.ID{}, fmt.Errorf("couldn't resolve container '%s' as '%s': %w", name, domain, err) @@ -217,9 +215,8 @@ func NewNNSResolver(address string, settings Settings) (*Resolver, error) { d.SetName(name) reqInfo := middleware.GetReqInfo(ctx) - if !slices.Contains(settings.DefaultNamespaces(), reqInfo.Namespace) { - d.SetZone(reqInfo.Namespace + nsDomain) - } + zone, _ := settings.FormContainerZone(reqInfo.Namespace) + d.SetZone(zone) cnrID, err := nns.ResolveContainerDomain(d) if err != nil { diff --git a/cmd/s3-gw/app.go b/cmd/s3-gw/app.go index e38914fc9..cdf649b2d 100644 --- a/cmd/s3-gw/app.go +++ b/cmd/s3-gw/app.go @@ -15,6 +15,7 @@ import ( "syscall" "time" + v2container "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/container" "git.frostfs.info/TrueCloudLab/frostfs-observability/tracing" grpctracing "git.frostfs.info/TrueCloudLab/frostfs-observability/tracing/grpc" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api" @@ -41,6 +42,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/spf13/viper" "go.uber.org/zap" + "golang.org/x/exp/slices" "google.golang.org/grpc" ) @@ -341,14 +343,19 @@ func (s *appSettings) setNamespaceHeader(nsHeader string) { s.mu.Unlock() } -func (s *appSettings) DefaultNamespaces() []string { +func (s *appSettings) FormContainerZone(ns string) (zone string, isDefault bool) { s.mu.RLock() - defer s.mu.RUnlock() - return s.defaultNamespaces + namespaces := s.defaultNamespaces + s.mu.RUnlock() + if slices.Contains(namespaces, ns) { + return v2container.SysAttributeZoneDefault, true + } + + return ns + ".ns", false } func (s *appSettings) setDefaultNamespaces(namespaces []string) { - for i := range namespaces { // to be set namespaces in evn variable as `S3_GW_KLUDGE_DEFAULT_NAMESPACES="" "root"` + for i := range namespaces { // to be set namespaces in env variable as `S3_GW_KLUDGE_DEFAULT_NAMESPACES="" "root"` namespaces[i] = strings.Trim(namespaces[i], "\"") } diff --git a/internal/frostfs/frostfs.go b/internal/frostfs/frostfs.go index d410e7b27..ab7a1e70f 100644 --- a/internal/frostfs/frostfs.go +++ b/internal/frostfs/frostfs.go @@ -126,6 +126,7 @@ func (x *FrostFS) CreateContainer(ctx context.Context, prm layer.PrmContainerCre if prm.Name != "" { var d container.Domain d.SetName(prm.Name) + d.SetZone(prm.Zone) container.WriteDomain(&cnr, d) container.SetName(&cnr, prm.Name)