Fix IAM identity loss on S3 restart migration (#8343)
* Fix IAM reload after legacy config migration Handle legacy identity.json metadata events by reloading from the credential manager instead of parsing event content, and watch the correct /etc/iam multi-file directories so identity changes are applied. Add regression tests for legacy deletion and /etc/iam/identities change events. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix auth_credentials_subscribe_test helper to not pollute global memory store The SaveConfiguration call was affecting other tests. Use local credential manager and ReplaceS3ApiConfiguration instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix IAM event watching: subscribe to IAM directories and improve directory matching - Add /etc/iam and its subdirectories (identities, policies, service_accounts) to directoriesToWatch - Fix directory matching to avoid false positives from sibling directories - Use exact match or prefix with trailing slash instead of plain HasPrefix - Prevents matching hypothetical /etc/iam/identities_backup directory This ensures IAM config change events are actually delivered to the handler. * fix tests --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
|
||||
// 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)
|
||||
// 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
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// Handle create/update
|
||||
if newEntry != nil && newEntry.Name == filer.IamIdentityFile {
|
||||
if err := s3a.iam.LoadS3ApiConfigurationFromBytes(newEntry.Content); 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
|
||||
}
|
||||
}
|
||||
|
||||
124
weed/s3api/auth_credentials_subscribe_test.go
Normal file
124
weed/s3api/auth_credentials_subscribe_test.go
Normal file
@@ -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
|
||||
}
|
||||
@@ -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"
|
||||
|
||||
@@ -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())
|
||||
|
||||
Reference in New Issue
Block a user