Skip to content

Commit

Permalink
Adding pagination for installation, parameter, and credential list re…
Browse files Browse the repository at this point in the history
…sult using skip and limit option (#2137)

* Add pagination option for installation list command using skip and limit flag

Signed-off-by: joshuabezaleel <[email protected]>

* Increase plugin start/stop timeouts

As I was adding back in net/rpc plugins (the legacy v0 plugins), I
realized that our plugin timeouts don't work well for net/rpc since it
is much slower than gRPC.

I've bumped both the plugin start and stop timeout defaults to make it
less likely that a user will run into the timeout, while still giving us
a good "oops the plugin is broken" timeout detection.

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

* Add InstallationStore.FindInstallations (#2119)

The advanced dependencies proposal needs to be able to search for
installations based on more complex critieria than is available in the
ListInstallations function (which is intended to support the porter
installation list command). FindInstallations lets us craft any valid
mongodb find query and execute it, returning a list of installations.

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

* Rename DisplayRun.ClaimID to ID

I missed this field when I did a sweep earlier to remove the use of the
word claim in the release/v1 branch. In the rest of the CLI's output we
call the run's id just ID or RunID, and should be consistent with that.

I've changed DisplayID.ClaimID to ID so that we aren't exposing the term
claim to our users (and it's not really the claim id anymore anyway).

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

* Support Docker TLS environment variables

We are using the docker cli library to build images and I had thought
this gave us automatic support for building against a remote docker
host. It works fine for DOCKER_HOST, but turns out the TLS configuration
environment variables are only parsed when the docker CLI flags are
bound (which doesn't occur when we use it as a library).

I've updated how we initialize the docker cli library so that
DOCKER_TLS_VERIFY and DOCKER_CERT_PATH are picked up and passed to the
library.

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

* Add vet and lint targets to magefile

Signed-off-by: Tanmay Chaudhry <[email protected]>
Signed-off-by: joshuabezaleel <[email protected]>

* Add ListOption input parameter struct and enable skip and limit option to credential and parameter list command as well

Signed-off-by: joshuabezaleel <[email protected]>

* Leave out default value for ListOption's properties

Signed-off-by: joshuabezaleel <[email protected]>

* Remove commented function signature

Signed-off-by: joshuabezaleel <[email protected]>

* Convert CreateListFilter to ToFindOptions method for ListOptions type receiver

Signed-off-by: joshuabezaleel <[email protected]>

Co-authored-by: Carolyn Van Slyck <[email protected]>
Co-authored-by: Tanmay Chaudhry <[email protected]>
  • Loading branch information
3 people authored Jun 15, 2022
1 parent db2fed1 commit d7009a2
Show file tree
Hide file tree
Showing 25 changed files with 238 additions and 64 deletions.
7 changes: 6 additions & 1 deletion cmd/porter/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
},
Expand All @@ -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
}
Expand Down
7 changes: 6 additions & 1 deletion cmd/porter/installations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
},
Expand All @@ -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
}
Expand Down
7 changes: 6 additions & 1 deletion cmd/porter/parameters.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
},
Expand All @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions docs/content/cli/build.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions docs/content/cli/bundles_build.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions docs/content/cli/credentials_list.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions docs/content/cli/installations_list.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions docs/content/cli/list.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions docs/content/cli/parameters_list.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions pkg/cli/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand Down
8 changes: 7 additions & 1 deletion pkg/porter/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 9 additions & 1 deletion pkg/porter/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ type ListOptions struct {
Namespace string
Name string
Labels []string
Skip int64
Limit int64
}

func (o *ListOptions) Validate() error {
Expand Down Expand Up @@ -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))
}
Expand Down
8 changes: 7 additions & 1 deletion pkg/porter/parameters.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 2 additions & 5 deletions pkg/storage/credential_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
31 changes: 26 additions & 5 deletions pkg/storage/credential_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/credentialset_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/installation_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 2 additions & 7 deletions pkg/storage/installation_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Loading

0 comments on commit d7009a2

Please sign in to comment.