From aecadb8041b4145618899f96cef0d0fef56c8148 Mon Sep 17 00:00:00 2001 From: Fredy Wijaya Date: Thu, 28 Mar 2024 20:11:01 +0000 Subject: [PATCH] Rewrite JNI code for XdsTestServer Signed-off-by: Fredy Wijaya --- mobile/test/common/integration/BUILD | 4 +- .../integration/xds_test_server_interface.cc | 58 --------------- .../integration/xds_test_server_interface.h | 42 ----------- .../envoymobile/engine/testing/BUILD | 20 ++++- .../envoymobile/engine/testing/TestJni.java | 69 ------------------ .../engine/testing/XdsTestServerFactory.java | 42 +++++++++++ mobile/test/jni/BUILD | 40 ++++++++-- .../test/jni/jni_xds_test_server_factory.cc | 73 +++++++++++++++++++ mobile/test/jni/test_jni_impl.cc | 46 ------------ mobile/test/kotlin/integration/BUILD | 1 + mobile/test/kotlin/integration/XdsTest.kt | 34 +++------ 11 files changed, 179 insertions(+), 250 deletions(-) delete mode 100644 mobile/test/common/integration/xds_test_server_interface.cc delete mode 100644 mobile/test/common/integration/xds_test_server_interface.h create mode 100644 mobile/test/java/io/envoyproxy/envoymobile/engine/testing/XdsTestServerFactory.java create mode 100644 mobile/test/jni/jni_xds_test_server_factory.cc diff --git a/mobile/test/common/integration/BUILD b/mobile/test/common/integration/BUILD index 0a74d56b6b10..8eb744c5f362 100644 --- a/mobile/test/common/integration/BUILD +++ b/mobile/test/common/integration/BUILD @@ -208,14 +208,12 @@ envoy_cc_test_library( ) envoy_cc_test_library( - name = "xds_test_server_interface_lib", + name = "xds_test_server_lib", srcs = [ "xds_test_server.cc", - "xds_test_server_interface.cc", ], hdrs = [ "xds_test_server.h", - "xds_test_server_interface.h", ], repository = "@envoy", deps = [ diff --git a/mobile/test/common/integration/xds_test_server_interface.cc b/mobile/test/common/integration/xds_test_server_interface.cc deleted file mode 100644 index d4476e768327..000000000000 --- a/mobile/test/common/integration/xds_test_server_interface.cc +++ /dev/null @@ -1,58 +0,0 @@ -#include "test/common/integration/xds_test_server_interface.h" - -#include "test/common/integration/xds_test_server.h" - -#include "extension_registry.h" - -// NOLINT(namespace-envoy) - -static std::shared_ptr strong_test_server_; -static std::weak_ptr weak_test_server_; - -static std::shared_ptr testServer() { return weak_test_server_.lock(); } - -void initXdsServer() { - // This is called via JNI from kotlin tests, and Envoy doesn't consider it a test thread - // which triggers some failures of `ASSERT_IS_MAIN_OR_TEST_THREAD()`. - Envoy::Thread::SkipAsserts skip; - - Envoy::ExtensionRegistry::registerFactories(); - strong_test_server_ = std::make_shared(); - weak_test_server_ = strong_test_server_; -} - -const char* getXdsServerHost() { - if (auto server = testServer()) { - const char* host = strdup(server->getHost().c_str()); - return host; - } - return ""; // failure -} - -int getXdsServerPort() { - if (auto server = testServer()) { - return server->getPort(); - } - return -1; // failure -} - -void startXdsServer() { - if (auto server = testServer()) { - server->start(); - } -} - -void sendDiscoveryResponse(const envoy::service::discovery::v3::DiscoveryResponse& response) { - if (auto server = testServer()) { - ASSERT(server); - server->send(response); - } -} - -void shutdownXdsServer() { - // Reset the primary handle to the test_server, - // but retain it long enough to synchronously shutdown. - auto server = strong_test_server_; - strong_test_server_.reset(); - server->shutdown(); -} diff --git a/mobile/test/common/integration/xds_test_server_interface.h b/mobile/test/common/integration/xds_test_server_interface.h deleted file mode 100644 index cebc5b18ae13..000000000000 --- a/mobile/test/common/integration/xds_test_server_interface.h +++ /dev/null @@ -1,42 +0,0 @@ -#pragma once - -#include - -#include "envoy/service/discovery/v3/discovery.pb.h" - -// NOLINT(namespace-envoy) - -#ifdef __cplusplus -extern "C" { // functions -#endif - -/** Initializes xDS server. */ -void initXdsServer(); - -/** Gets the xDS server host. `initXdsServer` must be called prior to calling this function. - */ -const char* getXdsServerHost(); - -/** - * Gets the xDS server port. `initXdsServer` must be called prior to calling this function. - */ -int getXdsServerPort(); - -/** - * Starts the xDS server. `initXdsServer` must be called prior to calling this function. - */ -void startXdsServer(); - -/** - * Sends the `DiscoveryResponse`. `startXdsServer` must be called prior to calling this function. - */ -void sendDiscoveryResponse(const envoy::service::discovery::v3::DiscoveryResponse& response); - -/** - * Shuts down the xDS server. `startXdsServer` must be called prior to calling this function. - */ -void shutdownXdsServer(); - -#ifdef __cplusplus -} // functions -#endif diff --git a/mobile/test/java/io/envoyproxy/envoymobile/engine/testing/BUILD b/mobile/test/java/io/envoyproxy/envoymobile/engine/testing/BUILD index f00e319f1083..a58af01812d3 100644 --- a/mobile/test/java/io/envoyproxy/envoymobile/engine/testing/BUILD +++ b/mobile/test/java/io/envoyproxy/envoymobile/engine/testing/BUILD @@ -30,7 +30,7 @@ android_library( data = [ "//test/jni:libenvoy_jni_http_test_server_factory.so", ], - visibility = ["//test:__subpackages__"], + visibility = ["//visibility:public"], deps = [ "//library/java/io/envoyproxy/envoymobile/engine:envoy_base_engine_lib", "//library/kotlin/io/envoyproxy/envoymobile:envoy_lib", @@ -46,7 +46,23 @@ android_library( data = [ "//test/jni:libenvoy_jni_http_proxy_test_server_factory.so", ], - visibility = ["//test:__subpackages__"], + visibility = ["//visibility:public"], + deps = [ + "//library/java/io/envoyproxy/envoymobile/engine:envoy_base_engine_lib", + "//library/kotlin/io/envoyproxy/envoymobile:envoy_lib", + ], +) + +android_library( + name = "xds_test_server_factory_lib", + testonly = True, + srcs = [ + "XdsTestServerFactory.java", + ], + data = [ + "//test/jni:libenvoy_jni_xds_test_server_factory.so", + ], + visibility = ["//visibility:public"], deps = [ "//library/java/io/envoyproxy/envoymobile/engine:envoy_base_engine_lib", "//library/kotlin/io/envoyproxy/envoymobile:envoy_lib", diff --git a/mobile/test/java/io/envoyproxy/envoymobile/engine/testing/TestJni.java b/mobile/test/java/io/envoyproxy/envoymobile/engine/testing/TestJni.java index e80df3f55b39..4201267132cb 100644 --- a/mobile/test/java/io/envoyproxy/envoymobile/engine/testing/TestJni.java +++ b/mobile/test/java/io/envoyproxy/envoymobile/engine/testing/TestJni.java @@ -1,84 +1,15 @@ package io.envoyproxy.envoymobile.engine.testing; -import java.util.concurrent.atomic.AtomicBoolean; import io.envoyproxy.envoymobile.engine.EnvoyConfiguration; /** * Wrapper class for test JNI functions */ public final class TestJni { - private static final AtomicBoolean xdsServerRunning = new AtomicBoolean(); - - /** - * Initializes the xDS test server. - * - * @throws IllegalStateException if it's already started. - */ - public static void initXdsTestServer() { - if (xdsServerRunning.get()) { - throw new IllegalStateException("xDS server is already running"); - } - nativeInitXdsTestServer(); - } - - /** - * Starts the xDS test server. - * - * @throws IllegalStateException if it's already started. - */ - public static void startXdsTestServer() { - if (!xdsServerRunning.compareAndSet(false, true)) { - throw new IllegalStateException("xDS server is already running"); - } - nativeStartXdsTestServer(); - } - - /** - * Sends the `DiscoveryResponse` message in the YAML format. - */ - public static void sendDiscoveryResponse(String yaml) { - if (!xdsServerRunning.get()) { - throw new IllegalStateException("xDS server is not running"); - } - nativeSendDiscoveryResponse(yaml); - } - - /** - * Shutdowns the xDS test server. No-op if the server is already shutdown. - */ - public static void shutdownXdsTestServer() { - if (!xdsServerRunning.compareAndSet(true, false)) { - return; - } - nativeShutdownXdsTestServer(); - } - - /** - * Gets the xDS test server host. - */ - public static String getXdsTestServerHost() { return nativeGetXdsTestServerHost(); } - - /** - * Gets the xDS test server port. - */ - public static int getXdsTestServerPort() { return nativeGetXdsTestServerPort(); } - public static String createYaml(EnvoyConfiguration envoyConfiguration) { return nativeCreateYaml(envoyConfiguration.createBootstrap()); } - private static native void nativeInitXdsTestServer(); - - private static native String nativeGetXdsTestServerHost(); - - private static native int nativeGetXdsTestServerPort(); - - private static native void nativeStartXdsTestServer(); - - private static native void nativeSendDiscoveryResponse(String yaml); - - private static native int nativeShutdownXdsTestServer(); - private static native String nativeCreateYaml(long bootstrap); private TestJni() {} diff --git a/mobile/test/java/io/envoyproxy/envoymobile/engine/testing/XdsTestServerFactory.java b/mobile/test/java/io/envoyproxy/envoymobile/engine/testing/XdsTestServerFactory.java new file mode 100644 index 000000000000..140134932a66 --- /dev/null +++ b/mobile/test/java/io/envoyproxy/envoymobile/engine/testing/XdsTestServerFactory.java @@ -0,0 +1,42 @@ +package io.envoyproxy.envoymobile.engine.testing; + +/** An xDS test server factory. */ +public class XdsTestServerFactory { + /** The instance of {@link XdsTestServer}. */ + public static class XdsTestServer { + private final long handle; // Used by the native code. + private final String host; + private final int port; + + private XdsTestServer(long handle, String host, int port) { + this.handle = handle; + this.host = host; + this.port = port; + } + + /** Starts the xDS server. */ + public native void start(); + + /** Gets the xDS host. */ + public String getHost() { return host; } + + /** Gets the xDS port. */ + public int getPort() { return port; } + + /** + * Sends a static `envoy::service::discovery::v3::DiscoveryResponse` message with the specified + * cluster name. + * + * TODO(fredyw): Update to take a DiscoveryResponse proto. + */ + public native void sendDiscoveryResponse(String clusterName); + + /** Shuts down the xDS server. */ + public native void shutdown(); + } + + static { System.loadLibrary("envoy_jni_xds_test_server_factory"); } + + /** Creates a new instance of {@link XdsTestServer}. */ + public static native XdsTestServer create(); +} diff --git a/mobile/test/jni/BUILD b/mobile/test/jni/BUILD index 48c6f053bc02..0bc3ccdae196 100644 --- a/mobile/test/jni/BUILD +++ b/mobile/test/jni/BUILD @@ -10,7 +10,7 @@ cc_library( name = "envoy_jni_with_test_extensions_lib", testonly = True, deps = [ - ":server_envoy_jni_lib", + ":test_jni_impl_lib", "//library/jni:envoy_jni_lib", "@envoy_build_config//:test_extensions", ], @@ -18,7 +18,7 @@ cc_library( # Library which contains all the JNI related targets. cc_library( - name = "server_envoy_jni_lib", + name = "test_jni_impl_lib", testonly = True, srcs = [ "test_jni_impl.cc", @@ -29,8 +29,7 @@ cc_library( }), deps = [ "//library/jni:envoy_jni_lib", - "//test/common/integration:test_server_lib", - "//test/common/integration:xds_test_server_interface_lib", + "@envoy//test/test_common:utility_lib", ], # We need this to ensure that we link this into the .so even though there are no code references. alwayslink = True, @@ -98,6 +97,37 @@ cc_binary( ], ) +# Library which contains JNI functions for the XdsTestServer. +cc_library( + name = "jni_xds_test_server_factory_lib", + testonly = True, + srcs = [ + "jni_xds_test_server_factory.cc", + ], + linkopts = select({ + "@envoy//bazel:dbg_build": ["-Wl,--build-id=sha1"], + "//conditions:default": [], + }), + deps = [ + "//library/jni:envoy_jni_lib", + "//library/jni:jni_helper_lib", + "//library/jni:jni_utility_lib", + "//test/common/integration:xds_test_server_lib", + ], + # We need this to ensure that we link this into the .so even though there are no code references. + alwayslink = True, +) + +cc_binary( + name = "libenvoy_jni_xds_test_server_factory.so", + testonly = True, + linkshared = True, + deps = [ + ":jni_xds_test_server_factory_lib", + "@envoy_mobile_extra_jni_deps//:extra_jni_dep", + ], +) + # Base binary (.so) for testing cc_binary( name = "libenvoy_jni_with_test_extensions.so", @@ -124,7 +154,7 @@ cc_library( name = "envoy_jni_with_test_and_listener_extensions_lib", testonly = True, deps = [ - ":server_envoy_jni_lib", + ":test_jni_impl_lib", "//library/jni:envoy_jni_lib", "@envoy//source/common/listener_manager:listener_manager_lib", ], diff --git a/mobile/test/jni/jni_xds_test_server_factory.cc b/mobile/test/jni/jni_xds_test_server_factory.cc new file mode 100644 index 000000000000..b993335b13e9 --- /dev/null +++ b/mobile/test/jni/jni_xds_test_server_factory.cc @@ -0,0 +1,73 @@ +#include + +#include "test/common/integration/xds_test_server.h" + +#include "extension_registry.h" +#include "library/jni/jni_helper.h" +#include "library/jni/jni_utility.h" + +// NOLINT(namespace-envoy) + +extern "C" JNIEXPORT jobject JNICALL +Java_io_envoyproxy_envoymobile_engine_testing_XdsTestServerFactory_create(JNIEnv* env, jclass) { + // This is called via JNI from kotlin tests, and Envoy doesn't consider it a test thread + // which triggers some failures of `ASSERT_IS_MAIN_OR_TEST_THREAD()`. + Envoy::Thread::SkipAsserts skip; + Envoy::JNI::JniHelper jni_helper(env); + + Envoy::ExtensionRegistry::registerFactories(); + Envoy::XdsTestServer* test_server = new Envoy::XdsTestServer(); + + auto java_xds_server_factory_class = jni_helper.findClass( + "io/envoyproxy/envoymobile/engine/testing/XdsTestServerFactory$XdsTestServer"); + auto java_init_method_id = jni_helper.getMethodId(java_xds_server_factory_class.get(), "", + "(JLjava/lang/String;I)V"); + auto host = Envoy::JNI::cppStringToJavaString(jni_helper, test_server->getHost()); + jint port = static_cast(test_server->getPort()); + return jni_helper + .newObject(java_xds_server_factory_class.get(), java_init_method_id, + reinterpret_cast(test_server), host.get(), port) + .release(); +} + +extern "C" JNIEXPORT void JNICALL +Java_io_envoyproxy_envoymobile_engine_testing_XdsTestServerFactory_00024XdsTestServer_start( + JNIEnv* env, jobject instance) { + Envoy::JNI::JniHelper jni_helper(env); + auto java_class = jni_helper.getObjectClass(instance); + auto java_handle_field_id = jni_helper.getFieldId(java_class.get(), "handle", "J"); + jlong java_handle = jni_helper.getLongField(instance, java_handle_field_id); + Envoy::XdsTestServer* test_server = reinterpret_cast(java_handle); + test_server->start(); +} + +extern "C" JNIEXPORT void JNICALL +Java_io_envoyproxy_envoymobile_engine_testing_XdsTestServerFactory_00024XdsTestServer_sendDiscoveryResponse( + JNIEnv* env, jobject instance, jstring cluster_name) { + Envoy::JNI::JniHelper jni_helper(env); + auto java_class = jni_helper.getObjectClass(instance); + auto java_handle_field_id = jni_helper.getFieldId(java_class.get(), "handle", "J"); + jlong java_handle = jni_helper.getLongField(instance, java_handle_field_id); + Envoy::XdsTestServer* test_server = reinterpret_cast(java_handle); + + envoy::service::discovery::v3::DiscoveryResponse response; + *response.mutable_version_info() = "v1"; + envoy::config::cluster::v3::Cluster cluster; + *cluster.mutable_name() = Envoy::JNI::javaStringToCppString(jni_helper, cluster_name); + response.add_resources()->PackFrom(cluster); + *response.mutable_type_url() = "type.googleapis.com/envoy.config.cluster.v3.Cluster"; + + test_server->send(response); +} + +extern "C" JNIEXPORT void JNICALL +Java_io_envoyproxy_envoymobile_engine_testing_XdsTestServerFactory_00024XdsTestServer_shutdown( + JNIEnv* env, jobject instance) { + Envoy::JNI::JniHelper jni_helper(env); + auto java_class = jni_helper.getObjectClass(instance); + auto java_handle_field_id = jni_helper.getFieldId(java_class.get(), "handle", "J"); + jlong java_handle = jni_helper.getLongField(instance, java_handle_field_id); + Envoy::XdsTestServer* test_server = reinterpret_cast(java_handle); + test_server->shutdown(); + delete test_server; +} diff --git a/mobile/test/jni/test_jni_impl.cc b/mobile/test/jni/test_jni_impl.cc index fc3054b269f6..f6ac55b5f26b 100644 --- a/mobile/test/jni/test_jni_impl.cc +++ b/mobile/test/jni/test_jni_impl.cc @@ -1,57 +1,11 @@ #include -#include "test/common/integration/xds_test_server_interface.h" #include "test/test_common/utility.h" #include "library/jni/jni_support.h" // NOLINT(namespace-envoy) -extern "C" JNIEXPORT void JNICALL -Java_io_envoyproxy_envoymobile_engine_testing_TestJni_nativeInitXdsTestServer(JNIEnv* env, - jclass clazz) { - initXdsServer(); -} - -extern "C" JNIEXPORT void JNICALL -Java_io_envoyproxy_envoymobile_engine_testing_TestJni_nativeStartXdsTestServer(JNIEnv* env, - jclass clazz) { - startXdsServer(); -} - -extern "C" JNIEXPORT jstring JNICALL -Java_io_envoyproxy_envoymobile_engine_testing_TestJni_nativeGetXdsTestServerHost(JNIEnv* env, - jclass clazz) { - return env->NewStringUTF(getXdsServerHost()); -} - -extern "C" JNIEXPORT jint JNICALL -Java_io_envoyproxy_envoymobile_engine_testing_TestJni_nativeGetXdsTestServerPort(JNIEnv* env, - jclass clazz) { - return getXdsServerPort(); -} - -#ifdef ENVOY_ENABLE_YAML -extern "C" JNIEXPORT void JNICALL -Java_io_envoyproxy_envoymobile_engine_testing_TestJni_nativeSendDiscoveryResponse(JNIEnv* env, - jclass clazz, - jstring yaml) { - const char* yaml_chars = env->GetStringUTFChars(yaml, /* isCopy= */ nullptr); - // The yaml utilities have non-relevant thread asserts. - Envoy::Thread::SkipAsserts skip; - envoy::service::discovery::v3::DiscoveryResponse response; - Envoy::TestUtility::loadFromYaml(yaml_chars, response); - sendDiscoveryResponse(response); - env->ReleaseStringUTFChars(yaml, yaml_chars); -} -#endif - -extern "C" JNIEXPORT void JNICALL -Java_io_envoyproxy_envoymobile_engine_testing_TestJni_nativeShutdownXdsTestServer(JNIEnv* env, - jclass clazz) { - shutdownXdsServer(); -} - #ifdef ENVOY_ENABLE_YAML extern "C" JNIEXPORT jstring JNICALL Java_io_envoyproxy_envoymobile_engine_testing_TestJni_nativeCreateYaml(JNIEnv* env, jclass, diff --git a/mobile/test/kotlin/integration/BUILD b/mobile/test/kotlin/integration/BUILD index 6499eecc513c..b951424d07b1 100644 --- a/mobile/test/kotlin/integration/BUILD +++ b/mobile/test/kotlin/integration/BUILD @@ -360,6 +360,7 @@ envoy_mobile_android_test( ":test_utilities", "//library/kotlin/io/envoyproxy/envoymobile:envoy_interfaces_lib", "//test/java/io/envoyproxy/envoymobile/engine/testing", + "//test/java/io/envoyproxy/envoymobile/engine/testing:xds_test_server_factory_lib", ], ) diff --git a/mobile/test/kotlin/integration/XdsTest.kt b/mobile/test/kotlin/integration/XdsTest.kt index 1551006ee8a2..11723560cd20 100644 --- a/mobile/test/kotlin/integration/XdsTest.kt +++ b/mobile/test/kotlin/integration/XdsTest.kt @@ -8,7 +8,7 @@ import io.envoyproxy.envoymobile.LogLevel import io.envoyproxy.envoymobile.XdsBuilder import io.envoyproxy.envoymobile.engine.AndroidJniLibrary import io.envoyproxy.envoymobile.engine.JniLibrary -import io.envoyproxy.envoymobile.engine.testing.TestJni +import io.envoyproxy.envoymobile.engine.testing.XdsTestServerFactory import java.io.File import java.util.concurrent.CountDownLatch import org.junit.After @@ -27,11 +27,13 @@ class XdsTest { JniLibrary.load() } + private lateinit var xdsTestServer: XdsTestServerFactory.XdsTestServer + @Before fun setUp() { val upstreamCert: String = File("../envoy/test/config/integration/certs/upstreamcacert.pem").readText() - TestJni.initXdsTestServer() + xdsTestServer = XdsTestServerFactory.create() val latch = CountDownLatch(1) engine = AndroidEngineBuilder(appContext) @@ -39,46 +41,28 @@ class XdsTest { .setOnEngineRunning { latch.countDown() } .setXds( XdsBuilder( - TestJni.getXdsTestServerHost(), - TestJni.getXdsTestServerPort(), + xdsTestServer.host, + xdsTestServer.port, ) .setSslRootCerts(upstreamCert) .addClusterDiscoveryService() ) .build() latch.await() - TestJni.startXdsTestServer() + xdsTestServer.start() } @After fun tearDown() { engine.terminate() - TestJni.shutdownXdsTestServer() + xdsTestServer.shutdown() } @Test fun `test xDS with CDS`() { // There are 2 initial clusters: base and base_clear. engine.waitForStatGe("cluster_manager.cluster_added", 2) - val cdsResponse = - """ - version_info: v1 - resources: - - "@type": type.googleapis.com/envoy.config.cluster.v3.Cluster - name: my_cluster - type: STATIC - connect_timeout: 5s - typed_extension_protocol_options: - envoy.extensions.upstreams.http.v3.HttpProtocolOptions: - "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions - explicit_http_config: - http2_protocol_options: - {} - type_url: type.googleapis.com/envoy.config.cluster.v3.Cluster - nonce: nonce1 - """ - .trimIndent() - TestJni.sendDiscoveryResponse(cdsResponse) + xdsTestServer.sendDiscoveryResponse("my_cluster") // There are now 3 clusters: base, base_cluster, and my_cluster. engine.waitForStatGe("cluster_manager.cluster_added", 3) }