diff --git a/weed/s3api/auth_credentials_subscribe.go b/weed/s3api/auth_credentials_subscribe.go index b0a04ffe1..30aad4fcb 100644 --- a/weed/s3api/auth_credentials_subscribe.go +++ b/weed/s3api/auth_credentials_subscribe.go @@ -61,40 +61,43 @@ func (s3a *S3ApiServer) onIamConfigChange(dir string, oldEntry *filer_pb.Entry, glog.V(1).Infof("Skipping IAM config update for static configuration") return nil } + if s3a.iam == nil { + return nil + } - // 1. Handle traditional single identity.json file - if dir == filer.IamConfigDirectory { - // Handle deletion: reset to empty config - if newEntry == nil && oldEntry != nil && oldEntry.Name == filer.IamIdentityFile { - glog.V(1).Infof("IAM config file deleted, clearing identities") - if err := s3a.iam.LoadS3ApiConfigurationFromBytes([]byte{}); err != nil { - glog.Warningf("failed to clear IAM config on deletion: %v", err) - return err - } - return nil + reloadIamConfig := func(reason string) error { + glog.V(1).Infof("IAM change detected in %s, reloading configuration", reason) + if err := s3a.iam.LoadS3ApiConfigurationFromCredentialManager(); err != nil { + glog.Errorf("failed to reload IAM configuration after change in %s: %v", reason, err) + return err } + return nil + } - // Handle create/update - if newEntry != nil && newEntry.Name == filer.IamIdentityFile { - if err := s3a.iam.LoadS3ApiConfigurationFromBytes(newEntry.Content); err != nil { + // 1. Handle traditional single identity.json file + if dir == filer.IamConfigDirectory { + // Handle create/update/delete events on legacy identity.json. + // During migration this file is renamed, which emits a delete event. + // Always reload from the credential manager so we keep the migrated identities. + if (oldEntry != nil && oldEntry.Name == filer.IamIdentityFile) || + (newEntry != nil && newEntry.Name == filer.IamIdentityFile) { + if err := reloadIamConfig(dir + "/" + filer.IamIdentityFile); err != nil { return err } - glog.V(1).Infof("updated %s/%s", dir, newEntry.Name) } return nil } // 2. Handle multiple-file identities and policies - // Watch /etc/seaweedfs/identities and /etc/seaweedfs/policies - isIdentityDir := strings.HasPrefix(dir, "/etc/seaweedfs/identities") - isPolicyDir := strings.HasPrefix(dir, "/etc/seaweedfs/policies") + // Watch /etc/iam/{identities,policies,service_accounts} + isIdentityDir := dir == filer.IamConfigDirectory+"/identities" || strings.HasPrefix(dir, filer.IamConfigDirectory+"/identities/") + isPolicyDir := dir == filer.IamConfigDirectory+"/policies" || strings.HasPrefix(dir, filer.IamConfigDirectory+"/policies/") + isServiceAccountDir := dir == filer.IamConfigDirectory+"/service_accounts" || strings.HasPrefix(dir, filer.IamConfigDirectory+"/service_accounts/") - if isIdentityDir || isPolicyDir { + if isIdentityDir || isPolicyDir || isServiceAccountDir { // For multiple-file mode, any change in these directories should trigger a full reload // from the credential manager (which handles the details of loading from multiple files). - glog.V(1).Infof("IAM change detected in %s, reloading configuration", dir) - if err := s3a.iam.LoadS3ApiConfigurationFromCredentialManager(); err != nil { - glog.Errorf("failed to reload IAM configuration after change in %s: %v", dir, err) + if err := reloadIamConfig(dir); err != nil { return err } } diff --git a/weed/s3api/auth_credentials_subscribe_test.go b/weed/s3api/auth_credentials_subscribe_test.go new file mode 100644 index 000000000..20046d4ec --- /dev/null +++ b/weed/s3api/auth_credentials_subscribe_test.go @@ -0,0 +1,124 @@ +package s3api + +import ( + "context" + "sync" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/credential" + _ "github.com/seaweedfs/seaweedfs/weed/credential/memory" + "github.com/seaweedfs/seaweedfs/weed/filer" + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" +) + +func TestOnIamConfigChangeLegacyIdentityDeletionReloadsConfiguration(t *testing.T) { + s3a := newTestS3ApiServerWithMemoryIAM(t, []*iam_pb.Identity{ + { + Name: "anonymous", + Actions: []string{ + "Read:test", + }, + }, + }) + + err := s3a.onIamConfigChange( + filer.IamConfigDirectory, + &filer_pb.Entry{Name: filer.IamIdentityFile}, + nil, + ) + if err != nil { + t.Fatalf("onIamConfigChange returned error for legacy identity deletion: %v", err) + } + + if !hasIdentity(s3a.iam, "anonymous") { + t.Fatalf("expected anonymous identity to remain loaded after legacy identity deletion event") + } +} + +func TestOnIamConfigChangeReloadsOnIamIdentityDirectoryChanges(t *testing.T) { + s3a := newTestS3ApiServerWithMemoryIAM(t, []*iam_pb.Identity{ + {Name: "anonymous"}, + }) + + // Seed initial in-memory IAM state. + if err := s3a.iam.LoadS3ApiConfigurationFromCredentialManager(); err != nil { + t.Fatalf("failed to load initial IAM configuration: %v", err) + } + if hasIdentity(s3a.iam, "alice") { + t.Fatalf("did not expect alice identity before creating user") + } + + if err := s3a.iam.credentialManager.CreateUser(context.Background(), &iam_pb.Identity{Name: "alice"}); err != nil { + t.Fatalf("failed to create alice in memory credential manager: %v", err) + } + + err := s3a.onIamConfigChange( + filer.IamConfigDirectory+"/identities", + nil, + &filer_pb.Entry{Name: "alice.json"}, + ) + if err != nil { + t.Fatalf("onIamConfigChange returned error for identities directory update: %v", err) + } + + if !hasIdentity(s3a.iam, "alice") { + t.Fatalf("expected alice identity to be loaded after /etc/iam/identities update") + } +} + +func newTestS3ApiServerWithMemoryIAM(t *testing.T, identities []*iam_pb.Identity) *S3ApiServer { + t.Helper() + + // Create S3ApiConfiguration for test with provided identities + config := &iam_pb.S3ApiConfiguration{ + Identities: identities, + Accounts: []*iam_pb.Account{}, + ServiceAccounts: []*iam_pb.ServiceAccount{}, + } + + // Create memory credential manager + cm, err := credential.NewCredentialManager(credential.StoreTypeMemory, nil, "") + if err != nil { + t.Fatalf("failed to create memory credential manager: %v", err) + } + + // Save test configuration + if err := cm.SaveConfiguration(context.Background(), config); err != nil { + t.Fatalf("failed to save test configuration: %v", err) + } + + // Create a test IAM instance + iam := &IdentityAccessManagement{ + m: sync.RWMutex{}, + nameToIdentity: make(map[string]*Identity), + accessKeyIdent: make(map[string]*Identity), + identities: []*Identity{}, + policies: make(map[string]*iam_pb.Policy), + accounts: make(map[string]*Account), + emailAccount: make(map[string]*Account), + hashes: make(map[string]*sync.Pool), + hashCounters: make(map[string]*int32), + isAuthEnabled: false, + stopChan: make(chan struct{}), + useStaticConfig: false, + credentialManager: cm, + } + + // Load test configuration + if err := iam.ReplaceS3ApiConfiguration(config); err != nil { + t.Fatalf("failed to load test configuration: %v", err) + } + + return &S3ApiServer{ + iam: iam, + } +} + +func hasIdentity(iam *IdentityAccessManagement, identityName string) bool { + iam.m.RLock() + defer iam.m.RUnlock() + + _, ok := iam.nameToIdentity[identityName] + return ok +} diff --git a/weed/s3api/auth_credentials_test.go b/weed/s3api/auth_credentials_test.go index 92cad7812..57e05913d 100644 --- a/weed/s3api/auth_credentials_test.go +++ b/weed/s3api/auth_credentials_test.go @@ -427,6 +427,13 @@ func TestNewIdentityAccessManagementWithStoreEnvVars(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + // Reset the memory store to avoid test pollution + if store := credential.Stores[0]; store.GetName() == credential.StoreTypeMemory { + if memStore, ok := store.(interface{ Reset() }); ok { + memStore.Reset() + } + } + // Set up environment variables if tt.accessKeyId != "" { os.Setenv("AWS_ACCESS_KEY_ID", tt.accessKeyId) @@ -467,6 +474,13 @@ func TestNewIdentityAccessManagementWithStoreEnvVars(t *testing.T) { // but contains no identities (e.g., only KMS settings), environment variables should still work. // This test validates the fix for issue #7311. func TestConfigFileWithNoIdentitiesAllowsEnvVars(t *testing.T) { + // Reset the memory store to avoid test pollution + if store := credential.Stores[0]; store.GetName() == credential.StoreTypeMemory { + if memStore, ok := store.(interface{ Reset() }); ok { + memStore.Reset() + } + } + // Set environment variables testAccessKey := "AKIATEST1234567890AB" testSecretKey := "testSecret1234567890123456789012345678901234" diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index 2b1f450b8..6defdbf10 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -254,7 +254,13 @@ func NewS3ApiServerWithStore(router *mux.Router, option *S3ApiServerOption, expl return nil, fmt.Errorf("failed to initialize SSE-S3 key manager: %w", err) } - go s3ApiServer.subscribeMetaEvents("s3", startTsNs, filer.DirectoryEtcRoot, []string{option.BucketsPath}) + go s3ApiServer.subscribeMetaEvents("s3", startTsNs, filer.DirectoryEtcRoot, []string{ + option.BucketsPath, + filer.IamConfigDirectory, + filer.IamConfigDirectory + "/identities", + filer.IamConfigDirectory + "/policies", + filer.IamConfigDirectory + "/service_accounts", + }) // Start bucket size metrics collection in background go s3ApiServer.startBucketSizeMetricsLoop(context.Background())