From 14234ca1b3aa0f0d4f4ff4e01b7d04584668ce2f Mon Sep 17 00:00:00 2001 From: Robert Lin Date: Thu, 5 May 2022 09:17:18 -0700 Subject: [PATCH] dev/sg: unify ExternalSecrets with secrets store, minor fixes for frontend-e2e (#34942) --- dev/sg/internal/run/command.go | 43 ++++++++++--------------- dev/sg/internal/run/run.go | 3 +- dev/sg/internal/secrets/secret.go | 22 +++++++++++++ dev/sg/internal/secrets/store.go | 52 +++++++++++++++++++++++++++++++ sg.config.yaml | 13 +++++--- 5 files changed, 101 insertions(+), 32 deletions(-) create mode 100644 dev/sg/internal/secrets/secret.go diff --git a/dev/sg/internal/run/command.go b/dev/sg/internal/run/command.go index 3630fc80c35..c21e26106a4 100644 --- a/dev/sg/internal/run/command.go +++ b/dev/sg/internal/run/command.go @@ -2,14 +2,12 @@ package run import ( "context" - "fmt" "io" "os/exec" - secretmanager "cloud.google.com/go/secretmanager/apiv1" "golang.org/x/sync/errgroup" - secretmanagerpb "google.golang.org/genproto/googleapis/cloud/secretmanager/v1" + "github.com/sourcegraph/sourcegraph/dev/sg/internal/secrets" "github.com/sourcegraph/sourcegraph/dev/sg/internal/stdout" "github.com/sourcegraph/sourcegraph/lib/errors" "github.com/sourcegraph/sourcegraph/lib/output" @@ -27,20 +25,15 @@ type Command struct { IgnoreStderr bool `yaml:"ignoreStderr"` DefaultArgs string `yaml:"defaultArgs"` ContinueWatchOnExit bool `yaml:"continueWatchOnExit"` - Secrets map[string]Secret `yaml:"secrets"` // Preamble is a short and visible message, displayed when the command is launched. Preamble string `yaml:"preamble"` + ExternalSecrets map[string]secrets.ExternalSecret `yaml:"external_secrets"` + // ATTENTION: If you add a new field here, be sure to also handle that // field in `Merge` (below). } -type Secret struct { - Provider string `yaml:"provider"` - Project string `yaml:"project"` - Name string `yaml:"name"` -} - func (c Command) Merge(other Command) Command { merged := c @@ -71,8 +64,8 @@ func (c Command) Merge(other Command) Command { merged.Env[k] = v } - for k, v := range other.Secrets { - merged.Secrets[k] = v + for k, v := range other.ExternalSecrets { + merged.ExternalSecrets[k] = v } if !equal(merged.Watch, other.Watch) && len(other.Watch) != 0 { @@ -133,29 +126,24 @@ func (sc *startedCmd) CapturedStderr() string { func getSecrets(ctx context.Context, cmd Command) (map[string]string, error) { secretsEnv := map[string]string{} - if len(cmd.Secrets) == 0 { + if len(cmd.ExternalSecrets) == 0 { return secretsEnv, nil } - client, err := secretmanager.NewClient(ctx) + secretsStore, err := secrets.FromContext(ctx) if err != nil { return nil, errors.Errorf("failed to create secretmanager client: %v", err) } - for envName, secret := range cmd.Secrets { - if secret.Provider != "gcloud" { - errors.Newf("Unknown secrets provider %s", secret.Provider) - } - path := fmt.Sprintf("projects/%s/secrets/%s/versions/latest", secret.Project, secret.Name) - req := &secretmanagerpb.AccessSecretVersionRequest{ - Name: path, - } - result, err := client.AccessSecretVersion(ctx, req) + + var errs error + for envName, secret := range cmd.ExternalSecrets { + secretsEnv[envName], err = secretsStore.GetExternal(ctx, secret) if err != nil { - return nil, errors.Wrapf(err, "failed to access secret %s from %s", secret.Name, secret.Project) + errs = errors.Append(errs, + errors.Wrapf(err, "failed to access secret %q for command %q", envName, cmd.Name)) } - secretsEnv[envName] = string(result.Payload.Data) } - return secretsEnv, nil + return secretsEnv, errs } func startCmd(ctx context.Context, dir string, cmd Command, parentEnv map[string]string) (*startedCmd, error) { @@ -172,7 +160,8 @@ func startCmd(ctx context.Context, dir string, cmd Command, parentEnv map[string secretsEnv, err := getSecrets(ctx, cmd) if err != nil { - return nil, errors.Wrapf(err, "cannot fetch secrets") + stdout.Out.WriteLine(output.Linef("", output.StyleWarning, "[%s] %s %s", + cmd.Name, output.EmojiFailure, err.Error())) } sc.Cmd.Env = makeEnv(parentEnv, secretsEnv, cmd.Env) diff --git a/dev/sg/internal/run/run.go b/dev/sg/internal/run/run.go index ed417315431..282bed79bd5 100644 --- a/dev/sg/internal/run/run.go +++ b/dev/sg/internal/run/run.go @@ -585,7 +585,8 @@ func Test(ctx context.Context, cmd Command, args []string, parentEnv map[string] secretsEnv, err := getSecrets(ctx, cmd) if err != nil { - return errors.Wrapf(err, "cannot fetch secrets") + stdout.Out.WriteLine(output.Linef("", output.StyleWarning, "[%s] %s %s", + cmd.Name, output.EmojiFailure, err.Error())) } if cmd.Preamble != "" { diff --git a/dev/sg/internal/secrets/secret.go b/dev/sg/internal/secrets/secret.go new file mode 100644 index 00000000000..ce35340b704 --- /dev/null +++ b/dev/sg/internal/secrets/secret.go @@ -0,0 +1,22 @@ +package secrets + +import ( + "fmt" + "time" +) + +type ExternalSecret struct { + Provider string `yaml:"provider"` + Project string `yaml:"project"` + Name string `yaml:"name"` +} + +func (s *ExternalSecret) id() string { + return fmt.Sprintf("%s/%s/%s", s.Provider, s.Project, s.Name) +} + +// externalSecretValue is the stored representation of an external secret's value +type externalSecretValue struct { + Fetched time.Time + Value string +} diff --git a/dev/sg/internal/secrets/store.go b/dev/sg/internal/secrets/store.go index 42df2931b31..26fc82114f8 100644 --- a/dev/sg/internal/secrets/store.go +++ b/dev/sg/internal/secrets/store.go @@ -3,8 +3,14 @@ package secrets import ( "context" "encoding/json" + "fmt" "io" "os" + "sync" + "time" + + secretmanager "cloud.google.com/go/secretmanager/apiv1" + secretmanagerpb "google.golang.org/genproto/googleapis/cloud/secretmanager/v1" "github.com/sourcegraph/sourcegraph/lib/errors" ) @@ -21,6 +27,10 @@ var ( type Store struct { filepath string m map[string]json.RawMessage + + secretmanagerOnce sync.Once + secretmanager *secretmanager.Client + secretmanagerErr error } type storeKey struct{} @@ -103,6 +113,37 @@ func (s *Store) Get(key string, target interface{}) error { return errors.Newf("%w: %s not found", ErrSecretNotFound, key) } +func (s *Store) GetExternal(ctx context.Context, secret ExternalSecret) (string, error) { + var value externalSecretValue + + // Check if we already have this secret + if err := s.Get(secret.id(), &value); err == nil { + return value.Value, nil + } + + if secret.Provider != "gcloud" { + return "", errors.Newf("Unknown secrets provider %q", secret.Provider) + } + + client, err := s.getSecretmanagerClient(ctx) + if err != nil { + return "", err + } + result, err := client.AccessSecretVersion(ctx, &secretmanagerpb.AccessSecretVersionRequest{ + Name: fmt.Sprintf("projects/%s/secrets/%s/versions/latest", secret.Project, secret.Name), + }) + if err != nil { + return "", errors.Wrapf(err, "failed to access secret %q from %q", secret.Name, secret.Project) + } + + // cache value, but don't save - TBD if we want to persist these secrets + value.Fetched = time.Now() + value.Value = string(result.Payload.Data) + s.Put(secret.id(), &value) + + return value.Value, nil +} + // Remove deletes a value from memory. func (s *Store) Remove(key string) error { if _, exists := s.m[key]; exists { @@ -120,3 +161,14 @@ func (s *Store) Keys() []string { } return keys } + +func (s *Store) getSecretmanagerClient(ctx context.Context) (*secretmanager.Client, error) { + s.secretmanagerOnce.Do(func() { + var err error + s.secretmanager, err = secretmanager.NewClient(ctx) + if err != nil { + s.secretmanagerErr = errors.Errorf("failed to create secretmanager client: %v", err) + } + }) + return s.secretmanager, s.secretmanagerErr +} diff --git a/sg.config.yaml b/sg.config.yaml index 64e1993f4e2..baa741448dc 100644 --- a/sg.config.yaml +++ b/sg.config.yaml @@ -752,7 +752,7 @@ commandsets: enterprise-e2e: <<: *enterprise_set env: - # EXTSVC_CONFIG_FILE being set prevents the e2e test suite to add + # EXTSVC_CONFIG_FILE being set prevents the e2e test suite to add # additional connections. EXTSVC_CONFIG_FILE: "" @@ -968,10 +968,15 @@ tests: frontend-e2e: preamble: | - sg start enterprise-e2e must be already running for these tests to work. + 'sg start enterprise-e2e' must be already running for these tests to work. If you run into authentication issues, you can run the following commands to fix them: - sg db reset-pg && sg db add-user -name test@sourcegraph.com -password supersecurepassword + + sg db reset-pg && sg db add-user --username 'test' --password 'supersecurepassword' + + The above command resets the database and creates a user like so: + + 👉 User test (test@sourcegraph.com) has been created and its password is supersecurepassword . cmd: | yarn run test-e2e env: @@ -979,7 +984,7 @@ tests: TEST_USER_PASSWORD: supersecurepassword SOURCEGRAPH_BASE_URL: https://sourcegraph.test:3443 BROWSER: chrome - secrets: + external_secrets: GH_TOKEN: provider: "gcloud" project: "sourcegraph-ci"