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

Do not resolve parameters defined in parameter sets if they are overriden #3309

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions pkg/porter/parameters.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ func validateParameterName(args []string) error {
}

// loadParameterSets loads parameter values per their parameter set strategies
func (p *Porter) loadParameterSets(ctx context.Context, bun cnab.ExtendedBundle, namespace string, params []string) (secrets.Set, error) {
func (p *Porter) loadParameterSets(ctx context.Context, bun cnab.ExtendedBundle, namespace string, params []string, overridenParameters secrets.StrategyList) (secrets.Set, error) {
resolvedParameters := secrets.Set{}

for _, name := range params {
Expand All @@ -380,6 +380,13 @@ func (p *Porter) loadParameterSets(ctx context.Context, bun cnab.ExtendedBundle,
return nil, err
}

var skipParams []string
for _, param := range pset.Parameters {
if overridenParameters.Contains(param.Name) {
skipParams = append(skipParams, param.Name)
}
}

// A parameter may correspond to a Porter-specific parameter type of 'file'
// If so, add value (filepath) directly to map and remove from pset
for paramName, paramDef := range bun.Parameters {
Expand All @@ -402,7 +409,7 @@ func (p *Porter) loadParameterSets(ctx context.Context, bun cnab.ExtendedBundle,
}
}

rc, err := p.Parameters.ResolveAll(ctx, pset)
rc, err := p.Parameters.ResolveAll(ctx, pset, skipParams...)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -884,7 +891,7 @@ func (p *Porter) applyActionOptionsToInstallation(ctx context.Context, ba Bundle
//
// 3. Resolve named parameter sets
//
resolvedParams, err := p.loadParameterSets(ctx, bun, o.Namespace, inst.ParameterSets)
resolvedParams, err := p.loadParameterSets(ctx, bun, o.Namespace, inst.ParameterSets, inst.Parameters.Parameters)
if err != nil {
return fmt.Errorf("unable to process provided parameter sets: %w", err)
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/secrets/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,12 @@ func (l StrategyList) Swap(i, j int) {
func (l StrategyList) Len() int {
return len(l)
}

func (l StrategyList) Contains(name string) bool {
for _, param := range l {
if param.Name == name {
return true
}
}
return false
}
7 changes: 6 additions & 1 deletion pkg/storage/parameter_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package storage
import (
"context"
"fmt"
"slices"
"strings"

"get.porter.sh/porter/pkg/secrets"
Expand Down Expand Up @@ -54,11 +55,15 @@ func (s ParameterStore) GetDataStore() Store {
return s.Documents
}

func (s ParameterStore) ResolveAll(ctx context.Context, params ParameterSet) (secrets.Set, error) {
func (s ParameterStore) ResolveAll(ctx context.Context, params ParameterSet, skipParams ...string) (secrets.Set, error) {
resolvedParams := make(secrets.Set)
var resolveErrors error

for _, param := range params.Parameters {
if slices.Contains(skipParams, param.Name) {
continue
}

var value string
var err error
if isHandledByHostPlugin(param.Source.Strategy) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/parameterset_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type ParameterSetProvider interface {
GetDataStore() Store

// ResolveAll parameter values in the parameter set.
ResolveAll(ctx context.Context, params ParameterSet) (secrets.Set, error)
ResolveAll(ctx context.Context, params ParameterSet, skipParams ...string) (secrets.Set, error)

// Validate the parameter set is defined properly.
Validate(ctx context.Context, params ParameterSet) error
Expand Down
30 changes: 30 additions & 0 deletions tests/integration/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,36 @@ func TestInstall_stringParam(t *testing.T) {
require.Contains(t, output, "Hello, Demo Time", "expected action output to contain provided file contents")
}

func TestInstall_overrideParamIncludedInParamSet(t *testing.T) {

p := porter.NewTestPorter(t)
defer p.Close()
ctx := p.SetupIntegrationTest()

p.AddTestBundleDir("testdata/bundles/bundle-with-string-params", false)

installOpts := porter.NewInstallOptions()
installOpts.Params = []string{"name=Demo Time"}
installOpts.ParameterSets = []string{"myparam"}
testParamSets := storage.NewParameterSet("", "myparam", secrets.SourceMap{
Name: "name",
Source: secrets.Source{
Strategy: host.SourceEnv,
Hint: "DEMO_TIME",
},
})
p.TestParameters.InsertParameterSet(ctx, testParamSets)

err := installOpts.Validate(ctx, []string{}, p.Porter)
require.NoError(t, err)

err = p.InstallBundle(ctx, installOpts)
require.NoError(t, err)

output := p.TestConfig.TestContext.GetOutput()
require.Contains(t, output, "Hello, Demo Time", "expected action output to contain provided file contents")
}

func TestInstall_SensitiveValuesAreNotLogged(t *testing.T) {
p := porter.NewTestPorter(t)
defer p.Close()
Expand Down
Loading