From b725605fe431a1186e0e971965e088206f707fb6 Mon Sep 17 00:00:00 2001 From: Dhia Ayachi Date: Thu, 22 Jul 2021 10:32:27 -0400 Subject: [PATCH] config raft apply silent error (#10657) * return an error when the index is not valid * check response as bool when applying `CAOpSetConfig` * remove check for bool response * fix error message and add check to test * fix comment * add changelog --- .changelog/10657.txt | 3 +++ agent/consul/state/connect_ca.go | 6 ++++-- agent/consul/state/connect_ca_test.go | 8 +++++--- 3 files changed, 12 insertions(+), 5 deletions(-) create mode 100644 .changelog/10657.txt diff --git a/.changelog/10657.txt b/.changelog/10657.txt new file mode 100644 index 000000000..4c273d49b --- /dev/null +++ b/.changelog/10657.txt @@ -0,0 +1,3 @@ +```release-note:bug +ca: report an error when setting the ca config fail because of an index check. +``` diff --git a/agent/consul/state/connect_ca.go b/agent/consul/state/connect_ca.go index 7e9ac11f9..ab54ae69a 100644 --- a/agent/consul/state/connect_ca.go +++ b/agent/consul/state/connect_ca.go @@ -2,7 +2,9 @@ package state import ( "fmt" + "github.com/hashicorp/go-memdb" + "github.com/pkg/errors" "github.com/hashicorp/consul/agent/structs" ) @@ -141,7 +143,7 @@ func (s *Store) CASetConfig(idx uint64, config *structs.CAConfiguration) error { // CACheckAndSetConfig is used to try updating the CA configuration with a // given Raft index. If the CAS index specified is not equal to the last observed index -// for the config, then the call is a noop, +// for the config, then the call will return an error, func (s *Store) CACheckAndSetConfig(idx, cidx uint64, config *structs.CAConfiguration) (bool, error) { tx := s.db.WriteTxn(idx) defer tx.Abort() @@ -157,7 +159,7 @@ func (s *Store) CACheckAndSetConfig(idx, cidx uint64, config *structs.CAConfigur // return early here. e, ok := existing.(*structs.CAConfiguration) if (ok && e.ModifyIndex != cidx) || (!ok && cidx != 0) { - return false, nil + return false, errors.Errorf("ModifyIndex did not match existing") } if err := s.caSetConfigTxn(idx, tx, config); err != nil { diff --git a/agent/consul/state/connect_ca_test.go b/agent/consul/state/connect_ca_test.go index 26e94cacc..ba4769720 100644 --- a/agent/consul/state/connect_ca_test.go +++ b/agent/consul/state/connect_ca_test.go @@ -4,6 +4,8 @@ import ( "reflect" "testing" + "github.com/hashicorp/consul/sdk/testutil" + "github.com/hashicorp/go-memdb" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -59,9 +61,9 @@ func TestStore_CAConfigCAS(t *testing.T) { ok, err := s.CACheckAndSetConfig(2, 0, &structs.CAConfiguration{ Provider: "static", }) - if ok || err != nil { - t.Fatalf("expected (false, nil), got: (%v, %#v)", ok, err) - } + + require.False(t, ok) + testutil.RequireErrorContains(t, err, "ModifyIndex did not match existing") // Check that the index is untouched and the entry // has not been updated.