diff --git a/cmd/porter/credentials.go b/cmd/porter/credentials.go index d71303ebc..597bba3a7 100644 --- a/cmd/porter/credentials.go +++ b/cmd/porter/credentials.go @@ -188,6 +188,8 @@ func buildCredentialsDeleteCommand(p *porter.Porter) *cobra.Command { f := cmd.Flags() f.StringVarP(&opts.Namespace, "namespace", "n", "", "Namespace in which the credential set is defined. Defaults to the global namespace.") + f.BoolVar(&opts.Force, "force", false, + "Force the delete even if the credential set is in use by an installation") return cmd } diff --git a/cmd/porter/parameters.go b/cmd/porter/parameters.go index 9f8412d55..9f6ce66ec 100644 --- a/cmd/porter/parameters.go +++ b/cmd/porter/parameters.go @@ -187,6 +187,8 @@ func buildParametersDeleteCommand(p *porter.Porter) *cobra.Command { f := cmd.Flags() f.StringVarP(&opts.Namespace, "namespace", "n", "", "Namespace in which the parameter set is defined. Defaults to the global namespace.") + f.BoolVar(&opts.Force, "force", false, + "Force the delete even if the parameter set is in use by an installation") return cmd } diff --git a/docs/content/docs/references/cli/credentials_delete.md b/docs/content/docs/references/cli/credentials_delete.md index 0d09b5f0f..f40f0627e 100644 --- a/docs/content/docs/references/cli/credentials_delete.md +++ b/docs/content/docs/references/cli/credentials_delete.md @@ -24,6 +24,7 @@ porter credentials delete NAME [flags] ### Options ``` + --force Force the delete even if the credential set is in use by an installation -h, --help help for delete -n, --namespace string Namespace in which the credential set is defined. Defaults to the global namespace. ``` diff --git a/docs/content/docs/references/cli/parameters_delete.md b/docs/content/docs/references/cli/parameters_delete.md index 81b6b5dac..ff2caf4bb 100644 --- a/docs/content/docs/references/cli/parameters_delete.md +++ b/docs/content/docs/references/cli/parameters_delete.md @@ -18,6 +18,7 @@ porter parameters delete NAME [flags] ### Options ``` + --force Force the delete even if the parameter set is in use by an installation -h, --help help for delete -n, --namespace string Namespace in which the parameter set is defined. Defaults to the global namespace. ``` diff --git a/pkg/porter/credentials.go b/pkg/porter/credentials.go index 9909eabac..20eb1fc25 100644 --- a/pkg/porter/credentials.go +++ b/pkg/porter/credentials.go @@ -16,6 +16,7 @@ import ( "get.porter.sh/porter/pkg/storage" "get.porter.sh/porter/pkg/tracing" dtprinter "github.com/carolynvs/datetime-printer" + "go.mongodb.org/mongo-driver/bson" "go.opentelemetry.io/otel/attribute" ) @@ -303,6 +304,7 @@ func (p *Porter) ShowCredential(ctx context.Context, opts CredentialShowOptions) type CredentialDeleteOptions struct { Name string Namespace string + Force bool } // DeleteCredential deletes the credential set corresponding to the provided @@ -314,9 +316,31 @@ func (p *Porter) DeleteCredential(ctx context.Context, opts CredentialDeleteOpti ) defer span.EndSpan() + if _, err := p.Credentials.GetCredentialSet(ctx, opts.Namespace, opts.Name); err != nil { + if errors.Is(err, storage.ErrNotFound{}) { + span.Debug("Credential set has already been deleted") + return nil + } + return span.Error(fmt.Errorf("unable to get credential set: %w", err)) + } + + if !opts.Force { + installations, err := p.findInstallationsUsingCredentialSet(ctx, opts.Namespace, opts.Name) + if err != nil { + return span.Error(fmt.Errorf("unable to check if credential set %s is in use: %w", opts.Name, err)) + } + if len(installations) > 0 { + names := make([]string, 0, len(installations)) + for _, inst := range installations { + names = append(names, fmt.Sprintf("%s/%s", inst.Namespace, inst.Name)) + } + return span.Error(fmt.Errorf("credential set %s is in use by the following installation(s): %s; if you are sure it should be deleted, retry the last command with the --force flag", opts.Name, strings.Join(names, ", "))) + } + } + err := p.Credentials.RemoveCredentialSet(ctx, opts.Namespace, opts.Name) if errors.Is(err, storage.ErrNotFound{}) { - span.Debug("nothing to remove, credential already does not exist") + span.Debug("Credential set has already been deleted") return nil } if err != nil { @@ -326,6 +350,49 @@ func (p *Porter) DeleteCredential(ctx context.Context, opts CredentialDeleteOpti return nil } +// findInstallationsUsingCredentialSet returns the installations that resolve +// the named credential set, following the same namespace fallback rule used +// by resolveCredentialSets: a namespaced set is only visible to +// installations in that namespace, while a global set (namespace == "") is +// visible to installations in any namespace, unless that namespace defines +// its own set with the same name, which shadows the global one. +func (p *Porter) findInstallationsUsingCredentialSet(ctx context.Context, namespace string, name string) ([]storage.Installation, error) { + filter := bson.M{"credentialSets": name} + if namespace != "" { + filter["namespace"] = namespace + } + + candidates, err := p.Installations.FindInstallations(ctx, storage.FindOptions{Filter: filter}) + if err != nil || namespace != "" { + return candidates, err + } + + shadowed := map[string]bool{} + var inUse []storage.Installation + for _, inst := range candidates { + if inst.Namespace == "" { + inUse = append(inUse, inst) + continue + } + + shadows, ok := shadowed[inst.Namespace] + if !ok { + _, getErr := p.Credentials.GetCredentialSet(ctx, inst.Namespace, name) + if getErr != nil && !errors.Is(getErr, storage.ErrNotFound{}) { + return nil, getErr + } + shadows = getErr == nil + shadowed[inst.Namespace] = shadows + } + + if !shadows { + inUse = append(inUse, inst) + } + } + + return inUse, nil +} + // Validate validates the args provided Porter's credential delete command func (o *CredentialDeleteOptions) Validate(args []string) error { if err := validateCredentialName(args); err != nil { diff --git a/pkg/porter/credentials_test.go b/pkg/porter/credentials_test.go index e074fddfe..fc1964284 100644 --- a/pkg/porter/credentials_test.go +++ b/pkg/porter/credentials_test.go @@ -366,6 +366,80 @@ func TestCredentialsDelete(t *testing.T) { } } +func TestCredentialsDelete_InUse(t *testing.T) { + ctx := context.Background() + + t.Run("blocked without force", func(t *testing.T) { + p := NewTestPorter(t) + defer p.Close() + + cs := storage.NewCredentialSet("dev", "kool-kreds") + require.NoError(t, p.Credentials.InsertCredentialSet(ctx, cs)) + + i := storage.NewInstallation("dev", "wordpress") + i.CredentialSets = []string{"kool-kreds"} + p.TestInstallations.CreateInstallation(i) + + err := p.DeleteCredential(ctx, CredentialDeleteOptions{Namespace: "dev", Name: "kool-kreds"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "dev/wordpress") + assert.Contains(t, err.Error(), "--force") + + _, err = p.Credentials.GetCredentialSet(ctx, "dev", "kool-kreds") + require.NoError(t, err, "expected credential set to remain") + }) + + t.Run("force deletes despite use", func(t *testing.T) { + p := NewTestPorter(t) + defer p.Close() + + cs := storage.NewCredentialSet("dev", "kool-kreds") + require.NoError(t, p.Credentials.InsertCredentialSet(ctx, cs)) + + i := storage.NewInstallation("dev", "wordpress") + i.CredentialSets = []string{"kool-kreds"} + p.TestInstallations.CreateInstallation(i) + + err := p.DeleteCredential(ctx, CredentialDeleteOptions{Namespace: "dev", Name: "kool-kreds", Force: true}) + require.NoError(t, err) + + _, err = p.Credentials.GetCredentialSet(ctx, "dev", "kool-kreds") + assert.ErrorIs(t, err, storage.ErrNotFound{}) + }) + + t.Run("missing set is idempotent even if name is referenced", func(t *testing.T) { + p := NewTestPorter(t) + defer p.Close() + + i := storage.NewInstallation("dev", "wordpress") + i.CredentialSets = []string{"kool-kreds"} + p.TestInstallations.CreateInstallation(i) + + err := p.DeleteCredential(ctx, CredentialDeleteOptions{Namespace: "dev", Name: "kool-kreds"}) + require.NoError(t, err, "deleting an already-missing set should not require --force") + }) + + t.Run("global set shadowed by local set is not in use", func(t *testing.T) { + p := NewTestPorter(t) + defer p.Close() + + globalCS := storage.NewCredentialSet("", "shared-creds") + require.NoError(t, p.Credentials.InsertCredentialSet(ctx, globalCS)) + localCS := storage.NewCredentialSet("dev", "shared-creds") + require.NoError(t, p.Credentials.InsertCredentialSet(ctx, localCS)) + + i := storage.NewInstallation("dev", "wordpress") + i.CredentialSets = []string{"shared-creds"} + p.TestInstallations.CreateInstallation(i) + + err := p.DeleteCredential(ctx, CredentialDeleteOptions{Namespace: "", Name: "shared-creds"}) + require.NoError(t, err, "the global set is shadowed by the local one, so it should not be considered in use") + + _, err = p.Credentials.GetCredentialSet(ctx, "", "shared-creds") + assert.ErrorIs(t, err, storage.ErrNotFound{}) + }) +} + func TestApplyOptions_Validate(t *testing.T) { t.Run("no file specified", func(t *testing.T) { tc := portercontext.NewTestContext(t) diff --git a/pkg/porter/parameters.go b/pkg/porter/parameters.go index 9b7b9bdd5..267b9feb6 100644 --- a/pkg/porter/parameters.go +++ b/pkg/porter/parameters.go @@ -300,6 +300,7 @@ func (p *Porter) ShowParameter(ctx context.Context, opts ParameterShowOptions) e type ParameterDeleteOptions struct { Name string Namespace string + Force bool } // DeleteParameter deletes the parameter set corresponding to the provided @@ -308,9 +309,31 @@ func (p *Porter) DeleteParameter(ctx context.Context, opts ParameterDeleteOption ctx, span := tracing.StartSpan(ctx) defer span.EndSpan() + if _, err := p.Parameters.GetParameterSet(ctx, opts.Namespace, opts.Name); err != nil { + if errors.Is(err, storage.ErrNotFound{}) { + span.Debug("Parameter set has already been deleted") + return nil + } + return span.Error(fmt.Errorf("unable to get parameter set: %w", err)) + } + + if !opts.Force { + installations, err := p.findInstallationsUsingParameterSet(ctx, opts.Namespace, opts.Name) + if err != nil { + return span.Error(fmt.Errorf("unable to check if parameter set %s is in use: %w", opts.Name, err)) + } + if len(installations) > 0 { + names := make([]string, 0, len(installations)) + for _, inst := range installations { + names = append(names, fmt.Sprintf("%s/%s", inst.Namespace, inst.Name)) + } + return span.Error(fmt.Errorf("parameter set %s is in use by the following installation(s): %s; if you are sure it should be deleted, retry the last command with the --force flag", opts.Name, strings.Join(names, ", "))) + } + } + err := p.Parameters.RemoveParameterSet(ctx, opts.Namespace, opts.Name) if errors.Is(err, storage.ErrNotFound{}) { - span.Debug("Cannot remove parameter set because it already doesn't exist") + span.Debug("Parameter set has already been deleted") return nil } if err != nil { @@ -320,6 +343,49 @@ func (p *Porter) DeleteParameter(ctx context.Context, opts ParameterDeleteOption return nil } +// findInstallationsUsingParameterSet returns the installations that resolve +// the named parameter set, following the same namespace fallback rule used +// by loadParameterSets: a namespaced set is only visible to installations +// in that namespace, while a global set (namespace == "") is visible to +// installations in any namespace, unless that namespace defines its own set +// with the same name, which shadows the global one. +func (p *Porter) findInstallationsUsingParameterSet(ctx context.Context, namespace string, name string) ([]storage.Installation, error) { + filter := bson.M{"parameterSets": name} + if namespace != "" { + filter["namespace"] = namespace + } + + candidates, err := p.Installations.FindInstallations(ctx, storage.FindOptions{Filter: filter}) + if err != nil || namespace != "" { + return candidates, err + } + + shadowed := map[string]bool{} + var inUse []storage.Installation + for _, inst := range candidates { + if inst.Namespace == "" { + inUse = append(inUse, inst) + continue + } + + shadows, ok := shadowed[inst.Namespace] + if !ok { + _, getErr := p.Parameters.GetParameterSet(ctx, inst.Namespace, name) + if getErr != nil && !errors.Is(getErr, storage.ErrNotFound{}) { + return nil, getErr + } + shadows = getErr == nil + shadowed[inst.Namespace] = shadows + } + + if !shadows { + inUse = append(inUse, inst) + } + } + + return inUse, nil +} + // Validate the args provided to the delete parameter command func (o *ParameterDeleteOptions) Validate(args []string) error { if err := validateParameterName(args); err != nil { diff --git a/pkg/porter/parameters_test.go b/pkg/porter/parameters_test.go index b499aecae..62d4e5626 100644 --- a/pkg/porter/parameters_test.go +++ b/pkg/porter/parameters_test.go @@ -1126,6 +1126,94 @@ func TestParametersCreate(t *testing.T) { } } +func TestParametersDelete(t *testing.T) { + ctx := context.Background() + + t.Run("not in use", func(t *testing.T) { + p := NewTestPorter(t) + defer p.Close() + + ps := storage.NewParameterSet("dev", "kool-params") + require.NoError(t, p.Parameters.InsertParameterSet(ctx, ps)) + + err := p.DeleteParameter(ctx, ParameterDeleteOptions{Namespace: "dev", Name: "kool-params"}) + require.NoError(t, err) + + _, err = p.Parameters.GetParameterSet(ctx, "dev", "kool-params") + assert.ErrorIs(t, err, storage.ErrNotFound{}) + }) + + t.Run("blocked without force", func(t *testing.T) { + p := NewTestPorter(t) + defer p.Close() + + ps := storage.NewParameterSet("dev", "kool-params") + require.NoError(t, p.Parameters.InsertParameterSet(ctx, ps)) + + i := storage.NewInstallation("dev", "wordpress") + i.ParameterSets = []string{"kool-params"} + p.TestInstallations.CreateInstallation(i) + + err := p.DeleteParameter(ctx, ParameterDeleteOptions{Namespace: "dev", Name: "kool-params"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "dev/wordpress") + assert.Contains(t, err.Error(), "--force") + + _, err = p.Parameters.GetParameterSet(ctx, "dev", "kool-params") + require.NoError(t, err, "expected parameter set to remain") + }) + + t.Run("force deletes despite use", func(t *testing.T) { + p := NewTestPorter(t) + defer p.Close() + + ps := storage.NewParameterSet("dev", "kool-params") + require.NoError(t, p.Parameters.InsertParameterSet(ctx, ps)) + + i := storage.NewInstallation("dev", "wordpress") + i.ParameterSets = []string{"kool-params"} + p.TestInstallations.CreateInstallation(i) + + err := p.DeleteParameter(ctx, ParameterDeleteOptions{Namespace: "dev", Name: "kool-params", Force: true}) + require.NoError(t, err) + + _, err = p.Parameters.GetParameterSet(ctx, "dev", "kool-params") + assert.ErrorIs(t, err, storage.ErrNotFound{}) + }) + + t.Run("missing set is idempotent even if name is referenced", func(t *testing.T) { + p := NewTestPorter(t) + defer p.Close() + + i := storage.NewInstallation("dev", "wordpress") + i.ParameterSets = []string{"kool-params"} + p.TestInstallations.CreateInstallation(i) + + err := p.DeleteParameter(ctx, ParameterDeleteOptions{Namespace: "dev", Name: "kool-params"}) + require.NoError(t, err, "deleting an already-missing set should not require --force") + }) + + t.Run("global set shadowed by local set is not in use", func(t *testing.T) { + p := NewTestPorter(t) + defer p.Close() + + globalPS := storage.NewParameterSet("", "shared-params") + require.NoError(t, p.Parameters.InsertParameterSet(ctx, globalPS)) + localPS := storage.NewParameterSet("dev", "shared-params") + require.NoError(t, p.Parameters.InsertParameterSet(ctx, localPS)) + + i := storage.NewInstallation("dev", "wordpress") + i.ParameterSets = []string{"shared-params"} + p.TestInstallations.CreateInstallation(i) + + err := p.DeleteParameter(ctx, ParameterDeleteOptions{Namespace: "", Name: "shared-params"}) + require.NoError(t, err, "the global set is shadowed by the local one, so it should not be considered in use") + + _, err = p.Parameters.GetParameterSet(ctx, "", "shared-params") + assert.ErrorIs(t, err, storage.ErrNotFound{}) + }) +} + func TestPorter_ParametersApply(t *testing.T) { t.Run("invalid schemaType", func(t *testing.T) { // Make sure that we are validating the display parameter set values, and not just the underlying stored parameter set