[#372] Drop kludge.acl_enabled flag

Now only APE container can be created using s3-gw

Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>

api/handler/acl_test.go

@@ -1301,21 +1301,6 @@ func TestBucketAclToAst(t *testing.T) {
 	require.Equal(t, expectedAst, actualAst)
 }
 
-func TestPutBucketACL(t *testing.T) {
-	tc := prepareHandlerContext(t)
-	tc.config.aclEnabled = true
-	bktName := "bucket-for-acl"
-
-	info := createBucket(tc, bktName)
-
-	header := map[string]string{api.AmzACL: "public-read"}
-	putBucketACL(tc, bktName, info.Box, header)
-
-	header = map[string]string{api.AmzACL: "private"}
-	putBucketACL(tc, bktName, info.Box, header)
-	checkLastRecords(t, tc, info.BktInfo, eacl.ActionDeny)
-}
-
 func TestPutBucketAPE(t *testing.T) {
 	hc := prepareHandlerContext(t)
 	bktName := "bucket-for-acl-ape"
@@ -1361,27 +1346,6 @@ func TestCreateObjectACLErrorAPE(t *testing.T) {
 	createMultipartUpload(hc, bktName, objName, map[string]string{api.AmzACL: basicACLPrivate})
 }
 
-func TestPutObjectACLBackwardCompatibility(t *testing.T) {
-	hc := prepareHandlerContext(t)
-	hc.config.aclEnabled = true
-	bktName, objName := "bucket-for-acl-ape", "object"
-
-	info := createBucket(hc, bktName)
-
-	putObjectWithHeadersBase(hc, bktName, objName, map[string]string{api.AmzACL: basicACLPrivate}, info.Box, nil)
-	putObjectWithHeadersBase(hc, bktName, objName, map[string]string{api.AmzACL: basicACLPublic}, info.Box, nil)
-
-	aclRes := getObjectACL(hc, bktName, objName)
-	require.Len(t, aclRes.AccessControlList, 2)
-	require.Equal(t, hex.EncodeToString(info.Key.PublicKey().Bytes()), aclRes.AccessControlList[0].Grantee.ID)
-	require.Equal(t, aclFullControl, aclRes.AccessControlList[0].Permission)
-	require.Equal(t, allUsersGroup, aclRes.AccessControlList[1].Grantee.URI)
-	require.Equal(t, aclFullControl, aclRes.AccessControlList[1].Permission)
-
-	aclBody := &AccessControlPolicy{}
-	putObjectACLBase(hc, bktName, objName, info.Box, nil, aclBody)
-}
-
 func TestBucketACLAPE(t *testing.T) {
 	hc := prepareHandlerContext(t)
 	bktName := "bucket-for-acl-ape"
@@ -1648,23 +1612,6 @@ func putBucketPolicy(hc *handlerContext, bktName string, bktPolicy engineiam.Pol
 	}
 }
 
-func checkLastRecords(t *testing.T, tc *handlerContext, bktInfo *data.BucketInfo, action eacl.Action) {
-	bktACL, err := tc.Layer().GetBucketACL(tc.Context(), bktInfo)
-	require.NoError(t, err)
-
-	length := len(bktACL.EACL.Records())
-
-	if length < 7 {
-		t.Fatalf("length of records is less than 7: '%d'", length)
-	}
-
-	for _, rec := range bktACL.EACL.Records()[length-7:] {
-		if rec.Action() != action || rec.Targets()[0].Role() != eacl.RoleOthers {
-			t.Fatalf("inavid last record: '%s', '%s', '%s',", rec.Action(), rec.Operation(), rec.Targets()[0].Role())
-		}
-	}
-}
-
 func createAccessBox(t *testing.T) (*accessbox.Box, *keys.PrivateKey) {
 	key, err := keys.NewPrivateKey()
 	require.NoError(t, err)

api/handler/api.go

@@ -38,7 +38,6 @@ type (
 		IsResolveListAllow() bool
 		BypassContentEncodingInChunks() bool
 		MD5Enabled() bool
-		ACLEnabled() bool
 		RetryMaxAttempts() int
 		RetryMaxBackoff() time.Duration
 		RetryStrategy() RetryStrategy

api/handler/handlers_test.go

@@ -72,7 +72,6 @@ type configMock struct {
 	defaultCopiesNumbers          []uint32
 	bypassContentEncodingInChunks bool
 	md5Enabled                    bool
-	aclEnabled                    bool
 }
 
 func (c *configMock) DefaultPlacementPolicy(_ string) netmap.PlacementPolicy {
@@ -120,10 +119,6 @@ func (c *configMock) MD5Enabled() bool {
 	return c.md5Enabled
 }
 
-func (c *configMock) ACLEnabled() bool {
-	return c.aclEnabled
-}
-
 func (c *configMock) ResolveNamespaceAlias(ns string) string {
 	return ns
 }

api/handler/put.go

@@ -814,11 +814,6 @@ func parseCannedACL(header http.Header) (string, error) {
 }
 
 func (h *handler) CreateBucketHandler(w http.ResponseWriter, r *http.Request) {
-	if h.cfg.ACLEnabled() {
-		h.createBucketHandlerACL(w, r)
-		return
-	}
-
 	h.createBucketHandlerPolicy(w, r)
 }
 
@@ -941,78 +936,6 @@ func (h *handler) putBucketSettingsRetryer() aws.RetryerV2 {
 	})
 }
 
-func (h *handler) createBucketHandlerACL(w http.ResponseWriter, r *http.Request) {
-	ctx := r.Context()
-	reqInfo := middleware.GetReqInfo(ctx)
-
-	boxData, err := middleware.GetBoxData(ctx)
-	if err != nil {
-		h.logAndSendError(w, "get access box from request", reqInfo, err)
-		return
-	}
-
-	key, p, err := h.parseCommonCreateBucketParams(reqInfo, boxData, r)
-	if err != nil {
-		h.logAndSendError(w, "parse create bucket params", reqInfo, err)
-		return
-	}
-
-	aclPrm := &layer.PutBucketACLParams{SessionToken: boxData.Gate.SessionTokenForSetEACL()}
-	if aclPrm.SessionToken == nil {
-		h.logAndSendError(w, "couldn't find session token for setEACL", reqInfo, errors.GetAPIError(errors.ErrAccessDenied))
-		return
-	}
-
-	bktACL, err := parseACLHeaders(r.Header, key)
-	if err != nil {
-		h.logAndSendError(w, "could not parse bucket acl", reqInfo, err)
-		return
-	}
-	resInfo := &resourceInfo{Bucket: reqInfo.BucketName}
-
-	aclPrm.EACL, err = bucketACLToTable(bktACL, resInfo)
-	if err != nil {
-		h.logAndSendError(w, "could translate bucket acl to eacl", reqInfo, err)
-		return
-	}
-
-	bktInfo, err := h.obj.CreateBucket(ctx, p)
-	if err != nil {
-		h.logAndSendError(w, "could not create bucket", reqInfo, err)
-		return
-	}
-	h.reqLogger(ctx).Info(logs.BucketIsCreated, zap.Stringer("container_id", bktInfo.CID))
-
-	aclPrm.BktInfo = bktInfo
-	if err = h.obj.PutBucketACL(r.Context(), aclPrm); err != nil {
-		h.logAndSendError(w, "could not put bucket e/ACL", reqInfo, err)
-		return
-	}
-
-	sp := &layer.PutSettingsParams{
-		BktInfo: bktInfo,
-		Settings: &data.BucketSettings{
-			OwnerKey:   key,
-			Versioning: data.VersioningUnversioned,
-		},
-	}
-
-	if p.ObjectLockEnabled {
-		sp.Settings.Versioning = data.VersioningEnabled
-	}
-
-	if err = h.obj.PutBucketSettings(ctx, sp); err != nil {
-		h.logAndSendError(w, "couldn't save bucket settings", reqInfo, err,
-			zap.String("container_id", bktInfo.CID.EncodeToString()))
-		return
-	}
-
-	if err = middleware.WriteSuccessResponseHeadersOnly(w); err != nil {
-		h.logAndSendError(w, "write response", reqInfo, err)
-		return
-	}
-}
-
 const s3ActionPrefix = "s3:"
 
 var (

api/handler/put_test.go

@@ -381,21 +381,6 @@ func TestCreateBucket(t *testing.T) {
 	createBucketAssertS3Error(hc, bktName, box2, s3errors.ErrBucketAlreadyExists)
 }
 
-func TestCreateOldBucketPutVersioning(t *testing.T) {
-	hc := prepareHandlerContext(t)
-	hc.config.aclEnabled = true
-	bktName := "bkt-name"
-
-	info := createBucket(hc, bktName)
-	settings, err := hc.tree.GetSettingsNode(hc.Context(), info.BktInfo)
-	require.NoError(t, err)
-	settings.OwnerKey = nil
-	err = hc.tree.PutSettingsNode(hc.Context(), info.BktInfo, settings)
-	require.NoError(t, err)
-
-	putBucketVersioning(t, hc, bktName, true)
-}
-
 func TestCreateNamespacedBucket(t *testing.T) {
 	hc := prepareHandlerContext(t)
 	bktName := "bkt-name"

api/middleware/policy.go

@@ -51,7 +51,6 @@ var withoutResourceOps = []string{
 
 type PolicySettings interface {
 	PolicyDenyByDefault() bool
-	ACLEnabled() bool
 }
 
 type FrostFSIDInformer interface {
@@ -150,7 +149,7 @@ func policyCheck(r *http.Request, cfg PolicyConfig) error {
 		return apiErr.GetAPIErrorWithError(apiErr.ErrAccessDenied, fmt.Errorf("policy check: %s", st.String()))
 	}
 
-	isAPE := !cfg.Settings.ACLEnabled()
+	isAPE := true
 	if bktInfo != nil {
 		isAPE = bktInfo.APEEnabled
 	}

api/router_mock_test.go

@@ -72,7 +72,6 @@ func (c *centerMock) Authenticate(*http.Request) (*middleware.Box, error) {
 
 type middlewareSettingsMock struct {
 	denyByDefault  bool
-	aclEnabled     bool
 	sourceIPHeader string
 }
 
@@ -92,10 +91,6 @@ func (r *middlewareSettingsMock) PolicyDenyByDefault() bool {
 	return r.denyByDefault
 }
 
-func (r *middlewareSettingsMock) ACLEnabled() bool {
-	return r.aclEnabled
-}
-
 type frostFSIDMock struct {
 	tags            map[string]string
 	validateError   bool
@@ -430,7 +425,7 @@ func (h *handlerMock) CreateBucketHandler(w http.ResponseWriter, r *http.Request
 
 	h.buckets[reqInfo.Namespace+reqInfo.BucketName] = &data.BucketInfo{
 		Name:       reqInfo.BucketName,
-		APEEnabled: !h.cfg.ACLEnabled(),
+		APEEnabled: true,
 	}
 
 	res := &handlerResult{

api/router_test.go

@@ -17,7 +17,9 @@ import (
 	apiErrors "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors"
 	s3middleware "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware"
 	"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/metrics"
+	cidtest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id/test"
 	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object"
+	usertest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user/test"
 	engineiam "git.frostfs.info/TrueCloudLab/policy-engine/iam"
 	"git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
 	"git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine"
@@ -37,6 +39,7 @@ type routerMock struct {
 	cfg                Config
 	middlewareSettings *middlewareSettingsMock
 	policyChecker      engine.LocalOverrideEngine
+	handler            *handlerMock
 }
 
 func (m *routerMock) ServeHTTP(w http.ResponseWriter, r *http.Request) {
@@ -64,12 +67,14 @@ func prepareRouter(t *testing.T, opts ...option) *routerMock {
 		Enabled:        true,
 	}
 
+	handlerTestMock := &handlerMock{t: t, cfg: middlewareSettings, buckets: map[string]*data.BucketInfo{}}
+
 	cfg := Config{
 		Throttle: middleware.ThrottleOpts{
 			Limit:          10,
 			BacklogTimeout: 30 * time.Second,
 		},
-		Handler:            &handlerMock{t: t, cfg: middlewareSettings, buckets: map[string]*data.BucketInfo{}},
+		Handler:            handlerTestMock,
 		Center:             &centerMock{t: t},
 		Log:                logger,
 		Metrics:            metrics.NewAppMetrics(metricsConfig),
@@ -91,6 +96,7 @@ func prepareRouter(t *testing.T, opts ...option) *routerMock {
 		cfg:                cfg,
 		middlewareSettings: middlewareSettings,
 		policyChecker:      policyChecker,
+		handler:            handlerTestMock,
 	}
 }
 
@@ -308,7 +314,6 @@ func TestACLAPE(t *testing.T) {
 		createOldBucket(router, bktNameOld)
 		createNewBucket(router, bktNameNew)
 
-		router.middlewareSettings.aclEnabled = false
 		router.middlewareSettings.denyByDefault = true
 
 		// Allow because of using old bucket
@@ -334,7 +339,6 @@ func TestACLAPE(t *testing.T) {
 		createOldBucket(router, bktNameOld)
 		createNewBucket(router, bktNameNew)
 
-		router.middlewareSettings.aclEnabled = false
 		router.middlewareSettings.denyByDefault = false
 
 		// Allow because of using old bucket
@@ -351,48 +355,6 @@ func TestACLAPE(t *testing.T) {
 		createBucketErr(router, ns, bktName, nil, apiErrors.ErrAccessDenied)
 		listBucketsErr(router, ns, apiErrors.ErrAccessDenied)
 	})
-
-	t.Run("acl enabled, ape deny by default", func(t *testing.T) {
-		router := prepareRouter(t)
-
-		ns, bktName, objName := "", "bucket", "object"
-		bktNameOld, bktNameNew := "old-bucket", "new-bucket"
-		createOldBucket(router, bktNameOld)
-		createNewBucket(router, bktNameNew)
-
-		router.middlewareSettings.aclEnabled = true
-		router.middlewareSettings.denyByDefault = true
-
-		// Allow because of using old bucket
-		putObject(router, ns, bktNameOld, objName, nil)
-		// Deny because of deny by default
-		putObjectErr(router, ns, bktNameNew, objName, nil, apiErrors.ErrAccessDenied)
-
-		// Allow because of old behavior
-		createBucket(router, ns, bktName)
-		listBuckets(router, ns)
-	})
-
-	t.Run("acl enabled, ape allow by default", func(t *testing.T) {
-		router := prepareRouter(t)
-
-		ns, bktName, objName := "", "bucket", "object"
-		bktNameOld, bktNameNew := "old-bucket", "new-bucket"
-		createOldBucket(router, bktNameOld)
-		createNewBucket(router, bktNameNew)
-
-		router.middlewareSettings.aclEnabled = true
-		router.middlewareSettings.denyByDefault = false
-
-		// Allow because of using old bucket
-		putObject(router, ns, bktNameOld, objName, nil)
-		// Allow because of allow by default
-		putObject(router, ns, bktNameNew, objName, nil)
-
-		// Allow because of old behavior
-		createBucket(router, ns, bktName)
-		listBuckets(router, ns)
-	})
 }
 
 func TestRequestParametersCheck(t *testing.T) {
@@ -726,10 +688,17 @@ func createNewBucket(router *routerMock, bktName string) {
 }
 
 func createSpecificBucket(router *routerMock, bktName string, old bool) {
-	aclEnabled := router.middlewareSettings.ACLEnabled()
+	router.handler.buckets[bktName] = &data.BucketInfo{
-	router.middlewareSettings.aclEnabled = old
+		Name:                    bktName,
-	createBucket(router, "", bktName)
+		Zone:                    "container",
-	router.middlewareSettings.aclEnabled = aclEnabled
+		CID:                     cidtest.ID(),
+		Owner:                   usertest.ID(),
+		Created:                 time.Now(),
+		LocationConstraint:      "default",
+		ObjectLockEnabled:       false,
+		HomomorphicHashDisabled: false,
+		APEEnabled:              !old,
+	}
 }
 
 func createBucket(router *routerMock, namespace, bktName string) {

cmd/s3-gw/app.go

@@ -97,7 +97,6 @@ type (
 		clientCut                     bool
 		maxBufferSizeForPut           uint64
 		md5Enabled                    bool
-		aclEnabled                    bool
 		namespaceHeader               string
 		defaultNamespaces             []string
 		policyDenyByDefault           bool
@@ -210,7 +209,6 @@ func newAppSettings(log *Logger, v *viper.Viper) *appSettings {
 func (s *appSettings) update(v *viper.Viper, log *zap.Logger) {
 	s.updateNamespacesSettings(v, log)
 	s.useDefaultXMLNamespace(v.GetBool(cfgKludgeUseDefaultXMLNS))
-	s.setACLEnabled(v.GetBool(cfgKludgeACLEnabled))
 	s.setBypassContentEncodingInChunks(v.GetBool(cfgKludgeBypassContentEncodingCheckInChunks))
 	s.setClientCut(v.GetBool(cfgClientCut))
 	s.setBufferMaxSizeForPut(v.GetUint64(cfgBufferMaxSizeForPut))
@@ -347,18 +345,6 @@ func (s *appSettings) setMD5Enabled(md5Enabled bool) {
 	s.mu.Unlock()
 }
 
-func (s *appSettings) setACLEnabled(enableACL bool) {
-	s.mu.Lock()
-	s.aclEnabled = enableACL
-	s.mu.Unlock()
-}
-
-func (s *appSettings) ACLEnabled() bool {
-	s.mu.RLock()
-	defer s.mu.RUnlock()
-	return s.aclEnabled
-}
-
 func (s *appSettings) NamespaceHeader() string {
 	s.mu.RLock()
 	defer s.mu.RUnlock()

cmd/s3-gw/app_settings.go

@@ -160,8 +160,6 @@ const ( // Settings.
 	cfgKludgeUseDefaultXMLNS                    = "kludge.use_default_xmlns"
 	cfgKludgeBypassContentEncodingCheckInChunks = "kludge.bypass_content_encoding_check_in_chunks"
 	cfgKludgeDefaultNamespaces                  = "kludge.default_namespaces"
-	cfgKludgeACLEnabled                         = "kludge.acl_enabled"
-
 	// Web.
 	cfgWebReadTimeout       = "web.read_timeout"
 	cfgWebReadHeaderTimeout = "web.read_header_timeout"
@@ -731,7 +729,6 @@ func newSettings() *viper.Viper {
 	v.SetDefault(cfgKludgeUseDefaultXMLNS, false)
 	v.SetDefault(cfgKludgeBypassContentEncodingCheckInChunks, false)
 	v.SetDefault(cfgKludgeDefaultNamespaces, defaultDefaultNamespaces)
-	v.SetDefault(cfgKludgeACLEnabled, false)
 
 	// web
 	v.SetDefault(cfgWebReadHeaderTimeout, defaultReadHeaderTimeout)

config/config.env

@@ -154,8 +154,6 @@ S3_GW_KLUDGE_USE_DEFAULT_XMLNS=false
 S3_GW_KLUDGE_BYPASS_CONTENT_ENCODING_CHECK_IN_CHUNKS=false
 # Namespaces that should be handled as default
 S3_GW_KLUDGE_DEFAULT_NAMESPACES="" "root"
-# Enable bucket/object ACL support for newly created buckets.
-S3_GW_KLUDGE_ACL_ENABLED=false
 
 S3_GW_TRACING_ENABLED=false
 S3_GW_TRACING_ENDPOINT="localhost:4318"

config/config.yaml

@@ -182,8 +182,6 @@ kludge:
   bypass_content_encoding_check_in_chunks: false
   # Namespaces that should be handled as default
   default_namespaces: [ "", "root" ]
-  # Enable bucket/object ACL support for newly created buckets.
-  acl_enabled: false
 
 runtime:
   soft_memory_limit: 1gb

docs/configuration.md

@@ -558,7 +558,6 @@ kludge:
   use_default_xmlns: false
   bypass_content_encoding_check_in_chunks: false
   default_namespaces: [ "", "root" ]
-  acl_enabled: false
 ```
 
 | Parameter                                 | Type       | SIGHUP reload | Default value | Description                                                                                                                                                                                      |
@@ -566,7 +565,6 @@ kludge:
 | `use_default_xmlns`                       | `bool`     | yes           | `false`       | Enable using default xml namespace `http://s3.amazonaws.com/doc/2006-03-01/` when parse xml bodies.                                                                                              |
 | `bypass_content_encoding_check_in_chunks` | `bool`     | yes           | `false`       | Use this flag to be able to use [chunked upload approach](https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-streaming.html) without having `aws-chunked` value in `Content-Encoding` header. |
 | `default_namespaces`                      | `[]string` | yes           | `["","root"]` | Namespaces that should be handled as default.                                                                                                                                                    |
-| `acl_enabled`                             | `bool`     | yes           | `false`       | Enable bucket/object ACL support for newly created buckets.                                                                                                                                      |
 
 # `runtime` section
 Contains runtime parameters.