Remove java17 from 1.3 build matrix

[DarshitChanpura]

Description

Removes Java-17 support from CI matrix as the expected behavior is that 1.3 will fail for Java-17. Refer this discussion more details: opensearch-project/job-scheduler#130

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.

[cliu123]

Please rebase.

[cliu123]

Are these needed to be removed?

security/src/test/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticatorTest.java

Lines 57 to 63 in 77d8861

	This test fails during Java 17 build due to a known bug: https://bugs.openjdk.java.net/browse/JDK-8251547
	TODO: This method should be removed once a fix is implemented
	*/
	@Before
	public void isJavaVersionBelow17(){
	assumeFalse(System.getProperty("java.version").startsWith("17"));
	}

[codecov-commenter]

Codecov Report

Merging #1668 (02b6baf) into 1.3 (776cea0) will decrease coverage by 0.03%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##                1.3    #1668      +/-   ##
============================================
- Coverage     64.62%   64.59%   -0.04%     
+ Complexity     3216     3215       -1     
============================================
  Files           247      247              
  Lines         17352    17351       -1     
  Branches       3082     3082              
============================================
- Hits          11214    11208       -6     
- Misses         4591     4592       +1     
- Partials       1547     1551       +4     

Impacted Files	Coverage Δ
...urity/ssl/transport/SecuritySSLNettyTransport.java	69.14% <0.00%> (-4.26%)	⬇️
...earch/security/ssl/util/SSLConnectionTestUtil.java	93.18% <0.00%> (-2.28%)	⬇️
...ecurity/ssl/rest/SecuritySSLReloadCertsAction.java	84.78% <0.00%> (-0.33%)	⬇️
...iance/ComplianceIndexingOperationListenerImpl.java	62.31% <0.00%> (+1.44%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 776cea0...02b6baf. Read the comment docs.

[peternied]

I don't think we need this change. We can support additional versions of the JDK without impacting the distribution.

[cliu123]

I don't think we need this change. We can support additional versions of the JDK without impacting the distribution.

JDK 17 breaks security plugin 1.3: #1653

[DarshitChanpura]

Are these needed to be removed?

security/src/test/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticatorTest.java

Lines 57 to 63 in 77d8861

	This test fails during Java 17 build due to a known bug: https://bugs.openjdk.java.net/browse/JDK-8251547
	TODO: This method should be removed once a fix is implemented
	*/
	@Before
	public void isJavaVersionBelow17(){
	assumeFalse(System.getProperty("java.version").startsWith("17"));
	}

We can remove this once we fix the Java-17 bug... but we can leave it as is right now

[peternied]

JDK 17 breaks security plugin 1.3: #1653

Yes there is a bug, you cannot run the distribution on JDK17, and distribution 1.3 will not support JDK17.

The the security plugin build will need to support JDK17 for us to fix that issue. No reason to disable it to re-enable it slightly later. We aren't 'fixing' the problem by disabling the CI build in this PR.

[DarshitChanpura]

JDK 17 breaks security plugin 1.3: #1653

Yes there is a bug, you cannot run the distribution on JDK17, and distribution 1.3 will not support JDK17.

The the security plugin build will need to support JDK17 for us to fix that issue. No reason to disable it to re-enable it slightly later. We aren't 'fixing' the problem by disabling the CI build in this PR.

Agreed...we can close this PR as this change won't is not affecting anything outside our CI pipeline

[cliu123]

The the security plugin build will need to support JDK17 for us to fix that issue. No reason to disable it to re-enable it slightly later. We aren't 'fixing' the problem by disabling the CI build in this PR.

@peternied @DarshitChanpura This PR is disabling JDK 17 only on 1.3 instead of main. We are not going to re-enable JDK 17 on 1.3. It has been decided that 1.3 will not support JDK 17:
opensearch-project/job-scheduler#130
opensearch-project/opensearch-plugins#64

JDK 17 will not be disabled on main, and the issues blocking security plugin from supporting JDK 17 will be fixed in 2.0(main). It would not make sense to keep JDK 17 that is not being supported on 1.3 branch CI. Please correct me if there are other reasons to keep JDK 17 but not to support it.

[cliu123]

We can remove this once we fix the Java-17 bug... but we can leave it as is right now

@DarshitChanpura IMHO, if 1.3 will not support JDK 17, those changes should be reverted. User is expected to see that 1.3 does not support JDK 17, which is explicitly documented, so it will be expected to fail those tests if users run those tests with JDK 17. The issues blocking JDK 17 support will only be fixed in 2.0 instead of 1.3.

[DarshitChanpura]

We can remove this once we fix the Java-17 bug... but we can leave it as is right now

@DarshitChanpura IMHO, if 1.3 will not support JDK 17, those changes should be reverted. User is expected to see that 1.3 does not support JDK 17, which is explicitly documented, so it will be expected to fail those tests if users run those tests with JDK 17. The issues blocking JDK 17 support will only be fixed in 2.0 instead of 1.3.

Okay...I'm gonna revert the test related change too...and we can decide whether we want to move forward with this merge or not.

[cliu123]

nit: Keep the empty row after imports.