You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2021/10/19 17:29:05 UTC
[GitHub] [phoenix] stoty opened a new pull request #1335: PHOENIX-6555 Wait for permissions to sync in Permission tests
stoty opened a new pull request #1335:
URL: https://github.com/apache/phoenix/pull/1335
This copies and used the permission sync code from the HBase tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #1335: PHOENIX-6555 Wait for permissions to sync in Permission tests
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1335:
URL: https://github.com/apache/phoenix/pull/1335#issuecomment-949316637
Thanks @virajjasani , I've addressed your points before commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #1335: PHOENIX-6555 Wait for permissions to sync in Permission tests
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1335:
URL: https://github.com/apache/phoenix/pull/1335#issuecomment-948354282
Thanks @stoty, will get to review this soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty closed pull request #1335: PHOENIX-6555 Wait for permissions to sync in Permission tests
Posted by GitBox <gi...@apache.org>.
stoty closed pull request #1335:
URL: https://github.com/apache/phoenix/pull/1335
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #1335: PHOENIX-6555 Wait for permissions to sync in Permission tests
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1335:
URL: https://github.com/apache/phoenix/pull/1335#issuecomment-947326083
The tests are successful, but they couldn't update the PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on a change in pull request #1335: PHOENIX-6555 Wait for permissions to sync in Permission tests
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #1335:
URL: https://github.com/apache/phoenix/pull/1335#discussion_r733682747
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java
##########
@@ -977,6 +1090,12 @@ public void aTestRXPermsReqdForPhoenixConn() throws Exception {
verifyDenied(getConnectionAction(), org.apache.hadoop.hbase.TableNotFoundException.class, regularUser1);
}
+ //Initialize Phoenix to avoid timeouts later
+ try (Connection conn = getConnection();
+ Statement stmt = conn.createStatement();) {
+ stmt.execute("select * from system.catalog");
+ }
Review comment:
better
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java
##########
@@ -252,26 +254,82 @@ public static HBaseTestingUtility getUtility(){
}
// Utility functions to grant permissions with HBase API
- void grantPermissions(String toUser, Set<String> tablesToGrant, Permission.Action... actions) throws Throwable {
- for (String table : tablesToGrant) {
- AccessControlClient.grant(getUtility().getConnection(), TableName.valueOf(table), toUser, null, null,
- actions);
- }
+ void grantPermissions(String toUser, Set<String> tablesToGrant, Permission.Action... actions)
+ throws Throwable {
+ updateACLs(getUtility(), new Callable<Void>() {
+ @Override
+ public Void call() throws Exception {
+ try {
+ for (String table : tablesToGrant) {
+ AccessControlClient.grant(getUtility().getConnection(),
+ TableName.valueOf(table), toUser, null, null, actions);
+ }
+ return null;
+ } catch (Throwable t) {
+ if (t instanceof Exception) {
+ throw (Exception) t;
+ } else {
+ throw new Exception(t);
+ }
+ }
+ }
+ });
}
void grantPermissions(String toUser, String namespace, Permission.Action... actions) throws Throwable {
- AccessControlClient.grant(getUtility().getConnection(), namespace, toUser, actions);
+ updateACLs(getUtility(), new Callable<Void>() {
+ @Override
+ public Void call() throws Exception {
+ try {
+ AccessControlClient.grant(getUtility().getConnection(), namespace, toUser, actions);
+ return null;
+ } catch (Throwable t) {
+ if (t instanceof Exception) {
+ throw (Exception) t;
+ } else {
+ throw new Exception(t);
+ }
+ }
+ }
+ });
}
void grantPermissions(String groupEntry, Permission.Action... actions) throws IOException, Throwable {
- AccessControlClient.grant(getUtility().getConnection(), groupEntry, actions);
+ updateACLs(getUtility(), new Callable<Void>() {
+ @Override
+ public Void call() throws Exception {
+ try {
+ AccessControlClient.grant(getUtility().getConnection(), groupEntry, actions);
+ return null;
+ } catch (Throwable t) {
+ if (t instanceof Exception) {
+ throw (Exception) t;
+ } else {
+ throw new Exception(t);
+ }
+ }
+ }
+ });
}
- // Utility functions to revoke permissions with HBase API
- void revokeAll() throws Throwable {
- AccessControlClient.revoke(getUtility().getConnection(), AuthUtil.toGroupEntry(GROUP_SYSTEM_ACCESS), Permission.Action.values() );
- AccessControlClient.revoke(getUtility().getConnection(), regularUser1.getShortName(), Permission.Action.values() );
- AccessControlClient.revoke(getUtility().getConnection(), unprivilegedUser.getShortName(), Permission.Action.values() );
+ void revokePermissions(String toUser, Set<String> tablesToGrant, Permission.Action... actions) throws Throwable {
+ updateACLs(getUtility(), new Callable<Void>() {
+ @Override
+ public Void call() throws Exception {
+ try {
+ for (String table : tablesToGrant) {
+ AccessControlClient.revoke(getUtility().getConnection(), TableName.valueOf(table), toUser, null, null, actions);
Review comment:
nit: checkstyle line len might complain?
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java
##########
@@ -84,13 +89,14 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
-import static org.junit.Assume.assumeFalse;
@FixMethodOrder(MethodSorters.NAME_ASCENDING)
public abstract class BasePermissionsIT extends BaseTest {
private static final Logger LOGGER = LoggerFactory.getLogger(BasePermissionsIT.class);
+ private static final int WAIT_TIME = 10000;
+
private static String SUPER_USER = System.getProperty("user.name");
Review comment:
Not related to this change, but can be made `final` if you would like.
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java
##########
@@ -252,26 +254,82 @@ public static HBaseTestingUtility getUtility(){
}
// Utility functions to grant permissions with HBase API
- void grantPermissions(String toUser, Set<String> tablesToGrant, Permission.Action... actions) throws Throwable {
- for (String table : tablesToGrant) {
- AccessControlClient.grant(getUtility().getConnection(), TableName.valueOf(table), toUser, null, null,
- actions);
- }
+ void grantPermissions(String toUser, Set<String> tablesToGrant, Permission.Action... actions)
+ throws Throwable {
+ updateACLs(getUtility(), new Callable<Void>() {
+ @Override
+ public Void call() throws Exception {
+ try {
+ for (String table : tablesToGrant) {
+ AccessControlClient.grant(getUtility().getConnection(),
+ TableName.valueOf(table), toUser, null, null, actions);
+ }
+ return null;
+ } catch (Throwable t) {
+ if (t instanceof Exception) {
+ throw (Exception) t;
+ } else {
+ throw new Exception(t);
+ }
+ }
+ }
+ });
}
void grantPermissions(String toUser, String namespace, Permission.Action... actions) throws Throwable {
- AccessControlClient.grant(getUtility().getConnection(), namespace, toUser, actions);
+ updateACLs(getUtility(), new Callable<Void>() {
+ @Override
+ public Void call() throws Exception {
+ try {
+ AccessControlClient.grant(getUtility().getConnection(), namespace, toUser, actions);
+ return null;
+ } catch (Throwable t) {
+ if (t instanceof Exception) {
+ throw (Exception) t;
+ } else {
+ throw new Exception(t);
+ }
+ }
+ }
+ });
}
void grantPermissions(String groupEntry, Permission.Action... actions) throws IOException, Throwable {
Review comment:
Looks like IOException is not required, again not relevant but good to update
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org