Skip to content

feat(acl list): add flags to filter by resource #1299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rkpattnaik780 merged 9 commits into from
Nov 12, 2021
Merged

feat(acl list): add flags to filter by resource #1299

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f075e62
feat(acl list): add flags to filter by resource
rkpattnaik780 Nov 8, 2021
d3561db
docs: update acl list docs
rkpattnaik780 Nov 8, 2021
4abc138
docs: add flag examples
rkpattnaik780 Nov 8, 2021
3afabd0
fix: address review comments
rkpattnaik780 Nov 9, 2021
b3fcf0f
fix: address review comments
rkpattnaik780 Nov 9, 2021
f2c902f
refactor: add autocompletions, change struct defs
rkpattnaik780 Nov 10, 2021
e292865
fix: change help texts
rkpattnaik780 Nov 11, 2021
bdd57bf
fix: remove localizer from flagset constructor
rkpattnaik780 Nov 11, 2021
e3313cf
fix: remove unused i18n key
rkpattnaik780 Nov 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/commands/rhoas_kafka_acl_list.adoc
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

50 changes: 49 additions & 1 deletion pkg/cmd/kafka/acl/list/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,12 @@ type options struct {
size int32
kafkaID string
principal string
output string

topic string
group string
cluster bool

output string
}

// NewListACLCommand creates a new command to list Kafka ACL rules
Expand Down Expand Up @@ -116,6 +121,9 @@ func NewListACLCommand(f *factory.Factory) *cobra.Command {
flags.AddUser(&userID)
flags.AddServiceAccount(&serviceAccount)
flags.AddAllAccounts(&allAccounts)
flags.AddCluster(&opts.cluster)
flags.AddTopic(&opts.topic)
flags.AddConsumerGroup(&opts.group)
Copy link
Contributor

@craicoverflow craicoverflow Nov 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to add dynamic completions for these.

Copy link
Contributor Author

@rkpattnaik780 rkpattnaik780 Nov 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it be good to add an additional field factory in the flagSet struct? That will make it easier to reuse completions for topic and group from cmdutils?

Copy link
Contributor

@craicoverflow craicoverflow Nov 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that would be a good move. You can remove the localizer and conn fields so and just use this :)

I'd recommend refactoring these completions APIs also to look like this one:

app-services-cli/pkg/kafka/cmdutil/util.go

Line 11 in 8edfc44

func RegisterNameFlagCompletionFunc(cmd *cobra.Command, f *factory.Factory) error {

They also probably should not be in a generic cmdutil package since they are kafka specific (I did that 😂)


return cmd
}
Expand All @@ -141,6 +149,40 @@ func runList(opts *options) (err error) {
req = req.Principal(principalQuery)
}

var selectedResourceTypeCount int
var resourceType string
var resourceName string

if opts.topic != "" {
selectedResourceTypeCount++
resourceType = aclutil.ResourceTypeTOPIC
resourceName = opts.topic
}

if opts.group != "" {
selectedResourceTypeCount++
resourceType = aclutil.ResourceTypeGROUP
resourceName = opts.group
}

if opts.cluster {
selectedResourceTypeCount++
resourceType = aclutil.ResourceTypeCLUSTER
resourceName = aclutil.KafkaCluster
}

if selectedResourceTypeCount > 1 {
return opts.localizer.MustLocalizeError("kafka.acl.common.error.oneResourceTypeAllowed", flagutil.ResourceTypeFlagEntries...)
}

if resourceType != "" {
req = req.ResourceType(aclutil.GetMappedResourceTypeFilterValue(resourceType))
}

if resourceName != "" {
req = req.ResourceName(aclutil.GetResourceName(resourceName))
}

permissionsData, httpRes, err := req.Execute()
if httpRes != nil {
defer httpRes.Body.Close()
Expand All @@ -150,6 +192,12 @@ func runList(opts *options) (err error) {
return err
}

if permissionsData.GetTotal() == 0 && opts.output == "" {
opts.logger.Info(opts.localizer.MustLocalize("kafka.acl.list.log.info.noACLs", localize.NewEntry("InstanceName", kafkaInstance.GetName())))

return nil
}

switch opts.output {
case dump.EmptyFormat:
opts.logger.Info("")
Expand Down
6 changes: 6 additions & 0 deletions pkg/localize/locales/en/cmd/acl.en.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ $ rhoas kafka acl list --user foo_user

# Display Kafka ACL rules for a specific service account
$ rhoas kafka acl list --service-account srvc-acct-f20a7561-7426-4f5a-b5e7-0ef2db31e15b

# Display Kafka ACL rules for a specific resource
$ rhoas kafka acl list --topic foo_topic_name
'''

[kafka.acl.list.cmd.shortDescription]
Expand Down Expand Up @@ -163,6 +166,9 @@ one = 'is'
[kafka.acl.list.startsWith]
one = 'starts with'

[kafka.acl.list.log.info.noACLs]
one = 'No ACLs found in Kafka instance "{{.InstanceName}}'

[kafka.acl.grantPermissions]

[kafka.acl.grantPermissions.cmd.shortDescription]
Expand Down