feat(kafka acl): add convenience commands #1179

rkpattnaik780 · 2021-10-05T12:21:47Z

Verification Steps

rhoas kafka acl grant-permissions --producer --user user_name --topic "*" should create ACL rules allowing user to write to all topics in the instance.
rhoas kafka acl grant-permissions --consumer --user user_name --topic-prefix "abc" should create ACL rules allowing user to consume from all topics starting with "abc".

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation change
Other (please specify)

Checklist

Documentation added for the feature
CI and all relevant tests are passing
Code Review completed
Verified independently by reviewer

pkg/cmd/kafka/acl/consumer/consumer.go

wtrocki · 2021-10-05T13:13:43Z

Separate PR:
[kafka.acl.list.cmd.longDescription] - let's copy what we have in the documentation to make this more documented.
We can forget about this later

pkg/cmd/kafka/acl/consumer/consumer.go

pkg/localize/locales/en/cmd/acl.en.toml

bhardesty · 2021-10-05T15:26:49Z

pkg/localize/locales/en/cmd/acl.en.toml

+one = 'service account ID to be used as principal'
+
+[kafka.acl.common.flag.topicPrefix.description]
+one = 'prefix topic name to define ACL rules for'


Suggested change

one = 'prefix topic name to define ACL rules for'

one = 'The prefix topic name to define ACL rules for'

pkg/localize/locales/en/cmd/acl.en.toml

pkg/cmd/kafka/acl/grant/grant.go

wtrocki · 2021-10-06T14:05:33Z

All ACL rules are aligned with the Kafka specification/patterns for consuming and producing content.
Great job! Little bit refinement and we can get that in.

For redability I would suggest to define ACL objects close to each other/in the group. Add them into array and then have single method that tries to execute based on that array in the loop. That will allow us to swap easily to batch API when available with single like of code.

pkg/localize/locales/en/cmd/acl.en.toml

pkg/cmd/kafka/acl/grant/grant.go

wtrocki

Reverified some mixture of flags and permissions

craicoverflow · 2021-10-11T14:05:44Z

@rkpattnaik780 could you go through the outstanding/unresolved feedback items and create issues for them to be followed up on?

pkg/cmd/kafka/acl/grant/grant.go

pkg/localize/locales/en/cmd/acl.en.toml

craicoverflow · 2021-10-11T14:21:03Z

Getting an i18n error:

$ rhoas kafka acl grant-permissions --service-account srvc1 --consumer --group "*" --topic-prefix "*" --topic "*"
panic: template: :1: unexpected "}" in operand

goroutine 1 [running]:
github.com/nicksnyder/go-i18n/v2/i18n.(*Localizer).MustLocalize(0xc0001ab800, 0xc0000bdbc8, 0x1deeb56, 0x8)
        /home/ephelan/.gvm/pkgsets/go1.16.4/global/pkg/mod/github.com/nicksnyder/go-i18n/[email protected]/i18n/localizer.go:211 +0x7c
github.com/redhat-developer/app-services-cli/pkg/localize/goi18n.(*Goi18n).MustLocalize(0xc00059ad70, 0x1e37997, 0x32, 0xc000010f98, 0x1, 0x1, 0x8, 0x1acb800)
        /home/ephelan/code/github.com/redhat-developer/app-services-cli/pkg/localize/goi18n/go_i18n.go:85 +0x165
github.com/redhat-developer/app-services-cli/pkg/localize/goi18n.(*Goi18n).MustLocalizeError(0xc00059ad70, 0x1e37997, 0x32, 0xc000010f98, 0x1, 0x1, 0xc00021a400, 0xc0000bdcf8)
        /home/ephelan/code/github.com/redhat-developer/app-services-cli/pkg/localize/goi18n/go_i18n.go:95 +0x67
github.com/redhat-developer/app-services-cli/pkg/cmd/kafka/acl/grant.validateFlagInputCombination(0xc00025bad0, 0xc0001dfa20, 0x0)
        /home/ephelan/code/github.com/redhat-developer/app-services-cli/pkg/cmd/kafka/acl/grant/grant.go:298 +0x2c3
github.com/redhat-developer/app-services-cli/pkg/cmd/kafka/acl/grant.NewGrantPermissionsACLCommand.func1(0xc000264000, 0xc000352510, 0x0, 0x9, 0x0, 0x0)
        /home/ephelan/code/github.com/redhat-developer/app-services-cli/pkg/cmd/kafka/acl/grant/grant.go:70 +0x98
github.com/spf13/cobra.(*Command).execute(0xc000264000, 0xc000352480, 0x9, 0x9, 0xc000264000, 0xc000352480)
        /home/ephelan/.gvm/pkgsets/go1.16.4/global/pkg/mod/github.com/spf13/[email protected]/command.go:856 +0x472
github.com/spf13/cobra.(*Command).ExecuteC(0xc00054bb80, 0x20454d4, 0x3, 0xc00054bb80)
        /home/ephelan/.gvm/pkgsets/go1.16.4/global/pkg/mod/github.com/spf13/[email protected]/command.go:974 +0x375
github.com/spf13/cobra.(*Command).Execute(...)
        /home/ephelan/.gvm/pkgsets/go1.16.4/global/pkg/mod/github.com/spf13/[email protected]/command.go:902
main.main()
        /home/ephelan/code/github.com/redhat-developer/app-services-cli/cmd/rhoas/main.go:43 +0x29d

craicoverflow · 2021-10-11T14:29:08Z

Getting another i18n error when combining --group and --group-prefix

$ rhoas kafka acl grant-permissions --service-account srvc1 --consumer --group "*" --topic "*" --group-prefix "*"
panic: template: :1: unexpected "}" in operand

goroutine 1 [running]:
github.com/nicksnyder/go-i18n/v2/i18n.(*Localizer).MustLocalize(0xc000209d60, 0xc0004afbc8, 0x1deeb56, 0x8)
        /home/ephelan/.gvm/pkgsets/go1.16.4/global/pkg/mod/github.com/nicksnyder/go-i18n/[email protected]/i18n/localizer.go:211 +0x7c
github.com/redhat-developer/app-services-cli/pkg/localize/goi18n.(*Goi18n).MustLocalize(0xc0000aaf50, 0x1e37997, 0x32, 0xc000465520, 0x1, 0x1, 0x8, 0x1acb800)
        /home/ephelan/code/github.com/redhat-developer/app-services-cli/pkg/localize/goi18n/go_i18n.go:85 +0x165
github.com/redhat-developer/app-services-cli/pkg/localize/goi18n.(*Goi18n).MustLocalizeError(0xc0000aaf50, 0x1e37997, 0x32, 0xc000465520, 0x1, 0x1, 0xc0001a0800, 0xc0004afcf8)
        /home/ephelan/code/github.com/redhat-developer/app-services-cli/pkg/localize/goi18n/go_i18n.go:95 +0x67
github.com/redhat-developer/app-services-cli/pkg/cmd/kafka/acl/grant.validateFlagInputCombination(0xc0004084e0, 0xc000271ad0, 0x0)
        /home/ephelan/code/github.com/redhat-developer/app-services-cli/pkg/cmd/kafka/acl/grant/grant.go:305 +0x1ed
github.com/redhat-developer/app-services-cli/pkg/cmd/kafka/acl/grant.NewGrantPermissionsACLCommand.func1(0xc00040e000, 0xc0004de5a0, 0x0, 0x9, 0x0, 0x0)
        /home/ephelan/code/github.com/redhat-developer/app-services-cli/pkg/cmd/kafka/acl/grant/grant.go:70 +0x98
github.com/spf13/cobra.(*Command).execute(0xc00040e000, 0xc0004de510, 0x9, 0x9, 0xc00040e000, 0xc0004de510)
        /home/ephelan/.gvm/pkgsets/go1.16.4/global/pkg/mod/github.com/spf13/[email protected]/command.go:856 +0x472
github.com/spf13/cobra.(*Command).ExecuteC(0xc000375180, 0x20454d4, 0x3, 0xc000375180)
        /home/ephelan/.gvm/pkgsets/go1.16.4/global/pkg/mod/github.com/spf13/[email protected]/command.go:974 +0x375
github.com/spf13/cobra.(*Command).Execute(...)
        /home/ephelan/.gvm/pkgsets/go1.16.4/global/pkg/mod/github.com/spf13/[email protected]/command.go:902
main.main()
        /home/ephelan/code/github.com/redhat-developer/app-services-cli/cmd/rhoas/main.go:43 +0x29d

craicoverflow · 2021-10-11T14:38:26Z

$ rhoas kafka acl grant-permissions --service-account srvc1 --consumer --topic-prefix "*" --group-prefix "*"
❌ Undefined response type. Run the command in verbose mode using the -v flag to see more information

I'm seeing this error again, perhaps @wtrocki's changes were lost from the most recent commit which @rkpattnaik780 did not have locally?

craicoverflow · 2021-10-11T14:39:50Z

I've fixed it now.

wtrocki · 2021-10-12T09:47:23Z

pkg/localize/locales/en/cmd/acl.en.toml

+
+[kafka.acl.grantPermissions.group.error.notAllowed]
+one = 'group flag is allowed only when consumer ACLs are to be added'


Not sure about this message. What that means?

rkpattnaik780 · 2021-10-12T10:43:55Z

Not sure about this message. What that means?
@wtrocki this is the error message when --group is passed for producer operation.

wtrocki · 2021-10-12T10:45:08Z

--group flag is allowed only with --consumer flag

but current message is ok as well

openshift-ci bot added the do-not-merge/work-in-progress label Oct 5, 2021

craicoverflow reviewed Oct 5, 2021

View reviewed changes

pkg/cmd/kafka/acl/consumer/consumer.go Outdated Show resolved Hide resolved

craicoverflow reviewed Oct 5, 2021

View reviewed changes

pkg/cmd/kafka/acl/consumer/consumer.go Outdated Show resolved Hide resolved

wtrocki reviewed Oct 5, 2021

View reviewed changes

pkg/cmd/kafka/acl/consumer/consumer.go Outdated Show resolved Hide resolved

rkpattnaik780 force-pushed the acl_convenience branch from 6243f78 to a1214dc Compare October 5, 2021 14:21

bhardesty reviewed Oct 5, 2021

View reviewed changes

pkg/localize/locales/en/cmd/acl.en.toml Outdated Show resolved Hide resolved

bhardesty reviewed Oct 5, 2021

View reviewed changes

pkg/localize/locales/en/cmd/acl.en.toml Outdated Show resolved Hide resolved

bhardesty reviewed Oct 5, 2021

View reviewed changes

pkg/localize/locales/en/cmd/acl.en.toml Outdated Show resolved Hide resolved

bhardesty reviewed Oct 5, 2021

View reviewed changes

pkg/localize/locales/en/cmd/acl.en.toml Outdated Show resolved Hide resolved

bhardesty reviewed Oct 5, 2021

View reviewed changes

pkg/localize/locales/en/cmd/acl.en.toml Outdated Show resolved Hide resolved

bhardesty reviewed Oct 5, 2021

View reviewed changes

pkg/localize/locales/en/cmd/acl.en.toml Outdated Show resolved Hide resolved

bhardesty reviewed Oct 5, 2021

View reviewed changes

pkg/localize/locales/en/cmd/acl.en.toml Outdated Show resolved Hide resolved

bhardesty reviewed Oct 5, 2021

View reviewed changes

pkg/localize/locales/en/cmd/acl.en.toml Outdated Show resolved Hide resolved

bhardesty reviewed Oct 5, 2021

View reviewed changes

pkg/localize/locales/en/cmd/acl.en.toml Outdated Show resolved Hide resolved

bhardesty reviewed Oct 5, 2021

View reviewed changes

pkg/localize/locales/en/cmd/acl.en.toml Outdated Show resolved Hide resolved

bhardesty reviewed Oct 5, 2021

View reviewed changes

pkg/localize/locales/en/cmd/acl.en.toml Outdated Show resolved Hide resolved

bhardesty reviewed Oct 5, 2021

View reviewed changes

pkg/localize/locales/en/cmd/acl.en.toml Outdated Show resolved Hide resolved

bhardesty reviewed Oct 5, 2021

View reviewed changes

pkg/localize/locales/en/cmd/acl.en.toml Outdated Show resolved Hide resolved

wtrocki reviewed Oct 6, 2021

View reviewed changes

pkg/cmd/kafka/acl/grant/grant.go Show resolved Hide resolved

wtrocki reviewed Oct 6, 2021

View reviewed changes

pkg/cmd/kafka/acl/grant/grant.go Outdated Show resolved Hide resolved

wtrocki reviewed Oct 6, 2021

View reviewed changes

pkg/cmd/kafka/acl/grant/grant.go Outdated Show resolved Hide resolved

wtrocki reviewed Oct 6, 2021

View reviewed changes

pkg/cmd/kafka/acl/grant/grant.go Outdated Show resolved Hide resolved

craicoverflow reviewed Oct 6, 2021

View reviewed changes

pkg/localize/locales/en/cmd/acl.en.toml Outdated Show resolved Hide resolved

craicoverflow added this to the Kafka ACL commands milestone Oct 6, 2021

craicoverflow reviewed Oct 6, 2021

View reviewed changes

pkg/cmd/kafka/acl/grant/grant.go Outdated Show resolved Hide resolved

wtrocki reviewed Oct 6, 2021

View reviewed changes

pkg/cmd/kafka/acl/grant/grant.go Outdated Show resolved Hide resolved

craicoverflow force-pushed the kafka_acl branch from 9199834 to 8cfc1b8 Compare October 7, 2021 16:34

wtrocki mentioned this pull request Oct 11, 2021

Validation for multiple flags missing or present should have common helper #1204

Closed

fix: fix list order

ad6b9ff

wtrocki approved these changes Oct 11, 2021

View reviewed changes

wtrocki reviewed Oct 11, 2021

View reviewed changes

pkg/cmd/kafka/acl/grant/grant.go Outdated Show resolved Hide resolved

craicoverflow reviewed Oct 11, 2021

View reviewed changes

pkg/localize/locales/en/cmd/acl.en.toml Outdated Show resolved Hide resolved

rkpattnaik780 added 2 commits October 11, 2021 19:55

docs: add examples for simultaneos produce, consume

f31f60f

fix: typo in i18n string

b6b9219

fix: pass group prefix

a2e17fa

craicoverflow self-requested a review October 11, 2021 14:42

craicoverflow approved these changes Oct 11, 2021

View reviewed changes

fix: change strings and pass pointer for aclbinding

7a66709

This was referenced Oct 12, 2021

Print ACLs to be generated after running grant-permissions command #1208

Closed

grant-permissions should check if rules already exist #1209

Closed

Modularize runGrantPermissions method to smaller bits #1210

Closed

Batch creation of ACL rules #1211

Closed

docs: change missing operation message

8ab4831

wtrocki reviewed Oct 12, 2021

View reviewed changes

docs: format flag

96e4788

wtrocki approved these changes Oct 12, 2021

View reviewed changes

wtrocki merged commit 4613182 into kafka_acl Oct 12, 2021

wtrocki deleted the acl_convenience branch October 12, 2021 10:50

wtrocki pushed a commit that referenced this pull request Oct 12, 2021

feat(kafka acl): add acl grant-permissions command (#1179)

77c542d

rkpattnaik780 mentioned this pull request Oct 20, 2021

Add convenience command to give a principal producer access to a certain topic #1175

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(kafka acl): add convenience commands #1179

feat(kafka acl): add convenience commands #1179

rkpattnaik780 commented Oct 5, 2021 •

edited by craicoverflow

Loading

wtrocki commented Oct 5, 2021

bhardesty Oct 5, 2021

wtrocki commented Oct 6, 2021 •

edited

Loading

wtrocki left a comment

craicoverflow commented Oct 11, 2021

craicoverflow commented Oct 11, 2021

craicoverflow commented Oct 11, 2021

craicoverflow commented Oct 11, 2021

craicoverflow commented Oct 11, 2021

wtrocki Oct 12, 2021

rkpattnaik780 commented Oct 12, 2021

wtrocki commented Oct 12, 2021 •

edited

Loading

	one = 'prefix topic name to define ACL rules for'
	one = 'The prefix topic name to define ACL rules for'


		[kafka.acl.grantPermissions.group.error.notAllowed]
		one = 'group flag is allowed only when consumer ACLs are to be added'

feat(kafka acl): add convenience commands #1179

feat(kafka acl): add convenience commands #1179

Conversation

rkpattnaik780 commented Oct 5, 2021 • edited by craicoverflow Loading

Verification Steps

Type of change

Checklist

wtrocki commented Oct 5, 2021

bhardesty Oct 5, 2021

Choose a reason for hiding this comment

wtrocki commented Oct 6, 2021 • edited Loading

wtrocki left a comment

Choose a reason for hiding this comment

craicoverflow commented Oct 11, 2021

craicoverflow commented Oct 11, 2021

craicoverflow commented Oct 11, 2021

craicoverflow commented Oct 11, 2021

craicoverflow commented Oct 11, 2021

wtrocki Oct 12, 2021

Choose a reason for hiding this comment

rkpattnaik780 commented Oct 12, 2021

wtrocki commented Oct 12, 2021 • edited Loading

rkpattnaik780 commented Oct 5, 2021 •

edited by craicoverflow

Loading

wtrocki commented Oct 6, 2021 •

edited

Loading

wtrocki commented Oct 12, 2021 •

edited

Loading