From 72f616f6ae24c9dc62e76b9faa9ae9fa7d372ca9 Mon Sep 17 00:00:00 2001 From: Wojciech Trocki Date: Tue, 15 Feb 2022 15:28:43 +0000 Subject: [PATCH 01/10] fix: allow to delete all ACLs for principal --- pkg/cmd/kafka/acl/delete/delete.go | 54 +++++++++++++++--------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/pkg/cmd/kafka/acl/delete/delete.go b/pkg/cmd/kafka/acl/delete/delete.go index 6d3629200..cf58d4bed 100644 --- a/pkg/cmd/kafka/acl/delete/delete.go +++ b/pkg/cmd/kafka/acl/delete/delete.go @@ -4,7 +4,6 @@ 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" - "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" "github.com/redhat-developer/app-services-cli/pkg/core/ioutil/icon" @@ -56,13 +55,13 @@ 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 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) - } + // if resourceErrors := aclcmdutil.ValidateAndSetResources(opts, aclFlagUtil.ResourceTypeFlagEntries); resourceErrors != nil { + // errorCollection = append(errorCollection, resourceErrors) + // } if principalErrors := validateAndSetOpts(opts); principalErrors != nil { errorCollection = append(errorCollection, principalErrors) @@ -110,21 +109,21 @@ func runDelete(instanceID string, opts *aclcmdutil.CrudOptions) error { return err } - resourceOperations, httpRes, err := adminAPI.AclsApi.GetAclResourceOperations(ctx).Execute() - if httpRes != nil { - defer httpRes.Body.Close() - } - if err != nil { - 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)), - ) - } + // resourceOperations, httpRes, err := adminAPI.AclsApi.GetAclResourceOperations(ctx).Execute() + // if httpRes != nil { + // defer httpRes.Body.Close() + // } + // if err != nil { + // 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)), + // ) + // } kafkaNameTmplEntry := localize.NewEntry("Name", kafkaInstance.GetName()) @@ -150,12 +149,13 @@ func runDelete(instanceID string, opts *aclcmdutil.CrudOptions) error { requestParams := getRequestParams(opts) deletedACLs, httpRes, err := adminAPI.AclsApi.DeleteAcls(ctx). - ResourceType(requestParams.resourceType). + //ResourceType(requestParams.resourceType). Principal(requestParams.principal). - PatternType(requestParams.patternType). - ResourceName(requestParams.resourceName). - Operation(requestParams.operation). - Permission(requestParams.permission). + + // PatternType(requestParams.patternType). + // ResourceName(requestParams.resourceName). + // Operation(requestParams.operation). + // Permission(requestParams.permission). Execute() if httpRes != nil { From 8de1f790814872c63f235cd6b445ffd35fe2cec2 Mon Sep 17 00:00:00 2001 From: Wojciech Trocki Date: Wed, 16 Feb 2022 12:47:24 +0000 Subject: [PATCH 02/10] feat: allow to delete more than one ACL at the time --- pkg/cmd/kafka/acl/aclcmdutil/util.go | 10 ++--- pkg/cmd/kafka/acl/create/create.go | 10 +++-- pkg/cmd/kafka/acl/delete/delete.go | 42 +++++++++++++------- pkg/core/localize/locales/en/cmd/acl.en.toml | 3 ++ 4 files changed, 39 insertions(+), 26 deletions(-) diff --git a/pkg/cmd/kafka/acl/aclcmdutil/util.go b/pkg/cmd/kafka/acl/aclcmdutil/util.go index deef9cf93..3114f71d5 100644 --- a/pkg/cmd/kafka/acl/aclcmdutil/util.go +++ b/pkg/cmd/kafka/acl/aclcmdutil/util.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 diff --git a/pkg/cmd/kafka/acl/create/create.go b/pkg/cmd/kafka/acl/create/create.go index e1f9c0cd9..73b7775b7 100644 --- a/pkg/cmd/kafka/acl/create/create.go +++ b/pkg/cmd/kafka/acl/create/create.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) diff --git a/pkg/cmd/kafka/acl/delete/delete.go b/pkg/cmd/kafka/acl/delete/delete.go index cf58d4bed..368fe7e69 100644 --- a/pkg/cmd/kafka/acl/delete/delete.go +++ b/pkg/cmd/kafka/acl/delete/delete.go @@ -55,13 +55,11 @@ 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 { errorCollection = append(errorCollection, principalErrors) @@ -148,15 +146,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). + 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) + } - // PatternType(requestParams.patternType). - // ResourceName(requestParams.resourceName). - // Operation(requestParams.operation). - // Permission(requestParams.permission). - Execute() + deletedACLs, httpRes, err := requestDeleteAcls.Execute() if httpRes != nil { defer httpRes.Body.Close() diff --git a/pkg/core/localize/locales/en/cmd/acl.en.toml b/pkg/core/localize/locales/en/cmd/acl.en.toml index cd3249954..6ee1f562d 100644 --- a/pkg/core/localize/locales/en/cmd/acl.en.toml +++ b/pkg/core/localize/locales/en/cmd/acl.en.toml @@ -291,6 +291,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" + # Delete an ACL for all users on the consumer group resource $ rhoas kafka acl delete --operation all --permission any --group "group-1" --all-accounts ''' From 0f9aceba115ec2222dd3f62258629b6fdad44828 Mon Sep 17 00:00:00 2001 From: Wojciech Trocki Date: Wed, 16 Feb 2022 15:47:43 +0000 Subject: [PATCH 03/10] fix: add new flags to support new flow for delete command --- pkg/cmd/kafka/acl/aclcmdutil/constants.go | 2 + pkg/cmd/kafka/acl/delete/delete.go | 70 ++++++++++++++------ pkg/core/cmdutil/flagutil/deprecation.go | 6 ++ pkg/core/localize/locales/en/cmd/acl.en.toml | 6 ++ 4 files changed, 63 insertions(+), 21 deletions(-) create mode 100644 pkg/core/cmdutil/flagutil/deprecation.go diff --git a/pkg/cmd/kafka/acl/aclcmdutil/constants.go b/pkg/cmd/kafka/acl/aclcmdutil/constants.go index f912e436b..f8d40df27 100644 --- a/pkg/cmd/kafka/acl/aclcmdutil/constants.go +++ b/pkg/cmd/kafka/acl/aclcmdutil/constants.go @@ -37,3 +37,5 @@ const ( PatternTypePREFIX = "prefix" PatternTypeANY = "any" ) + +var PatternTypes = []string{PatternTypeANY, PatternTypeLITERAL, PatternTypePREFIX} diff --git a/pkg/cmd/kafka/acl/delete/delete.go b/pkg/cmd/kafka/acl/delete/delete.go index 368fe7e69..38e4f0f18 100644 --- a/pkg/cmd/kafka/acl/delete/delete.go +++ b/pkg/cmd/kafka/acl/delete/delete.go @@ -4,6 +4,7 @@ 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" + "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" "github.com/redhat-developer/app-services-cli/pkg/core/ioutil/icon" @@ -16,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 { @@ -79,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) @@ -90,6 +92,25 @@ func NewDeleteCommand(f *factory.Factory) *cobra.Command { flags.AddAllAccounts(&allAccounts) flags.AddYes(&opts.SkipConfirm) + cmd.Flags().BoolVar( + &prefix, + "prefix", + false, + flagutil.DeprecateFlag(opts.Localizer.MustLocalize("kafka.acl.common.flag.delete.prefix.description")), + ) + + 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 } @@ -107,21 +128,24 @@ func runDelete(instanceID string, opts *aclcmdutil.CrudOptions) error { return err } - // resourceOperations, httpRes, err := adminAPI.AclsApi.GetAclResourceOperations(ctx).Execute() - // if httpRes != nil { - // defer httpRes.Body.Close() - // } - // if err != nil { - // 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)), - // ) - // } + resourceOperations, httpRes, err := adminAPI.AclsApi.GetAclResourceOperations(ctx).Execute() + if httpRes != nil { + defer httpRes.Body.Close() + } + if err != nil { + return err + } + + // 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()) @@ -238,9 +262,13 @@ func validateAndSetOpts(opts *aclcmdutil.CrudOptions) error { return opts.Localizer.MustLocalizeError("kafka.acl.common.error.noPrincipalsSelected") } - opts.PatternType = aclcmdutil.PatternTypeLITERAL + // Backwards compatibility: if prefix { opts.PatternType = aclcmdutil.PatternTypePREFIX + } else if patternTypeFlag == aclcmdutil.PatternTypeANY { + opts.PatternType = aclcmdutil.PatternTypeANY + } else { + opts.PatternType = aclcmdutil.PatternTypeLITERAL } if userID != "" { diff --git a/pkg/core/cmdutil/flagutil/deprecation.go b/pkg/core/cmdutil/flagutil/deprecation.go new file mode 100644 index 000000000..85300a7fc --- /dev/null +++ b/pkg/core/cmdutil/flagutil/deprecation.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 +} diff --git a/pkg/core/localize/locales/en/cmd/acl.en.toml b/pkg/core/localize/locales/en/cmd/acl.en.toml index 6ee1f562d..444ae215f 100644 --- a/pkg/core/localize/locales/en/cmd/acl.en.toml +++ b/pkg/core/localize/locales/en/cmd/acl.en.toml @@ -85,6 +85,12 @@ 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.delete.prefix.description] +one = 'Determine if the resource should be exact match or prefix. Use --pattern-type instead' + +[kafka.acl.common.flag.patterntypes.description] +one = 'Determine if the resource should be exact match, prefix or any {{.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' From d2baa6e7f4168063bab4728bf7ccf80a993a0837 Mon Sep 17 00:00:00 2001 From: Wojciech Trocki Date: Wed, 16 Feb 2022 15:48:36 +0000 Subject: [PATCH 04/10] fix: update documentation --- docs/commands/rhoas_kafka_acl_delete.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/commands/rhoas_kafka_acl_delete.md b/docs/commands/rhoas_kafka_acl_delete.md index 194508fd0..acb278563 100644 --- a/docs/commands/rhoas_kafka_acl_delete.md +++ b/docs/commands/rhoas_kafka_acl_delete.md @@ -19,6 +19,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" + # Delete an ACL for all users on the consumer group resource $ rhoas kafka acl delete --operation all --permission any --group "group-1" --all-accounts @@ -33,8 +36,9 @@ $ rhoas kafka acl delete --operation all --permission any --group "group-1" --al --instance-id string Kafka instance ID. Uses the current instance if not set --operation string Set the ACL operation. Choose from: "all", "alter", "alter-configs", "create", "delete", "describe", "describe-configs", "read", "write" -o, --output string Specify the output format. Choose from: "json", "yaml", "yml" + --pattern-type string Determine if the resource should be exact match, prefix or any [any literal prefix] (default "literal") --permission string Set the ACL permission. Choose from: "allow", "any", "deny" (default "any") - --prefix Determine if the resource should be exact match or prefix + --prefix DEPRECATED: Determine if the resource should be exact match or prefix. Use --pattern-type instead --service-account string Service account client ID used as principal for this operation --topic string Set the topic resource. When the --prefix option is also passed, this is used as the topic prefix --transactional-id string Set the transactional ID resource From 741bb5baa0651dad0c887c848ad3581469428010 Mon Sep 17 00:00:00 2001 From: Wojciech Trocki Date: Wed, 16 Feb 2022 16:14:24 +0000 Subject: [PATCH 05/10] fix: documentation for pattern type --- app-services-tools | 1 + docs/commands/rhoas_kafka_acl_delete.md | 2 +- pkg/cmd/kafka/acl/delete/delete.go | 11 ++++++++--- pkg/core/localize/locales/en/cmd/acl.en.toml | 2 +- 4 files changed, 11 insertions(+), 5 deletions(-) create mode 160000 app-services-tools diff --git a/app-services-tools b/app-services-tools new file mode 160000 index 000000000..3f5c1a37c --- /dev/null +++ b/app-services-tools @@ -0,0 +1 @@ +Subproject commit 3f5c1a37c045c77e62a123a36cdc8fa58891f5e5 diff --git a/docs/commands/rhoas_kafka_acl_delete.md b/docs/commands/rhoas_kafka_acl_delete.md index acb278563..345a58505 100644 --- a/docs/commands/rhoas_kafka_acl_delete.md +++ b/docs/commands/rhoas_kafka_acl_delete.md @@ -20,7 +20,7 @@ $ rhoas kafka acl delete --operation write --permission allow --topic all --user $ 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" +$ 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 diff --git a/pkg/cmd/kafka/acl/delete/delete.go b/pkg/cmd/kafka/acl/delete/delete.go index 38e4f0f18..7a54ced00 100644 --- a/pkg/cmd/kafka/acl/delete/delete.go +++ b/pkg/cmd/kafka/acl/delete/delete.go @@ -107,7 +107,7 @@ func NewDeleteCommand(f *factory.Factory) *cobra.Command { localize.NewEntry("Types", aclcmdutil.PatternTypes)), ) - cmd.RegisterFlagCompletionFunc("pattern-type", func(cmd *cobra.Command, _ []string, toComplete string) ([]string, cobra.ShellCompDirective) { + _ = cmd.RegisterFlagCompletionFunc("pattern-type", func(cmd *cobra.Command, _ []string, toComplete string) ([]string, cobra.ShellCompDirective) { return aclcmdutil.PatternTypes, cobra.ShellCompDirectiveNoSpace }) @@ -265,9 +265,14 @@ func validateAndSetOpts(opts *aclcmdutil.CrudOptions) error { // Backwards compatibility: if prefix { opts.PatternType = aclcmdutil.PatternTypePREFIX - } else if patternTypeFlag == aclcmdutil.PatternTypeANY { + } + + switch patternTypeFlag { + case aclcmdutil.PatternTypeANY: opts.PatternType = aclcmdutil.PatternTypeANY - } else { + case aclcmdutil.PatternTypePREFIX: + opts.PatternType = aclcmdutil.PatternTypePREFIX + case aclcmdutil.PatternTypeLITERAL: opts.PatternType = aclcmdutil.PatternTypeLITERAL } diff --git a/pkg/core/localize/locales/en/cmd/acl.en.toml b/pkg/core/localize/locales/en/cmd/acl.en.toml index 444ae215f..ce3c1c9f6 100644 --- a/pkg/core/localize/locales/en/cmd/acl.en.toml +++ b/pkg/core/localize/locales/en/cmd/acl.en.toml @@ -298,7 +298,7 @@ $ rhoas kafka acl delete --operation write --permission allow --topic all --user $ 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" +$ 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 From 9fa50ff4bd588c291f86b788ba14496976cb7256 Mon Sep 17 00:00:00 2001 From: Wojciech Trocki Date: Wed, 16 Feb 2022 16:15:52 +0000 Subject: [PATCH 06/10] fix: remove depreciation from prefix --- docs/commands/rhoas_kafka_acl_delete.md | 2 +- pkg/cmd/kafka/acl/delete/delete.go | 8 +------- pkg/core/localize/locales/en/cmd/acl.en.toml | 3 --- 3 files changed, 2 insertions(+), 11 deletions(-) diff --git a/docs/commands/rhoas_kafka_acl_delete.md b/docs/commands/rhoas_kafka_acl_delete.md index 345a58505..0940dad08 100644 --- a/docs/commands/rhoas_kafka_acl_delete.md +++ b/docs/commands/rhoas_kafka_acl_delete.md @@ -38,7 +38,7 @@ $ rhoas kafka acl delete --operation all --permission any --group "group-1" --al -o, --output string Specify the output format. Choose from: "json", "yaml", "yml" --pattern-type string Determine if the resource should be exact match, prefix or any [any literal prefix] (default "literal") --permission string Set the ACL permission. Choose from: "allow", "any", "deny" (default "any") - --prefix DEPRECATED: Determine if the resource should be exact match or prefix. Use --pattern-type instead + --prefix Determine if the resource should be exact match or prefix --service-account string Service account client ID used as principal for this operation --topic string Set the topic resource. When the --prefix option is also passed, this is used as the topic prefix --transactional-id string Set the transactional ID resource diff --git a/pkg/cmd/kafka/acl/delete/delete.go b/pkg/cmd/kafka/acl/delete/delete.go index 7a54ced00..4d3943015 100644 --- a/pkg/cmd/kafka/acl/delete/delete.go +++ b/pkg/cmd/kafka/acl/delete/delete.go @@ -91,13 +91,7 @@ func NewDeleteCommand(f *factory.Factory) *cobra.Command { flags.AddServiceAccount(&serviceAccount) flags.AddAllAccounts(&allAccounts) flags.AddYes(&opts.SkipConfirm) - - cmd.Flags().BoolVar( - &prefix, - "prefix", - false, - flagutil.DeprecateFlag(opts.Localizer.MustLocalize("kafka.acl.common.flag.delete.prefix.description")), - ) + flags.AddPrefix(&prefix) cmd.Flags().StringVar( &patternTypeFlag, diff --git a/pkg/core/localize/locales/en/cmd/acl.en.toml b/pkg/core/localize/locales/en/cmd/acl.en.toml index ce3c1c9f6..20261aacd 100644 --- a/pkg/core/localize/locales/en/cmd/acl.en.toml +++ b/pkg/core/localize/locales/en/cmd/acl.en.toml @@ -85,9 +85,6 @@ 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.delete.prefix.description] -one = 'Determine if the resource should be exact match or prefix. Use --pattern-type instead' - [kafka.acl.common.flag.patterntypes.description] one = 'Determine if the resource should be exact match, prefix or any {{.Types}}' From cddb20602d5d3c321ca5561f3a8876e3e27e05b2 Mon Sep 17 00:00:00 2001 From: Wojciech Trocki Date: Thu, 17 Feb 2022 11:56:16 +0000 Subject: [PATCH 07/10] fix: move prefix flag to be more important --- pkg/cmd/kafka/acl/delete/delete.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/kafka/acl/delete/delete.go b/pkg/cmd/kafka/acl/delete/delete.go index 4d3943015..d063637e6 100644 --- a/pkg/cmd/kafka/acl/delete/delete.go +++ b/pkg/cmd/kafka/acl/delete/delete.go @@ -257,9 +257,6 @@ func validateAndSetOpts(opts *aclcmdutil.CrudOptions) error { } // Backwards compatibility: - if prefix { - opts.PatternType = aclcmdutil.PatternTypePREFIX - } switch patternTypeFlag { case aclcmdutil.PatternTypeANY: @@ -270,6 +267,10 @@ func validateAndSetOpts(opts *aclcmdutil.CrudOptions) error { opts.PatternType = aclcmdutil.PatternTypeLITERAL } + if prefix { + opts.PatternType = aclcmdutil.PatternTypePREFIX + } + if userID != "" { opts.Principal = userID } else if serviceAccount != "" { From 9ced87042ba51ddc3e371fb303d323bd2a848b1a Mon Sep 17 00:00:00 2001 From: Wojciech Trocki Date: Thu, 17 Feb 2022 12:14:10 +0000 Subject: [PATCH 08/10] fix: remove app services tools --- app-services-tools | 1 - 1 file changed, 1 deletion(-) delete mode 160000 app-services-tools diff --git a/app-services-tools b/app-services-tools deleted file mode 160000 index 3f5c1a37c..000000000 --- a/app-services-tools +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 3f5c1a37c045c77e62a123a36cdc8fa58891f5e5 From 8295746d24de6f44bf86c3c52b3a3f2258dbcbec Mon Sep 17 00:00:00 2001 From: Wojciech Trocki Date: Thu, 17 Feb 2022 12:16:52 +0000 Subject: [PATCH 09/10] fix: clarify message for pattern-type --- docs/commands/rhoas_kafka_acl_delete.md | 2 +- pkg/core/localize/locales/en/cmd/acl.en.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/commands/rhoas_kafka_acl_delete.md b/docs/commands/rhoas_kafka_acl_delete.md index 0940dad08..8f8d6a358 100644 --- a/docs/commands/rhoas_kafka_acl_delete.md +++ b/docs/commands/rhoas_kafka_acl_delete.md @@ -36,7 +36,7 @@ $ rhoas kafka acl delete --operation all --permission any --group "group-1" --al --instance-id string Kafka instance ID. Uses the current instance if not set --operation string Set the ACL operation. Choose from: "all", "alter", "alter-configs", "create", "delete", "describe", "describe-configs", "read", "write" -o, --output string Specify the output format. Choose from: "json", "yaml", "yml" - --pattern-type string Determine if the resource should be exact match, prefix or any [any literal prefix] (default "literal") + --pattern-type string Allows to specify arguments matching strategy [any literal prefix] (default "literal") --permission string Set the ACL permission. Choose from: "allow", "any", "deny" (default "any") --prefix Determine if the resource should be exact match or prefix --service-account string Service account client ID used as principal for this operation diff --git a/pkg/core/localize/locales/en/cmd/acl.en.toml b/pkg/core/localize/locales/en/cmd/acl.en.toml index 20261aacd..f65921bca 100644 --- a/pkg/core/localize/locales/en/cmd/acl.en.toml +++ b/pkg/core/localize/locales/en/cmd/acl.en.toml @@ -86,7 +86,7 @@ one = 'Set the resource type to cluster' one = 'Determine if the resource should be exact match or prefix' [kafka.acl.common.flag.patterntypes.description] -one = 'Determine if the resource should be exact match, prefix or any {{.Types}}' +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' From d44f176dcd0a0c1601729edb42531d33c2dc3ee5 Mon Sep 17 00:00:00 2001 From: Wojciech Trocki Date: Thu, 17 Feb 2022 12:38:00 +0000 Subject: [PATCH 10/10] fix: documentation --- docs/commands/rhoas_kafka_acl_delete.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/commands/rhoas_kafka_acl_delete.md b/docs/commands/rhoas_kafka_acl_delete.md index 8f8d6a358..45e7b3ed1 100644 --- a/docs/commands/rhoas_kafka_acl_delete.md +++ b/docs/commands/rhoas_kafka_acl_delete.md @@ -36,7 +36,7 @@ $ rhoas kafka acl delete --operation all --permission any --group "group-1" --al --instance-id string Kafka instance ID. Uses the current instance if not set --operation string Set the ACL operation. Choose from: "all", "alter", "alter-configs", "create", "delete", "describe", "describe-configs", "read", "write" -o, --output string Specify the output format. Choose from: "json", "yaml", "yml" - --pattern-type string Allows to specify arguments matching strategy [any literal prefix] (default "literal") + --pattern-type string Allows to specify arguments matching strategy [any literal prefix] (default "literal") --permission string Set the ACL permission. Choose from: "allow", "any", "deny" (default "any") --prefix Determine if the resource should be exact match or prefix --service-account string Service account client ID used as principal for this operation