From d61237f9a3c1e6b50d2ad65a795480974d4dbe88 Mon Sep 17 00:00:00 2001 From: Kim Christensen <2461567+kichristensen@users.noreply.github.com> Date: Fri, 23 Aug 2024 19:16:07 +0200 Subject: [PATCH] Call host secrets plugin directly when resolving secrets (#3155) We should not require all secret plugins to call the host secrets plugin for non secret values. Instead we should call the host secrets plugin directly. Signed-off-by: Kim Christensen Co-authored-by: schristoff <28318173+schristoff@users.noreply.github.com> --- pkg/cnab/provider/credentials_test.go | 40 +++++++++++++++++++++++++ pkg/secrets/plugins/filesystem/store.go | 3 +- pkg/secrets/plugins/in-memory/store.go | 5 ++-- pkg/storage/credential_store.go | 19 ++++++++---- pkg/storage/helpers.go | 5 ++++ pkg/storage/parameter_store.go | 19 ++++++++---- pkg/storage/parameter_store_test.go | 32 ++++++++++++++++++++ 7 files changed, 108 insertions(+), 15 deletions(-) diff --git a/pkg/cnab/provider/credentials_test.go b/pkg/cnab/provider/credentials_test.go index d4aac3d86..4b3f9bf26 100644 --- a/pkg/cnab/provider/credentials_test.go +++ b/pkg/cnab/provider/credentials_test.go @@ -8,6 +8,7 @@ import ( "get.porter.sh/porter/pkg/secrets" "get.porter.sh/porter/pkg/storage" "github.com/cnabio/cnab-go/bundle" + "github.com/cnabio/cnab-go/secrets/host" "github.com/cnabio/cnab-go/valuesource" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -145,3 +146,42 @@ func TestRuntime_loadCredentials_WithApplyTo(t *testing.T) { }) } + +func TestRuntime_nonSecretValue_loadCredentials(t *testing.T) { + t.Parallel() + + r := NewTestRuntime(t) + defer r.Close() + + b := cnab.NewBundle(bundle.Bundle{ + Credentials: map[string]bundle.Credential{ + "password": { + Location: bundle.Location{ + EnvironmentVariable: "PASSWORD", + }, + }, + }, + }) + + run := storage.Run{ + Action: cnab.ActionInstall, + Credentials: storage.NewInternalCredentialSet(secrets.SourceMap{ + Name: "password", + Source: secrets.Source{ + Strategy: host.SourceValue, + Hint: "mypassword", + }, + }), + } + err := r.loadCredentials(context.Background(), b, &run) + require.NoError(t, err, "loadCredentials failed") + require.Equal(t, "sha256:9b6063069a6d911421cf53b30b91836b70957c30eddc70a760eff4765b8cede5", + run.CredentialsDigest, "expected loadCredentials to set the digest of resolved credentials") + require.NotEmpty(t, run.Credentials.Credentials[0].ResolvedValue, "expected loadCredentials to set the resolved value of the credentials on the Run") + + gotValues := run.Credentials.ToCNAB() + wantValues := valuesource.Set{ + "password": "mypassword", + } + assert.Equal(t, wantValues, gotValues, "resolved unexpected credential values") +} diff --git a/pkg/secrets/plugins/filesystem/store.go b/pkg/secrets/plugins/filesystem/store.go index 61b361c45..6f8a35bc3 100644 --- a/pkg/secrets/plugins/filesystem/store.go +++ b/pkg/secrets/plugins/filesystem/store.go @@ -90,8 +90,7 @@ func (s *Store) Resolve(ctx context.Context, keyName string, keyValue string) (s // check if the keyName is secret if keyName != secrets.SourceSecret { - value, err := s.hostStore.Resolve(ctx, keyName, keyValue) - return value, log.Error(err) + return "", log.Errorf("unsupported keyName %s", keyName) } path := filepath.Join(s.secretDir, keyValue) diff --git a/pkg/secrets/plugins/in-memory/store.go b/pkg/secrets/plugins/in-memory/store.go index 1fab53609..c4d8381c5 100644 --- a/pkg/secrets/plugins/in-memory/store.go +++ b/pkg/secrets/plugins/in-memory/store.go @@ -3,9 +3,9 @@ package inmemory import ( "context" "errors" + "fmt" "get.porter.sh/porter/pkg/secrets/plugins" - "github.com/cnabio/cnab-go/secrets/host" ) var _ plugins.SecretsProtocol = &Store{} @@ -38,8 +38,7 @@ func (s *Store) Resolve(ctx context.Context, keyName string, keyValue string) (s return value, nil } - hostStore := host.SecretStore{} - return hostStore.Resolve(keyName, keyValue) + return "", fmt.Errorf("unsupported keyName %s", keyName) } func (s *Store) Create(ctx context.Context, keyName string, keyValue string, value string) error { diff --git a/pkg/storage/credential_store.go b/pkg/storage/credential_store.go index 86e76535a..d0a177402 100644 --- a/pkg/storage/credential_store.go +++ b/pkg/storage/credential_store.go @@ -6,6 +6,7 @@ import ( "strings" "get.porter.sh/porter/pkg/secrets" + hostSecrets "get.porter.sh/porter/pkg/secrets/plugins/host" "get.porter.sh/porter/pkg/tracing" "github.com/cnabio/cnab-go/secrets/host" "github.com/hashicorp/go-multierror" @@ -21,14 +22,16 @@ const ( // providing typed access and additional business logic around // credential sets, usually referred to as "credentials" as a shorthand. type CredentialStore struct { - Documents Store - Secrets secrets.Store + Documents Store + Secrets secrets.Store + HostSecrets hostSecrets.Store } func NewCredentialStore(storage Store, secrets secrets.Store) *CredentialStore { return &CredentialStore{ - Documents: storage, - Secrets: secrets, + Documents: storage, + Secrets: secrets, + HostSecrets: hostSecrets.NewStore(), } } @@ -62,7 +65,13 @@ func (s CredentialStore) ResolveAll(ctx context.Context, creds CredentialSet) (s var resolveErrors error for _, cred := range creds.Credentials { - value, err := s.Secrets.Resolve(ctx, cred.Source.Strategy, cred.Source.Hint) + var value string + var err error + if isHandledByHostPlugin(cred.Source.Strategy) { + value, err = s.HostSecrets.Resolve(ctx, cred.Source.Strategy, cred.Source.Hint) + } else { + value, err = s.Secrets.Resolve(ctx, cred.Source.Strategy, cred.Source.Hint) + } if err != nil { resolveErrors = multierror.Append(resolveErrors, fmt.Errorf("unable to resolve credential %s.%s from %s %s: %w", creds.Name, cred.Name, cred.Source.Strategy, cred.Source.Hint, err)) } diff --git a/pkg/storage/helpers.go b/pkg/storage/helpers.go index 31e814dd4..da44fc026 100644 --- a/pkg/storage/helpers.go +++ b/pkg/storage/helpers.go @@ -3,6 +3,7 @@ package storage import ( "get.porter.sh/porter/pkg/config" "get.porter.sh/porter/pkg/storage/plugins/testplugin" + "github.com/cnabio/cnab-go/secrets/host" ) var _ Store = TestStore{} @@ -23,3 +24,7 @@ func NewTestStore(tc *config.TestConfig) TestStore { func (s TestStore) Close() error { return s.testPlugin.Close() } + +func isHandledByHostPlugin(strategy string) bool { + return strategy == host.SourceCommand || strategy == host.SourceEnv || strategy == host.SourcePath || strategy == host.SourceValue +} diff --git a/pkg/storage/parameter_store.go b/pkg/storage/parameter_store.go index cb72e3125..c0f885ee8 100644 --- a/pkg/storage/parameter_store.go +++ b/pkg/storage/parameter_store.go @@ -6,6 +6,7 @@ import ( "strings" "get.porter.sh/porter/pkg/secrets" + hostSecrets "get.porter.sh/porter/pkg/secrets/plugins/host" "get.porter.sh/porter/pkg/tracing" "github.com/cnabio/cnab-go/secrets/host" "github.com/hashicorp/go-multierror" @@ -20,14 +21,16 @@ const ( // ParameterStore provides access to parameter sets by instantiating plugins that // implement CRUD storage. type ParameterStore struct { - Documents Store - Secrets secrets.Store + Documents Store + Secrets secrets.Store + HostSecrets hostSecrets.Store } func NewParameterStore(storage Store, secrets secrets.Store) *ParameterStore { return &ParameterStore{ - Documents: storage, - Secrets: secrets, + Documents: storage, + Secrets: secrets, + HostSecrets: hostSecrets.NewStore(), } } @@ -56,7 +59,13 @@ func (s ParameterStore) ResolveAll(ctx context.Context, params ParameterSet) (se var resolveErrors error for _, param := range params.Parameters { - value, err := s.Secrets.Resolve(ctx, param.Source.Strategy, param.Source.Hint) + var value string + var err error + if isHandledByHostPlugin(param.Source.Strategy) { + value, err = s.HostSecrets.Resolve(ctx, param.Source.Strategy, param.Source.Hint) + } else { + value, err = s.Secrets.Resolve(ctx, param.Source.Strategy, param.Source.Hint) + } if err != nil { resolveErrors = multierror.Append(resolveErrors, fmt.Errorf("unable to resolve parameter %s.%s from %s %s: %w", params.Name, param.Name, param.Source.Strategy, param.Source.Hint, err)) } diff --git a/pkg/storage/parameter_store_test.go b/pkg/storage/parameter_store_test.go index 7fa4be96d..b0644442e 100644 --- a/pkg/storage/parameter_store_test.go +++ b/pkg/storage/parameter_store_test.go @@ -90,6 +90,38 @@ func TestParameterStore_CRUD(t *testing.T) { require.ErrorIs(t, err, ErrNotFound{}) } +func TestParameterStorage_ResolveNonSecret(t *testing.T) { + testParameterSet := NewParameterSet("", "myparamset", + secrets.SourceMap{ + Name: "param1", + Source: secrets.Source{ + Strategy: "secret", + Hint: "param1", + }, + }, + secrets.SourceMap{ + Name: "param2", + Source: secrets.Source{ + Strategy: "value", + Hint: "param2_value", + }, + }) + + paramStore := NewTestParameterProvider(t) + defer paramStore.Close() + + paramStore.AddSecret("param1", "param1_value") + + expected := secrets.Set{ + "param1": "param1_value", + "param2": "param2_value", + } + + resolved, err := paramStore.ResolveAll(context.Background(), testParameterSet) + require.NoError(t, err) + require.Equal(t, expected, resolved) +} + func TestParameterStorage_ResolveAll(t *testing.T) { // The inmemory secret store currently only supports secret sources // So all of these have this same source