mobile: removing legacy admin APIs. (envoyproxy#27969)

Admin APIs had been made test only a while back. Now that we have stats support without admin, we can remove the admin APIs entirely. Risk Level: low Testing: n/a Docs Changes: n/a Release Notes: n/a (removing test code) Signed-off-by: Alyssa Wilk <[email protected]>
ongres · Jun 15, 2023 · 677081a · 677081a
1 parent d0d2249
commit 677081a
Show file tree

Hide file tree

Showing 32 changed files with 65 additions and 263 deletions.
diff --git a/.github/workflows/mobile-android_build.yml b/.github/workflows/mobile-android_build.yml
@@ -214,7 +214,6 @@ jobs:
         cd mobile && ./bazelw build \
             $([ -z $GITHUB_TOKEN ] || echo "--config=remote-ci-macos") \
             --fat_apk_cpu=x86_64 \
-            --define=admin_functionality=enabled \
             --define envoy_mobile_listener=enabled \
             //test/kotlin/apps/experimental:hello_envoy_kt
           adb install -r --no-incremental bazel-bin/test/kotlin/apps/experimental/hello_envoy_kt.apk

diff --git a/.github/workflows/mobile-compile_time_options.yml b/.github/workflows/mobile-compile_time_options.yml
@@ -57,7 +57,6 @@ jobs:
             --config=ios \
             $([ -z $GITHUB_TOKEN ] || echo "--config=remote-ci-macos") \
             --define=signal_trace=disabled \
-            --define=admin_html=enabled \
             --define=envoy_mobile_request_compression=disabled \
             --define=envoy_mobile_stats_reporting=disabled \
             --define=envoy_mobile_swift_cxx_interop=disabled \
@@ -91,7 +90,6 @@ jobs:
             $([ -z $GITHUB_TOKEN ] || echo "--config=remote-ci-macos") \
             --fat_apk_cpu=x86_64 \
             --define=signal_trace=disabled \
-            --define=admin_html=enabled \
             --define=envoy_mobile_request_compression=disabled \
             --define=envoy_enable_http_datagrams=disabled \
             --define=google_grpc=disabled \

diff --git a/.github/workflows/mobile-ios_tests.yml b/.github/workflows/mobile-ios_tests.yml
@@ -29,7 +29,6 @@ jobs:
     - name: 'Run swift library tests'
       env:
         GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-      # runs with admin enabled due to regression testing admin interface
       # runs with the listener enabled due to IdleTimeoutTest not setting up a test backend.
       run: |
         cd mobile && ./bazelw test \
@@ -39,7 +38,6 @@ jobs:
             --define envoy_mobile_listener=enabled \
             --build_tests_only \
             $([ -z $GITHUB_TOKEN ] || echo "--config=remote-ci-macos") \
-            --define=admin_functionality=enabled \
             //test/swift/...
   objctests:
     if: ${{ needs.env.outputs.mobile_ios_tests == 'true' }}

diff --git a/mobile/library/cc/engine_builder.cc b/mobile/library/cc/engine_builder.cc
@@ -174,13 +174,6 @@ EngineBuilder& EngineBuilder::enableSocketTagging(bool socket_tagging_on) {
   return *this;
 }
 
-#ifdef ENVOY_ADMIN_FUNCTIONALITY
-EngineBuilder& EngineBuilder::enableAdminInterface(bool admin_interface_on) {
-  admin_interface_enabled_ = admin_interface_on;
-  return *this;
-}
-#endif
-
 #ifdef ENVOY_ENABLE_QUIC
 EngineBuilder& EngineBuilder::enableHttp3(bool http3_on) {
   enable_http3_ = http3_on;
@@ -851,23 +844,10 @@ std::unique_ptr<envoy::config::bootstrap::v3::Bootstrap> EngineBuilder::generate
     list->add_patterns()->set_exact("cluster_manager.cluster_added");
   }
 
-  // Admin
-  if (admin_interface_enabled_) {
-#ifdef ENVOY_ADMIN_FUNCTIONALITY
-    auto* admin_address = bootstrap->mutable_admin()->mutable_address()->mutable_socket_address();
-    admin_address->set_address("::1");
-    admin_address->set_port_value(9901);
-#else
-    throw std::runtime_error("Admin functionality was not compiled in this build of Envoy Mobile");
-#endif
-  } else {
-    // Default Envoy mobile to the lightweight API listener. This is not
-    // supported if the admin interface is enabled.
-    envoy::config::listener::v3::ApiListenerManager api;
-    auto* listener_manager = bootstrap->mutable_listener_manager();
-    listener_manager->mutable_typed_config()->PackFrom(api);
-    listener_manager->set_name("envoy.listener_manager_impl.api");
-  }
+  envoy::config::listener::v3::ApiListenerManager api;
+  auto* listener_manager = bootstrap->mutable_listener_manager();
+  listener_manager->mutable_typed_config()->PackFrom(api);
+  listener_manager->set_name("envoy.listener_manager_impl.api");
 
   return bootstrap;
 }

diff --git a/mobile/library/cc/engine_builder.h b/mobile/library/cc/engine_builder.h
@@ -92,9 +92,6 @@ class EngineBuilder {
   EngineBuilder& setForceAlwaysUsev6(bool value);
   EngineBuilder& addDnsPreresolveHostnames(const std::vector<std::string>& hostnames);
   EngineBuilder& addNativeFilter(std::string name, std::string typed_config);
-#ifdef ENVOY_ADMIN_FUNCTIONALITY
-  EngineBuilder& enableAdminInterface(bool admin_interface_on);
-#endif
 
 #ifdef ENVOY_MOBILE_STATS_REPORTING
   EngineBuilder& addStatsSinks(std::vector<std::string> stat_sinks);
@@ -162,7 +159,6 @@ class EngineBuilder {
 
   absl::flat_hash_map<std::string, KeyValueStoreSharedPtr> key_value_stores_{};
 
-  bool admin_interface_enabled_ = false;
   bool enable_interface_binding_ = false;
   bool enable_drain_post_dns_refresh_ = false;
   bool enforce_trust_chain_verification_ = true;

diff --git a/mobile/library/common/engine.cc b/mobile/library/common/engine.cc
@@ -21,8 +21,7 @@ Engine::Engine(envoy_engine_callbacks callbacks, envoy_logger logger,
   Envoy::Api::External::registerApi(std::string(envoy_event_tracker_api_name), &event_tracker_);
 }
 
-envoy_status_t Engine::run(const std::string config, const std::string log_level,
-                           const std::string admin_address_path) {
+envoy_status_t Engine::run(const std::string config, const std::string log_level) {
   // Start the Envoy on the dedicated thread. Note: due to how the assignment operator works with
   // std::thread, main_thread_ is the same object after this call, but its state is replaced with
   // that of the temporary. The temporary object's state becomes the default state, which does
@@ -33,9 +32,6 @@ envoy_status_t Engine::run(const std::string config, const std::string log_level
     options->setLogLevel(options->parseAndValidateLogLevel(log_level.c_str()));
   }
   options->setConcurrency(1);
-  if (!admin_address_path.empty()) {
-    options->setAdminAddressPath(admin_address_path);
-  }
   return run(std::move(options));
 }
 
@@ -186,28 +182,6 @@ envoy_status_t Engine::recordCounterInc(const std::string& elements, envoy_stats
   return ENVOY_SUCCESS;
 }
 
-envoy_status_t Engine::makeAdminCall(absl::string_view path, absl::string_view method,
-                                     envoy_data& out) {
-  ENVOY_LOG(trace, "admin call {} {}", method, path);
-  if (!server_->admin()) {
-    ENVOY_LOG(warn, "admin support compiled out.");
-    return ENVOY_FAILURE;
-  }
-
-  ASSERT(dispatcher_->isThreadSafe(), "admin calls must be run from the dispatcher's context");
-  auto response_headers = Http::ResponseHeaderMapImpl::create();
-  std::string body;
-  const auto code = server_->admin()->request(path, method, *response_headers, body);
-  if (code != Http::Code::OK) {
-    ENVOY_LOG(warn, "admin call failed with status {} body {}", static_cast<uint64_t>(code), body);
-    return ENVOY_FAILURE;
-  }
-
-  out = Data::Utility::copyToBridgeData(body);
-
-  return ENVOY_SUCCESS;
-}
-
 Event::ProvisionalDispatcher& Engine::dispatcher() { return *dispatcher_; }
 
 Http::Client& Engine::httpClient() {

diff --git a/mobile/library/common/engine.h b/mobile/library/common/engine.h
@@ -34,10 +34,8 @@ class Engine : public Logger::Loggable<Logger::Id::main> {
    * Run the engine with the provided configuration.
    * @param config, the Envoy bootstrap configuration to use.
    * @param log_level, the log level.
-   * @param admin_address_path to set --admin-address-path, or an empty string if not needed.
    */
-  envoy_status_t run(std::string config, std::string log_level,
-                     const std::string admin_address_path);
+  envoy_status_t run(std::string config, std::string log_level);
   envoy_status_t run(std::unique_ptr<Envoy::OptionsImpl>&& options);
 
   /**
@@ -72,16 +70,6 @@ class Engine : public Logger::Loggable<Logger::Id::main> {
   envoy_status_t recordCounterInc(const std::string& elements, envoy_stats_tags tags,
                                   uint64_t count);
 
-  /**
-   * Issue a call against the admin handler, populating the `out` parameter with the response if
-   * the call was successful.
-   * @param path the admin path to query.
-   * @param method the HTTP method to use (GET or POST).
-   * @param out the response body, populated if the call is successful.
-   * @returns ENVOY_SUCCESS if the call was successful and `out` was populated.
-   */
-  envoy_status_t makeAdminCall(absl::string_view path, absl::string_view method, envoy_data& out);
-
   /**
    * Dump Envoy stats into the returned buffer
    * @returns a buffer with referenced stats dumped in Envoy's standard text format.

diff --git a/mobile/library/common/engine_handle.cc b/mobile/library/common/engine_handle.cc
@@ -17,9 +17,9 @@ envoy_engine_t EngineHandle::initEngine(envoy_engine_callbacks callbacks, envoy_
 }
 
 envoy_status_t EngineHandle::runEngine(envoy_engine_t handle, const char* config,
-                                       const char* log_level, const char* admin_address_path) {
+                                       const char* log_level) {
   if (auto engine = reinterpret_cast<Envoy::Engine*>(handle)) {
-    engine->run(config, log_level, admin_address_path);
+    engine->run(config, log_level);
     return ENVOY_SUCCESS;
   }
   return ENVOY_FAILURE;

diff --git a/mobile/library/common/engine_handle.h b/mobile/library/common/engine_handle.h
@@ -25,15 +25,13 @@ class EngineHandle {
 private:
   static envoy_engine_t initEngine(envoy_engine_callbacks callbacks, envoy_logger logger,
                                    envoy_event_tracker event_tracker);
-  static envoy_status_t runEngine(envoy_engine_t, const char* config, const char* log_level,
-                                  const char* admin_address_path);
+  static envoy_status_t runEngine(envoy_engine_t, const char* config, const char* log_level);
   static envoy_status_t terminateEngine(envoy_engine_t handle, bool release);
 
   // Allow a specific list of functions to access the internal setup/teardown functionality.
   friend envoy_engine_t(::init_engine)(envoy_engine_callbacks callbacks, envoy_logger logger,
                                        envoy_event_tracker event_tracker);
-  friend envoy_status_t(::run_engine)(envoy_engine_t, const char* config, const char* log_level,
-                                      const char* admin_address_path);
+  friend envoy_status_t(::run_engine)(envoy_engine_t, const char* config, const char* log_level);
   friend envoy_status_t(::terminate_engine)(envoy_engine_t engine, bool release);
 };
 

diff --git a/mobile/library/common/jni/jni_interface.cc b/mobile/library/common/jni/jni_interface.cc
@@ -133,7 +133,7 @@ extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibra
 
   jint result;
   if (!bootstrap) {
-    result = run_engine(engine, string_config, string_log_level, "");
+    result = run_engine(engine, string_config, string_log_level);
   } else {
     auto options = std::make_unique<Envoy::OptionsImpl>();
     options->setConfigProto(std::move(bootstrap));
@@ -1195,11 +1195,10 @@ javaObjectArrayToStringPairVector(JNIEnv* env, jobjectArray entries) {
 }
 
 void configureBuilder(
-    JNIEnv* env, jstring grpc_stats_domain, jboolean admin_interface_enabled,
-    jlong connect_timeout_seconds, jlong dns_refresh_seconds,
-    jlong dns_failure_refresh_seconds_base, jlong dns_failure_refresh_seconds_max,
-    jlong dns_query_timeout_seconds, jlong dns_min_refresh_seconds,
-    jobjectArray dns_preresolve_hostnames, jboolean enable_dns_cache,
+    JNIEnv* env, jstring grpc_stats_domain, jlong connect_timeout_seconds,
+    jlong dns_refresh_seconds, jlong dns_failure_refresh_seconds_base,
+    jlong dns_failure_refresh_seconds_max, jlong dns_query_timeout_seconds,
+    jlong dns_min_refresh_seconds, jobjectArray dns_preresolve_hostnames, jboolean enable_dns_cache,
     jlong dns_cache_save_interval_seconds, jboolean enable_drain_post_dns_refresh,
     jboolean enable_http3, jboolean enable_gzip_decompression, jboolean enable_brotli_decompression,
     jboolean enable_socket_tagging, jboolean enable_interface_binding,
@@ -1231,9 +1230,6 @@ void configureBuilder(
 
   builder.setStreamIdleTimeoutSeconds((stream_idle_timeout_seconds));
   builder.setPerTryIdleTimeoutSeconds((per_try_idle_timeout_seconds));
-#ifdef ENVOY_ADMIN_FUNCTIONALITY
-  builder.enableAdminInterface(admin_interface_enabled == JNI_TRUE);
-#endif
   builder.enableGzipDecompression(enable_gzip_decompression == JNI_TRUE);
   builder.enableBrotliDecompression(enable_brotli_decompression == JNI_TRUE);
   builder.enableSocketTagging(enable_socket_tagging == JNI_TRUE);
@@ -1293,11 +1289,10 @@ void configureBuilder(
 }
 
 extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_createBootstrap(
-    JNIEnv* env, jclass, jstring grpc_stats_domain, jboolean admin_interface_enabled,
-    jlong connect_timeout_seconds, jlong dns_refresh_seconds,
-    jlong dns_failure_refresh_seconds_base, jlong dns_failure_refresh_seconds_max,
-    jlong dns_query_timeout_seconds, jlong dns_min_refresh_seconds,
-    jobjectArray dns_preresolve_hostnames, jboolean enable_dns_cache,
+    JNIEnv* env, jclass, jstring grpc_stats_domain, jlong connect_timeout_seconds,
+    jlong dns_refresh_seconds, jlong dns_failure_refresh_seconds_base,
+    jlong dns_failure_refresh_seconds_max, jlong dns_query_timeout_seconds,
+    jlong dns_min_refresh_seconds, jobjectArray dns_preresolve_hostnames, jboolean enable_dns_cache,
     jlong dns_cache_save_interval_seconds, jboolean enable_drain_post_dns_refresh,
     jboolean enable_http3, jboolean enable_gzip_decompression, jboolean enable_brotli_decompression,
     jboolean enable_socket_tagging, jboolean enable_interface_binding,
@@ -1314,7 +1309,7 @@ extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibr
   Envoy::Platform::EngineBuilder builder;
 
   configureBuilder(
-      env, grpc_stats_domain, admin_interface_enabled, connect_timeout_seconds, dns_refresh_seconds,
+      env, grpc_stats_domain, connect_timeout_seconds, dns_refresh_seconds,
       dns_failure_refresh_seconds_base, dns_failure_refresh_seconds_max, dns_query_timeout_seconds,
       dns_min_refresh_seconds, dns_preresolve_hostnames, enable_dns_cache,
       dns_cache_save_interval_seconds, enable_drain_post_dns_refresh, enable_http3,

diff --git a/mobile/library/common/main_interface.cc b/mobile/library/common/main_interface.cc
@@ -121,9 +121,8 @@ envoy_engine_t init_engine(envoy_engine_callbacks callbacks, envoy_logger logger
   return Envoy::EngineHandle::initEngine(callbacks, logger, event_tracker);
 }
 
-envoy_status_t run_engine(envoy_engine_t engine, const char* config, const char* log_level,
-                          const char* admin_path) {
-  return Envoy::EngineHandle::runEngine(engine, config, log_level, admin_path);
+envoy_status_t run_engine(envoy_engine_t engine, const char* config, const char* log_level) {
+  return Envoy::EngineHandle::runEngine(engine, config, log_level);
 }
 
 envoy_status_t terminate_engine(envoy_engine_t engine, bool release) {

diff --git a/mobile/library/common/main_interface.h b/mobile/library/common/main_interface.h
@@ -203,11 +203,9 @@ envoy_engine_t init_engine(envoy_engine_callbacks callbacks, envoy_logger logger
  * @param engine, handle to the engine to run.
  * @param config, the configuration blob to run envoy with.
  * @param log_level, the logging level to run envoy with.
- * @param admin_path, the file path to log the admin address to if desired.
  * @return envoy_status_t, the resulting status of the operation.
  */
-envoy_status_t run_engine(envoy_engine_t engine, const char* config, const char* log_level,
-                          const char* admin_path);
+envoy_status_t run_engine(envoy_engine_t engine, const char* config, const char* log_level);
 
 /**
  * Terminate an engine. Further interactions with a terminated engine, or streams created by a

diff --git a/mobile/library/java/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java b/mobile/library/java/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java
@@ -28,7 +28,6 @@ public enum TrustChainVerification {
     ACCEPT_UNTRUSTED;
   }
 
-  public final Boolean adminInterfaceEnabled;
   public final String grpcStatsDomain;
   public final Integer connectTimeoutSeconds;
   public final Integer dnsRefreshSeconds;
@@ -81,8 +80,6 @@ public enum TrustChainVerification {
   /**
    * Create a new instance of the configuration.
    *
-   * @param adminInterfaceEnabled                         whether admin interface should be enabled
-   *     or not.
    * @param grpcStatsDomain                               the domain to flush stats to.
    * @param connectTimeoutSeconds                         timeout for new network connections to
    *     hosts in
@@ -152,11 +149,11 @@ public enum TrustChainVerification {
    *     could be empty.
    */
   public EnvoyConfiguration(
-      boolean adminInterfaceEnabled, String grpcStatsDomain, int connectTimeoutSeconds,
-      int dnsRefreshSeconds, int dnsFailureRefreshSecondsBase, int dnsFailureRefreshSecondsMax,
-      int dnsQueryTimeoutSeconds, int dnsMinRefreshSeconds, List<String> dnsPreresolveHostnames,
-      boolean enableDNSCache, int dnsCacheSaveIntervalSeconds, boolean enableDrainPostDnsRefresh,
-      boolean enableHttp3, boolean enableGzipDecompression, boolean enableBrotliDecompression,
+      String grpcStatsDomain, int connectTimeoutSeconds, int dnsRefreshSeconds,
+      int dnsFailureRefreshSecondsBase, int dnsFailureRefreshSecondsMax, int dnsQueryTimeoutSeconds,
+      int dnsMinRefreshSeconds, List<String> dnsPreresolveHostnames, boolean enableDNSCache,
+      int dnsCacheSaveIntervalSeconds, boolean enableDrainPostDnsRefresh, boolean enableHttp3,
+      boolean enableGzipDecompression, boolean enableBrotliDecompression,
       boolean enableSocketTagging, boolean enableInterfaceBinding,
       int h2ConnectionKeepaliveIdleIntervalMilliseconds, int h2ConnectionKeepaliveTimeoutSeconds,
       int maxConnectionsPerHost, int statsFlushSeconds, int streamIdleTimeoutSeconds,
@@ -172,7 +169,6 @@ public EnvoyConfiguration(
       String nodeRegion, String nodeZone, String nodeSubZone, String cdsResourcesLocator,
       Integer cdsTimeoutSeconds, boolean enableCds) {
     JniLibrary.load();
-    this.adminInterfaceEnabled = adminInterfaceEnabled;
     this.grpcStatsDomain = grpcStatsDomain;
     this.connectTimeoutSeconds = connectTimeoutSeconds;
     this.dnsRefreshSeconds = dnsRefreshSeconds;
@@ -249,11 +245,11 @@ public long createBootstrap() {
     byte[][] runtime_guards = JniBridgeUtility.mapToJniBytes(runtimeGuards);
 
     return JniLibrary.createBootstrap(
-        grpcStatsDomain, adminInterfaceEnabled, connectTimeoutSeconds, dnsRefreshSeconds,
-        dnsFailureRefreshSecondsBase, dnsFailureRefreshSecondsMax, dnsQueryTimeoutSeconds,
-        dnsMinRefreshSeconds, dns_preresolve, enableDNSCache, dnsCacheSaveIntervalSeconds,
-        enableDrainPostDnsRefresh, enableHttp3, enableGzipDecompression, enableBrotliDecompression,
-        enableSocketTagging, enableInterfaceBinding, h2ConnectionKeepaliveIdleIntervalMilliseconds,
+        grpcStatsDomain, connectTimeoutSeconds, dnsRefreshSeconds, dnsFailureRefreshSecondsBase,
+        dnsFailureRefreshSecondsMax, dnsQueryTimeoutSeconds, dnsMinRefreshSeconds, dns_preresolve,
+        enableDNSCache, dnsCacheSaveIntervalSeconds, enableDrainPostDnsRefresh, enableHttp3,
+        enableGzipDecompression, enableBrotliDecompression, enableSocketTagging,
+        enableInterfaceBinding, h2ConnectionKeepaliveIdleIntervalMilliseconds,
         h2ConnectionKeepaliveTimeoutSeconds, maxConnectionsPerHost, statsFlushSeconds,
         streamIdleTimeoutSeconds, perTryIdleTimeoutSeconds, appVersion, appId,
         enforceTrustChainVerification, filter_chain, stats_sinks,

diff --git a/mobile/library/java/io/envoyproxy/envoymobile/engine/JniLibrary.java b/mobile/library/java/io/envoyproxy/envoymobile/engine/JniLibrary.java
@@ -299,8 +299,8 @@ public static native Object callCertificateVerificationFromNative(byte[][] certC
    *
    */
   public static native long createBootstrap(
-      String grpcStatsDomain, boolean adminInterfaceEnabled, long connectTimeoutSeconds,
-      long dnsRefreshSeconds, long dnsFailureRefreshSecondsBase, long dnsFailureRefreshSecondsMax,
+      String grpcStatsDomain, long connectTimeoutSeconds, long dnsRefreshSeconds,
+      long dnsFailureRefreshSecondsBase, long dnsFailureRefreshSecondsMax,
       long dnsQueryTimeoutSeconds, long dnsMinRefreshSeconds, byte[][] dnsPreresolveHostnames,
       boolean enableDNSCache, long dnsCacheSaveIntervalSeconds, boolean enableDrainPostDnsRefresh,
       boolean enableHttp3, boolean enableGzipDecompression, boolean enableBrotliDecompression,