Skip to content

TEZ-4435: use jackson v2 - jackson v1 is EOL and full of security issues - #231

Merged
abstractdog merged 5 commits into
apache:masterfrom
pjfanning:patch-1
Aug 2, 2022
Merged

TEZ-4435: use jackson v2 - jackson v1 is EOL and full of security issues#231
abstractdog merged 5 commits into
apache:masterfrom
pjfanning:patch-1

Conversation

@pjfanning

@pjfanningpjfanning commented Jul 19, 2022

Copy link
Copy Markdown
Member

@pjfanningpjfanning changed the title use jackson v2 - jackson v1 is EOL and full of security issuesTEZ-4435: use jackson v2 - jackson v1 is EOL and full of security issuesJul 19, 2022
@tez-yetus

This comment was marked as outdated.

@tez-yetus

This comment was marked as outdated.

@ayushtknayushtkn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems similar stuff was done in Hadoop as well.
We should restrict these imports as well so as to avoid future usage by others.
Something like this
https://github.com/apache/hadoop/pull/3789/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R276-R283

@pjfanning

Copy link
Copy Markdown
MemberAuthor

@ayushtkn I've never used that plugin before. I checked its docs and the docs don't match what Hadoop has. In https://github.com/skuzzle/restrict-imports-enforcer-rule - the XML element is called 'RestrictImports' but Hadoop pom.xml has 'restrictImports'.

I tried both on my Tez checkout and so far, the rule is not enforced either way.

@pjfanning

pjfanning commented Jul 21, 2022

Copy link
Copy Markdown
MemberAuthor

@pjfanning

Copy link
Copy Markdown
MemberAuthor

@ayushtkn I think I have the enforcement rule working now - the inherited=false flag seems to have stopped the rule being enforced in sub-modules. The 'restrictImports' XML tag seems to work despite the docs in https://github.com/skuzzle/restrict-imports-enforcer-rule

@tez-yetus

This comment was marked as outdated.

@tez-yetus

This comment was marked as outdated.

@tez-yetus

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 17sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚@author0m 0sThe patch does not contain any @author tags.
-1 ❌test4tests0m 0sThe patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+0 🆗mvndep6m 30sMaven dependency ordering for branch
+1 💚mvninstall9m 45smaster passed
+1 💚compile3m 18smaster passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚compile3m 10smaster passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚checkstyle1m 51smaster passed
+1 💚javadoc3m 18smaster passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚javadoc2m 40smaster passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+0 🆗spotbugs6m 35sUsed deprecated FindBugs config; considering switching to SpotBugs.
+1 💚findbugs7m 45smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 19sMaven dependency ordering for patch
+1 💚mvninstall4m 44sthe patch passed
+1 💚compile3m 16sthe patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚javac3m 16sthe patch passed
+1 💚compile3m 4sthe patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚javac3m 4sthe patch passed
+1 💚checkstyle1m 37sthe patch passed
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚xml0m 1sThe patch has no ill-formed XML file.
+1 💚javadoc3m 5sthe patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚javadoc2m 38sthe patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚findbugs7m 29sthe patch passed
_ Other Tests _
+1 💚unit0m 57stez-protobuf-history-plugin in the patch passed.
+1 💚unit72m 50sroot in the patch passed.
+1 💚asflicense1m 41sThe patch does not generate ASF License warnings.
143m 15s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-231/5/artifact/out/Dockerfile
GITHUB PR#231
JIRA IssueTEZ-4435
Optional Testsdupname asflicense javac javadoc unit xml compile spotbugs findbugs checkstyle
unameLinux 6968e11d1add 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitypersonality/tez.sh
git revisionmaster / a192ec4
Default JavaPrivate Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions/usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Test Resultshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-231/5/testReport/
Max. process+thread count1826 (vs. ulimit of 5500)
modulesC: tez-plugins/tez-protobuf-history-plugin . U: .
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-231/5/console
versionsgit=2.25.1 maven=3.6.3 findbugs=3.0.1
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@ayushtknayushtkn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM
@abstractdog can you help push this further

@pjfanning

Copy link
Copy Markdown
MemberAuthor

@abstractdog@ayushtkn would it be possible to get this merged?

@abstractdog
abstractdog self-requested a review August 2, 2022 08:18

@abstractdogabstractdog left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM +1
checked locally and enforcer rule works

@abstractdog
abstractdog merged commit 621a831 into apache:masterAug 2, 2022
@pjfanning
pjfanning deleted the patch-1 branch August 2, 2022 08:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@pjfanning@tez-yetus@abstractdog@ayushtkn