Skip to content

Commit

Permalink
Fix race condition causing some secrets to never appear
Browse files Browse the repository at this point in the history
  • Loading branch information
moskyb committed Nov 28, 2022
1 parent 9028219 commit 177b438
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 33 deletions.
17 changes: 13 additions & 4 deletions bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 := `[
{
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
34 changes: 12 additions & 22 deletions secrets/provider_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
12 changes: 5 additions & 7 deletions secrets/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}

0 comments on commit 177b438

Please sign in to comment.