fix: allow to delete all ACLs for principal (#1441)

Author: wtrocki

docs/commands/rhoas_kafka_acl_delete.md

@@ -37,3 +37,5 @@ const (
 	PatternTypePREFIX  = "prefix"
 	PatternTypeANY     = "any"
 )
+
+var PatternTypes = []string{PatternTypeANY, PatternTypeLITERAL, PatternTypePREFIX}

pkg/cmd/kafka/acl/aclcmdutil/constants.go

@@ -93,8 +93,8 @@ func IsValidResourceOperation(resourceType string, operation string, resourceOpe
 	return false, resourceOperations
 }
 
-// ValidateAndSetResources validates and sets resources options
-func ValidateAndSetResources(opts *CrudOptions, resourceTypeFlagEntries []*localize.TemplateEntry) error {
+// SetACLResources sets resources options and returns number of changed resources
+func SetACLResources(opts *CrudOptions) int {
 	var selectedResourceTypeCount int
 
 	if opts.Topic != "" {
@@ -118,11 +118,7 @@ func ValidateAndSetResources(opts *CrudOptions, resourceTypeFlagEntries []*local
 		opts.ResourceName = KafkaCluster
 	}
 
-	if selectedResourceTypeCount != 1 {
-		return opts.Localizer.MustLocalizeError("kafka.acl.common.error.oneResourceTypeAllowed", resourceTypeFlagEntries...)
-	}
-
-	return nil
+	return selectedResourceTypeCount
 }
 
 // ValidateAPIError checks for a HTTP error and maps it to a user friendly error

pkg/cmd/kafka/acl/aclcmdutil/util.go

@@ -3,7 +3,7 @@ package create
 import (
 	"github.com/AlecAivazis/survey/v2"
 	"github.com/redhat-developer/app-services-cli/pkg/cmd/kafka/acl/aclcmdutil"
-	aclFlagutil "github.com/redhat-developer/app-services-cli/pkg/cmd/kafka/acl/flagutil"
+	aclFlagUtil "github.com/redhat-developer/app-services-cli/pkg/cmd/kafka/acl/flagutil"
 	"github.com/redhat-developer/app-services-cli/pkg/core/cmdutil"
 	"github.com/redhat-developer/app-services-cli/pkg/core/cmdutil/flagutil"
 	"github.com/redhat-developer/app-services-cli/pkg/core/ioutil/dump"
@@ -63,8 +63,10 @@ func NewCreateCommand(f *factory.Factory) *cobra.Command {
 				errorCollection = append(errorCollection, opts.Localizer.MustLocalizeError("kafka.acl.common.flag.operation.required"))
 			}
 
-			if resourceErrors := aclcmdutil.ValidateAndSetResources(opts, aclFlagutil.ResourceTypeFlagEntries); resourceErrors != nil {
-				errorCollection = append(errorCollection, resourceErrors)
+			selectedResourceTypeCount := aclcmdutil.SetACLResources(opts)
+
+			if selectedResourceTypeCount != 1 {
+				errorCollection = append(errorCollection, opts.Localizer.MustLocalizeError("kafka.acl.common.error.oneResourceTypeAllowed", aclFlagUtil.ResourceTypeFlagEntries...))
 			}
 
 			if principalErrors := validateAndSetOpts(opts); principalErrors != nil {
@@ -79,7 +81,7 @@ func NewCreateCommand(f *factory.Factory) *cobra.Command {
 		},
 	}
 
-	flags := aclFlagutil.NewFlagSet(cmd, f)
+	flags := aclFlagUtil.NewFlagSet(cmd, f)
 
 	flags.AddPermissionCreate(&opts.Permission)
 	flags.AddOperationCreate(&opts.Operation)

pkg/cmd/kafka/acl/create/create.go

@@ -17,10 +17,11 @@ import (
 )
 
 var (
-	serviceAccount string
-	userID         string
-	allAccounts    bool
-	prefix         bool
+	serviceAccount  string
+	userID          string
+	allAccounts     bool
+	prefix          bool
+	patternTypeFlag string
 )
 
 type requestParams struct {
@@ -56,12 +57,10 @@ func NewDeleteCommand(f *factory.Factory) *cobra.Command {
 
 			var errorCollection []error
 
-			if opts.Operation == "" {
-				errorCollection = append(errorCollection, opts.Localizer.MustLocalizeError("kafka.acl.common.flag.operation.required"))
-			}
-
-			if resourceErrors := aclcmdutil.ValidateAndSetResources(opts, aclFlagUtil.ResourceTypeFlagEntries); resourceErrors != nil {
-				errorCollection = append(errorCollection, resourceErrors)
+			selectedResourceTypeCount := aclcmdutil.SetACLResources(opts)
+			if selectedResourceTypeCount > 1 {
+				errorCollection = append(errorCollection,
+					opts.Localizer.MustLocalizeError("kafka.acl.common.error.oneResourceTypeAllowed", aclFlagUtil.ResourceTypeFlagEntries...))
 			}
 
 			if principalErrors := validateAndSetOpts(opts); principalErrors != nil {
@@ -82,7 +81,7 @@ func NewDeleteCommand(f *factory.Factory) *cobra.Command {
 	flags.AddOperationFilter(&opts.Operation)
 
 	flags.AddCluster(&opts.Cluster)
-	flags.AddPrefix(&prefix)
+
 	flags.AddTopic(&opts.Topic)
 	flags.AddConsumerGroup(&opts.Group)
 	flags.AddTransactionalID(&opts.TransactionalID)
@@ -92,6 +91,19 @@ func NewDeleteCommand(f *factory.Factory) *cobra.Command {
 	flags.AddServiceAccount(&serviceAccount)
 	flags.AddAllAccounts(&allAccounts)
 	flags.AddYes(&opts.SkipConfirm)
+	flags.AddPrefix(&prefix)
+
+	cmd.Flags().StringVar(
+		&patternTypeFlag,
+		"pattern-type",
+		aclcmdutil.PatternTypeLITERAL,
+		opts.Localizer.MustLocalize("kafka.acl.common.flag.patterntypes.description",
+			localize.NewEntry("Types", aclcmdutil.PatternTypes)),
+	)
+
+	_ = cmd.RegisterFlagCompletionFunc("pattern-type", func(cmd *cobra.Command, _ []string, toComplete string) ([]string, cobra.ShellCompDirective) {
+		return aclcmdutil.PatternTypes, cobra.ShellCompDirectiveNoSpace
+	})
 
 	return cmd
 }
@@ -118,12 +130,15 @@ func runDelete(instanceID string, opts *aclcmdutil.CrudOptions) error {
 		return err
 	}
 
-	if isValidOp, validResourceOperations := aclcmdutil.IsValidResourceOperation(opts.ResourceType, opts.Operation, resourceOperations); !isValidOp {
-		return opts.Localizer.MustLocalizeError("kafka.acl.common.error.invalidResourceOperation",
-			localize.NewEntry("ResourceType", opts.ResourceType),
-			localize.NewEntry("Operation", opts.Operation),
-			localize.NewEntry("ValidOperationList", cmdutil.StringSliceToListStringWithQuotes(validResourceOperations)),
-		)
+	// Validate only when both are present
+	if opts.ResourceType != "" && opts.Operation != "" {
+		if isValidOp, validResourceOperations := aclcmdutil.IsValidResourceOperation(opts.ResourceType, opts.Operation, resourceOperations); !isValidOp {
+			return opts.Localizer.MustLocalizeError("kafka.acl.common.error.invalidResourceOperation",
+				localize.NewEntry("ResourceType", opts.ResourceType),
+				localize.NewEntry("Operation", opts.Operation),
+				localize.NewEntry("ValidOperationList", cmdutil.StringSliceToListStringWithQuotes(validResourceOperations)),
+			)
+		}
 	}
 
 	kafkaNameTmplEntry := localize.NewEntry("Name", kafkaInstance.GetName())
@@ -149,14 +164,29 @@ func runDelete(instanceID string, opts *aclcmdutil.CrudOptions) error {
 
 	requestParams := getRequestParams(opts)
 
-	deletedACLs, httpRes, err := adminAPI.AclsApi.DeleteAcls(ctx).
-		ResourceType(requestParams.resourceType).
-		Principal(requestParams.principal).
-		PatternType(requestParams.patternType).
-		ResourceName(requestParams.resourceName).
-		Operation(requestParams.operation).
-		Permission(requestParams.permission).
-		Execute()
+	requestDeleteAcls := adminAPI.AclsApi.DeleteAcls(ctx)
+	if requestParams.resourceType != "" {
+		requestDeleteAcls = requestDeleteAcls.ResourceType(requestParams.resourceType)
+	}
+
+	if requestParams.principal != "" {
+		requestDeleteAcls = requestDeleteAcls.Principal(requestParams.principal)
+	}
+
+	if requestParams.resourceName != "" {
+		requestDeleteAcls = requestDeleteAcls.ResourceName(requestParams.resourceName)
+	}
+	if requestParams.patternType != "" {
+		requestDeleteAcls = requestDeleteAcls.PatternType(requestParams.patternType)
+	}
+	if requestParams.operation != "" {
+		requestDeleteAcls = requestDeleteAcls.Operation(requestParams.operation)
+	}
+	if requestParams.permission != "" {
+		requestDeleteAcls = requestDeleteAcls.Permission(requestParams.permission)
+	}
+
+	deletedACLs, httpRes, err := requestDeleteAcls.Execute()
 
 	if httpRes != nil {
 		defer httpRes.Body.Close()
@@ -226,7 +256,17 @@ func validateAndSetOpts(opts *aclcmdutil.CrudOptions) error {
 		return opts.Localizer.MustLocalizeError("kafka.acl.common.error.noPrincipalsSelected")
 	}
 
-	opts.PatternType = aclcmdutil.PatternTypeLITERAL
+	// Backwards compatibility:
+
+	switch patternTypeFlag {
+	case aclcmdutil.PatternTypeANY:
+		opts.PatternType = aclcmdutil.PatternTypeANY
+	case aclcmdutil.PatternTypePREFIX:
+		opts.PatternType = aclcmdutil.PatternTypePREFIX
+	case aclcmdutil.PatternTypeLITERAL:
+		opts.PatternType = aclcmdutil.PatternTypeLITERAL
+	}
+
 	if prefix {
 		opts.PatternType = aclcmdutil.PatternTypePREFIX
 	}

pkg/cmd/kafka/acl/delete/delete.go

@@ -0,0 +1,6 @@
+package flagutil
+
+// DeprecateFlag provides a way to deprecate a flag by appending standard prefixes to the flag description.
+func DeprecateFlag(flagDescription string) string {
+	return "DEPRECATED: " + flagDescription
+}

pkg/core/cmdutil/flagutil/deprecation.go

@@ -85,6 +85,9 @@ one = 'Set the resource type to cluster'
 [kafka.acl.common.flag.prefix.description]
 one = 'Determine if the resource should be exact match or prefix'
 
+[kafka.acl.common.flag.patterntypes.description]
+one = 'Allows to specify arguments matching strategy {{.Types}}'
+
 [kafka.acl.common.flag.topic.description]
 one = 'Set the topic resource. When the --prefix option is also passed, this is used as the topic prefix'
 
@@ -291,6 +294,9 @@ $ rhoas kafka acl delete --operation write --permission allow --topic all --user
 # Delete an ACL for a service account
 $ rhoas kafka acl delete --operation all --permission any --topic "rhoas" --prefix --service-account "srvc-acct-11924479-43fe-42b4-9676-cf0c9aca81"
 
+# Delete all ACLs for a service account
+$ rhoas kafka acl delete --service-account "srvc-acct-11924479-43fe-42b4-9676-cf0c9aca81 --pattern-type=all"
+
 # Delete an ACL for all users on the consumer group resource
 $ rhoas kafka acl delete --operation all --permission any --group "group-1" --all-accounts
 '''