Add topic permissions commands #48

zymap · 2019-09-10T08:44:17Z

OUTPUT

get-permissions`

➜  pulsarctl git:(topic-permission) ./pulsarctl topic get-permissions -h
USED FOR:
    This command is used for getting the permissions on a topic.

REQUIRED PERMISSION:
    This command requires namespace admin permissions.

EXAMPLES:
    #Get the permissions on a topic <topic-name>
    pulsarctl topic get-permissions <topic-name>

OUTPUT:
    #normal output
    {
      "<role>": [
        "<action>"
      ]
    }

    #the topic name is not specified
    [✖]  only one argument is allowed to be used as a name

    #the topic name is not in the format of <tenant>/<namespace>/<topic> or <topic>
    [✖]  Invalid short topic name '<topic-name>', it should be in the format of <tenant>/<namespace>/<topic> or <topic>

    #the topic name is not in the format of <domain>://<tenant>/<namespace>/<topic>
    [✖]  Invalid complete topic name '<topic-name>', it should be in the format of <domain>://<tenant>/<namespace>/<topic>

    #the topic name is not in the format of <tenant>/<namespace>/<topic>
    [✖]  Invalid topic name '<topic-name>', it should be in the format of<tenant>/<namespace>/<topic>

    #the namespace name is not in the format of <tenant>/<namespace>
    [✖]  The complete name of namespace is invalid. complete name : <namespace-complete-name>

    #the tenant name and(or) namespace name is empty
    [✖]  Invalid tenant or namespace. [<tenant>/<namespace>]

    #the tenant name contains unsupported special chars. the alphanumeric (a-zA-Z0-9) and the special chars (-=:.%)  is allowed
    [✖]  Tenant name include unsupported special chars. tenant : [<namespace>]

    #the namespace name contains unsupported special chars. the  alphanumeric (a-zA-Z0-9) and the special chars (-=:.%) is allowed
    [✖]  Namespace name include unsupported special chars. namespace : [<namespace>]

Usage: pulsarctl topics get-permissions [flags]

Aliases: get-permissions, get

grant-permissions

➜  pulsarctl git:(topic-permission) ./pulsarctl topic grant-permissions -h
USED FOR:
    This command is used for granting permissions to a client role on a single topic.

REQUIRED PERMISSION:
    This command requires namespace admin permissions.

EXAMPLES:
    #Grant permissions to a client on a single topic <topic-name>
    pulsarctl topic grant-permissions --role <role> --actions <action-1> --actions <action-2> <topic-name>

OUTPUT:
    #normal output
    Grant role %s and actions %v to the topic %s successfully

    #the topic name is not specified
    [✖]  only one argument is allowed to be used as a name

    #the specified role is empty
    Invalid role name

    #the specified actions is not allowed.
    The auth action  only can be specified as 'produce', 'consume', or 'functions'. Invalid auth action '<actions>'

    #the topic name is not in the format of <tenant>/<namespace>/<topic> or <topic>
    [✖]  Invalid short topic name '<topic-name>', it should be in the format of <tenant>/<namespace>/<topic> or <topic>

    #the topic name is not in the format of <domain>://<tenant>/<namespace>/<topic>
    [✖]  Invalid complete topic name '<topic-name>', it should be in the format of <domain>://<tenant>/<namespace>/<topic>

    #the topic name is not in the format of <tenant>/<namespace>/<topic>
    [✖]  Invalid topic name '<topic-name>', it should be in the format of<tenant>/<namespace>/<topic>

    #the namespace name is not in the format of <tenant>/<namespace>
    [✖]  The complete name of namespace is invalid. complete name : <namespace-complete-name>

    #the tenant name and(or) namespace name is empty
    [✖]  Invalid tenant or namespace. [<tenant>/<namespace>]

    #the tenant name contains unsupported special chars. the alphanumeric (a-zA-Z0-9) and the special chars (-=:.%)  is allowed
    [✖]  Tenant name include unsupported special chars. tenant : [<namespace>]

    #the namespace name contains unsupported special chars. the  alphanumeric (a-zA-Z0-9) and the special chars (-=:.%) is allowed
    [✖]  Namespace name include unsupported special chars. namespace : [<namespace>]

Usage: pulsarctl topics grant-permissions [flags]

Aliases: grant-permissions, grant

revoke-permissions

➜  pulsarctl git:(topic-permission) ./pulsarctl topic revoke-permissions -h
USED FOR:
    This command is used for revoking permissions on a topic.

REQUIRED PERMISSION:
    This command requires namespace admin permissions.

EXAMPLES:
OUTPUT:
    #normal output
    Revoke permissions for the role <role> to the topic <topic-name> successfully


    #the specified role is empty
    Invalid role name

    #the topic name is not specified
    [✖]  only one argument is allowed to be used as a name

    #the topic name is not in the format of <tenant>/<namespace>/<topic> or <topic>
    [✖]  Invalid short topic name '<topic-name>', it should be in the format of <tenant>/<namespace>/<topic> or <topic>

    #the topic name is not in the format of <domain>://<tenant>/<namespace>/<topic>
    [✖]  Invalid complete topic name '<topic-name>', it should be in the format of <domain>://<tenant>/<namespace>/<topic>

    #the topic name is not in the format of <tenant>/<namespace>/<topic>
    [✖]  Invalid topic name '<topic-name>', it should be in the format of<tenant>/<namespace>/<topic>

    #the namespace name is not in the format of <tenant>/<namespace>
    [✖]  The complete name of namespace is invalid. complete name : <namespace-complete-name>

    #the tenant name and(or) namespace name is empty
    [✖]  Invalid tenant or namespace. [<tenant>/<namespace>]

    #the tenant name contains unsupported special chars. the alphanumeric (a-zA-Z0-9) and the special chars (-=:.%)  is allowed
    [✖]  Tenant name include unsupported special chars. tenant : [<namespace>]

    #the namespace name contains unsupported special chars. the  alphanumeric (a-zA-Z0-9) and the special chars (-=:.%) is allowed
    [✖]  Namespace name include unsupported special chars. namespace : [<namespace>]

Usage: pulsarctl topics revoke-permissions [flags]

Aliases: revoke-permissions, revoke

sijie

after thinking a bit more, the permissions related operations are spreading over multiple places. we should revisit it overall.

one solution is to move 'permissions' as the topic level command group. so the commands will be:

pulsarctl permissions grant --roles <roles> --actions <actions> <entity>
pulsarctl permissions revoke --roles <roles> --actions <actions> <entity>
pulsarctl permissions get <entity>.

pulsarctl should determine what entities (a namespace, a topic or a function) the operations apply to.

for example, tenant/ns is a namespace name. tenant/ns/topic is a topic name or a function name.

pkg/ctl/topic/permission/get.go

sijie · 2019-09-10T18:12:19Z

pkg/ctl/topic/permission/get.go

+		return err
+	}
+
+	admin := cmdutils.NewPulsarClient()


how do we close admin? If we don't close admin, it might be worth fixing this problem in a subsequent pull request.

Why do we need to close admin?

why don't we need to close admin? releasing/closing a resource is always be the right practice to follow.

I wonder is there has any resource we always keep with admin?

isn't PulsarAdmin client a network client? If it is a network client, doesn't it keep any network related resources?

sijie · 2019-09-10T18:12:53Z

pkg/ctl/topic/permission/get_test.go

+	args := []string{"get-permissions"}
+	_, _, nameErr, _ := TestTopicCommands(GetPermissionsCmd, args)
+	assert.NotNil(t, nameErr)
+	assert.Equal(t, "only one argument is allowed to be used as a name", nameErr.Error())


Suggested change

assert.Equal(t, "only one argument is allowed to be used as a name", nameErr.Error())

assert.Equal(t, "only one argument is allowed to be used as a topic name", nameErr.Error())

It's a common output for all commands which need one argument. The description will show which arg types the command needed.

The error message is too generic. Users won’t be able to know what is the exact error. So you need to customize the error message for different commands.

Okay. So maybe we can use a separate pull request to do this. It will modify all commands who use GetNameArg to ensure there is only one name argument.

sijie · 2019-09-10T18:21:11Z

pkg/ctl/topic/permission/grant.go

+		Out:  "Invalid role name",
+	}
+
+	actionsError := Output{


it seems to me that this is a topic level permissions. I don't think it should accept functions permissions.

we should prevent people from granting functions permissions for a topic.

if broker doesn't prevent, we should add a logic to reject granting functions permissions for a topic. please create a Pulsar issue to track adding the logic.

So who will grant functions permission?

functions permission is only applied to namespace level.

zymap · 2019-09-11T07:26:18Z

after thinking a bit more, the permissions related operations are spreading over multiple places. we should revisit it overall.

one solution is to move 'permissions' as the topic level command group. so the commands will be:

pulsarctl permissions grant --roles <roles> --actions <actions> <entity>

pulsarctl permissions revoke --roles <roles> --actions <actions> <entity>

pulsarctl permissions get <entity>.

pulsarctl should determine what entities (a namespace, a topic or a function) the operations apply to.

for example, tenant/ns is a namespace name. tenant/ns/topic is a topic name or a function name.

@sijie Good idea. I will add a new command group.

zymap · 2019-09-19T01:41:59Z

@sijie PTAL

pkg/ctl/topic/permission/grant.go

pkg/ctl/topic/permission/revoke.go

zymap · 2019-09-19T13:54:27Z

@sijie PTAL. Thanks.

## OUTPUT *get-permissions`* ``` ➜ pulsarctl git:(topic-permission) ./pulsarctl topic get-permissions -h USED FOR: This command is used for getting the permissions on a topic. REQUIRED PERMISSION: This command requires namespace admin permissions. EXAMPLES: #Get the permissions on a topic <topic-name> pulsarctl topic get-permissions <topic-name> OUTPUT: #normal output { "<role>": [ "<action>" ] } #the topic name is not specified [✖] only one argument is allowed to be used as a name #the topic name is not in the format of <tenant>/<namespace>/<topic> or <topic> [✖] Invalid short topic name '<topic-name>', it should be in the format of <tenant>/<namespace>/<topic> or <topic> #the topic name is not in the format of <domain>://<tenant>/<namespace>/<topic> [✖] Invalid complete topic name '<topic-name>', it should be in the format of <domain>://<tenant>/<namespace>/<topic> #the topic name is not in the format of <tenant>/<namespace>/<topic> [✖] Invalid topic name '<topic-name>', it should be in the format of<tenant>/<namespace>/<topic> #the namespace name is not in the format of <tenant>/<namespace> [✖] The complete name of namespace is invalid. complete name : <namespace-complete-name> #the tenant name and(or) namespace name is empty [✖] Invalid tenant or namespace. [<tenant>/<namespace>] #the tenant name contains unsupported special chars. the alphanumeric (a-zA-Z0-9) and the special chars (-=:.%) is allowed [✖] Tenant name include unsupported special chars. tenant : [<namespace>] #the namespace name contains unsupported special chars. the alphanumeric (a-zA-Z0-9) and the special chars (-=:.%) is allowed [✖] Namespace name include unsupported special chars. namespace : [<namespace>] Usage: pulsarctl topics get-permissions [flags] Aliases: get-permissions, get ``` *grant-permissions* ``` ➜ pulsarctl git:(topic-permission) ./pulsarctl topic grant-permissions -h USED FOR: This command is used for granting permissions to a client role on a single topic. REQUIRED PERMISSION: This command requires namespace admin permissions. EXAMPLES: #Grant permissions to a client on a single topic <topic-name> pulsarctl topic grant-permissions --role <role> --actions <action-1> --actions <action-2> <topic-name> OUTPUT: #normal output Grant role %s and actions %v to the topic %s successfully #the topic name is not specified [✖] only one argument is allowed to be used as a name #the specified role is empty Invalid role name #the specified actions is not allowed. The auth action only can be specified as 'produce', 'consume', or 'functions'. Invalid auth action '<actions>' #the topic name is not in the format of <tenant>/<namespace>/<topic> or <topic> [✖] Invalid short topic name '<topic-name>', it should be in the format of <tenant>/<namespace>/<topic> or <topic> #the topic name is not in the format of <domain>://<tenant>/<namespace>/<topic> [✖] Invalid complete topic name '<topic-name>', it should be in the format of <domain>://<tenant>/<namespace>/<topic> #the topic name is not in the format of <tenant>/<namespace>/<topic> [✖] Invalid topic name '<topic-name>', it should be in the format of<tenant>/<namespace>/<topic> #the namespace name is not in the format of <tenant>/<namespace> [✖] The complete name of namespace is invalid. complete name : <namespace-complete-name> #the tenant name and(or) namespace name is empty [✖] Invalid tenant or namespace. [<tenant>/<namespace>] #the tenant name contains unsupported special chars. the alphanumeric (a-zA-Z0-9) and the special chars (-=:.%) is allowed [✖] Tenant name include unsupported special chars. tenant : [<namespace>] #the namespace name contains unsupported special chars. the alphanumeric (a-zA-Z0-9) and the special chars (-=:.%) is allowed [✖] Namespace name include unsupported special chars. namespace : [<namespace>] Usage: pulsarctl topics grant-permissions [flags] Aliases: grant-permissions, grant ``` *revoke-permissions* ``` ➜ pulsarctl git:(topic-permission) ./pulsarctl topic revoke-permissions -h USED FOR: This command is used for revoking permissions on a topic. REQUIRED PERMISSION: This command requires namespace admin permissions. EXAMPLES: OUTPUT: #normal output Revoke permissions for the role <role> to the topic <topic-name> successfully #the specified role is empty Invalid role name #the topic name is not specified [✖] only one argument is allowed to be used as a name #the topic name is not in the format of <tenant>/<namespace>/<topic> or <topic> [✖] Invalid short topic name '<topic-name>', it should be in the format of <tenant>/<namespace>/<topic> or <topic> #the topic name is not in the format of <domain>://<tenant>/<namespace>/<topic> [✖] Invalid complete topic name '<topic-name>', it should be in the format of <domain>://<tenant>/<namespace>/<topic> #the topic name is not in the format of <tenant>/<namespace>/<topic> [✖] Invalid topic name '<topic-name>', it should be in the format of<tenant>/<namespace>/<topic> #the namespace name is not in the format of <tenant>/<namespace> [✖] The complete name of namespace is invalid. complete name : <namespace-complete-name> #the tenant name and(or) namespace name is empty [✖] Invalid tenant or namespace. [<tenant>/<namespace>] #the tenant name contains unsupported special chars. the alphanumeric (a-zA-Z0-9) and the special chars (-=:.%) is allowed [✖] Tenant name include unsupported special chars. tenant : [<namespace>] #the namespace name contains unsupported special chars. the alphanumeric (a-zA-Z0-9) and the special chars (-=:.%) is allowed [✖] Namespace name include unsupported special chars. namespace : [<namespace>] Usage: pulsarctl topics revoke-permissions [flags] Aliases: revoke-permissions, revoke ```

zymap added 18 commits September 7, 2019 14:31

Add partitioned topic command CURD

76a8fcc

Revert changes

5c0e462

Add test for command partitioned topic

e3ba23b

Revert the refactor

f2f4ea2

Add non-persistent topic commands

c1e54db

Revert changes

a18f62a

Revert changes

f70a8ec

Fix typo

44b32f5

Add partitions value check

d4b5230

Move to a new package

113cc41

import tests

4e271e7

Remove unused code

9a5f6be

Update permission description

35224c5

fix command list error list

3b03dc6

Address comments

b213f04

Address comments

dbb887f

Merge branch 'master' into topic-partitioned-commands

8c707c6

Fix conflict

513b9fd

zymap requested a review from sijie September 10, 2019 08:44

zymap added the area/pulsar/topics label Sep 10, 2019

zymap added this to the 0.0.1 milestone Sep 10, 2019

zymap self-assigned this Sep 10, 2019

Add topic permissions commands

bc433fd

zymap force-pushed the topic-permission branch from 9cd82c0 to bc433fd Compare September 10, 2019 08:48

Fix build error

732f842

sijie requested changes Sep 10, 2019

View reviewed changes

sijie mentioned this pull request Sep 18, 2019

pulsarctl MVP #2

Closed

29 tasks

Address comments

9f36df6

sijie requested changes Sep 19, 2019

View reviewed changes

pkg/ctl/topic/permission/grant.go Outdated Show resolved Hide resolved

pkg/ctl/topic/permission/revoke.go Outdated Show resolved Hide resolved

pkg/ctl/topic/permission/revoke.go Outdated Show resolved Hide resolved

Address comments

bc0f970

zymap changed the base branch from topic-partitioned-commands to master September 19, 2019 11:35

zymap added 2 commits September 19, 2019 19:39

Merge branch 'master' into topic-permission

98b4bdf

Fix build error

e4c1710

sijie approved these changes Sep 19, 2019

View reviewed changes

sijie merged commit 58d9832 into master Sep 19, 2019

sijie deleted the topic-permission branch September 19, 2019 15:34

	assert.Equal(t, "only one argument is allowed to be used as a name", nameErr.Error())
	assert.Equal(t, "only one argument is allowed to be used as a topic name", nameErr.Error())

Add topic permissions commands #48

Add topic permissions commands #48

Uh oh!

Conversation

zymap commented Sep 10, 2019

OUTPUT

Uh oh!

sijie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zymap commented Sep 11, 2019

Uh oh!

zymap commented Sep 19, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zymap commented Sep 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants