diff --git a/cmd/porter/credentials.go b/cmd/porter/credentials.go index 31c934c94..30d878411 100644 --- a/cmd/porter/credentials.go +++ b/cmd/porter/credentials.go @@ -143,7 +143,8 @@ The results may also be filtered by associated labels and the namespace in which porter credentials list --namespace prod porter credentials list --all-namespaces, porter credentials list --name myapp - porter credentials list --label env=dev`, + porter credentials list --label env=dev + porter credentials list --skip 2 --limit 2`, PreRunE: func(cmd *cobra.Command, args []string) error { return opts.Validate() }, @@ -163,6 +164,10 @@ The results may also be filtered by associated labels and the namespace in which "Filter the credential sets by a label formatted as: KEY=VALUE. May be specified multiple times.") f.StringVarP(&opts.RawFormat, "output", "o", "plaintext", "Specify an output format. Allowed values: plaintext, json, yaml") + f.Int64Var(&opts.Skip, "skip", 0, + "Skip the number of credential sets by a certain amount. Defaults to 0.") + f.Int64Var(&opts.Limit, "limit", 0, + "Limit the number of credential sets by a certain amount. Defaults to 0.") return cmd } diff --git a/cmd/porter/installations.go b/cmd/porter/installations.go index b78b16abd..04abb8b4a 100644 --- a/cmd/porter/installations.go +++ b/cmd/porter/installations.go @@ -44,7 +44,8 @@ Optional output formats include json and yaml.`, porter installations list -o json porter installations list --all-namespaces, porter installations list --label owner=myname --namespace dev - porter installations list --name myapp`, + porter installations list --name myapp + porter installations list --skip 2 --limit 2`, PreRunE: func(cmd *cobra.Command, args []string) error { return opts.Validate() }, @@ -64,6 +65,10 @@ Optional output formats include json and yaml.`, "Filter the installations by a label formatted as: KEY=VALUE. May be specified multiple times.") f.StringVarP(&opts.RawFormat, "output", "o", "plaintext", "Specify an output format. Allowed values: plaintext, json, yaml") + f.Int64Var(&opts.Skip, "skip", 0, + "Skip the number of installations by a certain amount. Defaults to 0.") + f.Int64Var(&opts.Limit, "limit", 0, + "Limit the number of installations by a certain amount. Defaults to 0.") return cmd } diff --git a/cmd/porter/parameters.go b/cmd/porter/parameters.go index ef004dd48..ea00b938c 100644 --- a/cmd/porter/parameters.go +++ b/cmd/porter/parameters.go @@ -142,7 +142,8 @@ The results may also be filtered by associated labels and the namespace in which porter parameters list --namespace prod -o json porter parameters list --all-namespaces, porter parameters list --name myapp - porter parameters list --label env=dev`, + porter parameters list --label env=dev + porter parameters list --skip 2 --limit 2`, PreRunE: func(cmd *cobra.Command, args []string) error { return opts.Validate() }, @@ -162,6 +163,10 @@ The results may also be filtered by associated labels and the namespace in which "Filter the parameter sets by a label formatted as: KEY=VALUE. May be specified multiple times.") f.StringVarP(&opts.RawFormat, "output", "o", "plaintext", "Specify an output format. Allowed values: plaintext, json, yaml") + f.Int64Var(&opts.Skip, "skip", 0, + "Skip the number of parameter sets by a certain amount. Defaults to 0.") + f.Int64Var(&opts.Limit, "limit", 0, + "Limit the number of parameter sets by a certain amount. Defaults to 0.") return cmd } diff --git a/docs/content/cli/build.md b/docs/content/cli/build.md index 83f25cd28..155de2177 100644 --- a/docs/content/cli/build.md +++ b/docs/content/cli/build.md @@ -39,8 +39,8 @@ porter build [flags] ``` --build-arg stringArray Set build arguments in the template Dockerfile (format: NAME=VALUE). May be specified multiple times. --custom stringArray Define an individual key-value pair for the custom section in the form of NAME=VALUE. Use dot notation to specify a nested custom field. May be specified multiple times. - -d, --dir string Path to the build context directory where all bundle assets are located. - -f, --file porter.yaml Path to the Porter manifest. Defaults to porter.yaml in the current directory. + -d, --dir string Path to the build context directory where all bundle assets are located. Defaults to the current directory. + -f, --file string Path to the Porter manifest. The path is relative to the build context directory. Defaults to porter.yaml in the current directory. -h, --help help for build --name string Override the bundle name --no-cache Do not use the Docker cache when building the bundle's invocation image. diff --git a/docs/content/cli/bundles_build.md b/docs/content/cli/bundles_build.md index 9f7a0f1bf..6498ac0df 100644 --- a/docs/content/cli/bundles_build.md +++ b/docs/content/cli/bundles_build.md @@ -39,8 +39,8 @@ porter bundles build [flags] ``` --build-arg stringArray Set build arguments in the template Dockerfile (format: NAME=VALUE). May be specified multiple times. --custom stringArray Define an individual key-value pair for the custom section in the form of NAME=VALUE. Use dot notation to specify a nested custom field. May be specified multiple times. - -d, --dir string Path to the build context directory where all bundle assets are located. - -f, --file porter.yaml Path to the Porter manifest. Defaults to porter.yaml in the current directory. + -d, --dir string Path to the build context directory where all bundle assets are located. Defaults to the current directory. + -f, --file string Path to the Porter manifest. The path is relative to the build context directory. Defaults to porter.yaml in the current directory. -h, --help help for build --name string Override the bundle name --no-cache Do not use the Docker cache when building the bundle's invocation image. diff --git a/docs/content/cli/credentials_list.md b/docs/content/cli/credentials_list.md index 5a5940d53..d0ec4fef5 100644 --- a/docs/content/cli/credentials_list.md +++ b/docs/content/cli/credentials_list.md @@ -26,6 +26,7 @@ porter credentials list [flags] porter credentials list --all-namespaces, porter credentials list --name myapp porter credentials list --label env=dev + porter credentials list --skip 2 --limit 2 ``` ### Options @@ -34,9 +35,11 @@ porter credentials list [flags] --all-namespaces Include all namespaces in the results. -h, --help help for list -l, --label strings Filter the credential sets by a label formatted as: KEY=VALUE. May be specified multiple times. + --limit int Limit the number of credential sets by a certain amount. Defaults to 0. --name string Filter the credential sets where the name contains the specified substring. -n, --namespace string Namespace in which the credential set is defined. Defaults to the global namespace. Use * to list across all namespaces. -o, --output string Specify an output format. Allowed values: plaintext, json, yaml (default "plaintext") + --skip int Skip the number of credential sets by a certain amount. Defaults to 0. ``` ### Options inherited from parent commands diff --git a/docs/content/cli/installations_list.md b/docs/content/cli/installations_list.md index 33e68c361..081fdf281 100644 --- a/docs/content/cli/installations_list.md +++ b/docs/content/cli/installations_list.md @@ -29,6 +29,7 @@ porter installations list [flags] porter installations list --all-namespaces, porter installations list --label owner=myname --namespace dev porter installations list --name myapp + porter installations list --skip 2 --limit 2 ``` ### Options @@ -37,9 +38,11 @@ porter installations list [flags] --all-namespaces Include all namespaces in the results. -h, --help help for list -l, --label strings Filter the installations by a label formatted as: KEY=VALUE. May be specified multiple times. + --limit int Limit the number of installations by a certain amount. Defaults to 0. --name string Filter the installations where the name contains the specified substring. -n, --namespace string Filter the installations by namespace. Defaults to the global namespace. -o, --output string Specify an output format. Allowed values: plaintext, json, yaml (default "plaintext") + --skip int Skip the number of installations by a certain amount. Defaults to 0. ``` ### Options inherited from parent commands diff --git a/docs/content/cli/list.md b/docs/content/cli/list.md index b3388da37..d7a44d0cd 100644 --- a/docs/content/cli/list.md +++ b/docs/content/cli/list.md @@ -29,6 +29,7 @@ porter list [flags] porter list --all-namespaces, porter list --label owner=myname --namespace dev porter list --name myapp + porter list --skip 2 --limit 2 ``` ### Options @@ -37,9 +38,11 @@ porter list [flags] --all-namespaces Include all namespaces in the results. -h, --help help for list -l, --label strings Filter the installations by a label formatted as: KEY=VALUE. May be specified multiple times. + --limit int Limit the number of installations by a certain amount. Defaults to 0. --name string Filter the installations where the name contains the specified substring. -n, --namespace string Filter the installations by namespace. Defaults to the global namespace. -o, --output string Specify an output format. Allowed values: plaintext, json, yaml (default "plaintext") + --skip int Skip the number of installations by a certain amount. Defaults to 0. ``` ### Options inherited from parent commands diff --git a/docs/content/cli/parameters_list.md b/docs/content/cli/parameters_list.md index 5787a42c9..a6ade63e6 100644 --- a/docs/content/cli/parameters_list.md +++ b/docs/content/cli/parameters_list.md @@ -26,6 +26,7 @@ porter parameters list [flags] porter parameters list --all-namespaces, porter parameters list --name myapp porter parameters list --label env=dev + porter parameters list --skip 2 --limit 2 ``` ### Options @@ -34,9 +35,11 @@ porter parameters list [flags] --all-namespaces Include all namespaces in the results. -h, --help help for list -l, --label strings Filter the parameter sets by a label formatted as: KEY=VALUE. May be specified multiple times. + --limit int Limit the number of parameter sets by a certain amount. Defaults to 0. --name string Filter the parameter sets where the name contains the specified substring. -n, --namespace string Namespace in which the parameter set is defined. Defaults to the global namespace. Use * to list across all namespaces. -o, --output string Specify an output format. Allowed values: plaintext, json, yaml (default "plaintext") + --skip int Skip the number of parameter sets by a certain amount. Defaults to 0. ``` ### Options inherited from parent commands diff --git a/pkg/cli/config.go b/pkg/cli/config.go index 2f7ca3328..f7ce91413 100644 --- a/pkg/cli/config.go +++ b/pkg/cli/config.go @@ -71,6 +71,8 @@ func getViperValue(flags *pflag.FlagSet, f *pflag.Flag) interface{} { switch flagType { case "int": out, err = flags.GetInt(f.Name) + case "int64": + out, err = flags.GetInt64(f.Name) case "string": out, err = flags.GetString(f.Name) case "bool": diff --git a/pkg/porter/credentials.go b/pkg/porter/credentials.go index f30b1365f..05e83720d 100644 --- a/pkg/porter/credentials.go +++ b/pkg/porter/credentials.go @@ -31,7 +31,13 @@ type CredentialEditOptions struct { // ListCredentials lists saved credential sets. func (p *Porter) ListCredentials(ctx context.Context, opts ListOptions) ([]storage.CredentialSet, error) { - return p.Credentials.ListCredentialSets(ctx, opts.GetNamespace(), opts.Name, opts.ParseLabels()) + return p.Credentials.ListCredentialSets(ctx, storage.ListOptions{ + Namespace: opts.GetNamespace(), + Name: opts.Name, + Labels: opts.ParseLabels(), + Skip: opts.Skip, + Limit: opts.Limit, + }) } // PrintCredentials prints saved credential sets. diff --git a/pkg/porter/list.go b/pkg/porter/list.go index a8aeb8765..84b746543 100644 --- a/pkg/porter/list.go +++ b/pkg/porter/list.go @@ -24,6 +24,8 @@ type ListOptions struct { Namespace string Name string Labels []string + Skip int64 + Limit int64 } func (o *ListOptions) Validate() error { @@ -218,7 +220,13 @@ func (p *Porter) ListInstallations(ctx context.Context, opts ListOptions) (Displ ctx, log := tracing.StartSpan(ctx) defer log.EndSpan() - installations, err := p.Installations.ListInstallations(ctx, opts.GetNamespace(), opts.Name, opts.ParseLabels()) + installations, err := p.Installations.ListInstallations(ctx, storage.ListOptions{ + Namespace: opts.GetNamespace(), + Name: opts.Name, + Labels: opts.ParseLabels(), + Skip: opts.Skip, + Limit: opts.Limit, + }) if err != nil { return nil, log.Error(fmt.Errorf("could not list installations: %w", err)) } diff --git a/pkg/porter/parameters.go b/pkg/porter/parameters.go index 2dc9467db..218944f65 100644 --- a/pkg/porter/parameters.go +++ b/pkg/porter/parameters.go @@ -38,7 +38,13 @@ type ParameterEditOptions struct { // ListParameters lists saved parameter sets. func (p *Porter) ListParameters(ctx context.Context, opts ListOptions) ([]storage.ParameterSet, error) { - return p.Parameters.ListParameterSets(ctx, opts.GetNamespace(), opts.Name, opts.ParseLabels()) + return p.Parameters.ListParameterSets(ctx, storage.ListOptions{ + Namespace: opts.GetNamespace(), + Name: opts.Name, + Labels: opts.ParseLabels(), + Skip: opts.Skip, + Limit: opts.Limit, + }) } // PrintParameters prints saved parameter sets. diff --git a/pkg/storage/credential_store.go b/pkg/storage/credential_store.go index 71acd6008..e30ee6d51 100644 --- a/pkg/storage/credential_store.go +++ b/pkg/storage/credential_store.go @@ -110,12 +110,9 @@ func (s CredentialStore) InsertCredentialSet(ctx context.Context, creds Credenti return s.Documents.Insert(ctx, CollectionCredentials, opts) } -func (s CredentialStore) ListCredentialSets(ctx context.Context, namespace string, name string, labels map[string]string) ([]CredentialSet, error) { +func (s CredentialStore) ListCredentialSets(ctx context.Context, listOptions ListOptions) ([]CredentialSet, error) { var out []CredentialSet - opts := FindOptions{ - Filter: CreateListFiler(namespace, name, labels), - } - err := s.Documents.Find(ctx, CollectionCredentials, opts, &out) + err := s.Documents.Find(ctx, CollectionCredentials, listOptions.ToFindOptions(), &out) return out, err } diff --git a/pkg/storage/credential_store_test.go b/pkg/storage/credential_store_test.go index 9ee62abc0..0336ad917 100644 --- a/pkg/storage/credential_store_test.go +++ b/pkg/storage/credential_store_test.go @@ -20,17 +20,17 @@ func TestCredentialStorage_CRUD(t *testing.T) { require.NoError(t, cp.InsertCredentialSet(context.Background(), cs)) - creds, err := cp.ListCredentialSets(context.Background(), "dev", "", nil) + creds, err := cp.ListCredentialSets(context.Background(), ListOptions{Namespace: "dev"}) require.NoError(t, err) require.Len(t, creds, 1, "expected 1 credential set") - require.Equal(t, cs.Name, creds[0].Name, "expected to retrieve secreks credentials") - require.Equal(t, cs.Namespace, creds[0].Namespace, "expected to retrieve secreks credentials") + require.Equal(t, cs.Name, creds[0].Name, "expected to retrieve sekrets credentials") + require.Equal(t, cs.Namespace, creds[0].Namespace, "expected to retrieve sekrets credentials") - creds, err = cp.ListCredentialSets(context.Background(), "", "", nil) + creds, err = cp.ListCredentialSets(context.Background(), ListOptions{}) require.NoError(t, err) require.Len(t, creds, 0, "expected no global credential sets") - creds, err = cp.ListCredentialSets(context.Background(), "*", "", nil) + creds, err = cp.ListCredentialSets(context.Background(), ListOptions{Namespace: "*"}) require.NoError(t, err) require.Len(t, creds, 1, "expected 1 credential set defined in all namespaces") @@ -45,9 +45,30 @@ func TestCredentialStorage_CRUD(t *testing.T) { require.NoError(t, err) assert.Len(t, cs.Credentials, 2) + cs2 := NewCredentialSet("dev", "sekrets-2", secrets.Strategy{ + Name: "password-2", Source: secrets.Source{ + Key: "secret-2", + Value: "dbPassword-2"}}) + require.NoError(t, cp.InsertCredentialSet(context.Background(), cs2)) + + creds, err = cp.ListCredentialSets(context.Background(), ListOptions{Namespace: "dev", Skip: 1}) + require.NoError(t, err) + require.Len(t, creds, 1, "expected 1 credential set") + require.Equal(t, cs2.Name, creds[0].Name, "expected to retrieve sekrets-2 credentials") + require.Equal(t, cs2.Namespace, creds[0].Namespace, "expected to retrieve sekrets-2 credentials") + + creds, err = cp.ListCredentialSets(context.Background(), ListOptions{Namespace: "dev", Limit: 1}) + require.NoError(t, err) + require.Len(t, creds, 1, "expected 1 credential set") + require.Equal(t, cs.Name, creds[0].Name, "expected to retrieve sekrets credentials") + require.Equal(t, cs.Namespace, creds[0].Namespace, "expected to retrieve sekrets credentials") + require.NoError(t, cp.RemoveCredentialSet(context.Background(), cs.Namespace, cs.Name)) + require.NoError(t, cp.RemoveCredentialSet(context.Background(), cs2.Namespace, cs2.Name)) _, err = cp.GetCredentialSet(context.Background(), cs.Namespace, cs.Name) require.ErrorIs(t, err, ErrNotFound{}) + _, err = cp.GetCredentialSet(context.Background(), cs2.Namespace, cs2.Name) + require.ErrorIs(t, err, ErrNotFound{}) } func TestCredentialStorage_Validate_GoodSources(t *testing.T) { diff --git a/pkg/storage/credentialset_provider.go b/pkg/storage/credentialset_provider.go index 8db1faa07..cfa83eb37 100644 --- a/pkg/storage/credentialset_provider.go +++ b/pkg/storage/credentialset_provider.go @@ -12,7 +12,7 @@ type CredentialSetProvider interface { ResolveAll(ctx context.Context, creds CredentialSet) (secrets.Set, error) Validate(ctx context.Context, creds CredentialSet) error InsertCredentialSet(ctx context.Context, creds CredentialSet) error - ListCredentialSets(ctx context.Context, namespace string, name string, labels map[string]string) ([]CredentialSet, error) + ListCredentialSets(ctx context.Context, listOptions ListOptions) ([]CredentialSet, error) GetCredentialSet(ctx context.Context, namespace string, name string) (CredentialSet, error) UpdateCredentialSet(ctx context.Context, creds CredentialSet) error RemoveCredentialSet(ctx context.Context, namespace string, name string) error diff --git a/pkg/storage/installation_provider.go b/pkg/storage/installation_provider.go index 6c8111c13..9d0b6b929 100644 --- a/pkg/storage/installation_provider.go +++ b/pkg/storage/installation_provider.go @@ -35,7 +35,7 @@ type InstallationProvider interface { GetInstallation(ctx context.Context, namespace string, name string) (Installation, error) // ListInstallations returns Installations sorted in ascending order by the namespace and then name. - ListInstallations(ctx context.Context, namespace string, name string, labels map[string]string) ([]Installation, error) + ListInstallations(ctx context.Context, listOption ListOptions) ([]Installation, error) // ListRuns returns Run documents sorted in ascending order by ID. ListRuns(ctx context.Context, namespace string, installation string) ([]Run, map[string][]Result, error) diff --git a/pkg/storage/installation_store.go b/pkg/storage/installation_store.go index 9dc4cf66e..bfda84df0 100644 --- a/pkg/storage/installation_store.go +++ b/pkg/storage/installation_store.go @@ -63,17 +63,12 @@ func (s InstallationStore) Initialize(ctx context.Context) error { return span.Error(err) } -func (s InstallationStore) ListInstallations(ctx context.Context, namespace string, name string, labels map[string]string) ([]Installation, error) { +func (s InstallationStore) ListInstallations(ctx context.Context, listOptions ListOptions) ([]Installation, error) { _, log := tracing.StartSpan(ctx) defer log.EndSpan() var out []Installation - findOpts := FindOptions{ - Sort: []string{"namespace", "name"}, - Filter: CreateListFiler(namespace, name, labels), - } - - err := s.store.Find(ctx, CollectionInstallations, findOpts, &out) + err := s.store.Find(ctx, CollectionInstallations, listOptions.ToFindOptions(), &out) return out, err } diff --git a/pkg/storage/installation_store_test.go b/pkg/storage/installation_store_test.go index 3a0c5f9d6..6c2ecaa34 100644 --- a/pkg/storage/installation_store_test.go +++ b/pkg/storage/installation_store_test.go @@ -165,7 +165,7 @@ func TestInstallationStorageProvider_Installations(t *testing.T) { defer cp.Close() t.Run("ListInstallations", func(t *testing.T) { - installations, err := cp.ListInstallations(context.Background(), "dev", "", nil) + installations, err := cp.ListInstallations(context.Background(), ListOptions{Namespace: "dev"}) require.NoError(t, err, "ListInstallations failed") require.Len(t, installations, 3, "Expected 3 installations") @@ -183,6 +183,36 @@ func TestInstallationStorageProvider_Installations(t *testing.T) { assert.Equal(t, cnab.StatusSucceeded, foo.Status.ResultStatus) }) + t.Run("ListInstallations with skip option", func(t *testing.T) { + installations, err := cp.ListInstallations(context.Background(), ListOptions{Namespace: "dev", Skip: 1}) + require.NoError(t, err, "ListInstallations failed") + + require.Len(t, installations, 2, "Expected 2 installations") + + bar := installations[0] + assert.Equal(t, "baz", bar.Name) + assert.Equal(t, cnab.StatusRunning, bar.Status.ResultStatus) + + baz := installations[1] + assert.Equal(t, "foo", baz.Name) + assert.Equal(t, cnab.StatusSucceeded, baz.Status.ResultStatus) + }) + + t.Run("ListInstallations with limit option", func(t *testing.T) { + installations, err := cp.ListInstallations(context.Background(), ListOptions{Namespace: "dev", Limit: 2}) + require.NoError(t, err, "ListInstallations failed") + + require.Len(t, installations, 2, "Expected 2 installations") + + bar := installations[0] + assert.Equal(t, "bar", bar.Name) + assert.Equal(t, cnab.StatusSucceeded, bar.Status.ResultStatus) + + baz := installations[1] + assert.Equal(t, "baz", baz.Name) + assert.Equal(t, cnab.StatusRunning, baz.Status.ResultStatus) + }) + t.Run("FindInstallations by label", func(t *testing.T) { opts := FindOptions{ Filter: map[string]interface{}{ @@ -236,14 +266,14 @@ func TestInstallationStorageProvider_DeleteInstallation(t *testing.T) { cp := generateInstallationData(t) defer cp.Close() - installations, err := cp.ListInstallations(context.Background(), "dev", "", nil) + installations, err := cp.ListInstallations(context.Background(), ListOptions{Namespace: "dev"}) require.NoError(t, err, "ListInstallations failed") assert.Len(t, installations, 3, "expected 3 installations") err = cp.RemoveInstallation(context.Background(), "dev", "foo") require.NoError(t, err, "RemoveInstallation failed") - installations, err = cp.ListInstallations(context.Background(), "dev", "", nil) + installations, err = cp.ListInstallations(context.Background(), ListOptions{Namespace: "dev"}) require.NoError(t, err, "ListInstallations failed") assert.Len(t, installations, 2, "expected foo to be deleted") diff --git a/pkg/storage/migrations/manager_test.go b/pkg/storage/migrations/manager_test.go index 52830e1cd..e559ad27b 100644 --- a/pkg/storage/migrations/manager_test.go +++ b/pkg/storage/migrations/manager_test.go @@ -61,15 +61,15 @@ func TestManager_NoMigrationEmptyHome(t *testing.T) { defer mgr.Close() claimStore := storage.NewInstallationStore(mgr) - _, err := claimStore.ListInstallations(context.Background(), "", "", nil) + _, err := claimStore.ListInstallations(context.Background(), storage.ListOptions{}) require.NoError(t, err, "ListInstallations failed") credStore := storage.NewCredentialStore(mgr, nil) - _, err = credStore.ListCredentialSets(context.Background(), "", "", nil) + _, err = credStore.ListCredentialSets(context.Background(), storage.ListOptions{}) require.NoError(t, err, "List credentials failed") paramStore := storage.NewParameterStore(mgr, nil) - _, err = paramStore.ListParameterSets(context.Background(), "", "", nil) + _, err = paramStore.ListParameterSets(context.Background(), storage.ListOptions{}) require.NoError(t, err, "List credentials failed") } @@ -101,7 +101,7 @@ storage.Schema{ID:"schema", Installations:"needs-migration", Credentials:"1.0.1" } t.Run("list", func(t *testing.T) { - _, err = claimStore.ListInstallations(context.Background(), "", "", nil) + _, err = claimStore.ListInstallations(context.Background(), storage.ListOptions{}) checkMigrationError(t, err) }) @@ -124,7 +124,7 @@ func TestClaimStorage_NoMigrationRequiredForEmptyHome(t *testing.T) { defer mgr.Close() claimStore := storage.NewInstallationStore(mgr) - names, err := claimStore.ListInstallations(context.Background(), "", "", nil) + names, err := claimStore.ListInstallations(context.Background(), storage.ListOptions{}) require.NoError(t, err, "ListInstallations failed") assert.Empty(t, names, "Expected an empty list of installations since porter home is new") } @@ -156,7 +156,7 @@ storage.Schema{ID:"schema", Installations:"1.0.1", Credentials:"needs-migration" } t.Run("list", func(t *testing.T) { - _, err = credStore.ListCredentialSets(context.Background(), "", "", nil) + _, err = credStore.ListCredentialSets(context.Background(), storage.ListOptions{}) checkMigrationError(t, err) }) @@ -177,7 +177,13 @@ func TestCredentialStorage_NoMigrationRequiredForEmptyHome(t *testing.T) { testSecrets := secrets.NewTestSecretsProvider() credStore := storage.NewTestCredentialProviderFor(t, mgr, testSecrets) - names, err := credStore.ListCredentialSets(context.Background(), "", "", nil) + names, err := credStore.ListCredentialSets(context.Background(), storage.ListOptions{ + Namespace: "", + Name: "", + Labels: nil, + Skip: 0, + Limit: 0, + }) require.NoError(t, err, "List failed") assert.Empty(t, names, "Expected an empty list of credentials since porter home is new") } @@ -209,7 +215,7 @@ storage.Schema{ID:"schema", Installations:"1.0.1", Credentials:"1.0.1", Paramete } t.Run("list", func(t *testing.T) { - _, err = paramStore.ListParameterSets(context.Background(), "", "", nil) + _, err = paramStore.ListParameterSets(context.Background(), storage.ListOptions{}) checkMigrationError(t, err) }) @@ -230,7 +236,7 @@ func TestParameterStorage_NoMigrationRequiredForEmptyHome(t *testing.T) { testSecrets := secrets.NewTestSecretsProvider() paramStore := storage.NewTestParameterProviderFor(t, mgr, testSecrets) - names, err := paramStore.ListParameterSets(context.Background(), "", "", nil) + names, err := paramStore.ListParameterSets(context.Background(), storage.ListOptions{}) require.NoError(t, err, "List failed") assert.Empty(t, names, "Expected an empty list of parameters since porter home is new") } diff --git a/pkg/storage/parameter_store.go b/pkg/storage/parameter_store.go index e1c641a2f..658cf9e9d 100644 --- a/pkg/storage/parameter_store.go +++ b/pkg/storage/parameter_store.go @@ -100,12 +100,9 @@ func (s ParameterStore) InsertParameterSet(ctx context.Context, params Parameter return s.Documents.Insert(ctx, CollectionParameters, opts) } -func (s ParameterStore) ListParameterSets(ctx context.Context, namespace string, name string, labels map[string]string) ([]ParameterSet, error) { +func (s ParameterStore) ListParameterSets(ctx context.Context, listOptions ListOptions) ([]ParameterSet, error) { var out []ParameterSet - opts := FindOptions{ - Filter: CreateListFiler(namespace, name, labels), - } - err := s.Documents.Find(ctx, CollectionParameters, opts, &out) + err := s.Documents.Find(ctx, CollectionParameters, listOptions.ToFindOptions(), &out) return out, err } diff --git a/pkg/storage/parameter_store_test.go b/pkg/storage/parameter_store_test.go index 4f75eb53c..055390a64 100644 --- a/pkg/storage/parameter_store_test.go +++ b/pkg/storage/parameter_store_test.go @@ -14,7 +14,7 @@ func TestParameterStore_CRUD(t *testing.T) { defer paramStore.Close() ctx := context.Background() - params, err := paramStore.ListParameterSets(ctx, "dev", "", nil) + params, err := paramStore.ListParameterSets(ctx, ListOptions{Namespace: "dev"}) require.NoError(t, err) require.Empty(t, params, "Find should return no entries") @@ -32,16 +32,16 @@ func TestParameterStore_CRUD(t *testing.T) { err = paramStore.InsertParameterSet(ctx, myParamSet) require.NoError(t, err, "Insert should successfully save") - params, err = paramStore.ListParameterSets(ctx, "dev", "", nil) + params, err = paramStore.ListParameterSets(ctx, ListOptions{Namespace: "dev"}) require.NoError(t, err) require.Len(t, params, 1, "expected 1 parameter set") - require.Equal(t, myParamSet.Name, params[0].Name, "expected to retrieve myparams") + require.Equal(t, myParamSet.Name, params[0].Name, "expected to retrieve myparam") - params, err = paramStore.ListParameterSets(ctx, "", "", nil) + params, err = paramStore.ListParameterSets(ctx, ListOptions{}) require.NoError(t, err) require.Len(t, params, 0, "expected no global parameter sets") - params, err = paramStore.ListParameterSets(ctx, "*", "", nil) + params, err = paramStore.ListParameterSets(ctx, ListOptions{Namespace: "*"}) require.NoError(t, err) require.Len(t, params, 1, "expected 1 parameter set defined in all namespaces") @@ -49,16 +49,45 @@ func TestParameterStore_CRUD(t *testing.T) { require.NoError(t, err) require.Equal(t, myParamSet, pset, "Get should return the saved parameter set") + myParamSet2 := NewParameterSet("dev", "myparams2", + secrets.Strategy{ + Name: "myparam2", + Source: secrets.Source{ + Key: "value2", + Value: "myparamvalue2", + }, + }) + myParamSet2.Status.Created = time.Date(2020, 1, 1, 12, 0, 0, 0, time.UTC) + myParamSet2.Status.Modified = myParamSet2.Status.Created + + err = paramStore.InsertParameterSet(ctx, myParamSet2) + require.NoError(t, err, "Insert should successfully save") + + params, err = paramStore.ListParameterSets(ctx, ListOptions{Namespace: "dev", Skip: 1}) + require.NoError(t, err) + require.Len(t, params, 1, "expected 1 parameter set") + require.Equal(t, myParamSet2.Name, params[0].Name, "expected to retrieve myparam2") + + params, err = paramStore.ListParameterSets(ctx, ListOptions{Namespace: "dev", Limit: 1}) + require.NoError(t, err) + require.Len(t, params, 1, "expected 1 parameter set") + require.Equal(t, myParamSet.Name, params[0].Name, "expected to retrieve myparam") + err = paramStore.RemoveParameterSet(ctx, myParamSet.Namespace, myParamSet.Name) require.NoError(t, err, "Remove should successfully delete the parameter set") - params, err = paramStore.ListParameterSets(ctx, "dev", "", nil) + err = paramStore.RemoveParameterSet(ctx, myParamSet2.Namespace, myParamSet2.Name) + require.NoError(t, err, "Remove should successfully delete the parameter set") + + params, err = paramStore.ListParameterSets(ctx, ListOptions{Namespace: "dev"}) require.NoError(t, err) require.Empty(t, params, "List should return no entries") pset, err = paramStore.GetParameterSet(ctx, "", myParamSet.Name) require.ErrorIs(t, err, ErrNotFound{}) + pset, err = paramStore.GetParameterSet(ctx, "", myParamSet2.Name) + require.ErrorIs(t, err, ErrNotFound{}) } func TestParameterStorage_ResolveAll(t *testing.T) { diff --git a/pkg/storage/parameterset_provider.go b/pkg/storage/parameterset_provider.go index 86944f8ae..64b35c3d3 100644 --- a/pkg/storage/parameterset_provider.go +++ b/pkg/storage/parameterset_provider.go @@ -17,7 +17,7 @@ type ParameterSetProvider interface { Validate(ctx context.Context, params ParameterSet) error InsertParameterSet(ctx context.Context, params ParameterSet) error - ListParameterSets(ctx context.Context, namespace string, name string, labels map[string]string) ([]ParameterSet, error) + ListParameterSets(ctx context.Context, listOptions ListOptions) ([]ParameterSet, error) GetParameterSet(ctx context.Context, namespace string, name string) (ParameterSet, error) UpdateParameterSet(ctx context.Context, params ParameterSet) error UpsertParameterSet(ctx context.Context, params ParameterSet) error diff --git a/pkg/storage/query.go b/pkg/storage/query.go index 0e686d2ac..f19dab6cb 100644 --- a/pkg/storage/query.go +++ b/pkg/storage/query.go @@ -288,20 +288,45 @@ func convertToRawJsonDocument(in interface{}, raw interface{}) error { return json.Unmarshal(data, raw) } -// CreateListFiler builds a query for a list of documents by: -// * matching namespace -// * name contains substring -// * labels contains all matches -func CreateListFiler(namespace string, name string, labels map[string]string) map[string]interface{} { +// ListOptions is the set of options available to the list operation +// on any storage provider. +type ListOptions struct { + // Namespace in which the particular result list is defined. + Namespace string + + // Name specifies whether the result list name contain the specified substring. + Name string + + // Labels is used to filter result list based on a key-value pair. + Labels map[string]string + + // Skip is the number of results to skip past and exclude from the results. + Skip int64 + + // Limit is the number of results to return. + Limit int64 +} + +// ToFindOptions builds a query for a list of documents with these conditions: +// * sorted in ascending order by namespace first and then name +// * filtered by matching namespace, name contains substring, and labels contain all matches +// * skipped and limited to a certain number of result +func (o ListOptions) ToFindOptions() FindOptions { filter := make(map[string]interface{}, 3) - if namespace != "*" { - filter["namespace"] = namespace + if o.Namespace != "*" { + filter["namespace"] = o.Namespace } - if name != "" { - filter["name"] = map[string]interface{}{"$regex": name} + if o.Name != "" { + filter["name"] = map[string]interface{}{"$regex": o.Name} } - for k, v := range labels { + for k, v := range o.Labels { filter["labels."+k] = v } - return filter + + return FindOptions{ + Sort: []string{"namespace", "name"}, + Filter: filter, + Skip: o.Skip, + Limit: o.Limit, + } } diff --git a/pkg/storage/query_test.go b/pkg/storage/query_test.go index 8c93646bf..133b5e3f0 100644 --- a/pkg/storage/query_test.go +++ b/pkg/storage/query_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" "go.mongodb.org/mongo-driver/bson" + "go.mongodb.org/mongo-driver/bson/primitive" ) var _ Document = TestDocument{} @@ -72,3 +73,27 @@ func TestFindOptions_ToPluginOptions(t *testing.T) { {"name", 1}} require.Equal(t, wantSortDoc, po.Sort) } + +func TestListOptions_ToFindOptions(t *testing.T) { + opts := ListOptions{ + Namespace: "dev", + Name: "name", + Labels: map[string]string{"key": "value"}, + Skip: 1, + Limit: 1, + } + + wantOpts := FindOptions{ + Sort: []string{"namespace", "name"}, + Skip: 1, + Limit: 1, + Filter: primitive.M{ + "labels.key": "value", + "name": map[string]interface{}{"$regex": "name"}, + "namespace": "dev", + }, + } + + gotOpts := opts.ToFindOptions() + require.Equal(t, wantOpts, gotOpts) +}