From 59a86c97bbca8d8fa5a374904adbcccea49f8355 Mon Sep 17 00:00:00 2001 From: Kim Christensen Date: Tue, 11 Jun 2024 14:47:55 +0200 Subject: [PATCH] Call host secrets plugin directly when resolving secrets 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 --- 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