Merge pull request #107059 from tbg/backport23.1-106515

release-23.1: DEPS: bump across etcd-io/raft#81 and disable conf change validation

DEPS.bzl

@@ -10327,10 +10327,10 @@ def go_deps():
         ],
         build_file_proto_mode = "default",
         importpath = "go.etcd.io/raft/v3",
-        sha256 = "cebe2f313d01c24a8927170d44ecba9e5da56de488245e95a662edd590ed9a92",
-        strip_prefix = "github.com/cockroachdb/raft/[email protected]20230713102459-53e0e3664cb0",
+        sha256 = "fcad7dc8a9d53bbb3d5ed050827004a70286649ca2fb66504d3dab44abb13a92",
+        strip_prefix = "github.com/cockroachdb/raft/[email protected]20230718062839-336c0ec27d85",
         urls = [
-            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/raft/v3/com_github_cockroachdb_raft_v3-v3.0.0-20230713102459-53e0e3664cb0.zip",
+            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/raft/v3/com_github_cockroachdb_raft_v3-v3.0.0-20230718062839-336c0ec27d85.zip",
         ],
     )
     go_repository(

build/bazelutil/distdir_files.bzl

@@ -312,7 +312,7 @@ DISTDIR_FILES = {
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/gostdlib/com_github_cockroachdb_gostdlib-v1.19.0.zip": "c4d516bcfe8c07b6fc09b8a9a07a95065b36c2855627cb3514e40c98f872b69e",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/logtags/com_github_cockroachdb_logtags-v0.0.0-20230118201751-21c54148d20b.zip": "ca7776f47e5fecb4c495490a679036bfc29d95bd7625290cfdb9abb0baf97476",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230712164400-52cfb819e866.zip": "bd2df4269fa5f4740b0f96663b6bc695e5ec7cb09f42ac0ffdc674d4f64abdd1",
-    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/raft/v3/com_github_cockroachdb_raft_v3-v3.0.0-20230713102459-53e0e3664cb0.zip": "cebe2f313d01c24a8927170d44ecba9e5da56de488245e95a662edd590ed9a92",
+    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/raft/v3/com_github_cockroachdb_raft_v3-v3.0.0-20230718062839-336c0ec27d85.zip": "fcad7dc8a9d53bbb3d5ed050827004a70286649ca2fb66504d3dab44abb13a92",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/redact/com_github_cockroachdb_redact-v1.1.4.zip": "9a723dfbb1627ae2f2b5d1374a59d6188ae9796e6dfb9a4622a4eb94321a4fac",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/returncheck/com_github_cockroachdb_returncheck-v0.0.0-20200612231554-92cdbca611dd.zip": "ce92ba4352deec995b1f2eecf16eba7f5d51f5aa245a1c362dfe24c83d31f82b",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/sentry-go/com_github_cockroachdb_sentry_go-v0.6.1-cockroachdb.2.zip": "fbb2207d02aecfdd411b1357efe1192dbb827959e36b7cab7491731ac55935c9",

go.mod

@@ -430,6 +430,6 @@ replace gopkg.in/yaml.v2 => github.com/cockroachdb/yaml v0.0.0-20210825132133-2d
 replace github.com/docker/docker => github.com/moby/moby v20.10.6+incompatible
 
 // Take etcd-io/raft from the branch corresponding to release-23.1 in our fork.
-replace go.etcd.io/raft/v3 => github.com/cockroachdb/raft/v3 v3.0.0-20230713102459-53e0e3664cb0
+replace go.etcd.io/raft/v3 => github.com/cockroachdb/raft/v3 v3.0.0-20230718062839-336c0ec27d85
 
 replace golang.org/x/time => github.com/cockroachdb/x-time v0.3.1-0.20230525123634-71747adb5d5c

go.sum

@@ -481,8 +481,8 @@ github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b h1:r6VH0faHjZe
 github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs=
 github.com/cockroachdb/pebble v0.0.0-20230712164400-52cfb819e866 h1:3Xr/jeWmpd3JZ8VCaKB6g1XOc/Bj99gNa3VIlZM7Oes=
 github.com/cockroachdb/pebble v0.0.0-20230712164400-52cfb819e866/go.mod h1:9lRMC4XN3/BLPtIp6kAKwIaHu369NOf2rMucPzipz50=
-github.com/cockroachdb/raft/v3 v3.0.0-20230713102459-53e0e3664cb0 h1:Lss4oj/NtrEHxHoLUp+aMqwwq4jyk7RegHfn1GT4RF4=
-github.com/cockroachdb/raft/v3 v3.0.0-20230713102459-53e0e3664cb0/go.mod h1:xa/jfCF4K9FpWEAXstT+Nw4ToWdmhJ8oeC5eburtypA=
+github.com/cockroachdb/raft/v3 v3.0.0-20230718062839-336c0ec27d85 h1:Lv0iiCw5BB6y/loaOgeC75OwB9x6Hi1XbBuhU9eQFNg=
+github.com/cockroachdb/raft/v3 v3.0.0-20230718062839-336c0ec27d85/go.mod h1:xa/jfCF4K9FpWEAXstT+Nw4ToWdmhJ8oeC5eburtypA=
 github.com/cockroachdb/redact v1.1.3/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
 github.com/cockroachdb/redact v1.1.4 h1:Y0XrVI2FAyofNyGveodTN//qdpPtFKcKTeCBsK3AHAQ=
 github.com/cockroachdb/redact v1.1.4/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=

pkg/kv/kvserver/client_raft_test.go

@@ -5999,3 +5999,65 @@ func TestRaftSnapshotsWithMVCCRangeKeysEverywhere(t *testing.T) {
 		require.Equal(t, kvpb.CheckConsistencyResponse_RANGE_CONSISTENT, result.Status, "%+v", result)
 	}
 }
+
+// TestInvalidConfChangeRejection is a regression test for [1]. It proposes
+// an (intentionally) invalid configuration change and makes sure that raft
+// does not drop it.
+//
+// [1]: https://github.com/cockroachdb/cockroach/issues/105797
+func TestInvalidConfChangeRejection(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	// This is a regression test against a stuck command, so set a timeout to get
+	// a shot at a graceful failure on regression.
+	ctx, cancel := context.WithTimeout(context.Background(), testutils.DefaultSucceedsSoonDuration)
+	defer cancel()
+
+	// When our configuration change shows up below raft, we need to apply it as a
+	// no-op, since the config change is intentionally invalid and assertions
+	// would fail if we were to try to actually apply it.
+	injErr := errors.New("injected error")
+	tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{
+		ReplicationMode: base.ReplicationManual,
+		ServerArgs: base.TestServerArgs{Knobs: base.TestingKnobs{Store: &kvserver.StoreTestingKnobs{
+			TestingApplyCalledTwiceFilter: func(args kvserverbase.ApplyFilterArgs) (int, *kvpb.Error) {
+				if args.Req != nil && args.Req.Txn != nil && args.Req.Txn.Name == "fake" {
+					return 0, kvpb.NewError(injErr)
+				}
+				return 0, nil
+			}}}},
+	})
+	defer tc.Stopper().Stop(ctx)
+
+	k := tc.ScratchRange(t)
+
+	repl := tc.GetFirstStoreFromServer(t, 0).LookupReplica(keys.MustAddr(k))
+
+	// Try to leave a joint config even though we're not in one. This is something
+	// that will lead raft to propose an empty entry instead of our conf change.
+	//
+	// See: https://github.com/cockroachdb/cockroach/issues/105797
+	var ba kvpb.BatchRequest
+	now := tc.Server(0).Clock().Now()
+	txn := roachpb.MakeTransaction("fake", k, roachpb.NormalUserPriority, now, 500*time.Millisecond.Nanoseconds(), 1)
+	ba.Txn = &txn
+	ba.Timestamp = now
+	ba.Add(&kvpb.EndTxnRequest{
+		RequestHeader: kvpb.RequestHeader{
+			Key: k,
+		},
+		Commit: true,
+		InternalCommitTrigger: &roachpb.InternalCommitTrigger{
+			ChangeReplicasTrigger: &roachpb.ChangeReplicasTrigger{
+				Desc: repl.Desc(),
+			},
+		},
+	})
+
+	_, pErr := repl.Send(ctx, &ba)
+	// Verify that we see the configuration change below raft, where we rejected it
+	// (since it would've otherwise blow up the Replica: after all, we intentionally
+	// proposed an invalid configuration change.
+	require.True(t, errors.Is(pErr.GoError(), injErr), "%+v", pErr.GoError())
+}

pkg/kv/kvserver/store.go

@@ -291,18 +291,19 @@ func newRaftConfig(
 	strg raft.Storage, id uint64, appliedIndex uint64, storeCfg StoreConfig, logger raft.Logger,
 ) *raft.Config {
 	return &raft.Config{
-		ID:                        id,
-		Applied:                   appliedIndex,
-		AsyncStorageWrites:        true,
-		ElectionTick:              storeCfg.RaftElectionTimeoutTicks,
-		HeartbeatTick:             storeCfg.RaftHeartbeatIntervalTicks,
-		MaxUncommittedEntriesSize: storeCfg.RaftMaxUncommittedEntriesSize,
-		MaxCommittedSizePerReady:  storeCfg.RaftMaxCommittedSizePerReady,
-		MaxSizePerMsg:             storeCfg.RaftMaxSizePerMsg,
-		MaxInflightMsgs:           storeCfg.RaftMaxInflightMsgs,
-		MaxInflightBytes:          storeCfg.RaftMaxInflightBytes,
-		Storage:                   strg,
-		Logger:                    logger,
+		ID:                          id,
+		Applied:                     appliedIndex,
+		AsyncStorageWrites:          true,
+		ElectionTick:                storeCfg.RaftElectionTimeoutTicks,
+		HeartbeatTick:               storeCfg.RaftHeartbeatIntervalTicks,
+		MaxUncommittedEntriesSize:   storeCfg.RaftMaxUncommittedEntriesSize,
+		MaxCommittedSizePerReady:    storeCfg.RaftMaxCommittedSizePerReady,
+		DisableConfChangeValidation: true, // see https://github.com/cockroachdb/cockroach/issues/105797
+		MaxSizePerMsg:               storeCfg.RaftMaxSizePerMsg,
+		MaxInflightMsgs:             storeCfg.RaftMaxInflightMsgs,
+		MaxInflightBytes:            storeCfg.RaftMaxInflightBytes,
+		Storage:                     strg,
+		Logger:                      logger,
 
 		PreVote: true,
 	}