Guard delete of in-use credential/parameter set

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
 }

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
 }

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.
 ```

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.
 ```

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 {

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)

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 {

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