handle racy updates to globalSite config (#19750)

```
==================
WARNING: DATA RACE
Read at 0x0000082be990 by goroutine 205:
  github.com/minio/minio/cmd.setCommonHeaders()

Previous write at 0x0000082be990 by main goroutine:
  github.com/minio/minio/cmd.lookupConfigs()
```
This commit is contained in:
Harshavardhana 2024-05-16 16:13:47 -07:00 committed by GitHub
parent aa3fde1784
commit 08d74819b6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
31 changed files with 95 additions and 62 deletions

View File

@ -783,7 +783,7 @@ func (a adminAPIHandlers) ImportBucketMetadataHandler(w http.ResponseWriter, r *
}
switch fileName {
case bucketNotificationConfig:
config, err := event.ParseConfig(io.LimitReader(reader, sz), globalSite.Region, globalEventNotifier.targetList)
config, err := event.ParseConfig(io.LimitReader(reader, sz), globalSite.Region(), globalEventNotifier.targetList)
if err != nil {
rpt.SetStatus(bucket, fileName, fmt.Errorf("%s (%s)", errorCodes[ErrMalformedXML].Description, err))
continue

View File

@ -2416,7 +2416,7 @@ func getServerInfo(ctx context.Context, pools, metrics bool, r *http.Request) ma
return madmin.InfoMessage{
Mode: string(mode),
Domain: domain,
Region: globalSite.Region,
Region: globalSite.Region(),
SQSARN: globalEventNotifier.GetARNList(false),
DeploymentID: globalDeploymentID(),
Buckets: buckets,

View File

@ -456,9 +456,9 @@ func (e errorCodeMap) ToAPIErrWithErr(errCode APIErrorCode, err error) APIError
if err != nil {
apiErr.Description = fmt.Sprintf("%s (%s)", apiErr.Description, err)
}
if globalSite.Region != "" {
if region := globalSite.Region(); region != "" {
if errCode == ErrAuthorizationHeaderMalformed {
apiErr.Description = fmt.Sprintf("The authorization header is malformed; the region is wrong; expecting '%s'.", globalSite.Region)
apiErr.Description = fmt.Sprintf("The authorization header is malformed; the region is wrong; expecting '%s'.", region)
return apiErr
}
}
@ -2579,7 +2579,7 @@ func getAPIErrorResponse(ctx context.Context, err APIError, resource, requestID,
BucketName: reqInfo.BucketName,
Key: reqInfo.ObjectName,
Resource: resource,
Region: globalSite.Region,
Region: globalSite.Region(),
RequestID: requestID,
HostID: hostID,
ActualObjectSize: err.ObjectSize,

View File

@ -54,7 +54,7 @@ func setCommonHeaders(w http.ResponseWriter) {
// Set `x-amz-bucket-region` only if region is set on the server
// by default minio uses an empty region.
if region := globalSite.Region; region != "" {
if region := globalSite.Region(); region != "" {
w.Header().Set(xhttp.AmzBucketRegion, region)
}
w.Header().Set(xhttp.AcceptRanges, "bytes")

View File

@ -954,9 +954,9 @@ func writeErrorResponse(ctx context.Context, w http.ResponseWriter, err APIError
switch err.Code {
case "InvalidRegion":
err.Description = fmt.Sprintf("Region does not match; expecting '%s'.", globalSite.Region)
err.Description = fmt.Sprintf("Region does not match; expecting '%s'.", globalSite.Region())
case "AuthorizationHeaderMalformed":
err.Description = fmt.Sprintf("The authorization header is malformed; the region is wrong; expecting '%s'.", globalSite.Region)
err.Description = fmt.Sprintf("The authorization header is malformed; the region is wrong; expecting '%s'.", globalSite.Region())
}
// Similar check to http.checkWriteHeaderCode

View File

@ -178,7 +178,7 @@ func validateAdminSignature(ctx context.Context, r *http.Request, region string)
logger.GetReqInfo(ctx).Cred = cred
logger.GetReqInfo(ctx).Owner = owner
logger.GetReqInfo(ctx).Region = globalSite.Region
logger.GetReqInfo(ctx).Region = globalSite.Region()
return cred, owner, ErrNone
}
@ -368,7 +368,7 @@ func authenticateRequest(ctx context.Context, r *http.Request, action policy.Act
}
cred, owner, s3Err = getReqAccessKeyV2(r)
case authTypeSigned, authTypePresigned:
region := globalSite.Region
region := globalSite.Region()
switch action {
case policy.GetBucketLocationAction, policy.ListAllMyBucketsAction:
region = ""
@ -384,7 +384,7 @@ func authenticateRequest(ctx context.Context, r *http.Request, action policy.Act
logger.GetReqInfo(ctx).Cred = cred
logger.GetReqInfo(ctx).Owner = owner
logger.GetReqInfo(ctx).Region = globalSite.Region
logger.GetReqInfo(ctx).Region = globalSite.Region()
// region is valid only for CreateBucketAction.
var region string
@ -684,7 +684,7 @@ func validateSignature(atype authType, r *http.Request) (auth.Credentials, bool,
}
cred, owner, s3Err = getReqAccessKeyV2(r)
case authTypePresigned, authTypeSigned:
region := globalSite.Region
region := globalSite.Region()
if s3Err = isReqAuthenticated(GlobalContext, r, region, serviceS3); s3Err != ErrNone {
return cred, owner, s3Err
}
@ -745,7 +745,7 @@ func isPutRetentionAllowed(bucketName, objectName string, retDays int, retDate t
func isPutActionAllowed(ctx context.Context, atype authType, bucketName, objectName string, r *http.Request, action policy.Action) (s3Err APIErrorCode) {
var cred auth.Credentials
var owner bool
region := globalSite.Region
region := globalSite.Region()
switch atype {
case authTypeUnknown:
return ErrSignatureVersionNotSupported

View File

@ -403,7 +403,7 @@ func TestIsReqAuthenticated(t *testing.T) {
// Validates all testcases.
for i, testCase := range testCases {
s3Error := isReqAuthenticated(ctx, testCase.req, globalSite.Region, serviceS3)
s3Error := isReqAuthenticated(ctx, testCase.req, globalSite.Region(), serviceS3)
if s3Error != testCase.s3Error {
if _, err := io.ReadAll(testCase.req.Body); toAPIErrorCode(ctx, err) != testCase.s3Error {
t.Fatalf("Test %d: Unexpected S3 error: want %d - got %d (got after reading request %s)", i, testCase.s3Error, s3Error, toAPIError(ctx, err).Code)
@ -443,7 +443,7 @@ func TestCheckAdminRequestAuthType(t *testing.T) {
{Request: mustNewPresignedRequest(http.MethodGet, "http://127.0.0.1:9000", 0, nil, t), ErrCode: ErrAccessDenied},
}
for i, testCase := range testCases {
if _, s3Error := checkAdminRequestAuth(ctx, testCase.Request, policy.AllAdminActions, globalSite.Region); s3Error != testCase.ErrCode {
if _, s3Error := checkAdminRequestAuth(ctx, testCase.Request, policy.AllAdminActions, globalSite.Region()); s3Error != testCase.ErrCode {
t.Errorf("Test %d: Unexpected s3error returned wanted %d, got %d", i, testCase.ErrCode, s3Error)
}
}

View File

@ -227,7 +227,7 @@ func (api objectAPIHandlers) GetBucketLocationHandler(w http.ResponseWriter, r *
// Generate response.
encodedSuccessResponse := encodeResponse(LocationResponse{})
// Get current region.
region := globalSite.Region
region := globalSite.Region()
if region != globalMinioDefaultRegion {
encodedSuccessResponse = encodeResponse(LocationResponse{
Location: region,

View File

@ -66,8 +66,9 @@ func (api objectAPIHandlers) GetBucketNotificationHandler(w http.ResponseWriter,
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
return
}
config.SetRegion(globalSite.Region)
if err = config.Validate(globalSite.Region, globalEventNotifier.targetList); err != nil {
region := globalSite.Region()
config.SetRegion(region)
if err = config.Validate(region, globalEventNotifier.targetList); err != nil {
arnErr, ok := err.(*event.ErrARNNotFound)
if ok {
for i, queue := range config.QueueList {
@ -134,7 +135,7 @@ func (api objectAPIHandlers) PutBucketNotificationHandler(w http.ResponseWriter,
return
}
config, err := event.ParseConfig(io.LimitReader(r.Body, r.ContentLength), globalSite.Region, globalEventNotifier.targetList)
config, err := event.ParseConfig(io.LimitReader(r.Body, r.ContentLength), globalSite.Region(), globalEventNotifier.targetList)
if err != nil {
apiErr := errorCodes.ToAPIErr(ErrMalformedXML)
if event.IsEventError(err) {

View File

@ -176,7 +176,10 @@ func minioConfigToConsoleFeatures() {
os.Setenv("CONSOLE_STS_DURATION", valueSession)
}
os.Setenv("CONSOLE_MINIO_REGION", globalSite.Region)
os.Setenv("CONSOLE_MINIO_SITE_NAME", globalSite.Name())
os.Setenv("CONSOLE_MINIO_SITE_REGION", globalSite.Region())
os.Setenv("CONSOLE_MINIO_REGION", globalSite.Region())
os.Setenv("CONSOLE_CERT_PASSWD", env.Get("MINIO_CERT_PASSWD", ""))
// This section sets Browser (console) stored config

View File

@ -362,7 +362,7 @@ func validateSubSysConfig(ctx context.Context, s config.Config, subSys string, o
}
case config.IdentityOpenIDSubSys:
if _, err := openid.LookupConfig(s,
NewHTTPTransport(), xhttp.DrainBody, globalSite.Region); err != nil {
NewHTTPTransport(), xhttp.DrainBody, globalSite.Region()); err != nil {
return err
}
case config.IdentityLDAPSubSys:
@ -383,7 +383,7 @@ func validateSubSysConfig(ctx context.Context, s config.Config, subSys string, o
}
case config.IdentityPluginSubSys:
if _, err := idplugin.LookupConfig(s[config.IdentityPluginSubSys][config.Default],
NewHTTPTransport(), xhttp.DrainBody, globalSite.Region); err != nil {
NewHTTPTransport(), xhttp.DrainBody, globalSite.Region()); err != nil {
return err
}
case config.SubnetSubSys:
@ -530,10 +530,11 @@ func lookupConfigs(s config.Config, objAPI ObjectLayer) {
// but not federation.
globalBucketFederation = etcdCfg.PathPrefix == "" && etcdCfg.Enabled
globalSite, err = config.LookupSite(s[config.SiteSubSys][config.Default], s[config.RegionSubSys][config.Default])
siteCfg, err := config.LookupSite(s[config.SiteSubSys][config.Default], s[config.RegionSubSys][config.Default])
if err != nil {
configLogIf(ctx, fmt.Errorf("Invalid site configuration: %w", err))
}
globalSite.Update(siteCfg)
globalAutoEncryption = crypto.LookupAutoEncryption() // Enable auto-encryption if enabled
if globalAutoEncryption && GlobalKMS == nil {

View File

@ -39,8 +39,8 @@ func TestServerConfig(t *testing.T) {
t.Fatalf("Init Test config failed")
}
if globalSite.Region != globalMinioDefaultRegion {
t.Errorf("Expecting region `us-east-1` found %s", globalSite.Region)
if globalSite.Region() != globalMinioDefaultRegion {
t.Errorf("Expecting region `us-east-1` found %s", globalSite.Region())
}
// Set new region and verify.
@ -52,8 +52,8 @@ func TestServerConfig(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if site.Region != "us-west-1" {
t.Errorf("Expecting region `us-west-1` found %s", globalSite.Region)
if site.Region() != "us-west-1" {
t.Errorf("Expecting region `us-west-1` found %s", globalSite.Region())
}
if err := saveServerConfig(context.Background(), objLayer, globalServerConfig); err != nil {

View File

@ -54,7 +54,7 @@ func (evnot *EventNotifier) GetARNList(onlyActive bool) []string {
if evnot == nil {
return arns
}
region := globalSite.Region
region := globalSite.Region()
for targetID, target := range evnot.targetList.TargetMap() {
// httpclient target is part of ListenNotification
// which doesn't need to be listed as part of the ARN list
@ -79,8 +79,9 @@ func (evnot *EventNotifier) set(bucket BucketInfo, meta BucketMetadata) {
if config == nil {
return
}
config.SetRegion(globalSite.Region)
if err := config.Validate(globalSite.Region, globalEventNotifier.targetList); err != nil {
region := globalSite.Region()
config.SetRegion(region)
if err := config.Validate(region, globalEventNotifier.targetList); err != nil {
if _, ok := err.(*event.ErrARNNotFound); !ok {
internalLogIf(GlobalContext, err)
}

View File

@ -458,7 +458,7 @@ func (driver *ftpDriver) MakeDir(ctx *ftp.Context, objPath string) (err error) {
}
if prefix == "" {
return clnt.MakeBucket(context.Background(), bucket, minio.MakeBucketOptions{Region: globalSite.Region})
return clnt.MakeBucket(context.Background(), bucket, minio.MakeBucketOptions{Region: globalSite.Region()})
}
dirPath := buildMinioDir(prefix)

View File

@ -209,9 +209,8 @@ var (
// This flag is set to 'true' when MINIO_UPDATE env is set to 'off'. Default is false.
globalInplaceUpdateDisabled = false
globalSite = config.Site{
Region: globalMinioDefaultRegion,
}
// Captures site name and region
globalSite config.Site
// MinIO local server address (in `host:port` format)
globalMinioAddr = ""

View File

@ -57,7 +57,7 @@ func parseLocationConstraint(r *http.Request) (location string, s3Error APIError
} // else for both err as nil or io.EOF
location = locationConstraint.Location
if location == "" {
location = globalSite.Region
location = globalSite.Region()
}
if !isValidLocation(location) {
return location, ErrInvalidRegion
@ -69,7 +69,8 @@ func parseLocationConstraint(r *http.Request) (location string, s3Error APIError
// Validates input location is same as configured region
// of MinIO server.
func isValidLocation(location string) bool {
return globalSite.Region == "" || globalSite.Region == location
region := globalSite.Region()
return region == "" || region == location
}
// Supported headers that needs to be extracted.
@ -243,7 +244,7 @@ func extractReqParams(r *http.Request) map[string]string {
return nil
}
region := globalSite.Region
region := globalSite.Region()
cred := getReqAccessCred(r, region)
principalID := cred.AccessKey

View File

@ -230,7 +230,7 @@ func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer, etcdClient *etc
globalServerConfigMu.RUnlock()
openidConfig, err := openid.LookupConfig(s,
NewHTTPTransport(), xhttp.DrainBody, globalSite.Region)
NewHTTPTransport(), xhttp.DrainBody, globalSite.Region())
if err != nil {
iamLogIf(ctx, fmt.Errorf("Unable to initialize OpenID: %w", err), logger.WarningKind)
}
@ -251,7 +251,7 @@ func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer, etcdClient *etc
}
authNPluginCfg, err := idplugin.LookupConfig(s[config.IdentityPluginSubSys][config.Default],
NewHTTPTransport(), xhttp.DrainBody, globalSite.Region)
NewHTTPTransport(), xhttp.DrainBody, globalSite.Region())
if err != nil {
iamLogIf(ctx, fmt.Errorf("Unable to initialize AuthNPlugin: %w", err), logger.WarningKind)
}

View File

@ -1328,7 +1328,7 @@ func getRemoteInstanceTransport() http.RoundTripper {
// Returns a minio-go Client configured to access remote host described by destDNSRecord
// Applicable only in a federated deployment
var getRemoteInstanceClient = func(r *http.Request, host string) (*miniogo.Core, error) {
cred := getReqAccessCred(r, globalSite.Region)
cred := getReqAccessCred(r, globalSite.Region())
// In a federated deployment, all the instances share config files
// and hence expected to have same credentials.
core, err := miniogo.NewCore(host, &miniogo.Options{
@ -2103,7 +2103,7 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req
}
case authTypePresigned, authTypeSigned:
if s3Err = reqSignatureV4Verify(r, globalSite.Region, serviceS3); s3Err != ErrNone {
if s3Err = reqSignatureV4Verify(r, globalSite.Region(), serviceS3); s3Err != ErrNone {
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Err), r.URL)
return
}
@ -2521,7 +2521,7 @@ func (api objectAPIHandlers) PutObjectExtractHandler(w http.ResponseWriter, r *h
}
case authTypePresigned, authTypeSigned:
if s3Err = reqSignatureV4Verify(r, globalSite.Region, serviceS3); s3Err != ErrNone {
if s3Err = reqSignatureV4Verify(r, globalSite.Region(), serviceS3); s3Err != ErrNone {
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Err), r.URL)
return
}

View File

@ -55,7 +55,7 @@ func getLambdaEventData(bucket, object string, cred auth.Credentials, r *http.Re
Creds: credentials.NewStaticV4(cred.AccessKey, cred.SecretKey, cred.SessionToken),
Secure: secure,
Transport: globalRemoteTargetTransport,
Region: globalSite.Region,
Region: globalSite.Region(),
})
if err != nil {
return levent.Event{}, err

View File

@ -686,7 +686,7 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http
return
}
case authTypePresigned, authTypeSigned:
if s3Error = reqSignatureV4Verify(r, globalSite.Region, serviceS3); s3Error != ErrNone {
if s3Error = reqSignatureV4Verify(r, globalSite.Region(), serviceS3); s3Error != ErrNone {
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
return
}

View File

@ -102,7 +102,7 @@ func selfSpeedTest(ctx context.Context, opts speedTestOpts) (res SpeedTestResult
clnt := globalMinioClient
if !globalAPIConfig.permitRootAccess() {
region := globalSite.Region
region := globalSite.Region()
if region == "" {
region = "us-east-1"
}

View File

@ -1135,7 +1135,7 @@ func serverMain(ctx *cli.Context) {
}
}()
region := globalSite.Region
region := globalSite.Region()
if region == "" {
region = "us-east-1"
}

View File

@ -109,7 +109,7 @@ func printServerCommonMsg(apiEndpoints []string) {
cred := globalActiveCred
// Get saved region.
region := globalSite.Region
region := globalSite.Region()
apiEndpointStr := strings.TrimSpace(strings.Join(apiEndpoints, " "))
// Colorize the message and print.
@ -146,7 +146,7 @@ func printLambdaTargets() {
}
arnMsg := color.Blue("Object Lambda ARNs: ")
for _, arn := range globalLambdaTargetList.List(globalSite.Region) {
for _, arn := range globalLambdaTargetList.List(globalSite.Region()) {
arnMsg += color.Bold(fmt.Sprintf("%s ", arn))
}
logger.Info(arnMsg + "\n")

View File

@ -398,7 +398,7 @@ func (f *sftpDriver) Filecmd(r *sftp.Request) (err error) {
}
if prefix == "" {
return clnt.MakeBucket(context.Background(), bucket, minio.MakeBucketOptions{Region: globalSite.Region})
return clnt.MakeBucket(context.Background(), bucket, minio.MakeBucketOptions{Region: globalSite.Region()})
}
dirPath := buildMinioDir(prefix)

View File

@ -175,7 +175,7 @@ func compareSignatureV4(sig1, sig2 string) bool {
// returns ErrNone if the signature matches.
func doesPolicySignatureV4Match(formValues http.Header) (auth.Credentials, APIErrorCode) {
// Server region.
region := globalSite.Region
region := globalSite.Region()
// Parse credential tag.
credHeader, s3Err := parseCredentialHeader("Credential="+formValues.Get(xhttp.AmzCredential), region, serviceS3)

View File

@ -111,7 +111,7 @@ func TestDoesPresignedSignatureMatch(t *testing.T) {
now := UTCNow()
credentialTemplate := "%s/%s/%s/s3/aws4_request"
region := globalSite.Region
region := globalSite.Region()
accessKeyID := globalActiveCred.AccessKey
testCases := []struct {
queryParams map[string]string

View File

@ -677,7 +677,7 @@ func (c *SiteReplicationSys) GetIDPSettings(ctx context.Context) madmin.IDPSetti
}
s.OpenID = globalIAMSys.OpenIDConfig.GetSettings()
if s.OpenID.Enabled {
s.OpenID.Region = globalSite.Region
s.OpenID.Region = globalSite.Region()
}
return s
}

View File

@ -106,7 +106,7 @@ func calculateSeedSignature(r *http.Request, trailers bool) (cred auth.Credentia
v4Auth := req.Header.Get(xhttp.Authorization)
// Parse signature version '4' header.
signV4Values, errCode := parseSignV4(v4Auth, globalSite.Region, serviceS3)
signV4Values, errCode := parseSignV4(v4Auth, globalSite.Region(), serviceS3)
if errCode != ErrNone {
return cred, "", "", time.Time{}, errCode
}

View File

@ -155,12 +155,12 @@ func checkAssumeRoleAuth(ctx context.Context, r *http.Request) (auth.Credentials
return auth.Credentials{}, ErrAccessDenied
}
s3Err := isReqAuthenticated(ctx, r, globalSite.Region, serviceSTS)
s3Err := isReqAuthenticated(ctx, r, globalSite.Region(), serviceSTS)
if s3Err != ErrNone {
return auth.Credentials{}, s3Err
}
user, _, s3Err := getReqAccessKeyV4(r, globalSite.Region, serviceSTS)
user, _, s3Err := getReqAccessKeyV4(r, globalSite.Region(), serviceSTS)
if s3Err != ErrNone {
return auth.Credentials{}, s3Err
}

View File

@ -750,7 +750,7 @@ func newTestStreamingRequest(method, urlStr string, dataLength, chunkSize int64,
func assembleStreamingChunks(req *http.Request, body io.ReadSeeker, chunkSize int64,
secretKey, signature string, currTime time.Time) (*http.Request, error,
) {
regionStr := globalSite.Region
regionStr := globalSite.Region()
var stream []byte
var buffer []byte
body.Seek(0, 0)
@ -858,7 +858,7 @@ func preSignV4(req *http.Request, accessKeyID, secretAccessKey string, expires i
return errors.New("Presign cannot be generated without access and secret keys")
}
region := globalSite.Region
region := globalSite.Region()
date := UTCNow()
scope := getScope(date, region)
credential := fmt.Sprintf("%s/%s", accessKeyID, scope)
@ -986,7 +986,7 @@ func signRequestV4(req *http.Request, accessKey, secretKey string) error {
}
sort.Strings(headers)
region := globalSite.Region
region := globalSite.Region()
// Get canonical headers.
var buf bytes.Buffer

View File

@ -24,6 +24,7 @@ import (
"regexp"
"sort"
"strings"
"sync"
"github.com/minio/madmin-go/v3"
"github.com/minio/minio-go/v7/pkg/set"
@ -544,10 +545,36 @@ var (
}
)
var siteLK sync.RWMutex
// Site - holds site info - name and region.
type Site struct {
Name string
Region string
name string
region string
}
// Update safe update the new site name and region
func (s *Site) Update(n Site) {
siteLK.Lock()
s.name = n.name
s.region = n.region
siteLK.Unlock()
}
// Name returns currently configured site name
func (s *Site) Name() string {
siteLK.RLock()
defer siteLK.RUnlock()
return s.name
}
// Region returns currently configured site region
func (s *Site) Region() string {
siteLK.RLock()
defer siteLK.RUnlock()
return s.region
}
var validRegionRegex = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9-_-]+$")
@ -590,7 +617,7 @@ func LookupSite(siteKV KVS, regionKV KVS) (s Site, err error) {
region)
return
}
s.Region = region
s.region = region
}
name := env.Get(EnvSiteName, siteKV.Get(NameKey))
@ -601,7 +628,7 @@ func LookupSite(siteKV KVS, regionKV KVS) (s Site, err error) {
name)
return
}
s.Name = name
s.name = name
}
return
}