You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "David Knupp (Code Review)" <ge...@cloudera.org> on 2020/04/24 06:51:19 UTC

[Impala-ASF-CR] IMPALA-9649: Add shiro* to the banned dependency maven plugin

David Knupp has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15796


Change subject: IMPALA-9649: Add shiro* to the banned dependency maven plugin
......................................................................

IMPALA-9649: Add shiro* to the banned dependency maven plugin

The earlier attempt to excludethe shiro-core and shiro-crypto-cipher jars
from fe/pom.xml failed to find all instances, and security scans picked
them up again. This patch also moves the shiro exclusions from the sentry
node to the dependencyManagement node, and excludes the jar from the
following:

- sentry-core-common
- sentry-provider-db
- sentry-provider-file
- sentry-provider-cache

Furthermore, to avoid compilation errors related to the absense of shiro, it
was necessary to remove delete the TestSentryResourceAuthorizationProvider
class, and any tests that referenced it. Since Sentry is not being used any
longer, this shouldn't be an issue.

Tested by running build, which didn't fail from banned dependency plugin,
as well running the standard set of tests on jenkins.impala.io.

Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
---
M fe/pom.xml
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java
D fe/src/test/java/org/apache/impala/testutil/TestSentryResourceAuthorizationProvider.java
3 files changed, 62 insertions(+), 163 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/15796/2
-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 2
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 7:

> Patch Set 7:
> 
> Running a gerrit-verify-dryrun-external on this. Assuming that passes, I think this makes sense. We are planning to get rid of Sentry, so it is ok to remove some Sentry specific tests.

The dryrun you started failed for some Jenkins/infra issue:

     an exception which occurred:
	in field com.cloudbees.groovy.cps.impl.ConstantBlock.value
	in object com.cloudbees.groovy.cps.impl.ConstantBlock@19d90a01
	in field com.cloudbees.groovy.cps.impl.CollectionLiteralBlock.argExps
	in object com.cloudbees.groovy.cps.impl.MapBlock@30f60e7
	in field com.cloudbees.groovy.cps.impl.FunctionCallBlock.argExps
	in object com.cloudbees.groovy.cps.impl.FunctionCallBlock@2d57145e
	in field com.cloudbees.groovy.cps.impl.CollectionLiteralBlock.argExps
	in object com.cloudbees.groovy.cps.impl.ListBlock@371b11c5

Note that I did my own dryrun earlier, which had passed.
https://jenkins.impala.io/job/gerrit-code-review-checks/5906/


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 7
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Apr 2020 16:40:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 7:

> Patch Set 7:
> 
> (1 comment)

Ah, I had no idea there were interdependencies here. With TestSentryResourceAuthorizationProvider being removed, I think it makes sense to remove these as well.


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 7
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 May 2020 18:48:58 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 9:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5733/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 9
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 May 2020 19:46:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Add shiro* to the banned dependency maven plugin

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Joe McDonnell, Dinesh Garg, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/15796

to look at the new patch set (#5).

Change subject: IMPALA-9649: Add shiro* to the banned dependency maven plugin
......................................................................

IMPALA-9649: Add shiro* to the banned dependency maven plugin

The earlier attempt to exclude the shiro-core and shiro-crypto-cipher jars
from fe/pom.xml failed to find all instances, and security scans picked
them up again. This patch also moves the shiro exclusions from the sentry
node to the dependencyManagement node, and excludes the jar from the
following:

- sentry-core-common
- sentry-provider-db
- sentry-provider-file
- sentry-provider-cache

Furthermore, to avoid compilation errors related to the absense of shiro, it
was necessary to remove the TestSentryResourceAuthorizationProvider class,
and any tests that referenced it. Since Sentry is not being used any longer,
this shouldn't be an issue.

Tested by running build, which didn't fail from banned dependency plugin,
as well running the standard set of tests on jenkins.impala.io.

Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
---
M fe/pom.xml
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java
D fe/src/test/java/org/apache/impala/testutil/TestSentryResourceAuthorizationProvider.java
3 files changed, 61 insertions(+), 163 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/15796/5
-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 5
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15796/6/fe/pom.xml
File fe/pom.xml:

http://gerrit.cloudera.org:8080/#/c/15796/6/fe/pom.xml@1418
PS6, Line 1418: 
> Sorry -- not sure what happened here. Fixing this in the next patch.
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 7
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Apr 2020 17:44:36 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 9:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5734/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 9
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 May 2020 20:59:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Add shiro* to the banned dependency maven plugin

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Add shiro* to the banned dependency maven plugin
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5890/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 4
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Apr 2020 22:50:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Add shiro* to the banned dependency maven plugin

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Add shiro* to the banned dependency maven plugin
......................................................................


Patch Set 4:

Patch set #4 is just a rebase.


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 4
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Apr 2020 22:05:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5730/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 7
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Apr 2020 21:51:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 6: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 6
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Apr 2020 13:21:09 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5912/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 7
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Apr 2020 18:29:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Add shiro* to the banned dependency maven plugin

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Add shiro* to the banned dependency maven plugin
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5865/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 2
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 24 Apr 2020 07:32:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................

IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

The earlier attempt to exclude the shiro-core and shiro-crypto-cipher jars
from fe/pom.xml failed to find all instances, and security scans picked
them up again. This patch also excludes the jar from the following:

- sentry-core-common
- sentry-provider-cache
- sentry-provider-db
- sentry-provider-file

Furthermore, to avoid compilation errors related to the absense of shiro, it
was necessary to remove the TestSentryResourceAuthorizationProvider class,
and any tests that referenced it. Since Sentry is not being used any longer,
this shouldn't be an issue.

Tested by running build, which didn't fail from banned dependency plugin,
as well running the standard set of tests on jenkins.impala.io.

Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Reviewed-on: http://gerrit.cloudera.org:8080/15796
Reviewed-by: Joe McDonnell <jo...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/pom.xml
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java
D fe/src/test/java/org/apache/impala/testutil/TestSentryResourceAuthorizationProvider.java
M tests/authorization/test_authorization.py
M tests/authorization/test_grant_revoke.py
D tests/authorization/test_owner_privileges.py
M tests/authorization/test_sentry.py
D tests/authorization/test_show_grant.py
8 files changed, 43 insertions(+), 1,004 deletions(-)

Approvals:
  Joe McDonnell: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 10
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 7: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 7
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 May 2020 03:08:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 9: Code-Review+2

Thanks for working on this


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 9
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 May 2020 20:58:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 9: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 9
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 May 2020 01:04:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15796/6/fe/pom.xml
File fe/pom.xml:

http://gerrit.cloudera.org:8080/#/c/15796/6/fe/pom.xml@1418
PS6, Line 1418: 
Sorry -- not sure what happened here. Fixing this in the next patch.



-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 6
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Apr 2020 17:42:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5906/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 6
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Apr 2020 08:50:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Joe McDonnell, Dinesh Garg, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/15796

to look at the new patch set (#8).

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................

IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

The earlier attempt to exclude the shiro-core and shiro-crypto-cipher jars
from fe/pom.xml failed to find all instances, and security scans picked
them up again. This patch also excludes the jar from the following:

- sentry-core-common
- sentry-provider-cache
- sentry-provider-db
- sentry-provider-file

Furthermore, to avoid compilation errors related to the absense of shiro, it
was necessary to remove the TestSentryResourceAuthorizationProvider class,
and any tests that referenced it. Since Sentry is not being used any longer,
this shouldn't be an issue.

Tested by running build, which didn't fail from banned dependency plugin,
as well running the standard set of tests on jenkins.impala.io.

Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
---
M fe/pom.xml
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java
D fe/src/test/java/org/apache/impala/testutil/TestSentryResourceAuthorizationProvider.java
M tests/authorization/test_authorization.py
M tests/authorization/test_grant_revoke.py
D tests/authorization/test_owner_privileges.py
M tests/authorization/test_sentry.py
D tests/authorization/test_show_grant.py
8 files changed, 43 insertions(+), 1,004 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/15796/8
-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 8
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5728/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 7
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Apr 2020 03:01:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5721/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 6
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Apr 2020 08:07:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 8:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5926/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 8
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 May 2020 20:30:09 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Add shiro* to the banned dependency maven plugin

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Joe McDonnell, Dinesh Garg, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/15796

to look at the new patch set (#4).

Change subject: IMPALA-9649: Add shiro* to the banned dependency maven plugin
......................................................................

IMPALA-9649: Add shiro* to the banned dependency maven plugin

The earlier attempt to exclude the shiro-core and shiro-crypto-cipher jars
from fe/pom.xml failed to find all instances, and security scans picked
them up again. This patch also moves the shiro exclusions from the sentry
node to the dependencyManagement node, and excludes the jar from the
following:

- sentry-core-common
- sentry-provider-db
- sentry-provider-file
- sentry-provider-cache

Furthermore, to avoid compilation errors related to the absense of shiro, it
was necessary to remove delete the TestSentryResourceAuthorizationProvider
class, and any tests that referenced it. Since Sentry is not being used any
longer, this shouldn't be an issue.

Tested by running build, which didn't fail from banned dependency plugin,
as well running the standard set of tests on jenkins.impala.io.

Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
---
M fe/pom.xml
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java
D fe/src/test/java/org/apache/impala/testutil/TestSentryResourceAuthorizationProvider.java
3 files changed, 61 insertions(+), 163 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/15796/4
-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 4
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15796/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15796/7//COMMIT_MSG@18
PS7, Line 18: Furthermore, to avoid compilation errors related to the absense of shiro, it
            : was necessary to remove the TestSentryResourceAuthorizationProvider class,
            : and any tests that referenced it.
There are some end-to-end tests that reference TestSentryResourceAuthorizationProvider:
authorization/test_authorization.py
authorization/test_grant_revoke.py
authorization/test_owner_privileges.py
authorization/test_sentry.py
authorization/test_show_grant.py

They are disabled, because we have Sentry tests disabled. Sentry support is already broken. I'm thinking about whether we should do anything about them in this change.



-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 7
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 May 2020 18:43:21 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15796/4/fe/pom.xml
File fe/pom.xml:

http://gerrit.cloudera.org:8080/#/c/15796/4/fe/pom.xml@1406
PS4, Line 1406:      Pin org.glassfish:javax.el explicitly.
              : 
              :       HBase depends on this indirectly, and it's configured with
              :       a range of versions. This causes Maven to talk to all configured
              :       repositories, leading both to a lot of chattiness, and also
              :       failures if one of the repositories is unavailable.
              :       -->
              :       <dependency>
              :         <groupId>org.glassfish</groupId>
              :         <artifactId>javax.el</artifactId>
              :         <version>3.0.1-b08</version>
              :       </dependency>
              : 
              :       <!-- IMPALA-9
> I have sympathy for trying to find a way to get rid of these dependencies. 
Done


http://gerrit.cloudera.org:8080/#/c/15796/4/fe/pom.xml@1422
PS4, Line 1422: hadoop-hdfs</artif
> This came from the approach I outlined above. I'm happy to try an alternate
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 6
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Apr 2020 08:06:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9649: Add shiro* to the banned dependency maven plugin

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Add shiro* to the banned dependency maven plugin
......................................................................


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/15796/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15796/4//COMMIT_MSG@21
PS4, Line 21: remove delete
Fixup redundant language


http://gerrit.cloudera.org:8080/#/c/15796/4/fe/pom.xml
File fe/pom.xml:

http://gerrit.cloudera.org:8080/#/c/15796/4/fe/pom.xml@1406
PS4, Line 1406:      <dependency>
              :         <groupId>org.apache.sentry</groupId>
              :         <artifactId>sentry-core-common</artifactId>
              :         <exclusions>
              :           <exclusion>
              :             <groupId>org.apache.shiro</groupId>
              :             <artifactId>shiro-crypto-cipher</artifactId>
              :           </exclusion>
              :           <exclusion>
              :             <groupId>org.apache.shiro</groupId>
              :             <artifactId>shiro-core</artifactId>
              :           </exclusion>
              :         </exclusions>
              :       </dependency>
I haven't use the dependencyManagement section. What is the significance of this being in the "dependencyManagement" section rather than a normal "dependencies" section? When would you pick one over the other?


http://gerrit.cloudera.org:8080/#/c/15796/4/fe/pom.xml@1422
PS4, Line 1422: sentry-provider-db
This artifact is listed here as well as above in the dependencies section, and it has other exclusions there. How does that work?



-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 4
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Apr 2020 23:19:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Joe McDonnell, Dinesh Garg, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/15796

to look at the new patch set (#7).

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................

IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

The earlier attempt to exclude the shiro-core and shiro-crypto-cipher jars
from fe/pom.xml failed to find all instances, and security scans picked
them up again. This patch also excludes the jar from the following:

- sentry-core-common
- sentry-provider-cache
- sentry-provider-db
- sentry-provider-file

Furthermore, to avoid compilation errors related to the absense of shiro, it
was necessary to remove the TestSentryResourceAuthorizationProvider class,
and any tests that referenced it. Since Sentry is not being used any longer,
this shouldn't be an issue.

Tested by running build, which didn't fail from banned dependency plugin,
as well running the standard set of tests on jenkins.impala.io.

Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
---
M fe/pom.xml
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java
D fe/src/test/java/org/apache/impala/testutil/TestSentryResourceAuthorizationProvider.java
3 files changed, 43 insertions(+), 154 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/15796/7
-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 7
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................


Patch Set 7:

Running a gerrit-verify-dryrun-external on this. Assuming that passes, I think this makes sense. We are planning to get rid of Sentry, so it is ok to remove some Sentry specific tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 7
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Apr 2020 03:01:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9649: Add shiro* to the banned dependency maven plugin

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Joe McDonnell, Dinesh Garg, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/15796

to look at the new patch set (#3).

Change subject: IMPALA-9649: Add shiro* to the banned dependency maven plugin
......................................................................

IMPALA-9649: Add shiro* to the banned dependency maven plugin

The earlier attempt to exclude the shiro-core and shiro-crypto-cipher jars
from fe/pom.xml failed to find all instances, and security scans picked
them up again. This patch also moves the shiro exclusions from the sentry
node to the dependencyManagement node, and excludes the jar from the
following:

- sentry-core-common
- sentry-provider-db
- sentry-provider-file
- sentry-provider-cache

Furthermore, to avoid compilation errors related to the absense of shiro, it
was necessary to remove delete the TestSentryResourceAuthorizationProvider
class, and any tests that referenced it. Since Sentry is not being used any
longer, this shouldn't be an issue.

Tested by running build, which didn't fail from banned dependency plugin,
as well running the standard set of tests on jenkins.impala.io.

Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
---
M fe/pom.xml
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java
D fe/src/test/java/org/apache/impala/testutil/TestSentryResourceAuthorizationProvider.java
3 files changed, 62 insertions(+), 163 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/15796/3
-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 3
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-9649: Add shiro* to the banned dependency maven plugin

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Add shiro* to the banned dependency maven plugin
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15796/4/fe/pom.xml
File fe/pom.xml:

http://gerrit.cloudera.org:8080/#/c/15796/4/fe/pom.xml@1406
PS4, Line 1406:      <dependency>
              :         <groupId>org.apache.sentry</groupId>
              :         <artifactId>sentry-core-common</artifactId>
              :         <exclusions>
              :           <exclusion>
              :             <groupId>org.apache.shiro</groupId>
              :             <artifactId>shiro-crypto-cipher</artifactId>
              :           </exclusion>
              :           <exclusion>
              :             <groupId>org.apache.shiro</groupId>
              :             <artifactId>shiro-core</artifactId>
              :           </exclusion>
              :         </exclusions>
              :       </dependency>
> So, with this being my first foray into doing anything with maven, I'll con
I have sympathy for trying to find a way to get rid of these dependencies. It's whack-a-mole.

Exclusions go on top level artifacts listed in the pom (the root of the tree) and apply to the entire tree starting with that artifact. They don't apply to the tree for any other top level artifact, so the exclusion needs to go on every top level artifact that has the banned artifact in its tree. There is no need to add exclusions for artifacts that are not listed in our pom. Long story short, there are only so many moles to whack.

Our pom.xml has multiple profiles (one for Hive 2, one for Hive 3), so it is important to add the exclusion for the right profile. (This doesn't apply to the shiro exclusion, but it does apply to netty.)

This is not making use of any special functionality of dependencyManagement. These are regular exclusions that work just as well in the ordinary location. Move these exclusions up to the regular exclusion section for each of these artifacts.



-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 4
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Apr 2020 02:11:45 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Joe McDonnell, Dinesh Garg, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/15796

to look at the new patch set (#6).

Change subject: IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin
......................................................................

IMPALA-9649: Exclude shiro* and add to banned dependency maven plugin

The earlier attempt to exclude the shiro-core and shiro-crypto-cipher jars
from fe/pom.xml failed to find all instances, and security scans picked
them up again. This patch also excludes the jar from the following:

- sentry-core-common
- sentry-provider-cache
- sentry-provider-db
- sentry-provider-file

Furthermore, to avoid compilation errors related to the absense of shiro, it
was necessary to remove the TestSentryResourceAuthorizationProvider class,
and any tests that referenced it. Since Sentry is not being used any longer,
this shouldn't be an issue.

Tested by running build, which didn't fail from banned dependency plugin,
as well running the standard set of tests on jenkins.impala.io.

Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
---
M fe/pom.xml
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java
D fe/src/test/java/org/apache/impala/testutil/TestSentryResourceAuthorizationProvider.java
3 files changed, 76 insertions(+), 154 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/15796/6
-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 6
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-9649: Add shiro* to the banned dependency maven plugin

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/15796 )

Change subject: IMPALA-9649: Add shiro* to the banned dependency maven plugin
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/15796/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15796/4//COMMIT_MSG@21
PS4, Line 21: remove the Te
> Fixup redundant language
Done


http://gerrit.cloudera.org:8080/#/c/15796/4/fe/pom.xml
File fe/pom.xml:

http://gerrit.cloudera.org:8080/#/c/15796/4/fe/pom.xml@1406
PS4, Line 1406:      <dependency>
              :         <groupId>org.apache.sentry</groupId>
              :         <artifactId>sentry-core-common</artifactId>
              :         <exclusions>
              :           <exclusion>
              :             <groupId>org.apache.shiro</groupId>
              :             <artifactId>shiro-crypto-cipher</artifactId>
              :           </exclusion>
              :           <exclusion>
              :             <groupId>org.apache.shiro</groupId>
              :             <artifactId>shiro-core</artifactId>
              :           </exclusion>
              :         </exclusions>
              :       </dependency>
> I haven't use the dependencyManagement section. What is the significance of
So, with this being my first foray into doing anything with maven, I'll confess that some of this results from me just googling and hacking, trying to figure out what works and what doesn't. So I will explain how I wound up arriving at this solution, but it's quite possible that this is not the best/approved way to do this.

This all actually came from working on excluding netty/netty-all (IMPALA-9648). In my first attempt at excluding netty-all, I added an exclusion in the dependency for hadoop-hdfs, thinking that was all I'd need to do. But after building with that change, I found that new instances of netty/netty-all were popping up in the dependency tree -- namely, under zookeeper and hadoop-distcp, which were themselves nested under hadoop-hdfs. When I went to add exclusions for netty-all in those spots as well (per a suggeston from Tim Armstrong that I'd probably need multiple netty-all exclusions), it turned out zookeeper and hadoop-distcp weren't even explicitly listed in our fe/pom.xml. I tried adding them as explicit dependencies under hadoop-hdfs, just so I could then exclude netty-all from them, but I was having trouble simply getting the build to succeed.

This is explained in the comments in https://issues.apache.org/jira/browse/IMPALA-9648.

I don't recall exactly where I saw the reference to dependencyManagement, but it was immediately helpful. I just kept rerunning the build and seeing where netty-all would pop up each time, then I'd add then that specific exclusion to dependencyManagement. After a couple iterations, netty-all was gone, and the plugin for banning dependencies allowed the build to pass.

So given that experience, when it seemed that excluding shiro-* had a similar pattern (each time I tried to exclude it, it would just show up somewhere else in the dependency tree the next time I built), I just applied the same approach.

For what it's worth, here are two articles that might be a little more clear:

https://stackoverflow.com/questions/2619598/differences-between-dependencymanagement-and-dependencies-in-maven
https://bhaskarvk.github.io/2007/06/maven-best-practices-use-dependency-management-for-multi-module-projects./


http://gerrit.cloudera.org:8080/#/c/15796/4/fe/pom.xml@1422
PS4, Line 1422: sentry-provider-db
> This artifact is listed here as well as above in the dependencies section, 
This came from the approach I outlined above. I'm happy to try an alternate approach, if you think it would be preferred.



-- 
To view, visit http://gerrit.cloudera.org:8080/15796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9994bf81c1d2e025a03925e8eccb147c34d66e
Gerrit-Change-Number: 15796
Gerrit-PatchSet: 5
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 29 Apr 2020 00:53:01 +0000
Gerrit-HasComments: Yes