Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11734 )
Change subject: IMPALA-7729: Fix invalidate metadata hang when there is an upper case role name ...................................................................... Patch Set 4: (7 comments) http://gerrit.cloudera.org:8080/#/c/11734/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11734/4//COMMIT_MSG@10 PS4, Line 10: original input role names I saw CatalogObjectCache is where users/roles are stored from AuthorizationPolicy. I see that the default, which is used, is to be case insensitive. So when it comes to names, they're lower-case in Impala. Issue is that it seems we sometimes use the name/key which is lowercase and sometimes the name from value (e.g., Principal) where case is preserved. http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java File fe/src/main/java/org/apache/impala/util/SentryProxy.java: http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@172 PS4, Line 172: String so this is lowercase or can it have casing that does not match the name that's stored in Role/User? http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@185 PS4, Line 185: existingRole ... and this contains a name that is not lower-cased? http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@186 PS4, Line 186: sentryRole.getRoleName() afaict, this will get lowercased when performing the lookup. http://gerrit.cloudera.org:8080/#/c/11734/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@198 PS4, Line 198: getRoleName ... and this is lowercased? http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py File tests/authorization/test_grant_revoke.py: http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@386 PS4, Line 386: grp.getgrnam( what's this? http://gerrit.cloudera.org:8080/#/c/11734/4/tests/authorization/test_grant_revoke.py@388 PS4, Line 388: "invalidate metadata" I think its good to have this test, but is there a more direct way to test this as well? For example, assign a privilege to a given role, then assign another privilege via sentry using different case. Will Impala see that the role then has both privs? -- To view, visit http://gerrit.cloudera.org:8080/11734 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5aa6f626ad3df4e9321ed18273d045517bc099c2 Gerrit-Change-Number: 11734 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Fri, 19 Oct 2018 07:06:07 +0000 Gerrit-HasComments: Yes
