Skip to content
This repository has been archived by the owner on Sep 18, 2023. It is now read-only.

[NSE-687]Try to upgrade log4j #691

Merged
merged 9 commits into from
Jan 20, 2022
Merged

[NSE-687]Try to upgrade log4j #691

merged 9 commits into from
Jan 20, 2022

Conversation

PHILO-HE
Copy link
Collaborator

No description provided.

@github-actions
Copy link

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/oap-project/native-sql-engine/issues

Then could you also rename commit message and pull request title in the following format?

[NSE-${ISSUES_ID}] ${detailed message}

See also:

@zhouyuan
Copy link
Collaborator

@weiting-chen

@weiting-chen
Copy link
Collaborator

@PHILO-HE
I did this before, the compile can pass with -DskipTests
But it cannot pass lots of Spark's unit tests which use some log4j 1.2 classes.
Check the detailed testing resulting in GitHub action.

@weiting-chen
Copy link
Collaborator

Let's address this issue to Gazelle v1.4 release.
Btw, please make sure to use log4j 2.17.1.
log4j 2.17.0 still has the security mitigation issue.

@weiting-chen weiting-chen self-requested a review January 11, 2022 08:01
@PHILO-HE
Copy link
Collaborator Author

Hi @weiting-chen, thanks for your reminding.
This drafted patch looks workable. I will refine it and bump to 2.17.1 according to your suggestion.

mvn clean install -N
cd arrow-data-source
mvn clean install -DskipTests -Dbuild_arrow=OFF
cd ..
mvn clean package -P full-scala-compiler -Phadoop-2.7.4 -am -pl native-sql-engine/core -DskipTests -Dbuild_arrow=OFF
Copy link
Collaborator

Choose a reason for hiding this comment

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

-Phadoop-2.7.4 is required since the default Hadoop version has been changed to 3.2.0.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just reverted the changes. Thanks!

<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-1.2-api</artifactId>
<version>2.17.0</version>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change version to 2.17.1

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just changed the version to 2.17.1. Thanks!

@PHILO-HE
Copy link
Collaborator Author

@weiting-chen, please have a validation on Jenkins uint tests. You mentioned there is a change in Jenkins script that needs to be reset for upgrade test.

@weiting-chen
Copy link
Collaborator

@weiting-chen, please have a validation on Jenkins uint tests. You mentioned there is a change in Jenkins script that needs to be reset for upgrade test.

Done to update Jenkins script, please help to submit an unit test request in Jenkins to verify if the PR can also work in Jenkins.

@zhouyuan zhouyuan changed the title Try to upgrade log4j [NSE-687]Try to upgrade log4j Jan 20, 2022
@github-actions
Copy link

#687

@zhouyuan zhouyuan merged commit 5759c78 into oap-project:master Jan 20, 2022
@zhouyuan
Copy link
Collaborator

@weiting-chen will merge this first to get jenkins running

@weiting-chen weiting-chen added the bug Something isn't working label Apr 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants