wip unit tests pass

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

Author: carolynvs

pkg/encoding/array_map.go

@@ -8,8 +8,6 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
-// TODO: Move this into its own package so that people can't muck with items
-
 // MapElement is the in-memory representation of the item when stored in a map.
 type MapElement interface {
 	ToArrayEntry(key string) ArrayElement
@@ -52,7 +50,26 @@ func (m *ArrayMap[T, K]) Items() map[string]T {
 	return result
 }
 
-func (m *ArrayMap[T, K]) UnsafeItems() map[string]T {
+func (m *ArrayMap[T, K]) ItemsSorted() []K {
+	if m == nil {
+		return nil
+	}
+
+	result := make([]K, len(m.items))
+	i := 0
+	for k, v := range m.items {
+		// I can't figure out how to constrain T such that ToArrayEntry returns K, so I'm doing a cast
+		result[i] = v.ToArrayEntry(k).(K)
+		i++
+	}
+	sort.SliceStable(result, func(i, j int) bool {
+		return result[i].GetKey() < result[j].GetKey()
+	})
+
+	return result
+}
+
+func (m *ArrayMap[T, K]) ItemsUnsafe() map[string]T {
 	if m == nil {
 		return nil
 	}
@@ -94,6 +111,9 @@ func (m *ArrayMap[T, K]) MarshalRaw() interface{} {
 	}
 
 	var raw []ArrayElement
+	if m.items == nil {
+		return raw
+	}
 
 	raw = make([]ArrayElement, 0, len(m.items))
 	for k, v := range m.items {

pkg/encoding/array_map_test.go

@@ -71,6 +71,7 @@ func TestArrayMap_Marshal(t *testing.T) {
 	require.NoError(t, err, "ReadFile failed")
 
 	gotData, err := yaml.Marshal(m)
+	require.NoError(t, err, "Marshal failed")
 	assert.Equal(t, string(wantData), string(gotData))
 }
 
@@ -86,7 +87,7 @@ func TestArrayMap_Unmarshal_DuplicateKeys(t *testing.T) {
 // check that when we round trip a null ArrayMap, it stays null and isn't initialized to an _empty_ ArrayMap
 // This impacts how it is marshaled later to yaml or json, because we often have fields tagged with omitempty
 // and so it must be null to not be written out.
-func TestArrayMap_RoundTrip_Null(t *testing.T) {
+func TestArrayMap_RoundTrip_Empty(t *testing.T) {
 	wantData, err := os.ReadFile("testdata/array-empty.json")
 	require.NoError(t, err, "ReadFile failed")
 

pkg/generator/credentials_test.go

@@ -51,7 +51,7 @@ func TestGoodCredentialsName(t *testing.T) {
 
 	cs, err := GenerateCredentials(opts)
 	require.NoError(t, err, "name should NOT have resulted in an error")
-	require.Equal(t, 3, cs.Credentials.Len(), "should have had 3 entries")
+	require.Equal(t, 3, cs.Len(), "should have had 3 entries")
 }
 
 func TestNoCredentials(t *testing.T) {

pkg/generator/generator_test.go

@@ -8,9 +8,7 @@ import (
 )
 
 func Test_genEmptySet(t *testing.T) {
-	expected := secrets.ValueMapping{
-		Source: secrets.Source{Hint: "TODO"},
-	}
+	expected := secrets.Source{Hint: "TODO"}
 
 	got, err := genEmptySet("emptyset", surveyParameters)
 	require.NoError(t, err)
@@ -20,5 +18,5 @@ func Test_genEmptySet(t *testing.T) {
 func Test_genSurvey_unsupported(t *testing.T) {
 	got, err := genSurvey("myturtleset", SurveyType("turtles"))
 	require.EqualError(t, err, "unsupported survey type: turtles")
-	require.Equal(t, secrets.ValueMapping{}, got)
+	require.Equal(t, secrets.Source{}, got)
 }

pkg/porter/credentials.go

@@ -272,8 +272,8 @@ func (p *Porter) ShowCredential(ctx context.Context, opts CredentialShowOptions)
 		var rows [][]string
 
 		// Iterate through all CredentialStrategies and add to rows
-		for csName, cs := range credSet.Iterate() {
-			rows = append(rows, []string{csName, cs.Source.Hint, cs.Source.Strategy})
+		for _, cs := range credSet.IterateSorted() {
+			rows = append(rows, []string{cs.Name, cs.Source.Hint, cs.Source.Strategy})
 		}
 
 		// Build and configure our tablewriter

pkg/porter/credentials_test.go

@@ -564,7 +564,7 @@ func TestPorter_CredentialsApply(t *testing.T) {
 		require.NoError(t, err, "Failed to retrieve applied credential set")
 
 		assert.Equal(t, "kool-kreds", cs.Name, "unexpected credential set name")
-		require.Len(t, cs.Credentials, 4, "expected 4 credentials in the set")
+		require.Equal(t, 4, cs.Len(), "expected 4 credentials in the set")
 		koolCred, ok := cs.Get("kool-config")
 		require.True(t, ok, "expected 'kool-config' to be present")
 		assert.Equal(t, "path", koolCred.Source.Strategy, "unexpected credential source")

pkg/porter/helpers.go

@@ -234,6 +234,7 @@ func (p *TestPorter) AddTestBundleDir(bundleDir string, generateUniqueName bool)
 // When they are different and PORTER_UPDATE_TEST_FILES is true, the file is updated to match
 // the new test output.
 func (p *TestPorter) CompareGoldenFile(goldenFile string, got string) {
+	p.T().Helper()
 	p.TestConfig.TestContext.CompareGoldenFile(goldenFile, got)
 }
 

pkg/porter/install_test.go

@@ -81,6 +81,7 @@ func TestPorter_ApplyParametersToInstallation(t *testing.T) {
 
 		newPS := storage.NewParameterSet("", "newps1")
 		newPS.SetStrategy("logLevel", secrets.HardCodedValueStrategy("11"))
+		p.TestParameters.InsertParameterSet(ctx, newPS)
 
 		inst := storage.NewInstallation("myns", "mybuns")
 		inst.Bundle = storage.OCIReferenceParts{

pkg/porter/lifecycle_test.go

@@ -448,7 +448,7 @@ func TestPorter_applyActionOptionsToInstallation_sanitizesParameters(t *testing.
 
 	err = p.applyActionOptionsToInstallation(ctx, opts, &i)
 	require.NoError(t, err)
-	require.Len(t, i.Parameters.Parameters, 2)
+	require.Equal(t, 2, i.Parameters.Len())
 
 	// there should be no sensitive value on installation record
 	for paramName, param := range i.Parameters.Iterate() {
@@ -474,11 +474,9 @@ func TestPorter_applyActionOptionsToInstallation_sanitizesParameters(t *testing.
 	// Check that when no parameter overrides are specified, we use the originally specified parameters from the previous run
 	wantParameters := &storage.ParameterSourceMap{}
 	wantParameters.Set("my-first-param", storage.ParameterSource{
-		Source:        secrets.Source{Strategy: "value", Hint: "1"},
-		ResolvedValue: "1"})
+		Source: secrets.Source{Strategy: "value", Hint: "1"}})
 	wantParameters.Set("my-second-param", storage.ParameterSource{
-		Source:        secrets.Source{Strategy: "secret", Hint: "INSTALLATIONID-my-second-param"},
-		ResolvedValue: "2"})
+		Source: secrets.Source{Strategy: "secret", Hint: "INSTALLATIONID-my-second-param"}})
 	require.Equal(t, wantParameters, i.Parameters.Parameters)
 }
 
@@ -518,23 +516,14 @@ func TestPorter_applyActionOptionsToInstallation_PreservesExistingParams(t *test
 
 	err = p.applyActionOptionsToInstallation(ctx, opts, &i)
 	require.NoError(t, err)
-	require.Len(t, i.Parameters.Parameters, 2)
+	require.Equal(t, 2, i.Parameters.Len())
 
 	// Check that overrides are applied on top of existing parameters
 	wantParameters := &storage.ParameterSourceMap{}
 	wantParameters.Set("my-first-param", storage.ParameterSource{
-		Source:        secrets.Source{Strategy: "value", Hint: "3"},
-		ResolvedValue: "3"})
+		Source: secrets.Source{Strategy: "value", Hint: "3"},
+	})
 	wantParameters.Set("my-second-param", storage.ParameterSource{
-		Source:        secrets.Source{Strategy: "secret", Hint: "INSTALLATIONID-my-second-param"},
-		ResolvedValue: "2"})
+		Source: secrets.Source{Strategy: "secret", Hint: "INSTALLATIONID-my-second-param"}})
 	require.Equal(t, wantParameters, i.Parameters.Parameters)
-
-	// Check the values stored are correct
-	params, err := p.Parameters.ResolveAll(ctx, i.Parameters)
-	require.NoError(t, err, "Failed to resolve the installation parameters")
-	require.Equal(t, secrets.Set{
-		"my-first-param":  "3", // Should have used the override
-		"my-second-param": "2", // Should have kept the existing value from the last run
-	}, params, "Incorrect parameter values were persisted on the installationß")
 }

pkg/porter/show_test.go

@@ -108,13 +108,13 @@ func TestPorter_ShowInstallationWithBundle(t *testing.T) {
 				r.BundleDigest = "sha256:88d68ef0bdb9cedc6da3a8e341a33e5d2f8bb19d0cf7ec3f1060d3f9eb73cae9"
 
 				r.ParameterOverrides = i.NewInternalParameterSet()
-				r.ParameterOverrides.SetStrategy("logLevel", secrets.HardCodedValueStrategy("3"))
-				r.ParameterOverrides.SetStrategy("secretString", secrets.HardCodedValueStrategy("foo"))
+				r.ParameterOverrides.Set("logLevel", secrets.HardCodedValue("3"))
+				r.ParameterOverrides.Set("secretString", secrets.HardCodedValue("foo"))
 
 				r.Parameters = i.NewInternalParameterSet()
-				r.ParameterOverrides.SetStrategy("logLevel", secrets.HardCodedValueStrategy("3"))
-				r.ParameterOverrides.SetStrategy("token", secrets.HardCodedValueStrategy("top-secret"))
-				r.ParameterOverrides.SetStrategy("secretString", secrets.HardCodedValueStrategy("foo"))
+				r.Parameters.Set("logLevel", secrets.HardCodedValue("3"))
+				r.Parameters.Set("token", secrets.HardCodedValue("top-secret"))
+				r.Parameters.Set("secretString", secrets.HardCodedValue("foo"))
 
 				r.ParameterSets = []string{"dev-env"}
 				r.ParameterOverrides.Parameters = p.SanitizeParameters(r.ParameterOverrides.Parameters, r.ID, bun)

pkg/portercontext/helpers.go

@@ -314,6 +314,7 @@ func (c *TestContext) hasChild(dir string, childName string) (string, bool) {
 // When they are different and PORTER_UPDATE_TEST_FILES is true, the file is updated to match
 // the new test output.
 func (c *TestContext) CompareGoldenFile(goldenFile string, got string) {
+	c.T.Helper()
 	test.CompareGoldenFile(c.T, goldenFile, got)
 }
 

pkg/secrets/map.go

@@ -13,7 +13,7 @@ func (m Map) Merge(overrides Map) Map {
 
 func (m Map) ToResolvedValues() map[string]string {
 	values := make(map[string]string, m.Len())
-	for k, v := range m.UnsafeItems() {
+	for k, v := range m.ItemsUnsafe() {
 		values[k] = v.ResolvedValue
 	}
 	return values

pkg/storage/credential_store_test.go

@@ -41,7 +41,7 @@ func TestCredentialStorage_CRUD(t *testing.T) {
 	require.NoError(t, cp.UpdateCredentialSet(context.Background(), cs))
 	cs, err = cp.GetCredentialSet(context.Background(), cs.Namespace, cs.Name)
 	require.NoError(t, err)
-	assert.Len(t, cs.Credentials, 2)
+	assert.Equal(t, 2, cs.Len())
 
 	cs2 := NewCredentialSet("dev", "sekrets-2")
 	cs2.SetStrategy("password-2", secrets.Source{

pkg/storage/credentialset.go

@@ -3,11 +3,11 @@ package storage
 import (
 	"context"
 	"fmt"
-	"get.porter.sh/porter/pkg/encoding"
 	"strings"
 	"time"
 
 	"get.porter.sh/porter/pkg/cnab"
+	"get.porter.sh/porter/pkg/encoding"
 	"get.porter.sh/porter/pkg/schema"
 	"get.porter.sh/porter/pkg/secrets"
 	"get.porter.sh/porter/pkg/tracing"
@@ -136,6 +136,10 @@ func (s CredentialSet) Iterate() map[string]CredentialSource {
 	return s.Credentials.Items()
 }
 
+func (s CredentialSet) IterateSorted() []NamedCredentialSource {
+	return s.Credentials.ItemsSorted()
+}
+
 func (s CredentialSet) SetStrategy(key string, source secrets.Source) {
 	s.Credentials.Set(key, CredentialSource{Source: source})
 }
@@ -151,6 +155,10 @@ func (s CredentialSet) Get(key string) (CredentialSource, bool) {
 	return s.Credentials.Get(key)
 }
 
+func (s CredentialSet) Len() int {
+	return s.Credentials.Len()
+}
+
 // Validate compares the given credentials with the spec.
 //
 // This will result in an error only when the following conditions are true:

pkg/storage/credentialset_test.go

@@ -30,7 +30,7 @@ func TestNewCredentialSet(t *testing.T) {
 	assert.Equal(t, cs.Status.Created, cs.Status.Modified, "Created and Modified should have the same timestamp")
 	assert.Equal(t, SchemaTypeCredentialSet, cs.SchemaType, "incorrect SchemaType")
 	assert.Equal(t, DefaultCredentialSetSchemaVersion, cs.SchemaVersion, "incorrect SchemaVersion")
-	assert.Len(t, cs.Credentials, 1, "Credentials should be initialized with 1 value")
+	assert.Equal(t, 1, cs.Len(), "Credentials should be initialized with 1 value")
 }
 
 func TestValidate(t *testing.T) {

pkg/storage/migrations/migration_test.go

@@ -236,7 +236,7 @@ func validateMigratedCredentialSets(ctx context.Context, t *testing.T, destStore
 	assert.Equal(t, "2022-06-06T16:06:52.099455-05:00", creds.Status.Created.Format(time.RFC3339Nano), "incorrect created timestamp")
 	assert.Equal(t, "2022-06-06T16:07:52.099455-05:00", creds.Status.Modified.Format(time.RFC3339Nano), "incorrect modified timestamp")
 	assert.Empty(t, creds.Labels, "incorrect labels")
-	require.Equal(t, 1, creds.Credentials.Len(), "incorrect number of credentials migrated")
+	require.Equal(t, 1, creds.Len(), "incorrect number of credentials migrated")
 
 	cred, ok := creds.Get("github-token")
 	require.True(t, ok, "expected 'github-token' to be present")
@@ -260,7 +260,7 @@ func validateMigratedParameterSets(ctx context.Context, t *testing.T, destStore
 	assert.Equal(t, "2022-06-06T16:06:21.635528-05:00", ps.Status.Created.Format(time.RFC3339Nano), "incorrect created timestamp")
 	assert.Equal(t, "2022-06-06T17:06:21.635528-05:00", ps.Status.Modified.Format(time.RFC3339Nano), "incorrect modified timestamp")
 	assert.Empty(t, ps.Labels, "incorrect labels")
-	require.Equal(t, 1, ps.Parameters.Len(), "incorrect number of parameters migrated")
+	require.Equal(t, 1, ps.Len(), "incorrect number of parameters migrated")
 
 	param, ok := ps.Get("name")
 	require.True(t, ok, "expected 'name' parameter to be present")

pkg/storage/parameterset_test.go

@@ -28,7 +28,7 @@ func TestNewParameterSet(t *testing.T) {
 	assert.Equal(t, ps.Status.Created, ps.Status.Modified, "Created and Modified should have the same timestamp")
 	assert.Equal(t, SchemaTypeParameterSet, ps.SchemaType, "incorrect SchemaType")
 	assert.Equal(t, DefaultParameterSetSchemaVersion, ps.SchemaVersion, "incorrect SchemaVersion")
-	assert.Len(t, ps.Parameters, 1, "Parameters should be initialized with 1 value")
+	assert.Equal(t, 1, ps.Len(), "Parameters should be initialized with 1 value")
 }
 
 func TestParameterSet_String(t *testing.T) {

pkg/storage/run_test.go

@@ -172,9 +172,9 @@ func TestRun_TypedParameterValues(t *testing.T) {
 	run := NewRun("dev", "mybuns")
 	run.Bundle = bun
 	run.Parameters = NewParameterSet(run.Namespace, run.Bundle.Name)
-	run.Parameters.SetStrategy("baz", secrets.HardCodedValueStrategy("baz-test"))
-	run.Parameters.SetStrategy("name", secrets.HardCodedValueStrategy("porter-test"))
-	run.Parameters.SetStrategy("porter-state", secrets.HardCodedValueStrategy(""))
+	run.Parameters.Set("baz", secrets.HardCodedValue("baz-test"))
+	run.Parameters.Set("name", secrets.HardCodedValue("porter-test"))
+	run.Parameters.Set("porter-state", secrets.HardCodedValue(""))
 	run.Parameters.Set("foo", ParameterSource{
 		Source:        secrets.Source{Strategy: "secret", Hint: "runID"},
 		ResolvedValue: "5"})

pkg/storage/testdata/marshaled_run.json

@@ -1 +1 @@
-{"schemaVersion":"","_id":"foo","created":"0001-01-01T00:00:00Z","namespace":"","installation":"","revision":"","action":"","bundleReference":"","bundleDigest":"","parameterOverrides":{"schemaVersion":"","namespace":"","name":"","parameters":[],"status":{"created":"0001-01-01T00:00:00Z","modified":"0001-01-01T00:00:00Z"}},"parameters":{"schemaVersion":"","namespace":"","name":"","parameters":[],"status":{"created":"0001-01-01T00:00:00Z","modified":"0001-01-01T00:00:00Z"}},"custom":null,"bundle":"{\"actions\":{\"logs\":{},\"test\":{\"modifies\":true}},\"description\":\"this is my bundle\",\"invocationImages\":[],\"name\":\"mybun\",\"schemaVersion\":\"schemaVersion\",\"version\":\"v0.1.0\"}"}
+{"schemaVersion":"","_id":"foo","created":"0001-01-01T00:00:00Z","namespace":"","installation":"","revision":"","action":"","bundleReference":"","bundleDigest":"","parameterOverrides":{"schemaVersion":"","namespace":"","name":"","parameters":null,"status":{"created":"0001-01-01T00:00:00Z","modified":"0001-01-01T00:00:00Z"}},"parameters":{"schemaVersion":"","namespace":"","name":"","parameters":null,"status":{"created":"0001-01-01T00:00:00Z","modified":"0001-01-01T00:00:00Z"}},"custom":null,"bundle":"{\"actions\":{\"logs\":{},\"test\":{\"modifies\":true}},\"description\":\"this is my bundle\",\"invocationImages\":[],\"name\":\"mybun\",\"schemaVersion\":\"schemaVersion\",\"version\":\"v0.1.0\"}"}

pkg/test/helper.go

@@ -69,6 +69,7 @@ func TestMainWithMockedCommandHandlers(m *testing.M) {
 // When they are different and PORTER_UPDATE_TEST_FILES is true, the file is updated to match
 // the new test output.
 func CompareGoldenFile(t *testing.T, goldenFile string, got string) {
+	t.Helper()
 	if os.Getenv("PORTER_UPDATE_TEST_FILES") == "true" {
 		os.MkdirAll(filepath.Dir(goldenFile), pkg.FileModeDirectory)
 		t.Logf("Updated test file %s to match latest test output", goldenFile)