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 <[email protected]>
Co-authored-by: schristoff <[email protected]>

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")
+}

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)

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 {

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))
 		}

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
+}

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))
 		}

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