Browse Source

fix: propagate commit error to caller so it becomes user visible (#4451)

...also ignore empty state in state manager
Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>
Moritz Johner 1 year ago
parent
commit
109cfce3ce

+ 5 - 3
pkg/controllers/externalsecret/externalsecret_controller_secret.go

@@ -37,8 +37,7 @@ import (
 )
 )
 
 
 // getProviderSecretData returns the provider's secret data with the provided ExternalSecret.
 // getProviderSecretData returns the provider's secret data with the provided ExternalSecret.
-func (r *Reconciler) getProviderSecretData(ctx context.Context, externalSecret *esv1beta1.ExternalSecret) (map[string][]byte, error) {
-	var err error
+func (r *Reconciler) getProviderSecretData(ctx context.Context, externalSecret *esv1beta1.ExternalSecret) (providerData map[string][]byte, err error) {
 	// We MUST NOT create multiple instances of a provider client (mostly due to limitations with GCP)
 	// We MUST NOT create multiple instances of a provider client (mostly due to limitations with GCP)
 	// Clientmanager keeps track of the client instances
 	// Clientmanager keeps track of the client instances
 	// that are created during the fetching process and closes clients
 	// that are created during the fetching process and closes clients
@@ -63,9 +62,12 @@ func (r *Reconciler) getProviderSecretData(ctx context.Context, externalSecret *
 		}
 		}
 		if commitErr := genState.Commit(); commitErr != nil {
 		if commitErr := genState.Commit(); commitErr != nil {
 			r.Log.Error(commitErr, "error committing generator state")
 			r.Log.Error(commitErr, "error committing generator state")
+			// At this point the original error can only be a NoSecretErr
+			// but we should return the commit error here as it's more important.
+			err = commitErr
 		}
 		}
 	}()
 	}()
-	providerData := make(map[string][]byte)
+	providerData = make(map[string][]byte)
 	for i, remoteRef := range externalSecret.Spec.DataFrom {
 	for i, remoteRef := range externalSecret.Spec.DataFrom {
 		var secretMap map[string][]byte
 		var secretMap map[string][]byte
 
 

+ 4 - 0
pkg/generator/statemanager/statemanager.go

@@ -123,6 +123,10 @@ func (m *Manager) EnqueueMoveStateToGC(stateKey string) {
 // EnqueueSetLatest sets the latest state for the given key.
 // EnqueueSetLatest sets the latest state for the given key.
 // It will commit the state on success or move the state to GC on failure.
 // It will commit the state on success or move the state to GC on failure.
 func (m *Manager) EnqueueSetLatest(ctx context.Context, stateKey, namespace string, resource *apiextensions.JSON, gen genapi.Generator, state genapi.GeneratorProviderState) {
 func (m *Manager) EnqueueSetLatest(ctx context.Context, stateKey, namespace string, resource *apiextensions.JSON, gen genapi.Generator, state genapi.GeneratorProviderState) {
+	if state == nil {
+		return
+	}
+
 	m.queue = append(m.queue, QueueItem{
 	m.queue = append(m.queue, QueueItem{
 		// Stores the state in GeneratorState resource
 		// Stores the state in GeneratorState resource
 		Commit: func() error {
 		Commit: func() error {