Fix race condition causing some secrets to never appear

bootstrap/bootstrap.go

@@ -114,6 +114,8 @@ func (b *Bootstrap) Run(ctx context.Context) (exitCode int) {
 		return shell.GetExitCode(err)
 	}
 
+	b.shell.Headerf("🤫 Fetching Build Secrets")
+
 	// Just pretend that these two jsons live in the pipeline.yml, and get passed through as env vars by the backend
 	secretProviderRegistryJSON := `[
   {
@@ -125,7 +127,7 @@ func (b *Bootstrap) Run(ctx context.Context) (exitCode int) {
     "id": "other-ssm",
     "type": "aws-ssm",
     "config": {
-      "role_arn": "arn:aws:iam::555555555555:role/benno-test-role-delete-after-2022-11-29"
+      "role_arn": "arn:aws:iam::253213882263:role/benno-test-role-delete-after-2022-11-29"
     }
   }
 ]`
@@ -159,7 +161,9 @@ func (b *Bootstrap) Run(ctx context.Context) (exitCode int) {
 	validationErrs := make([]error, 0, len(secretConfigs))
 	for _, c := range secretConfigs {
 		err := c.Validate()
-		validationErrs = append(validationErrs, err)
+		if err != nil {
+			validationErrs = append(validationErrs, err)
+		}
 	}
 
 	for _, err := range validationErrs {
@@ -170,7 +174,7 @@ func (b *Bootstrap) Run(ctx context.Context) (exitCode int) {
 		return 1
 	}
 
-	secrets, errors := secretProviderRegistry.FetchAll(secretConfigs)
+	fetchedSecrets, errors := secretProviderRegistry.FetchAll(secretConfigs)
 	if len(errors) > 0 {
 		b.shell.Errorf("Errors fetching secrets:")
 		for _, err := range errors {
@@ -179,14 +183,19 @@ func (b *Bootstrap) Run(ctx context.Context) (exitCode int) {
 		return 1
 	}
 
-	for _, secret := range secrets {
+	for _, secret := range fetchedSecrets {
 		// TODO: Automatically add env secrets to the redactor
 		err := secret.Store()
 		if err != nil {
 			b.shell.Errorf("Error storing secret: %v", err)
 		}
 
-		defer secret.Cleanup()
+		defer func(secret secrets.Secret) {
+			err := secret.Cleanup()
+			if err != nil {
+				b.shell.Warningf("Error cleaning up secret: %s", err)
+			}
+		}(secret)
 	}
 
 	var includePhase = func(phase string) bool {

secrets/provider_registry.go

@@ -48,42 +48,32 @@ func NewProviderRegistryFromJSON(config ProviderRegistryConfig, jsonIn string) (
 
 func (pr *ProviderRegistry) FetchAll(configs []SecretConfig) ([]Secret, []error) {
 	secrets := make([]Secret, 0, len(configs))
-	secretsCh := make(chan Secret)
-
 	errors := make([]error, 0, len(configs))
-	errorsCh := make(chan error)
 
-	var wg sync.WaitGroup
+	var (
+		wg  sync.WaitGroup
+		mtx sync.Mutex
+	)
+
+	wg.Add(len(configs))
 	for _, c := range configs {
-		wg.Add(1)
 		go func(config SecretConfig) {
 			defer wg.Done()
 
 			secret, err := pr.Fetch(config)
-			if err != nil {
+			mtx.Lock()
+			defer mtx.Unlock()
 
-				errorsCh <- err
+			if err != nil {
+				errors = append(errors, err)
 				return
 			}
-			secretsCh <- secret
-		}(c)
-	}
 
-	go func() {
-		for err := range errorsCh {
-			errors = append(errors, err)
-		}
-	}()
-
-	go func() {
-		for secret := range secretsCh {
 			secrets = append(secrets, secret)
-		}
-	}()
+		}(c)
+	}
 
 	wg.Wait()
-	close(secretsCh)
-	close(errorsCh)
 
 	return secrets, errors
 }

secrets/secret.go

@@ -9,7 +9,7 @@ import (
 
 type Secret interface {
 	Store() error
-	Cleanup() func()
+	Cleanup() error
 }
 
 func newSecret(config SecretConfig, environment env.Environment, value string) (Secret, error) {
@@ -42,8 +42,8 @@ func (s EnvSecret) Store() error {
 	return nil
 }
 
-func (s EnvSecret) Cleanup() func() {
-	return func() {}
+func (s EnvSecret) Cleanup() error {
+	return nil
 }
 
 type FileSecret struct {
@@ -62,8 +62,6 @@ func (s FileSecret) Store() error {
 	return os.WriteFile(s.FilePath, []byte(s.Value), 0777)
 }
 
-func (s FileSecret) Cleanup() func() {
-	return func() {
-		os.Remove(s.FilePath)
-	}
+func (s FileSecret) Cleanup() error {
+	return os.Remove(s.FilePath)
 }