Fix JS compile issues caused by OpenSearch JPMS Refactoring #730

prudhvigodithi · 2025-02-10T20:02:25Z

Description

Fix JS compile issues caused by OpenSearch JPMS Refactoring. For more details see the CI failures part of #729.

Related Issues

Part of opensearch-project/OpenSearch#8110, #715, #426 and opensearch-project/opensearch-build#3747.

Check List

New functionality includes testing.
New functionality has been documented.
API changes companion pull request created.
Commits are signed per the DCO using --signoff.
Public documentation issue/PR created.

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.

prudhvigodithi · 2025-02-10T20:03:52Z

Adding @peterzhuamazon @cwperks
Thanks
@getsaurabh02

prudhvigodithi · 2025-02-10T20:16:40Z

@cwperks can you please check the following error with Integ tests with Security

»  Caused by: java.lang.NullPointerException
»       at java.base/java.util.Objects.requireNonNull(Objects.java:233) ~[?:?]
»       at org.opensearch.security.OpenSearchSecurityPlugin.getActionFilters(OpenSearchSecurityPlugin.java:843) ~[?:?]

This looks to me like an error from security plugin ?

codecov · 2025-02-10T20:20:29Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 37.67%. Comparing base (a3c72f7) to head (2a04d92).
Report is 1 commits behind head on main.

❌ Your project status has failed because the head coverage (37.67%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff            @@
##               main     #730   +/-   ##
=========================================
  Coverage     37.67%   37.67%           
  Complexity      135      135           
=========================================
  Files            22       22           
  Lines          1189     1189           
  Branches        109      109           
=========================================
  Hits            448      448           
  Misses          704      704           
  Partials         37       37

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

peterzhuamazon · 2025-02-10T20:26:38Z

@cwperks can you please check the following error with Integ tests with Security
»  Caused by: java.lang.NullPointerException
»       at java.base/java.util.Objects.requireNonNull(Objects.java:233) ~[?:?]
»       at org.opensearch.security.OpenSearchSecurityPlugin.getActionFilters(OpenSearchSecurityPlugin.java:843) ~[?:?]
This looks to me like an error from security plugin ?

AFAIK @cwperks is working on making security compatible with lucene10/alpha1.

peterzhuamazon · 2025-02-10T20:28:07Z

Fix compilation issues after upgrade to Lucene 10 security#5053

peterzhuamazon · 2025-02-10T20:47:32Z

...ension-plugin/src/main/java/org/opensearch/jobscheduler/sampleextension/SampleJobRunner.java

@@ -32,7 +32,7 @@
 * The job runner should be a singleton class if it uses OpenSearch client or other objects passed
 * from OpenSearch. Because when registering the job runner to JobScheduler plugin, OpenSearch has
 * not invoke plugins' createComponents() method. That is saying the plugin is not completely initalized,
- * and the OpenSearch {@link org.opensearch.client.Client}, {@link ClusterService} and other objects
+ * and the OpenSearch {@link Client}, {@link ClusterService} and other objects


and the OpenSearch {@link org.opensearch.client.Client} should be changed to and the OpenSearch {@link org.opensearch.transport.client.Client}?

Since this org.opensearch.transport.client.Client is already part of the import, Client refers to the same package Peter. Thanks

cwperks · 2025-02-10T22:27:39Z

@cwperks can you please check the following error with Integ tests with Security
»  Caused by: java.lang.NullPointerException
»       at java.base/java.util.Objects.requireNonNull(Objects.java:233) ~[?:?]
»       at org.opensearch.security.OpenSearchSecurityPlugin.getActionFilters(OpenSearchSecurityPlugin.java:843) ~[?:?]
This looks to me like an error from security plugin ?
AFAIK @cwperks is working on making security compatible with lucene10/alpha1.

I've run into the same issue in the security plugin and I think its due to a mismatch of running core and security with different qualifiers.

If I run the core from opensearch-project/OpenSearch@8a5c3b5 (commit before Lucene 10) and run with the changes in security from opensearch-project/security#5053 I get the same NPE. It may happen in reverse too if core is running alpha1, but security running w/ current SNAPSHOT non-alpha1.

prudhvigodithi · 2025-02-10T23:56:45Z

If I run the core from opensearch-project/OpenSearch@8a5c3b5 (commit before Lucene 10) and run with the changes in security from opensearch-project/security#5053 I get the same NPE. It may happen in reverse too if core is running alpha1, but security running w/ current SNAPSHOT non-alpha1.

@cwperks just an FYI lot of refactoring has been done in Core (related to the JPMS phase-0) which may cause some breaking changes, see if this is the culprit.

Thanks

cwperks · 2025-02-11T03:08:41Z

If I run the core from opensearch-project/OpenSearch@8a5c3b5 (commit before Lucene 10) and run with the changes in security from opensearch-project/security#5053 I get the same NPE. It may happen in reverse too if core is running alpha1, but security running w/ current SNAPSHOT non-alpha1.

@cwperks just an FYI lot of refactoring has been done in Core (related to the JPMS phase-0) which may cause some breaking changes, see if this is the culprit.

Thanks

opensearch-project/security#5053 accounts for the changes in core to remove master or account for the package-split with org.opensearch.client package.

The checks are passing now so if you need to unblock the build I think its ok to merge, but I'm going to keep it open to allow maintainers to review it.

Thank you to @reta for helping with adapting to the core changes for Lucene 10 + other breaking changes :)

prudhvigodithi · 2025-02-11T05:31:48Z

Thanks @cwperks and @reta we can merge this PR once security side PR is merged that way we can re-try Integ tests with Security and see if they pass , WDYT ?

peterzhuamazon · 2025-02-11T16:36:11Z

@prudhvigodithi I rerun the test now as @cwperks PR is merged now.

Thanks.

Signed-off-by: Prudhvi Godithi <[email protected]>

prudhvigodithi · 2025-02-11T17:41:51Z

build.gradle

@@ -15,6 +15,9 @@ buildscript {
        // 2.2.0-SNAPSHOT -> 2.2.0.0-SNAPSHOT
        version_tokens = opensearch_version.tokenize('-')
        opensearch_build = version_tokens[0] + '.0'
+        if (buildVersionQualifier) {
+            opensearch_build += "-${buildVersionQualifier}"
+        }


Thanks for suggesting this @cwperks. The security test pass now.

Seems like this part was missing all the time 😄

prudhvigodithi requested review from joshpalis, saratvemulapalli, dbwiddis, kaituo, vibrantvarun and cwperks as code owners February 10, 2025 20:02

prudhvigodithi changed the title ~~Fix JS with OpenSearch JPMS Refactoring~~ Fix JS compile issues caused by OpenSearch JPMS Refactoring Feb 10, 2025

peterzhuamazon reviewed Feb 10, 2025

View reviewed changes

Fix JS with OpenSearch Refactoring

2a04d92

Signed-off-by: Prudhvi Godithi <[email protected]>

prudhvigodithi force-pushed the main branch from 9bb9613 to 2a04d92 Compare February 11, 2025 17:39

cwperks approved these changes Feb 11, 2025

View reviewed changes

prudhvigodithi commented Feb 11, 2025

View reviewed changes

peterzhuamazon approved these changes Feb 11, 2025

View reviewed changes

peterzhuamazon mentioned this pull request Feb 11, 2025

Fix test failure after lucene upgrade to 10 opensearch-project/ml-commons#3426

Merged

5 tasks

prudhvigodithi merged commit 62fb0c9 into opensearch-project:main Feb 11, 2025
13 of 14 checks passed

peterzhuamazon mentioned this pull request Feb 11, 2025

[BUG] BWC bug that cause query shard failure opensearch-project/OpenSearch#17125

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix JS compile issues caused by OpenSearch JPMS Refactoring #730

Fix JS compile issues caused by OpenSearch JPMS Refactoring #730

prudhvigodithi commented Feb 10, 2025 •

edited

Loading

prudhvigodithi commented Feb 10, 2025

prudhvigodithi commented Feb 10, 2025

codecov bot commented Feb 10, 2025 •

edited

Loading

peterzhuamazon commented Feb 10, 2025

peterzhuamazon commented Feb 10, 2025

peterzhuamazon Feb 10, 2025

peterzhuamazon Feb 10, 2025

prudhvigodithi Feb 10, 2025 •

edited

Loading

cwperks commented Feb 10, 2025

prudhvigodithi commented Feb 10, 2025

cwperks commented Feb 11, 2025 •

edited

Loading

prudhvigodithi commented Feb 11, 2025

peterzhuamazon commented Feb 11, 2025

prudhvigodithi Feb 11, 2025

peterzhuamazon Feb 11, 2025

Fix JS compile issues caused by OpenSearch JPMS Refactoring #730

Fix JS compile issues caused by OpenSearch JPMS Refactoring #730

Conversation

prudhvigodithi commented Feb 10, 2025 • edited Loading

Description

Related Issues

Check List

prudhvigodithi commented Feb 10, 2025

prudhvigodithi commented Feb 10, 2025

codecov bot commented Feb 10, 2025 • edited Loading

Codecov Report

peterzhuamazon commented Feb 10, 2025

peterzhuamazon commented Feb 10, 2025

peterzhuamazon Feb 10, 2025

Choose a reason for hiding this comment

peterzhuamazon Feb 10, 2025

Choose a reason for hiding this comment

prudhvigodithi Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

cwperks commented Feb 10, 2025

prudhvigodithi commented Feb 10, 2025

cwperks commented Feb 11, 2025 • edited Loading

prudhvigodithi commented Feb 11, 2025

peterzhuamazon commented Feb 11, 2025

prudhvigodithi Feb 11, 2025

Choose a reason for hiding this comment

peterzhuamazon Feb 11, 2025

Choose a reason for hiding this comment

prudhvigodithi commented Feb 10, 2025 •

edited

Loading

codecov bot commented Feb 10, 2025 •

edited

Loading

prudhvigodithi Feb 10, 2025 •

edited

Loading

cwperks commented Feb 11, 2025 •

edited

Loading