Skip to content

Conversation

@bharatviswa504
Copy link
Contributor

@bharatviswa504 bharatviswa504 commented Jul 28, 2020

What changes were proposed in this pull request?

Currently, the getacl and setacl commands return wrong information when an external authorizer such as Ranger is enabled. There should be a check to verify if Native Authorizer is enabled before returning any response for these two commands.

If an external authorizer is enabled, it should show a nice message about managing acls in external authorizer.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-4020

How was this patch tested?

Added UT

Copy link
Contributor

@xiaoyuyao xiaoyuyao left a comment

Choose a reason for hiding this comment

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

Thanks @bharatviswa504 for reporting the issue and the fix. It looks good to me overall. I have a few comments inline.

String authorizerClass = configuration.get(OZONE_ACL_AUTHORIZER_CLASS);
if (authorizerClass != null &&
!authorizerClass.equals(OZONE_ACL_AUTHORIZER_CLASS_DEFAULT)) {
System.out.print(String.format("When External Authorizer %s is " +
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move System.out.print to AclHandler#execute based on the checkExternalAuthorizer return?

Copy link
Contributor

Choose a reason for hiding this comment

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

This configuration seem like a server side configuration and should come from authoritative source such as OM discovery.

What if the client does know what is configured on OM but OM is actually using native authorizer? If check based on client, we will not be able to do acl operations.

@elek
Copy link
Member

elek commented Aug 24, 2020

/pending Review comments are not addressed + no clean build

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Marking this issue as un-mergeable as requested.

Please use /ready comment when it's resolved.

Review comments are not addressed + no clean build

@bharatviswa504
Copy link
Contributor Author

Closing this as HDDS-4089 is trying to fix acl commands when external authorizer is configured.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants