[
https://issues.apache.org/jira/browse/CASSANDRA-10552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14965002#comment-14965002
]
Sam Tunnicliffe commented on CASSANDRA-10552:
---------------------------------------------
I can see that making {{IResource}} a more accessible extension point might
have benefits for integration. What this patch alone won't do is provide a
means to manage permissions on custom resources via CQL. As it stands, the
grammar only permits the existing implementations in {{GRANT}} and {{REVOKE}}
statements. That could be extended to support something like {{GRANT ALL ON
CUSTOM RESOURCE foos/foo1 TO <role>}}, otherwise permissions will only be able
to be granted/revoked programatically.
CQL syntax aside, there's also a bug in the patch. Because {{IResource}}
implementations are lazily registered, attempting to create one from its
representation in the system table will fail if the class hasn't already been
loaded. A simple repro is to login as the default super user & create a
keyspace, then restart C*, login again & run {{LIST ALL PERMISSIONS}}. The
permissions representing data resources are ok as the class is referenced in
the static initializer of {{ClientState}}, but {{FunctionResource}} hasn't been
registered yet and so an {{IllegalArgumentException}} is thrown. For this to
work, we'll need some means of discovering all of the available IResource
implementations at startup and ensuring they're registered. While we're doing
that, it could be worth adding a {{StartupCheck}} to ensure that if auth is
enabled, there's a registered factory for every resource listed in
{{system_auth.role_permissions}}, in case a jar containing a custom
implementation is missing etc. The check could also verify that there are no
conflicts on resource prefixes.
I'd also like to formalize the structure of resource names a little, to make
the registration & factory stuff a little bit cleaner. Extracting the {{/}}
separator out into a constant on {{IResource}} would let us split out the root
element from a resource name & look up the factory for a given name directly,
rather than iterating & checking {{startsWith}}. It would also enforce a bit
more structure & consistency on custom implementations. It would also be good
to add something to the class level javadoc for {{IResource}} which explains
about registration & details the requirement for a static initializer etc I
notice that that doc is already outdated as it asserts that {{DataResource}} is
the only implementation, that's my bad so I'll update that on commit.
Lastly, it would be nice to get some test coverage for this. Aside from the
discovery bug, most regressions should be covered by the existing dtests, but I
think it'll be possible to add a unit test which exercises custom resource
impls. This would be valuable not least because it would require providing
custom resource impl, which may highlight assumptions we've previously made,
particularly in the case of CQL syntax etc.
> Pluggable IResources
> --------------------
>
> Key: CASSANDRA-10552
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10552
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Anthony Cozzie
> Assignee: Anthony Cozzie
> Fix For: 3.0.0
>
> Attachments: cassandra-3.0.0-10552.txt
>
>
> It is impossible to add new IResources because of the static method
> Resources.fromName(), which creates IResources from the text values in the
> authentication tables. This patch replaces the static list of checks with a
> hash table that can be extended.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
