From bdc20d1c1e19043ecf5a2630a03e2e99dd8fac54 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 31 Oct 2025 22:35:09 -0700 Subject: [PATCH] S3: load bucket object locking configuration if not found in cache (#7422) * load bucket object locking configuration if not found in cache * fix cache building, more specific error, add back metrics --- weed/s3api/auth_credentials_subscribe.go | 8 +- weed/s3api/s3api_bucket_config.go | 12 ++- weed/s3api/s3api_bucket_handlers.go | 1 + ...3api_bucket_handlers_object_lock_config.go | 85 ++++++++++++------- 4 files changed, 72 insertions(+), 34 deletions(-) diff --git a/weed/s3api/auth_credentials_subscribe.go b/weed/s3api/auth_credentials_subscribe.go index 68286a877..829a3d61c 100644 --- a/weed/s3api/auth_credentials_subscribe.go +++ b/weed/s3api/auth_credentials_subscribe.go @@ -109,6 +109,9 @@ func (s3a *S3ApiServer) updateBucketConfigCacheFromEntry(entry *filer_pb.Entry) bucket := entry.Name + glog.V(3).Infof("updateBucketConfigCacheFromEntry: called for bucket %s, ExtObjectLockEnabledKey=%s", + bucket, string(entry.Extended[s3_constants.ExtObjectLockEnabledKey])) + // Create new bucket config from the entry config := &BucketConfig{ Name: bucket, @@ -138,7 +141,9 @@ func (s3a *S3ApiServer) updateBucketConfigCacheFromEntry(entry *filer_pb.Entry) // Parse Object Lock configuration if present if objectLockConfig, found := LoadObjectLockConfigurationFromExtended(entry); found { config.ObjectLockConfig = objectLockConfig - glog.V(2).Infof("updateBucketConfigCacheFromEntry: cached Object Lock configuration for bucket %s", bucket) + glog.V(2).Infof("updateBucketConfigCacheFromEntry: cached Object Lock configuration for bucket %s: %+v", bucket, objectLockConfig) + } else { + glog.V(3).Infof("updateBucketConfigCacheFromEntry: no Object Lock configuration found for bucket %s", bucket) } } @@ -156,6 +161,7 @@ func (s3a *S3ApiServer) updateBucketConfigCacheFromEntry(entry *filer_pb.Entry) config.LastModified = time.Now() // Update cache + glog.V(3).Infof("updateBucketConfigCacheFromEntry: updating cache for bucket %s, ObjectLockConfig=%+v", bucket, config.ObjectLockConfig) s3a.bucketConfigCache.Set(bucket, config) } diff --git a/weed/s3api/s3api_bucket_config.go b/weed/s3api/s3api_bucket_config.go index 61cddc45a..26b114160 100644 --- a/weed/s3api/s3api_bucket_config.go +++ b/weed/s3api/s3api_bucket_config.go @@ -350,6 +350,8 @@ func (s3a *S3ApiServer) getBucketConfig(bucket string) (*BucketConfig, s3err.Err // Extract configuration from extended attributes if entry.Extended != nil { + glog.V(3).Infof("getBucketConfig: checking extended attributes for bucket %s, ExtObjectLockEnabledKey value=%s", + bucket, string(entry.Extended[s3_constants.ExtObjectLockEnabledKey])) if versioning, exists := entry.Extended[s3_constants.ExtVersioningKey]; exists { config.Versioning = string(versioning) } @@ -370,7 +372,9 @@ func (s3a *S3ApiServer) getBucketConfig(bucket string) (*BucketConfig, s3err.Err // Parse Object Lock configuration if present if objectLockConfig, found := LoadObjectLockConfigurationFromExtended(entry); found { config.ObjectLockConfig = objectLockConfig - glog.V(2).Infof("getBucketConfig: cached Object Lock configuration for bucket %s", bucket) + glog.V(3).Infof("getBucketConfig: loaded Object Lock config from extended attributes for bucket %s: %+v", bucket, objectLockConfig) + } else { + glog.V(3).Infof("getBucketConfig: no Object Lock config found in extended attributes for bucket %s", bucket) } } @@ -426,20 +430,26 @@ func (s3a *S3ApiServer) updateBucketConfig(bucket string, updateFn func(*BucketC } // Update Object Lock configuration if config.ObjectLockConfig != nil { + glog.V(3).Infof("updateBucketConfig: storing Object Lock config for bucket %s: %+v", bucket, config.ObjectLockConfig) if err := StoreObjectLockConfigurationInExtended(config.Entry, config.ObjectLockConfig); err != nil { glog.Errorf("updateBucketConfig: failed to store Object Lock configuration for bucket %s: %v", bucket, err) return s3err.ErrInternalError } + glog.V(3).Infof("updateBucketConfig: stored Object Lock config in extended attributes for bucket %s, key=%s, value=%s", + bucket, s3_constants.ExtObjectLockEnabledKey, string(config.Entry.Extended[s3_constants.ExtObjectLockEnabledKey])) } // Save to filer + glog.V(3).Infof("updateBucketConfig: saving entry to filer for bucket %s", bucket) err := s3a.updateEntry(s3a.option.BucketsPath, config.Entry) if err != nil { glog.Errorf("updateBucketConfig: failed to update bucket entry for %s: %v", bucket, err) return s3err.ErrInternalError } + glog.V(3).Infof("updateBucketConfig: saved entry to filer for bucket %s", bucket) // Update cache + glog.V(3).Infof("updateBucketConfig: updating cache for bucket %s, ObjectLockConfig=%+v", bucket, config.ObjectLockConfig) s3a.bucketConfigCache.Set(bucket, config) return s3err.ErrNone diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index 060d453b1..c3f934557 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -224,6 +224,7 @@ func (s3a *S3ApiServer) PutBucketHandler(w http.ResponseWriter, r *http.Request) // Set the cached Object Lock configuration bucketConfig.ObjectLockConfig = objectLockConfig + glog.V(3).Infof("PutBucketHandler: set ObjectLockConfig for bucket %s: %+v", bucket, objectLockConfig) return nil }) diff --git a/weed/s3api/s3api_bucket_handlers_object_lock_config.go b/weed/s3api/s3api_bucket_handlers_object_lock_config.go index 6747e6aaf..c779f80d7 100644 --- a/weed/s3api/s3api_bucket_handlers_object_lock_config.go +++ b/weed/s3api/s3api_bucket_handlers_object_lock_config.go @@ -2,11 +2,11 @@ package s3api import ( "encoding/xml" - "net/http" - "errors" + "net/http" "github.com/seaweedfs/seaweedfs/weed/glog" + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" stats_collect "github.com/seaweedfs/seaweedfs/weed/stats" @@ -82,7 +82,7 @@ func (s3a *S3ApiServer) GetObjectLockConfigurationHandler(w http.ResponseWriter, return } - var configXML []byte + glog.V(3).Infof("GetObjectLockConfigurationHandler: retrieved bucket config for %s, ObjectLockConfig=%+v", bucket, bucketConfig.ObjectLockConfig) // Check if we have cached Object Lock configuration if bucketConfig.ObjectLockConfig != nil { @@ -105,46 +105,67 @@ func (s3a *S3ApiServer) GetObjectLockConfigurationHandler(w http.ResponseWriter, glog.Errorf("GetObjectLockConfigurationHandler: failed to write config XML: %v", err) return } + + // Record metrics + stats_collect.RecordBucketActiveTime(bucket) + glog.V(3).Infof("GetObjectLockConfigurationHandler: successfully retrieved cached object lock config for %s", bucket) return } - // Fallback: check for legacy storage in extended attributes - if bucketConfig.Entry.Extended != nil { - // Check if Object Lock is enabled via boolean flag - if enabledBytes, exists := bucketConfig.Entry.Extended[s3_constants.ExtObjectLockEnabledKey]; exists { - enabled := string(enabledBytes) - if enabled == s3_constants.ObjectLockEnabled || enabled == "true" { - // Generate minimal XML configuration for enabled Object Lock without retention policies - minimalConfig := `Enabled` - configXML = []byte(minimalConfig) - } + // If no cached Object Lock configuration, reload entry from filer to get the latest extended attributes + // This handles cases where the cache might have a stale entry due to timing issues with metadata updates + glog.V(3).Infof("GetObjectLockConfigurationHandler: no cached ObjectLockConfig, reloading entry from filer for %s", bucket) + freshEntry, err := s3a.getEntry(s3a.option.BucketsPath, bucket) + if err != nil { + if errors.Is(err, filer_pb.ErrNotFound) { + glog.V(1).Infof("GetObjectLockConfigurationHandler: bucket %s not found while reloading entry", bucket) + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket) + return } - } - - // If no Object Lock configuration found, return error - if len(configXML) == 0 { - s3err.WriteErrorResponse(w, r, s3err.ErrObjectLockConfigurationNotFoundError) + glog.Errorf("GetObjectLockConfigurationHandler: failed to reload bucket entry: %v", err) + s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) return } - // Set response headers - w.Header().Set("Content-Type", "application/xml") - w.WriteHeader(http.StatusOK) + // Try to load Object Lock configuration from the fresh entry + // LoadObjectLockConfigurationFromExtended already checks ExtObjectLockEnabledKey and returns + // a basic configuration even when there's no default retention policy + if objectLockConfig, found := LoadObjectLockConfigurationFromExtended(freshEntry); found { + glog.V(3).Infof("GetObjectLockConfigurationHandler: loaded Object Lock config from fresh entry for %s: %+v", bucket, objectLockConfig) - // Write XML response - if _, err := w.Write([]byte(xml.Header)); err != nil { - glog.Errorf("GetObjectLockConfigurationHandler: failed to write XML header: %v", err) - return - } + // Rebuild the entire cached config from the fresh entry to maintain cache coherence + // This ensures all fields (Versioning, Owner, ACL, IsPublicRead, CORS, etc.) are up-to-date, + // not just ObjectLockConfig, before resetting the TTL + s3a.updateBucketConfigCacheFromEntry(freshEntry) - if _, err := w.Write(configXML); err != nil { - glog.Errorf("GetObjectLockConfigurationHandler: failed to write config XML: %v", err) + // Marshal and return the configuration + marshaledXML, err := xml.Marshal(objectLockConfig) + if err != nil { + glog.Errorf("GetObjectLockConfigurationHandler: failed to marshal Object Lock config: %v", err) + s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) + return + } + + w.Header().Set("Content-Type", "application/xml") + w.WriteHeader(http.StatusOK) + if _, err := w.Write([]byte(xml.Header)); err != nil { + glog.Errorf("GetObjectLockConfigurationHandler: failed to write XML header: %v", err) + return + } + if _, err := w.Write(marshaledXML); err != nil { + glog.Errorf("GetObjectLockConfigurationHandler: failed to write config XML: %v", err) + return + } + + // Record metrics + stats_collect.RecordBucketActiveTime(bucket) + + glog.V(3).Infof("GetObjectLockConfigurationHandler: successfully retrieved object lock config from fresh entry for %s", bucket) return } - // Record metrics - stats_collect.RecordBucketActiveTime(bucket) - - glog.V(3).Infof("GetObjectLockConfigurationHandler: successfully retrieved object lock config for %s", bucket) + // No Object Lock configuration found + glog.V(3).Infof("GetObjectLockConfigurationHandler: no Object Lock configuration found for %s", bucket) + s3err.WriteErrorResponse(w, r, s3err.ErrObjectLockConfigurationNotFoundError) }