Skip to content

Commit

Permalink
mobile: fix setRuntimeGuard Swift API (envoyproxy#26013)
Browse files Browse the repository at this point in the history
This was never fully hooked up to the C++ builder in
`-[EnvoyConfiguration applyToCXXBuilder]`.

Co-authored-by: Rafał Augustyniak <[email protected]>
Signed-off-by: JP Simard <[email protected]>
  • Loading branch information
jpsim and Augustyniak authored Mar 9, 2023
1 parent 1a48831 commit 07f9e8f
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 32 deletions.
7 changes: 7 additions & 0 deletions mobile/library/objective-c/EnvoyConfiguration.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,13 @@ NS_ASSUME_NONNULL_BEGIN
(NSDictionary<NSString *, id<EnvoyKeyValueStore>> *)
keyValueStores
statsSinks:(NSArray<NSString *> *)statsSinks;

/**
Generate a string description of the C++ Envoy bootstrap from this configuration.
For testing purposes only.
*/
- (NSString *)bootstrapDebugDescription;

@end

NS_ASSUME_NONNULL_END
10 changes: 10 additions & 0 deletions mobile/library/objective-c/EnvoyConfiguration.mm
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,11 @@ - (instancetype)initWithAdminInterfaceEnabled:(BOOL)adminInterfaceEnabled
builder.enableGzipDecompression(self.enableGzipDecompression);
builder.enableBrotliDecompression(self.enableBrotliDecompression);

for (NSString *key in self.runtimeGuards) {
BOOL value = [[self.runtimeGuards objectForKey:key] isEqualToString:@"true"];
builder.setRuntimeGuard([key toCXXString], value);
}

for (EMODirectResponse *directResponse in self.typedDirectResponses) {
builder.addDirectResponse([directResponse toCXX]);
}
Expand Down Expand Up @@ -244,4 +249,9 @@ - (instancetype)initWithAdminInterfaceEnabled:(BOOL)adminInterfaceEnabled
}
}

- (NSString *)bootstrapDebugDescription {
std::unique_ptr<envoy::config::bootstrap::v3::Bootstrap> bootstrap = [self generateBootstrap];
return @(bootstrap->ShortDebugString().c_str());
}

@end
72 changes: 40 additions & 32 deletions mobile/library/swift/EngineBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,44 @@ open class EngineBuilder: NSObject {
let engine = self.engineType.init(runningCallback: self.onEngineRunning, logger: self.logger,
eventTracker: self.eventTracker,
networkMonitoringMode: Int32(self.monitoringMode.rawValue))
let config = EnvoyConfiguration(
let config = self.makeConfig()

switch self.base {
case .custom(let yaml):
return EngineImpl(yaml: yaml, config: config, logLevel: self.logLevel, engine: engine)
case .standard:
return EngineImpl(config: config, logLevel: self.logLevel, engine: engine)
}
}

// MARK: - Internal

/// Add a specific implementation of `EnvoyEngine` to use for starting Envoy.
/// A new instance of this engine will be created when `build()` is called.
/// Used for testing, as initializing with `EnvoyEngine.Type` results in a
/// segfault: https://github.com/envoyproxy/envoy-mobile/issues/334
///
/// - parameter engineType: The specific implementation of `EnvoyEngine` to use for starting
/// Envoy.
///
/// - returns: This builder.
@discardableResult
func addEngineType(_ engineType: EnvoyEngine.Type) -> Self {
self.engineType = engineType
return self
}

/// Add a direct response to be used when configuring the engine.
/// This function is internal so it is not publicly exposed to production builders,
/// but is available for use by the `TestEngineBuilder`.
///
/// - parameter directResponse: The response configuration to add.
func addDirectResponseInternal(_ directResponse: DirectResponse) {
self.directResponses.append(directResponse)
}

func makeConfig() -> EnvoyConfiguration {
EnvoyConfiguration(
adminInterfaceEnabled: self.adminInterfaceEnabled,
grpcStatsDomain: self.grpcStatsDomain,
connectTimeoutSeconds: self.connectTimeoutSeconds,
Expand Down Expand Up @@ -610,38 +647,9 @@ open class EngineBuilder: NSObject {
keyValueStores: self.keyValueStores,
statsSinks: self.statsSinks
)

switch self.base {
case .custom(let yaml):
return EngineImpl(yaml: yaml, config: config, logLevel: self.logLevel, engine: engine)
case .standard:
return EngineImpl(config: config, logLevel: self.logLevel, engine: engine)
}
}

// MARK: - Internal

/// Add a specific implementation of `EnvoyEngine` to use for starting Envoy.
/// A new instance of this engine will be created when `build()` is called.
/// Used for testing, as initializing with `EnvoyEngine.Type` results in a
/// segfault: https://github.com/envoyproxy/envoy-mobile/issues/334
///
/// - parameter engineType: The specific implementation of `EnvoyEngine` to use for starting
/// Envoy.
///
/// - returns: This builder.
@discardableResult
func addEngineType(_ engineType: EnvoyEngine.Type) -> Self {
self.engineType = engineType
return self
}

/// Add a direct response to be used when configuring the engine.
/// This function is internal so it is not publicly exposed to production builders,
/// but is available for use by the `TestEngineBuilder`.
///
/// - parameter directResponse: The response configuration to add.
func addDirectResponseInternal(_ directResponse: DirectResponse) {
self.directResponses.append(directResponse)
func bootstrapDebugDescription() -> String {
self.makeConfig().bootstrapDebugDescription()
}
}
13 changes: 13 additions & 0 deletions mobile/test/swift/EngineBuilderTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,19 @@ final class EngineBuilderTests: XCTestCase {
MockEnvoyEngine.onRunWithYAML = nil
}

func testSetRuntimeGuard() {
let bootstrapDebugDescription = EngineBuilder()
.setRuntimeGuard("test_feature_false", true)
.setRuntimeGuard("test_feature_true", false)
.bootstrapDebugDescription()
XCTAssertTrue(
bootstrapDebugDescription.contains(#""test_feature_false" value { bool_value: true }"#)
)
XCTAssertTrue(
bootstrapDebugDescription.contains(#""test_feature_true" value { bool_value: false }"#)
)
}

func testMonitoringModeDefaultsToPathMonitor() {
let builder = EngineBuilder()
XCTAssertEqual(builder.monitoringMode, .pathMonitor)
Expand Down

0 comments on commit 07f9e8f

Please sign in to comment.