Skip to content

Partial Cherry-pick of #886 and Additional Adjustments#914

Merged
amitgalitz merged 1 commit intoopensearch-project:2.xfrom
kaituo:2.8_fix_build2
May 26, 2023
Merged

Partial Cherry-pick of #886 and Additional Adjustments#914
amitgalitz merged 1 commit intoopensearch-project:2.xfrom
kaituo:2.8_fix_build2

Conversation

@kaituo
Copy link
Collaborator

@kaituo kaituo commented May 26, 2023

Description

This commit includes a partial cherry-pick of #886 along with other necessary changes for compatibility with the 2.8 branch.

The cherry-picked changes from the anomaly-detection repository's main branch are not identical due to differences between the main and 2.8 branches. Specifically, the changes in opensearch-project/OpenSearch#7165 present in the main branch are not incorporated in 2.8. To reconcile this, modifications related to ImmutableMap and ImmutableOpenMap were undone.

Additionally, only partial changes from opensearch-project/OpenSearch@1e08b5a were applied to 2.8. To address compiler failures, OpenSearchRejectedExecutionException was relocated from the org.opensearch.common.util.concurrent package to the org.opensearch.core.concurrency package.

I also incorporate selected changes from PR #907 to enhance security in the 2.x branch. This update transitions from simpler passwords to a more secure, unguessable password system. The motivation behind this change is to address a potential security risk identified with simpler passwords (details at https://tinyurl.com/383em9zk). Without these updates, security tests would fail due to the vulnerability associated with simple passwords.

Tests completed:

  • Successful Gradle build

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.

jackiehanyang
jackiehanyang previously approved these changes May 26, 2023
@codecov
Copy link

codecov bot commented May 26, 2023

Codecov Report

Merging #914 (234e217) into 2.x (4766c7f) will decrease coverage by 0.03%.
The diff coverage is 60.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x     #914      +/-   ##
============================================
- Coverage     79.39%   79.36%   -0.03%     
+ Complexity     4315     4312       -3     
============================================
  Files           307      307              
  Lines         18144    18147       +3     
  Branches       1909     1909              
============================================
- Hits          14406    14403       -3     
- Misses         2819     2823       +4     
- Partials        919      921       +2     
Flag Coverage Δ
plugin 79.36% <60.00%> (-0.03%) ⬇️

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

Impacted Files Coverage Δ
...ensearch/ad/transport/handler/ADSearchHandler.java 95.83% <0.00%> (-4.17%) ⬇️
...arch/ad/transport/handler/AnomalyIndexHandler.java 77.02% <ø> (ø)
...ain/java/org/opensearch/ad/util/ExceptionUtil.java 70.45% <ø> (ø)
...c/main/java/org/opensearch/ad/util/ParseUtils.java 78.09% <75.00%> (+0.23%) ⬆️

... and 5 files with indirect coverage changes

…and Additional Adjustments

This commit includes a partial cherry-pick of opensearch-project#886 along with other necessary changes for compatibility with the 2.8 branch.

The cherry-picked changes from the anomaly-detection repository's main branch are not identical due to differences between the main and 2.8 branches. Specifically, the changes in opensearch-project/OpenSearch#7165 present in the main branch are not incorporated in 2.8. To reconcile this, modifications related to ImmutableMap and ImmutableOpenMap were undone.

Additionally, only partial changes from opensearch-project/OpenSearch@1e08b5a were applied to 2.8. To address compiler failures, OpenSearchRejectedExecutionException was relocated from the org.opensearch.common.util.concurrent package to the org.opensearch.core.concurrency package.

Tests completed:
- Successful Gradle build

Signed-off-by: Kaituo Li <kaituo@amazon.com>
implementation "org.opensearch.client:opensearch-rest-client:${opensearch_version}"
implementation group: 'com.google.guava', name: 'guava', version:'31.0.1-jre'
implementation group: 'com.google.guava', name: 'failureaccess', version:'1.0.1'
compileOnly group: 'com.google.guava', name: 'guava', version:'31.0.1-jre'
Copy link
Member

Choose a reason for hiding this comment

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

nit: these seemed to change in main in #770. Do you know why this was never moved to 2.x?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not sure. Maybe due to my mistakes forgetting to add the changes to build.gradle when backporting.

@amitgalitz
Copy link
Member

Should we make it standard to have separate limited PRs for any breaking change fixes in main to make backporting to 2.x easier?

@kaituo
Copy link
Collaborator Author

kaituo commented May 26, 2023

Should we make it standard to have separate limited PRs for any breaking change fixes in main to make backporting to 2.x easier?

It is hard to enforce that standard. When the main branch is broken, I will try to change everything to make it compile and pass tests. It is not easy to add one PR for each core breaking changes. The core selectively backport from main to 2.8 also adds the complexity of the current PR.

@amitgalitz amitgalitz merged commit 297ca1e into opensearch-project:2.x May 26, 2023
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.

4 participants