From 9bdfc3d6f8514dd7ede76ac5307d307b9a7b5d69 Mon Sep 17 00:00:00 2001 From: pineappleBest123 Date: Mon, 9 Mar 2026 20:48:31 -0700 Subject: [PATCH] Fix Java 11+ compatibility and add unit tests - Remove sun.reflect.generics.reflectiveObjects.NotImplementedException (internal JDK API removed in Java 11) and replace with standard UnsupportedOperationException - Add JUnit 5 dependency and maven-surefire-plugin 3.2.5 to pom.xml - AbstractMemberProviderTest: 7 tests covering md5 initialization and getEnv() lookup behaviour - KubernetesMemberProviderTest: 13 tests covering init() timeout parsing and getMembers() filtering logic using a stub StreamProvider --- pom.xml | 11 + .../cloud/KubernetesMemberProvider.java | 3 +- .../cloud/AbstractMemberProviderTest.java | 119 ++++++++ .../cloud/KubernetesMemberProviderTest.java | 287 ++++++++++++++++++ 4 files changed, 418 insertions(+), 2 deletions(-) create mode 100644 src/test/java/org/apache/tomcat/cloud/AbstractMemberProviderTest.java create mode 100644 src/test/java/org/apache/tomcat/cloud/KubernetesMemberProviderTest.java diff --git a/pom.xml b/pom.xml index c0c2ff3..dd63a0f 100644 --- a/pom.xml +++ b/pom.xml @@ -39,10 +39,21 @@ ${json.version} provided + + org.junit.jupiter + junit-jupiter + 5.10.2 + test + + + org.apache.maven.plugins + maven-surefire-plugin + 3.2.5 + org.apache.maven.plugins maven-install-plugin diff --git a/src/main/java/org/apache/tomcat/cloud/KubernetesMemberProvider.java b/src/main/java/org/apache/tomcat/cloud/KubernetesMemberProvider.java index db59d64..9ea8329 100644 --- a/src/main/java/org/apache/tomcat/cloud/KubernetesMemberProvider.java +++ b/src/main/java/org/apache/tomcat/cloud/KubernetesMemberProvider.java @@ -26,7 +26,6 @@ import org.json.JSONException; import org.json.JSONObject; import org.json.JSONTokener; -import sun.reflect.generics.reflectiveObjects.NotImplementedException; import java.io.IOException; import java.io.InputStream; @@ -96,7 +95,7 @@ public void init(Properties properties) throws IOException { streamProvider = new TokenStreamProvider(saToken, caCertFile); } else { // TODO: implement CertificateStreamProvider - throw new NotImplementedException(); + throw new UnsupportedOperationException("CertificateStreamProvider is not yet implemented"); } String ver = getEnv(ENV_PREFIX + "API_VERSION"); diff --git a/src/test/java/org/apache/tomcat/cloud/AbstractMemberProviderTest.java b/src/test/java/org/apache/tomcat/cloud/AbstractMemberProviderTest.java new file mode 100644 index 0000000..e4f8d84 --- /dev/null +++ b/src/test/java/org/apache/tomcat/cloud/AbstractMemberProviderTest.java @@ -0,0 +1,119 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.tomcat.cloud; + +import org.apache.catalina.tribes.Member; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.List; +import java.util.Properties; + +import static org.junit.jupiter.api.Assertions.*; + +class AbstractMemberProviderTest { + + private ConcreteProvider provider; + + @BeforeEach + void setUp() { + provider = new ConcreteProvider(); + } + + // ------------------------------------------------------------------------- + // constructor + // ------------------------------------------------------------------------- + + @Test + void constructor_initializesMd5() { + // MD5 is guaranteed by the Java Security specification on all compliant JVMs. + // If the algorithm were unavailable the field would silently remain null, which + // would cause a NullPointerException the first time digest() is called in a subclass. + assertNotNull(provider.md5, "md5 digest should be initialized in constructor"); + } + + // ------------------------------------------------------------------------- + // getEnv() + // ------------------------------------------------------------------------- + + @Test + void getEnv_returnsNullWhenKeyNotFound() { + assertNull(provider.getEnvForTest("THIS_ENV_KEY_DOES_NOT_EXIST_12345")); + } + + @Test + void getEnv_returnsNullWhenAllKeysMissing() { + assertNull(provider.getEnvForTest("FAKE_KEY_AAA_99999", "FAKE_KEY_BBB_99999")); + } + + @Test + void getEnv_noKeys_returnsNull() { + // Calling getEnv() with an empty varargs array must return null because the loop + // body never executes. + assertNull(provider.getEnvForTest()); + } + + @Test + void getEnv_nullKey_throwsNullPointerException() { + // System.getenv(null) throws NullPointerException per the Java specification. + // A null element in the keys array propagates that exception to the caller. + // This documents that null keys are not supported; callers must ensure all keys + // are non-null. + assertThrows(NullPointerException.class, + () -> provider.getEnvForTest((String) null)); + } + + @Test + void getEnv_returnsFirstNonNullValue() { + String pathValue = System.getenv("PATH"); + Assumptions.assumeTrue(pathValue != null, "PATH env var not available, skipping"); + + String result = provider.getEnvForTest("THIS_KEY_DOES_NOT_EXIST", "PATH"); + assertEquals(pathValue, result, "Should skip missing keys and return the first found value"); + } + + @Test + void getEnv_stopsAtFirstFoundKey() { + String pathValue = System.getenv("PATH"); + Assumptions.assumeTrue(pathValue != null, "PATH env var not available, skipping"); + + String result = provider.getEnvForTest("PATH", "THIS_KEY_DOES_NOT_EXIST"); + assertEquals(pathValue, result, "Should return immediately when the first key is found"); + } + + // ------------------------------------------------------------------------- + // Helper: concrete subclass to expose protected members for testing + // ------------------------------------------------------------------------- + + private static class ConcreteProvider extends AbstractMemberProvider { + + String getEnvForTest(String... keys) { + return getEnv(keys); + } + + @Override + public void init(Properties properties) throws Exception {} + + @Override + public List getMembers() throws Exception { + return Collections.emptyList(); + } + } +} diff --git a/src/test/java/org/apache/tomcat/cloud/KubernetesMemberProviderTest.java b/src/test/java/org/apache/tomcat/cloud/KubernetesMemberProviderTest.java new file mode 100644 index 0000000..9c33d29 --- /dev/null +++ b/src/test/java/org/apache/tomcat/cloud/KubernetesMemberProviderTest.java @@ -0,0 +1,287 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.tomcat.cloud; + +import org.apache.catalina.tribes.Member; +import org.apache.tomcat.cloud.stream.StreamProvider; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.io.ByteArrayInputStream; +import java.net.InetAddress; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.time.Duration; +import java.time.Instant; +import java.util.List; +import java.util.Properties; + +import static org.junit.jupiter.api.Assertions.*; + +class KubernetesMemberProviderTest { + + private static final Instant REFERENCE_START_TIME = Instant.parse("2024-06-01T12:00:00Z"); + + private TestableProvider provider; + + @BeforeEach + void setUp() { + provider = new TestableProvider(); + provider.setStartTimeForTest(REFERENCE_START_TIME); + provider.setHostNameForTest("current-host"); + provider.setPortForTest(4000); + } + + // ------------------------------------------------------------------------- + // init() + // + // NOTE: The happy path through init() — when the namespace IS configured — + // requires a full Kubernetes environment (SA token file, CA cert file, API + // server reachable). That path is not covered by these unit tests and should + // be addressed by integration tests running inside a real or simulated cluster. + // ------------------------------------------------------------------------- + + @Test + void init_throwsRuntimeExceptionWhenNamespaceEnvNotSet() { + assumeNamespaceNotConfigured(); + + Properties props = new Properties(); + props.setProperty("tcpListenPort", "4000"); + + RuntimeException ex = assertThrows(RuntimeException.class, () -> provider.init(props)); + assertTrue(ex.getMessage().contains("Namespace not set"), + "Exception message should indicate that namespace is not configured"); + } + + @Test + void init_readsTimeoutsFromProperties() { + assumeNamespaceNotConfigured(); + + Properties props = new Properties(); + props.setProperty("tcpListenPort", "4000"); + props.setProperty("connectionTimeout", "3000"); + props.setProperty("readTimeout", "5000"); + + // Timeout fields are assigned before the namespace check, so they are set + // even though an exception is ultimately thrown. + try { provider.init(props); } catch (Exception ignored) {} + + assertEquals(3000, provider.connectionTimeout); + assertEquals(5000, provider.readTimeout); + } + + @Test + void init_usesDefaultTimeoutsWhenNotSpecified() { + assumeNamespaceNotConfigured(); + + Properties props = new Properties(); + props.setProperty("tcpListenPort", "4000"); + + try { provider.init(props); } catch (Exception ignored) {} + + assertEquals(1000, provider.connectionTimeout, "Default connectionTimeout should be 1000"); + assertEquals(1000, provider.readTimeout, "Default readTimeout should be 1000"); + } + + // ------------------------------------------------------------------------- + // getMembers() + // ------------------------------------------------------------------------- + + @Test + void getMembers_returnsEmptyListWhenNoItems() throws Exception { + provider.setStreamProviderForTest(jsonStream("{\"items\":[]}")); + + List members = provider.getMembers(); + + assertTrue(members.isEmpty()); + } + + @Test + void getMembers_returnsRunningPodAsMember() throws Exception { + provider.setStreamProviderForTest(jsonStream( + podsJson(pod("pod-abc", "10.0.0.1", "Running", "2024-01-01T00:00:00Z")) + )); + + List members = provider.getMembers(); + + assertEquals(1, members.size()); + assertEquals("10.0.0.1", InetAddress.getByAddress(members.get(0).getHost()).getHostAddress()); + } + + @Test + void getMembers_skipsNonRunningPods() throws Exception { + provider.setStreamProviderForTest(jsonStream( + podsJson(pod("pod-pending", "10.0.0.2", "Pending", "2024-01-01T00:00:00Z"), + pod("pod-succeeded", "10.0.0.3", "Succeeded", "2024-01-01T00:00:00Z"), + pod("pod-failed", "10.0.0.4", "Failed", "2024-01-01T00:00:00Z"), + pod("pod-terminating", "10.0.0.5", "Terminating", "2024-01-01T00:00:00Z"), + pod("pod-unknown", "10.0.0.6", "Unknown", "2024-01-01T00:00:00Z")) + )); + + List members = provider.getMembers(); + + assertTrue(members.isEmpty(), "Pods not in Running phase should be excluded"); + } + + @Test + void getMembers_skipsSelfByHostName() throws Exception { + provider.setStreamProviderForTest(jsonStream( + podsJson(pod("current-host", "10.0.0.1", "Running", "2024-01-01T00:00:00Z")) + )); + + List members = provider.getMembers(); + + assertTrue(members.isEmpty(), "Pod matching local hostName should be excluded (self)"); + } + + @Test + void getMembers_returnsOnlyRunningNonSelfPods() throws Exception { + provider.setStreamProviderForTest(jsonStream( + podsJson(pod("pod-running-1", "10.0.0.1", "Running", "2024-01-01T00:00:00Z"), + pod("current-host", "10.0.0.2", "Running", "2024-01-01T00:00:00Z"), + pod("pod-pending", "10.0.0.3", "Pending", "2024-01-01T00:00:00Z"), + pod("pod-running-2", "10.0.0.4", "Running", "2024-01-01T00:00:00Z")) + )); + + List members = provider.getMembers(); + + assertEquals(2, members.size(), "Only pod-running-1 and pod-running-2 should be returned"); + } + + @Test + void getMembers_memberHasCorrectPort() throws Exception { + provider.setPortForTest(8888); + provider.setStreamProviderForTest(jsonStream( + podsJson(pod("pod-abc", "10.0.0.1", "Running", "2024-01-01T00:00:00Z")) + )); + + List members = provider.getMembers(); + + assertEquals(8888, members.get(0).getPort()); + } + + @Test + void getMembers_uniqueIdIsMd5OfPodName() throws Exception { + String podName = "pod-unique-id-test"; + provider.setStreamProviderForTest(jsonStream( + podsJson(pod(podName, "10.0.0.1", "Running", "2024-01-01T00:00:00Z")) + )); + + List members = provider.getMembers(); + + byte[] expectedId = MessageDigest.getInstance("md5").digest(podName.getBytes()); + assertArrayEquals(expectedId, members.get(0).getUniqueId(), + "Unique ID should be the MD5 digest of the pod name"); + } + + @Test + void getMembers_uniqueIdIsConsistentAcrossMultipleCalls() throws Exception { + // MessageDigest is stateful: verify that repeated getMembers() calls produce + // identical unique IDs, i.e. the shared md5 instance is not left in a dirty state. + String podName = "pod-consistency-test"; + String json = podsJson(pod(podName, "10.0.0.1", "Running", "2024-01-01T00:00:00Z")); + + provider.setStreamProviderForTest(jsonStream(json)); + List firstCall = provider.getMembers(); + + provider.setStreamProviderForTest(jsonStream(json)); + List secondCall = provider.getMembers(); + + assertArrayEquals(firstCall.get(0).getUniqueId(), secondCall.get(0).getUniqueId(), + "md5 digest must produce the same unique ID on repeated getMembers() calls"); + } + + @Test + void getMembers_aliveTimeIsComputedFromCreationToStartTime() throws Exception { + String creationTimestamp = "2024-01-01T00:00:00Z"; + provider.setStreamProviderForTest(jsonStream( + podsJson(pod("pod-abc", "10.0.0.1", "Running", creationTimestamp)) + )); + + List members = provider.getMembers(); + + long expectedAliveTimeMs = Duration.between( + Instant.parse(creationTimestamp), REFERENCE_START_TIME).getSeconds() * 1000L; + assertEquals(expectedAliveTimeMs, members.get(0).getMemberAliveTime(), + "aliveTime should be Duration.between(creationTime, startTime) converted to milliseconds"); + } + + @Test + void getMembers_continuesWhenItemIsMissingRequiredFields() throws Exception { + // The first item has no "status" object, causing a JSONException that the + // implementation catches and continues past. The second, well-formed item + // should still be returned. + String json = "{\"items\":[" + + "{\"metadata\":{\"name\":\"broken\"}}" + + "," + + pod("pod-ok", "10.0.0.9", "Running", "2024-01-01T00:00:00Z") + + "]}"; + provider.setStreamProviderForTest(jsonStream(json)); + + List members = assertDoesNotThrow(() -> provider.getMembers(), + "A missing required field on one item should not propagate as an exception"); + + assertEquals(1, members.size(), "The valid pod should still be returned"); + assertEquals("10.0.0.9", + InetAddress.getByAddress(members.get(0).getHost()).getHostAddress()); + } + + // ------------------------------------------------------------------------- + // Helpers + // ------------------------------------------------------------------------- + + /** + * Skips the enclosing test when the Kubernetes namespace environment variable is + * already set, because init() tests that rely on the namespace being absent would + * not reach the expected code path. + */ + private static void assumeNamespaceNotConfigured() { + Assumptions.assumeTrue( + System.getenv("OPENSHIFT_KUBE_PING_NAMESPACE") == null, + "OPENSHIFT_KUBE_PING_NAMESPACE is set in this environment, skipping test"); + } + + private static String pod(String name, String ip, String phase, String creationTimestamp) { + return String.format( + "{\"metadata\":{\"name\":\"%s\",\"creationTimestamp\":\"%s\"}," + + "\"status\":{\"phase\":\"%s\",\"podIP\":\"%s\"}}", + name, creationTimestamp, phase, ip); + } + + private static String podsJson(String... items) { + return "{\"items\":[" + String.join(",", items) + "]}"; + } + + private static StreamProvider jsonStream(String json) { + return (url, headers, connectTimeout, readTimeout) -> + new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8)); + } + + // ------------------------------------------------------------------------- + // Testable subclass + // ------------------------------------------------------------------------- + + static class TestableProvider extends KubernetesMemberProvider { + + void setStreamProviderForTest(StreamProvider sp) { this.streamProvider = sp; } + void setHostNameForTest(String hn) { this.hostName = hn; } + void setPortForTest(int p) { this.port = p; } + void setStartTimeForTest(Instant t) { this.startTime = t; } + } +}