Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call host secrets plugin directly when resolving secrets #3155

Merged
merged 7 commits into from
Aug 23, 2024
Merged
40 changes: 40 additions & 0 deletions pkg/cnab/provider/credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think we could make a testcontext and pass that in here, but we don't have to (the func is like NewPorterTestContext or NewTestContext ?)

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")
}
3 changes: 1 addition & 2 deletions pkg/secrets/plugins/filesystem/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions pkg/secrets/plugins/in-memory/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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 {
Expand Down
19 changes: 14 additions & 5 deletions pkg/storage/credential_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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(),
}
}

Expand Down Expand Up @@ -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))
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/storage/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -23,3 +24,7 @@ func NewTestStore(tc *config.TestConfig) TestStore {
func (s TestStore) Close() error {
return s.testPlugin.Close()
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should comment on what this logic is reasoning about

func isHandledByHostPlugin(strategy string) bool {
return strategy == host.SourceCommand || strategy == host.SourceEnv || strategy == host.SourcePath || strategy == host.SourceValue
}
19 changes: 14 additions & 5 deletions pkg/storage/parameter_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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(),
}
}

Expand Down Expand Up @@ -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))
}
Expand Down
32 changes: 32 additions & 0 deletions pkg/storage/parameter_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading