From 82402869566b2edffcd78d75b7c370a9f31a2cd4 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Sun, 10 Oct 2021 21:15:13 -0400 Subject: [PATCH] ca: remove state check in secondarySetPrimaryRoots This function is only ever called from operations that have already acquired the state lock, so checking the value of state can never fail. This change is being made in preparation for splitting out a separate type for the secondary logic. The state can't easily be shared, so really only the expored top-level functions should acquire the 'state lock'. --- agent/consul/leader_connect_ca.go | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index 30ce677c3..8955d7e78 100644 --- a/agent/consul/leader_connect_ca.go +++ b/agent/consul/leader_connect_ca.go @@ -183,19 +183,15 @@ func (e *caStateError) Error() string { } // secondarySetPrimaryRoots updates the most recently seen roots from the primary. -func (c *CAManager) secondarySetPrimaryRoots(newRoots structs.IndexedCARoots) error { +func (c *CAManager) secondarySetPrimaryRoots(newRoots structs.IndexedCARoots) { + // TODO: this could be a different lock, as long as its the same lock in secondaryGetPrimaryRoots c.stateLock.Lock() defer c.stateLock.Unlock() - - if c.state == caStateInitializing || c.state == caStateReconfig { - c.primaryRoots = newRoots - } else { - return fmt.Errorf("Cannot update primary roots in state %q", c.state) - } - return nil + c.primaryRoots = newRoots } func (c *CAManager) secondaryGetPrimaryRoots() structs.IndexedCARoots { + // TODO: this could be a different lock, as long as its the same lock in secondarySetPrimaryRoots c.stateLock.Lock() defer c.stateLock.Unlock() return c.primaryRoots @@ -430,9 +426,7 @@ func (c *CAManager) secondaryInitialize(provider ca.Provider, conf *structs.CACo if err := c.delegate.forwardDC("ConnectCA.Roots", c.serverConf.PrimaryDatacenter, &args, &roots); err != nil { return err } - if err := c.secondarySetPrimaryRoots(roots); err != nil { - return err - } + c.secondarySetPrimaryRoots(roots) // Configure the CA provider and initialize the intermediate certificate if necessary. if err := c.secondaryInitializeProvider(provider, roots); err != nil { @@ -1254,9 +1248,7 @@ func (c *CAManager) secondaryUpdateRoots(roots structs.IndexedCARoots) error { defer c.setState(caStateInitialized, false) // Update the cached primary roots now that the lock is held. - if err := c.secondarySetPrimaryRoots(roots); err != nil { - return err - } + c.secondarySetPrimaryRoots(roots) provider, _ := c.getCAProvider() if provider == nil { @@ -1317,6 +1309,7 @@ func (c *CAManager) secondaryInitializeProvider(provider ca.Provider, roots stru // method is used to detect when the secondary has received the roots from the // primary DC. func (c *CAManager) secondaryHasProviderRoots() bool { + // TODO: this could potentially also use primaryRoots instead of providerRoot c.providerLock.Lock() defer c.providerLock.Unlock() return c.providerRoot != nil