Mark config dirty if read differs from state (#21835)

* Refactor CRL writing config to passthrough cache

When reading the CRL config via API endpoint, always read through to the
disk, updating the cache in the process. Similarly, when writing to the
CRL config, read first from disk (updating the cache), and on write,
also write back through the cache, providing consistency without the
need to invalidate through markConfigDirty(...).

This will form the basis of the new pattern for config caching.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Refactor ACME writing config to passthrough cache

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

---------

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

builtin/logical/pki/acme_state.go

@@ -103,7 +103,7 @@ func (a *acmeState) reloadConfigIfRequired(sc *storageContext) error {
 
 	config, err := sc.getAcmeConfig()
 	if err != nil {
-		return fmt.Errorf("failed reading config: %w", err)
+		return fmt.Errorf("failed reading ACME config: %w", err)
 	}
 
 	a.config = *config
@@ -124,6 +124,29 @@ func (a *acmeState) getConfigWithUpdate(sc *storageContext) (*acmeConfigEntry, e
 	return &configCopy, nil
 }
 
+func (a *acmeState) getConfigWithForcedUpdate(sc *storageContext) (*acmeConfigEntry, error) {
+	a.markConfigDirty()
+	return a.getConfigWithUpdate(sc)
+}
+
+func (a *acmeState) writeConfig(sc *storageContext, config *acmeConfigEntry) (*acmeConfigEntry, error) {
+	a._config.Lock()
+	defer a._config.Unlock()
+
+	if err := sc.setAcmeConfig(config); err != nil {
+		a.markConfigDirty()
+		return nil, fmt.Errorf("failed writing ACME config: %w", err)
+	}
+
+	if config != nil {
+		a.config = *config
+	} else {
+		a.config = defaultAcmeConfig
+	}
+
+	return config, nil
+}
+
 func generateRandomBase64(srcBytes int) (string, error) {
 	data := make([]byte, 21)
 	if _, err := io.ReadFull(rand.Reader, data); err != nil {

builtin/logical/pki/crl_util.go

@@ -207,6 +207,41 @@ func (cb *crlBuilder) getConfigWithUpdate(sc *storageContext) (*crlConfig, error
 	return &configCopy, nil
 }
 
+func (cb *crlBuilder) getConfigWithForcedUpdate(sc *storageContext) (*crlConfig, error) {
+	cb.markConfigDirty()
+	return cb.getConfigWithUpdate(sc)
+}
+
+func (cb *crlBuilder) writeConfig(sc *storageContext, config *crlConfig) (*crlConfig, error) {
+	cb._config.Lock()
+	defer cb._config.Unlock()
+
+	if err := sc.setRevocationConfig(config); err != nil {
+		cb.markConfigDirty()
+		return nil, fmt.Errorf("failed writing CRL config: %w", err)
+	}
+
+	previousConfig := cb.config
+	if config != nil {
+		cb.config = *config
+	} else {
+		cb.config = defaultCrlConfig
+	}
+
+	triggerChangeNotification := true
+	if !cb.haveInitializedConfig {
+		cb.haveInitializedConfig = true
+		triggerChangeNotification = false // do not trigger on the initial loading of configuration.
+	}
+
+	// Certain things need to be triggered on all server types when crlConfig is loaded.
+	if triggerChangeNotification {
+		cb.notifyOnConfigChange(sc, previousConfig, cb.config)
+	}
+
+	return config, nil
+}
+
 func (cb *crlBuilder) checkForAutoRebuild(sc *storageContext) error {
 	cfg, err := cb.getConfigWithUpdate(sc)
 	if err != nil {

builtin/logical/pki/path_config_acme.go

@@ -70,7 +70,6 @@ func (sc *storageContext) setAcmeConfig(entry *acmeConfigEntry) error {
 		return fmt.Errorf("failed writing storage entry: %w", err)
 	}
 
-	sc.Backend.acmeState.markConfigDirty()
 	return nil
 }
 
@@ -141,7 +140,7 @@ func pathAcmeConfig(b *backend) *framework.Path {
 
 func (b *backend) pathAcmeRead(ctx context.Context, req *logical.Request, _ *framework.FieldData) (*logical.Response, error) {
 	sc := b.makeStorageContext(ctx, req.Storage)
-	config, err := sc.getAcmeConfig()
+	config, err := b.acmeState.getConfigWithForcedUpdate(sc)
 	if err != nil {
 		return nil, err
 	}
@@ -178,7 +177,7 @@ func genResponseFromAcmeConfig(config *acmeConfigEntry, warnings []string) *logi
 func (b *backend) pathAcmeWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
 	sc := b.makeStorageContext(ctx, req.Storage)
 
-	config, err := sc.getAcmeConfig()
+	config, err := b.acmeState.getConfigWithForcedUpdate(sc)
 	if err != nil {
 		return nil, err
 	}
@@ -315,9 +314,8 @@ func (b *backend) pathAcmeWrite(ctx context.Context, req *logical.Request, d *fr
 		}
 	}
 
-	err = sc.setAcmeConfig(config)
-	if err != nil {
-		return nil, err
+	if _, err := b.acmeState.writeConfig(sc, config); err != nil {
+		return nil, fmt.Errorf("failed persisting: %w", err)
 	}
 
 	return genResponseFromAcmeConfig(config, warnings), nil

builtin/logical/pki/path_config_crl.go

@@ -274,9 +274,10 @@ existing CRL and OCSP paths will return the unified CRL instead of a response ba
 
 func (b *backend) pathCRLRead(ctx context.Context, req *logical.Request, _ *framework.FieldData) (*logical.Response, error) {
 	sc := b.makeStorageContext(ctx, req.Storage)
-	config, err := sc.getRevocationConfig()
+
+	config, err := b.crlBuilder.getConfigWithForcedUpdate(sc)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed fetching CRL config: %w", err)
 	}
 
 	return genResponseFromCrlConfig(config), nil
@@ -284,7 +285,7 @@ func (b *backend) pathCRLRead(ctx context.Context, req *logical.Request, _ *fram
 
 func (b *backend) pathCRLWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
 	sc := b.makeStorageContext(ctx, req.Storage)
-	config, err := sc.getRevocationConfig()
+	config, err := b.crlBuilder.getConfigWithForcedUpdate(sc)
 	if err != nil {
 		return nil, err
 	}
@@ -409,17 +410,9 @@ func (b *backend) pathCRLWrite(ctx context.Context, req *logical.Request, d *fra
 		return logical.ErrorResponse("unified_crl=true requires auto_rebuild=true, as unified CRLs cannot be rebuilt on every revocation."), nil
 	}
 
-	entry, err := logical.StorageEntryJSON("config/crl", config)
-	if err != nil {
-		return nil, err
+	if _, err := b.crlBuilder.writeConfig(sc, config); err != nil {
+		return nil, fmt.Errorf("failed persisting CRL config: %w", err)
 	}
-	err = req.Storage.Put(ctx, entry)
-	if err != nil {
-		return nil, err
-	}
-
-	b.crlBuilder.markConfigDirty()
-	b.crlBuilder.reloadConfigIfRequired(sc)
 
 	resp := genResponseFromCrlConfig(config)
 

builtin/logical/pki/storage.go

@@ -1338,6 +1338,20 @@ func (sc *storageContext) getRevocationConfig() (*crlConfig, error) {
 	return &result, nil
 }
 
+func (sc *storageContext) setRevocationConfig(config *crlConfig) error {
+	entry, err := logical.StorageEntryJSON("config/crl", config)
+	if err != nil {
+		return fmt.Errorf("failed building storage entry JSON: %w", err)
+	}
+
+	err = sc.Storage.Put(sc.Context, entry)
+	if err != nil {
+		return fmt.Errorf("failed writing storage entry: %w", err)
+	}
+
+	return nil
+}
+
 func (sc *storageContext) getAutoTidyConfig() (*tidyConfig, error) {
 	entry, err := sc.Storage.Get(sc.Context, autoTidyConfigPath)
 	if err != nil {