From 7bbefe6a780f4fc089df7f629c44c36474f71e35 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 13 Nov 2026 02:52:41 -0800 Subject: [PATCH] credential: test changes landing between the IAM load and its migration Co-Authored-By: Claude Opus 5.5 --- .../filer_etc/filer_etc_identity_test.go | 97 +++++++++++++++++++ .../filer_etc/filer_etc_policy_test.go | 21 +++- 2 files changed, 117 insertions(+), 1 deletion(-) diff --git a/weed/credential/filer_etc/filer_etc_identity_test.go b/weed/credential/filer_etc/filer_etc_identity_test.go index 27d182c234..6e5ce74723 100644 --- a/weed/credential/filer_etc/filer_etc_identity_test.go +++ b/weed/credential/filer_etc/filer_etc_identity_test.go @@ -2,10 +2,13 @@ package filer_etc import ( "context" + "encoding/json" + "sync/atomic" "testing" "github.com/seaweedfs/seaweedfs/weed/filer" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -142,3 +145,97 @@ func TestLoadConfigurationSucceedsOnValidFiles(t *testing.T) { } assert.ElementsMatch(t, []string{"alice", "bob"}, names) } + +// changeAfterLoad runs change once, at the first filer lookup after the load +// read the legacy file, so it lands before the migration writes anything. +func changeAfterLoad(server *policyTestFilerServer, change func()) { + var lookups atomic.Int32 + server.beforeLookup = func(context.Context, string, string) error { + if lookups.Add(1) == 2 { + change() + } + return nil + } +} + +func setEntry(server *policyTestFilerServer, dir, name string, content []byte) { + _, _ = server.CreateEntry(context.Background(), &filer_pb.CreateEntryRequest{Directory: dir, Entry: &filer_pb.Entry{Name: name, Content: content, Attributes: &filer_pb.FuseAttributes{}}}) +} + +func storedIdentity(t *testing.T, server *policyTestFilerServer, name string) *iam_pb.Identity { + t.Helper() + server.mu.RLock() + entry, found := server.entries[filerEntryKey(filer.IamConfigDirectory+"/"+IamIdentitiesDirectory, name+".json")] + server.mu.RUnlock() + if !found { + return nil + } + identity := &iam_pb.Identity{} + require.NoError(t, json.Unmarshal(entry.Content, identity)) + return identity +} + +func storedFile(server *policyTestFilerServer, dir, name string) bool { + server.mu.RLock() + defer server.mu.RUnlock() + _, found := server.entries[filerEntryKey(dir, name)] + return found +} + +// The legacy-file migration runs after the load read every IAM file. A +// revoke that lands in between must not be undone by the migration. +func TestMigrationKeepsChangesMadeAfterTheLoad(t *testing.T) { + ctx := context.Background() + identDir := filer.IamConfigDirectory + "/" + IamIdentitiesDirectory + granted := []byte(`{"name":"anonymous","actions":["Read:b"]}`) + revoked := []byte(`{"name":"anonymous","actions":["Read:c"]}`) + legacy := func(identities string) []byte { return []byte(`{"identities":[` + identities + `]}`) } + + t.Run("rewritten identity file", func(t *testing.T) { + store, server := newPolicyTestStoreWithServer(t) + setEntry(server, filer.IamConfigDirectory, IamLegacyIdentityFile, legacy(string(granted))) + setEntry(server, identDir, "anonymous.json", granted) + changeAfterLoad(server, func() { setEntry(server, identDir, "anonymous.json", revoked) }) + + _, err := store.LoadConfiguration(ctx) + require.NoError(t, err) + assert.Equal(t, []string{"Read:c"}, storedIdentity(t, server, "anonymous").GetActions()) + assert.True(t, storedFile(server, filer.IamConfigDirectory, IamLegacyIdentityOldFile)) + }) + + t.Run("removed identity file", func(t *testing.T) { + store, server := newPolicyTestStoreWithServer(t) + setEntry(server, filer.IamConfigDirectory, IamLegacyIdentityFile, legacy(`{"name":"alice"}`)) + setEntry(server, identDir, "bob.json", []byte(`{"name":"bob"}`)) + changeAfterLoad(server, func() { + _, _ = server.DeleteEntry(ctx, &filer_pb.DeleteEntryRequest{Directory: identDir, Name: "bob.json"}) + }) + + _, err := store.LoadConfiguration(ctx) + require.NoError(t, err) + assert.Nil(t, storedIdentity(t, server, "bob")) + assert.NotNil(t, storedIdentity(t, server, "alice")) + }) + + t.Run("rewritten legacy file", func(t *testing.T) { + store, server := newPolicyTestStoreWithServer(t) + setEntry(server, filer.IamConfigDirectory, IamLegacyIdentityFile, legacy(string(granted))) + changeAfterLoad(server, func() { + setEntry(server, filer.IamConfigDirectory, IamLegacyIdentityFile, legacy(string(revoked))) + }) + + _, err := store.LoadConfiguration(ctx) + require.NoError(t, err) + if identity := storedIdentity(t, server, "anonymous"); identity != nil { + assert.Equal(t, []string{"Read:c"}, identity.GetActions()) + } + + s3cfg, err := store.LoadConfiguration(ctx) + require.NoError(t, err) + require.Len(t, s3cfg.Identities, 1) + assert.Equal(t, []string{"Read:c"}, s3cfg.Identities[0].GetActions()) + assert.Equal(t, []string{"Read:c"}, storedIdentity(t, server, "anonymous").GetActions()) + assert.False(t, storedFile(server, filer.IamConfigDirectory, IamLegacyIdentityFile)) + assert.True(t, storedFile(server, filer.IamConfigDirectory, IamLegacyIdentityOldFile)) + }) +} diff --git a/weed/credential/filer_etc/filer_etc_policy_test.go b/weed/credential/filer_etc/filer_etc_policy_test.go index 47b034ebc9..ace971bc8f 100644 --- a/weed/credential/filer_etc/filer_etc_policy_test.go +++ b/weed/credential/filer_etc/filer_etc_policy_test.go @@ -96,10 +96,29 @@ func (s *policyTestFilerServer) CreateEntry(_ context.Context, req *filer_pb.Cre s.mu.Lock() defer s.mu.Unlock() - s.entries[filerEntryKey(req.Directory, req.Entry.Name)] = cloneEntry(req.Entry) + key := filerEntryKey(req.Directory, req.Entry.Name) + if _, found := s.entries[key]; found && req.OExcl { + return &filer_pb.CreateEntryResponse{Error: filer_pb.ErrEntryAlreadyExists.Error(), ErrorCode: filer_pb.FilerError_ENTRY_ALREADY_EXISTS}, nil + } + s.entries[key] = cloneEntry(req.Entry) return &filer_pb.CreateEntryResponse{}, nil } +func (s *policyTestFilerServer) AtomicRenameEntry(_ context.Context, req *filer_pb.AtomicRenameEntryRequest) (*filer_pb.AtomicRenameEntryResponse, error) { + s.mu.Lock() + defer s.mu.Unlock() + + oldKey := filerEntryKey(req.OldDirectory, req.OldName) + entry, found := s.entries[oldKey] + if !found { + return nil, status.Error(codes.NotFound, filer_pb.ErrNotFound.Error()) + } + delete(s.entries, oldKey) + entry.Name = req.NewName + s.entries[filerEntryKey(req.NewDirectory, req.NewName)] = entry + return &filer_pb.AtomicRenameEntryResponse{}, nil +} + func (s *policyTestFilerServer) UpdateEntry(_ context.Context, req *filer_pb.UpdateEntryRequest) (*filer_pb.UpdateEntryResponse, error) { s.mu.RLock() beforeUpdate := s.beforeUpdate