Uploaded image for project: 'Sentry'
  1. Sentry
  2. SENTRY-872 Uber jira for HMS HA + Sentry HA redesign
  3. SENTRY-1825

Dropping a Hive database/table doesn't cleanup the permissions associated with it

    Details

    • Type: Sub-task
    • Status: Patch Available
    • Priority: Critical
    • Resolution: Unresolved
    • Affects Version/s: sentry-ha-redesign
    • Fix Version/s: None
    • Component/s: None
    • Labels:

      Description

      Sasha helped in finding this bug. Looks like dropping a database/table does no longer clean up the privileges associated with it.

      This problem is because of:
      https://github.com/apache/sentry/blob/sentry-ha-redesign/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java#L126-L127

      final HiveConf hiveConf = new HiveConf();
          hiveInstance = hiveConf.get(HiveAuthzConf.AuthzConfVars.AUTHZ_SERVER_NAME.getVar());
      

      With the latest redesign, we are only setting this property on Hive's (sentry-site.xml) and not on Sentry's (sentry-site.xml).

      So during permission grants, Hive ensures to supply the server1 for permission updates. But when we drop the table/database that has the perms attached, it goes through HMSFollower and this code sets the property as NULL as sentry-site.xml doesn't have this set. So it attempts to remove permissions with NULL server setting and this always returns without deleting anything.

      We need to ensure that the corresponding property is set on both (Sentry, Hive) sentry-site.xml to ensure referring to proper privileges.

        Issue Links

          Activity

          Hide
          akolb Alexander Kolbasov added a comment -

          Since kalyan kumar kalvagadda is doing HMSFollower refactoring assigning this to him.

          Show
          akolb Alexander Kolbasov added a comment - Since kalyan kumar kalvagadda is doing HMSFollower refactoring assigning this to him.
          Hide
          akolb Alexander Kolbasov added a comment -

          The fix for this should include a unit or e2e test that demonstrates the problem and verifies that it is fixed.

          Show
          akolb Alexander Kolbasov added a comment - The fix for this should include a unit or e2e test that demonstrates the problem and verifies that it is fixed.
          Hide
          akolb Alexander Kolbasov added a comment -

          This bug was first found by code inspection and then verified on a real setup.

          Show
          akolb Alexander Kolbasov added a comment - This bug was first found by code inspection and then verified on a real setup.
          Hide
          akolb Alexander Kolbasov added a comment -

          It also seems to be the confusion about the actual variable name to hold the value of the server - it may be referred to as hive.sentry.server or as sentry.hive.server.

          Please note that it has absolutely nothing to do with any real server - it is just a string that is almost always set to server1.

          Show
          akolb Alexander Kolbasov added a comment - It also seems to be the confusion about the actual variable name to hold the value of the server - it may be referred to as hive.sentry.server or as sentry.hive.server . Please note that it has absolutely nothing to do with any real server - it is just a string that is almost always set to server1 .
          Hide
          akolb Alexander Kolbasov added a comment - - edited

          CM always sets this as hive.sentry.server. It is present in sentry-site.xml which is generated for hive (but not in Sentry's sentry-site.xml file).

          Show
          akolb Alexander Kolbasov added a comment - - edited CM always sets this as hive.sentry.server . It is present in sentry-site.xml which is generated for hive (but not in Sentry's sentry-site.xml file).
          Hide
          LinaAtAustin Na Li added a comment -

          There are several problems:

          1) HMSFollower should read sentry-site.xml to get the server name, not from hive-site.xml through hiveConf. The input configuration for HMSFollower constructor is from sentry-site.xm. We should use that configuration to get server name.
          2) There are two variable names that could hold the value of the server.

          hive.sentry.server

          is deprecated and is in

          HiveAuthzConf.AuthzConfVars.AUTHZ_SERVER_NAME_DEPRECATED

          . The new name is

          sentry.hive.server

          in

          HiveAuthzConf.AuthzConfVars.AUTHZ_SERVER_NAME


          We should check the

          sentry.hive.server

          first. If it is not set, check the deprecated

          hive.sentry.server

          to be backward compatible.
          3) Whoever generates the sentry-site.xml for hive and sentry needs to make sure the variable is set for both host hive and sentry and of the same value.

          kalyan kumar kalvagadda will make update for items 1) and 2) as he is refectoring HMSFollower. I will make sure item 3) is handled by proper team. And add e2e test to show the issue and verify the fix. In e2e test, the sentry-site.xml will be manually generated in test setup

          Show
          LinaAtAustin Na Li added a comment - There are several problems: 1) HMSFollower should read sentry-site.xml to get the server name, not from hive-site.xml through hiveConf. The input configuration for HMSFollower constructor is from sentry-site.xm. We should use that configuration to get server name. 2) There are two variable names that could hold the value of the server. hive.sentry.server is deprecated and is in HiveAuthzConf.AuthzConfVars.AUTHZ_SERVER_NAME_DEPRECATED . The new name is sentry.hive.server in HiveAuthzConf.AuthzConfVars.AUTHZ_SERVER_NAME We should check the sentry.hive.server first. If it is not set, check the deprecated hive.sentry.server to be backward compatible. 3) Whoever generates the sentry-site.xml for hive and sentry needs to make sure the variable is set for both host hive and sentry and of the same value. kalyan kumar kalvagadda will make update for items 1) and 2) as he is refectoring HMSFollower. I will make sure item 3) is handled by proper team. And add e2e test to show the issue and verify the fix. In e2e test, the sentry-site.xml will be manually generated in test setup
          Hide
          LinaAtAustin Na Li added a comment -

          the issues 1) and 2) will be in sentry-1769 when refectoring HMSFollower.

          Show
          LinaAtAustin Na Li added a comment - the issues 1) and 2) will be in sentry-1769 when refectoring HMSFollower.
          Hide
          kkalyan kalyan kumar kalvagadda added a comment -

          Will be handling this code change separately as the HMSFollower refactoring is in it's final state of review. It's not a good idea to bring in new functional changes at this point in time.

          Show
          kkalyan kalyan kumar kalvagadda added a comment - Will be handling this code change separately as the HMSFollower refactoring is in it's final state of review. It's not a good idea to bring in new functional changes at this point in time.
          Hide
          LinaAtAustin Na Li added a comment -

          v1. get server name from sentry config and add test

          Show
          LinaAtAustin Na Li added a comment - v1. get server name from sentry config and add test
          Hide
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12877373/SENTRY-1825.001-sentry-ha-redesign.patch against sentry-ha-redesign.

          Overall: -1 due to 2 errors

          ERROR: mvn test exited 1
          ERROR: Failed: org.apache.sentry.binding.hive.TestHiveAuthzConf

          Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/3042/console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12877373/SENTRY-1825.001-sentry-ha-redesign.patch against sentry-ha-redesign. Overall: -1 due to 2 errors ERROR: mvn test exited 1 ERROR: Failed: org.apache.sentry.binding.hive.TestHiveAuthzConf Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/3042/console This message is automatically generated.
          Hide
          LinaAtAustin Na Li added a comment -

          v2. Fix failed test case in TestHiveAuthzConf

          Show
          LinaAtAustin Na Li added a comment - v2. Fix failed test case in TestHiveAuthzConf
          Hide
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12877386/SENTRY-1825.002-sentry-ha-redesign.patch against sentry-ha-redesign.

          Overall: +1 all checks pass

          SUCCESS: all tests passed

          Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/3043/console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12877386/SENTRY-1825.002-sentry-ha-redesign.patch against sentry-ha-redesign. Overall: +1 all checks pass SUCCESS: all tests passed Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/3043/console This message is automatically generated.
          Hide
          LinaAtAustin Na Li added a comment -

          v3. update based on Brian's comment

          Show
          LinaAtAustin Na Li added a comment - v3. update based on Brian's comment
          Hide
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12877674/SENTRY-1825.003-sentry-ha-redesign.patch against sentry-ha-redesign.

          Overall: +1 all checks pass

          SUCCESS: all tests passed

          Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/3056/console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12877674/SENTRY-1825.003-sentry-ha-redesign.patch against sentry-ha-redesign. Overall: +1 all checks pass SUCCESS: all tests passed Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/3056/console This message is automatically generated.
          Hide
          LinaAtAustin Na Li added a comment -

          v4. rebase on latest code

          Show
          LinaAtAustin Na Li added a comment - v4. rebase on latest code
          Hide
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12877860/SENTRY-1825.004-sentry-ha-redesign.patch against sentry-ha-redesign.

          Overall: +1 all checks pass

          SUCCESS: all tests passed

          Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/3069/console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12877860/SENTRY-1825.004-sentry-ha-redesign.patch against sentry-ha-redesign. Overall: +1 all checks pass SUCCESS: all tests passed Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/3069/console This message is automatically generated.
          Hide
          LinaAtAustin Na Li added a comment -

          v5. update base on Kalyan's code review

          Show
          LinaAtAustin Na Li added a comment - v5. update base on Kalyan's code review
          Hide
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12878056/SENTRY-1825.005-sentry-ha-redesign.patch against sentry-ha-redesign.

          Overall: -1 due to 6 errors

          ERROR: mvn test exited 1
          ERROR: Failed: org.apache.sentry.tests.e2e.solr.TestUpdateOperations
          ERROR: Failed: org.apache.sentry.tests.e2e.solr.db.integration.TestSolrUpdateOperations
          ERROR: Failed: org.apache.sentry.tests.e2e.solr.db.integration.TestSolrUpdateOperations
          ERROR: Failed: org.apache.sentry.tests.e2e.solr.TestQueryOperations
          ERROR: Failed: org.apache.sentry.tests.e2e.solr.TestDocLevelOperations

          Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/3078/console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12878056/SENTRY-1825.005-sentry-ha-redesign.patch against sentry-ha-redesign. Overall: -1 due to 6 errors ERROR: mvn test exited 1 ERROR: Failed: org.apache.sentry.tests.e2e.solr.TestUpdateOperations ERROR: Failed: org.apache.sentry.tests.e2e.solr.db.integration.TestSolrUpdateOperations ERROR: Failed: org.apache.sentry.tests.e2e.solr.db.integration.TestSolrUpdateOperations ERROR: Failed: org.apache.sentry.tests.e2e.solr.TestQueryOperations ERROR: Failed: org.apache.sentry.tests.e2e.solr.TestDocLevelOperations Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/3078/console This message is automatically generated.
          Hide
          LinaAtAustin Na Li added a comment -

          v5 again.

          Show
          LinaAtAustin Na Li added a comment - v5 again.
          Hide
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12878205/SENTRY-1825.005-sentry-ha-redesign.patch against sentry-ha-redesign.

          Overall: +1 all checks pass

          SUCCESS: all tests passed

          Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/3079/console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12878205/SENTRY-1825.005-sentry-ha-redesign.patch against sentry-ha-redesign. Overall: +1 all checks pass SUCCESS: all tests passed Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/3079/console This message is automatically generated.
          Hide
          akolb Alexander Kolbasov added a comment -

          The patch should be re-merged with the current bits.

          Show
          akolb Alexander Kolbasov added a comment - The patch should be re-merged with the current bits.

            People

            • Assignee:
              LinaAtAustin Na Li
              Reporter:
              vamsee Vamsee Yarlagadda
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:

                Development