Details

    • Bug
    • Status: Resolved
    • Critical
    • Resolution: Fixed
    • None
    • 1.7.0
    • Sentry
    • None

    Attachments

      1. SENTRY-1087.4.patch
        19 kB
        Hao Hao
      2. SENTRY-1087.3.patch
        19 kB
        Hao Hao
      3. SENTRY-1087.2.patch
        18 kB
        Hao Hao
      4. SENTRY-1087.1.patch
        19 kB
        Hao Hao
      5. SENTRY-1087.0.patch
        13 kB
        Hao Hao

      Issue Links

        Activity

          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12790292/SENTRY-1087.0.patch against master.

          Overall: -1 due to an error

          ERROR: failed to build with patch (exit code 1)

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

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12790292/SENTRY-1087.0.patch against master. Overall: -1 due to an error ERROR: failed to build with patch (exit code 1) Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/1249/console This message is automatically generated.
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12790293/SENTRY-1087.0.patch against master.

          Overall: -1 due to an error

          ERROR: failed to build with patch (exit code 1)

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

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12790293/SENTRY-1087.0.patch against master. Overall: -1 due to an error ERROR: failed to build with patch (exit code 1) Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/1250/console This message is automatically generated.
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12790298/SENTRY-1087.0.patch against master.

          Overall: -1 due to an error

          ERROR: failed to build with patch (exit code 1)

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

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12790298/SENTRY-1087.0.patch against master. Overall: -1 due to an error ERROR: failed to build with patch (exit code 1) Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/1251/console This message is automatically generated.
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12790302/SENTRY-1087.0.patch against master.

          Overall: -1 due to 3 errors

          ERROR: mvn test exited 1
          ERROR: Failed: org.apache.sentry.tests.e2e.metastore.TestMetastoreEndToEnd
          ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbMetadataObjectRetrieval

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

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12790302/SENTRY-1087.0.patch against master. Overall: -1 due to 3 errors ERROR: mvn test exited 1 ERROR: Failed: org.apache.sentry.tests.e2e.metastore.TestMetastoreEndToEnd ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbMetadataObjectRetrieval Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/1252/console This message is automatically generated.
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12790422/SENTRY-1087.0.patch against master.

          Overall: -1 due to 4 errors

          ERROR: mvn test exited 1
          ERROR: Failed: org.apache.sentry.tests.e2e.metastore.TestMetastoreEndToEnd
          ERROR: Failed: org.apache.sentry.tests.e2e.hive.TestPrivilegesAtFunctionScope
          ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbPrivilegesAtFunctionScope

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

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12790422/SENTRY-1087.0.patch against master. Overall: -1 due to 4 errors ERROR: mvn test exited 1 ERROR: Failed: org.apache.sentry.tests.e2e.metastore.TestMetastoreEndToEnd ERROR: Failed: org.apache.sentry.tests.e2e.hive.TestPrivilegesAtFunctionScope ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbPrivilegesAtFunctionScope Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/1254/console This message is automatically generated.
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12790546/SENTRY-1087.0.patch against master.

          Overall: -1 due to 8 errors

          ERROR: mvn test exited 1
          ERROR: Failed: org.apache.sentry.tests.e2e.metastore.TestMetastoreEndToEnd
          ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbPrivilegesAtColumnScope
          ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbPrivilegesAtColumnScope
          ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbPrivilegesAtColumnScope
          ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbPrivilegesAtColumnScope
          ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbPrivilegesAtColumnScope
          ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbPrivilegesAtColumnScope

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

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12790546/SENTRY-1087.0.patch against master. Overall: -1 due to 8 errors ERROR: mvn test exited 1 ERROR: Failed: org.apache.sentry.tests.e2e.metastore.TestMetastoreEndToEnd ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbPrivilegesAtColumnScope ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbPrivilegesAtColumnScope ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbPrivilegesAtColumnScope ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbPrivilegesAtColumnScope ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbPrivilegesAtColumnScope ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbPrivilegesAtColumnScope Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/1257/console This message is automatically generated.
          hahao Hao Hao added a comment -

          The failed test cases do not seems related to the changes.

          hahao Hao Hao added a comment - The failed test cases do not seems related to the changes.

          Is there a review board link for this? Seems like we are only whitelisting org.apache.hadoop.hive.serde2, is that the only in built serde?

          sravya Sravya Tirukkovalur added a comment - Is there a review board link for this? Seems like we are only whitelisting org.apache.hadoop.hive.serde2, is that the only in built serde?

          Nevermind, code change is small. I am reviewing it now.

          sravya Sravya Tirukkovalur added a comment - Nevermind, code change is small. I am reviewing it now.
          sravya Sravya Tirukkovalur added a comment - - edited

          Looks good to me. One point:
          Can we add a test case for custom serde?

          sravya Sravya Tirukkovalur added a comment - - edited Looks good to me. One point: Can we add a test case for custom serde?
          hahao Hao Hao added a comment -

          sravya Thanks a lot for reviewing it! Will add the custom serde test case.

          hahao Hao Hao added a comment - sravya Thanks a lot for reviewing it! Will add the custom serde test case.
          hahao Hao Hao added a comment -

          Added the custom serde test cases. Thanks!

          hahao Hao Hao added a comment - Added the custom serde test cases. Thanks!
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12790640/SENTRY-1087.1.patch against master.

          Overall: -1 due to 5 errors

          ERROR: mvn test exited 1
          ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbPrivilegesAtFunctionScope
          ERROR: Failed: org.apache.sentry.tests.e2e.metastore.TestMetastoreEndToEnd
          ERROR: Failed: org.apache.sentry.tests.e2e.hive.TestPrivilegesAtFunctionScope
          ERROR: Failed: org.apache.sentry.tests.e2e.hive.TestCustomSerdePrivileges

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

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12790640/SENTRY-1087.1.patch against master. Overall: -1 due to 5 errors ERROR: mvn test exited 1 ERROR: Failed: org.apache.sentry.tests.e2e.dbprovider.TestDbPrivilegesAtFunctionScope ERROR: Failed: org.apache.sentry.tests.e2e.metastore.TestMetastoreEndToEnd ERROR: Failed: org.apache.sentry.tests.e2e.hive.TestPrivilegesAtFunctionScope ERROR: Failed: org.apache.sentry.tests.e2e.hive.TestCustomSerdePrivileges Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/1265/console This message is automatically generated.
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12790670/SENTRY-1087.1.patch against master.

          Overall: -1 due to 2 errors

          ERROR: mvn test exited 1
          ERROR: Failed: org.apache.sentry.tests.e2e.metastore.TestMetastoreEndToEnd

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

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12790670/SENTRY-1087.1.patch against master. Overall: -1 due to 2 errors ERROR: mvn test exited 1 ERROR: Failed: org.apache.sentry.tests.e2e.metastore.TestMetastoreEndToEnd Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/1267/console This message is automatically generated.

          Can you move the ast parsing for serde class name and checking to a utility function so that we can reuse it in both TOK.CREATE_TABLE and TOK.ALTER_TABLE ?

          sravya Sravya Tirukkovalur added a comment - Can you move the ast parsing for serde class name and checking to a utility function so that we can reuse it in both TOK.CREATE_TABLE and TOK.ALTER_TABLE ?

          +1. LGTM

          sravya Sravya Tirukkovalur added a comment - +1. LGTM
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12790767/SENTRY-1087.2.patch against master.

          Overall: -1 due to 2 errors

          ERROR: mvn test exited 1
          ERROR: Failed: org.apache.sentry.provider.db.generic.tools.TestSentryShellSolr

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

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12790767/SENTRY-1087.2.patch against master. Overall: -1 due to 2 errors ERROR: mvn test exited 1 ERROR: Failed: org.apache.sentry.provider.db.generic.tools.TestSentryShellSolr Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/1270/console This message is automatically generated.
          lskuff Lenni Kuff added a comment -

          Patch looks good. My only comment is that this introduces a compatibility issue (Hive + Sentry users who upgrade to 1.7 may have jobs that previously worked fine start to fail). I wonder if we should disable these checks by default and provide a flag that optionally enable them?

          lskuff Lenni Kuff added a comment - Patch looks good. My only comment is that this introduces a compatibility issue (Hive + Sentry users who upgrade to 1.7 may have jobs that previously worked fine start to fail). I wonder if we should disable these checks by default and provide a flag that optionally enable them?
          hahao Hao Hao added a comment -

          lskuff Thanks for pointed out that issues. I added the flag HIVE_SENTRY_SERDE_URI_PRIVILIEGES_ENABLED to address the compatibility issues in the latest patch.

          hahao Hao Hao added a comment - lskuff Thanks for pointed out that issues. I added the flag HIVE_SENTRY_SERDE_URI_PRIVILIEGES_ENABLED to address the compatibility issues in the latest patch.
          lskuff Lenni Kuff added a comment -

          Thanks for updating the patch. A few additional comments:

          • getSerdeURI() - It's strange that this returns void. Should this be called setSerdeURI() ? Can you also comment someplace what a null serDe URI means (seems like it means we skip the authorization checks).
          • nit: can separate the check for serdeURIPrivilegesEnabled from the if statement in getSerdeURI()? For example:

          if (!serdeURIPrivilegesEnabled) return;

          if (...)

          • startsWith(..) - Can you rename this to something more meaningful? Maybe hasPrefixMatch()?
          • Since the checks are at done on the package namespace level, what's stopping someone from bypassing security checks by adding a JAR with the same namespace? For example: org.apache.hadoop.hive.serde2.UserSerDe?
          lskuff Lenni Kuff added a comment - Thanks for updating the patch. A few additional comments: getSerdeURI() - It's strange that this returns void. Should this be called setSerdeURI() ? Can you also comment someplace what a null serDe URI means (seems like it means we skip the authorization checks). nit: can separate the check for serdeURIPrivilegesEnabled from the if statement in getSerdeURI()? For example: if (!serdeURIPrivilegesEnabled) return; if (...) startsWith(..) - Can you rename this to something more meaningful? Maybe hasPrefixMatch()? Since the checks are at done on the package namespace level, what's stopping someone from bypassing security checks by adding a JAR with the same namespace? For example: org.apache.hadoop.hive.serde2.UserSerDe?
          hahao Hao Hao added a comment -

          lskuff Updated the patch based on your comments.

          hahao Hao Hao added a comment - lskuff Updated the patch based on your comments.
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12790813/SENTRY-1087.4.patch against master.

          Overall: -1 due to an error

          ERROR: failed to build with patch (exit code 1)

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

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12790813/SENTRY-1087.4.patch against master. Overall: -1 due to an error ERROR: failed to build with patch (exit code 1) Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/1274/console This message is automatically generated.
          lskuff Lenni Kuff added a comment -

          Looks like you have a PMD failure:

          [ERROR] Failed to execute goal org.apache.maven.plugins:maven-pmd-plugin:3.5:check (validate) on project sentry-binding-hive: You have 1 PMD violation. For more details see: /home/jenkins/jenkins-slave/workspace/PreCommit-SENTRY-Build/sentry-binding/sentry-binding-hive/target/pmd.xml -> [Help 1]
          

          Otherwise, +1 pending test results. Thanks hahao.

          lskuff Lenni Kuff added a comment - Looks like you have a PMD failure: [ERROR] Failed to execute goal org.apache.maven.plugins:maven-pmd-plugin:3.5:check (validate) on project sentry-binding-hive: You have 1 PMD violation. For more details see: /home/jenkins/jenkins-slave/workspace/PreCommit-SENTRY-Build/sentry-binding/sentry-binding-hive/target/pmd.xml -> [Help 1] Otherwise, +1 pending test results. Thanks hahao .
          hahao Hao Hao added a comment -

          lskuff Thanks a lot! I just fixed the PMD violation.

          hahao Hao Hao added a comment - lskuff Thanks a lot! I just fixed the PMD violation.
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12790804/SENTRY-1087.3.patch against master.

          Overall: -1 due to 2 errors

          ERROR: mvn test exited 1
          ERROR: Failed: org.apache.sentry.tests.e2e.metastore.TestMetastoreEndToEnd

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

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12790804/SENTRY-1087.3.patch against master. Overall: -1 due to 2 errors ERROR: mvn test exited 1 ERROR: Failed: org.apache.sentry.tests.e2e.metastore.TestMetastoreEndToEnd Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/1272/console This message is automatically generated.
          hadoopqa Hadoop QA added a comment -

          Here are the results of testing the latest attachment
          https://issues.apache.org/jira/secure/attachment/12790816/SENTRY-1087.4.patch against master.

          Overall: -1 due to 2 errors

          ERROR: mvn test exited 1
          ERROR: Failed: org.apache.sentry.tests.e2e.metastore.TestMetastoreEndToEnd

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

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - Here are the results of testing the latest attachment https://issues.apache.org/jira/secure/attachment/12790816/SENTRY-1087.4.patch against master. Overall: -1 due to 2 errors ERROR: mvn test exited 1 ERROR: Failed: org.apache.sentry.tests.e2e.metastore.TestMetastoreEndToEnd Console output: https://builds.apache.org/job/PreCommit-SENTRY-Build/1275/console This message is automatically generated.

          People

            hahao Hao Hao
            hahao Hao Hao
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: