Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes default version to 2.0.0-alpha1 and fixes CVE-2020-36518 #478

Merged

Conversation

amitgalitz
Copy link
Member

@amitgalitz amitgalitz commented Mar 30, 2022

Description

Changes in this PR:

  1. Updates default version qualifier to alpha1
  2. Fixes more mapping type changes and return type of method based of core changes
  3. updates Jackson databind to 2.13.2.2, jackson core to 2.13.2 and jacson annotations to 2.13.2.
    Jackson databind 2.13.2.2 fixes this issue CVE-2020-36518 (High) detected in jackson-databind-2.12.6.jar #436
  4. Removes JDK 14 support

Issues Resolved

resolves #436
resolves #479

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@amitgalitz amitgalitz requested a review from a team March 30, 2022 18:09
@opensearch-trigger-bot opensearch-trigger-bot bot added backport 1.x infra Changes to infrastructure, testing, CI/CD, pipelines, etc. labels Mar 30, 2022
.github/workflows/CI.yml Outdated Show resolved Hide resolved
@ohltyler
Copy link
Member

Since this PR is editing the workflow, want to remove JDK 14 from test matrix per #479 as well?

@amitgalitz
Copy link
Member Author

Since this PR is editing the workflow, want to remove JDK 14 from test matrix per #479 as well?

Removed :)

ohltyler
ohltyler previously approved these changes Mar 30, 2022
opensearch_version = System.getProperty("opensearch.version", "2.0.0-SNAPSHOT")
// 1.2.0 -> 1.2.0.0, and 1.2.0-SNAPSHOT -> 1.2.0.0-SNAPSHOT
opensearch_build = opensearch_version.replaceAll(/(\.\d)([^\d]*)$/, '$1.0$2')
opensearch_version = System.getProperty("opensearch.version", "2.0.0-alpha1-SNAPSHOT")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Infra team fixed this part, you can refer to this PR https://github.com/opensearch-project/ml-commons/pull/262/files

opensearch_version = System.getProperty("opensearch.version", "2.0.0-SNAPSHOT")
// 1.2.0 -> 1.2.0.0, and 1.2.0-SNAPSHOT -> 1.2.0.0-SNAPSHOT
opensearch_build = opensearch_version.replaceAll(/(\.\d)([^\d]*)$/, '$1.0$2')
opensearch_version = System.getProperty("opensearch.version", "2.0.0-alpha1-SNAPSHOT")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Infra team fixed this part, you can refer to this PR https://github.com/opensearch-project/ml-commons/pull/262/files

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed it to be the same as ml-commons

Copy link
Collaborator

@ylwu-amzn ylwu-amzn Mar 30, 2022

Choose a reason for hiding this comment

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

Thanks, btw, make sure the built AD artifact name is opensearch-anomaly-detection-2.0.0.0-alpha1-SNAPSHOT. I see line 633

version = "${project.version}" - "-SNAPSHOT"

Not sure what project.version is

Copy link
Member Author

@amitgalitz amitgalitz Mar 31, 2022

Choose a reason for hiding this comment

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

It actually gives a zip of opensearch-anomaly-detection-2.0.0.0-SNAPSHOT. Same case for ml-commons built zip (just tried on latest code) -> will investigate this, changing line 633 seems to have no effect

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh,I guess infra team's fix break this part for ml-commons, will take a look

Copy link
Collaborator

@ylwu-amzn ylwu-amzn Mar 31, 2022

Choose a reason for hiding this comment

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

For ml-commons, this one can generate artifact with alpha1, ./gradlew clean;./gradlew build -Dbuild.version_qualifier=alpha1. But artifact generated by ./gradlew build has no alpha1 in name even we set opensearch_version = System.getProperty("opensearch.version", "2.0.0-alpha1-SNAPSHOT"). @peterzhuamazon is this what we expected?

Copy link
Collaborator

Choose a reason for hiding this comment

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

From @peterzhuamazon , we are going to move to rc1 soon. So just keep this code, we can change to rc1 later.

ohltyler
ohltyler previously approved these changes Mar 30, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #478 (6cb6d3d) into main (407e061) will decrease coverage by 0.19%.
The diff coverage is 84.61%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #478      +/-   ##
============================================
- Coverage     78.14%   77.95%   -0.20%     
+ Complexity     4159     4147      -12     
============================================
  Files           296      296              
  Lines         17659    17652       -7     
  Branches       1879     1877       -2     
============================================
- Hits          13800    13760      -40     
- Misses         2964     2999      +35     
+ Partials        895      893       -2     
Flag Coverage Δ
plugin 77.95% <84.61%> (-0.20%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../handler/AbstractAnomalyDetectorActionHandler.java 61.63% <81.81%> (-0.55%) ⬇️
...opensearch/ad/indices/AnomalyDetectionIndices.java 72.12% <100.00%> (-0.06%) ⬇️
.../java/org/opensearch/ad/AnomalyDetectorRunner.java 37.64% <0.00%> (-5.89%) ⬇️
...c/main/java/org/opensearch/ad/util/ParseUtils.java 73.83% <0.00%> (-3.95%) ⬇️
...va/org/opensearch/ad/feature/SearchFeatureDao.java 82.78% <0.00%> (-3.86%) ⬇️
...ain/java/org/opensearch/ad/model/ModelProfile.java 70.90% <0.00%> (-1.82%) ⬇️
...rch/ad/transport/AnomalyResultTransportAction.java 80.13% <0.00%> (-0.69%) ⬇️

ylwu-amzn
ylwu-amzn previously approved these changes Mar 31, 2022
@amitgalitz amitgalitz merged commit 0ebb431 into opensearch-project:main Mar 31, 2022
@aksingh-es aksingh-es removed the infra Changes to infrastructure, testing, CI/CD, pipelines, etc. label Apr 21, 2022
Sign up for free to 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.

Drop support for JDK 14. CVE-2020-36518 (High) detected in jackson-databind-2.12.6.jar
5 participants