Split Installation into a spec and status (getporter#2435)

* Split Installation into a spec and status I have updated Installation to be composed of * Instance specific fields that make testing difficult, such as the ID. * Spec - all fields that a user would set on an installation * Status - our own internal tracking of the installation status. This makes it easier for us to test or work with just the values the user specified by having a spec that won't have auto-generated values like IDs, timestamps, etc. It also enables us to embed an installation spec in another data structure, like a workflow that runs multiple installations, without having unnecessary fields embeded like the status and id. It lets us think about just the desired state of the installation. Signed-off-by: Carolyn Van Slyck <[email protected]> * Move schemaVersion into InstallationSpec Since schemaVersion is user-specified, it should be available on the spec, and not hidden from the user in a field directly on Installation Signed-off-by: Carolyn Van Slyck <[email protected]> Signed-off-by: Carolyn Van Slyck <[email protected]>
reveurguy · Oct 27, 2022 · f5f1e25 · f5f1e25
1 parent 57dc2b6
commit f5f1e25
Show file tree

Hide file tree

Showing 11 changed files with 77 additions and 40 deletions.
diff --git a/pkg/cnab/provider/credentials_test.go b/pkg/cnab/provider/credentials_test.go
@@ -49,7 +49,12 @@ func TestRuntime_loadCredentials(t *testing.T) {
 		},
 	})
 
-	args := ActionArguments{Installation: storage.Installation{CredentialSets: []string{"mycreds"}}, Action: "install"}
+	args := ActionArguments{
+		Installation: storage.Installation{
+			InstallationSpec: storage.InstallationSpec{
+				CredentialSets: []string{"mycreds"}},
+		},
+		Action: "install"}
 	gotValues, err := r.loadCredentials(context.Background(), b, args)
 	require.NoError(t, err, "loadCredentials failed")
 
@@ -58,7 +63,12 @@ func TestRuntime_loadCredentials(t *testing.T) {
 	}
 	assert.Equal(t, wantValues, gotValues, "resolved unexpected credential values")
 
-	args = ActionArguments{Installation: storage.Installation{CredentialSets: []string{"/db-creds.json"}}, Action: "install"}
+	args = ActionArguments{
+		Installation: storage.Installation{
+			InstallationSpec: storage.InstallationSpec{
+				CredentialSets: []string{"/db-creds.json"}},
+		},
+		Action: "install"}
 	_, err = r.loadCredentials(context.Background(), b, args)
 	require.Error(t, err, "loadCredentials should not load from a file")
 }
@@ -137,7 +147,12 @@ func TestRuntime_loadCredentials_WithApplyTo(t *testing.T) {
 		require.NoError(t, err, "Save credential set failed")
 
 		b := getBundle(true)
-		args := ActionArguments{Installation: storage.Installation{CredentialSets: []string{"mycreds"}}, Action: "install"}
+		args := ActionArguments{
+			Installation: storage.Installation{
+				InstallationSpec: storage.InstallationSpec{
+					CredentialSets: []string{"mycreds"}},
+			},
+			Action: "install"}
 		gotValues, err := r.loadCredentials(context.Background(), b, args)
 		require.NoError(t, err, "loadCredentials failed")
 		assert.Equal(t, secrets.Set{"password": "mypassword"}, gotValues)

diff --git a/pkg/porter/apply.go b/pkg/porter/apply.go
@@ -85,12 +85,12 @@ func (p *Porter) InstallationApply(ctx context.Context, opts ApplyOptions) error
 
 		// Create a new installation
 		installation = storage.NewInstallation(input.Namespace, input.Name)
-		installation.Apply(inputInstallation)
+		installation.Apply(inputInstallation.InstallationSpec)
 
 		log.Info("Creating a new installation", attribute.String("installation", installation.String()))
 	} else {
 		// Apply the specified changes to the installation
-		installation.Apply(inputInstallation)
+		installation.Apply(inputInstallation.InstallationSpec)
 		if err := installation.Validate(); err != nil {
 			return err
 		}

diff --git a/pkg/porter/install_test.go b/pkg/porter/install_test.go
@@ -84,14 +84,14 @@ func TestPorter_applyActionOptionsToInstallation(t *testing.T) {
 			},
 		})
 
-		return ctx, p, &storage.Installation{
+		return ctx, p, &storage.Installation{InstallationSpec: storage.InstallationSpec{
 			Bundle: storage.OCIReferenceParts{
 				Repository: "example.com/mybuns",
 				Version:    "1.0.0",
 			},
 			ParameterSets:  []string{"oldps1"},
 			CredentialSets: []string{"oldcs1", "oldcs2"},
-		}
+		}}
 	}
 
 	t.Run("replace previous sets", func(t *testing.T) {

diff --git a/pkg/porter/invoke.go b/pkg/porter/invoke.go
@@ -63,7 +63,7 @@ func (p *Porter) InvokeBundle(ctx context.Context, opts InvokeOptions) error {
 		}
 
 		// Create an ephemeral installation just for this run
-		installation = storage.Installation{Namespace: opts.Namespace, Name: opts.Name}
+		installation = storage.Installation{InstallationSpec: storage.InstallationSpec{Namespace: opts.Namespace, Name: opts.Name}}
 	}
 	err = p.applyActionOptionsToInstallation(ctx, &installation, opts.BundleExecutionOptions)
 	if err != nil {

diff --git a/pkg/porter/lifecycle_test.go b/pkg/porter/lifecycle_test.go
@@ -118,7 +118,8 @@ func TestPorter_BuildActionArgs(t *testing.T) {
 
 		err := opts.Validate(ctx, nil, p.Porter)
 		require.NoError(t, err, "Validate failed")
-		existingInstall := storage.Installation{Name: opts.Name}
+		existingInstall := storage.Installation{InstallationSpec: storage.InstallationSpec{
+			Name: opts.Name}}
 		args, err := p.BuildActionArgs(ctx, existingInstall, opts)
 		require.NoError(t, err, "BuildActionArgs failed")
 

diff --git a/pkg/porter/list.go b/pkg/porter/list.go
@@ -152,17 +152,19 @@ func NewDisplayInstallation(installation storage.Installation) DisplayInstallati
 // a Installation record.
 func (d DisplayInstallation) ConvertToInstallation() (storage.Installation, error) {
 	i := storage.Installation{
-		SchemaVersion:  d.SchemaVersion,
-		ID:             d.ID,
-		Name:           d.Name,
-		Namespace:      d.Namespace,
-		Uninstalled:    d.Uninstalled,
-		Bundle:         d.Bundle,
-		Custom:         d.Custom,
-		Labels:         d.Labels,
-		CredentialSets: d.CredentialSets,
-		ParameterSets:  d.ParameterSets,
-		Status:         d.Status,
+		ID: d.ID,
+		InstallationSpec: storage.InstallationSpec{
+			SchemaVersion:  d.SchemaVersion,
+			Name:           d.Name,
+			Namespace:      d.Namespace,
+			Uninstalled:    d.Uninstalled,
+			Bundle:         d.Bundle,
+			Custom:         d.Custom,
+			Labels:         d.Labels,
+			CredentialSets: d.CredentialSets,
+			ParameterSets:  d.ParameterSets,
+		},
+		Status: d.Status,
 	}
 
 	var err error

diff --git a/pkg/porter/parameters_test.go b/pkg/porter/parameters_test.go
@@ -309,7 +309,7 @@ func Test_loadParameters_ParameterSourcePrecedence(t *testing.T) {
 		b, err := cnab.LoadBundle(r.Context, "bundle.json")
 		require.NoError(t, err, "ProcessBundle failed")
 
-		i := storage.Installation{Name: "mybun"}
+		i := storage.Installation{InstallationSpec: storage.InstallationSpec{Name: "mybun"}}
 		params, err := r.resolveParameters(context.Background(), i, b, cnab.ActionUpgrade, nil)
 		require.NoError(t, err)
 		assert.Equal(t, "foo_default", params["foo"],
@@ -333,7 +333,7 @@ func Test_loadParameters_ParameterSourcePrecedence(t *testing.T) {
 			"foo": "foo_override",
 		}
 
-		i := storage.Installation{Name: "mybun"}
+		i := storage.Installation{InstallationSpec: storage.InstallationSpec{Name: "mybun"}}
 		params, err := r.resolveParameters(context.Background(), i, b, cnab.ActionUpgrade, overrides)
 		require.NoError(t, err)
 		assert.Equal(t, "foo_override", params["foo"],
@@ -571,7 +571,7 @@ func Test_Paramapalooza(t *testing.T) {
 						bun.Parameters["my-param"] = param
 					}
 
-					i := storage.Installation{Name: "test"}
+					i := storage.Installation{InstallationSpec: storage.InstallationSpec{Name: "test"}}
 					overrides := map[string]string{}
 					// If param is provided (via --param/--param-file)
 					// it will be attached to args

diff --git a/pkg/porter/reconcile_test.go b/pkg/porter/reconcile_test.go
@@ -21,9 +21,9 @@ func TestPorter_IsInstallationInSync(t *testing.T) {
 		p := NewTestPorter(t)
 		defer p.Close()
 
-		i := storage.Installation{
+		i := storage.Installation{InstallationSpec: storage.InstallationSpec{
 			Uninstalled: true,
-		}
+		}}
 		insync, err := p.IsInstallationInSync(p.RootContext, i, nil, NewInstallOptions())
 		require.NoError(t, err)
 		assert.True(t, insync)
@@ -109,7 +109,9 @@ func TestPorter_IsInstallationInSync(t *testing.T) {
 		defer p.Close()
 
 		i := storage.Installation{
-			CredentialSets: []string{"newcreds"},
+			InstallationSpec: storage.InstallationSpec{
+				CredentialSets: []string{"newcreds"},
+			},
 			Status: storage.InstallationStatus{
 				Installed: &now,
 			},
@@ -133,7 +135,9 @@ func TestPorter_IsInstallationInSync(t *testing.T) {
 		defer p.Close()
 
 		i := storage.Installation{
-			Uninstalled: true, // trigger uninstall
+			InstallationSpec: storage.InstallationSpec{
+				Uninstalled: true, // trigger uninstall
+			},
 			Status: storage.InstallationStatus{
 				Installed: &now,
 			},
@@ -150,7 +154,9 @@ func TestPorter_IsInstallationInSync(t *testing.T) {
 
 		installTime := now.Add(-time.Second * 5)
 		i := storage.Installation{
-			Uninstalled: false,
+			InstallationSpec: storage.InstallationSpec{
+				Uninstalled: false,
+			},
 			Status: storage.InstallationStatus{
 				Installed:   &installTime,
 				Uninstalled: &now,

diff --git a/pkg/storage/installation.go b/pkg/storage/installation.go
@@ -19,12 +19,20 @@ import (
 var _ Document = Installation{}
 
 type Installation struct {
-	// SchemaVersion is the version of the installation state schema.
-	SchemaVersion schema.Version `json:"schemaVersion"`
-
 	// ID is the unique identifier for an installation record.
 	ID string `json:"id"`
 
+	InstallationSpec
+
+	// Status of the installation.
+	Status InstallationStatus `json:"status,omitempty"`
+}
+
+// InstallationSpec contains installation fields that represent the desired state of the installation.
+type InstallationSpec struct {
+	// SchemaVersion is the version of the installation state schema.
+	SchemaVersion schema.Version `json:"schemaVersion"`
+
 	// Name of the installation. Immutable.
 	Name string `json:"name"`
 
@@ -58,7 +66,7 @@ type Installation struct {
 	Status InstallationStatus `json:"status,omitempty"`
 }
 
-func (i Installation) String() string {
+func (i InstallationSpec) String() string {
 	return fmt.Sprintf("%s/%s", i.Namespace, i.Name)
 }
 
@@ -69,11 +77,13 @@ func (i Installation) DefaultDocumentFilter() map[string]interface{} {
 func NewInstallation(namespace string, name string) Installation {
 	now := time.Now()
 	return Installation{
-		SchemaVersion: InstallationSchemaVersion,
-		ID:            cnab.NewULID(),
-		Namespace:     namespace,
-		Name:          name,
-		Parameters:    NewInternalParameterSet(namespace, name),
+		ID: cnab.NewULID(),
+		InstallationSpec: InstallationSpec{
+			SchemaVersion: InstallationSchemaVersion,
+			Namespace:     namespace,
+			Name:          name,
+			Parameters:    NewInternalParameterSet(namespace, name),
+		},
 		Status: InstallationStatus{
 			Created:  now,
 			Modified: now,
@@ -115,7 +125,7 @@ func (i *Installation) ApplyResult(run Run, result Result) {
 // Apply user-provided changes to an existing installation.
 // Only updates fields that users are allowed to modify.
 // For example, Name, Namespace and Status cannot be modified.
-func (i *Installation) Apply(input Installation) {
+func (i *InstallationSpec) Apply(input InstallationSpec) {
 	i.Uninstalled = input.Uninstalled
 	i.Bundle = input.Bundle
 	i.Parameters = input.Parameters

diff --git a/pkg/storage/installation_test.go b/pkg/storage/installation_test.go
@@ -12,7 +12,7 @@ import (
 func TestInstallation_String(t *testing.T) {
 	t.Parallel()
 
-	i := Installation{Name: "mybun"}
+	i := Installation{InstallationSpec: InstallationSpec{Name: "mybun"}}
 	assert.Equal(t, "/mybun", i.String())
 
 	i.Namespace = "dev"

diff --git a/pkg/storage/migrations/manager_test.go b/pkg/storage/migrations/manager_test.go
@@ -42,7 +42,10 @@ func TestManager_LoadSchema(t *testing.T) {
 		m := NewTestManager(c)
 		defer m.Close()
 
-		err := m.store.Insert(context.Background(), storage.CollectionInstallations, storage.InsertOptions{Documents: []interface{}{storage.Installation{Name: "abc123"}}})
+		i := storage.Installation{InstallationSpec: storage.InstallationSpec{
+			Name: "abc123",
+		}}
+		err := m.store.Insert(context.Background(), storage.CollectionInstallations, storage.InsertOptions{Documents: []interface{}{i}})
 		require.NoError(t, err)
 
 		err = m.loadSchema(context.Background())