diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index 5e42b5f9d..cf010b353 100644 --- a/agent/consul/leader_connect_ca.go +++ b/agent/consul/leader_connect_ca.go @@ -535,6 +535,7 @@ func (c *CAManager) persistNewRootAndConfig(provider ca.Provider, newActiveRoot return err } + // Look up the existing CA config if a new one wasn't provided. var newConf structs.CAConfiguration _, storedConfig, err := state.CAConfig(nil) if err != nil { @@ -543,14 +544,28 @@ func (c *CAManager) persistNewRootAndConfig(provider ca.Provider, newActiveRoot if storedConfig == nil { return fmt.Errorf("local CA not initialized yet") } + // Exit early if the change is a no-op. + if newActiveRoot == nil && config != nil && config.Provider == storedConfig.Provider && reflect.DeepEqual(config.Config, storedConfig.Config) { + return nil + } + if config != nil { newConf = *config } else { newConf = *storedConfig } + + // Update the trust domain for the config if there's a new root, or keep the old + // one if the root isn't being updated. newConf.ModifyIndex = storedConfig.ModifyIndex if newActiveRoot != nil { newConf.ClusterID = newActiveRoot.ExternalTrustDomain + } else { + _, activeRoot, err := state.CARootActive(nil) + if err != nil { + return err + } + newConf.ClusterID = activeRoot.ExternalTrustDomain } // Persist any state the provider needs us to @@ -562,21 +577,19 @@ func (c *CAManager) persistNewRootAndConfig(provider ca.Provider, newActiveRoot // If there's a new active root, copy the root list and append it, updating // the old root with the time it was rotated out. var newRoots structs.CARoots - if newActiveRoot != nil { - for _, r := range oldRoots { - newRoot := *r - if newRoot.Active { - newRoot.Active = false - newRoot.RotatedOutAt = time.Now() - } - if newRoot.ExternalTrustDomain == "" { - newRoot.ExternalTrustDomain = newConf.ClusterID - } - newRoots = append(newRoots, &newRoot) + for _, r := range oldRoots { + newRoot := *r + if newRoot.Active { + newRoot.Active = false + newRoot.RotatedOutAt = time.Now() } + if newRoot.ExternalTrustDomain == "" { + newRoot.ExternalTrustDomain = newConf.ClusterID + } + newRoots = append(newRoots, &newRoot) + } + if newActiveRoot != nil { newRoots = append(newRoots, newActiveRoot) - } else { - newRoots = oldRoots } args := &structs.CARequest{ diff --git a/agent/consul/leader_connect_ca_test.go b/agent/consul/leader_connect_ca_test.go index a582f90ba..25708bc96 100644 --- a/agent/consul/leader_connect_ca_test.go +++ b/agent/consul/leader_connect_ca_test.go @@ -100,6 +100,7 @@ func (m *mockCAServerDelegate) raftApply(t structs.MessageType, msg interface{}) act, err = m.store.CACheckAndSetConfig(1, req.Config.ModifyIndex, req.Config) require.NoError(m.t, err) + require.True(m.t, act) } m.callbackCh <- fmt.Sprintf("raftApply/%s", t) return nil, nil diff --git a/agent/consul/leader_connect_test.go b/agent/consul/leader_connect_test.go index 5beca904b..cf7ddff42 100644 --- a/agent/consul/leader_connect_test.go +++ b/agent/consul/leader_connect_test.go @@ -700,22 +700,6 @@ func TestLeader_SecondaryCA_TransitionFromPrimary(t *testing.T) { require.NoError(t, s2.RPC("ConnectCA.Roots", &args, &dc2PrimaryRoots)) require.Len(t, dc2PrimaryRoots.Roots, 1) - // Set the ExternalTrustDomain to a blank string to simulate an old version (pre-1.4.0) - // it's fine to change the roots struct directly here because the RPC endpoint already - // makes a copy to return. - dc2PrimaryRoots.Roots[0].ExternalTrustDomain = "" - rootSetArgs := structs.CARequest{ - Op: structs.CAOpSetRoots, - Datacenter: "dc2", - Index: dc2PrimaryRoots.Index, - Roots: dc2PrimaryRoots.Roots, - } - resp, err := s2.raftApply(structs.ConnectCARequestType, rootSetArgs) - require.NoError(t, err) - if respErr, ok := resp.(error); ok { - t.Fatal(respErr) - } - // Shutdown s2 and restart it with the dc1 as the primary s2.Shutdown() dir3, s3 := testServerWithConfig(t, func(c *Config) {