fix(cli): clarify rule type deletion errors for bundles vs profiles

cmd/cli/app/ruletype/ruletype_delete.go

@@ -93,10 +93,10 @@ func deleteCommand(ctx context.Context, cmd *cobra.Command, _ []string, conn *gr
 	}
 
 	// Delete the rule types set for deletion
-	deletedRuleTypes, remainingRuleTypes := deleteRuleTypes(ctx, client, rulesToDelete, project)
+	deletedRuleTypes, bundleBlocked, profileBlocked := deleteRuleTypes(ctx, client, rulesToDelete, project)
 
 	// Print the results
-	printDeleteResults(cmd, deletedRuleTypes, remainingRuleTypes)
+	printDeleteResults(cmd, deletedRuleTypes, bundleBlocked, profileBlocked)
 
 	return nil
 }
@@ -106,57 +106,75 @@ func deleteRuleTypes(
 	client minderv1.RuleTypeServiceClient,
 	rulesToDelete []*minderv1.RuleType,
 	project string,
-) ([]string, []ruleTypeBlock) {
+) ([]string, []string, []ruleTypeBlock) {
 
 	var deletedRuleTypes []string
-	var remainingRuleTypes []ruleTypeBlock
+	var bundleBlocked []string
+	var profileBlocked []ruleTypeBlock
 
 	for _, ruleType := range rulesToDelete {
 		_, err := client.DeleteRuleType(ctx, &minderv1.DeleteRuleTypeRequest{
 			Context: &minderv1.Context{Project: &project},
 			Id:      ruleType.GetId(),
 		})
 		if err != nil {
+			if isBundleError(err) {
+				bundleBlocked = append(bundleBlocked, ruleType.GetName())
+				continue
+			}
+
 			profiles := extractProfiles(err.Error())
 
-			remainingRuleTypes = append(remainingRuleTypes, ruleTypeBlock{
+			profileBlocked = append(profileBlocked, ruleTypeBlock{
 				Name:     ruleType.GetName(),
 				Profiles: profiles,
 			})
 			continue
 		}
 		deletedRuleTypes = append(deletedRuleTypes, ruleType.GetName())
 	}
-	return deletedRuleTypes, remainingRuleTypes
+	return deletedRuleTypes, bundleBlocked, profileBlocked
 }
 
 func printDeleteResults(
 	cmd *cobra.Command,
 	deletedRuleTypes []string,
-	remainingRuleTypes []ruleTypeBlock,
+	bundleBlocked []string,
+	profileBlocked []ruleTypeBlock,
 ) {
-	if len(deletedRuleTypes) == 0 && len(remainingRuleTypes) == 0 {
+	if len(deletedRuleTypes) == 0 && len(bundleBlocked) == 0 && len(profileBlocked) == 0 {
 		cmd.Println("There are no rule types to delete")
 		return
 	}
 
 	if len(deletedRuleTypes) > 0 {
-		cmd.Println("\nThe following rule type(s) were successfully deleted:")
+		cmd.Println()
+		cmd.Println("The following rule type(s) were successfully deleted:")
 		for _, ruleType := range deletedRuleTypes {
 			cmd.Println(ruleType)
 		}
 	}
 
-	if len(remainingRuleTypes) > 0 {
-		cmd.Println("\nThe following rule type(s) are referenced by existing profiles and were not deleted:")
-		for _, ruleType := range remainingRuleTypes {
+	if len(bundleBlocked) > 0 {
+		cmd.Println()
+		cmd.Println("The following rule type(s) cannot be deleted because they are part of a bundle:")
+		for _, ruleType := range bundleBlocked {
+			cmd.Println(ruleType)
+		}
+	}
+
+	if len(profileBlocked) > 0 {
+		cmd.Println()
+		cmd.Println("The following rule type(s) are referenced by existing profiles and were not deleted:")
+		for _, ruleType := range profileBlocked {
 			cmd.Println(ruleType.Name)
 		}
 
-		cmd.Println("\nThey are referenced by profiles:")
+		cmd.Println()
+		cmd.Println("They are referenced by profiles:")
 
 		seen := map[string]bool{}
-		for _, b := range remainingRuleTypes {
+		for _, b := range profileBlocked {
 			for _, p := range b.Profiles {
 				if !seen[p] {
 					cmd.Println(p)
@@ -168,16 +186,21 @@ func printDeleteResults(
 	}
 }
 
+func isBundleError(err error) bool {
+	if err == nil {
+		return false
+	}
+	return strings.Contains(strings.ToLower(err.Error()), "cannot delete rule type from bundle")
+}
+
 func extractProfiles(errMsg string) []string {
 	var profilesRegex = regexp.MustCompile(`used by profiles (.+)`)
 	match := profilesRegex.FindStringSubmatch(strings.ToLower(errMsg))
 	if len(match) < 2 {
 		return []string{}
 	}
 
-	profilesPart := strings.TrimSpace(match[1])
-
-	parts := strings.Split(profilesPart, ",")
+	parts := strings.Split(match[1], ",")
 	for i := range parts {
 		parts[i] = strings.TrimSpace(parts[i])
 	}

cmd/cli/app/ruletype/ruletype_delete_test.go

@@ -0,0 +1,142 @@
+// SPDX-FileCopyrightText: Copyright 2023 The Minder Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package ruletype
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+
+	minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1"
+)
+
+func TestIsBundleError(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		err      error
+		expected bool
+	}{
+		{
+			name:     "nil error",
+			err:      nil,
+			expected: false,
+		},
+		{
+			name:     "standard error containing lower case bundle",
+			err:      errors.New("cannot delete rule type from bundle"),
+			expected: true,
+		},
+		{
+			name:     "grpc structured error containing upper case BUNDLE",
+			err:      status.Errorf(codes.InvalidArgument, "cannot delete rule type from BUNDLE"),
+			expected: true,
+		},
+		{
+			name:     "unrelated error",
+			err:      errors.New("some other error"),
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			result := isBundleError(tt.err)
+			require.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestExtractProfiles(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		errStr   string
+		expected []string
+	}{
+		{
+			name:     "single profile",
+			errStr:   "cannot delete: rule type is used by profiles foo",
+			expected: []string{"foo"},
+		},
+		{
+			name:     "multiple profiles",
+			errStr:   "cannot delete: rule type is used by profiles foo, bar, baz",
+			expected: []string{"foo", "bar", "baz"},
+		},
+		{
+			name:     "no profiles mentioned",
+			errStr:   "some other error",
+			expected: []string{},
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			result := extractProfiles(tt.errStr)
+			require.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+// mockRuleTypeServiceClient is a simple mock for testing deleteRuleTypes without gomock
+type mockRuleTypeServiceClient struct {
+	minderv1.RuleTypeServiceClient
+	deleteFunc func(ctx context.Context, in *minderv1.DeleteRuleTypeRequest, opts ...grpc.CallOption) (*minderv1.DeleteRuleTypeResponse, error)
+}
+
+func (m *mockRuleTypeServiceClient) DeleteRuleType(ctx context.Context, in *minderv1.DeleteRuleTypeRequest, opts ...grpc.CallOption) (*minderv1.DeleteRuleTypeResponse, error) {
+	if m.deleteFunc != nil {
+		return m.deleteFunc(ctx, in, opts...)
+	}
+	return &minderv1.DeleteRuleTypeResponse{}, nil
+}
+
+func TestDeleteRuleTypes_Categorization(t *testing.T) {
+	t.Parallel()
+
+	mockClient := &mockRuleTypeServiceClient{
+		deleteFunc: func(ctx context.Context, in *minderv1.DeleteRuleTypeRequest, opts ...grpc.CallOption) (*minderv1.DeleteRuleTypeResponse, error) {
+			switch in.GetId() {
+			case "rule1":
+				return &minderv1.DeleteRuleTypeResponse{}, nil
+			case "rule2":
+				return nil, status.Errorf(codes.InvalidArgument, "cannot delete rule type from bundle")
+			case "rule3":
+				return nil, status.Errorf(codes.FailedPrecondition, "cannot delete: rule type rule3 is used by profiles prof1, prof2")
+			default:
+				return nil, errors.New("unknown error")
+			}
+		},
+	}
+
+	rules := []*minderv1.RuleType{
+		{Id: "rule1", Name: "successful_rule"},
+		{Id: "rule2", Name: "bundle_rule"},
+		{Id: "rule3", Name: "profile_rule"},
+	}
+
+	deleted, bundleBlocked, profileBlocked := deleteRuleTypes(context.Background(), mockClient, rules, "project_id")
+
+	require.Len(t, deleted, 1)
+	require.Equal(t, "successful_rule", deleted[0])
+
+	require.Len(t, bundleBlocked, 1)
+	require.Equal(t, "bundle_rule", bundleBlocked[0])
+
+	require.Len(t, profileBlocked, 1)
+	require.Equal(t, "profile_rule", profileBlocked[0].Name)
+	require.Equal(t, []string{"prof1", "prof2"}, profileBlocked[0].Profiles)
+}