[JPMS Support] Refactor the codebase to eliminate top level split packages

[prudhvigodithi]

Description

• For Phase 0 - JPMS Support, refactor :server module cli package to eliminate top level split packages.

• The :libs:opensearch-cli and :server module both has org.opensearch.cli. Updated the :server org.opensearch.cli to org.opensearch.common.cli (part of :server).

• Deleted the class server/src/main/java/org/opensearch/cli/LoggingAwareCommand.java which is not used.

• Moved the common existing cli tests from :server to :libs (libs/cli/src/test/java/cli/).

• This change requires a simple refactor to eliminate the top level split package issue for org.opensearch.cli.

• Fix :distribution:tools:keystore-cli split level package problem org.opensearch.common.settings. Modify to org.opensearch.cli.keystore.

• Removed the libs/plugin-classloader and refactored the ExtendedPluginsClassLoader to :server.

• Moved some of the InstallPluginCommand and RemovePluginCommand logic to PluginsService itself, this way the Bundle, plugin and some of its methods that instantiates Bundle can be private (from PluginsService).

Related Issues

Part of #8110

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

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]

Adding @msfroh @reta @andrross to please take a look and your thoughts.
Thanks
@getsaurabh02

[github-actions]

❌ Gradle check result for 464104d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

✅ Gradle check result for 588bd0d: SUCCESS

[codecov]

Codecov Report

Attention: Patch coverage is 72.72727% with 6 lines in your changes missing coverage. Please review.

Project coverage is 72.26%. Comparing base (e6fc600) to head (cd7e3b3).
Report is 20 commits behind head on main.

Files with missing lines	Patch %	Lines
...in/java/org/opensearch/plugins/PluginsService.java	77.77%	3 Missing and 1 partial ⚠️
...nsearch/tools/cli/plugin/InstallPluginCommand.java	0.00%	1 Missing ⚠️
.../main/java/org/opensearch/bootstrap/Bootstrap.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #17153      +/-   ##
============================================
- Coverage     72.34%   72.26%   -0.08%     
+ Complexity    65481    65462      -19     
============================================
  Files          5300     5300              
  Lines        304330   304525     +195     
  Branches      44141    44174      +33     
============================================
- Hits         220158   220062      -96     
- Misses        66093    66415     +322     
+ Partials      18079    18048      -31     

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

[msfroh]

Looking through a lot of these CLI commands, I think we should move them out of :server. It's a tricky dance, though.

My intuition is as follows:

1. KeyStoreAwareCommand (and its descendants) should probably move to :distribution:tools:keystore-cli (though we need to move the constant MAX_PASSPHRASE_LENGTH to some common package, unless we're okay with duplicating it).
2. Ideally, we'd move EnvironmentAwareCommand into :lib:cli, but it depends on Environment and Settings, which are both in :server.

That's where it becomes clear that we have a lot more detangling to do (as highlighted on the linked issue). Can we at least try to move KeyStoreAwareCommand and friends to keystore-cli? (Even if it means moving MAX_PASSPHRASE_LENGTH somewhere else.)

[prudhvigodithi]

That's where it becomes clear that we have a lot more detangling to do (as highlighted on the linked issue). Can we at least try to move KeyStoreAwareCommand and friends to keystore-cli? (Even if it means moving MAX_PASSPHRASE_LENGTH somewhere else.)

Thanks @msfroh, when we try to pull the KeyStoreAwareCommand and its friends out of :server (and park in :libs) it becomes an bigger mess, we need to pull org.opensearch.node., org.opensearch.env., org.opensearch.common.settings, org.opensearch.common.logging out of server, which again has its transitive dependencies example org.opensearch.client which are again part of the :server. The high level goal of this PR is to support JPMS by eliminating top level split packages.

[prudhvigodithi]

Let me explore and see the refactoring (moving) the KeyStoreAwareCommand to keystore-cli and I will add some of analysis here.

[github-actions]

❌ Gradle check result for 9c20d11: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[prudhvigodithi]

@msfroh while working on moving KeyStoreAwareCommand and its related similar classes to keystore-cli, I have noticed the :distribution:tools:keystore-cli has a split level package problem org.opensearch.common.settings, hence I have refactored to org.opensearch.cli.keystore. I was able to move the keystore related classes to :distribution:tools:keystore-cli and fixed the problem related to MAX_PASSPHRASE_LENGTH, onboarded KeystoreConstants in :libs:opensearch-cli org.opensearch.cli which is common can be shared by the other modules. Please check.
@reta @andrross

[github-actions]

❌ Gradle check result for 0a8b149: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[prudhvigodithi]

Added missing Javadoc for:distribution:tools:keystore-cli: to the best of my knowledge (seen some assemble failures for missingJavadoc).

[msfroh]

I think this is about as good as we're going to get without a lot more refactoring, and is IMO a great start. Thanks @prudhvigodithi !

@reta , @andrross -- do you have more feedback?

[github-actions]

❌ Gradle check result for 6a60d4f: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for c5453ad: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

✅ Gradle check result for bae133d: SUCCESS

[github-actions]

✅ Gradle check result for e9fc941: SUCCESS

[github-actions]

❌ Gradle check result for 8ee2a72: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

✅ Gradle check result for cd7e3b3: SUCCESS