S3: map canned ACL to file permissions and add configurable default file mode (#8886)
* S3: map canned ACL to file permissions and add configurable default file mode S3 uploads were hardcoded to 0660 regardless of ACL headers. Now the X-Amz-Acl header maps to Unix file permissions per-object: - public-read, authenticated-read, bucket-owner-read → 0644 - public-read-write → 0666 - private, bucket-owner-full-control → 0660 Also adds -defaultFileMode / -s3.defaultFileMode flag to set a server-wide default when no ACL header is present. Closes #8874 * Address review feedback for S3 file mode feature - Extract hardcoded 0660 to defaultFileMode constant - Change parseDefaultFileMode to return error instead of calling Fatalf - Add -s3.defaultFileMode flag to filer.go and mini.go (was missing) - Add doc comment to S3Options about updating all four flag sites - Add TestResolveFileMode with 10 test cases covering ACL mapping, server default, and priority ordering
This commit is contained in:
@@ -146,6 +146,7 @@ func init() {
|
|||||||
filerS3Options.iamReadOnly = cmdFiler.Flag.Bool("s3.iam.readOnly", true, "disable IAM write operations on this server")
|
filerS3Options.iamReadOnly = cmdFiler.Flag.Bool("s3.iam.readOnly", true, "disable IAM write operations on this server")
|
||||||
filerS3Options.portIceberg = cmdFiler.Flag.Int("s3.port.iceberg", 8181, "Iceberg REST Catalog server listen port (0 to disable)")
|
filerS3Options.portIceberg = cmdFiler.Flag.Int("s3.port.iceberg", 8181, "Iceberg REST Catalog server listen port (0 to disable)")
|
||||||
filerS3Options.externalUrl = cmdFiler.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.")
|
filerS3Options.externalUrl = cmdFiler.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.")
|
||||||
|
filerS3Options.defaultFileMode = cmdFiler.Flag.String("s3.defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666")
|
||||||
|
|
||||||
// start webdav on filer
|
// start webdav on filer
|
||||||
filerStartWebDav = cmdFiler.Flag.Bool("webdav", false, "whether to start webdav gateway")
|
filerStartWebDav = cmdFiler.Flag.Bool("webdav", false, "whether to start webdav gateway")
|
||||||
|
|||||||
@@ -250,6 +250,7 @@ func initMiniS3Flags() {
|
|||||||
miniS3Options.auditLogConfig = cmdMini.Flag.String("s3.auditLogConfig", "", "path to the audit log config file")
|
miniS3Options.auditLogConfig = cmdMini.Flag.String("s3.auditLogConfig", "", "path to the audit log config file")
|
||||||
miniS3Options.allowDeleteBucketNotEmpty = miniS3AllowDeleteBucketNotEmpty
|
miniS3Options.allowDeleteBucketNotEmpty = miniS3AllowDeleteBucketNotEmpty
|
||||||
miniS3Options.externalUrl = cmdMini.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.")
|
miniS3Options.externalUrl = cmdMini.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.")
|
||||||
|
miniS3Options.defaultFileMode = cmdMini.Flag.String("s3.defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666")
|
||||||
// In mini mode, S3 uses the shared debug server started at line 681, not its own separate debug server
|
// In mini mode, S3 uses the shared debug server started at line 681, not its own separate debug server
|
||||||
miniS3Options.debug = new(bool) // explicitly false
|
miniS3Options.debug = new(bool) // explicitly false
|
||||||
miniS3Options.debugPort = cmdMini.Flag.Int("s3.debug.port", 6060, "http port for debugging (unused in mini mode)")
|
miniS3Options.debugPort = cmdMini.Flag.Int("s3.debug.port", 6060, "http port for debugging (unused in mini mode)")
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import (
|
|||||||
"net/http"
|
"net/http"
|
||||||
"os"
|
"os"
|
||||||
"runtime"
|
"runtime"
|
||||||
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
@@ -36,6 +37,9 @@ var (
|
|||||||
s3StandaloneOptions S3Options
|
s3StandaloneOptions S3Options
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// S3Options holds CLI flags for the S3 gateway.
|
||||||
|
// Flags are registered in multiple commands: s3.go (standalone), server.go, filer.go, and mini.go.
|
||||||
|
// When adding a new field, update all four flag registration sites.
|
||||||
type S3Options struct {
|
type S3Options struct {
|
||||||
filer *string
|
filer *string
|
||||||
bindIp *string
|
bindIp *string
|
||||||
@@ -68,6 +72,7 @@ type S3Options struct {
|
|||||||
debugPort *int
|
debugPort *int
|
||||||
cipher *bool
|
cipher *bool
|
||||||
externalUrl *string
|
externalUrl *string
|
||||||
|
defaultFileMode *string
|
||||||
}
|
}
|
||||||
|
|
||||||
func init() {
|
func init() {
|
||||||
@@ -103,6 +108,7 @@ func init() {
|
|||||||
s3StandaloneOptions.debugPort = cmdS3.Flag.Int("debug.port", 6060, "http port for debugging")
|
s3StandaloneOptions.debugPort = cmdS3.Flag.Int("debug.port", 6060, "http port for debugging")
|
||||||
s3StandaloneOptions.cipher = cmdS3.Flag.Bool("encryptVolumeData", false, "encrypt data on volume servers")
|
s3StandaloneOptions.cipher = cmdS3.Flag.Bool("encryptVolumeData", false, "encrypt data on volume servers")
|
||||||
s3StandaloneOptions.externalUrl = cmdS3.Flag.String("externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.")
|
s3StandaloneOptions.externalUrl = cmdS3.Flag.String("externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.")
|
||||||
|
s3StandaloneOptions.defaultFileMode = cmdS3.Flag.String("defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666")
|
||||||
}
|
}
|
||||||
|
|
||||||
var cmdS3 = &Command{
|
var cmdS3 = &Command{
|
||||||
@@ -232,6 +238,17 @@ func (s3opt *S3Options) resolveExternalUrl() string {
|
|||||||
return os.Getenv("S3_EXTERNAL_URL")
|
return os.Getenv("S3_EXTERNAL_URL")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (s3opt *S3Options) parseDefaultFileMode() (uint32, error) {
|
||||||
|
if s3opt.defaultFileMode == nil || *s3opt.defaultFileMode == "" {
|
||||||
|
return 0, nil
|
||||||
|
}
|
||||||
|
mode, err := strconv.ParseUint(*s3opt.defaultFileMode, 8, 32)
|
||||||
|
if err != nil {
|
||||||
|
return 0, fmt.Errorf("invalid defaultFileMode %q: %v", *s3opt.defaultFileMode, err)
|
||||||
|
}
|
||||||
|
return uint32(mode), nil
|
||||||
|
}
|
||||||
|
|
||||||
func (s3opt *S3Options) startS3Server() bool {
|
func (s3opt *S3Options) startS3Server() bool {
|
||||||
|
|
||||||
filerAddresses := pb.ServerAddresses(*s3opt.filer).ToAddresses()
|
filerAddresses := pb.ServerAddresses(*s3opt.filer).ToAddresses()
|
||||||
@@ -298,6 +315,11 @@ func (s3opt *S3Options) startS3Server() bool {
|
|||||||
*s3opt.bindIp = "0.0.0.0"
|
*s3opt.bindIp = "0.0.0.0"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
defaultFileMode, fileModeErr := s3opt.parseDefaultFileMode()
|
||||||
|
if fileModeErr != nil {
|
||||||
|
glog.Fatalf("S3 API Server startup error: %v", fileModeErr)
|
||||||
|
}
|
||||||
|
|
||||||
s3ApiServer, s3ApiServer_err = s3api.NewS3ApiServer(router, &s3api.S3ApiServerOption{
|
s3ApiServer, s3ApiServer_err = s3api.NewS3ApiServer(router, &s3api.S3ApiServerOption{
|
||||||
Filers: filerAddresses,
|
Filers: filerAddresses,
|
||||||
Masters: masterAddresses,
|
Masters: masterAddresses,
|
||||||
@@ -320,6 +342,7 @@ func (s3opt *S3Options) startS3Server() bool {
|
|||||||
BindIp: *s3opt.bindIp,
|
BindIp: *s3opt.bindIp,
|
||||||
GrpcPort: *s3opt.portGrpc,
|
GrpcPort: *s3opt.portGrpc,
|
||||||
ExternalUrl: s3opt.resolveExternalUrl(),
|
ExternalUrl: s3opt.resolveExternalUrl(),
|
||||||
|
DefaultFileMode: defaultFileMode,
|
||||||
})
|
})
|
||||||
if s3ApiServer_err != nil {
|
if s3ApiServer_err != nil {
|
||||||
glog.Fatalf("S3 API Server startup error: %v", s3ApiServer_err)
|
glog.Fatalf("S3 API Server startup error: %v", s3ApiServer_err)
|
||||||
|
|||||||
@@ -180,6 +180,7 @@ func init() {
|
|||||||
s3Options.iamReadOnly = cmdServer.Flag.Bool("s3.iam.readOnly", true, "disable IAM write operations on this server")
|
s3Options.iamReadOnly = cmdServer.Flag.Bool("s3.iam.readOnly", true, "disable IAM write operations on this server")
|
||||||
s3Options.cipher = cmdServer.Flag.Bool("s3.encryptVolumeData", false, "encrypt data on volume servers for S3 uploads")
|
s3Options.cipher = cmdServer.Flag.Bool("s3.encryptVolumeData", false, "encrypt data on volume servers for S3 uploads")
|
||||||
s3Options.externalUrl = cmdServer.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.")
|
s3Options.externalUrl = cmdServer.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.")
|
||||||
|
s3Options.defaultFileMode = cmdServer.Flag.String("s3.defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666")
|
||||||
|
|
||||||
sftpOptions.port = cmdServer.Flag.Int("sftp.port", 2022, "SFTP server listen port")
|
sftpOptions.port = cmdServer.Flag.Int("sftp.port", 2022, "SFTP server listen port")
|
||||||
sftpOptions.sshPrivateKey = cmdServer.Flag.String("sftp.sshPrivateKey", "", "path to the SSH private key file for host authentication")
|
sftpOptions.sshPrivateKey = cmdServer.Flag.String("sftp.sshPrivateKey", "", "path to the SSH private key file for host authentication")
|
||||||
|
|||||||
@@ -603,13 +603,14 @@ func (s3a *S3ApiServer) putToFiler(r *http.Request, filePath string, dataReader
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Create entry
|
// Create entry
|
||||||
|
fileMode := s3a.resolveFileMode(r)
|
||||||
entry := &filer_pb.Entry{
|
entry := &filer_pb.Entry{
|
||||||
Name: path.Base(filePath),
|
Name: path.Base(filePath),
|
||||||
IsDirectory: false,
|
IsDirectory: false,
|
||||||
Attributes: &filer_pb.FuseAttributes{
|
Attributes: &filer_pb.FuseAttributes{
|
||||||
Crtime: now.Unix(),
|
Crtime: now.Unix(),
|
||||||
Mtime: now.Unix(),
|
Mtime: now.Unix(),
|
||||||
FileMode: 0660,
|
FileMode: fileMode,
|
||||||
Uid: 0,
|
Uid: 0,
|
||||||
Gid: 0,
|
Gid: 0,
|
||||||
Mime: mimeType,
|
Mime: mimeType,
|
||||||
@@ -815,6 +816,28 @@ func (s3a *S3ApiServer) putToFiler(r *http.Request, filePath string, dataReader
|
|||||||
return etag, s3err.ErrNone, responseMetadata
|
return etag, s3err.ErrNone, responseMetadata
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const defaultFileMode = uint32(0660)
|
||||||
|
|
||||||
|
// resolveFileMode determines the file permission mode for an S3 upload.
|
||||||
|
// Priority: per-object X-Amz-Acl header > server default > defaultFileMode.
|
||||||
|
func (s3a *S3ApiServer) resolveFileMode(r *http.Request) uint32 {
|
||||||
|
if cannedAcl := r.Header.Get(s3_constants.AmzCannedAcl); cannedAcl != "" {
|
||||||
|
switch cannedAcl {
|
||||||
|
case s3_constants.CannedAclPublicRead, s3_constants.CannedAclAuthenticatedRead,
|
||||||
|
s3_constants.CannedAclBucketOwnerRead:
|
||||||
|
return 0644
|
||||||
|
case s3_constants.CannedAclPublicReadWrite:
|
||||||
|
return 0666
|
||||||
|
case s3_constants.CannedAclPrivate, s3_constants.CannedAclBucketOwnerFullControl:
|
||||||
|
return defaultFileMode
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if s3a.option.DefaultFileMode != 0 {
|
||||||
|
return s3a.option.DefaultFileMode
|
||||||
|
}
|
||||||
|
return defaultFileMode
|
||||||
|
}
|
||||||
|
|
||||||
func setEtag(w http.ResponseWriter, etag string) {
|
func setEtag(w http.ResponseWriter, etag string) {
|
||||||
if etag != "" {
|
if etag != "" {
|
||||||
if strings.HasPrefix(etag, "\"") {
|
if strings.HasPrefix(etag, "\"") {
|
||||||
|
|||||||
@@ -301,3 +301,41 @@ func TestWithObjectWriteLockSerializesConcurrentPreconditions(t *testing.T) {
|
|||||||
t.Fatalf("expected %d precondition failures, got %d", workers-1, preconditionFailedCount)
|
t.Fatalf("expected %d precondition failures, got %d", workers-1, preconditionFailedCount)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestResolveFileMode(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
acl string
|
||||||
|
defaultFileMode uint32
|
||||||
|
expected uint32
|
||||||
|
}{
|
||||||
|
{"no acl, no default", "", 0, 0660},
|
||||||
|
{"no acl, with default", "", 0644, 0644},
|
||||||
|
{"private", s3_constants.CannedAclPrivate, 0, 0660},
|
||||||
|
{"private overrides default", s3_constants.CannedAclPrivate, 0644, 0660},
|
||||||
|
{"public-read", s3_constants.CannedAclPublicRead, 0, 0644},
|
||||||
|
{"public-read overrides default", s3_constants.CannedAclPublicRead, 0666, 0644},
|
||||||
|
{"public-read-write", s3_constants.CannedAclPublicReadWrite, 0, 0666},
|
||||||
|
{"authenticated-read", s3_constants.CannedAclAuthenticatedRead, 0, 0644},
|
||||||
|
{"bucket-owner-read", s3_constants.CannedAclBucketOwnerRead, 0, 0644},
|
||||||
|
{"bucket-owner-full-control", s3_constants.CannedAclBucketOwnerFullControl, 0, 0660},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
s3a := &S3ApiServer{
|
||||||
|
option: &S3ApiServerOption{
|
||||||
|
DefaultFileMode: tt.defaultFileMode,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
req := httptest.NewRequest(http.MethodPut, "/bucket/object", nil)
|
||||||
|
if tt.acl != "" {
|
||||||
|
req.Header.Set(s3_constants.AmzCannedAcl, tt.acl)
|
||||||
|
}
|
||||||
|
got := s3a.resolveFileMode(req)
|
||||||
|
if got != tt.expected {
|
||||||
|
t.Errorf("resolveFileMode() = %04o, want %04o", got, tt.expected)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -60,6 +60,7 @@ type S3ApiServerOption struct {
|
|||||||
BindIp string
|
BindIp string
|
||||||
GrpcPort int
|
GrpcPort int
|
||||||
ExternalUrl string // external URL clients use, for signature verification behind a reverse proxy
|
ExternalUrl string // external URL clients use, for signature verification behind a reverse proxy
|
||||||
|
DefaultFileMode uint32 // default file permission mode for S3 uploads (e.g. 0660, 0644)
|
||||||
}
|
}
|
||||||
|
|
||||||
type S3ApiServer struct {
|
type S3ApiServer struct {
|
||||||
|
|||||||
Reference in New Issue
Block a user