From 6105a7fd9f55fde192b8dc9bbdac79422236ec2c Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Tue, 13 Sep 2022 15:06:40 -0700 Subject: [PATCH 1/2] connect/ca: don't discard old roots on primaryInitialize --- agent/consul/leader_connect_ca.go | 15 +------ agent/consul/leader_connect_test.go | 65 +++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 14 deletions(-) diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index 4de55ba03c..4d9a42a9ad 100644 --- a/agent/consul/leader_connect_ca.go +++ b/agent/consul/leader_connect_ca.go @@ -564,22 +564,9 @@ func (c *CAManager) primaryInitialize(provider ca.Provider, conf *structs.CAConf return nil } - // Get the highest index - idx, _, err := state.CARoots(nil) - if err != nil { + if err := c.persistNewRootAndConfig(provider, rootCA, conf); err != nil { return err } - - // Store the root cert in raft - _, err = c.delegate.ApplyCARequest(&structs.CARequest{ - Op: structs.CAOpSetRoots, - Index: idx, - Roots: []*structs.CARoot{rootCA}, - }) - if err != nil { - return fmt.Errorf("raft apply failed: %w", err) - } - c.setCAProvider(provider, rootCA) c.logger.Info("initialized primary datacenter CA with provider", "provider", conf.Provider) diff --git a/agent/consul/leader_connect_test.go b/agent/consul/leader_connect_test.go index c8b361b03b..bf6fa95221 100644 --- a/agent/consul/leader_connect_test.go +++ b/agent/consul/leader_connect_test.go @@ -691,6 +691,71 @@ func TestConnectCA_ConfigurationSet_RootRotation_Secondary(t *testing.T) { require.NoError(t, err) } +func TestCAManager_Initialize_Vault_KeepOldRoots_Primary(t *testing.T) { + ca.SkipIfVaultNotPresent(t) + + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + t.Parallel() + + testVault := ca.NewTestVaultServer(t) + defer testVault.Stop() + + dir1pre, s1pre := testServer(t) + defer os.RemoveAll(dir1pre) + defer s1pre.Shutdown() + codec := rpcClient(t, s1pre) + defer codec.Close() + + testrpc.WaitForLeader(t, s1pre.RPC, "dc1") + + // Update the CA config to use Vault - this should force the generation of a new root cert. + vaultCAConf := &structs.CAConfiguration{ + Provider: "vault", + Config: map[string]interface{}{ + "Address": testVault.Addr, + "Token": testVault.RootToken, + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }, + } + + args := &structs.CARequest{ + Datacenter: "dc1", + Config: vaultCAConf, + } + var reply interface{} + + require.NoError(t, msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationSet", args, &reply)) + + // Should have 2 roots now. + _, roots, err := s1pre.fsm.State().CARoots(nil) + require.NoError(t, err) + require.Len(t, roots, 2) + + // Shutdown s1pre and restart it to trigger the primary CA init. + s1pre.Shutdown() + + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.DataDir = s1pre.config.DataDir + c.NodeName = s1pre.config.NodeName + c.NodeID = s1pre.config.NodeID + }) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + + testrpc.WaitForLeader(t, s1.RPC, "dc1") + + // Roots should be unchanged + _, rootsAfterRestart, err := s1.fsm.State().CARoots(nil) + require.NoError(t, err) + require.Len(t, rootsAfterRestart, 2) + require.Equal(t, roots[0].ID, rootsAfterRestart[0].ID) + require.Equal(t, roots[1].ID, rootsAfterRestart[1].ID) +} + func TestCAManager_Initialize_Vault_FixesSigningKeyID_Primary(t *testing.T) { ca.SkipIfVaultNotPresent(t) From 573701fc474cf694ce7726090d1632d1871eeb10 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Tue, 13 Sep 2022 15:55:17 -0700 Subject: [PATCH 2/2] Add changelog note --- .changelog/14598.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/14598.txt diff --git a/.changelog/14598.txt b/.changelog/14598.txt new file mode 100644 index 0000000000..f76b4958d1 --- /dev/null +++ b/.changelog/14598.txt @@ -0,0 +1,3 @@ +```release-note:bug +connect: Fixed a bug where old root CAs would be removed from the primary datacenter after switching providers and restarting the cluster. +```