From 1c57036a799273d17b9db012498478643465a5ac Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 26 Aug 2019 15:00:01 -0700 Subject: [PATCH 01/17] android: move yaml configuration down to java layer Signed-off-by: Alan Chiu --- .../io/envoyproxy/envoymobile/engine/BUILD | 1 - .../engine/EnvoyConfiguration.java | 44 ++++++++++--- .../engine/EnvoyConfigurationImpl.java | 15 ----- .../envoymobile/engine/EnvoyEngine.java | 8 +-- .../envoymobile/engine/EnvoyEngineImpl.java | 17 +++--- .../io/envoyproxy/envoymobile/engine/BUILD | 13 ++++ .../engine/EnvoyConfigurationTest.kt | 61 +++++++++++++++++++ .../src/io/envoyproxy/envoymobile/Envoy.kt | 7 ++- .../io/envoyproxy/envoymobile/EnvoyBuilder.kt | 35 +---------- .../envoymobile/EnvoyBuilderTest.kt | 56 ++--------------- .../io/envoyproxy/envoymobile/EnvoyTest.kt | 22 ++++--- 11 files changed, 147 insertions(+), 132 deletions(-) delete mode 100644 library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfigurationImpl.java create mode 100644 library/java/test/io/envoyproxy/envoymobile/engine/BUILD create mode 100644 library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/BUILD b/library/java/src/io/envoyproxy/envoymobile/engine/BUILD index 8685c5446a..63d6c651df 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/BUILD +++ b/library/java/src/io/envoyproxy/envoymobile/engine/BUILD @@ -22,7 +22,6 @@ envoy_mobile_java_library( name = "envoy_base_engine_lib", srcs = [ "EnvoyConfiguration.java", - "EnvoyConfigurationImpl.java", "EnvoyEngine.java", "EnvoyEngineImpl.java", "EnvoyHTTPStream.java", diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java index 70d41c0536..a62d88f154 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java @@ -1,11 +1,39 @@ package io.envoyproxy.envoymobile.engine; -public interface EnvoyConfiguration { - - /** - * Provides a default configuration template that may be used for starting Envoy. - * - * @return A template that may be used as a starting point for constructing configurations. - */ - String templateString(); +public class EnvoyConfiguration { + + public final String configYAML; + public final int connectTimeoutSeconds; + public final int dnsRefreshSeconds; + public final int statsFlushSeconds; + + public EnvoyConfiguration(String configYAML, int connectTimeoutSeconds, int dnsRefreshSeconds, int statsFlushSeconds) { + this.configYAML = configYAML; + this.connectTimeoutSeconds = connectTimeoutSeconds; + this.dnsRefreshSeconds = dnsRefreshSeconds; + this.statsFlushSeconds = statsFlushSeconds; + } + + public String resolve(String defaultConfigurationYAML) { + String resolvedYAML; + if (configYAML == null) { + resolvedYAML = defaultConfigurationYAML; + } else { + resolvedYAML = configYAML; + } + String resolvedConfiguration = resolvedYAML + .replace("{{ connect_timeout }}", String.valueOf(connectTimeoutSeconds)) + .replace("{{ dns_refresh_rate }}", String.valueOf(dnsRefreshSeconds)) + .replace("{{ stats_flush_interval }}", String.valueOf(statsFlushSeconds)); + if (resolvedConfiguration.contains("{{")) { + throw new ConfigurationException(); + } + return resolvedConfiguration; + } + + static class ConfigurationException extends RuntimeException { + ConfigurationException() { + super("Unresolved Template Key"); + } + } } diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfigurationImpl.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfigurationImpl.java deleted file mode 100644 index 0f9db4e706..0000000000 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfigurationImpl.java +++ /dev/null @@ -1,15 +0,0 @@ -package io.envoyproxy.envoymobile.engine; - -public class EnvoyConfigurationImpl implements EnvoyConfiguration { - - /** - * Provides a default configuration template that may be used for starting Envoy. - * - * @return A template that may be used as a starting point for constructing configurations. - */ - @Override - public String templateString() { - JniLibrary.load(); - return JniLibrary.templateString(); - } -} diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngine.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngine.java index 42e7b1a75e..aacb67b4b3 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngine.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngine.java @@ -12,19 +12,19 @@ public interface EnvoyEngine { EnvoyHTTPStream startStream(EnvoyObserver observer); /** - * Run the Envoy engine with the provided config and log level. + * Run the Envoy engine with the provided envoyConfiguration and log level. * * @param config The configuration file with which to start Envoy. * @return A status indicating if the action was successful. */ - int runWithConfig(String config); + int runWithConfig(EnvoyConfiguration envoyConfiguration); /** - * Run the Envoy engine with the provided config and log level. + * Run the Envoy engine with the provided envoyConfiguration and log level. * * @param config The configuration file with which to start Envoy. * @param logLevel The log level to use when starting Envoy. * @return int A status indicating if the action was successful. */ - int runWithConfig(String config, String logLevel); + int runWithConfig(EnvoyConfiguration envoyConfiguration, String logLevel); } diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java index eb29328cf4..7435b55ee2 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java @@ -24,27 +24,28 @@ public EnvoyHTTPStream startStream(EnvoyObserver observer) { } /** - * Run the Envoy engine with the provided config and log level. + * Run the Envoy engine with the provided envoyConfiguration and log level. * - * @param config The configuration file with which to start Envoy. + * @param envoyConfiguration The configuration file with which to start Envoy. * @return A status indicating if the action was successful. */ @Override - public int runWithConfig(String config) { - return runWithConfig(config, "info"); + public int runWithConfig(EnvoyConfiguration envoyConfiguration) { + return runWithConfig(envoyConfiguration, "info"); } /** - * Run the Envoy engine with the provided config and log level. + * Run the Envoy engine with the provided envoyConfiguration and log level. * - * @param config The configuration file with which to start Envoy. + * @param envoyConfiguration The configuration file with which to start Envoy. * @param logLevel The log level to use when starting Envoy. * @return int A status indicating if the action was successful. */ @Override - public int runWithConfig(String config, String logLevel) { + public int runWithConfig(EnvoyConfiguration envoyConfiguration, String logLevel) { try { - return JniLibrary.runEngine(config, logLevel); + String resolvedConfig = envoyConfiguration.resolve(JniLibrary.templateString()); + return JniLibrary.runEngine(resolvedConfig, logLevel); } catch (Throwable throwable) { // TODO: Need to have a way to log the exception somewhere return 1; diff --git a/library/java/test/io/envoyproxy/envoymobile/engine/BUILD b/library/java/test/io/envoyproxy/envoymobile/engine/BUILD new file mode 100644 index 0000000000..c24647aa5d --- /dev/null +++ b/library/java/test/io/envoyproxy/envoymobile/engine/BUILD @@ -0,0 +1,13 @@ +licenses(["notice"]) # Apache 2 + +load("//bazel:kotlin_test.bzl", "envoy_mobile_kt_test") + +envoy_mobile_kt_test( + name = "envoy_configuration_test", + srcs = [ + "EnvoyConfigurationTest.kt", + ], + deps = [ + "//library/kotlin/src/io/envoyproxy/envoymobile:envoy_interfaces_lib", + ], +) \ No newline at end of file diff --git a/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt b/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt new file mode 100644 index 0000000000..dd960a8122 --- /dev/null +++ b/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt @@ -0,0 +1,61 @@ +package io.envoyproxy.envoymobile.engine + +//private const val TEST_CONFIG = """ +//mock_template: +//- name: mock +// connect_timeout: {{ connect_timeout }} +// dns_refresh_rate: {{ dns_refresh_rate }} +// stats_flush_interval: {{ stats_flush_interval }} +//""" + + +class EnvoyConfigurationTest { +// TODO: Write these tests well + +// @Test +// fun `specifying connection timeout overrides default`() { +// builder = EnvoyBuilder() +// builder.addConfigYAML(TEST_CONFIG) +// builder.addEngineType { engine } +// +// builder.addConnectTimeoutSeconds(1234) +// val envoy = builder.build() +// +// assertThat(envoy.envoyConfiguration).contains("connect_timeout: 1234s") +// } +// +// @Test +// fun `specifying DNS refresh overrides default`() { +// builder = EnvoyBuilder() +// builder.addConfigYAML(TEST_CONFIG) +// builder.addEngineType { engine } +// +// builder.addDNSRefreshSeconds(1234) +// val envoy = builder.build() +// assertThat(envoy.envoyConfiguration).contains("dns_refresh_rate: 1234s") +// } +// +// @Test +// fun `specifying stats flush overrides default`() { +// builder = EnvoyBuilder() +// builder.addConfigYAML(TEST_CONFIG) +// builder.addEngineType { engine } +// +// builder.addStatsFlushSeconds(1234) +// builder.build() +// val envoy = builder.build() +// assertThat(envoy.envoyConfiguration).contains("stats_flush_interval: 1234s") +// +// } +// +// @Test(expected = ConfigurationException::class) +// fun `specifying configs with invalid templates will throw on build`() { +// builder = EnvoyBuilder() +// builder.addConfigYAML(TEST_CONFIG) +// builder.addEngineType { engine } +// +// builder.addConfigYAML("{{ missing }}") +// builder.build() +// } + +} \ No newline at end of file diff --git a/library/kotlin/src/io/envoyproxy/envoymobile/Envoy.kt b/library/kotlin/src/io/envoyproxy/envoymobile/Envoy.kt index e35123d330..62ba780f40 100644 --- a/library/kotlin/src/io/envoyproxy/envoymobile/Envoy.kt +++ b/library/kotlin/src/io/envoyproxy/envoymobile/Envoy.kt @@ -1,5 +1,6 @@ package io.envoyproxy.envoymobile +import io.envoyproxy.envoymobile.engine.EnvoyConfiguration import io.envoyproxy.envoymobile.engine.EnvoyEngine import java.nio.ByteBuffer @@ -21,15 +22,15 @@ enum class LogLevel(internal val level: String) { */ class Envoy constructor( private val engine: EnvoyEngine, - internal val config: String, + internal val envoyConfiguration: EnvoyConfiguration, internal val logLevel: LogLevel = LogLevel.INFO ) : Client { - constructor(engine: EnvoyEngine, config: String) : this(engine, config, LogLevel.INFO) + constructor(engine: EnvoyEngine, envoyConfiguration: EnvoyConfiguration) : this(engine, envoyConfiguration, LogLevel.INFO) // Dedicated thread for running this instance of Envoy. private val runner: Thread = Thread(ThreadGroup("Envoy"), Runnable { - engine.runWithConfig(config.trim(), logLevel.level) + engine.runWithConfig(envoyConfiguration, logLevel.level) }) /** diff --git a/library/kotlin/src/io/envoyproxy/envoymobile/EnvoyBuilder.kt b/library/kotlin/src/io/envoyproxy/envoymobile/EnvoyBuilder.kt index 02c5ee2542..9825782566 100644 --- a/library/kotlin/src/io/envoyproxy/envoymobile/EnvoyBuilder.kt +++ b/library/kotlin/src/io/envoyproxy/envoymobile/EnvoyBuilder.kt @@ -1,29 +1,20 @@ package io.envoyproxy.envoymobile import io.envoyproxy.envoymobile.engine.EnvoyConfiguration -import io.envoyproxy.envoymobile.engine.EnvoyConfigurationImpl import io.envoyproxy.envoymobile.engine.EnvoyEngine import io.envoyproxy.envoymobile.engine.EnvoyEngineImpl -class ConfigurationException : Exception("Unresolved Template Key") open class EnvoyBuilder internal constructor( - private val envoyConfiguration: EnvoyConfiguration ) { private var logLevel = LogLevel.INFO - private var configYAML: String + private var configYAML: String? = null private var engineType: () -> EnvoyEngine = { EnvoyEngineImpl() } private var connectTimeoutSeconds = 30 private var dnsRefreshSeconds = 60 private var statsFlushSeconds = 60 - constructor() : this(EnvoyConfigurationImpl()) - - init { - configYAML = envoyConfiguration.templateString() - } - /** * Add a log level to use with Envoy. * @param logLevel the log level to use with Envoy. @@ -40,7 +31,7 @@ open class EnvoyBuilder internal constructor( * @param configYAML the YAML file to use as a configuration. */ fun addConfigYAML(configYAML: String?): EnvoyBuilder { - this.configYAML = configYAML ?: envoyConfiguration.templateString() + this.configYAML = configYAML return this } @@ -80,7 +71,7 @@ open class EnvoyBuilder internal constructor( * @return A new instance of Envoy. */ fun build(): Envoy { - return Envoy(engineType(), resolvedYAML(), logLevel) + return Envoy(engineType(), EnvoyConfiguration(configYAML, connectTimeoutSeconds, dnsRefreshSeconds, statsFlushSeconds), logLevel) } /** @@ -92,24 +83,4 @@ open class EnvoyBuilder internal constructor( this.engineType = engineType return this } - - - /** Processes the YAML template provided, replacing keys with values from the configuration. - * - * @parameter template: The template YAML file to use. - * @returns: A resolved YAML file with all template keys replaced. - * @throws ConfigurationException when the yaml configuration replacement is incomplete - */ - private fun resolvedYAML(): String { - val resolvedTemplate = configYAML - .replace("{{ connect_timeout }}", "${connectTimeoutSeconds}s") - .replace("{{ dns_refresh_rate }}", "${dnsRefreshSeconds}s") - .replace("{{ stats_flush_interval }}", "${statsFlushSeconds}s") - - if (resolvedTemplate.contains("{{")) { - throw ConfigurationException() - } - - return resolvedTemplate - } } diff --git a/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt b/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt index 13e5467ffb..0fe5816cbe 100644 --- a/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt +++ b/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt @@ -3,7 +3,6 @@ package io.envoyproxy.envoymobile.io.envoyproxy.envoymobile import io.envoyproxy.envoymobile.ConfigurationException import io.envoyproxy.envoymobile.EnvoyBuilder import io.envoyproxy.envoymobile.LogLevel -import io.envoyproxy.envoymobile.engine.EnvoyConfiguration import io.envoyproxy.envoymobile.engine.EnvoyEngine import org.assertj.core.api.Assertions.assertThat import org.junit.Test @@ -22,24 +21,23 @@ class EnvoyBuilderTest { private lateinit var builder: EnvoyBuilder - private var envoyConfiguration: EnvoyConfiguration = mock(EnvoyConfiguration::class.java) private var engine: EnvoyEngine = mock(EnvoyEngine::class.java) @Test fun `adding custom config builder uses custom config for running Envoy`() { - `when`(envoyConfiguration.templateString()).thenReturn(TEST_CONFIG) - builder = EnvoyBuilder(envoyConfiguration) + builder = EnvoyBuilder() + builder.addConfigYAML(TEST_CONFIG) builder.addEngineType { engine } builder.addConfigYAML("mock_template:") val envoy = builder.build() - assertThat(envoy.config).isEqualTo("mock_template:") + assertThat(envoy.envoyConfiguration.configYAML).isEqualTo("mock_template:") } @Test fun `adding log level builder uses log level for running Envoy`() { - `when`(envoyConfiguration.templateString()).thenReturn(TEST_CONFIG) - builder = EnvoyBuilder(envoyConfiguration) + builder = EnvoyBuilder() + builder.addConfigYAML(TEST_CONFIG) builder.addEngineType { engine } builder.addLogLevel(LogLevel.DEBUG) @@ -47,48 +45,4 @@ class EnvoyBuilderTest { assertThat(envoy.logLevel).isEqualTo(LogLevel.DEBUG) } - @Test - fun `specifying connection timeout overrides default`() { - `when`(envoyConfiguration.templateString()).thenReturn(TEST_CONFIG) - builder = EnvoyBuilder(envoyConfiguration) - builder.addEngineType { engine } - - builder.addConnectTimeoutSeconds(1234) - val envoy = builder.build() - assertThat(envoy.config).contains("connect_timeout: 1234s") - } - - @Test - fun `specifying DNS refresh overrides default`() { - `when`(envoyConfiguration.templateString()).thenReturn(TEST_CONFIG) - builder = EnvoyBuilder(envoyConfiguration) - builder.addEngineType { engine } - - builder.addDNSRefreshSeconds(1234) - val envoy = builder.build() - assertThat(envoy.config).contains("dns_refresh_rate: 1234s") - } - - @Test - fun `specifying stats flush overrides default`() { - `when`(envoyConfiguration.templateString()).thenReturn(TEST_CONFIG) - builder = EnvoyBuilder(envoyConfiguration) - builder.addEngineType { engine } - - builder.addStatsFlushSeconds(1234) - builder.build() - val envoy = builder.build() - assertThat(envoy.config).contains("stats_flush_interval: 1234s") - - } - - @Test(expected = ConfigurationException::class) - fun `specifying configs with invalid templates will throw on build`() { - `when`(envoyConfiguration.templateString()).thenReturn(TEST_CONFIG) - builder = EnvoyBuilder(envoyConfiguration) - builder.addEngineType { engine } - - builder.addConfigYAML("{{ missing }}") - builder.build() - } } diff --git a/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyTest.kt b/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyTest.kt index 7ab60aabc1..85fd3a412e 100644 --- a/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyTest.kt +++ b/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyTest.kt @@ -4,6 +4,7 @@ import io.envoyproxy.envoymobile.Envoy import io.envoyproxy.envoymobile.RequestBuilder import io.envoyproxy.envoymobile.RequestMethod import io.envoyproxy.envoymobile.ResponseHandler +import io.envoyproxy.envoymobile.engine.EnvoyConfiguration import io.envoyproxy.envoymobile.engine.EnvoyEngine import io.envoyproxy.envoymobile.engine.EnvoyHTTPStream import org.junit.Test @@ -18,11 +19,12 @@ class EnvoyTest { private val engine = mock(EnvoyEngine::class.java) private val stream = mock(EnvoyHTTPStream::class.java) + private val config = EnvoyConfiguration("", 0, 0, 0) @Test fun `starting a stream on envoy sends headers`() { `when`(engine.startStream(any())).thenReturn(stream) - val envoy = Envoy(engine, "") + val envoy = Envoy(engine, config) val expectedHeaders = mapOf( "key_1" to listOf("value_a"), @@ -47,7 +49,7 @@ class EnvoyTest { @Test fun `sending data on stream stream forwards data to the underlying stream`() { `when`(engine.startStream(any())).thenReturn(stream) - val envoy = Envoy(engine, "") + val envoy = Envoy(engine, config) val emitter = envoy.send( RequestBuilder( @@ -68,7 +70,7 @@ class EnvoyTest { @Test fun `sending metadata on stream forwards metadata to the underlying stream`() { `when`(engine.startStream(any())).thenReturn(stream) - val envoy = Envoy(engine, "") + val envoy = Envoy(engine, config) val metadata = mapOf("key_1" to listOf("value_a")) val emitter = envoy.send( @@ -88,7 +90,7 @@ class EnvoyTest { @Test fun `closing stream sends empty trailers to the underlying stream`() { `when`(engine.startStream(any())).thenReturn(stream) - val envoy = Envoy(engine, "") + val envoy = Envoy(engine, config) val emitter = envoy.send( RequestBuilder( @@ -107,7 +109,7 @@ class EnvoyTest { @Test fun `closing stream with trailers sends trailers to the underlying stream `() { `when`(engine.startStream(any())).thenReturn(stream) - val envoy = Envoy(engine, "") + val envoy = Envoy(engine, config) val trailers = mapOf("key_1" to listOf("value_a")) val emitter = envoy.send( @@ -127,7 +129,7 @@ class EnvoyTest { @Test fun `sending request on envoy sends headers`() { `when`(engine.startStream(any())).thenReturn(stream) - val envoy = Envoy(engine, "") + val envoy = Envoy(engine, config) val expectedHeaders = mapOf( "key_1" to listOf("value_a"), @@ -153,7 +155,7 @@ class EnvoyTest { @Test fun `sending request on envoy passes the body buffer`() { `when`(engine.startStream(any())).thenReturn(stream) - val envoy = Envoy(engine, "") + val envoy = Envoy(engine, config) val body = ByteBuffer.allocate(0) envoy.send( @@ -172,7 +174,7 @@ class EnvoyTest { @Test fun `sending request on envoy without trailers sends empty trailers`() { `when`(engine.startStream(any())).thenReturn(stream) - val envoy = Envoy(engine, "") + val envoy = Envoy(engine, config) val body = ByteBuffer.allocate(0) envoy.send( @@ -191,7 +193,7 @@ class EnvoyTest { @Test fun `sending request on envoy sends trailers`() { `when`(engine.startStream(any())).thenReturn(stream) - val envoy = Envoy(engine, "") + val envoy = Envoy(engine, config) val trailers = mapOf("key_1" to listOf("value_a")) envoy.send( @@ -211,7 +213,7 @@ class EnvoyTest { @Test fun `cancelling stream cancels the underlying stream`() { `when`(engine.startStream(any())).thenReturn(stream) - val envoy = Envoy(engine, "") + val envoy = Envoy(engine, config) val trailers = mapOf("key_1" to listOf("value_a")) val emitter = envoy.send( From 57606e96b4d3ab33ab9ee882582d1b2f2cd87793 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 26 Aug 2019 15:03:05 -0700 Subject: [PATCH 02/17] clean up Signed-off-by: Alan Chiu --- .../envoymobile/engine/EnvoyConfiguration.java | 15 +++++++-------- .../test/io/envoyproxy/envoymobile/engine/BUILD | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java index a62d88f154..97e9f5e3fa 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java @@ -7,7 +7,8 @@ public class EnvoyConfiguration { public final int dnsRefreshSeconds; public final int statsFlushSeconds; - public EnvoyConfiguration(String configYAML, int connectTimeoutSeconds, int dnsRefreshSeconds, int statsFlushSeconds) { + public EnvoyConfiguration(String configYAML, int connectTimeoutSeconds, int dnsRefreshSeconds, + int statsFlushSeconds) { this.configYAML = configYAML; this.connectTimeoutSeconds = connectTimeoutSeconds; this.dnsRefreshSeconds = dnsRefreshSeconds; @@ -21,10 +22,10 @@ public String resolve(String defaultConfigurationYAML) { } else { resolvedYAML = configYAML; } - String resolvedConfiguration = resolvedYAML - .replace("{{ connect_timeout }}", String.valueOf(connectTimeoutSeconds)) - .replace("{{ dns_refresh_rate }}", String.valueOf(dnsRefreshSeconds)) - .replace("{{ stats_flush_interval }}", String.valueOf(statsFlushSeconds)); + String resolvedConfiguration = + resolvedYAML.replace("{{ connect_timeout }}", String.valueOf(connectTimeoutSeconds)) + .replace("{{ dns_refresh_rate }}", String.valueOf(dnsRefreshSeconds)) + .replace("{{ stats_flush_interval }}", String.valueOf(statsFlushSeconds)); if (resolvedConfiguration.contains("{{")) { throw new ConfigurationException(); } @@ -32,8 +33,6 @@ public String resolve(String defaultConfigurationYAML) { } static class ConfigurationException extends RuntimeException { - ConfigurationException() { - super("Unresolved Template Key"); - } + ConfigurationException() { super("Unresolved Template Key"); } } } diff --git a/library/java/test/io/envoyproxy/envoymobile/engine/BUILD b/library/java/test/io/envoyproxy/envoymobile/engine/BUILD index c24647aa5d..6e79dabacd 100644 --- a/library/java/test/io/envoyproxy/envoymobile/engine/BUILD +++ b/library/java/test/io/envoyproxy/envoymobile/engine/BUILD @@ -10,4 +10,4 @@ envoy_mobile_kt_test( deps = [ "//library/kotlin/src/io/envoyproxy/envoymobile:envoy_interfaces_lib", ], -) \ No newline at end of file +) From ee7b4abc533588c503925b10572d23bc60db621d Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 26 Aug 2019 15:08:47 -0700 Subject: [PATCH 03/17] new line Signed-off-by: Alan Chiu --- .../io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt b/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt index dd960a8122..29fa48b966 100644 --- a/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt +++ b/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt @@ -58,4 +58,4 @@ class EnvoyConfigurationTest { // builder.build() // } -} \ No newline at end of file +} From 50da5fba702378dfdee0d4407da9333ab7472a05 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 26 Aug 2019 15:37:20 -0700 Subject: [PATCH 04/17] push Signed-off-by: Alan Chiu --- .../kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt b/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt index 0fe5816cbe..882a7edeab 100644 --- a/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt +++ b/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt @@ -1,12 +1,10 @@ package io.envoyproxy.envoymobile.io.envoyproxy.envoymobile -import io.envoyproxy.envoymobile.ConfigurationException import io.envoyproxy.envoymobile.EnvoyBuilder import io.envoyproxy.envoymobile.LogLevel import io.envoyproxy.envoymobile.engine.EnvoyEngine import org.assertj.core.api.Assertions.assertThat import org.junit.Test -import org.mockito.Mockito.`when` import org.mockito.Mockito.mock private const val TEST_CONFIG = """ From cfed93ae28755b4fc60f8fe3fa774b1cf0ce64ad Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Mon, 26 Aug 2019 18:18:47 -0700 Subject: [PATCH 05/17] fix Signed-off-by: Alan Chiu --- .../envoyproxy/envoymobile/engine/EnvoyConfiguration.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java index 97e9f5e3fa..a472b3a0ea 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java @@ -23,9 +23,10 @@ public String resolve(String defaultConfigurationYAML) { resolvedYAML = configYAML; } String resolvedConfiguration = - resolvedYAML.replace("{{ connect_timeout }}", String.valueOf(connectTimeoutSeconds)) - .replace("{{ dns_refresh_rate }}", String.valueOf(dnsRefreshSeconds)) - .replace("{{ stats_flush_interval }}", String.valueOf(statsFlushSeconds)); + resolvedYAML + .replace("{{ connect_timeout }}", String.format("%ss", connectTimeoutSeconds)) + .replace("{{ dns_refresh_rate }}", String.format("%ss", dnsRefreshSeconds)) + .replace("{{ stats_flush_interval }}", String.format("%ss", statsFlushSeconds)); if (resolvedConfiguration.contains("{{")) { throw new ConfigurationException(); } From b008e8e5f055f522cbf0c175c18f4229f731a296 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Tue, 27 Aug 2019 10:08:35 -0700 Subject: [PATCH 06/17] format Signed-off-by: Alan Chiu --- .../io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java index a472b3a0ea..adb3edc153 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java @@ -23,8 +23,7 @@ public String resolve(String defaultConfigurationYAML) { resolvedYAML = configYAML; } String resolvedConfiguration = - resolvedYAML - .replace("{{ connect_timeout }}", String.format("%ss", connectTimeoutSeconds)) + resolvedYAML.replace("{{ connect_timeout }}", String.format("%ss", connectTimeoutSeconds)) .replace("{{ dns_refresh_rate }}", String.format("%ss", dnsRefreshSeconds)) .replace("{{ stats_flush_interval }}", String.format("%ss", statsFlushSeconds)); if (resolvedConfiguration.contains("{{")) { From ecdf0fd0eeae7b65e421ea123884fe6bccc1c184 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Tue, 3 Sep 2019 11:46:26 -0700 Subject: [PATCH 07/17] clena up and add tests Signed-off-by: Alan Chiu --- .../engine/EnvoyConfiguration.java | 32 +++++--- .../envoymobile/engine/EnvoyEngineImpl.java | 5 +- .../engine/EnvoyConfigurationTest.kt | 80 ++++++------------- .../io/envoyproxy/envoymobile/EnvoyBuilder.kt | 7 +- .../envoymobile/EnvoyBuilderTest.kt | 34 ++++++++ .../io/envoyproxy/envoymobile/EnvoyTest.kt | 2 +- 6 files changed, 90 insertions(+), 70 deletions(-) diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java index adb3edc153..68a8658292 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java @@ -3,29 +3,33 @@ public class EnvoyConfiguration { public final String configYAML; - public final int connectTimeoutSeconds; - public final int dnsRefreshSeconds; - public final int statsFlushSeconds; + public final Integer connectTimeoutSeconds; + public final Integer dnsRefreshSeconds; + public final Integer statsFlushSeconds; - public EnvoyConfiguration(String configYAML, int connectTimeoutSeconds, int dnsRefreshSeconds, - int statsFlushSeconds) { + public EnvoyConfiguration(String configYAML) { this.configYAML = configYAML; + this.connectTimeoutSeconds = null; + this.dnsRefreshSeconds = null; + this.statsFlushSeconds = null; + } + + public EnvoyConfiguration(int connectTimeoutSeconds, int dnsRefreshSeconds, int statsFlushSeconds) { + this.configYAML = null; this.connectTimeoutSeconds = connectTimeoutSeconds; this.dnsRefreshSeconds = dnsRefreshSeconds; this.statsFlushSeconds = statsFlushSeconds; } + + public String resolve(String defaultConfigurationYAML) { - String resolvedYAML; - if (configYAML == null) { - resolvedYAML = defaultConfigurationYAML; - } else { - resolvedYAML = configYAML; - } String resolvedConfiguration = - resolvedYAML.replace("{{ connect_timeout }}", String.format("%ss", connectTimeoutSeconds)) + defaultConfigurationYAML + .replace("{{ connect_timeout }}", String.format("%ss", connectTimeoutSeconds)) .replace("{{ dns_refresh_rate }}", String.format("%ss", dnsRefreshSeconds)) .replace("{{ stats_flush_interval }}", String.format("%ss", statsFlushSeconds)); + if (resolvedConfiguration.contains("{{")) { throw new ConfigurationException(); } @@ -33,6 +37,8 @@ public String resolve(String defaultConfigurationYAML) { } static class ConfigurationException extends RuntimeException { - ConfigurationException() { super("Unresolved Template Key"); } + ConfigurationException() { + super("Unresolved Template Key"); + } } } diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java index 7435b55ee2..34ab997f29 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java @@ -44,7 +44,10 @@ public int runWithConfig(EnvoyConfiguration envoyConfiguration) { @Override public int runWithConfig(EnvoyConfiguration envoyConfiguration, String logLevel) { try { - String resolvedConfig = envoyConfiguration.resolve(JniLibrary.templateString()); + + String resolvedConfig = envoyConfiguration.configYAML == null + ? envoyConfiguration.resolve(JniLibrary.templateString()) + : envoyConfiguration.configYAML; return JniLibrary.runEngine(resolvedConfig, logLevel); } catch (Throwable throwable) { // TODO: Need to have a way to log the exception somewhere diff --git a/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt b/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt index 29fa48b966..d3b4437ad9 100644 --- a/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt +++ b/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt @@ -1,61 +1,33 @@ package io.envoyproxy.envoymobile.engine -//private const val TEST_CONFIG = """ -//mock_template: -//- name: mock -// connect_timeout: {{ connect_timeout }} -// dns_refresh_rate: {{ dns_refresh_rate }} -// stats_flush_interval: {{ stats_flush_interval }} -//""" +import org.junit.Test + +private const val TEST_CONFIG = """ +mock_template: +- name: mock + connect_timeout: {{ connect_timeout }} + dns_refresh_rate: {{ dns_refresh_rate }} + stats_flush_interval: {{ stats_flush_interval }} +""" class EnvoyConfigurationTest { -// TODO: Write these tests well - -// @Test -// fun `specifying connection timeout overrides default`() { -// builder = EnvoyBuilder() -// builder.addConfigYAML(TEST_CONFIG) -// builder.addEngineType { engine } -// -// builder.addConnectTimeoutSeconds(1234) -// val envoy = builder.build() -// -// assertThat(envoy.envoyConfiguration).contains("connect_timeout: 1234s") -// } -// -// @Test -// fun `specifying DNS refresh overrides default`() { -// builder = EnvoyBuilder() -// builder.addConfigYAML(TEST_CONFIG) -// builder.addEngineType { engine } -// -// builder.addDNSRefreshSeconds(1234) -// val envoy = builder.build() -// assertThat(envoy.envoyConfiguration).contains("dns_refresh_rate: 1234s") -// } -// -// @Test -// fun `specifying stats flush overrides default`() { -// builder = EnvoyBuilder() -// builder.addConfigYAML(TEST_CONFIG) -// builder.addEngineType { engine } -// -// builder.addStatsFlushSeconds(1234) -// builder.build() -// val envoy = builder.build() -// assertThat(envoy.envoyConfiguration).contains("stats_flush_interval: 1234s") -// -// } -// -// @Test(expected = ConfigurationException::class) -// fun `specifying configs with invalid templates will throw on build`() { -// builder = EnvoyBuilder() -// builder.addConfigYAML(TEST_CONFIG) -// builder.addEngineType { engine } -// -// builder.addConfigYAML("{{ missing }}") -// builder.build() -// } + @Test + fun `resolving with default configuraiton resolves with values`() { + val envoyConfiguration = EnvoyConfiguration(123, 234, 345) + + val resolvedTemplate = envoyConfiguration.resolve(TEST_CONFIG) + assertThat(resolvedTemplate).contains("connect_timeout: 123s") + assertThat(resolvedTemplate).contains("dns_refresh_rate: 234s") + assertThat(resolvedTemplate).contains("stats_flush_interval: 345s") + } + + + @Test(expected = ConfigurationException::class) + fun `resolve templates with invalid templates will throw on build`() { + val envoyConfiguration = EnvoyConfiguration(123, 234, 345) + + envoyConfiguration.resolve("{{ }}") + } } diff --git a/library/kotlin/src/io/envoyproxy/envoymobile/EnvoyBuilder.kt b/library/kotlin/src/io/envoyproxy/envoymobile/EnvoyBuilder.kt index 9825782566..cae9d8b43b 100644 --- a/library/kotlin/src/io/envoyproxy/envoymobile/EnvoyBuilder.kt +++ b/library/kotlin/src/io/envoyproxy/envoymobile/EnvoyBuilder.kt @@ -71,7 +71,12 @@ open class EnvoyBuilder internal constructor( * @return A new instance of Envoy. */ fun build(): Envoy { - return Envoy(engineType(), EnvoyConfiguration(configYAML, connectTimeoutSeconds, dnsRefreshSeconds, statsFlushSeconds), logLevel) + val config = if (configYAML == null) { + EnvoyConfiguration(connectTimeoutSeconds, dnsRefreshSeconds, statsFlushSeconds) + } else { + EnvoyConfiguration(configYAML) + } + return Envoy(engineType(), config, logLevel) } /** diff --git a/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt b/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt index 882a7edeab..134018bf1e 100644 --- a/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt +++ b/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt @@ -43,4 +43,38 @@ class EnvoyBuilderTest { assertThat(envoy.logLevel).isEqualTo(LogLevel.DEBUG) } + @Test + fun `specifying connection timeout overrides default`() { + `when`(envoyConfiguration.templateString()).thenReturn(TEST_CONFIG) + builder = EnvoyBuilder(envoyConfiguration) + builder.addEngineType { engine } + + builder.addConnectTimeoutSeconds(1234) + val envoy = builder.build() + assertThat(envoy.envoyConfiguration.connectTimeoutSeconds).isEqualTo(1234) + } + + @Test + fun `specifying DNS refresh overrides default`() { + `when`(envoyConfiguration.templateString()).thenReturn(TEST_CONFIG) + builder = EnvoyBuilder(envoyConfiguration) + builder.addEngineType { engine } + + builder.addDNSRefreshSeconds(1234) + val envoy = builder.build() + assertThat(envoy.envoyConfiguration.dnsRefreshSeconds).isEqualTo(1234) + } + + @Test + fun `specifying stats flush overrides default`() { + `when`(envoyConfiguration.templateString()).thenReturn(TEST_CONFIG) + builder = EnvoyBuilder(envoyConfiguration) + builder.addEngineType { engine } + + builder.addStatsFlushSeconds(1234) + builder.build() + val envoy = builder.build() + assertThat(envoy.envoyConfiguration.statsFlushSeconds).isEqualTo(1234) + + } } diff --git a/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyTest.kt b/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyTest.kt index 85fd3a412e..08ba2da627 100644 --- a/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyTest.kt +++ b/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyTest.kt @@ -19,7 +19,7 @@ class EnvoyTest { private val engine = mock(EnvoyEngine::class.java) private val stream = mock(EnvoyHTTPStream::class.java) - private val config = EnvoyConfiguration("", 0, 0, 0) + private val config = EnvoyConfiguration("") @Test fun `starting a stream on envoy sends headers`() { From 1c37fb1fda1d4b1a7786e7327a5654ad37eea27f Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Tue, 3 Sep 2019 12:39:12 -0700 Subject: [PATCH 08/17] fix Signed-off-by: Alan Chiu --- .../engine/EnvoyConfiguration.java | 33 +++++++++++++++---- .../envoymobile/engine/EnvoyEngineImpl.java | 4 +-- .../engine/EnvoyConfigurationTest.kt | 4 +-- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java index 68a8658292..49c9a5f702 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java @@ -7,6 +7,11 @@ public class EnvoyConfiguration { public final Integer dnsRefreshSeconds; public final Integer statsFlushSeconds; + /** + * Create an EnvoyConfiguration with a user provided configuration YAML. + * + * @param configYAML configuration to be used for starting Envoy. + */ public EnvoyConfiguration(String configYAML) { this.configYAML = configYAML; this.connectTimeoutSeconds = null; @@ -14,16 +19,32 @@ public EnvoyConfiguration(String configYAML) { this.statsFlushSeconds = null; } - public EnvoyConfiguration(int connectTimeoutSeconds, int dnsRefreshSeconds, int statsFlushSeconds) { + /** + * Create an EnvoyConfiguration with a user provided configuration values. + * + * @param connectTimeoutSeconds timeout for new network connections to hosts in the cluster. + * @param dnsRefreshSeconds rate in seconds to refresh DNS. + * @param statsFlushSeconds interval at which to flush Envoy stats. + */ + public EnvoyConfiguration(int connectTimeoutSeconds, int dnsRefreshSeconds, + int statsFlushSeconds) { this.configYAML = null; this.connectTimeoutSeconds = connectTimeoutSeconds; this.dnsRefreshSeconds = dnsRefreshSeconds; this.statsFlushSeconds = statsFlushSeconds; } - - - public String resolve(String defaultConfigurationYAML) { + /** + * Resolves the provided default configuration with the field values: + * - connectTimeoutSeconds + * - dnsRefreshSeconds + * - statsFlushSeconds + * + * @param defaultConfigurationYAML the default template configuration. + * @return String, the resolved template. + * @throws ConfigurationException, when the template provided is not fully resolved. + */ + String resolveTemplate(String defaultConfigurationYAML) { String resolvedConfiguration = defaultConfigurationYAML .replace("{{ connect_timeout }}", String.format("%ss", connectTimeoutSeconds)) @@ -37,8 +58,6 @@ public String resolve(String defaultConfigurationYAML) { } static class ConfigurationException extends RuntimeException { - ConfigurationException() { - super("Unresolved Template Key"); - } + ConfigurationException() { super("Unresolved Template Key"); } } } diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java index 34ab997f29..70e53d1c7c 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java @@ -46,8 +46,8 @@ public int runWithConfig(EnvoyConfiguration envoyConfiguration, String logLevel) try { String resolvedConfig = envoyConfiguration.configYAML == null - ? envoyConfiguration.resolve(JniLibrary.templateString()) - : envoyConfiguration.configYAML; + ? envoyConfiguration.resolveTemplate(JniLibrary.templateString()) + : envoyConfiguration.configYAML; return JniLibrary.runEngine(resolvedConfig, logLevel); } catch (Throwable throwable) { // TODO: Need to have a way to log the exception somewhere diff --git a/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt b/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt index d3b4437ad9..3d11caa260 100644 --- a/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt +++ b/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt @@ -17,7 +17,7 @@ class EnvoyConfigurationTest { fun `resolving with default configuraiton resolves with values`() { val envoyConfiguration = EnvoyConfiguration(123, 234, 345) - val resolvedTemplate = envoyConfiguration.resolve(TEST_CONFIG) + val resolvedTemplate = envoyConfiguration.resolveTemplate(TEST_CONFIG) assertThat(resolvedTemplate).contains("connect_timeout: 123s") assertThat(resolvedTemplate).contains("dns_refresh_rate: 234s") assertThat(resolvedTemplate).contains("stats_flush_interval: 345s") @@ -28,6 +28,6 @@ class EnvoyConfigurationTest { fun `resolve templates with invalid templates will throw on build`() { val envoyConfiguration = EnvoyConfiguration(123, 234, 345) - envoyConfiguration.resolve("{{ }}") + envoyConfiguration.resolveTemplate("{{ }}") } } From 3272f7eeefb85686d6485d33dd8c600987369c40 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Tue, 3 Sep 2019 12:53:50 -0700 Subject: [PATCH 09/17] OOOOPS Signed-off-by: Alan Chiu --- .../envoymobile/engine/EnvoyConfiguration.java | 1 + .../test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt | 9 +++------ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java index 49c9a5f702..ed1820be98 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java @@ -39,6 +39,7 @@ public EnvoyConfiguration(int connectTimeoutSeconds, int dnsRefreshSeconds, * - connectTimeoutSeconds * - dnsRefreshSeconds * - statsFlushSeconds + * This default configuration is provided by the native layer * * @param defaultConfigurationYAML the default template configuration. * @return String, the resolved template. diff --git a/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt b/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt index 134018bf1e..979883580d 100644 --- a/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt +++ b/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt @@ -45,8 +45,7 @@ class EnvoyBuilderTest { @Test fun `specifying connection timeout overrides default`() { - `when`(envoyConfiguration.templateString()).thenReturn(TEST_CONFIG) - builder = EnvoyBuilder(envoyConfiguration) + builder = EnvoyBuilder() builder.addEngineType { engine } builder.addConnectTimeoutSeconds(1234) @@ -56,8 +55,7 @@ class EnvoyBuilderTest { @Test fun `specifying DNS refresh overrides default`() { - `when`(envoyConfiguration.templateString()).thenReturn(TEST_CONFIG) - builder = EnvoyBuilder(envoyConfiguration) + builder = EnvoyBuilder() builder.addEngineType { engine } builder.addDNSRefreshSeconds(1234) @@ -67,8 +65,7 @@ class EnvoyBuilderTest { @Test fun `specifying stats flush overrides default`() { - `when`(envoyConfiguration.templateString()).thenReturn(TEST_CONFIG) - builder = EnvoyBuilder(envoyConfiguration) + builder = EnvoyBuilder() builder.addEngineType { engine } builder.addStatsFlushSeconds(1234) From f94cb71772917cbee0a2a2917ad939a8f569d1c9 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Tue, 3 Sep 2019 12:55:40 -0700 Subject: [PATCH 10/17] period Signed-off-by: Alan Chiu --- .../io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java index ed1820be98..4a83c77856 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java @@ -39,7 +39,7 @@ public EnvoyConfiguration(int connectTimeoutSeconds, int dnsRefreshSeconds, * - connectTimeoutSeconds * - dnsRefreshSeconds * - statsFlushSeconds - * This default configuration is provided by the native layer + * This default configuration is provided by the native layer. * * @param defaultConfigurationYAML the default template configuration. * @return String, the resolved template. From bf2bca687e70f4436eb7037e2bf6dde41b5a4a79 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Tue, 3 Sep 2019 13:05:04 -0700 Subject: [PATCH 11/17] spellcheck Signed-off-by: Alan Chiu --- .../io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt b/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt index 3d11caa260..72376636c4 100644 --- a/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt +++ b/library/java/test/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt @@ -14,7 +14,7 @@ mock_template: class EnvoyConfigurationTest { @Test - fun `resolving with default configuraiton resolves with values`() { + fun `resolving with default configuration resolves with values`() { val envoyConfiguration = EnvoyConfiguration(123, 234, 345) val resolvedTemplate = envoyConfiguration.resolveTemplate(TEST_CONFIG) From af59361343b7a5e819f4493192ae77eca2390b7a Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Tue, 3 Sep 2019 13:27:08 -0700 Subject: [PATCH 12/17] fix things Signed-off-by: Alan Chiu --- .../envoymobile/engine/EnvoyEngine.java | 6 ++--- .../envoymobile/engine/EnvoyEngineImpl.java | 24 ++++++++----------- .../src/io/envoyproxy/envoymobile/Envoy.kt | 16 +++++++++---- .../io/envoyproxy/envoymobile/EnvoyBuilder.kt | 8 +++---- .../envoymobile/EnvoyBuilderTest.kt | 8 +++---- 5 files changed, 32 insertions(+), 30 deletions(-) diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngine.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngine.java index aacb67b4b3..351aa988fc 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngine.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngine.java @@ -14,15 +14,15 @@ public interface EnvoyEngine { /** * Run the Envoy engine with the provided envoyConfiguration and log level. * - * @param config The configuration file with which to start Envoy. + * @param configurationYAML The configuration yaml with which to start Envoy. * @return A status indicating if the action was successful. */ - int runWithConfig(EnvoyConfiguration envoyConfiguration); + int runWithConfig(String configurationYAML, String logLevel); /** * Run the Envoy engine with the provided envoyConfiguration and log level. * - * @param config The configuration file with which to start Envoy. + * @param envoyConfiguration The EnvoyConfiguration used to start Envoy. * @param logLevel The log level to use when starting Envoy. * @return int A status indicating if the action was successful. */ diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java index 70e53d1c7c..ba1cad1916 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java @@ -26,32 +26,28 @@ public EnvoyHTTPStream startStream(EnvoyObserver observer) { /** * Run the Envoy engine with the provided envoyConfiguration and log level. * - * @param envoyConfiguration The configuration file with which to start Envoy. + * @param configurationYAML The configuration yaml with which to start Envoy. * @return A status indicating if the action was successful. */ @Override - public int runWithConfig(EnvoyConfiguration envoyConfiguration) { - return runWithConfig(envoyConfiguration, "info"); + public int runWithConfig(String configurationYAML, String logLevel) { + try { + return JniLibrary.runEngine(configurationYAML, logLevel); + } catch (Throwable throwable) { + // TODO: Need to have a way to log the exception somewhere + return 1; + } } /** * Run the Envoy engine with the provided envoyConfiguration and log level. * - * @param envoyConfiguration The configuration file with which to start Envoy. + * @param envoyConfiguration The EnvoyConfiguration used to start Envoy. * @param logLevel The log level to use when starting Envoy. * @return int A status indicating if the action was successful. */ @Override public int runWithConfig(EnvoyConfiguration envoyConfiguration, String logLevel) { - try { - - String resolvedConfig = envoyConfiguration.configYAML == null - ? envoyConfiguration.resolveTemplate(JniLibrary.templateString()) - : envoyConfiguration.configYAML; - return JniLibrary.runEngine(resolvedConfig, logLevel); - } catch (Throwable throwable) { - // TODO: Need to have a way to log the exception somewhere - return 1; - } + return runWithConfig(envoyConfiguration.resolveTemplate(JniLibrary.templateString()), logLevel); } } diff --git a/library/kotlin/src/io/envoyproxy/envoymobile/Envoy.kt b/library/kotlin/src/io/envoyproxy/envoymobile/Envoy.kt index 62ba780f40..c6b35400a8 100644 --- a/library/kotlin/src/io/envoyproxy/envoymobile/Envoy.kt +++ b/library/kotlin/src/io/envoyproxy/envoymobile/Envoy.kt @@ -20,17 +20,23 @@ enum class LogLevel(internal val level: String) { /** * Wrapper class that allows for easy calling of Envoy's JNI interface in native Java. */ -class Envoy constructor( +class Envoy private constructor( private val engine: EnvoyEngine, - internal val envoyConfiguration: EnvoyConfiguration, - internal val logLevel: LogLevel = LogLevel.INFO + internal val envoyConfiguration: EnvoyConfiguration?, + internal val configurationYAML: String?, + internal val logLevel: LogLevel ) : Client { - constructor(engine: EnvoyEngine, envoyConfiguration: EnvoyConfiguration) : this(engine, envoyConfiguration, LogLevel.INFO) + constructor(engine: EnvoyEngine, envoyConfiguration: EnvoyConfiguration, logLevel: LogLevel = LogLevel.INFO) : this(engine, envoyConfiguration, null, logLevel) + constructor(engine: EnvoyEngine, configurationYAML: String, logLevel: LogLevel = LogLevel.INFO) : this(engine, null, configurationYAML, logLevel) // Dedicated thread for running this instance of Envoy. private val runner: Thread = Thread(ThreadGroup("Envoy"), Runnable { - engine.runWithConfig(envoyConfiguration, logLevel.level) + if (envoyConfiguration == null) { + engine.runWithConfig(configurationYAML, logLevel.level) + }else { + engine.runWithConfig(envoyConfiguration, logLevel.level) + } }) /** diff --git a/library/kotlin/src/io/envoyproxy/envoymobile/EnvoyBuilder.kt b/library/kotlin/src/io/envoyproxy/envoymobile/EnvoyBuilder.kt index cae9d8b43b..0f24258e2f 100644 --- a/library/kotlin/src/io/envoyproxy/envoymobile/EnvoyBuilder.kt +++ b/library/kotlin/src/io/envoyproxy/envoymobile/EnvoyBuilder.kt @@ -71,12 +71,12 @@ open class EnvoyBuilder internal constructor( * @return A new instance of Envoy. */ fun build(): Envoy { - val config = if (configYAML == null) { - EnvoyConfiguration(connectTimeoutSeconds, dnsRefreshSeconds, statsFlushSeconds) + val configurationYAML = configYAML + if (configurationYAML == null) { + return Envoy(engineType(), EnvoyConfiguration(connectTimeoutSeconds, dnsRefreshSeconds, statsFlushSeconds), logLevel) } else { - EnvoyConfiguration(configYAML) + return Envoy(engineType(), configurationYAML, logLevel) } - return Envoy(engineType(), config, logLevel) } /** diff --git a/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt b/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt index 979883580d..eb18f3ef0e 100644 --- a/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt +++ b/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyBuilderTest.kt @@ -29,7 +29,7 @@ class EnvoyBuilderTest { builder.addConfigYAML("mock_template:") val envoy = builder.build() - assertThat(envoy.envoyConfiguration.configYAML).isEqualTo("mock_template:") + assertThat(envoy.configurationYAML).isEqualTo("mock_template:") } @Test @@ -50,7 +50,7 @@ class EnvoyBuilderTest { builder.addConnectTimeoutSeconds(1234) val envoy = builder.build() - assertThat(envoy.envoyConfiguration.connectTimeoutSeconds).isEqualTo(1234) + assertThat(envoy.envoyConfiguration!!.connectTimeoutSeconds).isEqualTo(1234) } @Test @@ -60,7 +60,7 @@ class EnvoyBuilderTest { builder.addDNSRefreshSeconds(1234) val envoy = builder.build() - assertThat(envoy.envoyConfiguration.dnsRefreshSeconds).isEqualTo(1234) + assertThat(envoy.envoyConfiguration!!.dnsRefreshSeconds).isEqualTo(1234) } @Test @@ -71,7 +71,7 @@ class EnvoyBuilderTest { builder.addStatsFlushSeconds(1234) builder.build() val envoy = builder.build() - assertThat(envoy.envoyConfiguration.statsFlushSeconds).isEqualTo(1234) + assertThat(envoy.envoyConfiguration!!.statsFlushSeconds).isEqualTo(1234) } } From 075ddea817243a6828dd70b59b451e43973599be Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Wed, 4 Sep 2019 09:14:21 -0700 Subject: [PATCH 13/17] missed deletion Signed-off-by: Alan Chiu --- .../envoymobile/engine/EnvoyConfiguration.java | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java index 4a83c77856..5a3867c136 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java @@ -2,23 +2,10 @@ public class EnvoyConfiguration { - public final String configYAML; public final Integer connectTimeoutSeconds; public final Integer dnsRefreshSeconds; public final Integer statsFlushSeconds; - /** - * Create an EnvoyConfiguration with a user provided configuration YAML. - * - * @param configYAML configuration to be used for starting Envoy. - */ - public EnvoyConfiguration(String configYAML) { - this.configYAML = configYAML; - this.connectTimeoutSeconds = null; - this.dnsRefreshSeconds = null; - this.statsFlushSeconds = null; - } - /** * Create an EnvoyConfiguration with a user provided configuration values. * @@ -28,7 +15,6 @@ public EnvoyConfiguration(String configYAML) { */ public EnvoyConfiguration(int connectTimeoutSeconds, int dnsRefreshSeconds, int statsFlushSeconds) { - this.configYAML = null; this.connectTimeoutSeconds = connectTimeoutSeconds; this.dnsRefreshSeconds = dnsRefreshSeconds; this.statsFlushSeconds = statsFlushSeconds; From fdf2442b6a5f3fdea750afcc0f128479eed9db72 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Wed, 4 Sep 2019 09:27:38 -0700 Subject: [PATCH 14/17] pr feedback Signed-off-by: Alan Chiu --- .../envoymobile/engine/EnvoyConfiguration.java | 11 ++++------- .../io/envoyproxy/envoymobile/engine/EnvoyEngine.java | 4 ++-- .../envoymobile/engine/EnvoyEngineImpl.java | 2 +- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java index 5a3867c136..e15f78658d 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java @@ -21,19 +21,16 @@ public EnvoyConfiguration(int connectTimeoutSeconds, int dnsRefreshSeconds, } /** - * Resolves the provided default configuration with the field values: - * - connectTimeoutSeconds - * - dnsRefreshSeconds - * - statsFlushSeconds + * Resolves the provided configuration template using properties on this configuration. * This default configuration is provided by the native layer. * - * @param defaultConfigurationYAML the default template configuration. + * @param templateYAML the default template configuration. * @return String, the resolved template. * @throws ConfigurationException, when the template provided is not fully resolved. */ - String resolveTemplate(String defaultConfigurationYAML) { + String resolveTemplate(String templateYAML) { String resolvedConfiguration = - defaultConfigurationYAML + templateYAML .replace("{{ connect_timeout }}", String.format("%ss", connectTimeoutSeconds)) .replace("{{ dns_refresh_rate }}", String.format("%ss", dnsRefreshSeconds)) .replace("{{ stats_flush_interval }}", String.format("%ss", statsFlushSeconds)); diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngine.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngine.java index 351aa988fc..785fdfc70e 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngine.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngine.java @@ -12,7 +12,7 @@ public interface EnvoyEngine { EnvoyHTTPStream startStream(EnvoyObserver observer); /** - * Run the Envoy engine with the provided envoyConfiguration and log level. + * Run the Envoy engine with the provided yaml string and log level. * * @param configurationYAML The configuration yaml with which to start Envoy. * @return A status indicating if the action was successful. @@ -20,7 +20,7 @@ public interface EnvoyEngine { int runWithConfig(String configurationYAML, String logLevel); /** - * Run the Envoy engine with the provided envoyConfiguration and log level. + * Run the Envoy engine with the provided EnvoyConfiguration and log level. * * @param envoyConfiguration The EnvoyConfiguration used to start Envoy. * @param logLevel The log level to use when starting Envoy. diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java index ba1cad1916..6e8cfa6667 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java @@ -24,7 +24,7 @@ public EnvoyHTTPStream startStream(EnvoyObserver observer) { } /** - * Run the Envoy engine with the provided envoyConfiguration and log level. + * Run the Envoy engine with the provided yaml string and log level. * * @param configurationYAML The configuration yaml with which to start Envoy. * @return A status indicating if the action was successful. From 9e326dd1eecc09305f21231e930cf9d72842dead Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Wed, 4 Sep 2019 10:46:43 -0700 Subject: [PATCH 15/17] cool Signed-off-by: Alan Chiu --- .../io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java index e15f78658d..dda717e2e0 100644 --- a/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java +++ b/library/java/src/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java @@ -30,8 +30,7 @@ public EnvoyConfiguration(int connectTimeoutSeconds, int dnsRefreshSeconds, */ String resolveTemplate(String templateYAML) { String resolvedConfiguration = - templateYAML - .replace("{{ connect_timeout }}", String.format("%ss", connectTimeoutSeconds)) + templateYAML.replace("{{ connect_timeout }}", String.format("%ss", connectTimeoutSeconds)) .replace("{{ dns_refresh_rate }}", String.format("%ss", dnsRefreshSeconds)) .replace("{{ stats_flush_interval }}", String.format("%ss", statsFlushSeconds)); From 1dddd8e2f2fbdb94eaaca5c4b6ca0b4a1ffd6158 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Wed, 4 Sep 2019 13:24:16 -0700 Subject: [PATCH 16/17] fix Signed-off-by: Alan Chiu --- library/kotlin/src/io/envoyproxy/envoymobile/ResponseHandler.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/kotlin/src/io/envoyproxy/envoymobile/ResponseHandler.kt b/library/kotlin/src/io/envoyproxy/envoymobile/ResponseHandler.kt index 0ab40afc2b..8c533c6b28 100644 --- a/library/kotlin/src/io/envoyproxy/envoymobile/ResponseHandler.kt +++ b/library/kotlin/src/io/envoyproxy/envoymobile/ResponseHandler.kt @@ -17,7 +17,7 @@ class ResponseHandler(val executor: Executor) { override fun getExecutor(): Executor = responseHandler.executor override fun onHeaders(headers: Map>, endStream: Boolean) { - val statusCode = headers!![":status"]?.first()?.toIntOrNull() ?: 0 + val statusCode = headers[":status"]?.first()?.toIntOrNull() ?: 0 responseHandler.onHeadersClosure(headers, statusCode, endStream) } From 3cdc7d5a771fbc94a2d4ce3c3311bb7190e571b2 Mon Sep 17 00:00:00 2001 From: Alan Chiu Date: Wed, 4 Sep 2019 13:29:17 -0700 Subject: [PATCH 17/17] fix Signed-off-by: Alan Chiu --- library/kotlin/test/io/envoyproxy/envoymobile/EnvoyTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyTest.kt b/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyTest.kt index 08ba2da627..8ef1a161f8 100644 --- a/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyTest.kt +++ b/library/kotlin/test/io/envoyproxy/envoymobile/EnvoyTest.kt @@ -13,13 +13,13 @@ import org.mockito.Mockito.`when` import org.mockito.Mockito.mock import org.mockito.Mockito.verify import java.nio.ByteBuffer -import java.util.concurrent.Executor; +import java.util.concurrent.Executor class EnvoyTest { private val engine = mock(EnvoyEngine::class.java) private val stream = mock(EnvoyHTTPStream::class.java) - private val config = EnvoyConfiguration("") + private val config = EnvoyConfiguration(0,0,0) @Test fun `starting a stream on envoy sends headers`() {