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
This commit is contained in:
Dhia Ayachi 2021-07-22 10:32:27 -04:00 committed by GitHub
parent c9c80b5ef6
commit b725605fe4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 12 additions and 5 deletions

3
.changelog/10657.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
ca: report an error when setting the ca config fail because of an index check.
```

View File

@ -2,7 +2,9 @@ package state
import ( import (
"fmt" "fmt"
"github.com/hashicorp/go-memdb" "github.com/hashicorp/go-memdb"
"github.com/pkg/errors"
"github.com/hashicorp/consul/agent/structs" "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 // 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 // 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) { func (s *Store) CACheckAndSetConfig(idx, cidx uint64, config *structs.CAConfiguration) (bool, error) {
tx := s.db.WriteTxn(idx) tx := s.db.WriteTxn(idx)
defer tx.Abort() defer tx.Abort()
@ -157,7 +159,7 @@ func (s *Store) CACheckAndSetConfig(idx, cidx uint64, config *structs.CAConfigur
// return early here. // return early here.
e, ok := existing.(*structs.CAConfiguration) e, ok := existing.(*structs.CAConfiguration)
if (ok && e.ModifyIndex != cidx) || (!ok && cidx != 0) { 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 { if err := s.caSetConfigTxn(idx, tx, config); err != nil {

View File

@ -4,6 +4,8 @@ import (
"reflect" "reflect"
"testing" "testing"
"github.com/hashicorp/consul/sdk/testutil"
"github.com/hashicorp/go-memdb" "github.com/hashicorp/go-memdb"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
@ -59,9 +61,9 @@ func TestStore_CAConfigCAS(t *testing.T) {
ok, err := s.CACheckAndSetConfig(2, 0, &structs.CAConfiguration{ ok, err := s.CACheckAndSetConfig(2, 0, &structs.CAConfiguration{
Provider: "static", 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 // Check that the index is untouched and the entry
// has not been updated. // has not been updated.