Use secrets.Map for ParameterSourceMap

Signed-off-by: Carolyn Van Slyck <[email protected]>

Author: carolynvs

pkg/encoding/array_map.go

@@ -24,7 +24,6 @@ type ArrayMap[T MapElement, K ArrayElement] struct {
 	items map[string]T
 }
 
-// TODO(carolyn): Can I make this work without K?
 func MakeArrayMap[T MapElement, K ArrayElement](len int) ArrayMap[T, K] {
 	return ArrayMap[T, K]{
 		items: make(map[string]T, len),
@@ -133,6 +132,10 @@ func (m *ArrayMap[T, K]) UnmarshalRaw(raw []K) error {
 		return nil
 	}
 
+	if m == nil {
+		*m = ArrayMap[T, K]{}
+	}
+
 	m.items = make(map[string]T, len(raw))
 	for _, rawItem := range raw {
 		if _, hasKey := m.items[rawItem.GetKey()]; hasKey {

pkg/generator/parameters_test.go

@@ -115,5 +115,5 @@ func TestSkipParameters(t *testing.T) {
 	pset, err := opts.GenerateParameters()
 	require.NoError(t, err, "parameters generation should not have resulted in an error")
 	assert.Equal(t, "skip-params", pset.Name, "Name was not set")
-	require.Empty(t, pset.Parameters, "parameter set should have empty parameters section")
+	require.Equal(t, 0, pset.Len(), "parameter set should have empty parameters section")
 }

pkg/porter/helpers.go

@@ -240,7 +240,7 @@ func (p *TestPorter) CompareGoldenFile(goldenFile string, got string) {
 
 // CreateInstallation saves an installation record into claim store and store
 // sensitive parameters into secret store.
-func (p *TestPorter) SanitizeParameters(raw *storage.ParameterSourceMap, recordID string, bun cnab.ExtendedBundle) *storage.ParameterSourceMap {
+func (p *TestPorter) SanitizeParameters(raw storage.ParameterSourceMap, recordID string, bun cnab.ExtendedBundle) storage.ParameterSourceMap {
 	strategies, err := p.Sanitizer.CleanParameters(context.Background(), raw, bun, recordID)
 	require.NoError(p.T(), err)
 

pkg/porter/lifecycle_test.go

@@ -472,7 +472,7 @@ func TestPorter_applyActionOptionsToInstallation_sanitizesParameters(t *testing.
 	require.NoError(t, err)
 
 	// Check that when no parameter overrides are specified, we use the originally specified parameters from the previous run
-	wantParameters := &storage.ParameterSourceMap{}
+	wantParameters := storage.NewParameterSourceMap()
 	wantParameters.Set("my-first-param", storage.ParameterSource{
 		Source: secrets.Source{Strategy: "value", Hint: "1"}})
 	wantParameters.Set("my-second-param", storage.ParameterSource{
@@ -519,7 +519,7 @@ func TestPorter_applyActionOptionsToInstallation_PreservesExistingParams(t *test
 	require.Equal(t, 2, i.Parameters.Len())
 
 	// Check that overrides are applied on top of existing parameters
-	wantParameters := &storage.ParameterSourceMap{}
+	wantParameters := storage.NewParameterSourceMap()
 	wantParameters.Set("my-first-param", storage.ParameterSource{
 		Source: secrets.Source{Strategy: "value", Hint: "3"},
 	})

pkg/porter/parameters_test.go

@@ -859,7 +859,7 @@ func TestPorter_ParametersApply(t *testing.T) {
 
 		assert.Equal(t, "mypset", ps.Name, "unexpected parameter set name")
 
-		wantParams := &storage.ParameterSourceMap{}
+		wantParams := storage.NewParameterSourceMap()
 		wantParams.Set("foo", storage.ParameterSource{
 			Source: secrets.Source{
 				Strategy: "secret",

pkg/secrets/map.go

@@ -1,11 +1,29 @@
 package secrets
 
-import "get.porter.sh/porter/pkg/encoding"
+import (
+	"encoding/json"
+
+	"get.porter.sh/porter/pkg/encoding"
+	"gopkg.in/yaml.v3"
+)
 
 type Map struct {
 	*encoding.ArrayMap[ValueMapping, NamedValueMapping]
 }
 
+func NewMap() Map {
+	return Map{
+		ArrayMap: &encoding.ArrayMap[ValueMapping, NamedValueMapping]{},
+	}
+}
+
+func MakeMap(len int) Map {
+	items := encoding.MakeArrayMap[ValueMapping, NamedValueMapping](len)
+	return Map{
+		ArrayMap: &items,
+	}
+}
+
 func (m Map) Merge(overrides Map) Map {
 	result := m.ArrayMap.Merge(overrides.ArrayMap)
 	return Map{ArrayMap: result}
@@ -19,6 +37,30 @@ func (m Map) ToResolvedValues() map[string]string {
 	return values
 }
 
+func (m *Map) UnmarshalJSON(b []byte) error {
+	// Ensure that ArrayMap is not nil before its custom UnmarshalJson is called
+	// Otherwise it causes a panic
+	if m.ArrayMap == nil {
+		m.ArrayMap = &encoding.ArrayMap[ValueMapping, NamedValueMapping]{}
+	}
+
+	// Cheap trick to unmarshal without re-triggering this UnmarshalJson call
+	type RawMap Map
+	return json.Unmarshal(b, (*RawMap)(m))
+}
+
+func (m *Map) UnmarshalYAML(value *yaml.Node) error {
+	// Ensure that ArrayMap is not nil before its custom UnmarshalYAML is called
+	// Otherwise it causes a panic
+	if m.ArrayMap == nil {
+		m.ArrayMap = &encoding.ArrayMap[ValueMapping, NamedValueMapping]{}
+	}
+
+	// Cheap trick to unmarshal without re-triggering this UnmarshalYAML call
+	type RawMap Map
+	return value.Decode((*RawMap)(m))
+}
+
 var _ encoding.MapElement = ValueMapping{}
 
 // ValueMapping maps from a parameter or credential name to a source strategy for resolving its value.

pkg/storage/migrations/migration.go

@@ -591,16 +591,15 @@ func convertParameterSet(namespace string, data []byte) (storage.ParameterSet, e
 		},
 	}
 
-	destParams := storage.MakeParameterSourceMap(len(src.Parameters))
-	dest.Parameters = &destParams
+	dest.Parameters = storage.MakeParameterSourceMap(len(src.Parameters))
 	for _, srcParam := range src.Parameters {
 		destParam := storage.ParameterSource{
 			Source: secrets.Source{
 				Strategy: srcParam.Source.Key,
 				Hint:     srcParam.Source.Value,
 			},
 		}
-		destParams.Set(srcParam.Name, destParam)
+		dest.Parameters.Set(srcParam.Name, destParam)
 	}
 
 	return dest, nil

pkg/storage/migrations/migration_test.go

@@ -153,7 +153,7 @@ func validateMigratedInstallations(ctx context.Context, t *testing.T, c *config.
 	assert.Empty(t, inst.Labels, "We didn't allow setting labels on installations in v0, so this can't be populated")
 	assert.Empty(t, inst.CredentialSets, "We didn't track credential sets used when running a bundle in v0, so this can't be populated")
 	assert.Empty(t, inst.ParameterSets, "We didn't track parameter sets used when running a bundle in v0, so this can't be populated")
-	assert.Empty(t, inst.Parameters.Parameters, "We didn't track manually specified parameters when running a bundle in v0, so this can't be populated")
+	assert.Equal(t, 0, inst.Parameters.Len(), "We didn't track manually specified parameters when running a bundle in v0, so this can't be populated")
 
 	// Validate the installation status, which is calculated based on the runs and their results
 	assert.Equal(t, "2022-04-28T16:09:42.65907-05:00", inst.Status.Created.Format(time.RFC3339Nano), "Created timestamp should be set to the timestamp of the first run")
@@ -185,7 +185,7 @@ func validateMigratedInstallations(ctx context.Context, t *testing.T, c *config.
 	assert.Empty(t, lastRun.Custom, "We didn't set custom datadata on claims in v0, so this can't be populated")
 	assert.Equal(t, "2022-04-29T16:13:20.48026-05:00", lastRun.Created.Format(time.RFC3339Nano), "incorrect run created timestamp")
 	assert.Empty(t, lastRun.ParameterSets, "We didn't track run parameter sets in v0, so this can't be populated")
-	assert.Empty(t, lastRun.ParameterOverrides, "We didn't track run parameter overrides in v0, so this can't be populated")
+	assert.Equal(t, 0, lastRun.ParameterOverrides.Len(), "We didn't track run parameter overrides in v0, so this can't be populated")
 	assert.Empty(t, lastRun.CredentialSets, "We didn't track run credential sets in v0, so this can't be populated")
 	assert.Equal(t, 1, lastRun.Parameters.Len(), "expected one parameter set on the run")
 

pkg/storage/parameterset.go

@@ -3,7 +3,6 @@ package storage
 import (
 	"context"
 	"fmt"
-	"get.porter.sh/porter/pkg/encoding"
 	"strings"
 	"time"
 
@@ -45,12 +44,13 @@ type ParameterSetSpec struct {
 	Labels map[string]string `json:"labels,omitempty" yaml:"labels,omitempty" toml:"labels,omitempty"`
 
 	// Parameters is a list of parameter specs.
-	Parameters *ParameterSourceMap `json:"parameters" yaml:"parameters" toml:"parameters"`
+	Parameters ParameterSourceMap `json:"parameters" yaml:"parameters" toml:"parameters"`
 }
 
-type ParameterSourceMap = encoding.ArrayMap[ParameterSource, NamedParameterSource]
+type ParameterSourceMap = secrets.Map
 
-var MakeParameterSourceMap = encoding.MakeArrayMap[ParameterSource, NamedParameterSource]
+var MakeParameterSourceMap = secrets.MakeMap
+var NewParameterSourceMap = secrets.NewMap
 
 // TODO(generics)
 type ParameterSource = secrets.ValueMapping
@@ -74,7 +74,7 @@ func NewParameterSet(namespace string, name string) ParameterSet {
 			SchemaVersion: DefaultParameterSetSchemaVersion,
 			Namespace:     namespace,
 			Name:          name,
-			Parameters:    &ParameterSourceMap{},
+			Parameters:    NewParameterSourceMap(),
 		},
 		Status: ParameterSetStatus{
 			Created:  now,
@@ -132,9 +132,6 @@ func (s ParameterSet) String() string {
 }
 
 func (s ParameterSet) Iterate() map[string]ParameterSource {
-	if s.Parameters == nil {
-		return nil
-	}
 	return s.Parameters.Items()
 }
 
@@ -143,9 +140,6 @@ func (s ParameterSet) SetStrategy(key string, source secrets.Source) {
 }
 
 func (s ParameterSet) Set(key string, source ParameterSource) {
-	if s.Parameters == nil { // TODO(carolyn): do this in all the places
-		s.Parameters = &ParameterSourceMap{}
-	}
 	s.Parameters.Set(key, source)
 }
 

pkg/storage/run_test.go

@@ -210,5 +210,5 @@ func TestRun_MarshalJSON(t *testing.T) {
 	err = json.Unmarshal(data, &r2)
 	require.NoError(t, err, "Unmarshal failed")
 
-	assert.Equal(t, r1, r2, "The run did not survive the round trip")
+	assert.Equal(t, r1.Bundle, r2.Bundle, "The Run's bundle did not survive the round trip")
 }

pkg/storage/sanitizer.go

@@ -29,20 +29,20 @@ func NewSanitizer(parameterstore ParameterSetProvider, secretstore secrets.Store
 // transform the raw value into secret strategies.
 // The id argument is used to associate the reference key with the corresponding
 // run or installation record in porter's database.
-func (s *Sanitizer) CleanRawParameters(ctx context.Context, params map[string]interface{}, bun cnab.ExtendedBundle, id string) (*ParameterSourceMap, error) {
+func (s *Sanitizer) CleanRawParameters(ctx context.Context, params map[string]interface{}, bun cnab.ExtendedBundle, id string) (ParameterSourceMap, error) {
 	strategies := MakeParameterSourceMap(len(params))
 	for name, value := range params {
 		stringVal, err := bun.WriteParameterToString(name, value)
 		if err != nil {
-			return nil, err
+			return ParameterSourceMap{}, err
 		}
 
 		strategies.Set(name, secrets.HardCodedValue(stringVal))
 	}
 
-	result, err := s.CleanParameters(ctx, &strategies, bun, id)
+	result, err := s.CleanParameters(ctx, strategies, bun, id)
 	if err != nil {
-		return nil, err
+		return ParameterSourceMap{}, err
 	}
 
 	return result, nil
@@ -53,15 +53,15 @@ func (s *Sanitizer) CleanRawParameters(ctx context.Context, params map[string]in
 // Sanitized value after saving sensitive data to secrets store.
 // The id argument is used to associate the reference key with the corresponding
 // run or installation record in porter's database.
-func (s *Sanitizer) CleanParameters(ctx context.Context, dirtyParams *ParameterSourceMap, bun cnab.ExtendedBundle, id string) (*ParameterSourceMap, error) {
+func (s *Sanitizer) CleanParameters(ctx context.Context, dirtyParams ParameterSourceMap, bun cnab.ExtendedBundle, id string) (ParameterSourceMap, error) {
 	cleanedParams := MakeParameterSourceMap(dirtyParams.Len())
 	for paramName, param := range dirtyParams.Items() {
 		// Store sensitive hard-coded values in a secret store
 		if param.Source.Strategy == host.SourceValue && bun.IsSensitiveParameter(paramName) {
 			cleaned := sanitizedParam(paramName, param, id)
 			err := s.secrets.Create(ctx, cleaned.Source.Strategy, cleaned.Source.Hint, cleaned.ResolvedValue)
 			if err != nil {
-				return nil, fmt.Errorf("failed to save sensitive param to secrete store: %w", err)
+				return ParameterSourceMap{}, fmt.Errorf("failed to save sensitive param to secrete store: %w", err)
 			}
 
 			cleanedParams.Set(paramName, cleaned)
@@ -71,10 +71,10 @@ func (s *Sanitizer) CleanParameters(ctx context.Context, dirtyParams *ParameterS
 	}
 
 	if cleanedParams.Len() == 0 {
-		return nil, nil
+		return NewParameterSourceMap(), nil
 	}
 
-	return &cleanedParams, nil
+	return cleanedParams, nil
 
 }
 

pkg/storage/sanitizer_test.go

@@ -28,7 +28,7 @@ func TestSanitizer_Parameters(t *testing.T) {
 	recordID := "01FZVC5AVP8Z7A78CSCP1EJ604"
 	sensitiveParamName := "my-second-param"
 	sensitiveParamKey := recordID + "-" + sensitiveParamName
-	expected := &storage.ParameterSourceMap{}
+	expected := storage.NewParameterSourceMap()
 	expected.Set("my-first-param", secrets.HardCodedValue("1"))
 	expected.Set(sensitiveParamName, storage.ParameterSource{Source: secrets.Source{
 		Strategy: secrets.SourceSecret,
@@ -108,7 +108,7 @@ func TestSanitizer_CleanParameters(t *testing.T) {
 			gotParams, err := r.Sanitizer.CleanParameters(ctx, inst.Parameters.Parameters, bun, inst.ID)
 			require.NoError(t, err, "CleanParameters failed")
 
-			wantParms := &storage.ParameterSourceMap{}
+			wantParms := storage.NewParameterSourceMap()
 			wantParms.Set(tc.paramName, storage.ParameterSource{Source: tc.wantSource})
 			require.Equal(t, wantParms, gotParams, "unexpected value returned from CleanParameters")
 		})