From 529fe737efc17d7d664b1fa8200631e31706069f Mon Sep 17 00:00:00 2001 From: Giulio Micheloni Date: Wed, 14 Jul 2021 11:50:23 +0200 Subject: [PATCH 1/3] acl: acl replication routine to report the last error message --- agent/consul/acl_replication.go | 3 ++- agent/consul/acl_replication_test.go | 2 ++ agent/consul/leader.go | 4 ++-- agent/structs/acl.go | 1 + 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/agent/consul/acl_replication.go b/agent/consul/acl_replication.go index 4035b8ec43..f76900356c 100644 --- a/agent/consul/acl_replication.go +++ b/agent/consul/acl_replication.go @@ -484,11 +484,12 @@ func (s *Server) IsACLReplicationEnabled() bool { s.config.ACLTokenReplication } -func (s *Server) updateACLReplicationStatusError() { +func (s *Server) updateACLReplicationStatusError(errorMsg error) { s.aclReplicationStatusLock.Lock() defer s.aclReplicationStatusLock.Unlock() s.aclReplicationStatus.LastError = time.Now().Round(time.Second).UTC() + s.aclReplicationStatus.LastErrorMessage = errorMsg } func (s *Server) updateACLReplicationStatusIndex(replicationType structs.ACLReplicationType, index uint64) { diff --git a/agent/consul/acl_replication_test.go b/agent/consul/acl_replication_test.go index 26726fe360..841d18020a 100644 --- a/agent/consul/acl_replication_test.go +++ b/agent/consul/acl_replication_test.go @@ -1,6 +1,7 @@ package consul import ( + "errors" "fmt" "os" "strconv" @@ -780,6 +781,7 @@ func TestACLReplication_TokensRedacted(t *testing.T) { require.True(r, status.ReplicatedTokenIndex < token2.CreateIndex, "ReplicatedTokenIndex is not less than the token2s create index") // ensures that token replication is erroring require.True(r, status.LastError.After(minErrorTime), "Replication LastError not after the minErrorTime") + require.Equal(r, status.LastErrorMessage, errors.New("failed to retrieve unredacted tokens - replication token in use does not grant acl:write")) }) } diff --git a/agent/consul/leader.go b/agent/consul/leader.go index 391b73ca76..c5c35b7905 100644 --- a/agent/consul/leader.go +++ b/agent/consul/leader.go @@ -810,7 +810,7 @@ func (s *Server) runLegacyACLReplication(ctx context.Context) error { 0, ) lastRemoteIndex = 0 - s.updateACLReplicationStatusError() + s.updateACLReplicationStatusError(err) legacyACLLogger.Warn("Legacy ACL replication error (will retry if still leader)", "error", err) } else { metrics.SetGauge([]string{"leader", "replication", "acl-legacy", "status"}, @@ -927,7 +927,7 @@ func (s *Server) runACLReplicator( 0, ) lastRemoteIndex = 0 - s.updateACLReplicationStatusError() + s.updateACLReplicationStatusError(err) logger.Warn("ACL replication error (will retry if still leader)", "error", err, ) diff --git a/agent/structs/acl.go b/agent/structs/acl.go index ed2b0791ae..1a93f88421 100644 --- a/agent/structs/acl.go +++ b/agent/structs/acl.go @@ -1269,6 +1269,7 @@ type ACLReplicationStatus struct { ReplicatedTokenIndex uint64 LastSuccess time.Time LastError time.Time + LastErrorMessage error } // ACLTokenSetRequest is used for token creation and update operations From 814ef6b103cf9e89f41df35928025c091aaee3ac Mon Sep 17 00:00:00 2001 From: Giulio Micheloni Date: Thu, 15 Jul 2021 11:31:44 +0200 Subject: [PATCH 2/3] acl: fix error type into a string type for serialization issue acl_endpoint_test.go:507: Error Trace: acl_endpoint_test.go:507 retry.go:148 retry.go:149 retry.go:103 acl_endpoint_test.go:504 Error: Received unexpected error: codec.decoder: decodeValue: Cannot decode non-nil codec value into nil error (1 methods) Test: TestACLEndpoint_ReplicationStatus --- agent/consul/acl_replication.go | 2 +- agent/consul/acl_replication_test.go | 3 +-- agent/structs/acl.go | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/agent/consul/acl_replication.go b/agent/consul/acl_replication.go index f76900356c..6c5d07f311 100644 --- a/agent/consul/acl_replication.go +++ b/agent/consul/acl_replication.go @@ -489,7 +489,7 @@ func (s *Server) updateACLReplicationStatusError(errorMsg error) { defer s.aclReplicationStatusLock.Unlock() s.aclReplicationStatus.LastError = time.Now().Round(time.Second).UTC() - s.aclReplicationStatus.LastErrorMessage = errorMsg + s.aclReplicationStatus.LastErrorMessage = errorMsg.Error() } func (s *Server) updateACLReplicationStatusIndex(replicationType structs.ACLReplicationType, index uint64) { diff --git a/agent/consul/acl_replication_test.go b/agent/consul/acl_replication_test.go index 841d18020a..0bb96a8457 100644 --- a/agent/consul/acl_replication_test.go +++ b/agent/consul/acl_replication_test.go @@ -1,7 +1,6 @@ package consul import ( - "errors" "fmt" "os" "strconv" @@ -781,7 +780,7 @@ func TestACLReplication_TokensRedacted(t *testing.T) { require.True(r, status.ReplicatedTokenIndex < token2.CreateIndex, "ReplicatedTokenIndex is not less than the token2s create index") // ensures that token replication is erroring require.True(r, status.LastError.After(minErrorTime), "Replication LastError not after the minErrorTime") - require.Equal(r, status.LastErrorMessage, errors.New("failed to retrieve unredacted tokens - replication token in use does not grant acl:write")) + require.Equal(r, status.LastErrorMessage, "failed to retrieve unredacted tokens - replication token in use does not grant acl:write") }) } diff --git a/agent/structs/acl.go b/agent/structs/acl.go index 1a93f88421..f8029f1a73 100644 --- a/agent/structs/acl.go +++ b/agent/structs/acl.go @@ -1269,7 +1269,7 @@ type ACLReplicationStatus struct { ReplicatedTokenIndex uint64 LastSuccess time.Time LastError time.Time - LastErrorMessage error + LastErrorMessage string } // ACLTokenSetRequest is used for token creation and update operations From d4a3fe33e800f374a3553a9621d495e9d3a02f77 Mon Sep 17 00:00:00 2001 From: Giulio Micheloni Date: Fri, 6 Aug 2021 22:35:27 +0100 Subject: [PATCH 3/3] String type instead of error type and changelog. --- .changelog/10612.txt | 3 +++ agent/consul/acl_replication.go | 4 ++-- agent/consul/leader.go | 4 ++-- api/acl.go | 1 + 4 files changed, 8 insertions(+), 4 deletions(-) create mode 100644 .changelog/10612.txt diff --git a/.changelog/10612.txt b/.changelog/10612.txt new file mode 100644 index 0000000000..e9a54002da --- /dev/null +++ b/.changelog/10612.txt @@ -0,0 +1,3 @@ +```release-note:improvement +acl: replication routine to report the last error message. +``` diff --git a/agent/consul/acl_replication.go b/agent/consul/acl_replication.go index 6c5d07f311..2fa89f0f50 100644 --- a/agent/consul/acl_replication.go +++ b/agent/consul/acl_replication.go @@ -484,12 +484,12 @@ func (s *Server) IsACLReplicationEnabled() bool { s.config.ACLTokenReplication } -func (s *Server) updateACLReplicationStatusError(errorMsg error) { +func (s *Server) updateACLReplicationStatusError(errorMsg string) { s.aclReplicationStatusLock.Lock() defer s.aclReplicationStatusLock.Unlock() s.aclReplicationStatus.LastError = time.Now().Round(time.Second).UTC() - s.aclReplicationStatus.LastErrorMessage = errorMsg.Error() + s.aclReplicationStatus.LastErrorMessage = errorMsg } func (s *Server) updateACLReplicationStatusIndex(replicationType structs.ACLReplicationType, index uint64) { diff --git a/agent/consul/leader.go b/agent/consul/leader.go index c5c35b7905..230b9f5a78 100644 --- a/agent/consul/leader.go +++ b/agent/consul/leader.go @@ -810,7 +810,7 @@ func (s *Server) runLegacyACLReplication(ctx context.Context) error { 0, ) lastRemoteIndex = 0 - s.updateACLReplicationStatusError(err) + s.updateACLReplicationStatusError(err.Error()) legacyACLLogger.Warn("Legacy ACL replication error (will retry if still leader)", "error", err) } else { metrics.SetGauge([]string{"leader", "replication", "acl-legacy", "status"}, @@ -927,7 +927,7 @@ func (s *Server) runACLReplicator( 0, ) lastRemoteIndex = 0 - s.updateACLReplicationStatusError(err) + s.updateACLReplicationStatusError(err.Error()) logger.Warn("ACL replication error (will retry if still leader)", "error", err, ) diff --git a/api/acl.go b/api/acl.go index d94c2807a7..e0072e9b0c 100644 --- a/api/acl.go +++ b/api/acl.go @@ -97,6 +97,7 @@ type ACLReplicationStatus struct { ReplicatedTokenIndex uint64 LastSuccess time.Time LastError time.Time + LastErrorMessage string } // ACLServiceIdentity represents a high-level grant of all necessary privileges