ios: e2e hello world demo for swift (#353)

Description: this PR fixes a couple bugs to allow e2e use of the Envoy Mobile library in the swift iOS demo. Risk Level: low - fixing bugs Testing: ./bazelw build --config=ios --xcode_version=10.3.0.10G8 //:ios_dist and then ./bazelw run --config=ios --xcode_version=10.3.0.10G8 //examples/swift/hello_world:app Signed-off-by: Jose Nino <[email protected]> Signed-off-by: JP Simard <[email protected]>
envoyproxy · Nov 28, 2022 · 12d91ba · 12d91ba
1 parent c36e286
commit 12d91ba
Show file tree

Hide file tree

Showing 7 changed files with 32 additions and 41 deletions.
diff --git a/mobile/library/common/config_template.cc b/mobile/library/common/config_template.cc
@@ -61,9 +61,9 @@ const char* config_template = R"(
         - lb_endpoints:
             - endpoint:
                 address:
-                  socket_address: {address: example.com, port_value: 443}
+                  socket_address: {address: s3.amazonaws.com, port_value: 443}
     tls_context:
-      sni: example.com
+      sni: s3.amazonaws.com
     type: LOGICAL_DNS
   - name: default_egress
     connect_timeout: {{ connect_timeout }}

diff --git a/mobile/library/common/http/dispatcher.cc b/mobile/library/common/http/dispatcher.cc
@@ -14,7 +14,8 @@ Dispatcher::DirectStreamCallbacks::DirectStreamCallbacks(envoy_stream_t stream,
 void Dispatcher::DirectStreamCallbacks::onHeaders(HeaderMapPtr&& headers, bool end_stream) {
   ENVOY_LOG(debug, "[S{}] response headers for stream (end_stream={}):\n{}", stream_handle_,
             end_stream, *headers);
-  observer_.on_headers(Utility::toBridgeHeaders(*headers), end_stream, observer_.context);
+  envoy_headers bridge_headers = Utility::toBridgeHeaders(*headers);
+  observer_.on_headers(bridge_headers, end_stream, observer_.context);
 }
 
 void Dispatcher::DirectStreamCallbacks::onData(Buffer::Instance& data, bool end_stream) {
@@ -53,7 +54,7 @@ envoy_status_t Dispatcher::startStream(envoy_stream_t new_stream_handle, envoy_o
   event_dispatcher_.post([this, observer, new_stream_handle]() -> void {
     DirectStreamCallbacksPtr callbacks =
         std::make_unique<DirectStreamCallbacks>(new_stream_handle, observer, *this);
-    AsyncClient& async_client = cluster_manager_.httpAsyncClientForCluster("egress_cluster");
+    AsyncClient& async_client = cluster_manager_.httpAsyncClientForCluster("base");
     AsyncClient::Stream* underlying_stream = async_client.start(*callbacks, {});
 
     if (!underlying_stream) {

diff --git a/mobile/library/common/main_interface.cc b/mobile/library/common/main_interface.cc
@@ -93,6 +93,9 @@ envoy_status_t run_engine(const char* config, const char* log_level) {
   // https://github.com/lyft/envoy-mobile/issues/34
   try {
     main_common_ = std::make_unique<Envoy::MainCommon>(5, envoy_argv);
+    // TODO: this call should be done in a post init callback.
+    // related issue: https://github.com/lyft/envoy-mobile/issues/285.
+    setup_envoy();
   } catch (const Envoy::NoServingException& e) {
     return ENVOY_SUCCESS;
   } catch (const Envoy::MalformedArgvException& e) {

diff --git a/mobile/library/common/main_interface.h b/mobile/library/common/main_interface.h
@@ -91,8 +91,6 @@ envoy_engine_t init_engine();
  */
 envoy_status_t run_engine(const char* config, const char* log_level);
 
-void setup_envoy();
-
 #ifdef __cplusplus
 } // functions
 #endif
diff --git a/mobile/library/objective-c/EnvoyEngine.h b/mobile/library/objective-c/EnvoyEngine.h
@@ -137,10 +137,6 @@ typedef NSDictionary<NSString *, NSArray<NSString *> *> EnvoyHeaders;
  */
 - (int)runWithConfig:(NSString *)config logLevel:(NSString *)logLevel;
 
-/// Performs necessary setup after Envoy has initialized and started running.
-/// TODO: create a post-initialization callback from Envoy to handle this automatically.
-- (void)setup;
-
 /**
  Opens a new HTTP stream attached to this engine.
 

diff --git a/mobile/library/objective-c/EnvoyEngine.m b/mobile/library/objective-c/EnvoyEngine.m
@@ -41,10 +41,6 @@ - (int)runWithConfig:(NSString *)config logLevel:(NSString *)logLevel {
   }
 }
 
-- (void)setup {
-  setup_envoy();
-}
-
 - (EnvoyHTTPStream *)startStreamWithObserver:(EnvoyObserver *)observer {
   return [[EnvoyHTTPStream alloc] initWithHandle:init_stream(_engineHandle) observer:observer];
 }
@@ -64,8 +60,8 @@ + (NSString *)templateString {
 #pragma mark - Utilities to move elsewhere
 
 typedef struct {
-  atomic_bool *canceled;
   EnvoyObserver *observer;
+  atomic_bool *canceled;
 } ios_context;
 
 // static envoy_data toUnmanagedNativeData(NSData *data) {
@@ -89,8 +85,8 @@ static envoy_data toManagedNativeString(NSString *s) {
 
 static envoy_headers toNativeHeaders(EnvoyHeaders *headers) {
   envoy_header_size_t length = 0;
-  for (id headerList in headers) {
-    length += [headerList count];
+  for (id headerKey in headers) {
+    length += [headers[headerKey] count];
   }
   envoy_header *header_array = (envoy_header *)malloc(sizeof(envoy_header) * length);
   envoy_header_size_t header_index = 0;
@@ -141,6 +137,7 @@ static envoy_headers toNativeHeaders(EnvoyHeaders *headers) {
 static void ios_on_headers(envoy_headers headers, bool end_stream, void *context) {
   ios_context *c = (ios_context *)context;
   EnvoyObserver *observer = c->observer;
+  // TODO: protection against null pointers.
   dispatch_async(observer.dispatchQueue, ^{
     if (atomic_load(c->canceled)) {
       return;
@@ -222,7 +219,7 @@ static void ios_on_error(envoy_error error, void *context) {
 @implementation EnvoyHTTPStream {
   EnvoyHTTPStream *_strongSelf;
   EnvoyObserver *_platformObserver;
-  envoy_observer *_nativeObserver;
+  envoy_observer _nativeObserver;
   envoy_stream_t _streamHandle;
 }
 
@@ -237,19 +234,17 @@ - (instancetype)initWithHandle:(uint64_t)handle observer:(EnvoyObserver *)observ
   _platformObserver = observer;
 
   // Create callback context
-  ios_context *context = (ios_context *)malloc(sizeof(ios_context));
+  ios_context *context = malloc(sizeof(ios_context));
   context->observer = observer;
-  context->canceled = (atomic_bool *)(malloc(sizeof(atomic_bool)));
+  context->canceled = malloc(sizeof(atomic_bool));
   atomic_store(context->canceled, NO);
 
   // Create native observer
-  envoy_observer *native_obs = (envoy_observer *)malloc(sizeof(envoy_observer));
-  envoy_observer native_init = {ios_on_headers, ios_on_data,     ios_on_trailers, ios_on_metadata,
-                                ios_on_error,   ios_on_complete, context};
-  memcpy(native_obs, &native_init, sizeof(envoy_observer));
+  envoy_observer native_obs = {ios_on_headers, ios_on_data,     ios_on_trailers, ios_on_metadata,
+                               ios_on_error,   ios_on_complete, context};
   _nativeObserver = native_obs;
 
-  envoy_status_t result = start_stream(_streamHandle, *native_obs);
+  envoy_status_t result = start_stream(_streamHandle, native_obs);
   if (result != ENVOY_SUCCESS) {
     return nil;
   }
@@ -260,12 +255,10 @@ - (instancetype)initWithHandle:(uint64_t)handle observer:(EnvoyObserver *)observ
 }
 
 - (void)dealloc {
-  envoy_observer *native_obs = _nativeObserver;
-  _nativeObserver = nil;
-  ios_context *context = native_obs->context;
+  envoy_observer native_obs = _nativeObserver;
+  ios_context *context = native_obs.context;
   free(context->canceled);
   free(context);
-  free(native_obs);
 }
 
 - (void)sendHeaders:(EnvoyHeaders *)headers close:(BOOL)close {
@@ -286,7 +279,7 @@ - (void)sendTrailers:(EnvoyHeaders *)trailers {
 }
 
 - (int)cancel {
-  ios_context *context = _nativeObserver->context;
+  ios_context *context = _nativeObserver.context;
   // Step 1: atomically and synchronously prevent the execution of further callbacks other than
   // on_cancel.
   if (!atomic_exchange(context->canceled, YES)) {

diff --git a/mobile/test/common/http/dispatcher_test.cc b/mobile/test/common/http/dispatcher_test.cc
@@ -91,7 +91,7 @@ TEST_F(DispatcherTest, BasicStreamHeadersOnly) {
   envoy_headers c_headers = Utility::toBridgeHeaders(headers);
 
   // Create a stream.
-  EXPECT_CALL(cm_, httpAsyncClientForCluster("egress_cluster"))
+  EXPECT_CALL(cm_, httpAsyncClientForCluster("base"))
       .WillOnce(ReturnRef(cm_.async_client_));
   EXPECT_CALL(cm_.async_client_, start(_, _))
       .WillOnce(
@@ -146,7 +146,7 @@ TEST_F(DispatcherTest, ResetStream) {
     cc->on_complete = true;
   };
 
-  EXPECT_CALL(cm_, httpAsyncClientForCluster("egress_cluster"))
+  EXPECT_CALL(cm_, httpAsyncClientForCluster("base"))
       .WillOnce(ReturnRef(cm_.async_client_));
   EXPECT_CALL(cm_.async_client_, start(_, _))
       .WillOnce(
@@ -202,7 +202,7 @@ TEST_F(DispatcherTest, MultipleStreams) {
   envoy_headers c_headers = Utility::toBridgeHeaders(headers);
 
   // Create a stream.
-  EXPECT_CALL(cm_, httpAsyncClientForCluster("egress_cluster"))
+  EXPECT_CALL(cm_, httpAsyncClientForCluster("base"))
       .WillOnce(ReturnRef(cm_.async_client_));
   EXPECT_CALL(cm_.async_client_, start(_, _))
       .WillOnce(
@@ -254,7 +254,7 @@ TEST_F(DispatcherTest, MultipleStreams) {
   envoy_headers c_headers2 = Utility::toBridgeHeaders(headers2);
 
   // Create a stream.
-  EXPECT_CALL(cm_, httpAsyncClientForCluster("egress_cluster"))
+  EXPECT_CALL(cm_, httpAsyncClientForCluster("base"))
       .WillOnce(ReturnRef(cm_.async_client_));
   EXPECT_CALL(cm_.async_client_, start(_, _))
       .WillOnce(
@@ -328,7 +328,7 @@ TEST_F(DispatcherTest, LocalResetAfterStreamStart) {
   envoy_headers c_headers = Utility::toBridgeHeaders(headers);
 
   // Create a stream.
-  EXPECT_CALL(cm_, httpAsyncClientForCluster("egress_cluster"))
+  EXPECT_CALL(cm_, httpAsyncClientForCluster("base"))
       .WillOnce(ReturnRef(cm_.async_client_));
   EXPECT_CALL(cm_.async_client_, start(_, _))
       .WillOnce(
@@ -401,7 +401,7 @@ TEST_F(DispatcherTest, RemoteResetAfterStreamStart) {
   envoy_headers c_headers = Utility::toBridgeHeaders(headers);
 
   // Create a stream.
-  EXPECT_CALL(cm_, httpAsyncClientForCluster("egress_cluster"))
+  EXPECT_CALL(cm_, httpAsyncClientForCluster("base"))
       .WillOnce(ReturnRef(cm_.async_client_));
   EXPECT_CALL(cm_.async_client_, start(_, _))
       .WillOnce(
@@ -446,7 +446,7 @@ TEST_F(DispatcherTest, DestroyWithActiveStream) {
   envoy_headers c_headers = Utility::toBridgeHeaders(headers);
 
   // Create a stream.
-  EXPECT_CALL(cm_, httpAsyncClientForCluster("egress_cluster"))
+  EXPECT_CALL(cm_, httpAsyncClientForCluster("base"))
       .WillOnce(ReturnRef(cm_.async_client_));
   EXPECT_CALL(cm_.async_client_, start(_, _))
       .WillOnce(Return(client_.start(stream_callbacks_, AsyncClient::StreamOptions())));
@@ -477,7 +477,7 @@ TEST_F(DispatcherTest, ResetInOnHeaders) {
   envoy_headers c_headers = Utility::toBridgeHeaders(headers);
 
   // Create a stream.
-  EXPECT_CALL(cm_, httpAsyncClientForCluster("egress_cluster"))
+  EXPECT_CALL(cm_, httpAsyncClientForCluster("base"))
       .WillOnce(ReturnRef(cm_.async_client_));
   EXPECT_CALL(cm_.async_client_, start(_, _))
       .WillOnce(Return(client_.start(stream_callbacks_, AsyncClient::StreamOptions())));
@@ -519,7 +519,7 @@ TEST_F(DispatcherTest, StreamTimeout) {
   HttpTestUtility::addDefaultHeaders(headers);
   envoy_headers c_headers = Utility::toBridgeHeaders(headers);
 
-  EXPECT_CALL(cm_, httpAsyncClientForCluster("egress_cluster"))
+  EXPECT_CALL(cm_, httpAsyncClientForCluster("base"))
       .WillOnce(ReturnRef(cm_.async_client_));
   EXPECT_CALL(cm_.async_client_, start(_, _))
       .WillOnce(Return(client_.start(stream_callbacks_, AsyncClient::StreamOptions().setTimeout(
@@ -568,7 +568,7 @@ TEST_F(DispatcherTest, StreamTimeoutHeadReply) {
   HttpTestUtility::addDefaultHeaders(headers, "HEAD");
   envoy_headers c_headers = Utility::toBridgeHeaders(headers);
 
-  EXPECT_CALL(cm_, httpAsyncClientForCluster("egress_cluster"))
+  EXPECT_CALL(cm_, httpAsyncClientForCluster("base"))
       .WillOnce(ReturnRef(cm_.async_client_));
   EXPECT_CALL(cm_.async_client_, start(_, _))
       .WillOnce(Return(client_.start(stream_callbacks_, AsyncClient::StreamOptions().setTimeout(
@@ -607,7 +607,7 @@ TEST_F(DispatcherTest, DisableTimerWithStream) {
   HttpTestUtility::addDefaultHeaders(headers, "HEAD");
   envoy_headers c_headers = Utility::toBridgeHeaders(headers);
 
-  EXPECT_CALL(cm_, httpAsyncClientForCluster("egress_cluster"))
+  EXPECT_CALL(cm_, httpAsyncClientForCluster("base"))
       .WillOnce(ReturnRef(cm_.async_client_));
   EXPECT_CALL(cm_.async_client_, start(_, _))
       .WillOnce(Return(client_.start(stream_callbacks_, AsyncClient::StreamOptions().setTimeout(