From defc464d199ac26c2d75324774ab97635764c6f0 Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Tue, 5 Dec 2023 18:53:20 +0100 Subject: [PATCH 01/14] refactoring --- pkg/aclmanager/aclmanager.go | 111 ++++++++++++++---------------- pkg/aclmanager/aclmanager_test.go | 110 +++++++++++++++-------------- 2 files changed, 111 insertions(+), 110 deletions(-) diff --git a/pkg/aclmanager/aclmanager.go b/pkg/aclmanager/aclmanager.go index 993297a..f3ae00c 100644 --- a/pkg/aclmanager/aclmanager.go +++ b/pkg/aclmanager/aclmanager.go @@ -9,6 +9,7 @@ import ( "log/slog" "regexp" "strings" + "sync/atomic" "time" ) @@ -31,6 +32,8 @@ type AclManager struct { Username string Password string RedisClient *redis.Client + primary atomic.Bool + nodes map[string]int } // New creates a new AclManager @@ -48,15 +51,19 @@ func New(addr string, username string, password string) *AclManager { } } -type NodeInfo struct { - Address string - Function int -} +// findNodes returns a list of nodes in the cluster based on the redis info replication command +func (a *AclManager) findNodes(ctx context.Context) (err error) { + slog.Debug("Finding nodes") + replicationInfo, err := a.RedisClient.Info(ctx, "replication").Result() + if err != nil { + return err + } -func parseRedisOutput(output string) (nodes []NodeInfo, err error) { - var masterHost, masterPort string + a.primary.Store(role.MatchString(replicationInfo)) - scanner := bufio.NewScanner(strings.NewReader(output)) + var masterHost, masterPort string + var nodes []string + scanner := bufio.NewScanner(strings.NewReader(replicationInfo)) for scanner.Scan() { line := scanner.Text() @@ -68,43 +75,27 @@ func parseRedisOutput(output string) (nodes []NodeInfo, err error) { slog.Debug("Parsing line looking for Follower", "content", line) if matches := primaryPortRegex.FindStringSubmatch(line); matches != nil { masterPort = matches[1] - nodes = append(nodes, NodeInfo{Address: fmt.Sprintf("%s:%s", masterHost, masterPort), Function: Primary}) + nodes = append(nodes, fmt.Sprintf("%s:%s", masterHost, masterPort)) } if matches := followerRegex.FindStringSubmatch(line); matches != nil { ip := matches[followerRegex.SubexpIndex("ip")] port := matches[followerRegex.SubexpIndex("port")] - nodes = append(nodes, NodeInfo{Address: fmt.Sprintf("%s:%s", ip, port), Function: Follower}) + nodes = append(nodes, fmt.Sprintf("%s:%s", ip, port)) } } if err := scanner.Err(); err != nil { - return nodes, err - } - - return nodes, err -} - -// FindNodes returns a list of nodes in the cluster based on the redis info replication command -func (a *AclManager) FindNodes() (nodes []NodeInfo, err error) { - slog.Debug("Finding nodes") - replicationInfo, err := a.RedisClient.Info(context.Background(), "replication").Result() - if err != nil { - return nodes, err - } - - nodes, err = parseRedisOutput(replicationInfo) - if err != nil { - return nodes, err + return err } - return nodes, err + return err } // CurrentFunction check if the current node is the Primary node -func (a *AclManager) CurrentFunction() (function int, err error) { +func (a *AclManager) CurrentFunction(ctx context.Context) (function int, err error) { slog.Debug("Check node current function") - replicationInfo, err := a.RedisClient.Info(context.Background(), "replication").Result() + replicationInfo, err := a.RedisClient.Info(ctx, "replication").Result() if err != nil { return function, err } @@ -116,35 +107,33 @@ func (a *AclManager) CurrentFunction() (function int, err error) { return Follower, err } -// SyncAcls connects to master node and syncs the acls to the current node -func (a *AclManager) SyncAcls() (err error) { - slog.Debug("Syncing acls") - nodes, err := a.FindNodes() +func (a *AclManager) Primary(ctx context.Context) (primary *AclManager, err error) { + err = a.findNodes(ctx) if err != nil { - return err + return nil, err } - ctx := context.Background() - for _, node := range nodes { - if node.Function == Primary { - if a.Addr == node.Address { - return err - } - master := redis.NewClient(&redis.Options{ - Addr: node.Address, - Username: a.Username, - Password: a.Password, - }) - defer master.Close() - - added, deleted, err := mirrorAcls(ctx, master, a.RedisClient) - if err != nil { - return fmt.Errorf("error syncing acls: %v", err) - } - slog.Info("Synced acls", "added", added, "deleted", deleted) + for address, function := range a.nodes { + if function == Primary { + return New(address, a.Username, a.Username), err } } + return nil, err +} + +// SyncAcls connects to master node and syncs the acls to the current node +func (a *AclManager) SyncAcls(ctx context.Context, primary *AclManager) (err error) { + slog.Debug("Syncing acls") + if primary == nil { + return fmt.Errorf("no primary found") + } + added, deleted, err := mirrorAcls(ctx, primary.RedisClient, a.RedisClient) + if err != nil { + return fmt.Errorf("error syncing acls: %v", err) + } + slog.Info("Synced acls", "added", added, "deleted", deleted) + return err } @@ -242,22 +231,28 @@ func (a *AclManager) Loop(ctx context.Context) (err error) { ticker := time.NewTicker(viper.GetDuration("syncInterval") * time.Second) defer ticker.Stop() + var primary *AclManager for { select { case <-ctx.Done(): return err case <-ticker.C: - function, e := a.CurrentFunction() + function, e := a.CurrentFunction(ctx) if err != nil { - slog.Warn("unable to check if it's a primary", "message", e) - err = fmt.Errorf("unable to check if it's a primary: %w", e) + slog.Warn("unable to check if it's a Primary", "message", e) + err = fmt.Errorf("unable to check if it's a Primary: %w", e) } if function == Follower { - e = a.SyncAcls() + primary, err = a.Primary(ctx) + if err != nil { + slog.Warn("unable to find Primary", "message", e) + continue + } + e = a.SyncAcls(ctx, primary) if err != nil { - slog.Warn("unable to sync acls from primary", "message", e) + slog.Warn("unable to sync acls from Primary", "message", e) } - err = fmt.Errorf("unable to sync acls from primary: %w", e) + err = fmt.Errorf("unable to sync acls from Primary: %w", e) } } } diff --git a/pkg/aclmanager/aclmanager_test.go b/pkg/aclmanager/aclmanager_test.go index c0f8fc6..805e6ae 100644 --- a/pkg/aclmanager/aclmanager_test.go +++ b/pkg/aclmanager/aclmanager_test.go @@ -50,34 +50,33 @@ repl_backlog_first_byte_offset:1 repl_backlog_histlen:434` ) +type NodeInfo struct { + Address string + Function int +} + func TestFindNodes(t *testing.T) { // Sample master and slave output for testing tests := []struct { name string mockResp string - want []NodeInfo + want map[string]int wantErr bool }{ { name: "parse master output", mockResp: primaryOutput, - want: []NodeInfo{ - { - Address: "172.21.0.3:6379", - Function: Follower, - }, + want: map[string]int{ + "172.21.0.3:6379": Follower, }, wantErr: false, }, { name: "parse Follower output", mockResp: followerOutput, - want: []NodeInfo{ - { - Address: "172.21.0.2:6379", - Function: Primary, - }, + want: map[string]int{ + "172.21.0.2:6379": Primary, }, wantErr: false, }, @@ -100,13 +99,21 @@ func TestFindNodes(t *testing.T) { mock.ExpectInfo("replication").SetVal(tt.mockResp) } aclManager := AclManager{RedisClient: redisClient} + ctx := context.Background() - nodes, err := aclManager.FindNodes() + err := aclManager.findNodes(ctx) if (err != nil) != tt.wantErr { - t.Errorf("FindNodes() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("findNodes() error = %v, wantErr %v", err, tt.wantErr) return } - assert.Equal(t, tt.want, nodes) + for address, function := range aclManager.nodes { + if wantFunction, ok := tt.want[address]; ok { + if wantFunction != function { + t.Errorf("findNodes() wanted function %v not found", function) + } + } + t.Errorf("findNodes() wanted address %v not found", address) + } }) } } @@ -312,10 +319,10 @@ func TestIsItPrimary(t *testing.T) { // Mocking the response for the Info function mock.ExpectInfo("replication").SetVal(tt.mockResp) aclManager := AclManager{RedisClient: redisClient} - - nodes, err := aclManager.CurrentFunction() + ctx := context.Background() + nodes, err := aclManager.CurrentFunction(ctx) if (err != nil) != tt.wantErr { - t.Errorf("FindNodes() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("findNodes() error = %v, wantErr %v", err, tt.wantErr) return } @@ -356,8 +363,9 @@ func TestCurrentFunction_Error(t *testing.T) { // Mocking the response for the Info function mock.ExpectInfo("replication").SetErr(fmt.Errorf("error")) aclManager := AclManager{RedisClient: redisClient} + ctx := context.Background() - _, err := aclManager.CurrentFunction() + _, err := aclManager.CurrentFunction(ctx) assert.Error(t, err) } @@ -370,7 +378,7 @@ func TestAclManager_Loop(t *testing.T) { expectError error }{ { - name: "primary node", + name: "Primary node", aclManager: &AclManager{ Addr: "localhost:6379", Password: "password", @@ -379,7 +387,7 @@ func TestAclManager_Loop(t *testing.T) { wantErr: false, }, { - name: "primary node with error", + name: "Primary node with error", aclManager: &AclManager{ Addr: "localhost:6379", Password: "password", @@ -396,19 +404,18 @@ func TestAclManager_Loop(t *testing.T) { Username: "username", }, wantErr: true, - expectError: fmt.Errorf("unable to check if it's a primary"), + expectError: fmt.Errorf("unable to check if it's a Primary"), + }, + { + name: "follower node", + aclManager: &AclManager{ + Addr: "localhost:6379", + Password: "password", + Username: "username", + }, + wantErr: false, + expectError: nil, }, - // TODO: refactor to be able to test this case - //{ - // name: "follower node", - // aclManager: &AclManager{ - // Addr: "localhost:6379", - // Password: "password", - // Username: "username", - // }, - // wantErr: false, - // expectError: fmt.Errorf("error syncing acls"), - //}, } for _, tt := range tests { @@ -417,7 +424,7 @@ func TestAclManager_Loop(t *testing.T) { tt.aclManager.RedisClient = redisClient if tt.wantErr { - if tt.name == "primary node" { + if tt.name == "Primary node" { mock.ExpectInfo("replication").SetErr(fmt.Errorf("error")) mock.ExpectInfo("replication").SetErr(fmt.Errorf("error")) mock.ExpectInfo("replication").SetErr(fmt.Errorf("error")) @@ -438,7 +445,7 @@ func TestAclManager_Loop(t *testing.T) { mock.ExpectInfo("replication").SetVal(followerOutput) } } else { - if tt.name == "primary node" { + if tt.name == "Primary node" { mock.ExpectInfo("replication").SetVal(primaryOutput) mock.ExpectInfo("replication").SetVal(primaryOutput) mock.ExpectInfo("replication").SetVal(primaryOutput) @@ -446,7 +453,6 @@ func TestAclManager_Loop(t *testing.T) { mock.ExpectInfo("replication").SetVal(primaryOutput) mock.ExpectInfo("replication").SetVal(primaryOutput) mock.ExpectInfo("replication").SetVal(primaryOutput) - } } @@ -492,20 +498,20 @@ func TestClosePanic(t *testing.T) { assert.Panics(t, func() { aclManager.Close() }) } -func BenchmarkParseRedisOutputFollower(b *testing.B) { - for i := 0; i < b.N; i++ { - _, err := parseRedisOutput(followerOutput) - if err != nil { - b.Fatal(err) - } - } -} - -func BenchmarkParseRedisOutputMaster(b *testing.B) { - for i := 0; i < b.N; i++ { - _, err := parseRedisOutput(primaryOutput) - if err != nil { - b.Fatal(err) - } - } -} +//func BenchmarkParseRedisOutputFollower(b *testing.B) { +// for i := 0; i < b.N; i++ { +// _, err := parseRedisOutput(followerOutput) +// if err != nil { +// b.Fatal(err) +// } +// } +//} +// +//func BenchmarkParseRedisOutputMaster(b *testing.B) { +// for i := 0; i < b.N; i++ { +// _, err := parseRedisOutput(primaryOutput) +// if err != nil { +// b.Fatal(err) +// } +// } +//} From 6649d7d98c3bc2e6a4042845dd6a330db19ceae1 Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Tue, 5 Dec 2023 18:55:19 +0100 Subject: [PATCH 02/14] cleanup nodes --- pkg/aclmanager/aclmanager.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/aclmanager/aclmanager.go b/pkg/aclmanager/aclmanager.go index f3ae00c..520f503 100644 --- a/pkg/aclmanager/aclmanager.go +++ b/pkg/aclmanager/aclmanager.go @@ -85,6 +85,12 @@ func (a *AclManager) findNodes(ctx context.Context) (err error) { } } + for _, node := range nodes { + if _, ok := a.nodes[node]; !ok { + delete(a.nodes, node) + } + } + if err := scanner.Err(); err != nil { return err } From bab6b8cb4e99966265529ffbdb10c9ff558a2983 Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Tue, 5 Dec 2023 19:11:35 +0100 Subject: [PATCH 03/14] update runOnce --- cmd/root.go | 2 +- cmd/run.go | 2 +- cmd/runOnce.go | 28 ++++++++++++++++++++++++---- pkg/aclmanager/aclmanager.go | 18 ++++++++++-------- pkg/aclmanager/aclmanager_test.go | 5 ----- 5 files changed, 36 insertions(+), 19 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index beae394..8299082 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -48,7 +48,7 @@ var logger = slog.New( // rootCmd represents the base command when called without any subcommands var rootCmd = &cobra.Command{ Use: "bedel", - Short: "Small utility to sync redis acls with a master instance", + Short: "Small utility to sync redis acls with a primary instance", } // Execute adds all child commands to the root command and sets flags appropriately. diff --git a/cmd/run.go b/cmd/run.go index a456adc..5e248df 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -24,7 +24,7 @@ import ( // runCmd represents the run command var runCmd = &cobra.Command{ Use: "run", - Short: "Run the acl manager in mood loop, it will sync the follower with the master", + Short: "Run the acl manager in mood loop, it will sync the follower with the primary", Run: func(cmd *cobra.Command, args []string) { mgr := aclmanager.New(viper.GetString("address"), viper.GetString("username"), viper.GetString("password")) ctx := cmd.Context() diff --git a/cmd/runOnce.go b/cmd/runOnce.go index 3f133f8..a62ca6f 100644 --- a/cmd/runOnce.go +++ b/cmd/runOnce.go @@ -18,6 +18,8 @@ package cmd import ( "github.com/ncode/bedel/pkg/aclmanager" "github.com/spf13/viper" + "log/slog" + "os" "github.com/spf13/cobra" ) @@ -25,12 +27,30 @@ import ( // runOnceCmd represents the runOnce command var runOnceCmd = &cobra.Command{ Use: "runOnce", - Short: "Run the acl manager once, it will sync the follower with the master", + Short: "Run the acl manager once, it will sync the follower with the primary", Run: func(cmd *cobra.Command, args []string) { - mgr := aclmanager.New(viper.GetString("address"), viper.GetString("username"), viper.GetString("password")) - err := mgr.SyncAcls() + ctx := cmd.Context() + aclManager := aclmanager.New(viper.GetString("address"), viper.GetString("username"), viper.GetString("password")) + function, err := aclManager.CurrentFunction(ctx) if err != nil { - panic(err) + slog.Warn("unable to check if it's a Primary", "message", err) + os.Exit(1) + } + if function == aclmanager.Follower { + primary, err := aclManager.Primary(ctx) + if err != nil { + slog.Warn("unable to find Primary", "message", err) + os.Exit(1) + } + var added, deleted []string + added, deleted, err = aclManager.SyncAcls(ctx, primary) + if err != nil { + slog.Warn("unable to sync acls from Primary", "message", err) + os.Exit(1) + } + slog.Info("Synced acls from Primary", "added", added, "deleted", deleted) + } else { + slog.Info("Not a follower, nothing to do") } }, } diff --git a/pkg/aclmanager/aclmanager.go b/pkg/aclmanager/aclmanager.go index 520f503..2692a61 100644 --- a/pkg/aclmanager/aclmanager.go +++ b/pkg/aclmanager/aclmanager.go @@ -129,18 +129,17 @@ func (a *AclManager) Primary(ctx context.Context) (primary *AclManager, err erro } // SyncAcls connects to master node and syncs the acls to the current node -func (a *AclManager) SyncAcls(ctx context.Context, primary *AclManager) (err error) { +func (a *AclManager) SyncAcls(ctx context.Context, primary *AclManager) (added []string, deleted []string, err error) { slog.Debug("Syncing acls") if primary == nil { - return fmt.Errorf("no primary found") + return added, deleted, fmt.Errorf("no primary found") } - added, deleted, err := mirrorAcls(ctx, primary.RedisClient, a.RedisClient) + added, deleted, err = mirrorAcls(ctx, primary.RedisClient, a.RedisClient) if err != nil { - return fmt.Errorf("error syncing acls: %v", err) + return added, deleted, fmt.Errorf("error syncing acls: %v", err) } - slog.Info("Synced acls", "added", added, "deleted", deleted) - return err + return added, deleted, err } // Close closes the redis client @@ -254,11 +253,14 @@ func (a *AclManager) Loop(ctx context.Context) (err error) { slog.Warn("unable to find Primary", "message", e) continue } - e = a.SyncAcls(ctx, primary) + var added, deleted []string + added, deleted, err = a.SyncAcls(ctx, primary) if err != nil { slog.Warn("unable to sync acls from Primary", "message", e) + err = fmt.Errorf("unable to sync acls from Primary: %w", e) + continue } - err = fmt.Errorf("unable to sync acls from Primary: %w", e) + slog.Info("Synced acls from Primary", "added", added, "deleted", deleted) } } } diff --git a/pkg/aclmanager/aclmanager_test.go b/pkg/aclmanager/aclmanager_test.go index 805e6ae..e88bfba 100644 --- a/pkg/aclmanager/aclmanager_test.go +++ b/pkg/aclmanager/aclmanager_test.go @@ -50,11 +50,6 @@ repl_backlog_first_byte_offset:1 repl_backlog_histlen:434` ) -type NodeInfo struct { - Address string - Function int -} - func TestFindNodes(t *testing.T) { // Sample master and slave output for testing From 30d0addad02928abf6e8174bd7570c64872d1898 Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Tue, 5 Dec 2023 19:33:06 +0100 Subject: [PATCH 04/14] fix tests --- pkg/aclmanager/aclmanager_test.go | 83 +++++++++++++++++++------------ 1 file changed, 50 insertions(+), 33 deletions(-) diff --git a/pkg/aclmanager/aclmanager_test.go b/pkg/aclmanager/aclmanager_test.go index e88bfba..ca99aa5 100644 --- a/pkg/aclmanager/aclmanager_test.go +++ b/pkg/aclmanager/aclmanager_test.go @@ -190,14 +190,15 @@ func TestListAcls_Error(t *testing.T) { func TestMirrorAcls(t *testing.T) { tests := []struct { - name string - sourceAcls []interface{} - destinationAcls []interface{} - expectedDeleted []string - expectedAdded []string - listAclsError error - redisDoError error - wantErr bool + name string + sourceAcls []interface{} + destinationAcls []interface{} + expectedDeleted []string + expectedAdded []string + listAclsError error + redisDoError error + wantSourceErr bool + wantDestinationErr bool }{ { name: "ACLs synced with deletions", @@ -205,33 +206,34 @@ func TestMirrorAcls(t *testing.T) { destinationAcls: []interface{}{"user acl1", "user acl3"}, expectedDeleted: []string{"acl3"}, expectedAdded: []string{"acl2"}, - wantErr: false, + wantSourceErr: false, }, { - name: "ACLs synced with Error om SETUSER", - sourceAcls: []interface{}{"user acl1", "user acl2"}, - destinationAcls: []interface{}{"user acl1", "user acl3"}, - redisDoError: fmt.Errorf("DELUSER"), - wantErr: true, + name: "ACLs synced with Error om SETUSER", + sourceAcls: []interface{}{"user acl1", "user acl2"}, + destinationAcls: []interface{}{"user acl1", "user acl3"}, + redisDoError: fmt.Errorf("DELUSER"), + wantDestinationErr: true, }, { - name: "ACLs synced with Error on SETUSER", - sourceAcls: []interface{}{"user acl1", "user acl2"}, - destinationAcls: []interface{}{"user acl1", "user acl3"}, - redisDoError: fmt.Errorf("SETUSER"), - wantErr: true, + name: "ACLs synced with Error on SETUSER", + sourceAcls: []interface{}{"user acl1", "user acl2"}, + destinationAcls: []interface{}{"user acl1", "user acl3"}, + redisDoError: fmt.Errorf("SETUSER"), + wantSourceErr: false, + wantDestinationErr: true, }, { name: "No ACLs to delete", sourceAcls: []interface{}{"user acl1", "user acl2"}, destinationAcls: []interface{}{"user acl1", "user acl2"}, expectedDeleted: nil, - wantErr: false, + wantSourceErr: false, }, { name: "Error listing source ACLs", listAclsError: fmt.Errorf("error listing source ACLs"), - wantErr: true, + wantSourceErr: true, }, } @@ -240,39 +242,54 @@ func TestMirrorAcls(t *testing.T) { sourceClient, sourceMock := redismock.NewClientMock() destinationClient, destMock := redismock.NewClientMock() - if tt.listAclsError != nil { + if tt.listAclsError != nil && tt.wantSourceErr { sourceMock.ExpectDo("ACL", "LIST").SetErr(tt.listAclsError) } else { sourceMock.ExpectDo("ACL", "LIST").SetVal(tt.sourceAcls) } - if tt.listAclsError != nil { + if tt.listAclsError != nil && tt.wantDestinationErr { destMock.ExpectDo("ACL", "LIST").SetErr(tt.listAclsError) } else { destMock.ExpectDo("ACL", "LIST").SetVal(tt.destinationAcls) if tt.expectedDeleted != nil { - for _, acl := range tt.expectedDeleted { - if tt.wantErr && tt.redisDoError.Error() == "DELUSER" { - destMock.ExpectDo("ACL", "DELUSER", acl).SetErr(tt.redisDoError) + for _, username := range tt.expectedDeleted { + if tt.wantDestinationErr && tt.redisDoError.Error() == "DELUSER" { + destMock.ExpectDo("ACL", "DELUSER", username).SetErr(tt.redisDoError) continue } - destMock.ExpectDo("ACL", "DELUSER", acl).SetVal("OK") + destMock.ExpectDo("ACL", "DELUSER", username).SetVal("OK") } } if tt.expectedAdded != nil { - for _, acl := range tt.expectedAdded { - if tt.wantErr && tt.redisDoError.Error() == "SETUSER" { - destMock.ExpectDo("ACL", "SETUSER", acl).SetErr(tt.redisDoError) + for _, username := range tt.expectedAdded { + if tt.wantDestinationErr && tt.redisDoError.Error() == "SETUSER" { + destMock.ExpectDo("ACL", "SETUSER", username).SetErr(tt.redisDoError) continue } - destMock.ExpectDo("ACL", "SETUSER", acl).SetVal("OK") + destMock.ExpectDo("ACL", "SETUSER", username).SetVal("OK") } } } added, deleted, err := mirrorAcls(context.Background(), sourceClient, destinationClient) - if (err != nil) != tt.wantErr { - t.Errorf("mirrorAcls() error = %v, wantErr %v", err, tt.wantErr) + if err != nil { + if tt.wantSourceErr { + if tt.listAclsError != nil && !strings.Contains(err.Error(), tt.listAclsError.Error()) { + t.Errorf("mirrorAcls() error = %v, wantErr %v", err, tt.listAclsError) + } + if tt.redisDoError != nil && !strings.Contains(err.Error(), tt.redisDoError.Error()) { + t.Errorf("mirrorAcls() error = %v, wantErr %v", err, tt.redisDoError) + } + } + if !tt.wantDestinationErr { + if tt.listAclsError != nil && !strings.Contains(err.Error(), tt.listAclsError.Error()) { + t.Errorf("mirrorAcls() error = %v, wantErr %v", err, tt.listAclsError) + } + if tt.redisDoError != nil && !strings.Contains(err.Error(), tt.redisDoError.Error()) { + t.Errorf("mirrorAcls() error = %v, wantErr %v", err, tt.redisDoError) + } + } } if !reflect.DeepEqual(deleted, tt.expectedDeleted) { t.Errorf("mirrorAcls() deleted = %v, expectedDeleted %v", deleted, tt.expectedDeleted) From 8223f5b95eada5a5400790801371952f8aa2f9ee Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Tue, 5 Dec 2023 19:36:30 +0100 Subject: [PATCH 05/14] cover list destination acl errors --- pkg/aclmanager/aclmanager_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/aclmanager/aclmanager_test.go b/pkg/aclmanager/aclmanager_test.go index ca99aa5..332f823 100644 --- a/pkg/aclmanager/aclmanager_test.go +++ b/pkg/aclmanager/aclmanager_test.go @@ -235,6 +235,11 @@ func TestMirrorAcls(t *testing.T) { listAclsError: fmt.Errorf("error listing source ACLs"), wantSourceErr: true, }, + { + name: "Error listing destination ACLs", + listAclsError: fmt.Errorf("error listing destination ACLs"), + wantDestinationErr: true, + }, } for _, tt := range tests { From 81fea6db6d9a0b261d3a2782c4c9db4e72dde45e Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Tue, 5 Dec 2023 21:58:04 +0100 Subject: [PATCH 06/14] test primary --- pkg/aclmanager/aclmanager.go | 10 ++-- pkg/aclmanager/aclmanager_test.go | 82 ++++++++++++++++++++++++++++--- 2 files changed, 81 insertions(+), 11 deletions(-) diff --git a/pkg/aclmanager/aclmanager.go b/pkg/aclmanager/aclmanager.go index 2692a61..f0911da 100644 --- a/pkg/aclmanager/aclmanager.go +++ b/pkg/aclmanager/aclmanager.go @@ -16,6 +16,7 @@ import ( const ( Primary = iota Follower + Unknown ) var ( @@ -101,13 +102,12 @@ func (a *AclManager) findNodes(ctx context.Context) (err error) { // CurrentFunction check if the current node is the Primary node func (a *AclManager) CurrentFunction(ctx context.Context) (function int, err error) { slog.Debug("Check node current function") - replicationInfo, err := a.RedisClient.Info(ctx, "replication").Result() + err = a.findNodes(ctx) if err != nil { - return function, err + return Unknown, err } - - if role.MatchString(replicationInfo) { - return Primary, nil + if a.primary.Load() { + return Primary, err } return Follower, err diff --git a/pkg/aclmanager/aclmanager_test.go b/pkg/aclmanager/aclmanager_test.go index 332f823..86371a4 100644 --- a/pkg/aclmanager/aclmanager_test.go +++ b/pkg/aclmanager/aclmanager_test.go @@ -310,10 +310,11 @@ func TestIsItPrimary(t *testing.T) { // Sample Primary and Follower output for testing tests := []struct { - name string - mockResp string - want int - wantErr bool + name string + mockResp string + want int + wantErr bool + RedisExpectInfoError error }{ { name: "parse Primary output", @@ -327,6 +328,13 @@ func TestIsItPrimary(t *testing.T) { want: Follower, wantErr: false, }, + { + name: "parse primary error", + mockResp: primaryOutput, + want: Primary, + wantErr: true, + RedisExpectInfoError: fmt.Errorf("asasas"), + }, } for _, tt := range tests { @@ -334,13 +342,19 @@ func TestIsItPrimary(t *testing.T) { redisClient, mock := redismock.NewClientMock() // Mocking the response for the Info function - mock.ExpectInfo("replication").SetVal(tt.mockResp) + if tt.wantErr { + mock.ExpectInfo("replication").SetErr(tt.RedisExpectInfoError) + } else { + mock.ExpectInfo("replication").SetVal(tt.mockResp) + } aclManager := AclManager{RedisClient: redisClient} ctx := context.Background() nodes, err := aclManager.CurrentFunction(ctx) if (err != nil) != tt.wantErr { + if strings.Contains(err.Error(), tt.RedisExpectInfoError.Error()) { + return + } t.Errorf("findNodes() error = %v, wantErr %v", err, tt.wantErr) - return } assert.Equal(t, tt.want, nodes) @@ -386,6 +400,62 @@ func TestCurrentFunction_Error(t *testing.T) { assert.Error(t, err) } +func TestAclManager_Primary(t *testing.T) { + tests := []struct { + name string + mockResp string + want string + wantErr bool + }{ + { + name: "parse master output", + mockResp: primaryOutput, + wantErr: false, + }, + { + name: "parse Follower output", + mockResp: followerOutput, + want: "172.21.0.2:6379", + wantErr: false, + }, + { + name: "error on replicationInfo", + mockResp: followerOutput, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + redisClient, mock := redismock.NewClientMock() + + // Mocking the response for the Info function + if tt.wantErr { + mock.ExpectInfo("replication").SetErr(fmt.Errorf("error")) + } else { + mock.ExpectInfo("replication").SetVal(tt.mockResp) + } + aclManager := AclManager{RedisClient: redisClient} + ctx := context.Background() + + primary, err := aclManager.Primary(ctx) + + if tt.wantErr { + assert.Error(t, err) + assert.Nil(t, primary) + } else { + assert.NoError(t, err) + if tt.want == "" { + assert.Nil(t, primary) + return + } + assert.NotNil(t, primary) + assert.Equal(t, tt.want, primary.Addr) + } + }) + } +} + func TestAclManager_Loop(t *testing.T) { viper.Set("syncInterval", 4) tests := []struct { From 92acda99b3ae2e8e18db38f101c283cb95b186db Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Tue, 5 Dec 2023 22:20:31 +0100 Subject: [PATCH 07/14] update tests --- pkg/aclmanager/aclmanager.go | 3 +++ pkg/aclmanager/aclmanager_test.go | 24 +++++++++++++----------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/pkg/aclmanager/aclmanager.go b/pkg/aclmanager/aclmanager.go index f0911da..0e5363c 100644 --- a/pkg/aclmanager/aclmanager.go +++ b/pkg/aclmanager/aclmanager.go @@ -49,6 +49,7 @@ func New(addr string, username string, password string) *AclManager { Username: username, Password: password, RedisClient: redisClient, + nodes: make(map[string]int), } } @@ -77,12 +78,14 @@ func (a *AclManager) findNodes(ctx context.Context) (err error) { if matches := primaryPortRegex.FindStringSubmatch(line); matches != nil { masterPort = matches[1] nodes = append(nodes, fmt.Sprintf("%s:%s", masterHost, masterPort)) + a.nodes[fmt.Sprintf("%s:%s", masterHost, masterPort)] = Primary } if matches := followerRegex.FindStringSubmatch(line); matches != nil { ip := matches[followerRegex.SubexpIndex("ip")] port := matches[followerRegex.SubexpIndex("port")] nodes = append(nodes, fmt.Sprintf("%s:%s", ip, port)) + a.nodes[fmt.Sprintf("%s:%s", ip, port)] = Follower } } diff --git a/pkg/aclmanager/aclmanager_test.go b/pkg/aclmanager/aclmanager_test.go index 86371a4..aed8797 100644 --- a/pkg/aclmanager/aclmanager_test.go +++ b/pkg/aclmanager/aclmanager_test.go @@ -93,7 +93,7 @@ func TestFindNodes(t *testing.T) { } else { mock.ExpectInfo("replication").SetVal(tt.mockResp) } - aclManager := AclManager{RedisClient: redisClient} + aclManager := AclManager{RedisClient: redisClient, nodes: make(map[string]int)} ctx := context.Background() err := aclManager.findNodes(ctx) @@ -106,6 +106,7 @@ func TestFindNodes(t *testing.T) { if wantFunction != function { t.Errorf("findNodes() wanted function %v not found", function) } + return } t.Errorf("findNodes() wanted address %v not found", address) } @@ -306,9 +307,7 @@ func TestMirrorAcls(t *testing.T) { } } -func TestIsItPrimary(t *testing.T) { - // Sample Primary and Follower output for testing - +func TestCurrentFunction(t *testing.T) { tests := []struct { name string mockResp string @@ -331,9 +330,9 @@ func TestIsItPrimary(t *testing.T) { { name: "parse primary error", mockResp: primaryOutput, - want: Primary, + want: Unknown, wantErr: true, - RedisExpectInfoError: fmt.Errorf("asasas"), + RedisExpectInfoError: fmt.Errorf("error"), }, } @@ -347,14 +346,14 @@ func TestIsItPrimary(t *testing.T) { } else { mock.ExpectInfo("replication").SetVal(tt.mockResp) } - aclManager := AclManager{RedisClient: redisClient} + aclManager := AclManager{RedisClient: redisClient, nodes: make(map[string]int)} ctx := context.Background() nodes, err := aclManager.CurrentFunction(ctx) if (err != nil) != tt.wantErr { - if strings.Contains(err.Error(), tt.RedisExpectInfoError.Error()) { + if !strings.Contains(err.Error(), tt.RedisExpectInfoError.Error()) { + t.Errorf("findNodes() error = %v, wantErr %v", err, tt.wantErr) return } - t.Errorf("findNodes() error = %v, wantErr %v", err, tt.wantErr) } assert.Equal(t, tt.want, nodes) @@ -435,11 +434,10 @@ func TestAclManager_Primary(t *testing.T) { } else { mock.ExpectInfo("replication").SetVal(tt.mockResp) } - aclManager := AclManager{RedisClient: redisClient} + aclManager := AclManager{RedisClient: redisClient, nodes: make(map[string]int)} ctx := context.Background() primary, err := aclManager.Primary(ctx) - if tt.wantErr { assert.Error(t, err) assert.Nil(t, primary) @@ -470,6 +468,7 @@ func TestAclManager_Loop(t *testing.T) { Addr: "localhost:6379", Password: "password", Username: "username", + nodes: make(map[string]int), }, wantErr: false, }, @@ -479,6 +478,7 @@ func TestAclManager_Loop(t *testing.T) { Addr: "localhost:6379", Password: "password", Username: "username", + nodes: make(map[string]int), }, wantErr: true, expectError: fmt.Errorf("error"), @@ -489,6 +489,7 @@ func TestAclManager_Loop(t *testing.T) { Addr: "localhost:6379", Password: "password", Username: "username", + nodes: make(map[string]int), }, wantErr: true, expectError: fmt.Errorf("unable to check if it's a Primary"), @@ -499,6 +500,7 @@ func TestAclManager_Loop(t *testing.T) { Addr: "localhost:6379", Password: "password", Username: "username", + nodes: make(map[string]int), }, wantErr: false, expectError: nil, From 8cb400f445da614c46f3f37e22a66a85a4d97fc0 Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Tue, 5 Dec 2023 22:30:54 +0100 Subject: [PATCH 08/14] adds test --- pkg/aclmanager/aclmanager_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/pkg/aclmanager/aclmanager_test.go b/pkg/aclmanager/aclmanager_test.go index aed8797..2f9715d 100644 --- a/pkg/aclmanager/aclmanager_test.go +++ b/pkg/aclmanager/aclmanager_test.go @@ -58,6 +58,7 @@ func TestFindNodes(t *testing.T) { mockResp string want map[string]int wantErr bool + nodes map[string]int }{ { name: "parse master output", @@ -81,6 +82,20 @@ func TestFindNodes(t *testing.T) { want: nil, wantErr: true, }, + { + name: "ensure old nodes are removed", + mockResp: primaryOutput, + want: map[string]int{ + "172.21.0.3:6379": Follower, + }, + wantErr: false, + nodes: map[string]int{ + "192.168.0.1:6379": Follower, + "192.168.0.2:6379": Follower, + "192.168.0.3:6379": Follower, + "192.168.0.4:6379": Follower, + }, + }, } for _, tt := range tests { @@ -101,6 +116,13 @@ func TestFindNodes(t *testing.T) { t.Errorf("findNodes() error = %v, wantErr %v", err, tt.wantErr) return } + if tt.name == "ensure old nodes are removed" { + for address, _ := range aclManager.nodes { + if _, ok := tt.nodes[address]; ok { + t.Errorf("findNodes() address %v shound not be found", address) + } + } + } for address, function := range aclManager.nodes { if wantFunction, ok := tt.want[address]; ok { if wantFunction != function { From 09b796221e632ed18a99fc778e3e1b8ada17490e Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Tue, 5 Dec 2023 22:33:25 +0100 Subject: [PATCH 09/14] validate scan error first --- pkg/aclmanager/aclmanager.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/aclmanager/aclmanager.go b/pkg/aclmanager/aclmanager.go index 0e5363c..03791a1 100644 --- a/pkg/aclmanager/aclmanager.go +++ b/pkg/aclmanager/aclmanager.go @@ -89,16 +89,16 @@ func (a *AclManager) findNodes(ctx context.Context) (err error) { } } + if err := scanner.Err(); err != nil { + return err + } + for _, node := range nodes { if _, ok := a.nodes[node]; !ok { delete(a.nodes, node) } } - if err := scanner.Err(); err != nil { - return err - } - return err } From e3c784f41026b9cb4f289ade18dd308b1527188e Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Tue, 5 Dec 2023 22:40:29 +0100 Subject: [PATCH 10/14] adds extra case --- pkg/aclmanager/aclmanager.go | 10 +++++----- pkg/aclmanager/aclmanager_test.go | 21 ++++++++++++--------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/pkg/aclmanager/aclmanager.go b/pkg/aclmanager/aclmanager.go index 03791a1..ba3d968 100644 --- a/pkg/aclmanager/aclmanager.go +++ b/pkg/aclmanager/aclmanager.go @@ -124,7 +124,7 @@ func (a *AclManager) Primary(ctx context.Context) (primary *AclManager, err erro for address, function := range a.nodes { if function == Primary { - return New(address, a.Username, a.Username), err + return New(address, a.Username, a.Password), err } } @@ -247,8 +247,8 @@ func (a *AclManager) Loop(ctx context.Context) (err error) { case <-ticker.C: function, e := a.CurrentFunction(ctx) if err != nil { - slog.Warn("unable to check if it's a Primary", "message", e) - err = fmt.Errorf("unable to check if it's a Primary: %w", e) + slog.Warn("unable to check if it's a Primary", "message", err) + err = fmt.Errorf("unable to check if it's a Primary: %w", err) } if function == Follower { primary, err = a.Primary(ctx) @@ -259,8 +259,8 @@ func (a *AclManager) Loop(ctx context.Context) (err error) { var added, deleted []string added, deleted, err = a.SyncAcls(ctx, primary) if err != nil { - slog.Warn("unable to sync acls from Primary", "message", e) - err = fmt.Errorf("unable to sync acls from Primary: %w", e) + slog.Warn("unable to sync acls from Primary", "message", err) + err = fmt.Errorf("unable to sync acls from Primary: %w", err) continue } slog.Info("Synced acls from Primary", "added", added, "deleted", deleted) diff --git a/pkg/aclmanager/aclmanager_test.go b/pkg/aclmanager/aclmanager_test.go index 2f9715d..25760c5 100644 --- a/pkg/aclmanager/aclmanager_test.go +++ b/pkg/aclmanager/aclmanager_test.go @@ -456,22 +456,25 @@ func TestAclManager_Primary(t *testing.T) { } else { mock.ExpectInfo("replication").SetVal(tt.mockResp) } - aclManager := AclManager{RedisClient: redisClient, nodes: make(map[string]int)} + aclManager := AclManager{RedisClient: redisClient, Username: "username", Password: "password", nodes: make(map[string]int)} ctx := context.Background() primary, err := aclManager.Primary(ctx) if tt.wantErr { assert.Error(t, err) assert.Nil(t, primary) - } else { - assert.NoError(t, err) - if tt.want == "" { - assert.Nil(t, primary) - return - } - assert.NotNil(t, primary) - assert.Equal(t, tt.want, primary.Addr) + return + } + + assert.NoError(t, err) + if tt.want == "" { + assert.Nil(t, primary) + return } + assert.NotNil(t, primary) + assert.Equal(t, tt.want, primary.Addr) + assert.Equal(t, aclManager.Username, primary.Username) + assert.Equal(t, aclManager.Password, primary.Password) }) } } From f79ad12fca8a91b152d6cc2ed6f5184519855432 Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Tue, 5 Dec 2023 22:42:37 +0100 Subject: [PATCH 11/14] update test case --- pkg/aclmanager/aclmanager_test.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/pkg/aclmanager/aclmanager_test.go b/pkg/aclmanager/aclmanager_test.go index 25760c5..37fed15 100644 --- a/pkg/aclmanager/aclmanager_test.go +++ b/pkg/aclmanager/aclmanager_test.go @@ -444,6 +444,11 @@ func TestAclManager_Primary(t *testing.T) { mockResp: followerOutput, wantErr: true, }, + { + name: "username and password", + mockResp: followerOutput, + wantErr: false, + }, } for _, tt := range tests { @@ -473,8 +478,10 @@ func TestAclManager_Primary(t *testing.T) { } assert.NotNil(t, primary) assert.Equal(t, tt.want, primary.Addr) - assert.Equal(t, aclManager.Username, primary.Username) - assert.Equal(t, aclManager.Password, primary.Password) + if tt.name == "username and password" { + assert.Equal(t, aclManager.Username, primary.Username) + assert.Equal(t, aclManager.Password, primary.Password) + } }) } } From 963a9418499e6d528045929ff1e0600a59cf3c0f Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Tue, 5 Dec 2023 22:46:42 +0100 Subject: [PATCH 12/14] fix test --- pkg/aclmanager/aclmanager_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/aclmanager/aclmanager_test.go b/pkg/aclmanager/aclmanager_test.go index 37fed15..b4dc89a 100644 --- a/pkg/aclmanager/aclmanager_test.go +++ b/pkg/aclmanager/aclmanager_test.go @@ -471,6 +471,12 @@ func TestAclManager_Primary(t *testing.T) { return } + if tt.name == "username and password" { + assert.Equal(t, aclManager.Username, primary.Username) + assert.Equal(t, aclManager.Password, primary.Password) + return + } + assert.NoError(t, err) if tt.want == "" { assert.Nil(t, primary) @@ -478,10 +484,6 @@ func TestAclManager_Primary(t *testing.T) { } assert.NotNil(t, primary) assert.Equal(t, tt.want, primary.Addr) - if tt.name == "username and password" { - assert.Equal(t, aclManager.Username, primary.Username) - assert.Equal(t, aclManager.Password, primary.Password) - } }) } } From 6757a4bb3e3ff1772c7d32b60d665391f0716bb9 Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Tue, 5 Dec 2023 23:05:35 +0100 Subject: [PATCH 13/14] update tests and sync acls --- pkg/aclmanager/aclmanager.go | 32 +++++++++++-------------------- pkg/aclmanager/aclmanager_test.go | 21 +++++++++++++++++--- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/pkg/aclmanager/aclmanager.go b/pkg/aclmanager/aclmanager.go index ba3d968..e20f362 100644 --- a/pkg/aclmanager/aclmanager.go +++ b/pkg/aclmanager/aclmanager.go @@ -131,20 +131,6 @@ func (a *AclManager) Primary(ctx context.Context) (primary *AclManager, err erro return nil, err } -// SyncAcls connects to master node and syncs the acls to the current node -func (a *AclManager) SyncAcls(ctx context.Context, primary *AclManager) (added []string, deleted []string, err error) { - slog.Debug("Syncing acls") - if primary == nil { - return added, deleted, fmt.Errorf("no primary found") - } - added, deleted, err = mirrorAcls(ctx, primary.RedisClient, a.RedisClient) - if err != nil { - return added, deleted, fmt.Errorf("error syncing acls: %v", err) - } - - return added, deleted, err -} - // Close closes the redis client func (a *AclManager) Close() error { return a.RedisClient.Close() @@ -178,15 +164,19 @@ func listAcls(ctx context.Context, client *redis.Client) (acls []string, err err return acls, nil } -// mirrorAcls returns a list of acls in the cluster based on the redis acl list command -func mirrorAcls(ctx context.Context, source *redis.Client, destination *redis.Client) (added []string, deleted []string, err error) { - slog.Debug("Mirroring acls") - sourceAcls, err := listAcls(ctx, source) +// SyncAcls connects to master node and syncs the acls to the current node +func (a *AclManager) SyncAcls(ctx context.Context, primary *AclManager) (added []string, deleted []string, err error) { + slog.Debug("Syncing acls") + if primary == nil { + return added, deleted, fmt.Errorf("no primary found") + } + + sourceAcls, err := listAcls(ctx, primary.RedisClient) if err != nil { return nil, nil, fmt.Errorf("error listing source acls: %v", err) } - destinationAcls, err := listAcls(ctx, destination) + destinationAcls, err := listAcls(ctx, a.RedisClient) if err != nil { return nil, nil, fmt.Errorf("error listing current acls: %v", err) } @@ -209,7 +199,7 @@ func mirrorAcls(ctx context.Context, source *redis.Client, destination *redis.Cl } else { // If not found in source, delete from destination slog.Debug("Deleting ACL", "username", username) - if err := destination.Do(context.Background(), "ACL", "DELUSER", username).Err(); err != nil { + if err := a.RedisClient.Do(context.Background(), "ACL", "DELUSER", username).Err(); err != nil { return nil, nil, fmt.Errorf("error deleting acl: %v", err) } deleted = append(deleted, username) @@ -225,7 +215,7 @@ func mirrorAcls(ctx context.Context, source *redis.Client, destination *redis.Cl for i, s := range command { commandInterfce[i] = s } - if err := destination.Do(context.Background(), commandInterfce...).Err(); err != nil { + if err := a.RedisClient.Do(context.Background(), commandInterfce...).Err(); err != nil { return nil, nil, fmt.Errorf("error setting acl: %v", err) } added = append(added, username) diff --git a/pkg/aclmanager/aclmanager_test.go b/pkg/aclmanager/aclmanager_test.go index b4dc89a..1df135c 100644 --- a/pkg/aclmanager/aclmanager_test.go +++ b/pkg/aclmanager/aclmanager_test.go @@ -263,12 +263,27 @@ func TestMirrorAcls(t *testing.T) { listAclsError: fmt.Errorf("error listing destination ACLs"), wantDestinationErr: true, }, + { + name: "Invalid aclManagerPrimary", + listAclsError: fmt.Errorf("error listing destination ACLs"), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - sourceClient, sourceMock := redismock.NewClientMock() - destinationClient, destMock := redismock.NewClientMock() + primaryClient, sourceMock := redismock.NewClientMock() + followerClient, destMock := redismock.NewClientMock() + + aclManagerPrimary := &AclManager{RedisClient: primaryClient, nodes: make(map[string]int)} + aclManagerFollower := &AclManager{RedisClient: followerClient, nodes: make(map[string]int)} + + if tt.name == "Invalid aclManagerPrimary" { + aclManagerPrimary = nil + _, _, err := aclManagerFollower.SyncAcls(context.Background(), aclManagerPrimary) + assert.Error(t, err) + assert.Equal(t, "no primary found", err.Error()) + return + } if tt.listAclsError != nil && tt.wantSourceErr { sourceMock.ExpectDo("ACL", "LIST").SetErr(tt.listAclsError) @@ -300,7 +315,7 @@ func TestMirrorAcls(t *testing.T) { } } - added, deleted, err := mirrorAcls(context.Background(), sourceClient, destinationClient) + added, deleted, err := aclManagerFollower.SyncAcls(context.Background(), aclManagerPrimary) if err != nil { if tt.wantSourceErr { if tt.listAclsError != nil && !strings.Contains(err.Error(), tt.listAclsError.Error()) { From c5f5fabfb0eba71885026b4a8733baf49557c964 Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Tue, 5 Dec 2023 23:06:40 +0100 Subject: [PATCH 14/14] removing dead var --- pkg/aclmanager/aclmanager.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/aclmanager/aclmanager.go b/pkg/aclmanager/aclmanager.go index e20f362..476ac0a 100644 --- a/pkg/aclmanager/aclmanager.go +++ b/pkg/aclmanager/aclmanager.go @@ -188,14 +188,12 @@ func (a *AclManager) SyncAcls(ctx context.Context, primary *AclManager) (added [ } // Delete ACLs not in source and remove from the toAdd map if present in destination - var insync uint for _, acl := range destinationAcls { username := strings.Split(acl, " ")[1] if _, found := toAdd[acl]; found { // If found in source, don't need to add, so remove from map delete(toAdd, acl) slog.Debug("ACL already in sync", "username", username) - insync++ } else { // If not found in source, delete from destination slog.Debug("Deleting ACL", "username", username)