Skip to content

Conversation

@smengcl
Copy link
Contributor

@smengcl smengcl commented Mar 29, 2023

What changes were proposed in this pull request?

It turns out excluding jackson in HDDS-7924 might have broken actual ranger-intg client usage:

E           23/03/28 20:05:54 INFO rpc.RpcClient: Creating Tenant: 'tenant303241680030324', with new volume: 'tenant303241680030324'
E           	at org.apache.ranger.plugin.util.RangerRESTClient.buildClient(RangerRESTClient.java:232)
E           	at org.apache.ranger.plugin.util.RangerRESTClient.getClient(RangerRESTClient.java:215)
E           	at org.apache.ranger.plugin.util.RangerRESTClient.post(RangerRESTClient.java:576)
E           	at org.apache.ranger.RangerClient.invokeREST(RangerClient.java:422)
E           	at org.apache.ranger.RangerClient.lambda$responseHandler$0(RangerClient.java:464)
E           	at org.apache.ranger.RangerClient.responseHandler(RangerClient.java:462)
E           	at org.apache.ranger.RangerClient.callAPI(RangerClient.java:527)
E           	at org.apache.ranger.RangerClient.createRole(RangerClient.java:334)
E           Caused by: java.lang.ClassNotFoundException: org.codehaus.jackson.jaxrs.JacksonJsonProvider
E           	... 26 more

Unfortunately our acceptance tests didn't cover the actual Ranger client testing because the infra is not there yet. iirc @djordje-mijatovic is trying to improve the docker dev env to spin up actual Ranger, at which point we could write new acceptance tests that uses ranger-intg client to run against actual Ranger Admin Server so that we can avoid such issues in the future.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-8320

How was this patch tested?

  • Pending internal system tests to pass.
  • New acceptance tests to be added after Ranger docker dev env is ready. Filed HDDS-8321 for it.

Change-Id: Ibf720025c5eaf0bec781c59b9b0cedcbb0289fb6
@smengcl smengcl added the bug Something isn't working label Mar 29, 2023
smengcl added 2 commits March 29, 2023 16:51
…vergence enforcer check.

Change-Id: Ie67563da617f3a0dc6e4cb3659cc8718838727ae
@smengcl smengcl marked this pull request as draft March 30, 2023 01:24
@smengcl
Copy link
Contributor Author

smengcl commented Mar 30, 2023

Marking this as draft as @swamirishi is still tuning the Maven POM.

Will add LICENSE later.

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @smengcl for the patch and sorry for the trouble caused by the exclusion.

Comment on lines +161 to +165
<dependency>
<groupId>org.codehaus.jackson</groupId>
<artifactId>jackson-core-asl</artifactId>
<version>${jackson1.version}</version>
</dependency>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what Ranger brings without the exclusion:

[INFO] |     +- org.codehaus.jackson:jackson-jaxrs:jar:1.9.13:compile
[INFO] |     |  +- org.codehaus.jackson:jackson-core-asl:jar:1.9.13:compile
[INFO] |     |  \- org.codehaus.jackson:jackson-mapper-asl:jar:1.9.13:compile

If we want to add explicit dependency, I think we should add jackson-jaxrs, not jackson-core-asl.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @adoroszlai for looking at this. @swamirishi has added jaxrs in #4504 .

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jaxrs doesn't have the internal build version. Ozone depends on specific internal build jackson-core-asl & jackson-mapper-asl. In order to fix this we need to exclude jackson-core-asl & jackson-mapper-asl from jackson-jaxrs & include the specified version to fix the conflict.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check #4504 for the fix.

@smengcl
Copy link
Contributor Author

smengcl commented Mar 30, 2023

Closing this one in favor of #4504 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants