From c6859b3fb07031e3ea1fcd0230bacb070315d93d Mon Sep 17 00:00:00 2001
From: Dhia Ayachi <dhia@hashicorp.com>
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 0000000000..4c273d49b3
--- /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 7e9ac11f99..ab54ae69a2 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 26e94cacc6..ba47697205 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.