From 78dae309ef33a72996653a59cbf4dd74443d1b1b Mon Sep 17 00:00:00 2001 From: Marcin Falkowski Date: Tue, 5 Jan 2021 15:29:11 +0100 Subject: [PATCH 01/11] use allegro/envoy fork (#237) --- .../servicemesh/envoycontrol/config/envoy/EnvoyContainer.kt | 2 +- tools/envoy/Dockerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyContainer.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyContainer.kt index 1116cf856..9df564fa2 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyContainer.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyContainer.kt @@ -32,7 +32,7 @@ class EnvoyContainer( const val EGRESS_LISTENER_CONTAINER_PORT = 5000 const val INGRESS_LISTENER_CONTAINER_PORT = 5001 - const val DEFAULT_IMAGE = "marcinfalkowski/envoy-dev:v1.16.1-dev-lua-segfault-fix-1-16-0-backport-20201118-df9dc819" + const val DEFAULT_IMAGE = "allegro/envoy-dev:v1.16.1-dev-lua-segfault-fix-1-16-0-backport-20201118-df9dc819" private const val ADMIN_PORT = 10000 } diff --git a/tools/envoy/Dockerfile b/tools/envoy/Dockerfile index 7b6d0312d..a6f9678d0 100644 --- a/tools/envoy/Dockerfile +++ b/tools/envoy/Dockerfile @@ -1,4 +1,4 @@ -FROM marcinfalkowski/envoy-dev:v1.16.1-dev-lua-segfault-fix-1-16-0-backport-20201118-df9dc819 +FROM allegro/envoy-dev:v1.16.1-dev-lua-segfault-fix-1-16-0-backport-20201118-df9dc819 ENV PORT=9999:9999 ENV PORT=80:80 From 78d1807e155a8f9fac54b82a963021a3794ae4be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20S=C5=82onka?= Date: Fri, 8 Jan 2021 11:41:15 +0100 Subject: [PATCH 02/11] Fix overlapping paths in incoming permissions (#238) * reproduced error * Make actual rules contain log endpoints with any principal * Remove unused variables * Add braces * Change unlistedAndLoggedEndpointsPolicies via Marcin suggestion * Add flag to disable/enable fix for overlapping paths --- docs/configuration.md | 1 + .../snapshot/SnapshotProperties.kt | 1 + .../listeners/filters/RBACFilterFactory.kt | 15 +- .../filters/RBACFilterFactoryTest.kt | 49 ++++++- .../ClientNameTrustedHeaderTest.kt | 3 +- ...IncomingPermissionsDisabledInClientTest.kt | 3 +- .../IncomingPermissionsEmptyClientsTest.kt | 3 +- .../IncomingPermissionsEmptyEndpointsTest.kt | 3 +- .../IncomingPermissionsLoggingModeTest.kt | 1 + ...IncomingPermissionsOverlappingPathsTest.kt | 137 ++++++++++++++++++ .../IncomingPermissionsPathMatchingTest.kt | 3 +- .../IncomingPermissionsRequestIdTest.kt | 1 + .../SourceIpBasedAuthenticationTest.kt | 1 + .../StatusRouteIncomingPermissionsTest.kt | 1 + .../permissions/TlsBasedAuthenticationTest.kt | 1 + .../permissions/TlsClientCertRequiredTest.kt | 1 + 16 files changed, 210 insertions(+), 14 deletions(-) create mode 100644 envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsOverlappingPathsTest.kt diff --git a/docs/configuration.md b/docs/configuration.md index 086e3be69..d0a9eea7d 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -86,6 +86,7 @@ Property **envoy-control.envoy.snapshot.incoming-permissions.tls-authentication.service-name-wildcard-regex** | Regex to match service-names for "wildcard" client identifier. By default it will match all service names of length greater than zero (.+). It is used in place of {service-name} placeholder in san-uri-format. | .+ **envoy-control.envoy.snapshot.incoming-permissions.tls-authentication.services-allowed-to-use-wildcard** | Services that are allowed to have wildcard in incoming.clients field | empty set **envoy-control.envoy.snapshot.incoming-permissions.tls-authentication.wildcard-client-identifier** | Special value (wildcard) that signifies that the service accepts traffic from all other validated services | * +**envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix** | Make RBAC factory generate rules for endpoints with log policy in actual "rules" of RBAC engine to fix unintuitive behaviour when overlapping paths are defined. | false **envoy-control.envoy.snapshot.outgoing-permissions.enabled** | Enable outgoing permissions | false **envoy-control.envoy.snapshot.outgoing-permissions.all-services-dependencies.identifier** | Special value (wildcard) that signifies that the service depends on all other services | * **envoy-control.envoy.snapshot.outgoing-permissions.all-services-dependencies.not-included-by-prefix** | Services not included in dependencies for services with wildcard in outgoing.dependency field. Matched by service name prefix. | empty list diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt index 17baa5ffa..82ff97722 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt @@ -73,6 +73,7 @@ class IncomingPermissionsProperties { var sourceIpAuthentication = SourceIpAuthenticationProperties() var selectorMatching: MutableMap = mutableMapOf() var tlsAuthentication = TlsAuthenticationProperties() + var overlappingPathsFix = false // TODO: to be removed when proved it did not mess up anything } class SelectorMatching { diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterFactory.kt index ba24bc521..3a61a5dd0 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterFactory.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterFactory.kt @@ -52,6 +52,7 @@ class RBACFilterFactory( companion object { private val logger by logger() private const val ALLOW_UNLISTED_POLICY_NAME = "ALLOW_UNLISTED_POLICY" + private const val ALLOW_LOGGED_POLICY_NAME = "ALLOW_LOGGED_POLICY" private const val STATUS_ROUTE_POLICY_NAME = "STATUS_ALLOW_ALL_POLICY" private val EXACT_IP_MASK = UInt32Value.of(32) } @@ -62,7 +63,6 @@ class RBACFilterFactory( data class EndpointWithPolicy(val endpoint: IncomingEndpoint, val policy: Policy.Builder) private fun getIncomingEndpointPolicies( - serviceName: String, incomingPermissions: Incoming, snapshot: GlobalSnapshot, roles: List @@ -86,14 +86,13 @@ class RBACFilterFactory( data class Rules(val shadowRules: RBAC.Builder, val actualRules: RBAC.Builder) private fun getRules( - serviceName: String, incomingPermissions: Incoming, snapshot: GlobalSnapshot, roles: List ): Rules { val incomingEndpointsPolicies = getIncomingEndpointPolicies( - serviceName, incomingPermissions, snapshot, roles + incomingPermissions, snapshot, roles ) val restrictedEndpointsPolicies = incomingEndpointsPolicies.asSequence() @@ -132,7 +131,12 @@ class RBACFilterFactory( loggedEndpointsPolicies: Iterable ): Map { return if (incomingPermissions.unlistedEndpointsPolicy == Incoming.UnlistedPolicy.LOG) { - allowUnlistedEndpointsPolicy(restrictedEndpointsPolicies) + if (incomingPermissionsProperties.overlappingPathsFix) { + allowLoggedEndpointsPolicy(loggedEndpointsPolicies) + + allowUnlistedEndpointsPolicy(restrictedEndpointsPolicies) + } else { + allowUnlistedEndpointsPolicy(restrictedEndpointsPolicies) + } } else { allowLoggedEndpointsPolicy(loggedEndpointsPolicies) } @@ -161,7 +165,7 @@ class RBACFilterFactory( return mapOf() } - return mapOf(ALLOW_UNLISTED_POLICY_NAME to Policy.newBuilder() + return mapOf(ALLOW_LOGGED_POLICY_NAME to Policy.newBuilder() .addPrincipals(anyPrincipal) .addPermissions(anyOf(allLoggedEndpointsPermissions)) .build() @@ -322,7 +326,6 @@ class RBACFilterFactory( fun createHttpFilter(group: Group, snapshot: GlobalSnapshot): HttpFilter? { return if (incomingPermissionsProperties.enabled && group.proxySettings.incoming.permissionsEnabled) { val rules = getRules( - group.serviceName, group.proxySettings.incoming, snapshot, group.proxySettings.incoming.roles diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterFactoryTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterFactoryTest.kt index dde981914..f9d1755b5 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterFactoryTest.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterFactoryTest.kt @@ -31,7 +31,10 @@ import pl.allegro.tech.servicemesh.envoycontrol.snapshot.StatusRouteProperties @Suppress("LargeClass") // TODO: https://github.com/allegro/envoy-control/issues/121 internal class RBACFilterFactoryTest { private val rbacFilterFactory = RBACFilterFactory( - IncomingPermissionsProperties().also { it.enabled = true }, + IncomingPermissionsProperties().also { + it.enabled = true + it.overlappingPathsFix = true + }, StatusRouteProperties() ) private val rbacFilterFactoryWithSourceIpAuth = RBACFilterFactory( @@ -542,7 +545,7 @@ internal class RBACFilterFactoryTest { val expectedActual = """ { "policies": { - "ALLOW_UNLISTED_POLICY": { + "ALLOW_LOGGED_POLICY": { "permissions": [{ "or_rules": { "rules": [{ @@ -1042,6 +1045,46 @@ internal class RBACFilterFactoryTest { private val expectedAnyPermissionJson = """ { "policies": { + "ALLOW_LOGGED_POLICY": { + "permissions": [{ + "or_rules": { + "rules": [ + { + "and_rules": { + "rules": [ + { + "url_path": { + "path": { + "exact": "/example" + } + } + }, + { + "or_rules": { + "rules": [ + { + "header": { + "name": ":method", + "exact_match": "GET" + } + }, + { + "header": { + "name": ":method", + "exact_match": "POST" + } + } + ] + } + } + ] + } + } + ] + } + }], + "principals": [ $anyTrue ] + }, "ALLOW_UNLISTED_POLICY": { "permissions": [ $anyTrue @@ -1072,7 +1115,7 @@ internal class RBACFilterFactoryTest { """ private val expectedUnlistedClientsPermissions = """{ "policies": { - "ALLOW_UNLISTED_POLICY": { + "ALLOW_LOGGED_POLICY": { "permissions": [ { "or_rules": { diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/ClientNameTrustedHeaderTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/ClientNameTrustedHeaderTest.kt index 720a5fbb0..0646dbc46 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/ClientNameTrustedHeaderTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/ClientNameTrustedHeaderTest.kt @@ -32,7 +32,8 @@ class ClientNameTrustedHeaderTest { "envoy-control.envoy.snapshot.incoming-permissions.tls-authentication.require-client-certificate" to false, "envoy-control.envoy.snapshot.incoming-permissions.trusted-client-identity-header" to "x-client-name-trusted", "envoy-control.envoy.snapshot.incoming-permissions.tls-authentication.san-uri-format" to "spiffe://{service-name}", - "envoy-control.envoy.snapshot.incoming-permissions.tls-authentication.service-name-wildcard-regex" to ".+" + "envoy-control.envoy.snapshot.incoming-permissions.tls-authentication.service-name-wildcard-regex" to ".+", + "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true )) @JvmField diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsDisabledInClientTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsDisabledInClientTest.kt index 63b54cf8f..7de1a00e3 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsDisabledInClientTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsDisabledInClientTest.kt @@ -13,7 +13,8 @@ internal class IncomingPermissionsDisabledInClientTest : EnvoyControlTestConfigu companion object { private val properties = mapOf( - "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true + "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, + "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true ) @JvmStatic diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsEmptyClientsTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsEmptyClientsTest.kt index 8b6afd5d8..280b68670 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsEmptyClientsTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsEmptyClientsTest.kt @@ -17,7 +17,8 @@ import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyContainer internal class IncomingPermissionsEmptyClientsTest : EnvoyControlTestConfiguration() { companion object { private val properties = mapOf( - "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true + "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, + "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true ) // language=yaml diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsEmptyEndpointsTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsEmptyEndpointsTest.kt index 662bd7875..937d59523 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsEmptyEndpointsTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsEmptyEndpointsTest.kt @@ -15,7 +15,8 @@ import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyContainer internal class IncomingPermissionsEmptyEndpointsTest : EnvoyControlTestConfiguration() { companion object { private val properties = mapOf( - "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true + "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, + "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true ) // language=yaml diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsLoggingModeTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsLoggingModeTest.kt index a1cb2d5e1..b168ffffb 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsLoggingModeTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsLoggingModeTest.kt @@ -28,6 +28,7 @@ internal class IncomingPermissionsLoggingModeTest : EnvoyControlTestConfiguratio private const val prefix = "envoy-control.envoy.snapshot" private val properties = { sourceClientIp: String -> mapOf( "$prefix.incoming-permissions.enabled" to true, + "$prefix.incoming-permissions.overlapping-paths-fix" to true, "$prefix.incoming-permissions.source-ip-authentication.ip-from-range.source-ip-client" to "$sourceClientIp/32", "$prefix.routes.status.create-virtual-cluster" to true, diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsOverlappingPathsTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsOverlappingPathsTest.kt new file mode 100644 index 000000000..2eca56187 --- /dev/null +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsOverlappingPathsTest.kt @@ -0,0 +1,137 @@ +package pl.allegro.tech.servicemesh.envoycontrol.permissions + +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.RegisterExtension +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk +import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted +import pl.allegro.tech.servicemesh.envoycontrol.config.Echo1EnvoyAuthConfig +import pl.allegro.tech.servicemesh.envoycontrol.config.Echo2EnvoyAuthConfig +import pl.allegro.tech.servicemesh.envoycontrol.config.Echo3EnvoyAuthConfig +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension + +class IncomingPermissionsOverlappingPathsTest { + + companion object { + + // language=yaml + private val echoYaml = """ + node: + metadata: + proxy_settings: + incoming: + unlistedEndpointsPolicy: log + endpoints: + - pathRegex: "/a/b/.+" + clients: [ echo3 ] + unlistedClientsPolicy: log + - pathRegex: "/a/.+" + clients: [ echo3 ] + unlistedClientsPolicy: blockAndLog + outgoing: + dependencies: [] + """.trimIndent() + + // language=yaml + private val echo3Yaml = """ + node: + metadata: + proxy_settings: + incoming: + unlistedEndpointsPolicy: blockAndLog + endpoints: + - pathRegex: "/a/b/.+" + clients: [ echo3 ] + unlistedClientsPolicy: log + - pathRegex: "/a/.+" + clients: [ echo3 ] + unlistedClientsPolicy: blockAndLog + outgoing: + dependencies: [] + """.trimIndent() + + // language=yaml + private val echo2Yaml = """ + node: + metadata: + proxy_settings: + outgoing: + dependencies: + - service: "echo" + - service: "echo3" + """.trimIndent() + + private val echoConfig = Echo1EnvoyAuthConfig.copy(configOverride = echoYaml) + private val echo2Config = Echo2EnvoyAuthConfig.copy(configOverride = echo2Yaml) + private val echo3Config = Echo3EnvoyAuthConfig.copy(configOverride = echo3Yaml) + + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension( + consul, mapOf( + "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, + "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true + ) + ) + + @JvmField + @RegisterExtension + val echoService = EchoServiceExtension() + + @JvmField + @RegisterExtension + val echoEnvoy = EnvoyExtension(envoyControl, config = echoConfig, localService = echoService) + + @JvmField + @RegisterExtension + val echo2Envoy = EnvoyExtension(envoyControl, config = echo2Config) + + @JvmField + @RegisterExtension + val echo3Envoy = EnvoyExtension(envoyControl, config = echo3Config, localService = echoService) + } + + @BeforeEach + fun beforeEach() { + consul.server.operations.registerServiceWithEnvoyOnIngress( + echoEnvoy, + name = "echo", + tags = listOf("mtls:enabled") + ) + consul.server.operations.registerServiceWithEnvoyOnIngress( + echo3Envoy, + name = "echo3", + tags = listOf("mtls:enabled") + ) + waitForEnvoysInitialized() + } + + private fun waitForEnvoysInitialized() { + untilAsserted { + assertThat(echo2Envoy.container.admin().isEndpointHealthy("echo", echoEnvoy.container.ipAddress())).isTrue() + assertThat(echo2Envoy.container.admin().isEndpointHealthy("echo3", echo3Envoy.container.ipAddress())).isTrue() + } + } + + @Test + fun `should allow defining endpoints with policy log that are subset of blockAndLog when unlinstedEnpointsPolicy is log`() { + // expect + val response = echo2Envoy.egressOperations.callService(service = "echo", pathAndQuery = "/a/b/c") + assertThat(response).isOk() + } + + @Test + fun `should allow defining endpoints with policy log that are subset of blockAndLog when unlinstedEnpointsPolicy is blockAndLog`() { + // expect + val response = echo2Envoy.egressOperations.callService(service = "echo3", pathAndQuery = "/a/b/c") + assertThat(response).isOk() + } +} diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsPathMatchingTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsPathMatchingTest.kt index 202b27639..221fca022 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsPathMatchingTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsPathMatchingTest.kt @@ -57,7 +57,8 @@ class IncomingPermissionsPathMatchingTest { @JvmField @RegisterExtension val envoyControl = EnvoyControlExtension(consul, mapOf( - "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true + "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, + "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true )) @JvmField diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsRequestIdTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsRequestIdTest.kt index 61817433c..83a11fc4f 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsRequestIdTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsRequestIdTest.kt @@ -43,6 +43,7 @@ class IncomingPermissionsRequestIdTest { @RegisterExtension val envoyControl = EnvoyControlExtension(consul, mapOf( "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, + "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true, "envoy-control.envoy.snapshot.incoming-permissions.request-identification-headers" to listOf("x-request-id") )) diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/SourceIpBasedAuthenticationTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/SourceIpBasedAuthenticationTest.kt index 6822cfd17..59c619f3f 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/SourceIpBasedAuthenticationTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/SourceIpBasedAuthenticationTest.kt @@ -25,6 +25,7 @@ internal class SourceIpBasedAuthenticationTest : EnvoyControlTestConfiguration() private const val prefix = "envoy-control.envoy.snapshot" private val properties = { mapOf( "$prefix.incoming-permissions.enabled" to true, + "$prefix.incoming-permissions.overlapping-paths-fix" to true, "$prefix.outgoing-permissions.services-allowed-to-use-wildcard" to setOf("echo"), "$prefix.incoming-permissions.source-ip-authentication.ip-from-service-discovery.enabled-for-incoming-services" to listOf("echo"), diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/StatusRouteIncomingPermissionsTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/StatusRouteIncomingPermissionsTest.kt index 1a29c51df..79647c633 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/StatusRouteIncomingPermissionsTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/StatusRouteIncomingPermissionsTest.kt @@ -14,6 +14,7 @@ internal class StatusRouteIncomingPermissionsTest : EnvoyControlTestConfiguratio private val properties = mapOf( "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, + "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true, "envoy-control.envoy.snapshot.routes.status.create-virtual-cluster" to true, "envoy-control.envoy.snapshot.routes.status.endpoints" to mutableListOf(EndpointMatch().also { it.path = "/status/" }), "envoy-control.envoy.snapshot.routes.status.enabled" to true diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/TlsBasedAuthenticationTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/TlsBasedAuthenticationTest.kt index 27e12b5f5..04b569b1d 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/TlsBasedAuthenticationTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/TlsBasedAuthenticationTest.kt @@ -31,6 +31,7 @@ internal class TlsBasedAuthenticationTest { private val ecProperties = mapOf( "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, + "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true, "envoy-control.envoy.snapshot.incoming-permissions.tls-authentication.services-allowed-to-use-wildcard" to listOf("echo3"), "envoy-control.envoy.snapshot.outgoing-permissions.services-allowed-to-use-wildcard" to setOf("echo"), "envoy-control.envoy.snapshot.routes.status.create-virtual-cluster" to true, diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/TlsClientCertRequiredTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/TlsClientCertRequiredTest.kt index 718b041be..91e2a189b 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/TlsClientCertRequiredTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/TlsClientCertRequiredTest.kt @@ -24,6 +24,7 @@ class TlsClientCertRequiredTest { @RegisterExtension val envoyControl = EnvoyControlExtension(consul, mapOf( "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, + "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true, "envoy-control.envoy.snapshot.incoming-permissions.tls-authentication.require-client-certificate" to true, "envoy-control.envoy.snapshot.outgoing-permissions.services-allowed-to-use-wildcard" to setOf("echo") )) From 8e2d044465245ecf6d45e8280197b18fc1f000ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Dziedziak?= Date: Mon, 11 Jan 2021 12:34:15 +0100 Subject: [PATCH 03/11] Remove deprecated field for v3 api (#234) Co-authored-by: Lukasz Dziedziak --- .../snapshot/EnvoySnapshotFactory.kt | 3 ++- .../routes/EnvoyEgressRoutesFactory.kt | 19 +++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt index 75feb93f1..50b35de74 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt @@ -227,7 +227,8 @@ class EnvoySnapshotFactory( val routes = listOf( egressRoutesFactory.createEgressRouteConfig( group.serviceName, egressRouteSpecification, - group.listenersConfig?.addUpstreamExternalAddressHeader ?: false + group.listenersConfig?.addUpstreamExternalAddressHeader ?: false, + group.version ), ingressRoutesFactory.createSecuredIngressRouteConfig(group.proxySettings) ) diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyEgressRoutesFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyEgressRoutesFactory.kt index 77ffc6bab..8db944dae 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyEgressRoutesFactory.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyEgressRoutesFactory.kt @@ -5,11 +5,13 @@ import io.envoyproxy.controlplane.cache.TestResources import io.envoyproxy.envoy.config.core.v3.HeaderValue import io.envoyproxy.envoy.config.core.v3.HeaderValueOption import io.envoyproxy.envoy.config.route.v3.DirectResponseAction +import io.envoyproxy.envoy.config.route.v3.InternalRedirectPolicy import io.envoyproxy.envoy.config.route.v3.Route import io.envoyproxy.envoy.config.route.v3.RouteAction import io.envoyproxy.envoy.config.route.v3.RouteConfiguration import io.envoyproxy.envoy.config.route.v3.RouteMatch import io.envoyproxy.envoy.config.route.v3.VirtualHost +import pl.allegro.tech.servicemesh.envoycontrol.groups.ResourceVersion import pl.allegro.tech.servicemesh.envoycontrol.snapshot.RouteSpecification import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SnapshotProperties @@ -67,7 +69,8 @@ class EnvoyEgressRoutesFactory( fun createEgressRouteConfig( serviceName: String, routes: Collection, - addUpstreamAddressHeader: Boolean + addUpstreamAddressHeader: Boolean, + resourceVersion: ResourceVersion = ResourceVersion.V3 ): RouteConfiguration { val virtualHosts = routes.map { routeSpecification -> VirtualHost.newBuilder() @@ -81,7 +84,7 @@ class EnvoyEgressRoutesFactory( .build() ) .setRoute( - createRouteAction(routeSpecification) + createRouteAction(routeSpecification, resourceVersion) ).build() ) .build() @@ -110,7 +113,10 @@ class EnvoyEgressRoutesFactory( return routeConfiguration.build() } - private fun createRouteAction(routeSpecification: RouteSpecification): RouteAction.Builder { + private fun createRouteAction( + routeSpecification: RouteSpecification, + resourceVersion: ResourceVersion + ): RouteAction.Builder { val routeAction = RouteAction.newBuilder() .setCluster(routeSpecification.clusterName) @@ -120,7 +126,12 @@ class EnvoyEgressRoutesFactory( } if (routeSpecification.settings.handleInternalRedirect) { - routeAction.internalRedirectAction = RouteAction.InternalRedirectAction.HANDLE_INTERNAL_REDIRECT + when (resourceVersion) { + ResourceVersion.V2 -> + routeAction.setInternalRedirectAction(RouteAction.InternalRedirectAction.HANDLE_INTERNAL_REDIRECT) + ResourceVersion.V3 -> + routeAction.internalRedirectPolicy = InternalRedirectPolicy.newBuilder().build() + } } if (properties.egress.hostHeaderRewriting.enabled && routeSpecification.settings.rewriteHostHeader) { From 835a72e920024722ce02a5ab8630be357f4b971b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemek=20Pi=C3=B3rkowski?= Date: Tue, 12 Jan 2021 11:15:08 +0100 Subject: [PATCH 04/11] Support for generate_request_id and preserve_external_request_id listener config (#233) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * generate_request_id and preserve_external_request_id config added * new flags added to test configs * tests added * fix ktlint issues Co-authored-by: Przemyslaw Piorkowski Co-authored-by: Krzysztof Słonka --- .../servicemesh/envoycontrol/groups/Groups.kt | 4 + .../envoycontrol/groups/MetadataNodeGroup.kt | 6 + .../listeners/EnvoyListenersFactory.kt | 4 + .../servicemesh/envoycontrol/RequestIdTest.kt | 119 ++++++++++++++++++ .../assertions/HttpsEchoResponseAssertions.kt | 20 +++ .../config/service/HttpsEchoContainer.kt | 11 +- .../ClientNameTrustedHeaderTest.kt | 44 +++---- .../ssl/EnvoyHttpsDependencyTest.kt | 9 +- .../src/main/resources/envoy/config_ads.yaml | 2 + .../envoy/config_ads_all_dependencies.yaml | 2 + ...fig_ads_disabled_endpoint_permissions.yaml | 2 + .../envoy/config_ads_no_dependencies.yaml | 2 + .../main/resources/envoy/config_ads_v2.yaml | 2 + .../src/main/resources/envoy/config_auth.yaml | 2 + .../src/main/resources/envoy/config_xds.yaml | 2 + 15 files changed, 190 insertions(+), 41 deletions(-) create mode 100644 envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/RequestIdTest.kt create mode 100644 envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/assertions/HttpsEchoResponseAssertions.kt diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/Groups.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/Groups.kt index bdccd822a..a1d69f96c 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/Groups.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/Groups.kt @@ -34,6 +34,8 @@ data class ListenersConfig( val egressHost: String, val egressPort: Int, val useRemoteAddress: Boolean = defaultUseRemoteAddress, + val generateRequestId: Boolean = defaultGenerateRequestId, + val preserveExternalRequestId: Boolean = defaultPreserveExternalRequestId, val accessLogEnabled: Boolean = defaultAccessLogEnabled, val enableLuaScript: Boolean = defaultEnableLuaScript, val accessLogPath: String = defaultAccessLogPath, @@ -46,6 +48,8 @@ data class ListenersConfig( companion object { const val defaultAccessLogPath = "/dev/stdout" const val defaultUseRemoteAddress = false + const val defaultGenerateRequestId = false + const val defaultPreserveExternalRequestId = false const val defaultAccessLogEnabled = false const val defaultEnableLuaScript = false const val defaultAddUpstreamExternalAddressHeader = false diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroup.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroup.kt index c3ec263b8..ea5216f6d 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroup.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroup.kt @@ -101,6 +101,10 @@ class MetadataNodeGroup( val useRemoteAddress = metadata.fieldsMap["use_remote_address"]?.boolValue ?: ListenersConfig.defaultUseRemoteAddress + val generateRequestId = metadata.fieldsMap["generate_request_id"]?.boolValue + ?: ListenersConfig.defaultGenerateRequestId + val preserveExternalRequestId = metadata.fieldsMap["preserve_external_request_id"]?.boolValue + ?: ListenersConfig.defaultPreserveExternalRequestId val accessLogEnabled = metadata.fieldsMap["access_log_enabled"]?.boolValue ?: ListenersConfig.defaultAccessLogEnabled val enableLuaScript = metadata.fieldsMap["enable_lua_script"]?.boolValue @@ -120,6 +124,8 @@ class MetadataNodeGroup( listenersHostPort.egressHost, listenersHostPort.egressPort, useRemoteAddress, + generateRequestId, + preserveExternalRequestId, accessLogEnabled, enableLuaScript, accessLogPath, diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/EnvoyListenersFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/EnvoyListenersFactory.kt index be669b9ae..c1933dd3f 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/EnvoyListenersFactory.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/EnvoyListenersFactory.kt @@ -218,6 +218,8 @@ class EnvoyListenersFactory( .setStatPrefix("egress_http") .setRds(egressRds(group.communicationMode, group.version)) .setHttpProtocolOptions(egressHttp1ProtocolOptions()) + .setPreserveExternalRequestId(listenersConfig.preserveExternalRequestId) + .setGenerateRequestId(boolValue(listenersConfig.generateRequestId)) addHttpFilters(connectionManagerBuilder, egressFilters, group, globalSnapshot) @@ -296,6 +298,8 @@ class EnvoyListenersFactory( val connectionManagerBuilder = HttpConnectionManager.newBuilder() .setStatPrefix(statPrefix) .setUseRemoteAddress(boolValue(listenersConfig.useRemoteAddress)) + .setGenerateRequestId(boolValue(listenersConfig.generateRequestId)) + .setPreserveExternalRequestId(listenersConfig.preserveExternalRequestId) .setDelayedCloseTimeout(durationInSeconds(0)) .setCommonHttpProtocolOptions(httpProtocolOptions) .setCodecType(HttpConnectionManager.CodecType.AUTO) diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/RequestIdTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/RequestIdTest.kt new file mode 100644 index 000000000..3c350be87 --- /dev/null +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/RequestIdTest.kt @@ -0,0 +1,119 @@ +package pl.allegro.tech.servicemesh.envoycontrol + +import okhttp3.Headers +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.extension.RegisterExtension +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.MethodSource +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk +import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.GenericServiceExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.HttpsEchoContainer +import pl.allegro.tech.servicemesh.envoycontrol.config.service.asHttpsEchoResponse + +class RequestIdTest { + + companion object { + @JvmStatic + fun extraHeadersSource() = listOf( + emptyMap(), + mapOf("x-forwarded-for" to "123.321.231.111"), + mapOf("x-forwarded-for" to "111.111.222.222,123.123.231.231") + ) + + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension(consul) + + @JvmField + @RegisterExtension + val localService = GenericServiceExtension(HttpsEchoContainer()) + + @JvmField + @RegisterExtension + val envoy = EnvoyExtension(envoyControl, localService) + + @JvmField + @RegisterExtension + val externalService = GenericServiceExtension(HttpsEchoContainer()) + } + + @BeforeEach + fun setup() { + consul.server.operations.registerService(externalService, name = "service-1") + } + + @ParameterizedTest + @MethodSource("extraHeadersSource") + fun `should propagate x-request-id on the egress port when it is available in request`(extraHeaders: Map) { + // given + val requestIdHeader = mapOf("x-request-id" to "egress-fake-request-id") + + untilAsserted { + // when + val response = envoy.egressOperations + .callService(service = "service-1", headers = requestIdHeader + extraHeaders) + .asHttpsEchoResponse() + + // then + assertThat(response).isOk() + assertThat(response.requestHeaders).containsEntry("x-request-id", "egress-fake-request-id") + } + } + + @ParameterizedTest + @MethodSource("extraHeadersSource") + fun `should generate x-request-id on the egress port when it is missing in request`(extraHeaders: Map) { + untilAsserted { + // when + val response = envoy.egressOperations + .callService(service = "service-1", headers = extraHeaders) + .asHttpsEchoResponse() + + // then + assertThat(response).isOk() + assertThat(response.requestHeaders).hasEntrySatisfying("x-request-id") { assertThat(it).isNotBlank() } + } + } + + @ParameterizedTest + @MethodSource("extraHeadersSource") + fun `should propagate x-request-id on the ingress port when it is available in request`(extraHeaders: Map) { + // given + val requestIdHeader = mapOf("x-request-id" to "ingress-fake-request-id") + + untilAsserted { + // when + val response = envoy.ingressOperations + .callLocalService(endpoint = "/", headers = Headers.of(requestIdHeader + extraHeaders)) + .asHttpsEchoResponse() + + // then + assertThat(response).isOk() + assertThat(response.requestHeaders).containsEntry("x-request-id", "ingress-fake-request-id") + } + } + + @ParameterizedTest + @MethodSource("extraHeadersSource") + fun `should generate x-request-id on the ingress port when it is missing in request`(extraHeaders: Map) { + untilAsserted { + // when + val response = envoy.ingressOperations + .callLocalService(endpoint = "/", headers = Headers.of(extraHeaders)) + .asHttpsEchoResponse() + + // then + assertThat(response).isOk() + assertThat(response.requestHeaders).hasEntrySatisfying("x-request-id") { assertThat(it).isNotBlank() } + } + } +} diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/assertions/HttpsEchoResponseAssertions.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/assertions/HttpsEchoResponseAssertions.kt new file mode 100644 index 000000000..adf1cc509 --- /dev/null +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/assertions/HttpsEchoResponseAssertions.kt @@ -0,0 +1,20 @@ +package pl.allegro.tech.servicemesh.envoycontrol.assertions + +import org.assertj.core.api.Assertions +import org.assertj.core.api.ObjectAssert +import pl.allegro.tech.servicemesh.envoycontrol.config.service.HttpsEchoContainer +import pl.allegro.tech.servicemesh.envoycontrol.config.service.HttpsEchoResponse + +fun ObjectAssert.isOk(): ObjectAssert { + matches { it.response.isSuccessful } + return this +} + +fun ObjectAssert.hasSNI(serverName: String): ObjectAssert = satisfies { + val actualServerName = HttpsEchoResponse.objectMapper.readTree(it.body).at("/connection/servername").textValue() + Assertions.assertThat(actualServerName).isEqualTo(serverName) +} + +fun ObjectAssert.isFrom(container: HttpsEchoContainer) = satisfies { + Assertions.assertThat(container.containerName()).isEqualTo(it.hostname) +} diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/service/HttpsEchoContainer.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/service/HttpsEchoContainer.kt index c559f6872..3fe9b2ac2 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/service/HttpsEchoContainer.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/service/HttpsEchoContainer.kt @@ -4,8 +4,6 @@ import com.fasterxml.jackson.databind.DeserializationFeature import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.module.kotlin.convertValue import okhttp3.Response -import org.assertj.core.api.Assertions.assertThat -import org.assertj.core.api.ObjectAssert import pl.allegro.tech.servicemesh.envoycontrol.config.BaseEnvoyTest import pl.allegro.tech.servicemesh.envoycontrol.config.containers.SSLGenericContainer @@ -43,11 +41,4 @@ class HttpsEchoResponse(val response: Response) { val hostname by lazy { objectMapper.readTree(body).at("/os/hostname").textValue() } } -fun ObjectAssert.hasSNI(serverName: String): ObjectAssert = satisfies { - val actualServerName = HttpsEchoResponse.objectMapper.readTree(it.body).at("/connection/servername").textValue() - assertThat(actualServerName).isEqualTo(serverName) -} - -fun ObjectAssert.isFrom(container: HttpsEchoContainer) = satisfies { - assertThat(container.containerName()).isEqualTo(it.hostname) -} +fun Response.asHttpsEchoResponse() = HttpsEchoResponse(this) diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/ClientNameTrustedHeaderTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/ClientNameTrustedHeaderTest.kt index 0646dbc46..03ada4f26 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/ClientNameTrustedHeaderTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/ClientNameTrustedHeaderTest.kt @@ -5,6 +5,7 @@ import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.RegisterExtension +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isFrom import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted import pl.allegro.tech.servicemesh.envoycontrol.config.Echo1EnvoyAuthConfig @@ -15,8 +16,7 @@ import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension import pl.allegro.tech.servicemesh.envoycontrol.config.service.GenericServiceExtension import pl.allegro.tech.servicemesh.envoycontrol.config.service.HttpsEchoContainer -import pl.allegro.tech.servicemesh.envoycontrol.config.service.HttpsEchoResponse -import pl.allegro.tech.servicemesh.envoycontrol.config.service.isFrom +import pl.allegro.tech.servicemesh.envoycontrol.config.service.asHttpsEchoResponse import java.time.Duration class ClientNameTrustedHeaderTest { @@ -116,66 +116,56 @@ class ClientNameTrustedHeaderTest { val response = envoy2.ingressOperations.callLocalService( "/endpoint", Headers.of(mapOf("x-client-name-trusted" to "fake-service")) - ) + ).asHttpsEchoResponse() // then assertThat(response).isOk() - HttpsEchoResponse(response).also { - assertThat(it).isFrom(service.container()) - assertThat(it.requestHeaders["x-client-name-trusted"]).isNull() - } + assertThat(response).isFrom(service.container()) + assertThat(response.requestHeaders).doesNotContainKey("x-client-name-trusted") } @Test fun `should add trusted client identity header to ingress request to local service`() { // when - val response = envoy2.egressOperations.callService("echo", emptyMap(), "/endpoint") + val response = envoy2.egressOperations.callService("echo", emptyMap(), "/endpoint").asHttpsEchoResponse() // then assertThat(response).isOk() - HttpsEchoResponse(response).also { - assertThat(it).isFrom(service.container()) - assertThat(it.requestHeaders["x-client-name-trusted"]).isEqualTo("echo2") - } + assertThat(response).isFrom(service.container()) + assertThat(response.requestHeaders).containsEntry("x-client-name-trusted", "echo2") } @Test fun `should override trusted client identity header in ingress request to local service`() { // when val headers = mapOf("x-client-name-trusted" to "fake-service") - val response = envoy2.egressOperations.callService("echo", headers, "/endpoint") + val response = envoy2.egressOperations.callService("echo", headers, "/endpoint").asHttpsEchoResponse() // then assertThat(response).isOk() - HttpsEchoResponse(response).also { - assertThat(it).isFrom(service.container()) - assertThat(it.requestHeaders["x-client-name-trusted"]).isEqualTo("echo2") - } + assertThat(response).isFrom(service.container()) + assertThat(response.requestHeaders).containsEntry("x-client-name-trusted", "echo2") } @Test fun `should set trusted client identity header based on all URIs in certificate SAN field`() { // when - val response = envoy4MultipleSANs.egressOperations.callService("echo", emptyMap(), "/endpoint") + val response = envoy4MultipleSANs.egressOperations.callService("echo", emptyMap(), "/endpoint").asHttpsEchoResponse() // then assertThat(response).isOk() - HttpsEchoResponse(response).also { - assertThat(it).isFrom(service.container()) - assertThat(it.requestHeaders["x-client-name-trusted"]).isEqualTo("echo4, echo4-special, echo4-admin") - } + assertThat(response).isFrom(service.container()) + assertThat(response.requestHeaders).containsEntry("x-client-name-trusted", "echo4, echo4-special, echo4-admin") } @Test fun `should not set trusted client identity header based on URIs in certificate SAN fields having invalid format`() { // when - val response = envoy5InvalidSANs.egressOperations.callService("echo", emptyMap(), "/endpoint") + val response = envoy5InvalidSANs.egressOperations.callService("echo", emptyMap(), "/endpoint").asHttpsEchoResponse() // then assertThat(response).isOk() - HttpsEchoResponse(response).also { - assertThat(it).isFrom(service.container()) - assertThat(it.requestHeaders["x-client-name-trusted"]).isNull() - } + assertThat(response).isFrom(service.container()) + assertThat(response.requestHeaders).doesNotContainKey("x-client-name-trusted") } } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ssl/EnvoyHttpsDependencyTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ssl/EnvoyHttpsDependencyTest.kt index d3a3e7178..14325038e 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ssl/EnvoyHttpsDependencyTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ssl/EnvoyHttpsDependencyTest.kt @@ -7,8 +7,9 @@ import org.junit.jupiter.api.Test import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlRunnerTestApp import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyControlTestConfiguration import pl.allegro.tech.servicemesh.envoycontrol.config.service.HttpsEchoContainer -import pl.allegro.tech.servicemesh.envoycontrol.config.service.HttpsEchoResponse -import pl.allegro.tech.servicemesh.envoycontrol.config.service.hasSNI +import pl.allegro.tech.servicemesh.envoycontrol.assertions.hasSNI +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk +import pl.allegro.tech.servicemesh.envoycontrol.config.service.asHttpsEchoResponse class EnvoyCurrentVersionHttpsDependencyTest : EnvoyHttpsDependencyTest() { companion object { @@ -46,13 +47,13 @@ abstract class EnvoyHttpsDependencyTest : EnvoyControlTestConfiguration() { fun `should include SNI in request to upstream`() { // when val response = untilAsserted { - val response = callDomain("my.example.com") + val response = callDomain("my.example.com").asHttpsEchoResponse() assertThat(response).isOk() response } // then - assertThat(HttpsEchoResponse(response)).hasSNI("my.example.com") + assertThat(response).hasSNI("my.example.com") } } diff --git a/envoy-control-tests/src/main/resources/envoy/config_ads.yaml b/envoy-control-tests/src/main/resources/envoy/config_ads.yaml index a2cc14ea1..26c4d0a95 100644 --- a/envoy-control-tests/src/main/resources/envoy/config_ads.yaml +++ b/envoy-control-tests/src/main/resources/envoy/config_ads.yaml @@ -26,6 +26,8 @@ node: egress_host: "0.0.0.0" egress_port: 5000 use_remote_address: true + generate_request_id: true + preserve_external_request_id: true access_log_enabled: false add_upstream_external_address_header: true resources_dir: "/etc/envoy/extra" diff --git a/envoy-control-tests/src/main/resources/envoy/config_ads_all_dependencies.yaml b/envoy-control-tests/src/main/resources/envoy/config_ads_all_dependencies.yaml index 943fb5e92..b6a5788a6 100644 --- a/envoy-control-tests/src/main/resources/envoy/config_ads_all_dependencies.yaml +++ b/envoy-control-tests/src/main/resources/envoy/config_ads_all_dependencies.yaml @@ -25,6 +25,8 @@ node: egress_host: "0.0.0.0" egress_port: 5000 use_remote_address: true + generate_request_id: true + preserve_external_request_id: true access_log_enabled: false resources_dir: "/etc/envoy/extra" service_name: test-service diff --git a/envoy-control-tests/src/main/resources/envoy/config_ads_disabled_endpoint_permissions.yaml b/envoy-control-tests/src/main/resources/envoy/config_ads_disabled_endpoint_permissions.yaml index 620f4c989..ddee4bdbc 100644 --- a/envoy-control-tests/src/main/resources/envoy/config_ads_disabled_endpoint_permissions.yaml +++ b/envoy-control-tests/src/main/resources/envoy/config_ads_disabled_endpoint_permissions.yaml @@ -25,6 +25,8 @@ node: egress_host: "0.0.0.0" egress_port: 5000 use_remote_address: true + generate_request_id: true + preserve_external_request_id: true access_log_enabled: false resources_dir: "/etc/envoy/extra" proxy_settings: diff --git a/envoy-control-tests/src/main/resources/envoy/config_ads_no_dependencies.yaml b/envoy-control-tests/src/main/resources/envoy/config_ads_no_dependencies.yaml index 1e698af19..3f34c2520 100644 --- a/envoy-control-tests/src/main/resources/envoy/config_ads_no_dependencies.yaml +++ b/envoy-control-tests/src/main/resources/envoy/config_ads_no_dependencies.yaml @@ -25,6 +25,8 @@ node: egress_host: "0.0.0.0" egress_port: 5000 use_remote_address: true + generate_request_id: true + preserve_external_request_id: true access_log_enabled: false resources_dir: "/etc/envoy/extra" diff --git a/envoy-control-tests/src/main/resources/envoy/config_ads_v2.yaml b/envoy-control-tests/src/main/resources/envoy/config_ads_v2.yaml index f49c24bba..5ea7254ee 100644 --- a/envoy-control-tests/src/main/resources/envoy/config_ads_v2.yaml +++ b/envoy-control-tests/src/main/resources/envoy/config_ads_v2.yaml @@ -23,6 +23,8 @@ node: egress_host: "0.0.0.0" egress_port: 5000 use_remote_address: true + generate_request_id: true + preserve_external_request_id: true access_log_enabled: false add_upstream_external_address_header: true resources_dir: "/etc/envoy/extra" diff --git a/envoy-control-tests/src/main/resources/envoy/config_auth.yaml b/envoy-control-tests/src/main/resources/envoy/config_auth.yaml index 03f88b2f5..b6c8c5ff8 100644 --- a/envoy-control-tests/src/main/resources/envoy/config_auth.yaml +++ b/envoy-control-tests/src/main/resources/envoy/config_auth.yaml @@ -26,6 +26,8 @@ node: egress_host: "0.0.0.0" egress_port: 5000 use_remote_address: true + generate_request_id: true + preserve_external_request_id: true access_log_enabled: false add_upstream_external_address_header: true has_static_secrets_defined: true diff --git a/envoy-control-tests/src/main/resources/envoy/config_xds.yaml b/envoy-control-tests/src/main/resources/envoy/config_xds.yaml index a326ffa3a..1b8e116b9 100644 --- a/envoy-control-tests/src/main/resources/envoy/config_xds.yaml +++ b/envoy-control-tests/src/main/resources/envoy/config_xds.yaml @@ -28,6 +28,8 @@ node: egress_host: "0.0.0.0" egress_port: 5000 use_remote_address: true + generate_request_id: true + preserve_external_request_id: true access_log_enabled: false add_upstream_external_address_header: true resources_dir: "/etc/envoy/extra" From ad38c0e3ce477ab8dc84666584a3569441a0c6eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Dziedziak?= Date: Tue, 12 Jan 2021 13:05:59 +0100 Subject: [PATCH 05/11] Migrate Tests to new approach (#236) * Migrate SnapshotDebugTest to new approach * Migrate IncomingPermissionsDisbaledInECTest * Migrate IncomingPermissionsEmptyClientsTest * Migrate IncomingPermissionsEmptyEndpointsTest * Migrate StatusRouteIncomingPermissionsTest * Migrate IncomingPermissionsDisabledInClientTest Co-authored-by: Lukasz Dziedziak --- .../envoycontrol/SnapshotDebugTest.kt | 68 ++++++---- .../config/envoy/EnvoyContainer.kt | 3 +- .../config/envoy/EnvoyExtension.kt | 7 +- ...IncomingPermissionsDisabledInClientTest.kt | 49 ++++--- .../IncomingPermissionsDisabledInECTest.kt | 55 +++++--- .../IncomingPermissionsEmptyClientsTest.kt | 123 +++++++++--------- .../IncomingPermissionsEmptyEndpointsTest.kt | 76 ++++++----- .../StatusRouteIncomingPermissionsTest.kt | 48 ++++--- 8 files changed, 250 insertions(+), 179 deletions(-) diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/SnapshotDebugTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/SnapshotDebugTest.kt index f9fe4bfa1..0968c41ff 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/SnapshotDebugTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/SnapshotDebugTest.kt @@ -1,34 +1,48 @@ package pl.allegro.tech.servicemesh.envoycontrol import org.assertj.core.api.Assertions.assertThat -import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test -import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyControlTestConfiguration +import org.junit.jupiter.api.extension.RegisterExtension +import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension -open class SnapshotDebugTest : EnvoyControlTestConfiguration() { +open class SnapshotDebugTest { companion object { - @JvmStatic - @BeforeAll - fun setupTest() { - setup() - } + + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension(consul) + + @JvmField + @RegisterExtension + val service = EchoServiceExtension() + + @JvmField + @RegisterExtension + val envoy = EnvoyExtension(envoyControl, service) } @Test open fun `should return snapshot debug info containing snapshot versions`() { // given - registerService(name = "echo") - val nodeMetadata = envoyContainer1.admin().nodeInfo() - waitForReadyServices("echo") + consul.server.operations.registerService(service, name = "echo") + val nodeMetadata = envoy.container.admin().nodeInfo() untilAsserted { // when - val snapshot = envoyControl1.getSnapshot(nodeMetadata) - val edsVersion = envoyContainer1.admin().statValue("cluster.echo.version") - val cdsVersion = envoyContainer1.admin().statValue("cluster_manager.cds.version") - val rdsVersion = envoyContainer1.admin().statValue("http.egress_http.rds.default_routes.version") - val ldsVersion = envoyContainer1.admin().statValue("listener_manager.lds.version") + val snapshot = envoyControl.app.getSnapshot(nodeMetadata) + val edsVersion = envoy.container.admin().statValue("cluster.echo.version") + val cdsVersion = envoy.container.admin().statValue("cluster_manager.cds.version") + val rdsVersion = envoy.container.admin().statValue("http.egress_http.rds.default_routes.version") + val ldsVersion = envoy.container.admin().statValue("listener_manager.lds.version") // then assertThat(snapshot.versions!!.clusters.metric).isEqualTo(cdsVersion) @@ -41,12 +55,12 @@ open class SnapshotDebugTest : EnvoyControlTestConfiguration() { @Test open fun `should return snapshot debug info containing snapshot contents`() { // given - registerService(name = "echo") - val nodeMetadata = envoyContainer1.admin().nodeInfo() + consul.server.operations.registerService(service, name = "echo") + val nodeMetadata = envoy.container.admin().nodeInfo() untilAsserted { // when - val snapshot = envoyControl1.getSnapshot(nodeMetadata) + val snapshot = envoyControl.app.getSnapshot(nodeMetadata) // then assertThat(snapshot.snapshot!!["clusters"]).isNotEmpty() @@ -91,7 +105,7 @@ open class SnapshotDebugTest : EnvoyControlTestConfiguration() { @Test open fun `should inform about missing snapshot when given node does not exist`() { // when - val snapshot = envoyControl1.getSnapshot(missingNodeJson) + val snapshot = envoyControl.app.getSnapshot(missingNodeJson) // then assertThat(snapshot.found).isFalse() @@ -101,7 +115,7 @@ open class SnapshotDebugTest : EnvoyControlTestConfiguration() { open fun `should return global snapshot debug info from xds`() { untilAsserted { // when - val snapshot = envoyControl1.getGlobalSnapshot(xds = true) + val snapshot = envoyControl.app.getGlobalSnapshot(xds = true) // then assertThat(snapshot.snapshot!!["clusters"]).isNotEmpty() @@ -114,16 +128,20 @@ open class SnapshotDebugTest : EnvoyControlTestConfiguration() { open fun `should return global snapshot debug info from ads`() { untilAsserted { // when - val snapshotXdsNull = envoyControl1.getGlobalSnapshot(xds = null) - val snapshotXdsFalse = envoyControl1.getGlobalSnapshot(xds = false) + val snapshotXdsNull = envoyControl.app.getGlobalSnapshot(xds = null) + val snapshotXdsFalse = envoyControl.app.getGlobalSnapshot(xds = false) // then assertThat(snapshotXdsNull.snapshot!!["clusters"]).isNotEmpty() assertThat(snapshotXdsNull.snapshot["endpoints"]).isNotEmpty() assertThat(snapshotXdsFalse.snapshot!!["clusters"]).isNotEmpty() assertThat(snapshotXdsFalse.snapshot["endpoints"]).isNotEmpty() - assertThat(snapshotXdsNull.snapshot["clusters"].first()["edsClusterConfig"]["edsConfig"].toString()).contains("ads") - assertThat(snapshotXdsFalse.snapshot["clusters"].first()["edsClusterConfig"]["edsConfig"].toString()).contains("ads") + assertThat(snapshotXdsNull.snapshot["clusters"].first()["edsClusterConfig"]["edsConfig"].toString()).contains( + "ads" + ) + assertThat(snapshotXdsFalse.snapshot["clusters"].first()["edsClusterConfig"]["edsConfig"].toString()).contains( + "ads" + ) } } } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyContainer.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyContainer.kt index 9df564fa2..ea88d7c24 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyContainer.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyContainer.kt @@ -16,7 +16,8 @@ class EnvoyContainer( private val envoyControl2XdsPort: Int = envoyControl1XdsPort, private val logLevel: String = "info", image: String = DEFAULT_IMAGE -) : SSLGenericContainer(dockerfileBuilder = DockerfileBuilder() +) : SSLGenericContainer( + dockerfileBuilder = DockerfileBuilder() .from(image) .run("apt-get update && apt-get install -y curl iproute2 iptables") ) { diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt index b511c919b..f6f451900 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt @@ -6,6 +6,7 @@ import org.junit.jupiter.api.extension.AfterEachCallback import org.junit.jupiter.api.extension.BeforeAllCallback import org.junit.jupiter.api.extension.ExtensionContext import org.testcontainers.containers.Network +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isRbacAccessLog import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyConfig import pl.allegro.tech.servicemesh.envoycontrol.config.RandomConfigFile @@ -16,7 +17,7 @@ import pl.allegro.tech.servicemesh.envoycontrol.logger class EnvoyExtension( private val envoyControl: EnvoyControlExtension, private val localService: ServiceExtension<*>? = null, - config: EnvoyConfig = RandomConfigFile + private val config: EnvoyConfig = RandomConfigFile ) : BeforeAllCallback, AfterAllCallback, AfterEachCallback { companion object { @@ -60,4 +61,8 @@ class EnvoyExtension( } } } + + fun recordRBACLogs() { + container.logRecorder.recordLogs(::isRbacAccessLog) + } } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsDisabledInClientTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsDisabledInClientTest.kt index 7de1a00e3..7ac233902 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsDisabledInClientTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsDisabledInClientTest.kt @@ -2,39 +2,54 @@ package pl.allegro.tech.servicemesh.envoycontrol.permissions import okhttp3.Headers import org.assertj.core.api.Assertions.assertThat -import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.RegisterExtension +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isFrom +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk +import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted import pl.allegro.tech.servicemesh.envoycontrol.config.AdsWithDisabledEndpointPermissions -import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlRunnerTestApp -import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyControlTestConfiguration +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension -internal class IncomingPermissionsDisabledInClientTest : EnvoyControlTestConfiguration() { +internal class IncomingPermissionsDisabledInClientTest { companion object { - private val properties = mapOf( - "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, - "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension( + consul, mapOf( + "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, + "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true + ) ) - @JvmStatic - @BeforeAll - fun setupTest() { - setup(appFactoryForEc1 = { consulPort -> - EnvoyControlRunnerTestApp(properties = properties, consulPort = consulPort) - }, envoyConfig = AdsWithDisabledEndpointPermissions) - } + @JvmField + @RegisterExtension + val service = EchoServiceExtension() + + @JvmField + @RegisterExtension + val envoy = EnvoyExtension(envoyControl, localService = service, config = AdsWithDisabledEndpointPermissions) } @Test fun `should allow access to endpoint by any client when endpoint permissions not defined in client`() { untilAsserted { // when - val response = callLocalService(endpoint = "/", - headers = Headers.of(mapOf("x-service-name" to "any"))) + val response = envoy.ingressOperations.callLocalService( + "/", + headers = Headers.of(mapOf("x-service-name" to "any")) + ) // then - assertThat(response).isOk().isFrom(localServiceContainer) + assertThat(response).isOk().isFrom(service) } } } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsDisabledInECTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsDisabledInECTest.kt index 0c81851a1..0c9281458 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsDisabledInECTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsDisabledInECTest.kt @@ -2,37 +2,52 @@ package pl.allegro.tech.servicemesh.envoycontrol.permissions import okhttp3.Headers import org.assertj.core.api.Assertions.assertThat -import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test -import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlRunnerTestApp -import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyControlTestConfiguration +import org.junit.jupiter.api.extension.RegisterExtension +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isFrom +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk +import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension -internal class IncomingPermissionsDisabledInECTest : EnvoyControlTestConfiguration() { +internal class IncomingPermissionsDisabledInECTest { companion object { - private val properties = mapOf( - "envoy-control.envoy.snapshot.incoming-permissions.enabled" to false + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension( + consul, mapOf( + "envoy-control.envoy.snapshot.incoming-permissions.enabled" to false + ) ) - @JvmStatic - @BeforeAll - fun setupTest() { - setup(appFactoryForEc1 = { consulPort -> - EnvoyControlRunnerTestApp(properties = properties, consulPort = consulPort) - }) - } + @JvmField + @RegisterExtension + val service = EchoServiceExtension() + + @JvmField + @RegisterExtension + val envoy = EnvoyExtension(envoyControl, localService = service) } @Test fun `should allow access to endpoint by authorized client`() { untilAsserted { // when - val response = callLocalService(endpoint = "/endpoint", - headers = Headers.of(mapOf("x-service-name" to "authorizedClient"))) + val response = envoy.ingressOperations.callLocalService( + endpoint = "/endpoint", + headers = Headers.of(mapOf("x-service-name" to "authorizedClient")) + ) // then - assertThat(response).isOk().isFrom(localServiceContainer) + assertThat(response).isOk().isFrom(service) } } @@ -40,11 +55,13 @@ internal class IncomingPermissionsDisabledInECTest : EnvoyControlTestConfigurati fun `should allow access to endpoint by unauthorized client when endpoint permissions disabled`() { untilAsserted { // when - val response = callLocalService(endpoint = "/endpoint", - headers = Headers.of(mapOf("x-service-name" to "unuthorizedClient"))) + val response = envoy.ingressOperations.callLocalService( + endpoint = "/endpoint", + headers = Headers.of(mapOf("x-service-name" to "unuthorizedClient")) + ) // then - assertThat(response).isOk().isFrom(localServiceContainer) + assertThat(response).isOk().isFrom(service) } } } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsEmptyClientsTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsEmptyClientsTest.kt index 280b68670..0cb882a3b 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsEmptyClientsTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsEmptyClientsTest.kt @@ -2,27 +2,27 @@ package pl.allegro.tech.servicemesh.envoycontrol.permissions import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterEach -import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.RegisterExtension import pl.allegro.tech.servicemesh.envoycontrol.assertions.hasOneAccessDenialWithActionBlock import pl.allegro.tech.servicemesh.envoycontrol.assertions.hasOneAccessDenialWithActionLog -import pl.allegro.tech.servicemesh.envoycontrol.assertions.isRbacAccessLog +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isForbidden +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isFrom +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk import pl.allegro.tech.servicemesh.envoycontrol.config.Echo1EnvoyAuthConfig import pl.allegro.tech.servicemesh.envoycontrol.config.Echo2EnvoyAuthConfig -import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlRunnerTestApp -import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyControlTestConfiguration -import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyContainer +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension -internal class IncomingPermissionsEmptyClientsTest : EnvoyControlTestConfiguration() { +internal class IncomingPermissionsEmptyClientsTest { companion object { - private val properties = mapOf( - "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, - "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true - ) // language=yaml - private val echoConfig = Echo1EnvoyAuthConfig.copy(configOverride = """ + private val echoConfig = Echo1EnvoyAuthConfig.copy( + configOverride = """ node: metadata: proxy_settings: @@ -31,10 +31,12 @@ internal class IncomingPermissionsEmptyClientsTest : EnvoyControlTestConfigurati endpoints: - path: /blocked-for-all clients: [] - """.trimIndent()) + """.trimIndent() + ) // language=yaml - private val echo2Config = Echo2EnvoyAuthConfig.copy(configOverride = """ + private val echo2Config = Echo2EnvoyAuthConfig.copy( + configOverride = """ node: metadata: proxy_settings: @@ -43,114 +45,115 @@ internal class IncomingPermissionsEmptyClientsTest : EnvoyControlTestConfigurati - path: /logged-for-all clients: [] unlistedClientsPolicy: log - """.trimIndent()) - - private val echoEnvoy by lazy { envoyContainer1 } - private val echoLocalService by lazy { localServiceContainer } - - private val echo2Envoy by lazy { envoyContainer2 } - private val echo2LocalService by lazy { echoContainer2 } - - @JvmStatic - @BeforeAll - fun setupTest() { - setup(appFactoryForEc1 = { consulPort -> - EnvoyControlRunnerTestApp(properties = properties, consulPort = consulPort) }, - envoys = 2, - envoyConfig = echoConfig, - secondEnvoyConfig = echo2Config + """.trimIndent() + ) + + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension( + consul, mapOf( + "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, + "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true ) - waitForEnvoysInitialized() - } + ) - private fun waitForEnvoysInitialized() { - untilAsserted { - assertThat(echoEnvoy.admin().isIngressReady()).isTrue() - assertThat(echo2Envoy.admin().isIngressReady()).isTrue() - } - } + @JvmField + @RegisterExtension + val echo = EchoServiceExtension() + + @JvmField + @RegisterExtension + val echo2 = EchoServiceExtension() + + @JvmField + @RegisterExtension + val envoy1 = EnvoyExtension(envoyControl, localService = echo, config = echoConfig) + + @JvmField + @RegisterExtension + val envoy2 = EnvoyExtension(envoyControl, localService = echo2, config = echo2Config) } @Test fun `echo should deny clients access to 'blocked-for-all' endpoint`() { // when - val echoResponse = callEnvoyIngress(envoy = echoEnvoy, path = "/blocked-for-all") + val echoResponse = envoy1.ingressOperations.callLocalService("/blocked-for-all") // then assertThat(echoResponse).isForbidden() - assertThat(echoEnvoy).hasOneAccessDenialWithActionBlock( + assertThat(envoy1.container).hasOneAccessDenialWithActionBlock( protocol = "http", path = "/blocked-for-all", method = "GET", clientName = "", trustedClient = false, - clientIp = echoEnvoy.gatewayIp() + clientIp = envoy1.container.gatewayIp() ) } @Test fun `echo should allow clients access to 'unlisted' endpoint and log it`() { // when - val echoResponse = callEnvoyIngress(envoy = echoEnvoy, path = "/unlisted") + val echoResponse = envoy1.ingressOperations.callLocalService("/unlisted") // then - assertThat(echoResponse).isOk().isFrom(echoLocalService) - assertThat(echoEnvoy).hasOneAccessDenialWithActionLog( + assertThat(echoResponse).isOk().isFrom(echo) + assertThat(envoy1.container).hasOneAccessDenialWithActionLog( protocol = "http", path = "/unlisted", method = "GET", clientName = "", - clientIp = echoEnvoy.gatewayIp() + clientIp = envoy1.container.gatewayIp() ) } @Test fun `echo2 should allow clients access to 'logged-for-all' endpoint and log it`() { // when - val echo2Response = callEnvoyIngress(envoy = echo2Envoy, path = "/logged-for-all") + val echo2Response = envoy2.ingressOperations.callLocalService("/logged-for-all") // then - assertThat(echo2Response).isOk().isFrom(echo2LocalService) - assertThat(echo2Envoy).hasOneAccessDenialWithActionLog( + assertThat(echo2Response).isOk().isFrom(echo2) + assertThat(envoy2.container).hasOneAccessDenialWithActionLog( protocol = "http", path = "/logged-for-all", method = "GET", clientName = "", - clientIp = echo2Envoy.gatewayIp() + clientIp = envoy2.container.gatewayIp() ) } @Test fun `echo2 should deny clients access to 'unlisted' endpoint`() { // when - val echo2Response = callEnvoyIngress(envoy = echo2Envoy, path = "/unlisted") + val echo2Response = envoy2.ingressOperations.callLocalService("/unlisted") // then assertThat(echo2Response).isForbidden() - assertThat(echo2Envoy).hasOneAccessDenialWithActionBlock( + assertThat(envoy2.container).hasOneAccessDenialWithActionBlock( protocol = "http", path = "/unlisted", method = "GET", clientName = "", trustedClient = false, - clientIp = echo2Envoy.gatewayIp() + clientIp = envoy2.container.gatewayIp() ) } @BeforeEach fun startRecordingRBACLogs() { - listOf(echoEnvoy, echo2Envoy).forEach { it.recordRBACLogs() } + listOf(envoy1, envoy2).forEach { it.recordRBACLogs() } } @AfterEach - override fun cleanupTest() { - listOf(echoEnvoy, echo2Envoy).forEach { - it.admin().resetCounters() - it.logRecorder.stopRecording() + fun cleanupTest() { + listOf(envoy1, envoy2).forEach { + it.container.admin().resetCounters() + it.container.logRecorder.stopRecording() } } - - private fun EnvoyContainer.recordRBACLogs() { - logRecorder.recordLogs(::isRbacAccessLog) - } } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsEmptyEndpointsTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsEmptyEndpointsTest.kt index 937d59523..828cda0ce 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsEmptyEndpointsTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsEmptyEndpointsTest.kt @@ -2,81 +2,79 @@ package pl.allegro.tech.servicemesh.envoycontrol.permissions import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterEach -import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.RegisterExtension import pl.allegro.tech.servicemesh.envoycontrol.assertions.hasOneAccessDenialWithActionLog -import pl.allegro.tech.servicemesh.envoycontrol.assertions.isRbacAccessLog +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isFrom +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk import pl.allegro.tech.servicemesh.envoycontrol.config.Echo1EnvoyAuthConfig -import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlRunnerTestApp -import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyControlTestConfiguration -import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyContainer +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension -internal class IncomingPermissionsEmptyEndpointsTest : EnvoyControlTestConfiguration() { +internal class IncomingPermissionsEmptyEndpointsTest { companion object { - private val properties = mapOf( - "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, - "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true - ) // language=yaml - private val echoConfig = Echo1EnvoyAuthConfig.copy(configOverride = """ + private val echoConfig = Echo1EnvoyAuthConfig.copy( + configOverride = """ node: metadata: proxy_settings: incoming: unlistedEndpointsPolicy: log endpoints: [] - """.trimIndent()) + """.trimIndent() + ) - private val echoEnvoy by lazy { envoyContainer1 } - private val echoLocalService by lazy { localServiceContainer } + @JvmField + @RegisterExtension + val consul = ConsulExtension() - @JvmStatic - @BeforeAll - fun setupTest() { - setup(appFactoryForEc1 = { consulPort -> - EnvoyControlRunnerTestApp(properties = properties, consulPort = consulPort) }, - envoyConfig = echoConfig + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension( + consul, mapOf( + "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, + "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true ) - waitForEnvoysInitialized() - } + ) - private fun waitForEnvoysInitialized() { - untilAsserted { - assertThat(echoEnvoy.admin().isIngressReady()).isTrue() - } - } + @JvmField + @RegisterExtension + val echo = EchoServiceExtension() + + @JvmField + @RegisterExtension + val envoy = EnvoyExtension(envoyControl, localService = echo, config = echoConfig) } @Test fun `echo should allow any client to access any endpoint and log request`() { // when - val echoResponse = callEnvoyIngress(envoy = echoEnvoy, path = "/some-endpoint") + val echoResponse = envoy.ingressOperations.callLocalService("/some-endpoint") // then - assertThat(echoResponse).isOk().isFrom(echoLocalService) - assertThat(echoEnvoy).hasOneAccessDenialWithActionLog( + assertThat(echoResponse).isOk().isFrom(echo) + assertThat(envoy.container).hasOneAccessDenialWithActionLog( protocol = "http", path = "/some-endpoint", method = "GET", clientName = "", - clientIp = echoEnvoy.gatewayIp() + clientIp = envoy.container.gatewayIp() ) } @BeforeEach fun startRecordingRBACLogs() { - echoEnvoy.recordRBACLogs() + envoy.recordRBACLogs() } @AfterEach - override fun cleanupTest() { - echoEnvoy.admin().resetCounters() - echoEnvoy.logRecorder.stopRecording() - } - - private fun EnvoyContainer.recordRBACLogs() { - logRecorder.recordLogs(::isRbacAccessLog) + fun cleanupTest() { + envoy.container.admin().resetCounters() + envoy.container.logRecorder.stopRecording() } } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/StatusRouteIncomingPermissionsTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/StatusRouteIncomingPermissionsTest.kt index 79647c633..4c5bcdef8 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/StatusRouteIncomingPermissionsTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/StatusRouteIncomingPermissionsTest.kt @@ -1,14 +1,18 @@ package pl.allegro.tech.servicemesh.envoycontrol.permissions -import okhttp3.Headers import org.assertj.core.api.Assertions.assertThat -import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test -import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlRunnerTestApp -import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyControlTestConfiguration +import org.junit.jupiter.api.extension.RegisterExtension +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isFrom +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk +import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension import pl.allegro.tech.servicemesh.envoycontrol.snapshot.EndpointMatch -internal class StatusRouteIncomingPermissionsTest : EnvoyControlTestConfiguration() { +internal class StatusRouteIncomingPermissionsTest { companion object { @@ -16,30 +20,40 @@ internal class StatusRouteIncomingPermissionsTest : EnvoyControlTestConfiguratio "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, "envoy-control.envoy.snapshot.incoming-permissions.overlapping-paths-fix" to true, "envoy-control.envoy.snapshot.routes.status.create-virtual-cluster" to true, - "envoy-control.envoy.snapshot.routes.status.endpoints" to mutableListOf(EndpointMatch().also { it.path = "/status/" }), + "envoy-control.envoy.snapshot.routes.status.endpoints" to mutableListOf(EndpointMatch().also { + it.path = "/status/" + }), "envoy-control.envoy.snapshot.routes.status.enabled" to true ) - @JvmStatic - @BeforeAll - fun setupTest() { - setup(appFactoryForEc1 = { consulPort -> - EnvoyControlRunnerTestApp(properties = properties, consulPort = consulPort) - }) - } + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension(consul, properties) + + @JvmField + @RegisterExtension + val service = EchoServiceExtension() + + @JvmField + @RegisterExtension + val envoy = EnvoyExtension(envoyControl, localService = service) } @Test fun `should allow access to status endpoint by all clients`() { untilAsserted { // when - val response = callLocalService("/status/", Headers.of()) - val statusUpstreamOk = envoyContainer1.admin().statValue( - "vhost.secured_local_service.vcluster.status.upstream_rq_200" + val response = envoy.ingressOperations.callLocalService("/status/") + val statusUpstreamOk = envoy.container.admin().statValue( + "vhost.secured_local_service.vcluster.status.upstream_rq_200" )?.toInt() // then - assertThat(response).isOk().isFrom(localServiceContainer) + assertThat(response).isOk().isFrom(service) assertThat(statusUpstreamOk).isGreaterThan(0) } } From 0192b38fd7110d8e1b84a19aedef0fd76ab511a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20S=C5=82onka?= Date: Tue, 12 Jan 2021 14:50:54 +0100 Subject: [PATCH 06/11] Migrate tests to new approach (#207) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Migrate tests to new approach * Migrate EnvoySANValidationTest * Migrate StatusRouteIncomingPermissionsTest and EnvoyHttpsDependencyTest * Migrate ServiceTagsTest * Resolve conflict after merge Co-authored-by: Lukasz Dziedziak Co-authored-by: Łukasz Dziedziak --- .../envoycontrol/AdminRouteTest.kt | 58 ++-- .../envoycontrol/CanaryLoadBalancingTest.kt | 65 ++-- .../envoycontrol/ChaosControllerTest.kt | 53 +-- ...lusterCircuitBreakerDefaultSettingsTest.kt | 43 ++- .../EndpointMetadataMergingTests.kt | 48 +-- .../ServiceTagsAndCanaryTestBase.kt | 10 +- .../envoycontrol/ServiceTagsTest.kt | 302 ++++++++++-------- .../envoycontrol/config/envoy/CallStats.kt | 13 +- .../config/envoy/EnvoyExtension.kt | 11 + .../StatusRouteIncomingPermissionsTest.kt | 5 +- .../permissions/TlsBasedAuthenticationTest.kt | 6 +- .../ssl/EnvoyHttpsDependencyTest.kt | 54 ++-- .../ssl/EnvoySANValidationTest.kt | 46 ++- 13 files changed, 423 insertions(+), 291 deletions(-) diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/AdminRouteTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/AdminRouteTest.kt index 6734e7d2c..43eed23c3 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/AdminRouteTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/AdminRouteTest.kt @@ -5,18 +5,19 @@ import okhttp3.MediaType import okhttp3.RequestBody import okhttp3.Response import org.assertj.core.api.Assertions.assertThat -import org.junit.jupiter.api.BeforeAll -import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.RegisterExtension import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.Arguments import org.junit.jupiter.params.provider.MethodSource -import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlRunnerTestApp -import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyControlTestConfiguration +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SecuredRoute import java.util.stream.Stream -internal class AdminRouteTest : EnvoyControlTestConfiguration() { +internal class AdminRouteTest { companion object { private val properties = mapOf( @@ -32,16 +33,21 @@ internal class AdminRouteTest : EnvoyControlTestConfiguration() { ) ) - @JvmStatic - @BeforeAll - fun setupAdminRoutesTest() { - setup(appFactoryForEc1 = { consulPort -> EnvoyControlRunnerTestApp(properties, consulPort) }) - } + @JvmField + @RegisterExtension + val consul = ConsulExtension() - @BeforeEach - fun stopLocalService() { - localServiceContainer.stop() - } + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension(consul, properties) + + @JvmField + @RegisterExtension + val service = EchoServiceExtension() + + @JvmField + @RegisterExtension + val envoy = EnvoyExtension(envoyControl, service) @JvmStatic fun disableOnHeaderTestCases(): Stream { @@ -49,44 +55,44 @@ internal class AdminRouteTest : EnvoyControlTestConfiguration() { return Stream.of( Arguments.of("admin root", { - callLocalService( + envoy.ingressOperations.callLocalService( endpoint = "/status/envoy/", headers = Headers.of(mapOf(disableHeader)) ) }), Arguments.of("admin root without trailing slash", { - callLocalService( + envoy.ingressOperations.callLocalService( endpoint = "/status/envoy", headers = Headers.of(mapOf(disableHeader)) ) }), Arguments.of("clusters", { - callLocalService( + envoy.ingressOperations.callLocalService( endpoint = "/status/envoy/clusters", headers = Headers.of(mapOf(disableHeader)) ) }), Arguments.of("config dump as unauthorized", { - callLocalService( + envoy.ingressOperations.callLocalService( endpoint = "/status/envoy/config_dump", headers = Headers.of(mapOf(disableHeader)) ) }), Arguments.of("config dump as authorized", { - callLocalService( + envoy.ingressOperations.callLocalService( endpoint = "/status/envoy/config_dump", headers = Headers.of(mapOf(disableHeader, "authorization" to "admin_secret_token")) ) }), Arguments.of("reset counters as unauthorized", { - callPostLocalService( + envoy.ingressOperations.callPostLocalService( endpoint = "/status/envoy/reset_counters", headers = Headers.of(mapOf(disableHeader)), body = RequestBody.create(MediaType.get("application/json"), "{}") ) }), Arguments.of("reset counters as authorized", { - callPostLocalService( + envoy.ingressOperations.callPostLocalService( endpoint = "/status/envoy/reset_counters", headers = Headers.of(mapOf(disableHeader, "authorization" to "admin_secret_token")), body = RequestBody.create(MediaType.get("application/json"), "{}") @@ -99,7 +105,7 @@ internal class AdminRouteTest : EnvoyControlTestConfiguration() { @Test fun `should get admin redirected port on ingress port when enabled`() { // when - val response = callLocalService(endpoint = "/status/envoy", headers = Headers.of(emptyMap())) + val response = envoy.ingressOperations.callLocalService(endpoint = "/status/envoy", headers = Headers.of(emptyMap())) // then assertThat(response.isSuccessful).isTrue() @@ -108,20 +114,20 @@ internal class AdminRouteTest : EnvoyControlTestConfiguration() { @Test fun `should get access to secured endpoints when authorized only`() { // when - val configDumpResponseUnauthorized = callLocalService( + val configDumpResponseUnauthorized = envoy.ingressOperations.callLocalService( endpoint = "/status/envoy/config_dump", headers = Headers.of(emptyMap()) ) - val configDumpResponseAuthorized = callLocalService( + val configDumpResponseAuthorized = envoy.ingressOperations.callLocalService( endpoint = "/status/envoy/config_dump", headers = Headers.of(mapOf("authorization" to "admin_secret_token")) ) - val resetCountersResponseUnauthorized = callPostLocalService( + val resetCountersResponseUnauthorized = envoy.ingressOperations.callPostLocalService( endpoint = "/status/envoy/reset_counters", headers = Headers.of(emptyMap()), body = RequestBody.create(MediaType.get("application/json"), "{}") ) - val resetCountersResponseAuthorized = callPostLocalService( + val resetCountersResponseAuthorized = envoy.ingressOperations.callPostLocalService( endpoint = "/status/envoy/reset_counters", headers = Headers.of(mapOf("authorization" to "admin_secret_token")), body = RequestBody.create(MediaType.get("application/json"), "{}") diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/CanaryLoadBalancingTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/CanaryLoadBalancingTest.kt index c8f3333d1..6e2cd93c7 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/CanaryLoadBalancingTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/CanaryLoadBalancingTest.kt @@ -1,14 +1,18 @@ package pl.allegro.tech.servicemesh.envoycontrol import org.assertj.core.api.Assertions.assertThat -import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test -import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlRunnerTestApp -import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyControlTestConfiguration +import org.junit.jupiter.api.extension.RegisterExtension +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk +import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.CallStats +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.ResponseWithBody +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension -open class CanaryLoadBalancingTest : EnvoyControlTestConfiguration() { +open class CanaryLoadBalancingTest { companion object { private val properties = mapOf( @@ -20,26 +24,35 @@ open class CanaryLoadBalancingTest : EnvoyControlTestConfiguration() { "envoy-control.envoy.snapshot.load-balancing.canary.metadata-value" to "1" ) - @JvmStatic - @BeforeAll - fun setupTest() { - setup(appFactoryForEc1 = { consulPort -> - EnvoyControlRunnerTestApp(properties = properties, consulPort = consulPort) - }) - } - } + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension(consul, properties) - private val canaryContainer = echoContainer - private val regularContainer = echoContainer2 + @JvmField + @RegisterExtension + val canaryContainer = EchoServiceExtension() + + @JvmField + @RegisterExtension + val regularContainer = EchoServiceExtension() + + @JvmField + @RegisterExtension + val envoy = EnvoyExtension(envoyControl) + } @Test fun `should balance load according to weights`() { // given - registerService(name = "echo", container = canaryContainer, tags = listOf("canary", "weight:1")) - registerService(name = "echo", container = regularContainer, tags = listOf("weight:20")) + consul.server.operations.registerService(name = "echo", extension = canaryContainer, tags = listOf("canary", "weight:1")) + consul.server.operations.registerService(name = "echo", extension = regularContainer, tags = listOf("weight:20")) untilAsserted { - callService("echo").also { + envoy.egressOperations.callService("echo").also { assertThat(it).isOk() } } @@ -48,7 +61,7 @@ open class CanaryLoadBalancingTest : EnvoyControlTestConfiguration() { val stats = callEchoServiceRepeatedly( minRepeat = 30, maxRepeat = 200, - repeatUntil = { response -> response.isFrom(canaryContainer) } + repeatUntil = { response -> response.isFrom(canaryContainer.container()) } ) // then @@ -79,11 +92,11 @@ open class CanaryLoadBalancingTest : EnvoyControlTestConfiguration() { @Test fun `should route request to canary instance only`() { // given - registerService(name = "echo", container = canaryContainer, tags = listOf("canary", "weight:1")) - registerService(name = "echo", container = regularContainer, tags = listOf("weight:20")) + consul.server.operations.registerService(name = "echo", extension = canaryContainer, tags = listOf("canary", "weight:1")) + consul.server.operations.registerService(name = "echo", extension = regularContainer, tags = listOf("weight:20")) untilAsserted { - callService("echo").also { + envoy.egressOperations.callService("echo").also { assertThat(it).isOk() } } @@ -103,11 +116,11 @@ open class CanaryLoadBalancingTest : EnvoyControlTestConfiguration() { @Test open fun `should route to both canary and regular instances when canary weight is 0`() { - registerService(name = "echo", container = canaryContainer, tags = listOf("canary", "weight:0")) - registerService(name = "echo", container = regularContainer, tags = listOf("weight:20")) + consul.server.operations.registerService(name = "echo", extension = canaryContainer, tags = listOf("canary", "weight:0")) + consul.server.operations.registerService(name = "echo", extension = regularContainer, tags = listOf("weight:20")) untilAsserted { - callService("echo").also { + envoy.egressOperations.callService("echo").also { assertThat(it).isOk() } } @@ -116,7 +129,7 @@ open class CanaryLoadBalancingTest : EnvoyControlTestConfiguration() { val stats = callEchoServiceRepeatedly( minRepeat = 30, maxRepeat = 200, - repeatUntil = { response -> response.isFrom(canaryContainer) } + repeatUntil = { response -> response.isFrom(canaryContainer.container()) } ) // then @@ -134,7 +147,7 @@ open class CanaryLoadBalancingTest : EnvoyControlTestConfiguration() { headers: Map = mapOf() ): CallStats { val stats = callStats() - callServiceRepeatedly( + envoy.egressOperations.callServiceRepeatedly( service = "echo", stats = stats, minRepeat = minRepeat, diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ChaosControllerTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ChaosControllerTest.kt index c936ff251..862555887 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ChaosControllerTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ChaosControllerTest.kt @@ -7,13 +7,14 @@ import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.module.kotlin.KotlinModule import okhttp3.Response import org.assertj.core.api.Assertions.assertThat -import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.RegisterExtension import org.springframework.http.HttpStatus import pl.allegro.tech.servicemesh.envoycontrol.chaos.api.ExperimentsListResponse import pl.allegro.tech.servicemesh.envoycontrol.chaos.api.NetworkDelay import pl.allegro.tech.servicemesh.envoycontrol.chaos.api.NetworkDelayResponse -import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyControlTestConfiguration +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension import java.util.UUID private val sampleNetworkDelayRequest = NetworkDelay( @@ -24,24 +25,26 @@ private val sampleNetworkDelayRequest = NetworkDelay( ) private val sampleNetworkDelayId = UUID.randomUUID().toString() -internal class ChaosControllerTest : EnvoyControlTestConfiguration() { +internal class ChaosControllerTest { private val objectMapper: ObjectMapper = ObjectMapper() .registerModule(KotlinModule()) .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) companion object { - @JvmStatic - @BeforeAll - fun setupTest() { - setup() - } + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension(consul) } @Test fun `should return UNAUTHORIZED for invalid user`() { // when - val response = envoyControl1.postChaosFaultRequest( + val response = envoyControl.app.postChaosFaultRequest( username = "bad-user", password = "wrong-pass", networkDelay = sampleNetworkDelayRequest @@ -55,7 +58,7 @@ internal class ChaosControllerTest : EnvoyControlTestConfiguration() { fun `should post a chaos fault request and get response with storage object`() { // when val response = convertResponseToNetworkDelayResponse( - envoyControl1.postChaosFaultRequest(networkDelay = sampleNetworkDelayRequest) + envoyControl.app.postChaosFaultRequest(networkDelay = sampleNetworkDelayRequest) ) // then @@ -71,7 +74,7 @@ internal class ChaosControllerTest : EnvoyControlTestConfiguration() { @Test fun `should accept delete request and return NO_CONTENT (204)`() { // when - val response = envoyControl1.deleteChaosFaultRequest(faultId = sampleNetworkDelayId) + val response = envoyControl.app.deleteChaosFaultRequest(faultId = sampleNetworkDelayId) // then assertThat(response.code()).isEqualTo(HttpStatus.NO_CONTENT.value()) @@ -80,7 +83,7 @@ internal class ChaosControllerTest : EnvoyControlTestConfiguration() { @Test fun `should return experiment list with OK (200) status`() { // when - val response = envoyControl1.getExperimentsListRequest() + val response = envoyControl.app.getExperimentsListRequest() // then assertThat(response.code()).isEqualTo(HttpStatus.OK.value()) @@ -90,7 +93,7 @@ internal class ChaosControllerTest : EnvoyControlTestConfiguration() { fun `should return empty experiment list when no experiment is running`() { // when val response: ExperimentsListResponse = convertResponseToExperimentsListResponse( - envoyControl1.getExperimentsListRequest() + envoyControl.app.getExperimentsListRequest() ) // then @@ -102,15 +105,15 @@ internal class ChaosControllerTest : EnvoyControlTestConfiguration() { // given removeAllFromStorage() val item1 = convertResponseToNetworkDelayResponse( - envoyControl1.postChaosFaultRequest(networkDelay = sampleNetworkDelayRequest) + envoyControl.app.postChaosFaultRequest(networkDelay = sampleNetworkDelayRequest) ) val item2 = convertResponseToNetworkDelayResponse( - envoyControl1.postChaosFaultRequest(networkDelay = sampleNetworkDelayRequest) + envoyControl.app.postChaosFaultRequest(networkDelay = sampleNetworkDelayRequest) ) // when val itemsList = convertResponseToExperimentsListResponse( - envoyControl1.getExperimentsListRequest() + envoyControl.app.getExperimentsListRequest() ) // then @@ -125,25 +128,25 @@ internal class ChaosControllerTest : EnvoyControlTestConfiguration() { // given removeAllFromStorage() val item1 = convertResponseToNetworkDelayResponse( - envoyControl1.postChaosFaultRequest(networkDelay = sampleNetworkDelayRequest) + envoyControl.app.postChaosFaultRequest(networkDelay = sampleNetworkDelayRequest) ) val item2 = convertResponseToNetworkDelayResponse( - envoyControl1.postChaosFaultRequest(networkDelay = sampleNetworkDelayRequest) + envoyControl.app.postChaosFaultRequest(networkDelay = sampleNetworkDelayRequest) ) val item3 = convertResponseToNetworkDelayResponse( - envoyControl1.postChaosFaultRequest(networkDelay = sampleNetworkDelayRequest) + envoyControl.app.postChaosFaultRequest(networkDelay = sampleNetworkDelayRequest) ) val itemsList = convertResponseToExperimentsListResponse( - envoyControl1.getExperimentsListRequest() + envoyControl.app.getExperimentsListRequest() ) assertThat(itemsList.experimentList.size).isEqualTo(3) // when - val response = envoyControl1.deleteChaosFaultRequest(faultId = item2.id) + val response = envoyControl.app.deleteChaosFaultRequest(faultId = item2.id) // then val resultItemsList = convertResponseToExperimentsListResponse( - envoyControl1.getExperimentsListRequest() + envoyControl.app.getExperimentsListRequest() ) with(resultItemsList.experimentList) { assertThat(size).isEqualTo(2) @@ -153,15 +156,15 @@ internal class ChaosControllerTest : EnvoyControlTestConfiguration() { private fun removeAllFromStorage() { var response = convertResponseToExperimentsListResponse( - envoyControl1.getExperimentsListRequest() + envoyControl.app.getExperimentsListRequest() ) for (item in response.experimentList) { - envoyControl1.deleteChaosFaultRequest(faultId = item.id) + envoyControl.app.deleteChaosFaultRequest(faultId = item.id) } response = convertResponseToExperimentsListResponse( - envoyControl1.getExperimentsListRequest() + envoyControl.app.getExperimentsListRequest() ) assertThat(response.experimentList.size).isEqualTo(0) } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ClusterCircuitBreakerDefaultSettingsTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ClusterCircuitBreakerDefaultSettingsTest.kt index d9759531f..35e76a3ce 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ClusterCircuitBreakerDefaultSettingsTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ClusterCircuitBreakerDefaultSettingsTest.kt @@ -1,13 +1,18 @@ package pl.allegro.tech.servicemesh.envoycontrol import org.assertj.core.api.Assertions.assertThat -import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test -import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlRunnerTestApp -import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyControlTestConfiguration +import org.junit.jupiter.api.extension.RegisterExtension +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isFrom +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk +import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension import pl.allegro.tech.servicemesh.envoycontrol.snapshot.Threshold -internal class ClusterCircuitBreakerDefaultSettingsTest : EnvoyControlTestConfiguration() { +internal class ClusterCircuitBreakerDefaultSettingsTest { companion object { private val properties = mapOf( @@ -25,25 +30,35 @@ internal class ClusterCircuitBreakerDefaultSettingsTest : EnvoyControlTestConfig } ) - @JvmStatic - @BeforeAll - fun setupTest() { - setup(appFactoryForEc1 = { consulPort -> EnvoyControlRunnerTestApp(properties, consulPort) }) - } + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension(consul, properties) + + @JvmField + @RegisterExtension + val service = EchoServiceExtension() + + @JvmField + @RegisterExtension + val envoy = EnvoyExtension(envoyControl, service) } @Test fun `should enable setting circuit breaker threstholds setting`() { // given - registerService(name = "echo") + consul.server.operations.registerService(name = "echo", extension = service) untilAsserted { - val response = callEcho() - assertThat(response).isOk().isFrom(echoContainer) + val response = envoy.egressOperations.callService("echo") + assertThat(response).isOk().isFrom(service) } // when - val maxRequestsSetting = envoyContainer1.admin().circuitBreakerSetting("echo", "max_requests", "default_priority") - val maxRetriesSetting = envoyContainer1.admin().circuitBreakerSetting("echo", "max_retries", "high_priority") + val maxRequestsSetting = envoy.container.admin().circuitBreakerSetting("echo", "max_requests", "default_priority") + val maxRetriesSetting = envoy.container.admin().circuitBreakerSetting("echo", "max_retries", "high_priority") // then assertThat(maxRequestsSetting).isEqualTo(3) diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt index 42cef9342..c281cf7e1 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt @@ -1,35 +1,45 @@ package pl.allegro.tech.servicemesh.envoycontrol import org.assertj.core.api.Assertions.assertThat -import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test -import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlRunnerTestApp -import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyControlTestConfiguration +import org.junit.jupiter.api.extension.RegisterExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.CallStats +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension -open class EndpointMetadataMergingTests : EnvoyControlTestConfiguration() { +open class EndpointMetadataMergingTests { companion object { private val properties = mapOf( "envoy-control.envoy.snapshot.routing.service-tags.enabled" to true ) - @JvmStatic - @BeforeAll - fun setupTest() { - setup(appFactoryForEc1 = { consulPort -> - EnvoyControlRunnerTestApp(properties = properties, consulPort = consulPort) - }) - } + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension(consul, properties) + + @JvmField + @RegisterExtension + val service = EchoServiceExtension() + + @JvmField + @RegisterExtension + val envoy = EnvoyExtension(envoyControl, service) } @Test fun `should merge all service tags of endpoints with the same ip and port`() { // given - registerService(name = "echo", container = echoContainer, tags = listOf("ipsum")) - registerService(name = "echo", container = echoContainer, tags = listOf("lorem", "dolom")) + consul.server.operations.registerService(name = "echo", extension = service, tags = listOf("ipsum")) + consul.server.operations.registerService(name = "echo", extension = service, tags = listOf("lorem", "dolom")) - waitForReadyServices("echo") + envoy.waitForReadyServices("echo") // when val ipsumStats = callEchoServiceRepeatedly(repeat = 1, tag = "ipsum") @@ -37,9 +47,9 @@ open class EndpointMetadataMergingTests : EnvoyControlTestConfiguration() { val dolomStats = callEchoServiceRepeatedly(repeat = 1, tag = "dolom") // then - assertThat(ipsumStats.hits(echoContainer)).isEqualTo(1) - assertThat(loremStats.hits(echoContainer)).isEqualTo(1) - assertThat(dolomStats.hits(echoContainer)).isEqualTo(1) + assertThat(ipsumStats.hits(service)).isEqualTo(1) + assertThat(loremStats.hits(service)).isEqualTo(1) + assertThat(dolomStats.hits(service)).isEqualTo(1) } protected open fun callEchoServiceRepeatedly( @@ -47,8 +57,8 @@ open class EndpointMetadataMergingTests : EnvoyControlTestConfiguration() { tag: String? = null, assertNoErrors: Boolean = true ): CallStats { - val stats = CallStats(listOf(echoContainer)) - callServiceRepeatedly( + val stats = CallStats(listOf(service)) + envoy.egressOperations.callServiceRepeatedly( service = "echo", stats = stats, minRepeat = repeat, diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsAndCanaryTestBase.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsAndCanaryTestBase.kt index 0cd98cb35..e2135383e 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsAndCanaryTestBase.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsAndCanaryTestBase.kt @@ -114,9 +114,7 @@ interface ServiceTagsAndCanaryTestBase { } } - fun callStats() = CallStats(listOf( - loremCanaryService.container(), loremRegularService.container(), ipsumRegularService.container() - )) + fun callStats() = CallStats(listOf(loremCanaryService, loremRegularService, ipsumRegularService)) fun callEchoServiceRepeatedly( repeat: Int, @@ -140,9 +138,9 @@ interface ServiceTagsAndCanaryTestBase { } val CallStats.loremCanaryHits: Int - get() = this.hits(loremCanaryService.container()) + get() = this.hits(loremCanaryService) val CallStats.loremRegularHits: Int - get() = this.hits(loremRegularService.container()) + get() = this.hits(loremRegularService) val CallStats.ipsumRegularHits: Int - get() = this.hits(ipsumRegularService.container()) + get() = this.hits(ipsumRegularService) } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsTest.kt index 3160caa7a..e10e4d7e1 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsTest.kt @@ -1,15 +1,15 @@ package pl.allegro.tech.servicemesh.envoycontrol import org.assertj.core.api.Assertions.assertThat -import org.junit.jupiter.api.AfterAll -import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test -import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlRunnerTestApp -import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyControlTestConfiguration -import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoContainer +import org.junit.jupiter.api.extension.RegisterExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.CallStats +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension -open class ServiceTagsTest : EnvoyControlTestConfiguration() { +open class ServiceTagsTest { companion object { private val properties = mapOf( @@ -25,95 +25,110 @@ open class ServiceTagsTest : EnvoyControlTestConfiguration() { "envoy-control.envoy.snapshot.load-balancing.canary.enabled" to false ) - @JvmStatic - @BeforeAll - fun setupTest() { - setup(appFactoryForEc1 = { consulPort -> - EnvoyControlRunnerTestApp(properties = properties, consulPort = consulPort) - }) - } - - val regularContainer = echoContainer - val loremContainer = echoContainer2 - val loremIpsumContainer = EchoContainer() - val genericContainer = EchoContainer() - - private val containersToStart = listOf(loremIpsumContainer, genericContainer) - - @JvmStatic - @BeforeAll - fun startContainers() { - containersToStart.parallelStream().forEach { it.start() } - } - - @JvmStatic - @AfterAll - fun cleanup() { - containersToStart.parallelStream().forEach { it.stop() } - } + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension(consul, properties) + + @JvmField + @RegisterExtension + val regularService = EchoServiceExtension() + + @JvmField + @RegisterExtension + val loremService = EchoServiceExtension() + + @JvmField + @RegisterExtension + val loremIpsumService = EchoServiceExtension() + + @JvmField + @RegisterExtension + val genericService = EchoServiceExtension() + + @JvmField + @RegisterExtension + val envoy = EnvoyExtension(envoyControl, regularService) } protected fun registerServices() { - registerService(name = "echo", container = regularContainer, tags = listOf()) - registerService(name = "echo", container = loremContainer, tags = listOf("lorem", "blacklisted")) - registerService(name = "echo", container = loremIpsumContainer, tags = listOf("lorem", "ipsum")) + consul.server.operations.registerService(name = "echo", extension = regularService, tags = emptyList()) + consul.server.operations.registerService( + name = "echo", + extension = loremService, + tags = listOf("lorem", "blacklisted") + ) + consul.server.operations.registerService( + name = "echo", + extension = loremIpsumService, + tags = listOf("lorem", "ipsum") + ) } @Test open fun `should route requests to instance with tag ipsum`() { // given registerServices() - waitForReadyServices("echo") + envoy.waitForReadyServices("echo") // when val stats = callEchoServiceRepeatedly(repeat = 10, tag = "ipsum") // then assertThat(stats.totalHits).isEqualTo(10) - assertThat(stats.hits(regularContainer)).isEqualTo(0) - assertThat(stats.hits(loremContainer)).isEqualTo(0) - assertThat(stats.hits(loremIpsumContainer)).isEqualTo(10) + assertThat(stats.hits(regularService)).isEqualTo(0) + assertThat(stats.hits(loremService)).isEqualTo(0) + assertThat(stats.hits(loremIpsumService)).isEqualTo(10) } @Test open fun `should route requests to instances with tag lorem`() { // given registerServices() - waitForReadyServices("echo") + envoy.waitForReadyServices("echo") // when val stats = callEchoServiceRepeatedly(repeat = 20, tag = "lorem") // then assertThat(stats.totalHits).isEqualTo(20) - assertThat(stats.hits(regularContainer)).isEqualTo(0) - assertThat(stats.hits(loremContainer)).isGreaterThan(2) - assertThat(stats.hits(loremIpsumContainer)).isGreaterThan(2) - assertThat(stats.hits(loremContainer) + stats.hits(loremIpsumContainer)).isEqualTo(20) + assertThat(stats.hits(regularService)).isEqualTo(0) + assertThat(stats.hits(loremService)).isGreaterThan(2) + assertThat(stats.hits(loremIpsumService)).isGreaterThan(2) + assertThat(stats.hits(loremService) + stats.hits(loremIpsumService)).isEqualTo(20) } @Test open fun `should route requests to all instances`() { // given registerServices() - waitForReadyServices("echo") + envoy.waitForReadyServices("echo") // when val stats = callEchoServiceRepeatedly(repeat = 20) // then assertThat(stats.totalHits).isEqualTo(20) - assertThat(stats.hits(regularContainer)).isGreaterThan(1) - assertThat(stats.hits(loremContainer)).isGreaterThan(1) - assertThat(stats.hits(loremIpsumContainer)).isGreaterThan(1) - assertThat(stats.hits(regularContainer) + stats.hits(loremContainer) + stats.hits(loremIpsumContainer)).isEqualTo(20) + assertThat(stats.hits(regularService)).isGreaterThan(1) + assertThat(stats.hits(loremService)).isGreaterThan(1) + assertThat(stats.hits(loremIpsumService)).isGreaterThan(1) + assertThat( + stats.hits(regularService) + stats.hits(loremService) + stats.hits( + loremIpsumService + ) + ).isEqualTo( + 20 + ) } @Test open fun `should return 503 if instance with requested tag is not found`() { // given registerServices() - waitForReadyServices("echo") + envoy.waitForReadyServices("echo") // when val stats = callEchoServiceRepeatedly(repeat = 10, tag = "dolom", assertNoErrors = false) @@ -121,16 +136,16 @@ open class ServiceTagsTest : EnvoyControlTestConfiguration() { // then assertThat(stats.totalHits).isEqualTo(10) assertThat(stats.failedHits).isEqualTo(10) - assertThat(stats.hits(regularContainer)).isEqualTo(0) - assertThat(stats.hits(loremContainer)).isEqualTo(0) - assertThat(stats.hits(loremIpsumContainer)).isEqualTo(0) + assertThat(stats.hits(regularService)).isEqualTo(0) + assertThat(stats.hits(loremService)).isEqualTo(0) + assertThat(stats.hits(loremIpsumService)).isEqualTo(0) } @Test open fun `should return 503 if requested tag is blacklisted`() { // given registerServices() - waitForReadyServices("echo") + envoy.waitForReadyServices("echo") // when val stats = callEchoServiceRepeatedly(repeat = 10, tag = "blacklisted", assertNoErrors = false) @@ -138,164 +153,188 @@ open class ServiceTagsTest : EnvoyControlTestConfiguration() { // then assertThat(stats.totalHits).isEqualTo(10) assertThat(stats.failedHits).isEqualTo(10) - assertThat(stats.hits(regularContainer)).isEqualTo(0) - assertThat(stats.hits(loremContainer)).isEqualTo(0) - assertThat(stats.hits(loremIpsumContainer)).isEqualTo(0) + assertThat(stats.hits(regularService)).isEqualTo(0) + assertThat(stats.hits(loremService)).isEqualTo(0) + assertThat(stats.hits(loremIpsumService)).isEqualTo(0) } @Test open fun `should route request with three tags if combination is valid`() { // given - val matchingContainer = loremContainer - val notMatchingContainer = loremIpsumContainer + val matching = loremService + val notMatching = loremIpsumService - registerService( - name = "service-1", container = matchingContainer, - tags = listOf("version:v1.5", "hardware:c32", "role:master")) - registerService( - name = "service-1", container = notMatchingContainer, - tags = listOf("version:v1.5", "hardware:c64", "role:master")) + consul.server.operations.registerService( + name = "service-1", extension = matching, + tags = listOf("version:v1.5", "hardware:c32", "role:master") + ) + consul.server.operations.registerService( + name = "service-1", extension = notMatching, + tags = listOf("version:v1.5", "hardware:c64", "role:master") + ) - waitForReadyServices("service-1") + envoy.waitForReadyServices("service-1") // when val stats = callServiceRepeatedly( - service = "service-1", repeat = 10, tag = "hardware:c32,role:master,version:v1.5") + service = "service-1", repeat = 10, tag = "hardware:c32,role:master,version:v1.5" + ) // then assertThat(stats.totalHits).isEqualTo(10) - assertThat(stats.hits(matchingContainer)).isEqualTo(10) - assertThat(stats.hits(notMatchingContainer)).isEqualTo(0) + assertThat(stats.hits(matching)).isEqualTo(10) + assertThat(stats.hits(notMatching)).isEqualTo(0) } @Test open fun `should not route request with multiple tags if service is not whitelisted`() { // given - val matchingContainer = loremContainer + val matching = loremService - registerService( - name = "service-3", container = matchingContainer, - tags = listOf("version:v1.5", "hardware:c32", "role:master")) + consul.server.operations.registerService( + name = "service-3", extension = matching, + tags = listOf("version:v1.5", "hardware:c32", "role:master") + ) - waitForReadyServices("service-3") + envoy.waitForReadyServices("service-3") // when val threeTagsStats = callServiceRepeatedly( - service = "service-3", repeat = 10, tag = "hardware:c32,role:master,version:v1.5", assertNoErrors = false) + service = "service-3", repeat = 10, tag = "hardware:c32,role:master,version:v1.5", assertNoErrors = false + ) val twoTagsStats = callServiceRepeatedly( - service = "service-3", repeat = 10, tag = "hardware:c32,role:master", assertNoErrors = false) + service = "service-3", repeat = 10, tag = "hardware:c32,role:master", assertNoErrors = false + ) val oneTagStats = callServiceRepeatedly( - service = "service-3", repeat = 10, tag = "role:master") + service = "service-3", repeat = 10, tag = "role:master" + ) // then assertThat(threeTagsStats.totalHits).isEqualTo(10) assertThat(threeTagsStats.failedHits).isEqualTo(10) - assertThat(threeTagsStats.hits(matchingContainer)).isEqualTo(0) + assertThat(threeTagsStats.hits(matching)).isEqualTo(0) assertThat(twoTagsStats.totalHits).isEqualTo(10) assertThat(twoTagsStats.failedHits).isEqualTo(10) - assertThat(twoTagsStats.hits(matchingContainer)).isEqualTo(0) + assertThat(twoTagsStats.hits(matching)).isEqualTo(0) assertThat(oneTagStats.totalHits).isEqualTo(10) assertThat(oneTagStats.failedHits).isEqualTo(0) - assertThat(oneTagStats.hits(matchingContainer)).isEqualTo(10) + assertThat(oneTagStats.hits(matching)).isEqualTo(10) } @Test open fun `should not route request with three tags if combination is not allowed`() { // given - val service1MatchingContainer = loremContainer - val service2MatchingContainer = loremIpsumContainer + val service1Matching = loremService + val service2Matching = loremIpsumService - registerService( - name = "service-1", container = service1MatchingContainer, - tags = listOf("version:v1.5", "hardware:c32", "ram:512")) - registerService( - name = "service-2", container = service2MatchingContainer, - tags = listOf("version:v1.5", "hardware:c32", "role:master")) + consul.server.operations.registerService( + name = "service-1", extension = service1Matching, + tags = listOf("version:v1.5", "hardware:c32", "ram:512") + ) + consul.server.operations.registerService( + name = "service-2", extension = service2Matching, + tags = listOf("version:v1.5", "hardware:c32", "role:master") + ) - waitForReadyServices("service-1", "service-2") + envoy.waitForReadyServices("service-1", "service-2") // when val service1Stats = callServiceRepeatedly( - service = "service-1", repeat = 10, tag = "hardware:c32,ram:512,version:v1.5", assertNoErrors = false) + service = "service-1", repeat = 10, tag = "hardware:c32,ram:512,version:v1.5", assertNoErrors = false + ) val service2Stats = callServiceRepeatedly( - service = "service-2", repeat = 10, tag = "hardware:c32,role:master,version:v1.5", assertNoErrors = false) + service = "service-2", repeat = 10, tag = "hardware:c32,role:master,version:v1.5", assertNoErrors = false + ) // then assertThat(service1Stats.totalHits).isEqualTo(10) assertThat(service1Stats.failedHits).isEqualTo(10) - assertThat(service1Stats.hits(service1MatchingContainer)).isEqualTo(0) + assertThat(service1Stats.hits(service1Matching)).isEqualTo(0) assertThat(service2Stats.totalHits).isEqualTo(10) assertThat(service2Stats.failedHits).isEqualTo(10) - assertThat(service2Stats.hits(service1MatchingContainer)).isEqualTo(0) + assertThat(service2Stats.hits(service1Matching)).isEqualTo(0) } @Test open fun `should route request with two tags if combination is valid`() { // given - val service1MatchingContainer = loremContainer - val service1NotMatchingContainer = regularContainer - val service2MasterContainer = loremIpsumContainer - val service2SecondaryContainer = genericContainer - - registerService( - name = "service-1", container = service1MatchingContainer, - tags = listOf("version:v2.0", "hardware:c32", "role:master")) - registerService( - name = "service-1", container = service1NotMatchingContainer, - tags = listOf("version:v1.5", "hardware:c32", "role:master")) - registerService( - name = "service-2", container = service2MasterContainer, - tags = listOf("version:v1.5", "hardware:c32", "role:master")) - registerService( - name = "service-2", container = service2SecondaryContainer, - tags = listOf("version:v2.0", "hardware:c32", "role:secondary")) - - waitForReadyServices("service-1", "service-2") + val service1Matching = loremService + val service1NotMatching = regularService + val service2Master = loremIpsumService + val service2Secondary = genericService + + consul.server.operations.registerService( + name = "service-1", extension = service1Matching, + tags = listOf("version:v2.0", "hardware:c32", "role:master") + ) + consul.server.operations.registerService( + name = "service-1", extension = service1NotMatching, + tags = listOf("version:v1.5", "hardware:c32", "role:master") + ) + consul.server.operations.registerService( + name = "service-2", extension = service2Master, + tags = listOf("version:v1.5", "hardware:c32", "role:master") + ) + consul.server.operations.registerService( + name = "service-2", extension = service2Secondary, + tags = listOf("version:v2.0", "hardware:c32", "role:secondary") + ) + + envoy.waitForReadyServices("service-1", "service-2") // when val service1Stats = callServiceRepeatedly( - service = "service-1", repeat = 10, tag = "hardware:c32,version:v2.0") + service = "service-1", repeat = 10, tag = "hardware:c32,version:v2.0" + ) val service2MasterStats = callServiceRepeatedly( - service = "service-2", repeat = 10, tag = "hardware:c32,version:v1.5") + service = "service-2", repeat = 10, tag = "hardware:c32,version:v1.5" + ) val service2SecondaryStats = callServiceRepeatedly( - service = "service-2", repeat = 10, tag = "role:secondary,version:v2.0") + service = "service-2", repeat = 10, tag = "role:secondary,version:v2.0" + ) // then assertThat(service1Stats.totalHits).isEqualTo(10) - assertThat(service1Stats.hits(service1MatchingContainer)).isEqualTo(10) + assertThat(service1Stats.hits(service1Matching)).isEqualTo(10) assertThat(service2MasterStats.totalHits).isEqualTo(10) - assertThat(service2MasterStats.hits(service2MasterContainer)).isEqualTo(10) + assertThat(service2MasterStats.hits(service2Master)).isEqualTo(10) assertThat(service2SecondaryStats.totalHits).isEqualTo(10) - assertThat(service2SecondaryStats.hits(service2SecondaryContainer)).isEqualTo(10) + assertThat(service2SecondaryStats.hits(service2Secondary)).isEqualTo(10) } @Test open fun `should not route request with two tags if combination is not allowed`() { // given - val matchingContainer = loremContainer + val matching = loremService - registerService( - name = "service-2", container = matchingContainer, - tags = listOf("version:v1.5", "hardware:c32", "role:master")) + consul.server.operations.registerService( + name = "service-2", extension = matching, + tags = listOf("version:v1.5", "hardware:c32", "role:master") + ) - waitForReadyServices("service-2") + envoy.waitForReadyServices("service-2") // when val stats = callServiceRepeatedly( - service = "service-2", repeat = 10, tag = "hardware:c32,role:master", assertNoErrors = false) + service = "service-2", repeat = 10, tag = "hardware:c32,role:master", assertNoErrors = false + ) // then assertThat(stats.totalHits).isEqualTo(10) assertThat(stats.failedHits).isEqualTo(10) - assertThat(stats.hits(matchingContainer)).isEqualTo(0) + assertThat(stats.hits(matching)).isEqualTo(0) } - protected fun callEchoServiceRepeatedly(repeat: Int, tag: String? = null, assertNoErrors: Boolean = true): CallStats { + protected fun callEchoServiceRepeatedly( + repeat: Int, + tag: String? = null, + assertNoErrors: Boolean = true + ): CallStats { return callServiceRepeatedly( service = "echo", repeat = repeat, @@ -304,11 +343,16 @@ open class ServiceTagsTest : EnvoyControlTestConfiguration() { ) } - protected open fun callStats() = CallStats(listOf(regularContainer, loremContainer) + containersToStart) + protected open fun callStats() = CallStats(listOf(regularService, loremService, loremIpsumService, genericService)) - protected open fun callServiceRepeatedly(service: String, repeat: Int, tag: String? = null, assertNoErrors: Boolean = true): CallStats { + protected open fun callServiceRepeatedly( + service: String, + repeat: Int, + tag: String? = null, + assertNoErrors: Boolean = true + ): CallStats { val stats = callStats() - callServiceRepeatedly( + envoy.egressOperations.callServiceRepeatedly( service = service, stats = stats, minRepeat = repeat, diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/CallStats.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/CallStats.kt index 5a02ee361..d12a88c69 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/CallStats.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/CallStats.kt @@ -1,17 +1,20 @@ package pl.allegro.tech.servicemesh.envoycontrol.config.envoy -import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoContainer +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension -class CallStats(private val containers: List) { +class CallStats(private val serviceExtensions: List) { var failedHits: Int = 0 var totalHits: Int = 0 - private var containerHits: MutableMap = containers.associate { it.containerId to 0 }.toMutableMap() + private var containerHits: MutableMap = + serviceExtensions.associate { it.container().containerId to 0 }.toMutableMap() - fun hits(container: EchoContainer) = containerHits[container.containerId] ?: 0 + fun hits(extension: EchoServiceExtension) = containerHits[extension.container().containerId] ?: 0 fun addResponse(response: ResponseWithBody) { - containers.firstOrNull { response.isFrom(it) } + serviceExtensions + .map { it.container() } + .firstOrNull { response.isFrom(it) } ?.let { containerHits.compute(it.containerId) { _, i -> i?.inc() } } if (!response.isOk()) failedHits++ totalHits++ diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt index f6f451900..21d029eab 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt @@ -6,6 +6,7 @@ import org.junit.jupiter.api.extension.AfterEachCallback import org.junit.jupiter.api.extension.BeforeAllCallback import org.junit.jupiter.api.extension.ExtensionContext import org.testcontainers.containers.Network +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk import pl.allegro.tech.servicemesh.envoycontrol.assertions.isRbacAccessLog import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyConfig @@ -53,6 +54,16 @@ class EnvoyExtension( container.admin().resetCounters() } + fun waitForReadyServices(vararg serviceNames: String) { + serviceNames.forEach { + untilAsserted { + egressOperations.callService(it).also { + assertThat(it).isOk() + } + } + } + } + fun waitForAvailableEndpoints(vararg serviceNames: String) { val admin = container.admin() serviceNames.forEach { diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/StatusRouteIncomingPermissionsTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/StatusRouteIncomingPermissionsTest.kt index 4c5bcdef8..b200a1d88 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/StatusRouteIncomingPermissionsTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/StatusRouteIncomingPermissionsTest.kt @@ -1,5 +1,6 @@ package pl.allegro.tech.servicemesh.envoycontrol.permissions +import okhttp3.Headers import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.RegisterExtension @@ -40,14 +41,14 @@ internal class StatusRouteIncomingPermissionsTest { @JvmField @RegisterExtension - val envoy = EnvoyExtension(envoyControl, localService = service) + val envoy = EnvoyExtension(envoyControl, service) } @Test fun `should allow access to status endpoint by all clients`() { untilAsserted { // when - val response = envoy.ingressOperations.callLocalService("/status/") + val response = envoy.ingressOperations.callLocalService("/status/", Headers.of()) val statusUpstreamOk = envoy.container.admin().statValue( "vhost.secured_local_service.vcluster.status.upstream_rq_200" )?.toInt() diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/TlsBasedAuthenticationTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/TlsBasedAuthenticationTest.kt index 04b569b1d..f9ae67a7b 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/TlsBasedAuthenticationTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/TlsBasedAuthenticationTest.kt @@ -204,13 +204,13 @@ internal class TlsBasedAuthenticationTest { assertNoErrors = true, minRepeat = 2, maxRepeat = 2, - stats = CallStats(listOf(service1.container(), service2.container())) + stats = CallStats(listOf(service1, service2)) ) // then assertThat(callStats.failedHits).isEqualTo(0) - assertThat(callStats.hits(service1.container())).isEqualTo(1) - assertThat(callStats.hits(service2.container())).isEqualTo(1) + assertThat(callStats.hits(service1)).isEqualTo(1) + assertThat(callStats.hits(service2)).isEqualTo(1) val defaultToPlaintextMatchesCount = echo1Envoy.container.admin().statValue("cluster.echo2.plaintext_match.total_match_count")?.toInt() assertThat(defaultToPlaintextMatchesCount).isEqualTo(1) diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ssl/EnvoyHttpsDependencyTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ssl/EnvoyHttpsDependencyTest.kt index 14325038e..ecc05cac0 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ssl/EnvoyHttpsDependencyTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ssl/EnvoyHttpsDependencyTest.kt @@ -4,42 +4,50 @@ import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test -import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlRunnerTestApp -import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyControlTestConfiguration -import pl.allegro.tech.servicemesh.envoycontrol.config.service.HttpsEchoContainer +import org.junit.jupiter.api.extension.RegisterExtension import pl.allegro.tech.servicemesh.envoycontrol.assertions.hasSNI import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk -import pl.allegro.tech.servicemesh.envoycontrol.config.service.asHttpsEchoResponse - -class EnvoyCurrentVersionHttpsDependencyTest : EnvoyHttpsDependencyTest() { - companion object { - @JvmStatic - @BeforeAll - fun setupTest() { - setup(appFactoryForEc1 = { consulPort -> EnvoyControlRunnerTestApp(properties, consulPort) }) - setupTestCommon() - } - } -} +import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.GenericServiceExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.HttpsEchoContainer +import pl.allegro.tech.servicemesh.envoycontrol.config.service.HttpsEchoResponse -abstract class EnvoyHttpsDependencyTest : EnvoyControlTestConfiguration() { +class EnvoyHttpsDependencyTest { companion object { @JvmStatic protected val properties = mapOf( "envoy-control.envoy.snapshot.trustedCaFile" to "/app/root-ca.crt" ) - val httpsEchoContainer = HttpsEchoContainer() + + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension(consul, properties) + + @JvmField + @RegisterExtension + val httpsService = GenericServiceExtension(HttpsEchoContainer()) + + @JvmField + @RegisterExtension + val envoy = EnvoyExtension(envoyControl, httpsService) @JvmStatic - fun setupTestCommon() { - httpsEchoContainer.start() - envoyContainer1.addHost("my.example.com", httpsEchoContainer.ipAddress()) + @BeforeAll + fun setup() { + envoy.container.addHost("my.example.com", httpsService.container().ipAddress()) } @JvmStatic @AfterAll fun teardown() { - httpsEchoContainer.stop() + envoy.container.removeHost("my.example.com") } } @@ -47,13 +55,13 @@ abstract class EnvoyHttpsDependencyTest : EnvoyControlTestConfiguration() { fun `should include SNI in request to upstream`() { // when val response = untilAsserted { - val response = callDomain("my.example.com").asHttpsEchoResponse() + val response = envoy.egressOperations.callDomain("my.example.com") assertThat(response).isOk() response } // then - assertThat(response).hasSNI("my.example.com") + assertThat(HttpsEchoResponse(response)).hasSNI("my.example.com") } } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ssl/EnvoySANValidationTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ssl/EnvoySANValidationTest.kt index f6e680fd7..8288bc300 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ssl/EnvoySANValidationTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ssl/EnvoySANValidationTest.kt @@ -4,30 +4,50 @@ import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test -import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlRunnerTestApp -import pl.allegro.tech.servicemesh.envoycontrol.config.EnvoyControlTestConfiguration +import org.junit.jupiter.api.extension.RegisterExtension +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isUnreachable +import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.GenericServiceExtension import pl.allegro.tech.servicemesh.envoycontrol.config.service.HttpsEchoContainer -class EnvoySANValidationTest : EnvoyControlTestConfiguration() { +class EnvoySANValidationTest { companion object { private val properties = mapOf( "envoy-control.envoy.snapshot.trustedCaFile" to "/app/root-ca.crt" ) - val httpsEchoContainer = HttpsEchoContainer() + + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension(consul, properties) + + @JvmField + @RegisterExtension + val httpsService = GenericServiceExtension(HttpsEchoContainer()) + + @JvmField + @RegisterExtension + val envoy = EnvoyExtension(envoyControl, httpsService) @JvmStatic @BeforeAll - fun setupTest() { - setup(appFactoryForEc1 = { consulPort -> EnvoyControlRunnerTestApp(properties, consulPort) }) - httpsEchoContainer.start() - envoyContainer1.addHost("my.example.com", httpsEchoContainer.ipAddress()) - envoyContainer1.addHost("bad.host.example.com", httpsEchoContainer.ipAddress()) + fun setup() { + envoy.container.addHost("my.example.com", httpsService.container().ipAddress()) + envoy.container.addHost("bad.host.example.com", httpsService.container().ipAddress()) } @JvmStatic @AfterAll fun teardown() { - httpsEchoContainer.stop() + envoy.container.removeHost("my.example.com") + envoy.container.removeHost("bad.host.example.com") } } @@ -35,7 +55,7 @@ class EnvoySANValidationTest : EnvoyControlTestConfiguration() { fun `should validate SAN part of certificate`() { untilAsserted { // when - val reachableResponse = callDomain("my.example.com") + val reachableResponse = envoy.egressOperations.callDomain("my.example.com") assertThat(reachableResponse).isOk() } @@ -45,9 +65,9 @@ class EnvoySANValidationTest : EnvoyControlTestConfiguration() { fun `should reject certificate without matching SAN`() { untilAsserted { // when - val response = callDomain("bad.host.example.com") + val response = envoy.egressOperations.callDomain("bad.host.example.com") - val sanFailCount = envoyContainer1.admin().statValue( + val sanFailCount = envoy.container.admin().statValue( "cluster.bad_host_example_com_443.ssl.fail_verify_san" )?.toInt() assertThat(sanFailCount).isGreaterThan(0) From 19550eb34c7b84f9a11018cdc846de9c105e57c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Dziedziak?= Date: Wed, 13 Jan 2021 11:42:38 +0100 Subject: [PATCH 07/11] Extend tests (#241) * Fix problem with extending class * Fix other extensions * Added function which check if service deregistered Co-authored-by: Lukasz Dziedziak --- .../envoycontrol/CanaryLoadBalancingTest.kt | 40 ++++++++++++------- .../envoycontrol/ServiceTagsTest.kt | 26 +++++++----- .../envoycontrol/SnapshotDebugTest.kt | 36 ++++++++++------- .../config/envoy/EnvoyExtension.kt | 9 +++++ 4 files changed, 71 insertions(+), 40 deletions(-) diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/CanaryLoadBalancingTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/CanaryLoadBalancingTest.kt index 6e2cd93c7..7ab2c9b6a 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/CanaryLoadBalancingTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/CanaryLoadBalancingTest.kt @@ -48,11 +48,11 @@ open class CanaryLoadBalancingTest { @Test fun `should balance load according to weights`() { // given - consul.server.operations.registerService(name = "echo", extension = canaryContainer, tags = listOf("canary", "weight:1")) - consul.server.operations.registerService(name = "echo", extension = regularContainer, tags = listOf("weight:20")) + consul.server.operations.registerService(name = "echo", extension = canaryContainer(), tags = listOf("canary", "weight:1")) + consul.server.operations.registerService(name = "echo", extension = regularContainer(), tags = listOf("weight:20")) untilAsserted { - envoy.egressOperations.callService("echo").also { + envoy().egressOperations.callService("echo").also { assertThat(it).isOk() } } @@ -61,7 +61,7 @@ open class CanaryLoadBalancingTest { val stats = callEchoServiceRepeatedly( minRepeat = 30, maxRepeat = 200, - repeatUntil = { response -> response.isFrom(canaryContainer.container()) } + repeatUntil = { response -> response.isFrom(canaryContainer().container()) } ) // then @@ -92,11 +92,11 @@ open class CanaryLoadBalancingTest { @Test fun `should route request to canary instance only`() { // given - consul.server.operations.registerService(name = "echo", extension = canaryContainer, tags = listOf("canary", "weight:1")) - consul.server.operations.registerService(name = "echo", extension = regularContainer, tags = listOf("weight:20")) + consul.server.operations.registerService(name = "echo", extension = canaryContainer(), tags = listOf("canary", "weight:1")) + consul.server.operations.registerService(name = "echo", extension = regularContainer(), tags = listOf("weight:20")) untilAsserted { - envoy.egressOperations.callService("echo").also { + envoy().egressOperations.callService("echo").also { assertThat(it).isOk() } } @@ -116,11 +116,11 @@ open class CanaryLoadBalancingTest { @Test open fun `should route to both canary and regular instances when canary weight is 0`() { - consul.server.operations.registerService(name = "echo", extension = canaryContainer, tags = listOf("canary", "weight:0")) - consul.server.operations.registerService(name = "echo", extension = regularContainer, tags = listOf("weight:20")) + consul.server.operations.registerService(name = "echo", extension = canaryContainer(), tags = listOf("canary", "weight:0")) + consul.server.operations.registerService(name = "echo", extension = regularContainer(), tags = listOf("weight:20")) untilAsserted { - envoy.egressOperations.callService("echo").also { + envoy().egressOperations.callService("echo").also { assertThat(it).isOk() } } @@ -129,7 +129,7 @@ open class CanaryLoadBalancingTest { val stats = callEchoServiceRepeatedly( minRepeat = 30, maxRepeat = 200, - repeatUntil = { response -> response.isFrom(canaryContainer.container()) } + repeatUntil = { response -> response.isFrom(canaryContainer().container()) } ) // then @@ -138,7 +138,7 @@ open class CanaryLoadBalancingTest { assertThat(stats.canaryHits).isGreaterThan(0) } - protected open fun callStats() = CallStats(listOf(canaryContainer, regularContainer)) + protected open fun callStats() = CallStats(listOf(canaryContainer(), regularContainer())) fun callEchoServiceRepeatedly( minRepeat: Int, @@ -147,7 +147,7 @@ open class CanaryLoadBalancingTest { headers: Map = mapOf() ): CallStats { val stats = callStats() - envoy.egressOperations.callServiceRepeatedly( + envoy().egressOperations.callServiceRepeatedly( service = "echo", stats = stats, minRepeat = minRepeat, @@ -159,7 +159,17 @@ open class CanaryLoadBalancingTest { } val CallStats.regularHits: Int - get() = this.hits(regularContainer) + get() = this.hits(regularContainer()) val CallStats.canaryHits: Int - get() = this.hits(canaryContainer) + get() = this.hits(canaryContainer()) + + open fun envoyControl() = envoyControl + + open fun envoy() = envoy + + open fun consul() = consul + + open fun canaryContainer() = canaryContainer + + open fun regularContainer() = regularContainer } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsTest.kt index e10e4d7e1..e2bba46fc 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsTest.kt @@ -72,7 +72,7 @@ open class ServiceTagsTest { open fun `should route requests to instance with tag ipsum`() { // given registerServices() - envoy.waitForReadyServices("echo") + envoy().waitForReadyServices("echo") // when val stats = callEchoServiceRepeatedly(repeat = 10, tag = "ipsum") @@ -88,7 +88,7 @@ open class ServiceTagsTest { open fun `should route requests to instances with tag lorem`() { // given registerServices() - envoy.waitForReadyServices("echo") + envoy().waitForReadyServices("echo") // when val stats = callEchoServiceRepeatedly(repeat = 20, tag = "lorem") @@ -105,7 +105,7 @@ open class ServiceTagsTest { open fun `should route requests to all instances`() { // given registerServices() - envoy.waitForReadyServices("echo") + envoy().waitForReadyServices("echo") // when val stats = callEchoServiceRepeatedly(repeat = 20) @@ -128,7 +128,7 @@ open class ServiceTagsTest { open fun `should return 503 if instance with requested tag is not found`() { // given registerServices() - envoy.waitForReadyServices("echo") + envoy().waitForReadyServices("echo") // when val stats = callEchoServiceRepeatedly(repeat = 10, tag = "dolom", assertNoErrors = false) @@ -145,7 +145,7 @@ open class ServiceTagsTest { open fun `should return 503 if requested tag is blacklisted`() { // given registerServices() - envoy.waitForReadyServices("echo") + envoy().waitForReadyServices("echo") // when val stats = callEchoServiceRepeatedly(repeat = 10, tag = "blacklisted", assertNoErrors = false) @@ -173,7 +173,7 @@ open class ServiceTagsTest { tags = listOf("version:v1.5", "hardware:c64", "role:master") ) - envoy.waitForReadyServices("service-1") + envoy().waitForReadyServices("service-1") // when val stats = callServiceRepeatedly( @@ -196,7 +196,7 @@ open class ServiceTagsTest { tags = listOf("version:v1.5", "hardware:c32", "role:master") ) - envoy.waitForReadyServices("service-3") + envoy().waitForReadyServices("service-3") // when val threeTagsStats = callServiceRepeatedly( @@ -238,7 +238,7 @@ open class ServiceTagsTest { tags = listOf("version:v1.5", "hardware:c32", "role:master") ) - envoy.waitForReadyServices("service-1", "service-2") + envoy().waitForReadyServices("service-1", "service-2") // when val service1Stats = callServiceRepeatedly( @@ -283,7 +283,7 @@ open class ServiceTagsTest { tags = listOf("version:v2.0", "hardware:c32", "role:secondary") ) - envoy.waitForReadyServices("service-1", "service-2") + envoy().waitForReadyServices("service-1", "service-2") // when val service1Stats = callServiceRepeatedly( @@ -317,7 +317,7 @@ open class ServiceTagsTest { tags = listOf("version:v1.5", "hardware:c32", "role:master") ) - envoy.waitForReadyServices("service-2") + envoy().waitForReadyServices("service-2") // when val stats = callServiceRepeatedly( @@ -352,7 +352,7 @@ open class ServiceTagsTest { assertNoErrors: Boolean = true ): CallStats { val stats = callStats() - envoy.egressOperations.callServiceRepeatedly( + envoy().egressOperations.callServiceRepeatedly( service = service, stats = stats, minRepeat = repeat, @@ -362,4 +362,8 @@ open class ServiceTagsTest { ) return stats } + + open fun envoy() = envoy + + open fun envoyControl() = envoyControl } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/SnapshotDebugTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/SnapshotDebugTest.kt index 0968c41ff..a21194891 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/SnapshotDebugTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/SnapshotDebugTest.kt @@ -33,16 +33,16 @@ open class SnapshotDebugTest { @Test open fun `should return snapshot debug info containing snapshot versions`() { // given - consul.server.operations.registerService(service, name = "echo") - val nodeMetadata = envoy.container.admin().nodeInfo() + consul().server.operations.registerService(service(), name = "echo") + val nodeMetadata = envoy().container.admin().nodeInfo() untilAsserted { // when - val snapshot = envoyControl.app.getSnapshot(nodeMetadata) - val edsVersion = envoy.container.admin().statValue("cluster.echo.version") - val cdsVersion = envoy.container.admin().statValue("cluster_manager.cds.version") - val rdsVersion = envoy.container.admin().statValue("http.egress_http.rds.default_routes.version") - val ldsVersion = envoy.container.admin().statValue("listener_manager.lds.version") + val snapshot = envoyControl().app.getSnapshot(nodeMetadata) + val edsVersion = envoy().container.admin().statValue("cluster.echo.version") + val cdsVersion = envoy().container.admin().statValue("cluster_manager.cds.version") + val rdsVersion = envoy().container.admin().statValue("http.egress_http.rds.default_routes.version") + val ldsVersion = envoy().container.admin().statValue("listener_manager.lds.version") // then assertThat(snapshot.versions!!.clusters.metric).isEqualTo(cdsVersion) @@ -55,12 +55,12 @@ open class SnapshotDebugTest { @Test open fun `should return snapshot debug info containing snapshot contents`() { // given - consul.server.operations.registerService(service, name = "echo") - val nodeMetadata = envoy.container.admin().nodeInfo() + consul().server.operations.registerService(service(), name = "echo") + val nodeMetadata = envoy().container.admin().nodeInfo() untilAsserted { // when - val snapshot = envoyControl.app.getSnapshot(nodeMetadata) + val snapshot = envoyControl().app.getSnapshot(nodeMetadata) // then assertThat(snapshot.snapshot!!["clusters"]).isNotEmpty() @@ -105,7 +105,7 @@ open class SnapshotDebugTest { @Test open fun `should inform about missing snapshot when given node does not exist`() { // when - val snapshot = envoyControl.app.getSnapshot(missingNodeJson) + val snapshot = envoyControl().app.getSnapshot(missingNodeJson) // then assertThat(snapshot.found).isFalse() @@ -115,7 +115,7 @@ open class SnapshotDebugTest { open fun `should return global snapshot debug info from xds`() { untilAsserted { // when - val snapshot = envoyControl.app.getGlobalSnapshot(xds = true) + val snapshot = envoyControl().app.getGlobalSnapshot(xds = true) // then assertThat(snapshot.snapshot!!["clusters"]).isNotEmpty() @@ -128,8 +128,8 @@ open class SnapshotDebugTest { open fun `should return global snapshot debug info from ads`() { untilAsserted { // when - val snapshotXdsNull = envoyControl.app.getGlobalSnapshot(xds = null) - val snapshotXdsFalse = envoyControl.app.getGlobalSnapshot(xds = false) + val snapshotXdsNull = envoyControl().app.getGlobalSnapshot(xds = null) + val snapshotXdsFalse = envoyControl().app.getGlobalSnapshot(xds = false) // then assertThat(snapshotXdsNull.snapshot!!["clusters"]).isNotEmpty() @@ -144,4 +144,12 @@ open class SnapshotDebugTest { ) } } + + open fun consul() = consul + + open fun envoyControl() = envoyControl + + open fun envoy() = envoy + + open fun service() = service } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt index 21d029eab..40adeeb7c 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt @@ -73,6 +73,15 @@ class EnvoyExtension( } } + fun waitForNoAvailableEndpoints(vararg serviceNames: String) { + val admin = container.admin() + serviceNames.forEach { + untilAsserted { + assertThat(admin.numOfEndpoints(it)).isEqualTo(0) + } + } + } + fun recordRBACLogs() { container.logRecorder.recordLogs(::isRbacAccessLog) } From 0f8edf5345604be6b8532f7989ed7aef262094fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Dziedziak?= Date: Tue, 19 Jan 2021 13:42:53 +0100 Subject: [PATCH 08/11] Allow defined clients to use all endpoints (#219) * Allow defined clients to use all endpoints * Renamed properties and vars * Fixed name * Log request from allowed clients * Remove deprecated field * Log if it is special client * Fixed one condition * Fixed allowed_client value assign, removed unnecessary config, added new test * Simplified condition - review, upgrade testcontainer version * Moved client_name check out of the loop Co-authored-by: Lukasz Dziedziak --- build.gradle | 2 +- docs/configuration.md | 1 + .../snapshot/SnapshotProperties.kt | 1 + .../listeners/filters/LuaFilterFactory.kt | 23 ++++-- .../listeners/filters/RBACFilterFactory.kt | 9 ++- .../resources/lua/ingress_rbac_logging.lua | 42 +++++++++-- .../filters/RBACFilterFactoryTest.kt | 60 ++++++++++++++- .../src/main/resources/application.yaml | 2 +- .../assertions/EnvoyAssertions.kt | 7 ++ .../IncomingPermissionsLoggingModeTest.kt | 31 +++++++- .../lua_spec/ingress_rbac_logging_spec.lua | 75 ++++++++++++++++--- 11 files changed, 225 insertions(+), 28 deletions(-) diff --git a/build.gradle b/build.gradle index 8cdb985c2..174ec0d3a 100644 --- a/build.gradle +++ b/build.gradle @@ -62,7 +62,7 @@ allprojects { assertj : '3.16.1', jackson : '2.11.2', toxiproxy : '2.1.3', - testcontainers : '1.15.0-rc2', + testcontainers : '1.15.1', reactor : '3.3.10.RELEASE', consul_recipes : '0.8.3', mockito : '3.3.3', diff --git a/docs/configuration.md b/docs/configuration.md index d0a9eea7d..a95a8d319 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -65,6 +65,7 @@ Property -------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- **envoy-control.envoy.snapshot.incoming-permissions.enabled** | Enable incoming permissions | false **envoy-control.envoy.snapshot.incoming-permissions.client-identity-headers** | Headers that identify the client calling the endpoint. In most cases `client-identity-header` should include `service-name-header` value to correctly identify other services in the mesh. | [ x-service-name ] +**envoy-control.envoy.snapshot.incoming-permissions.clients-allowed-to-all-endpoints** | Client names which are allowed to even call service if incoming permissions are enabled. | empty list **envoy-control.envoy.snapshot.incoming-permissions.request-identification-headers** | Headers that are used to identify requests in incoming permissions logs. | [ x-request-id ] **envoy-control.envoy.snapshot.incoming-permissions.trusted-client-identity-header** | Header that securely identify the client calling the endpoint. It's added by Envoy to a request to local service. Local service can trust this header, it always contains only confirmed client identities. Set to empty string to disable. | x-client-name-trusted **envoy-control.envoy.snapshot.incoming-permissions.service-name-header** | Name of a header to propagate a called endpoint's service name upstream | x-service-name diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt index 82ff97722..78a30fb79 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt @@ -73,6 +73,7 @@ class IncomingPermissionsProperties { var sourceIpAuthentication = SourceIpAuthenticationProperties() var selectorMatching: MutableMap = mutableMapOf() var tlsAuthentication = TlsAuthenticationProperties() + var clientsAllowedToAllEndpoints = mutableListOf() var overlappingPathsFix = false // TODO: to be removed when proved it did not mess up anything } diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/LuaFilterFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/LuaFilterFactory.kt index 8a9c2c270..e50d55d7d 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/LuaFilterFactory.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/LuaFilterFactory.kt @@ -49,17 +49,28 @@ class LuaFilterFactory(incomingPermissionsProperties: IncomingPermissionsPropert .build() ).build() ) - .putFields("trusted_client_identity_header", + .putFields( + "trusted_client_identity_header", Value.newBuilder() .setStringValue(trustedClientIdentityHeader) .build() ) - .putFields("san_uri_lua_pattern", + .putFields( + "san_uri_lua_pattern", Value.newBuilder() .setStringValue( SanUriMatcherFactory(incomingPermissionsProperties.tlsAuthentication) .sanUriWildcardRegexForLua ).build() + ).putFields("clients_allowed_to_all_endpoints", + Value.newBuilder() + .setListValue(ListValue.newBuilder() + .addAllValues( + incomingPermissionsProperties.clientsAllowedToAllEndpoints + .map { Value.newBuilder().setStringValue(it).build() } + ) + .build() + ).build() ) .build() ).build() @@ -68,13 +79,13 @@ class LuaFilterFactory(incomingPermissionsProperties: IncomingPermissionsPropert ingressRbacLoggingFilter.takeIf { group.proxySettings.incoming.permissionsEnabled } private val ingressClientNameHeaderScript: String = this::class.java.classLoader - .getResource("lua/ingress_client_name_header.lua")!!.readText() + .getResource("lua/ingress_client_name_header.lua")!!.readText() private val ingressClientNameHeaderFilter: HttpFilter = HttpFilter.newBuilder() - .setName("ingress.client.lua") - .setTypedConfig(Any.pack(Lua.newBuilder().setInlineCode(ingressClientNameHeaderScript).build())) - .build() + .setName("ingress.client.lua") + .setTypedConfig(Any.pack(Lua.newBuilder().setInlineCode(ingressClientNameHeaderScript).build())) + .build() fun ingressClientNameHeaderFilter(): HttpFilter? = ingressClientNameHeaderFilter.takeIf { trustedClientIdentityHeader.isNotEmpty() } diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterFactory.kt index 3a61a5dd0..6bbdc9a99 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterFactory.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterFactory.kt @@ -39,6 +39,9 @@ class RBACFilterFactory( private val anyPrincipal = Principal.newBuilder().setAny(true).build() private val denyForAllPrincipal = Principal.newBuilder().setNotId(anyPrincipal).build() + private val fullAccessClients = incomingPermissionsProperties.clientsAllowedToAllEndpoints.map { + ClientWithSelector(name = it) + } private val sanUriMatcherFactory = SanUriMatcherFactory(incomingPermissionsProperties.tlsAuthentication) init { @@ -199,7 +202,7 @@ class RBACFilterFactory( roles.find { it.name == clientOrRole.name }?.clients ?: setOf(clientOrRole) } // sorted order ensures that we do not duplicate rules - return clients.toSortedSet() + return (clients + fullAccessClients).toSortedSet() } private fun mapClientWithSelectorToPrincipals( @@ -258,7 +261,7 @@ class RBACFilterFactory( val principals = it.value.map { ipWithPrefix -> val (ip, prefixLength) = ipWithPrefix.split("/") - Principal.newBuilder().setSourceIp(CidrRange.newBuilder() + Principal.newBuilder().setDirectRemoteIp(CidrRange.newBuilder() .setAddressPrefix(ip) .setPrefixLen(UInt32Value.of(prefixLength.toInt())).build()) .build() @@ -292,7 +295,7 @@ class RBACFilterFactory( } }.orEmpty().map { address -> Principal.newBuilder() - .setSourceIp(CidrRange.newBuilder() + .setDirectRemoteIp(CidrRange.newBuilder() .setAddressPrefix(address.socketAddress.address) .setPrefixLen(EXACT_IP_MASK).build()) .build() diff --git a/envoy-control-core/src/main/resources/lua/ingress_rbac_logging.lua b/envoy-control-core/src/main/resources/lua/ingress_rbac_logging.lua index 9652cd505..2b97fd4b7 100644 --- a/envoy-control-core/src/main/resources/lua/ingress_rbac_logging.lua +++ b/envoy-control-core/src/main/resources/lua/ingress_rbac_logging.lua @@ -4,13 +4,16 @@ function envoy_on_request(handle) local xff_header = handle:headers():get("x-forwarded-for") local metadata = handle:streamInfo():dynamicMetadata() local client_identity_header_names = handle:metadata():get("client_identity_headers") or {} + local clients_allowed_to_all_endpoints = handle:metadata():get("clients_allowed_to_all_endpoints") or {} local request_id_header_names = handle:metadata():get("request_id_headers") or {} local request_id = first_header_value_from_list(request_id_header_names, handle) local trusted_header_name = handle:metadata():get("trusted_client_identity_header") or "" local client_name = "" + local allowed_client = false local trusted_client = false if trusted_header_name ~= "" then client_name = handle:headers():get(trusted_header_name) or "" + allowed_client = is_allowed_client(client_name, clients_allowed_to_all_endpoints) if client_name ~= "" then trusted_client = true end @@ -18,6 +21,7 @@ function envoy_on_request(handle) if client_name == "" then client_name = first_header_value_from_list(client_identity_header_names, handle) + allowed_client = is_allowed_client(client_name, clients_allowed_to_all_endpoints) if trusted_header_name ~= "" and client_name ~= "" and handle:connection():ssl() ~= nil then client_name = client_name .. " (not trusted)" end @@ -28,6 +32,7 @@ function envoy_on_request(handle) metadata:set("envoy.filters.http.lua", "request.info.xff_header", xff_header) metadata:set("envoy.filters.http.lua", "request.info.client_name", client_name) metadata:set("envoy.filters.http.lua", "request.info.trusted_client", trusted_client) + metadata:set("envoy.filters.http.lua", "request.info.allowed_client", allowed_client) metadata:set("envoy.filters.http.lua", "request.info.request_id", request_id) end @@ -42,16 +47,43 @@ function first_header_value_from_list(header_list, handle) return "" end +function is_allowed_client(client_name, clients_allowed_to_all_endpoints) + if client_name == "" then + return false + end + for _,h in ipairs(clients_allowed_to_all_endpoints) do + if h ~= "" and client_name == h then + return true + end + end + + return false +end + function envoy_on_response(handle) - local rbacMetadata = handle:streamInfo():dynamicMetadata():get("envoy.filters.http.rbac") + local rbacMetadata = handle:streamInfo():dynamicMetadata():get("envoy.filters.http.rbac") or {} + local lua_metadata = handle:streamInfo():dynamicMetadata():get("envoy.filters.http.lua") or {} + local allowed_client = lua_metadata["request.info.allowed_client"] or false - if rbacMetadata == nil or rbacMetadata["shadow_engine_result"] ~= "denied" then - return + if should_log_request(rbacMetadata, allowed_client) then + log_request(lua_metadata, handle) end +end - local lua_metadata = handle:streamInfo():dynamicMetadata():get("envoy.filters.http.lua") or {} +function should_log_request(rbacMetadata, allowed_client) + local is_shadow_denied = (rbacMetadata["shadow_engine_result"] or "") == "denied" + + if (not is_shadow_denied) and allowed_client == true then + return true + end + + return is_shadow_denied +end + +function log_request(lua_metadata, handle) local client_name = lua_metadata["request.info.client_name"] or "" local trusted_client = lua_metadata["request.info.trusted_client"] or false + local allowed_client = lua_metadata["request.info.allowed_client"] or false local path = lua_metadata["request.info.path"] or "" local protocol = handle:connection():ssl() == nil and "http" or "https" local method = lua_metadata["request.info.method"] or "" @@ -59,7 +91,7 @@ function envoy_on_response(handle) local source_ip = string.match(xff_header, '[^,]+$') or "" local request_id = lua_metadata["request.info.request_id"] or "" local statusCode = handle:headers():get(":status") or "0" - handle:logInfo("\nINCOMING_PERMISSIONS { \"method\": \""..method.."\", \"path\": \""..path.."\", \"clientIp\": \""..source_ip.."\", \"clientName\": \""..escape(client_name).."\", \"trustedClient\": "..tostring(trusted_client)..", \"protocol\": \""..protocol.."\", \"requestId\": \""..escape(request_id).."\", \"statusCode\": "..statusCode.." }") + handle:logInfo("\nINCOMING_PERMISSIONS { \"method\": \""..method.."\", \"path\": \""..path.."\", \"clientIp\": \""..source_ip.."\", \"clientName\": \""..escape(client_name).."\", \"trustedClient\": "..tostring(trusted_client)..", \"clientAllowedToAllEndpoints\": "..tostring(allowed_client)..", \"protocol\": \""..protocol.."\", \"requestId\": \""..escape(request_id).."\", \"statusCode\": "..statusCode.." }") end escapeList = { diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterFactoryTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterFactoryTest.kt index f9d1755b5..f537c2844 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterFactoryTest.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterFactoryTest.kt @@ -83,6 +83,13 @@ internal class RBACFilterFactoryTest { }, StatusRouteProperties() ) + private val rbacFilterFactoryWithAllowAllEndpointsForClient = RBACFilterFactory( + IncomingPermissionsProperties().also { + it.enabled = true + it.clientsAllowedToAllEndpoints = mutableListOf("allowed-client") + }, + StatusRouteProperties() + ) val snapshot = GlobalSnapshot( SnapshotResources.create(listOf(), ""), @@ -702,6 +709,30 @@ internal class RBACFilterFactoryTest { assertThat(generated).isEqualTo(expectedRbacBuilder) } + @Test + fun `should generate RBAC rules for incoming permissions with client allowed to all endpoints`() { + // given + val expectedRbacBuilder = getRBACFilter(expectedEndpointPermissionsWithAllowedClientForAllEndpoints) + val incomingPermission = Incoming( + permissionsEnabled = true, + endpoints = listOf(IncomingEndpoint( + "/example", + PathMatchingType.PATH, + setOf("GET"), + setOf(ClientWithSelector("client1")) + )) + ) + + // when + val generated = rbacFilterFactoryWithAllowAllEndpointsForClient.createHttpFilter( + createGroup(incomingPermission), + snapshotForSourceIpAuth + ) + + // then + assertThat(generated).isEqualTo(expectedRbacBuilder) + } + private val expectedEndpointPermissionsWithDifferentRulesForDifferentClientsJson = """ { "policies": { @@ -1262,6 +1293,33 @@ internal class RBACFilterFactoryTest { } """ + private val expectedEndpointPermissionsWithAllowedClientForAllEndpoints = """ + { + "policies": { + "IncomingEndpoint(path=/example, pathMatchingType=PATH, methods=[GET], clients=[ClientWithSelector(name=client1, selector=null)], unlistedClientsPolicy=BLOCKANDLOG)": { + "permissions": [ + { + "and_rules": { + "rules": [ + ${pathRule("/example")}, + { + "or_rules": { + "rules": [ + ${methodRule("GET")} + ] + } + } + ] + } + } + ], "principals": [ + ${authenticatedPrincipal("allowed-client")}, ${authenticatedPrincipal("client1")} + ] + } + } + } + """ + private fun pathRule(path: String): String { return """{ "url_path": { @@ -1293,7 +1351,7 @@ internal class RBACFilterFactoryTest { private fun principalSourceIp(address: String, prefixLen: Int = 32): String { return """{ - "source_ip": { + "direct_remote_ip": { "address_prefix": "$address", "prefix_len": $prefixLen } diff --git a/envoy-control-runner/src/main/resources/application.yaml b/envoy-control-runner/src/main/resources/application.yaml index c9a928831..46ea03a2c 100644 --- a/envoy-control-runner/src/main/resources/application.yaml +++ b/envoy-control-runner/src/main/resources/application.yaml @@ -17,4 +17,4 @@ management: metrics.enabled: true prometheus.enabled: true endpoints.web.exposure.include: "*" - metrics.export.prometheus.enabled: true \ No newline at end of file + metrics.export.prometheus.enabled: true diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/assertions/EnvoyAssertions.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/assertions/EnvoyAssertions.kt index c850d8d78..d34808dce 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/assertions/EnvoyAssertions.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/assertions/EnvoyAssertions.kt @@ -12,6 +12,7 @@ private class RbacLog( val method: String? = null, val clientName: String? = null, val trustedClient: Boolean? = null, + val clientAllowedToAllEndpoints: Boolean? = null, val clientIp: String? = null, val statusCode: String? = null, val requestId: String? = null @@ -50,6 +51,7 @@ fun ObjectAssert.hasOneAccessDenialWithActionBlock( clientName = clientName, trustedClient = trustedClient, clientIp = clientIp, + clientAllowedToAllEndpoints = false, statusCode = "403" ) ) @@ -60,6 +62,7 @@ fun ObjectAssert.hasOneAccessDenialWithActionLog( method: String? = null, clientName: String? = null, trustedClient: Boolean? = null, + clientAllowedToAllEndpoints: Boolean? = null, clientIp: String? = null, requestId: String? = null ): ObjectAssert = hasOneAccessDenial( @@ -73,6 +76,7 @@ fun ObjectAssert.hasOneAccessDenialWithActionLog( statusCode = "200", clientName = clientName, trustedClient = trustedClient, + clientAllowedToAllEndpoints = clientAllowedToAllEndpoints, requestId = requestId ) ) @@ -118,6 +122,9 @@ private fun ObjectAssert.matchesRbacAccessDeniedLog(logPredicate: RbacLo logPredicate.trustedClient?.let { assertThat(parsed.trustedClient).isEqualTo(logPredicate.trustedClient) } + logPredicate.clientAllowedToAllEndpoints?.let { + assertThat(parsed.clientAllowedToAllEndpoints).isEqualTo(logPredicate.clientAllowedToAllEndpoints) + } logPredicate.statusCode?.let { assertThat(parsed.statusCode).isEqualTo(logPredicate.statusCode) } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsLoggingModeTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsLoggingModeTest.kt index b168ffffb..46ad48e2c 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsLoggingModeTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsLoggingModeTest.kt @@ -33,7 +33,8 @@ internal class IncomingPermissionsLoggingModeTest : EnvoyControlTestConfiguratio "$sourceClientIp/32", "$prefix.routes.status.create-virtual-cluster" to true, "$prefix.routes.status.endpoints" to mutableListOf(EndpointMatch().also { it.path = "/status/" }), - "$prefix.routes.status.enabled" to true + "$prefix.routes.status.enabled" to true, + "$prefix.incoming-permissions.clients-allowed-to-all-endpoints" to listOf("allowed-client") ) } // language=yaml @@ -710,6 +711,34 @@ internal class IncomingPermissionsLoggingModeTest : EnvoyControlTestConfiguratio ) } + @Test + fun `echo2 should allow special client with client identity header over https and log request`() { + // given + val insecureClient = ClientsFactory.createInsecureClient() + + // when + val echo2Response = callEnvoyIngress( + envoy = echo2Envoy, + path = "/log-special-clients", + headers = mapOf("x-service-name" to "allowed-client"), + useSsl = true, + client = insecureClient + ) + + // then + assertThat(echo2Response).isOk().isFrom(echo2LocalService) + assertThat(echo2Envoy.ingressSslRequests).isOne() + assertThat(echo2Envoy).hasOneAccessDenialWithActionLog( + protocol = "https", + path = "/log-special-clients", + method = "GET", + clientName = "allowed-client (not trusted)", + trustedClient = false, + clientAllowedToAllEndpoints = true, + clientIp = echo2Envoy.gatewayIp() + ) + } + @BeforeEach fun startRecordingRBACLogs() { echoEnvoy.recordRBACLogs() diff --git a/envoy-control-tests/src/main/resources/lua_spec/ingress_rbac_logging_spec.lua b/envoy-control-tests/src/main/resources/lua_spec/ingress_rbac_logging_spec.lua index 2fd69fbe8..e99856c5c 100644 --- a/envoy-control-tests/src/main/resources/lua_spec/ingress_rbac_logging_spec.lua +++ b/envoy-control-tests/src/main/resources/lua_spec/ingress_rbac_logging_spec.lua @@ -2,8 +2,8 @@ require("ingress_rbac_logging") local _ = match._ local contains = function(substring) return match.matches(substring, nil, true) end -local function formatLog(method, path, source_ip, client_name, protocol, request_id, status_code, trusted_client) - return "\nINCOMING_PERMISSIONS { \"method\": \"" .. method .. "\", \"path\": \"" .. path .. "\", \"clientIp\": \"" .. source_ip .. "\", \"clientName\": \"" .. client_name .. "\", \"trustedClient\": " .. tostring(trusted_client) .. ", \"protocol\": \"" .. protocol .. "\", \"requestId\": \"" .. request_id .. "\", \"statusCode\": " .. status_code .. " }" +local function formatLog(method, path, source_ip, client_name, protocol, request_id, status_code, trusted_client, allowed_client) + return "\nINCOMING_PERMISSIONS { \"method\": \"" .. method .. "\", \"path\": \"" .. path .. "\", \"clientIp\": \"" .. source_ip .. "\", \"clientName\": \"" .. client_name .. "\", \"trustedClient\": " .. tostring(trusted_client) .. ", \"clientAllowedToAllEndpoints\": " .. tostring(allowed_client) .. ", \"protocol\": \"" .. protocol .. "\", \"requestId\": \"" .. request_id .. "\", \"statusCode\": " .. status_code .. " }" end local function handlerMock(headers, dynamic_metadata, https, filter_metadata) @@ -108,6 +108,46 @@ describe("envoy_on_request:", function() assert.spy(metadata.set).was_called_with(_, "envoy.filters.http.lua", "request.info.request_id", "123-456-789") end) + it("should set allowed_client for defined client", function() + -- given + local headers = { + ['x-service-name'] = 'allowed_client' + } + local filter_metadata = { + ['client_identity_headers'] = { 'x-service-name' }, + ['clients_allowed_to_all_endpoints'] = { 'allowed_client' } + } + + local handle = handlerMock(headers, {}, nil, filter_metadata) + local metadata = handle:streamInfo():dynamicMetadata() + + -- when + envoy_on_request(handle) + + -- then + assert.spy(metadata.set).was_called_with(_, "envoy.filters.http.lua", "request.info.allowed_client", true) + end) + + it("should set allowed_client to false for unknown client", function() + -- given + local headers = { + ['x-service-name'] = 'not_allowed_client' + } + local filter_metadata = { + ['client_identity_headers'] = { 'x-service-name' }, + ['clients_allowed_to_all_endpoints'] = { 'allowed_client' } + } + + local handle = handlerMock(headers, {}, nil, filter_metadata) + local metadata = handle:streamInfo():dynamicMetadata() + + -- when + envoy_on_request(handle) + + -- then + assert.spy(metadata.set).was_called_with(_, "envoy.filters.http.lua", "request.info.allowed_client", false) + end) + it("should set client_name from x-client-name-trusted header", function() -- given local headers = { @@ -312,7 +352,7 @@ describe("envoy_on_response:", function() envoy_on_response(handle) -- then - assert.spy(handle.logInfo).was_called_with(_, formatLog("POST", "/path?query=val", "127.1.1.3", "service-first", "https", "", "403", false)) + assert.spy(handle.logInfo).was_called_with(_, formatLog("POST", "/path?query=val", "127.1.1.3", "service-first", "https", "", "403", false, false)) assert.spy(handle.logInfo).was_called(1) end) @@ -325,7 +365,7 @@ describe("envoy_on_response:", function() envoy_on_response(handle) -- then - assert.spy(handle.logInfo).was_called_with(_, formatLog("POST", "/path?query=val", "127.1.1.3", "service-first", "http", "", "403", false)) + assert.spy(handle.logInfo).was_called_with(_, formatLog("POST", "/path?query=val", "127.1.1.3", "service-first", "http", "", "403", false, false)) assert.spy(handle.logInfo).was_called(1) end) @@ -338,7 +378,7 @@ describe("envoy_on_response:", function() envoy_on_response(handle) -- then - assert.spy(handle.logInfo).was_called_with(_, formatLog("POST", "/path?query=val", "127.1.1.3", "service-first", "https", "", "200", false)) + assert.spy(handle.logInfo).was_called_with(_, formatLog("POST", "/path?query=val", "127.1.1.3", "service-first", "https", "", "200", false, false)) assert.spy(handle.logInfo).was_called(1) end) @@ -352,7 +392,7 @@ describe("envoy_on_response:", function() envoy_on_response(handle) -- then - assert.spy(handle.logInfo).was_called_with(_, formatLog("", "", "", "", "https", "", "0", false)) + assert.spy(handle.logInfo).was_called_with(_, formatLog("", "", "", "", "https", "", "0", false, false)) assert.spy(handle.logInfo).was_called(1) end) @@ -366,7 +406,7 @@ describe("envoy_on_response:", function() envoy_on_response(handle) -- then - assert.spy(handle.logInfo).was_called_with(_, formatLog("", "", "", "", "https", "", "0", false)) + assert.spy(handle.logInfo).was_called_with(_, formatLog("", "", "", "", "https", "", "0", false, false)) assert.spy(handle.logInfo).was_called(1) end) @@ -379,7 +419,24 @@ describe("envoy_on_response:", function() envoy_on_response(handle) -- then - assert.spy(handle.logInfo).was_called_with(_, formatLog("POST", "", "127.1.1.3", "service-first", "https", "", "403", false)) + assert.spy(handle.logInfo).was_called_with(_, formatLog("POST", "", "127.1.1.3", "service-first", "https", "", "403", false, false)) + assert.spy(handle.logInfo).was_called(1) + end) + end) + + describe("should log requests:", function() + + it("with globally allowed client", function () + -- given + metadata['envoy.filters.http.rbac']['shadow_engine_result'] = 'allowed' + metadata['envoy.filters.http.lua']['request.info.allowed_client'] = true + local handle = handlerMock(headers, metadata, ssl) + + -- when + envoy_on_response(handle) + + -- then + assert.spy(handle.logInfo).was_called_with(_, formatLog("POST", "/path?query=val", "127.1.1.3", "service-first", "https", "", "403", false, true)) assert.spy(handle.logInfo).was_called(1) end) end) @@ -397,7 +454,6 @@ describe("envoy_on_response:", function() -- then assert.spy(handle.logInfo).was_not_called() - assert.spy(metadataMock.get).was_not_called_with(_, 'envoy.filters.http.lua') end) it("authorized request", function() @@ -411,7 +467,6 @@ describe("envoy_on_response:", function() -- then assert.spy(handle.logInfo).was_not_called() - assert.spy(metadataMock.get).was_not_called_with(_, 'envoy.filters.http.lua') end) end) From 87ec579af4ba5965d6476d91a33bcc59504575d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Dziedziak?= Date: Tue, 26 Jan 2021 11:34:13 +0100 Subject: [PATCH 09/11] Added more tests for allowed client (#244) * Added more tests for allowed client * Added one more test and moved from LoggingModeTest * Small changes - test names, ssl -> tls Co-authored-by: Lukasz Dziedziak --- .../assertions/EnvoyAssertions.kt | 38 ++- .../IncomingPermissionsAllowedClientTest.kt | 282 ++++++++++++++++++ .../IncomingPermissionsLoggingModeTest.kt | 28 -- 3 files changed, 316 insertions(+), 32 deletions(-) create mode 100644 envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsAllowedClientTest.kt diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/assertions/EnvoyAssertions.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/assertions/EnvoyAssertions.kt index d34808dce..53642d7f3 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/assertions/EnvoyAssertions.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/assertions/EnvoyAssertions.kt @@ -40,7 +40,8 @@ fun ObjectAssert.hasOneAccessDenialWithActionBlock( method: String, clientName: String, trustedClient: Boolean, - clientIp: String + clientIp: String, + clientAllowedToAllEndpoints: Boolean = false ): ObjectAssert = hasOneAccessDenial( requestBlocked = true, protocol = protocol, @@ -51,11 +52,37 @@ fun ObjectAssert.hasOneAccessDenialWithActionBlock( clientName = clientName, trustedClient = trustedClient, clientIp = clientIp, - clientAllowedToAllEndpoints = false, + clientAllowedToAllEndpoints = clientAllowedToAllEndpoints, statusCode = "403" ) ) +fun ObjectAssert.hasOneAccessAllowedWithActionLog( + protocol: String, + path: String? = null, + method: String? = null, + clientName: String? = null, + trustedClient: Boolean? = null, + clientAllowedToAllEndpoints: Boolean? = null, + clientIp: String? = null, + requestId: String? = null +): ObjectAssert = hasOneAccessDenial( + requestBlocked = false, + protocol = protocol, + shadowDenied = false, + logPredicate = RbacLog( + protocol = protocol, + path = path, + method = method, + clientIp = clientIp, + statusCode = "200", + clientName = clientName, + trustedClient = trustedClient, + clientAllowedToAllEndpoints = clientAllowedToAllEndpoints, + requestId = requestId + ) +) + fun ObjectAssert.hasOneAccessDenialWithActionLog( protocol: String, path: String? = null, @@ -84,7 +111,8 @@ fun ObjectAssert.hasOneAccessDenialWithActionLog( private fun ObjectAssert.hasOneAccessDenial( requestBlocked: Boolean, protocol: String, - logPredicate: RbacLog + logPredicate: RbacLog, + shadowDenied: Boolean = true ) = satisfies { val admin = it.admin() val blockedRequestsCount = admin.statValue("http.ingress_$protocol.rbac.denied")?.toInt() @@ -95,7 +123,9 @@ private fun ObjectAssert.hasOneAccessDenial( } else { assertThat(blockedRequestsCount).isZero() } - assertThat(loggedRequestsCount).isOne() + if (shadowDenied) { + assertThat(loggedRequestsCount).isOne() + } assertThat(it.logRecorder.getRecordedLogs()).filteredOn(::isRbacAccessLog) .hasSize(1).first() diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsAllowedClientTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsAllowedClientTest.kt new file mode 100644 index 000000000..5cc9d4825 --- /dev/null +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsAllowedClientTest.kt @@ -0,0 +1,282 @@ +package pl.allegro.tech.servicemesh.envoycontrol.permissions + +import okhttp3.Headers +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.RegisterExtension +import pl.allegro.tech.servicemesh.envoycontrol.assertions.hasOneAccessAllowedWithActionLog +import pl.allegro.tech.servicemesh.envoycontrol.assertions.hasOneAccessDenialWithActionBlock +import pl.allegro.tech.servicemesh.envoycontrol.assertions.hasOneAccessDenialWithActionLog +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isForbidden +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isFrom +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk +import pl.allegro.tech.servicemesh.envoycontrol.config.Echo1EnvoyAuthConfig +import pl.allegro.tech.servicemesh.envoycontrol.config.Echo2EnvoyAuthConfig +import pl.allegro.tech.servicemesh.envoycontrol.config.Echo3EnvoyAuthConfig +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyContainer +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension +import pl.allegro.tech.servicemesh.envoycontrol.snapshot.EndpointMatch + +internal class IncomingPermissionsAllowedClientTest { + + companion object { + private const val prefix = "envoy-control.envoy.snapshot" + private val properties = mapOf( + "$prefix.incoming-permissions.enabled" to true, + "$prefix.incoming-permissions.overlapping-paths-fix" to true, + "$prefix.routes.status.create-virtual-cluster" to true, + "$prefix.routes.status.endpoints" to mutableListOf(EndpointMatch().also { it.path = "/status/" }), + "$prefix.routes.status.enabled" to true, + "$prefix.incoming-permissions.clients-allowed-to-all-endpoints" to listOf("echo3") + ) + + // language=yaml + private fun proxySettings(unlistedEndpointsPolicy: String) = """ + node: + metadata: + proxy_settings: + incoming: + unlistedEndpointsPolicy: $unlistedEndpointsPolicy + endpoints: + - path: "/block-unlisted-clients" + clients: ["echo"] + unlistedClientsPolicy: blockAndLog + - path: "/log-unlisted-clients" + methods: [GET] + clients: ["echo"] + unlistedClientsPolicy: log + outgoing: + dependencies: [] + """.trimIndent() + + // language=yaml + private val echo3Yaml = """ + node: + metadata: + proxy_settings: + outgoing: + dependencies: + - service: "echo" + - service: "echo2" + """.trimIndent() + + private val echoConfig = Echo1EnvoyAuthConfig.copy( + configOverride = proxySettings(unlistedEndpointsPolicy = "blockAndLog") + ) + + private val echo2Config = Echo2EnvoyAuthConfig.copy( + configOverride = proxySettings(unlistedEndpointsPolicy = "log") + ) + + private val envoy3Config = Echo3EnvoyAuthConfig.copy(configOverride = echo3Yaml) + + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension(consul, properties) + + @JvmField + @RegisterExtension + val service = EchoServiceExtension() + + @JvmField + @RegisterExtension + val echoEnvoy = EnvoyExtension(envoyControl, localService = service, config = echoConfig) + + @JvmField + @RegisterExtension + val echo2Envoy = EnvoyExtension(envoyControl, localService = service, config = echo2Config) + + @JvmField + @RegisterExtension + val envoy3 = EnvoyExtension(envoyControl, config = envoy3Config) + } + + @Test + fun `echo should allow special client with name from the certificate to access endpoint and log it when policy log`() { + // given + consul.server.operations.registerServiceWithEnvoyOnIngress( + name = "echo", + extension = echoEnvoy, + tags = listOf("mtls:enabled") + ) + envoy3.waitForAvailableEndpoints("echo") + + // when + val echoResponse = envoy3.egressOperations.callService( + service = "echo", + pathAndQuery = "/log-unlisted-clients", + headers = mapOf("x-service-name" to "allowed-client") + ) + + // then + assertThat(echoResponse).isOk().isFrom(service) + assertThat(echoEnvoy.container).hasOneAccessAllowedWithActionLog( + protocol = "https", + path = "/log-unlisted-clients", + method = "GET", + clientName = "echo3", + trustedClient = true, + clientAllowedToAllEndpoints = true, + clientIp = envoy3.container.ipAddress() + ) + } + + @Test + fun `echo should allow special client with name from the certificate to access endpoint and log it when policy blockAndLog`() { + // given + consul.server.operations.registerServiceWithEnvoyOnIngress( + name = "echo", + extension = echoEnvoy, + tags = listOf("mtls:enabled") + ) + envoy3.waitForAvailableEndpoints("echo") + + // when + val echoResponse = envoy3.egressOperations.callService( + service = "echo", + pathAndQuery = "/block-unlisted-clients" + ) + + // then + assertThat(echoResponse).isOk().isFrom(service) + assertThat(echoEnvoy.container).hasOneAccessAllowedWithActionLog( + protocol = "https", + path = "/block-unlisted-clients", + method = "GET", + clientName = "echo3", + trustedClient = true, + clientAllowedToAllEndpoints = true, + clientIp = envoy3.container.ipAddress() + ) + } + + @Test + fun `echo should allow special client with name from header to access endpoint and log it when policy log`() { + // given + consul.server.operations.registerServiceWithEnvoyOnIngress( + name = "echo", + extension = echoEnvoy, + tags = listOf("mtls:enabled") + ) + envoy3.waitForAvailableEndpoints("echo") + + // when + val echoResponse = echoEnvoy.ingressOperations.callLocalService( + endpoint = "/log-unlisted-clients", + headers = Headers.of(mapOf("x-service-name" to "echo3")) + ) + + // then + assertThat(echoResponse).isOk().isFrom(service) + assertThat(echoEnvoy.container).hasOneAccessDenialWithActionLog( + protocol = "http", + path = "/log-unlisted-clients", + method = "GET", + clientName = "echo3", + trustedClient = false, + clientAllowedToAllEndpoints = true, + clientIp = echoEnvoy.container.gatewayIp() + ) + } + + @Test + fun `echo should block special client with name from header to access endpoint and log it when policy blockAndLog`() { + // given + consul.server.operations.registerServiceWithEnvoyOnIngress( + name = "echo", + extension = echoEnvoy, + tags = listOf("mtls:enabled") + ) + envoy3.waitForAvailableEndpoints("echo") + + // when + val echoResponse = echoEnvoy.ingressOperations.callLocalService( + endpoint = "/block-unlisted-clients", + headers = Headers.of(mapOf("x-service-name" to "echo3")) + ) + + // then + assertThat(echoResponse).isForbidden() + assertThat(echoEnvoy.container).hasOneAccessDenialWithActionBlock( + protocol = "http", + path = "/block-unlisted-clients", + method = "GET", + clientName = "echo3", + trustedClient = false, + clientAllowedToAllEndpoints = true, + clientIp = echoEnvoy.container.gatewayIp() + ) + } + + @Test + fun `echo2 should allow special client with name from header over https and log request when unlistedEndpointsPolicy is log`() { + // when + val echo2Response = echo2Envoy.ingressOperations.callLocalServiceInsecure( + endpoint = "/log-unlisted-endpoint", + headers = Headers.of(mapOf("x-service-name" to "echo3")), + useTls = true + ) + + // then + assertThat(echo2Response).isOk().isFrom(service) + assertThat(echo2Envoy.container.ingressTlsRequests()).isOne() + assertThat(echo2Envoy.container).hasOneAccessDenialWithActionLog( + protocol = "https", + path = "/log-unlisted-endpoint", + method = "GET", + clientName = "echo3 (not trusted)", + trustedClient = false, + clientAllowedToAllEndpoints = true, + clientIp = echo2Envoy.container.gatewayIp() + ) + } + + @Test + fun `echo should block special client with name from header over https and log request when unlistedEndpointsPolicy is blockAndLog`() { + // when + val echoResponse = echoEnvoy.ingressOperations.callLocalServiceInsecure( + endpoint = "/block-and-log-unlisted-endpoint", + headers = Headers.of(mapOf("x-service-name" to "echo3")), + useTls = true + ) + + // then + assertThat(echoResponse).isForbidden() + assertThat(echoEnvoy.container.ingressTlsRequests()).isOne() + assertThat(echoEnvoy.container).hasOneAccessDenialWithActionBlock( + protocol = "https", + path = "/block-and-log-unlisted-endpoint", + method = "GET", + clientName = "echo3 (not trusted)", + trustedClient = false, + clientAllowedToAllEndpoints = true, + clientIp = echoEnvoy.container.gatewayIp() + ) + } + + @BeforeEach + fun startRecordingRBACLogs() { + echoEnvoy.recordRBACLogs() + echo2Envoy.recordRBACLogs() + } + + @AfterEach + fun cleanupTest() { + echoEnvoy.container.admin().resetCounters() + echoEnvoy.container.logRecorder.stopRecording() + echo2Envoy.container.admin().resetCounters() + echo2Envoy.container.logRecorder.stopRecording() + } + + private fun EnvoyContainer.ingressTlsRequests() = + this.admin().statValue("http.ingress_https.downstream_rq_completed")?.toInt() +} diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsLoggingModeTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsLoggingModeTest.kt index 46ad48e2c..66fb196a5 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsLoggingModeTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/IncomingPermissionsLoggingModeTest.kt @@ -711,34 +711,6 @@ internal class IncomingPermissionsLoggingModeTest : EnvoyControlTestConfiguratio ) } - @Test - fun `echo2 should allow special client with client identity header over https and log request`() { - // given - val insecureClient = ClientsFactory.createInsecureClient() - - // when - val echo2Response = callEnvoyIngress( - envoy = echo2Envoy, - path = "/log-special-clients", - headers = mapOf("x-service-name" to "allowed-client"), - useSsl = true, - client = insecureClient - ) - - // then - assertThat(echo2Response).isOk().isFrom(echo2LocalService) - assertThat(echo2Envoy.ingressSslRequests).isOne() - assertThat(echo2Envoy).hasOneAccessDenialWithActionLog( - protocol = "https", - path = "/log-special-clients", - method = "GET", - clientName = "allowed-client (not trusted)", - trustedClient = false, - clientAllowedToAllEndpoints = true, - clientIp = echo2Envoy.gatewayIp() - ) - } - @BeforeEach fun startRecordingRBACLogs() { echoEnvoy.recordRBACLogs() From 36f15ad9b4be8bf9796d7d6971a36e7a9f3434e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Dziedziak?= Date: Mon, 1 Feb 2021 09:43:08 +0100 Subject: [PATCH 10/11] Local replay mapper basic implementation and tests (#227) * First iteration, added basic implementation and tests * Fixed lint * Added docs and configuration * Added possibility to define nested json format * Allow to define custom content type * Added content type to configuration * Code review changes --- docs/configuration.md | 90 +-- docs/features/local_reply_mapper.md | 135 +++++ docs/index.md | 1 + .../servicemesh/envoycontrol/ControlPlane.kt | 6 +- .../envoycontrol/groups/MetadataNodeGroup.kt | 6 +- .../envoycontrol/groups/NodeMetadata.kt | 23 +- .../snapshot/SnapshotProperties.kt | 29 + .../listeners/EnvoyListenersFactory.kt | 11 +- .../config/LocalReplyConfigFactory.kt | 208 +++++++ .../filters/AccessLogFilterFactory.kt | 29 - .../utils/StatusCodeFilterParser.kt | 34 ++ .../envoycontrol/EnvoySnapshotFactoryTest.kt | 3 +- .../groups/MetadataNodeGroupTest.kt | 39 +- .../envoycontrol/groups/NodeMetadataTest.kt | 13 +- .../config/LocalReplyConfigFactoryTest.kt | 524 ++++++++++++++++++ .../envoycontrol/LocalReplyMappingTest.kt | 115 ++++ 16 files changed, 1139 insertions(+), 127 deletions(-) create mode 100644 docs/features/local_reply_mapper.md create mode 100644 envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/config/LocalReplyConfigFactory.kt delete mode 100644 envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/AccessLogFilterFactory.kt create mode 100644 envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/utils/StatusCodeFilterParser.kt create mode 100644 envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/config/LocalReplyConfigFactoryTest.kt create mode 100644 envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/LocalReplyMappingTest.kt diff --git a/docs/configuration.md b/docs/configuration.md index a95a8d319..f4f8704dd 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -21,44 +21,58 @@ Property **envoy-control.server.snapshot-cleanup.collection-interval-millis** | How often the collection background action should run | 10s ## Snapshot properties -Property | Description | Default value -------------------------------------------------------------------------------------------------------------| ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- -**envoy-control.envoy.snapshot.dynamicListeners.enabled** | Enable or disable creating listeners using dynamic configuration | true -**envoy-control.envoy.snapshot.dynamic-listeners.http-filters.access-log.time-format** | Time format for access logs | "%START_TIME(%FT%T.%3fZ)%" -**envoy-control.envoy.snapshot.dynamic-listeners.http-filters.access-log.message-format** | Message format for access logs | "%PROTOCOL% %REQ(:METHOD)% %REQ(:authority)% %REQ(:PATH)% %DOWNSTREAM_REMOTE_ADDRESS% -> %UPSTREAM_HOST%" -**envoy-control.envoy.snapshot.dynamic-listeners.http-filters.access-log.level** | Logging level for access logs | "TRACE" -**envoy-control.envoy.snapshot.dynamic-listeners.http-filters.access-log.logger** | Logger name for access logs | "envoy.AccessLog" -**envoy-control.envoy.snapshot.dynamic-listeners.http-filters.ingress-xff-num-trusted-hops** | Number of trusted hops for ingress filter (refer to [envoy docs](https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/headers.html?highlight=xff_num_trusted_hops#x-forwarded-for)) | 1 -**envoy-control.envoy.snapshot.eds-connection-timeout** | Connection timeout for EDS clusters | 2s -**envoy-control.envoy.snapshot.egress.common-http.idle-timeout** | Set idle timeout for all HTTP connections (HTTP/1 and HTTP/2) | 120s -**envoy-control.envoy.snapshot.egress.common-http.request-timeout** | Set request timeout for all routes (HTTP/1 and HTTP/2) | 120s -**envoy-control.envoy.snapshot.egress.common-http.circuit-breakers.high-threshold.max-connections** | The maximum number of connections that Envoy will make to the upstream cluster for high priority threshold. | 1024 -**envoy-control.envoy.snapshot.egress.common-http.circuit-breakers.high-threshold.max-pending-requests** | The maximum number of pending requests that Envoy will allow to the upstream cluster for high priority threshold. | 1024 -**envoy-control.envoy.snapshot.egress.common-http.circuit-breakers.high-threshold.max-requests** | The maximum number of parallel requests that Envoy will make to the upstream cluster for high priority threshold. | 1024 -**envoy-control.envoy.snapshot.egress.common-http.circuit-breakers.high-threshold.max-retries** | The maximum number of parallel retries that Envoy will allow to the upstream cluster for high priority threshold. | 3 -**envoy-control.envoy.snapshot.egress.common-http.circuit-breakers.default-threshold.max-connections** | The maximum number of connections that Envoy will make to the upstream cluster for default priority threshold. | 1024 -**envoy-control.envoy.snapshot.egress.common-http.circuit-breakers.default-threshold.max-pending-requests** | The maximum number of pending requests that Envoy will allow to the upstream cluster for default priority threshold. | 1024 -**envoy-control.envoy.snapshot.egress.common-http.circuit-breakers.default-threshold.max-requests** | The maximum number of parallel requests that Envoy will make to the upstream cluster for default priority threshold. | 1024 -**envoy-control.envoy.snapshot.egress.common-http.circuit-breakers.default-threshold.max-retries** | The maximum number of parallel retries that Envoy will allow to the upstream cluster for default priority threshold. | 3 -**envoy-control.envoy.snapshot.egress.never-remove-clusters** | Don't remove cluster, when corresponding service disappears from services source. Only remove all instances. | true -**envoy-control.envoy.snapshot.egress.cluster-not-found-status-code** | Status code when cluster is not found | 503 -**envoy-control.envoy.snapshot.egress.http2.enabled** | Enable http2 for clusters that use envoy | true -**envoy-control.envoy.snapshot.egress.http2.tag-name** | Tag to be used to identify if instance uses envoy | envoy -**envoy-control.envoy.snapshot.egress.handle-internal-redirect** | Handle redirects by Envoy | false -**envoy-control.envoy.snapshot.egress.host-header-rewriting.enabled** | Enable rewriting Host header with value from specified header | false -**envoy-control.envoy.snapshot.egress.host-header-rewriting.custom-host-header** | Header name which value will override Host header | "x-envoy-original-host" -**envoy-control.envoy.snapshot.ingress.headers-to-remove** | List of headers to sanitize | empty list -**envoy-control.envoy.snapshot.local-service.idle-timeout** | Idle timeout between client to envoy | 60s -**envoy-control.envoy.snapshot.local-service.response-timeout** | Response timeout for localService | 15s -**envoy-control.envoy.snapshot.local-service.connection-idle-timeout** | Connection idle timeout for localService | 120s -**envoy-control.envoy.snapshot.routes.status.enabled** | Enable status route | false -**envoy-control.envoy.snapshot.routes.status.endpoints** | List of endpoints with path or prefix of status routes | /status -**envoy-control.envoy.snapshot.routes.status.create-virtual-cluster** | Create virtual cluster for status route | false -**envoy-control.envoy.snapshot.state-sample-duration** | Duration of state sampling (this is used to prevent surges in consul events overloading control plane) | 1s -**envoy-control.envoy.snapshot.xds-cluster-name** | Name of cluster for xDS operations | envoy-control-xds -**envoy-control.envoy.snapshot.enabled-communication-modes.ads** | Enable or disable support for ADS communication mode | true -**envoy-control.envoy.snapshot.enabled-communication-modes.xds** | Enable or disable support for XDS communication mode | true -**envoy-control.envoy.snapshot.should-send-missing-endpoints** | Enable sending missing Endpoints - when Envoy requests for not existing cluster in snapshot control-plane will respond with empty Endpoint definition | false +Property | Description | Default value +-------------------------------------------------------------------------------------------------------------| --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- +**envoy-control.envoy.snapshot.dynamic-listeners.enabled** | Enable or disable creating listeners using dynamic configuration | true +**envoy-control.envoy.snapshot.dynamic-listeners.http-filters.access-log.time-format** | Time format for access logs | "%START_TIME(%FT%T.%3fZ)%" +**envoy-control.envoy.snapshot.dynamic-listeners.http-filters.access-log.message-format** | Message format for access logs | "%PROTOCOL% %REQ(:METHOD)% %REQ(:authority)% %REQ(:PATH)% %DOWNSTREAM_REMOTE_ADDRESS% -> %UPSTREAM_HOST%" +**envoy-control.envoy.snapshot.dynamic-listeners.http-filters.access-log.level** | Logging level for access logs | "TRACE" +**envoy-control.envoy.snapshot.dynamic-listeners.http-filters.access-log.logger** | Logger name for access logs | "envoy.AccessLog" +**envoy-control.envoy.snapshot.dynamic-listeners.http-filters.ingress-xff-num-trusted-hops** | Number of trusted hops for ingress filter (refer to [envoy docs](https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/headers.html?highlight=xff_num_trusted_hops#x-forwarded-for)) | 1 +**envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.enabled** | Enable or disable creating local reply mapper configuration (refer to [envoy docs](https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/local_reply)) | false +**envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.response-format.text-format** | Text message format with placeholders (refer to [envoy docs](https://www.envoyproxy.io/docs/envoy/latest/configuration/observability/access_log/usage#command-operators)) | "" +**envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.response-format.json-format** | JSON message format with placeholders for matched response (refer to [envoy docs](https://www.envoyproxy.io/docs/envoy/latest/configuration/observability/access_log/usage#command-operators)). | "" +**envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.response-format.content-type** | Response content-type header value | "" +**envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.matchers.status-code-matcher** | Matcher which handle specific status codes formatted as string e.g.: EQ:400 - equal to status code 400 | "" +**envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.matchers.header-matcher.name** | Header name to match | "" +**envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.matchers.header-matcher.exact-match** | Header value to match for specified header (only one of: exactMatch, regexMatch can be specified. If none is specified, header name presence matcher will be used) | "" +**envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.matchers.header-matcher.regex-match** | Header value regex to match for specified header (only one of: exactMatch, regexMatch can be specified. If none is specified, header name presence matcher will be used) | "" +**envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.matchers.response-flag-matcher** | Response flags to match (refer to [envoy docs](https://www.envoyproxy.io/docs/envoy/latest/configuration/observability/access_log/usage#command-operators)) | empty list +**envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.matchers.status-code-to-return** | Status code to return for matched response | 0 (disabled) +**envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.matchers.body-to-return** | Response message to return for matched response | "" +**envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.matchers.response-format.text-format** | Text message format with placeholders for matched response | "" +**envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.matchers.response-format.json-format** | JSON message format with placeholders for matched response | empty map +**envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.matchers.response-format.content-type** | Response content-type header value | "" +**envoy-control.envoy.snapshot.eds-connection-timeout** | Connection timeout for EDS clusters | 2s +**envoy-control.envoy.snapshot.egress.common-http.idle-timeout** | Set idle timeout for all HTTP connections (HTTP/1 and HTTP/2) | 120s +**envoy-control.envoy.snapshot.egress.common-http.request-timeout** | Set request timeout for all routes (HTTP/1 and HTTP/2) | 120s +**envoy-control.envoy.snapshot.egress.common-http.circuit-breakers.high-threshold.max-connections** | The maximum number of connections that Envoy will make to the upstream cluster for high priority threshold. | 1024 +**envoy-control.envoy.snapshot.egress.common-http.circuit-breakers.high-threshold.max-pending-requests** | The maximum number of pending requests that Envoy will allow to the upstream cluster for high priority threshold. | 1024 +**envoy-control.envoy.snapshot.egress.common-http.circuit-breakers.high-threshold.max-requests** | The maximum number of parallel requests that Envoy will make to the upstream cluster for high priority threshold. | 1024 +**envoy-control.envoy.snapshot.egress.common-http.circuit-breakers.high-threshold.max-retries** | The maximum number of parallel retries that Envoy will allow to the upstream cluster for high priority threshold. | 3 +**envoy-control.envoy.snapshot.egress.common-http.circuit-breakers.default-threshold.max-connections** | The maximum number of connections that Envoy will make to the upstream cluster for default priority threshold. | 1024 +**envoy-control.envoy.snapshot.egress.common-http.circuit-breakers.default-threshold.max-pending-requests** | The maximum number of pending requests that Envoy will allow to the upstream cluster for default priority threshold. | 1024 +**envoy-control.envoy.snapshot.egress.common-http.circuit-breakers.default-threshold.max-requests** | The maximum number of parallel requests that Envoy will make to the upstream cluster for default priority threshold. | 1024 +**envoy-control.envoy.snapshot.egress.common-http.circuit-breakers.default-threshold.max-retries** | The maximum number of parallel retries that Envoy will allow to the upstream cluster for default priority threshold. | 3 +**envoy-control.envoy.snapshot.egress.never-remove-clusters** | Don't remove cluster, when corresponding service disappears from services source. Only remove all instances. | true +**envoy-control.envoy.snapshot.egress.cluster-not-found-status-code** | Status code when cluster is not found | 503 +**envoy-control.envoy.snapshot.egress.http2.enabled** | Enable http2 for clusters that use envoy | true +**envoy-control.envoy.snapshot.egress.http2.tag-name** | Tag to be used to identify if instance uses envoy | envoy +**envoy-control.envoy.snapshot.egress.handle-internal-redirect** | Handle redirects by Envoy | false +**envoy-control.envoy.snapshot.egress.host-header-rewriting.enabled** | Enable rewriting Host header with value from specified header | false +**envoy-control.envoy.snapshot.egress.host-header-rewriting.custom-host-header** | Header name which value will override Host header | "x-envoy-original-host" +**envoy-control.envoy.snapshot.ingress.headers-to-remove** | List of headers to sanitize | empty list +**envoy-control.envoy.snapshot.local-service.idle-timeout** | Idle timeout between client to envoy | 60s +**envoy-control.envoy.snapshot.local-service.response-timeout** | Response timeout for localService | 15s +**envoy-control.envoy.snapshot.local-service.connection-idle-timeout** | Connection idle timeout for localService | 120s +**envoy-control.envoy.snapshot.routes.status.enabled** | Enable status route | false +**envoy-control.envoy.snapshot.routes.status.endpoints** | List of endpoints with path or prefix of status routes | /status +**envoy-control.envoy.snapshot.routes.status.create-virtual-cluster** | Create virtual cluster for status route | false +**envoy-control.envoy.snapshot.state-sample-duration** | Duration of state sampling (this is used to prevent surges in consul events overloading control plane) | 1s +**envoy-control.envoy.snapshot.xds-cluster-name** | Name of cluster for xDS operations | envoy-control-xds +**envoy-control.envoy.snapshot.enabled-communication-modes.ads** | Enable or disable support for ADS communication mode | true +**envoy-control.envoy.snapshot.enabled-communication-modes.xds** | Enable or disable support for XDS communication mode | true +**envoy-control.envoy.snapshot.should-send-missing-endpoints** | Enable sending missing Endpoints - when Envoy requests for not existing cluster in snapshot control-plane will respond with empty Endpoint definition | false ## Permissions Property | Description | Default value diff --git a/docs/features/local_reply_mapper.md b/docs/features/local_reply_mapper.md new file mode 100644 index 000000000..102660ce8 --- /dev/null +++ b/docs/features/local_reply_mapper.md @@ -0,0 +1,135 @@ +# Local reply modification configuration + +Envoy Control allows defining custom format for responses returned by Envoy. Thanks to [Envoy functionality](https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/local_reply) +it is possible to configure the custom format, status code for specific responses. + +## Define default response format for responses returned by Envoy + +It is possible to define a custom format for all responses returned by Envoy. Envoy can return response either +in a text format or JSON format. It is possible to define only one of: `textFormat` and `jsonFormat`. +If the format isn't specified, then default from Envoy is returned. + +### Configure text format response + +Property `envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.response-format.text-format` allows configuring text response format. +Text format supports [command operators](https://www.envoyproxy.io/docs/envoy/latest/configuration/observability/access_log/usage#config-access-log-command-operators) that allows +operating on request/response data. + +An example configuration: + +```yaml +envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.response-format.text-format: "my custom response with flags: %RESPONSE_FLAGS%" +``` + +Response: + +```text +"my custom response with flags: UF" +``` + +### Configure JSON format response + +Property `envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.response-format.json-format` allows configuring JSON response format. +It accepts a JSON formatted string with constant string and [command operators](https://www.envoyproxy.io/docs/envoy/latest/configuration/observability/access_log/usage#config-access-log-command-operators). + +An example configuration: + +```yaml +envoy-control: + envoy: + snapshot: + dynamic-listeners: + local-reply-mapper: + response-format: + json-format: """{ + "myKey":"My custom body", + "responseFlags":"%RESPONSE_FLAGS%", + "host":"%REQ(:authority)%" + }""" +``` + +Response: + +```json +{ + "myKey" : "My custom body", + "responseFlags" : "UF", + "host" : "example-service" +} +``` + +## Match specific response + +It is possible to define custom response or override status code only for specific responses. In that case you can use matchers +which allows defining custom response status and response body for matched responses. Currently, 3 types of matchers are supported: +- status code matcher +- response flag matcher +- header matcher + +You can choose only one of: `statusCodeMatcher`, `headerMatcher`, `responseFlagMatcher`. Also, configuration supports response body format override +for matched responses. + +### Status code matcher + +Allows filtering only specific status codes: An expected format is: `{operator}`:`{status code}`. + +Allowed operators are: + +* `le` - lower equal +* `eq` - equal +* `ge` - greater equal + +Example: + +```yaml +statusCodeMatcher: "EQ:400" +``` + +By default, it is an empty string which means that matcher is disabled. + +### Header matcher + +Allows filtering responses based on header presence or value. Only one of: `exactMatch`, `regexMatch` can be used. If none is used +then presence matcher will match responses with specified header name. + +Example: + +```yaml + headerMatcher: + name: "host" + exactMatch: "service1" +``` + +By default, all fields are equals to empty string which means that matcher is disabled. + +### Response flags matcher + +Allows filtering responses based on response flags (refer to [Envoy docs](https://www.envoyproxy.io/docs/envoy/latest/configuration/observability/access_log/usage#config-access-log-format-response-flags)). + +Example: + +```yaml + responseFlagMatcher: + - "UF" + - "NR" +``` + +By default, it is set an empty list which means there is no filtering by response flags. + +### Status code override + +When response is matched and property `statusCodeToReturn` for this matcher is defined then Envoy will change response status code +to value of the property `statusCodeToReturn`. By default, it is set to 0 which means that status code won't be overridden. + +### Custom body + +When response is matched and property `bodyToReturn` for this matcher is defined then Envoy will set body to value of the property `bodyToReturn`. +If you defined custom format then the value can be accessed by using placeholder `%LOCAL_REPLY_BODY%` (refer to [Envoy docs](https://www.envoyproxy.io/docs/envoy/latest/configuration/observability/access_log/usage#config-access-log-format-filter-state)). + +By default, it is an empty string which means that body won't be overridden. + +### Different response format for different matchers + +It is possible to configure different response formats for different matchers. If matcher configuration has `responseFormat` configuration then +it will be used instead of response format defined at `localReplyMapper` level. When there is no configuration, default Envoy's format will be returned. + diff --git a/docs/index.md b/docs/index.md index 3dbf4724a..c9e0c5412 100644 --- a/docs/index.md +++ b/docs/index.md @@ -13,6 +13,7 @@ Data Plane that is platform agnostic. * [Weighted load balancing and canary support](features/load_balancing.md) * [Service tags routing support](features/service_tags.md) * [Access log filter](features/access_log_filter.md) +* [Local reply modification](features/local_reply_mapper.md) ## Why another Control Plane? Our use case for Service Mesh is running 800 microservices on [Mesos](https://mesos.apache.org/) / [Marathon](https://mesosphere.github.io/marathon/) stack. diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ControlPlane.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ControlPlane.kt index 9435e9119..4d8ffe67c 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ControlPlane.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ControlPlane.kt @@ -30,7 +30,6 @@ import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SnapshotsVersions import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.clusters.EnvoyClustersFactory import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.endpoints.EnvoyEndpointsFactory import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.EnvoyListenersFactory -import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters.AccessLogFilterFactory import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters.EnvoyHttpFilters import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.routes.EnvoyEgressRoutesFactory import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.routes.EnvoyIngressRoutesFactory @@ -95,11 +94,8 @@ class ControlPlane private constructor( var metrics: EnvoyControlMetrics = DefaultEnvoyControlMetrics(meterRegistry = meterRegistry) var envoyHttpFilters: EnvoyHttpFilters = EnvoyHttpFilters.emptyFilters - val accessLogFilterFactory = AccessLogFilterFactory() - var nodeGroup: NodeGroup = MetadataNodeGroup( - properties = properties.envoy.snapshot, - accessLogFilterFactory = accessLogFilterFactory + properties = properties.envoy.snapshot ) fun build(changes: Flux): ControlPlane { diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroup.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroup.kt index ea5216f6d..4ce3fb958 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroup.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroup.kt @@ -5,13 +5,11 @@ import com.google.protobuf.Value import io.envoyproxy.controlplane.cache.NodeGroup import pl.allegro.tech.servicemesh.envoycontrol.logger import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SnapshotProperties -import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters.AccessLogFilterFactory import io.envoyproxy.envoy.api.v2.core.Node as NodeV2 import io.envoyproxy.envoy.config.core.v3.Node as NodeV3 class MetadataNodeGroup( - val properties: SnapshotProperties, - val accessLogFilterFactory: AccessLogFilterFactory + val properties: SnapshotProperties ) : NodeGroup { private val logger by logger() @@ -84,7 +82,7 @@ class MetadataNodeGroup( val egressHostValue = metadata.fieldsMap["egress_host"] val egressPortValue = metadata.fieldsMap["egress_port"] val accessLogFilterSettings = AccessLogFilterSettings( - metadata.fieldsMap["access_log_filter"], accessLogFilterFactory + metadata.fieldsMap["access_log_filter"] ) val listenersHostPort = metadataToListenersHostPort( diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadata.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadata.kt index 4fbac8e2f..7c71bf413 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadata.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadata.kt @@ -5,10 +5,10 @@ import com.google.protobuf.Struct import com.google.protobuf.Value import com.google.protobuf.util.Durations import io.envoyproxy.controlplane.server.exception.RequestException -import io.envoyproxy.envoy.config.accesslog.v3.ComparisonFilter import io.grpc.Status import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SnapshotProperties -import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters.AccessLogFilterFactory +import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.util.StatusCodeFilterParser +import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.util.StatusCodeFilterSettings import java.net.URL import java.text.ParseException @@ -25,17 +25,9 @@ class NodeMetadata(metadata: Struct, properties: SnapshotProperties) { val proxySettings: ProxySettings = ProxySettings(metadata.fieldsMap["proxy_settings"], properties) } -data class AccessLogFilterSettings( - val statusCodeFilterSettings: StatusCodeFilterSettings? -) { - constructor(proto: Value?, accessLogFilterFactory: AccessLogFilterFactory) : this( - statusCodeFilterSettings = proto?.field("status_code_filter").toStatusCodeFilter(accessLogFilterFactory) - ) - - data class StatusCodeFilterSettings( - val comparisonOperator: ComparisonFilter.Op, - val comparisonCode: Int - ) +data class AccessLogFilterSettings(val proto: Value?) { + val statusCodeFilterSettings: StatusCodeFilterSettings? = proto?.field("status_code_filter") + .toStatusCodeFilter() } data class ProxySettings( @@ -67,10 +59,9 @@ private fun getCommunicationMode(proto: Value?): CommunicationMode { } } -fun Value?.toStatusCodeFilter(accessLogFilterFactory: AccessLogFilterFactory): - AccessLogFilterSettings.StatusCodeFilterSettings? { +fun Value?.toStatusCodeFilter(): StatusCodeFilterSettings? { return this?.stringValue?.let { - accessLogFilterFactory.parseStatusCodeFilter(it.toUpperCase()) + StatusCodeFilterParser.parseStatusCodeFilter(it.toUpperCase()) } } diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt index 78a30fb79..99ef76fe1 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt @@ -36,6 +36,7 @@ class MetricsProperties { class ListenersFactoryProperties { var enabled = true var httpFilters = HttpFiltersProperties() + var localReplyMapper = LocalReplyMapperProperties() } class HttpFiltersProperties { @@ -86,6 +87,7 @@ class TlsAuthenticationProperties { var servicesAllowedToUseWildcard: MutableSet = mutableSetOf() var tlsContextMetadataMatchKey = "acceptMTLS" var protocol = TlsProtocolProperties() + /** if true, a request without a cert will be rejected during handshake and will not reach RBAC filter */ var requireClientCertificate: Boolean = false var validationContextSecretName: String = "validation_context" @@ -266,3 +268,30 @@ class HostHeaderRewritingProperties { var enabled = false var customHostHeader = "x-envoy-original-host" } + +class LocalReplyMapperProperties { + var enabled = false + var responseFormat = ResponseFormat() + var matchers = emptyList() +} + +class MatcherAndMapper { + var statusCodeMatcher = "" + var headerMatcher = HeaderMatcher() + var responseFlagMatcher = emptyList() + var responseFormat = ResponseFormat() + var statusCodeToReturn = 0 + var bodyToReturn = "" +} + +class HeaderMatcher { + var name = "" + var exactMatch: String = "" + var regexMatch: String = "" +} + +class ResponseFormat { + var textFormat = "" + var jsonFormat = "" + var contentType = "" +} diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/EnvoyListenersFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/EnvoyListenersFactory.kt index c1933dd3f..e1c1fa8a1 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/EnvoyListenersFactory.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/EnvoyListenersFactory.kt @@ -42,7 +42,9 @@ import pl.allegro.tech.servicemesh.envoycontrol.groups.ListenersConfig import pl.allegro.tech.servicemesh.envoycontrol.groups.ResourceVersion import pl.allegro.tech.servicemesh.envoycontrol.snapshot.GlobalSnapshot import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SnapshotProperties +import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.config.LocalReplyConfigFactory import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters.EnvoyHttpFilters +import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.util.StatusCodeFilterSettings import com.google.protobuf.Any as ProtobufAny typealias HttpFilterFactory = (node: Group, snapshot: GlobalSnapshot) -> HttpFilter? @@ -62,6 +64,9 @@ class EnvoyListenersFactory( private val accessLogLogger = stringValue(listenersFactoryProperties.httpFilters.accessLog.logger) private val egressRdsInitialFetchTimeout: Duration = durationInSeconds(20) private val ingressRdsInitialFetchTimeout: Duration = durationInSeconds(30) + private val localReplyConfig = LocalReplyConfigFactory( + snapshotProperties.dynamicListeners.localReplyMapper + ).configuration private val tlsProperties = snapshotProperties.incomingPermissions.tlsAuthentication private val requireClientCertificate = BoolValue.of(tlsProperties.requireClientCertificate) @@ -251,6 +256,10 @@ class EnvoyListenersFactory( ) } + if (listenersFactoryProperties.localReplyMapper.enabled) { + connectionManagerBuilder.localReplyConfig = localReplyConfig + } + return Filter.newBuilder() .setName("envoy.filters.network.http_connection_manager") .setTypedConfig(ProtobufAny.pack( @@ -351,7 +360,7 @@ class EnvoyListenersFactory( } } - fun AccessLog.Builder.buildFromSettings(settings: AccessLogFilterSettings.StatusCodeFilterSettings) { + fun AccessLog.Builder.buildFromSettings(settings: StatusCodeFilterSettings) { this.setFilter( AccessLogFilter.newBuilder().setStatusCodeFilter( StatusCodeFilter.newBuilder() diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/config/LocalReplyConfigFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/config/LocalReplyConfigFactory.kt new file mode 100644 index 000000000..22dc929ed --- /dev/null +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/config/LocalReplyConfigFactory.kt @@ -0,0 +1,208 @@ +package pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.config + +import com.google.protobuf.Struct +import com.google.protobuf.UInt32Value +import com.google.protobuf.util.JsonFormat +import io.envoyproxy.envoy.config.accesslog.v3.AccessLogFilter +import io.envoyproxy.envoy.config.accesslog.v3.ComparisonFilter +import io.envoyproxy.envoy.config.accesslog.v3.HeaderFilter +import io.envoyproxy.envoy.config.accesslog.v3.ResponseFlagFilter +import io.envoyproxy.envoy.config.accesslog.v3.StatusCodeFilter +import io.envoyproxy.envoy.config.core.v3.DataSource +import io.envoyproxy.envoy.config.core.v3.RuntimeUInt32 +import io.envoyproxy.envoy.config.core.v3.SubstitutionFormatString +import io.envoyproxy.envoy.config.route.v3.HeaderMatcher +import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.LocalReplyConfig +import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.ResponseMapper +import io.envoyproxy.envoy.type.matcher.v3.RegexMatcher +import pl.allegro.tech.servicemesh.envoycontrol.snapshot.HeaderMatcher as HeaderMatcherProperties +import pl.allegro.tech.servicemesh.envoycontrol.snapshot.LocalReplyMapperProperties +import pl.allegro.tech.servicemesh.envoycontrol.snapshot.MatcherAndMapper +import pl.allegro.tech.servicemesh.envoycontrol.snapshot.ResponseFormat +import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.util.StatusCodeFilterParser + +class LocalReplyConfigFactory( + localReplyMapperProperties: LocalReplyMapperProperties, + private val jsonParser: JsonFormat.Parser = JsonFormat.parser() +) { + var configuration: LocalReplyConfig = LocalReplyConfig.getDefaultInstance() + + init { + + if (localReplyMapperProperties.enabled) { + validateResponseFormatProperties(localReplyMapperProperties.responseFormat) + localReplyMapperProperties.matchers.forEach { + validateMatchersDefinition(it) + validateResponseFormatProperties(it.responseFormat) + } + configuration = createLocalReplyMapper(localReplyMapperProperties) + } + } + + private fun createLocalReplyMapper(localReplyMapperProperties: LocalReplyMapperProperties): LocalReplyConfig { + val localReplyBuilder = LocalReplyConfig.newBuilder() + if (localReplyMapperProperties.matchers.isNotEmpty()) { + localReplyMapperProperties.matchers.forEach { + localReplyBuilder.addMappers(createFilteredResponseMapper(it)) + } + } + createResponseFormat(localReplyMapperProperties.responseFormat)?.let { + localReplyBuilder.setBodyFormat(it) + } + return localReplyBuilder.build() + } + + private fun createFilteredResponseMapper(matcherAndMapper: MatcherAndMapper): ResponseMapper.Builder { + val responseMapperBuilder = ResponseMapper.newBuilder() + createResponseFormat(matcherAndMapper.responseFormat)?.let { + responseMapperBuilder.setBodyFormatOverride(it) + } + setResponseBody(matcherAndMapper, responseMapperBuilder) + setStatusCode(matcherAndMapper, responseMapperBuilder) + + return when { + matcherAndMapper.headerMatcher.name.isNotEmpty() -> { + responseMapperBuilder.setFilter( + AccessLogFilter.newBuilder().setHeaderFilter(createHeaderFilter(matcherAndMapper.headerMatcher)) + ) + responseMapperBuilder + } + matcherAndMapper.responseFlagMatcher.isNotEmpty() -> { + responseMapperBuilder.setFilter( + AccessLogFilter.newBuilder().setResponseFlagFilter( + createResponseFlagFilter(matcherAndMapper.responseFlagMatcher) + ) + ) + responseMapperBuilder + } + matcherAndMapper.statusCodeMatcher.isNotEmpty() -> { + responseMapperBuilder.setFilter( + AccessLogFilter.newBuilder().setStatusCodeFilter( + createStatusCodeFilter(matcherAndMapper.statusCodeMatcher) + ) + ) + responseMapperBuilder + } + else -> { + responseMapperBuilder + } + } + } + + private fun setStatusCode( + matcherAndMapper: MatcherAndMapper, + responseMapperBuilder: ResponseMapper.Builder + ) { + if (matcherAndMapper.statusCodeToReturn != 0) { + responseMapperBuilder.setStatusCode(UInt32Value.newBuilder().setValue(matcherAndMapper.statusCodeToReturn)) + } + } + + private fun setResponseBody( + matcherAndMapper: MatcherAndMapper, + responseMapperBuilder: ResponseMapper.Builder + ) { + if (matcherAndMapper.bodyToReturn.isNotEmpty()) { + responseMapperBuilder.setBody(DataSource.newBuilder().setInlineString(matcherAndMapper.bodyToReturn)) + } + } + + private fun createStatusCodeFilter(statusCode: String): StatusCodeFilter.Builder { + val parsedStatusCodeMatcher = StatusCodeFilterParser.parseStatusCodeFilter(statusCode) + return StatusCodeFilter.newBuilder().setComparison( + ComparisonFilter.newBuilder() + .setOp(parsedStatusCodeMatcher.comparisonOperator) + .setValue( + RuntimeUInt32.newBuilder() + .setDefaultValue(parsedStatusCodeMatcher.comparisonCode) + .setRuntimeKey("local_reply_mapper_http_code") + .build() + ) + ) + } + + private fun createResponseFlagFilter(responseFlags: List): ResponseFlagFilter.Builder { + return ResponseFlagFilter.newBuilder().addAllFlags(responseFlags) + } + + private fun createHeaderFilter(headerMatcher: HeaderMatcherProperties): HeaderFilter.Builder { + val headerFilterBuilder = HeaderFilter.newBuilder() + val headerMatcherBuilder = HeaderMatcher.newBuilder() + .setName(headerMatcher.name) + return when { + headerMatcher.regexMatch.isNotEmpty() -> { + headerFilterBuilder.setHeader( + headerMatcherBuilder.setSafeRegexMatch( + RegexMatcher.newBuilder() + .setGoogleRe2(RegexMatcher.GoogleRE2.getDefaultInstance()) + .setRegex(headerMatcher.regexMatch) + ) + ) + } + headerMatcher.exactMatch.isNotEmpty() -> { + headerFilterBuilder.setHeader(headerMatcherBuilder.setExactMatch(headerMatcher.exactMatch)) + } + else -> { + headerFilterBuilder.setHeader(headerMatcherBuilder.setPresentMatch(true)) + } + } + } + + private fun createResponseFormat(responseFormat: ResponseFormat): SubstitutionFormatString? { + val responseFormatBuilder = SubstitutionFormatString.newBuilder() + if (responseFormat.contentType.isNotEmpty()) { + responseFormatBuilder.contentType = responseFormat.contentType + } + + return when { + responseFormat.textFormat.isNotEmpty() -> { + responseFormatBuilder.setTextFormat(responseFormat.textFormat).build() + } + responseFormat.jsonFormat.isNotEmpty() -> { + val responseBody = Struct.newBuilder() + jsonParser.merge(responseFormat.jsonFormat, responseBody) + responseFormatBuilder.setJsonFormat(responseBody.build()).build() + } + else -> { + null + } + } + } + + private fun validateMatchersDefinition(matcherAndMapper: MatcherAndMapper) { + var definitions = 0 + + if (matcherAndMapper.statusCodeMatcher.isNotEmpty()) { + definitions += 1 + } + + if (matcherAndMapper.responseFlagMatcher.isNotEmpty()) { + definitions += 1 + } + if (matcherAndMapper.headerMatcher.name.isNotBlank()) { + definitions += 1 + validateHeaderMatcher(matcherAndMapper.headerMatcher) + } + + if (definitions != 1) { + throw IllegalArgumentException( + "One and only one of: headerMatcher, responseFlagMatcher, statusCodeMatcher has to be defined.") + } + } + + private fun validateHeaderMatcher(headerMatcher: HeaderMatcherProperties) { + if (headerMatcher.exactMatch.isNotEmpty() && headerMatcher.regexMatch.isNotEmpty()) { + throw IllegalArgumentException( + "Only one of: exactMatch, regexMatch can be defined." + ) + } + } + + private fun validateResponseFormatProperties(responseFormat: ResponseFormat) { + if (responseFormat.jsonFormat.isNotEmpty() && responseFormat.textFormat.isNotEmpty()) { + throw IllegalArgumentException( + "Only one of: jsonFormat, textFormat can be defined." + ) + } + } +} diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/AccessLogFilterFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/AccessLogFilterFactory.kt deleted file mode 100644 index 4a7e00732..000000000 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/AccessLogFilterFactory.kt +++ /dev/null @@ -1,29 +0,0 @@ -package pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters - -import com.google.re2j.Pattern -import io.envoyproxy.envoy.config.accesslog.v3.ComparisonFilter -import pl.allegro.tech.servicemesh.envoycontrol.groups.AccessLogFilterSettings -import pl.allegro.tech.servicemesh.envoycontrol.groups.NodeMetadataValidationException - -class AccessLogFilterFactory { - private val operators: Array = arrayOf( - ComparisonFilter.Op.LE, ComparisonFilter.Op.GE, ComparisonFilter.Op.EQ - ) - private val delimiter: Char = ':' - private val statusCodeFilterPattern: Pattern = Pattern.compile( - """^(${operators.joinToString("|")})$delimiter(\d{3})$""" - ) - - fun parseStatusCodeFilter(value: String): AccessLogFilterSettings.StatusCodeFilterSettings { - if (!statusCodeFilterPattern.matches(value)) { - throw NodeMetadataValidationException( - "Invalid access log status code filter. Expected OPERATOR:STATUS_CODE" - ) - } - val split = value.split(delimiter) - return AccessLogFilterSettings.StatusCodeFilterSettings( - comparisonOperator = ComparisonFilter.Op.valueOf(split[0]), - comparisonCode = split[1].toInt() - ) - } -} diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/utils/StatusCodeFilterParser.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/utils/StatusCodeFilterParser.kt new file mode 100644 index 000000000..5abcbe744 --- /dev/null +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/utils/StatusCodeFilterParser.kt @@ -0,0 +1,34 @@ +package pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.util + +import com.google.re2j.Pattern +import io.envoyproxy.envoy.config.accesslog.v3.ComparisonFilter +import pl.allegro.tech.servicemesh.envoycontrol.groups.NodeMetadataValidationException + +object StatusCodeFilterParser { + private const val DELIMITER: Char = ':' + + private val operators: Array = arrayOf( + ComparisonFilter.Op.LE, ComparisonFilter.Op.GE, ComparisonFilter.Op.EQ + ) + private val statusCodeFilterPattern: Pattern = Pattern.compile( + """^(${operators.joinToString("|")})$DELIMITER(\d{3})$""" + ) + + fun parseStatusCodeFilter(value: String): StatusCodeFilterSettings { + if (!statusCodeFilterPattern.matches(value)) { + throw NodeMetadataValidationException( + "Invalid access log status code filter. Expected OPERATOR:STATUS_CODE" + ) + } + val split = value.split(DELIMITER) + return StatusCodeFilterSettings( + comparisonOperator = ComparisonFilter.Op.valueOf(split[0]), + comparisonCode = split[1].toInt() + ) + } +} + +data class StatusCodeFilterSettings( + val comparisonOperator: ComparisonFilter.Op, + val comparisonCode: Int +) diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoySnapshotFactoryTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoySnapshotFactoryTest.kt index 3bc9bf7ff..3aaec10d2 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoySnapshotFactoryTest.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoySnapshotFactoryTest.kt @@ -25,7 +25,6 @@ import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SnapshotsVersions import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.clusters.EnvoyClustersFactory import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.endpoints.EnvoyEndpointsFactory import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.EnvoyListenersFactory -import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters.AccessLogFilterFactory import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters.EnvoyHttpFilters import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.routes.EnvoyEgressRoutesFactory import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.routes.EnvoyIngressRoutesFactory @@ -135,7 +134,7 @@ class EnvoySnapshotFactoryTest { ingressPort = INGRESS_PORT, egressHost = EGRESS_HOST, egressPort = EGRESS_PORT, - accessLogFilterSettings = AccessLogFilterSettings(null, AccessLogFilterFactory()) + accessLogFilterSettings = AccessLogFilterSettings(null) ) } diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroupTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroupTest.kt index d24fc04de..458521f53 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroupTest.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroupTest.kt @@ -14,18 +14,16 @@ import org.junit.jupiter.params.provider.CsvSource import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode.ADS import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode.XDS import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SnapshotProperties -import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters.AccessLogFilterFactory import pl.allegro.tech.servicemesh.envoycontrol.snapshot.serviceDependencies import io.envoyproxy.envoy.api.v2.core.Node as NodeV2 import io.envoyproxy.envoy.config.core.v3.Node as NodeV3 class MetadataNodeGroupTest { - val accessLogFilterFactory = AccessLogFilterFactory() @Test fun `should assign to group with all dependencies`() { // given - val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true), accessLogFilterFactory) + val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true)) val node = nodeV3(serviceDependencies = setOf("*", "a", "b", "c"), ads = false) // when @@ -49,7 +47,7 @@ class MetadataNodeGroupTest { @Test fun `should assign to group with no dependencies`() { // given - val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true), accessLogFilterFactory) + val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true)) // when val group = nodeGroup.hash(NodeV3.newBuilder().build()) @@ -66,7 +64,7 @@ class MetadataNodeGroupTest { @Test fun `should assign to group with listed dependencies`() { // given - val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true), accessLogFilterFactory) + val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true)) val node = nodeV3(serviceDependencies = setOf("a", "b", "c"), ads = false) // when @@ -84,7 +82,7 @@ class MetadataNodeGroupTest { @Test fun `should assign to group with all dependencies on ads`() { // given - val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true), accessLogFilterFactory) + val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true)) val node = nodeV3(serviceDependencies = setOf("*"), ads = true) // when @@ -102,7 +100,7 @@ class MetadataNodeGroupTest { @Test fun `should assign to group with listed dependencies on ads`() { // given - val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true), accessLogFilterFactory) + val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true)) val node = nodeV3(serviceDependencies = setOf("a", "b", "c"), ads = true) // when @@ -120,7 +118,7 @@ class MetadataNodeGroupTest { @Test fun `should assign to group with all dependencies when outgoing-permissions is not enabled`() { // given - val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = false), accessLogFilterFactory) + val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = false)) val node = nodeV3(serviceDependencies = setOf("a", "b", "c"), ads = true) // when @@ -140,7 +138,7 @@ class MetadataNodeGroupTest { @Test fun `should not include service settings when incoming permissions are disabled`() { // given - val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true), accessLogFilterFactory) + val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true)) val node = nodeV3( serviceDependencies = setOf("a", "b", "c"), ads = false, serviceName = "app1", @@ -162,7 +160,7 @@ class MetadataNodeGroupTest { @Test fun `should not include service settings when incoming permissions are disabled for all dependencies`() { // given - val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true), accessLogFilterFactory) + val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true)) val node = nodeV3(serviceDependencies = setOf("*"), ads = false, serviceName = "app1", incomingSettings = true) // when @@ -176,8 +174,7 @@ class MetadataNodeGroupTest { fun `should include service settings when incoming permissions are enabled`() { // given val nodeGroup = MetadataNodeGroup( - createSnapshotProperties(outgoingPermissions = true, incomingPermissions = true), - accessLogFilterFactory + createSnapshotProperties(outgoingPermissions = true, incomingPermissions = true) ) val node = nodeV3( serviceDependencies = setOf("a", "b"), @@ -203,8 +200,7 @@ class MetadataNodeGroupTest { fun `should include service settings when incoming permissions are enabled for all dependencies`() { // given val nodeGroup = MetadataNodeGroup( - createSnapshotProperties(outgoingPermissions = true, incomingPermissions = true), - accessLogFilterFactory + createSnapshotProperties(outgoingPermissions = true, incomingPermissions = true) ) val node = nodeV3(serviceDependencies = setOf("*"), ads = false, serviceName = "app1", incomingSettings = true) @@ -224,7 +220,7 @@ class MetadataNodeGroupTest { @Test fun `should parse proto incoming timeout policy`() { // when - val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true), accessLogFilterFactory) + val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true)) val node = nodeV3( serviceDependencies = setOf("*"), ads = true, incomingSettings = true, responseTimeout = "777s", idleTimeout = "13.33s" @@ -241,7 +237,7 @@ class MetadataNodeGroupTest { @Test fun `should parse proto with custom healthCheck definition`() { // when - val nodeGroup = MetadataNodeGroup(createSnapshotProperties(incomingPermissions = true), accessLogFilterFactory) + val nodeGroup = MetadataNodeGroup(createSnapshotProperties(incomingPermissions = true)) val node = nodeV3( serviceDependencies = setOf("*"), ads = true, incomingSettings = true, healthCheckPath = "/status/ping", healthCheckClusterName = "local_service_health_check" @@ -259,7 +255,7 @@ class MetadataNodeGroupTest { @Test fun `should set listeners config access log status code filter according to metadata`() { // given - val nodeGroup = MetadataNodeGroup(createSnapshotProperties(), accessLogFilterFactory) + val nodeGroup = MetadataNodeGroup(createSnapshotProperties()) val metadata = createMetadataBuilderWithDefaults() metadata!!.putFields("access_log_filter", accessLogFilterProto("EQ:400")) @@ -281,7 +277,7 @@ class MetadataNodeGroupTest { ) fun `should throw exception and not set listeners config access log status code filter if invalid config`(config: String) { // given - val nodeGroup = MetadataNodeGroup(createSnapshotProperties(), accessLogFilterFactory) + val nodeGroup = MetadataNodeGroup(createSnapshotProperties()) val metadata = createMetadataBuilderWithDefaults() metadata!!.putFields("access_log_filter", accessLogFilterProto(config)) @@ -298,7 +294,7 @@ class MetadataNodeGroupTest { @Test fun `should not set listeners config access log status code filter if not defined`() { // given - val nodeGroup = MetadataNodeGroup(createSnapshotProperties(), accessLogFilterFactory) + val nodeGroup = MetadataNodeGroup(createSnapshotProperties()) val metadata = createMetadataBuilderWithDefaults() // when @@ -311,7 +307,7 @@ class MetadataNodeGroupTest { @Test fun `should throw exception when V2 node request configuration and support is disabled`() { // given - val nodeGroup = MetadataNodeGroup(createSnapshotProperties(), accessLogFilterFactory) + val nodeGroup = MetadataNodeGroup(createSnapshotProperties()) val metadata = createMetadataBuilderWithDefaults() // expects @@ -329,8 +325,7 @@ class MetadataNodeGroupTest { fun `should assign V2 node to group with listed dependencies when support for V2 is enabled`() { // given val nodeGroup = MetadataNodeGroup( - createSnapshotProperties(outgoingPermissions = true, supportV2 = true), - accessLogFilterFactory + createSnapshotProperties(outgoingPermissions = true, supportV2 = true) ) val node = nodeV2(serviceDependencies = setOf("a", "b", "c"), ads = false) diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataTest.kt index 2c5e1f3ff..e1456b2d5 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataTest.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataTest.kt @@ -12,7 +12,6 @@ import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.Arguments.arguments import org.junit.jupiter.params.provider.MethodSource import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SnapshotProperties -import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters.AccessLogFilterFactory @Suppress("LargeClass") class NodeMetadataTest { @@ -625,9 +624,7 @@ class NodeMetadataTest { val proto = accessLogFilterProto(statusCodeFilter = input) // when - val statusCodeFilterSettings = proto.structValue?.fieldsMap?.get("status_code_filter").toStatusCodeFilter( - AccessLogFilterFactory() - ) + val statusCodeFilterSettings = proto.structValue?.fieldsMap?.get("status_code_filter").toStatusCodeFilter() // expects assertThat(statusCodeFilterSettings?.comparisonCode).isEqualTo(code) @@ -642,9 +639,7 @@ class NodeMetadataTest { // expects val exception = assertThrows { - proto.structValue?.fieldsMap?.get("status_code_filter").toStatusCodeFilter( - AccessLogFilterFactory() - ) + proto.structValue?.fieldsMap?.get("status_code_filter").toStatusCodeFilter() } assertThat(exception.status.description) .isEqualTo("Invalid access log status code filter. Expected OPERATOR:STATUS_CODE") @@ -658,9 +653,7 @@ class NodeMetadataTest { // expects val exception = assertThrows { - proto.structValue?.fieldsMap?.get("status_code_filter").toStatusCodeFilter( - AccessLogFilterFactory() - ) + proto.structValue?.fieldsMap?.get("status_code_filter").toStatusCodeFilter() } assertThat(exception.status.description) .isEqualTo("Invalid access log status code filter. Expected OPERATOR:STATUS_CODE") diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/config/LocalReplyConfigFactoryTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/config/LocalReplyConfigFactoryTest.kt new file mode 100644 index 000000000..1613da04e --- /dev/null +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/config/LocalReplyConfigFactoryTest.kt @@ -0,0 +1,524 @@ +package pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.config + +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatExceptionOfType +import org.junit.jupiter.api.Test +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.Arguments +import org.junit.jupiter.params.provider.MethodSource +import pl.allegro.tech.servicemesh.envoycontrol.snapshot.HeaderMatcher +import pl.allegro.tech.servicemesh.envoycontrol.snapshot.LocalReplyMapperProperties +import pl.allegro.tech.servicemesh.envoycontrol.snapshot.MatcherAndMapper +import pl.allegro.tech.servicemesh.envoycontrol.snapshot.ResponseFormat + +class LocalReplyConfigFactoryTest { + + companion object { + @JvmStatic + fun invalidMatcherCombinations() = listOf( + Arguments.of(MatcherAndMapper().apply { + responseFlagMatcher = listOf("UH", "UF") + statusCodeMatcher = "EQ:400" + headerMatcher = HeaderMatcher().apply { + name = "test" + } + }, "Invalid configuration with all machers defined"), + Arguments.of(MatcherAndMapper().apply { + responseFlagMatcher = listOf("UH", "UF") + headerMatcher = HeaderMatcher().apply { + name = "test" + } + }, "Invalid configuration with responseFlagMatcher, headerMatcher defined"), + Arguments.of(MatcherAndMapper().apply { + statusCodeMatcher = "EQ:400" + headerMatcher = HeaderMatcher().apply { + name = "test" + } + }, "Invalid configuration with statusCodeMatcher, headerMatcher defined"), + Arguments.of(MatcherAndMapper().apply { + responseFlagMatcher = listOf("UH", "UF") + headerMatcher = HeaderMatcher().apply { + name = "test" + } + }, "Invalid configuration with responseFlagMatcher, headerMatcher defined"), + Arguments.of(MatcherAndMapper().apply { + statusCodeToReturn = 500 + }, "Invalid configuration with no matcher defined") + ) + } + + @Test + fun `should set text format for response format`() { + // given + val properties = LocalReplyMapperProperties().apply { + enabled = true + responseFormat = ResponseFormat().apply { + textFormat = "%LOCAL_REPLY%" + } + } + // when + val factory = LocalReplyConfigFactory(properties) + + // then + assertThat(factory.configuration.bodyFormat.textFormat).isEqualTo(properties.responseFormat.textFormat) + } + + @Test + fun `should set json format for response format`() { + // given + val expected = expectedConfigWhenBodyFormatIsCustomJson + val properties = LocalReplyMapperProperties().apply { + enabled = true + responseFormat = ResponseFormat().apply { + jsonFormat = """{ + "body":"%BODY%", + "statusCode":"%STATUS_CODE%" + }""".trimIndent() + } + } + // when + val factory = LocalReplyConfigFactory(properties) + + // then + assertThat(factory.configuration.toString().trimIndent()).isEqualTo(expected) + } + + @Test + fun `should set status code matcher and overridden response format`() { + // given + val expected = expectedConfigWithStatusCodeMatcherAndOverriddenResponseFormatAndOverriddenStatusCode + val properties = LocalReplyMapperProperties().apply { + enabled = true + matchers = listOf( + MatcherAndMapper().apply { + statusCodeToReturn = 500 + bodyToReturn = "Something went wrong and matched with 400" + statusCodeMatcher = "EQ:400" + responseFormat = ResponseFormat().apply { + jsonFormat = """{ + "body":"status code body" + }""" + } + } + ) + responseFormat = ResponseFormat().apply { + jsonFormat = """{ + "body":"%BODY%", + "statusCode":"%STATUS_CODE%" + }""" + } + } + // when + val factory = LocalReplyConfigFactory(properties) + + // then + assertThat(factory.configuration.toString().trimIndent()).isEqualTo(expected) + } + + @Test + fun `should create configuration for exact header matcher`() { + // given + val expected = expectedConfigForHeaderExactMatch + val properties = LocalReplyMapperProperties().apply { + enabled = true + matchers = listOf( + MatcherAndMapper().apply { + headerMatcher = HeaderMatcher().apply { + name = ":path" + exactMatch = "match_this" + } + } + ) + } + // when + val factory = LocalReplyConfigFactory(properties) + + // then + assertThat(factory.configuration.toString().trimIndent()).isEqualTo(expected) + } + + @Test + fun `should create configuration for regex header matcher`() { + // given + val expected = expectedConfigForHeaderRegexMatch + val properties = LocalReplyMapperProperties().apply { + enabled = true + matchers = listOf( + MatcherAndMapper().apply { + headerMatcher = HeaderMatcher().apply { + name = ":path" + regexMatch = "regex_*" + } + } + ) + } + // when + val factory = LocalReplyConfigFactory(properties) + + // then + assertThat(factory.configuration.toString().trimIndent()).isEqualTo(expected) + } + + @Test + fun `should create configuration for presence header matcher`() { + // given + val expected = expectedConfigForHeaderPresenceMatch + val properties = LocalReplyMapperProperties().apply { + enabled = true + matchers = listOf( + MatcherAndMapper().apply { + headerMatcher = HeaderMatcher().apply { + name = ":path" + } + } + ) + } + // when + val factory = LocalReplyConfigFactory(properties) + + // then + assertThat(factory.configuration.toString().trimIndent()).isEqualTo(expected) + } + + @Test + fun `should create configuration for response flags matcher`() { + // given + val expected = expectedConfigForResponseFlagsMatcher + val properties = LocalReplyMapperProperties().apply { + enabled = true + matchers = listOf( + MatcherAndMapper().apply { + responseFlagMatcher = listOf("UH", "UF") + } + ) + } + // when + val factory = LocalReplyConfigFactory(properties) + + // then + assertThat(factory.configuration.toString().trimIndent()).isEqualTo(expected) + } + + @Test + fun `should create nested json format with custom content type`() { + // given + val expected = expectedNestedResponseFormat + val properties = LocalReplyMapperProperties().apply { + enabled = true + responseFormat.apply { + contentType = "application/envoy+json" + jsonFormat = """{ + "destination":{ + "service-tag":"test", + "responseFlags":["UH","UF"], + "listOfIntegers":[1, 2, 3] + }, + "reason":1, + "listOfMap":[ + { + "test":"test" + }, + { + "test2":"test2" + } + ] + }""".trimIndent() + } + } + // when + val factory = LocalReplyConfigFactory(properties) + + // then + assertThat(factory.configuration.toString().trimIndent()).isEqualTo(expected) + } + + @ParameterizedTest + @MethodSource("invalidMatcherCombinations") + fun `should throw exception when more then one matcher is defined in MatcherAndMapper`( + matcherAndMapper: MatcherAndMapper + ) { + // given + val properties = LocalReplyMapperProperties().apply { + enabled = true + matchers = listOf(matcherAndMapper) + } + + // expects + assertThatExceptionOfType(IllegalArgumentException::class.java) + .isThrownBy { LocalReplyConfigFactory(properties) } + .satisfies { + assertThat(it.message).isEqualTo( + "One and only one of: headerMatcher, responseFlagMatcher, statusCodeMatcher has to be defined." + ) + } + } + + @Test + fun `should throw exception when more then one matcher is defined for HeaderMatcher`() { + // given + val properties = LocalReplyMapperProperties().apply { + enabled = true + matchers = listOf( + MatcherAndMapper().apply { + headerMatcher = HeaderMatcher().apply { + name = ":path" + regexMatch = "regex_*" + exactMatch = "exact" + } + } + ) + } + + // expects + assertThatExceptionOfType(IllegalArgumentException::class.java) + .isThrownBy { LocalReplyConfigFactory(properties) } + .satisfies { + assertThat(it.message).isEqualTo( + "Only one of: exactMatch, regexMatch can be defined." + ) + } + } + + @Test + fun `should throw exception when more then one property is defined in response format`() { + // given + val properties = LocalReplyMapperProperties().apply { + enabled = true + responseFormat = ResponseFormat().apply { + jsonFormat = """{ + "body":"custom response body" + }""" + textFormat = "custom response body" + } + } + + // expects + assertThatExceptionOfType(IllegalArgumentException::class.java) + .isThrownBy { LocalReplyConfigFactory(properties) } + .satisfies { + assertThat(it.message).isEqualTo( + "Only one of: jsonFormat, textFormat can be defined." + ) + } + } + + @Test + fun `should throw exception when more then one property is defined in specific matcher response format`() { + // given + val properties = LocalReplyMapperProperties().apply { + enabled = true + matchers = listOf( + MatcherAndMapper().apply { + headerMatcher = HeaderMatcher().apply { + name = ":path" + regexMatch = "regex_*" + } + responseFormat = ResponseFormat().apply { + textFormat = "custom response body" + jsonFormat = """{ + "body":"custom response body" + }""" + } + } + ) + } + + // expects + assertThatExceptionOfType(IllegalArgumentException::class.java) + .isThrownBy { LocalReplyConfigFactory(properties) } + .satisfies { + assertThat(it.message).isEqualTo( + "Only one of: jsonFormat, textFormat can be defined." + ) + } + } + + private val expectedConfigForResponseFlagsMatcher = """mappers { + filter { + response_flag_filter { + flags: "UH" + flags: "UF" + } + } +}""" + + private val expectedConfigForHeaderRegexMatch = """mappers { + filter { + header_filter { + header { + name: ":path" + safe_regex_match { + google_re2 { + } + regex: "regex_*" + } + } + } + } +}""" + + private val expectedConfigForHeaderExactMatch = """mappers { + filter { + header_filter { + header { + name: ":path" + exact_match: "match_this" + } + } + } +}""".trimIndent() + + private val expectedConfigForHeaderPresenceMatch = """mappers { + filter { + header_filter { + header { + name: ":path" + present_match: true + } + } + } +}""".trimIndent() + + private val expectedConfigWhenBodyFormatIsCustomJson = """body_format { + json_format { + fields { + key: "body" + value { + string_value: "%BODY%" + } + } + fields { + key: "statusCode" + value { + string_value: "%STATUS_CODE%" + } + } + } +}""".trimIndent() + + private val expectedConfigWithStatusCodeMatcherAndOverriddenResponseFormatAndOverriddenStatusCode = """mappers { + filter { + status_code_filter { + comparison { + value { + default_value: 400 + runtime_key: "local_reply_mapper_http_code" + } + } + } + } + status_code { + value: 500 + } + body { + inline_string: "Something went wrong and matched with 400" + } + body_format_override { + json_format { + fields { + key: "body" + value { + string_value: "status code body" + } + } + } + } +} +body_format { + json_format { + fields { + key: "body" + value { + string_value: "%BODY%" + } + } + fields { + key: "statusCode" + value { + string_value: "%STATUS_CODE%" + } + } + } +}""".trimIndent() + + private val expectedNestedResponseFormat = """body_format { + json_format { + fields { + key: "destination" + value { + struct_value { + fields { + key: "service-tag" + value { + string_value: "test" + } + } + fields { + key: "responseFlags" + value { + list_value { + values { + string_value: "UH" + } + values { + string_value: "UF" + } + } + } + } + fields { + key: "listOfIntegers" + value { + list_value { + values { + number_value: 1.0 + } + values { + number_value: 2.0 + } + values { + number_value: 3.0 + } + } + } + } + } + } + } + fields { + key: "reason" + value { + number_value: 1.0 + } + } + fields { + key: "listOfMap" + value { + list_value { + values { + struct_value { + fields { + key: "test" + value { + string_value: "test" + } + } + } + } + values { + struct_value { + fields { + key: "test2" + value { + string_value: "test2" + } + } + } + } + } + } + } + } + content_type: "application/envoy+json" +}""".trimIndent() +} diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/LocalReplyMappingTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/LocalReplyMappingTest.kt new file mode 100644 index 000000000..166db58c3 --- /dev/null +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/LocalReplyMappingTest.kt @@ -0,0 +1,115 @@ +package pl.allegro.tech.servicemesh.envoycontrol + +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.RegisterExtension +import pl.allegro.tech.servicemesh.envoycontrol.assertions.isUnreachable +import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension + +class LocalReplyMappingTest { + + companion object { + private const val prefix = "envoy-control.envoy.snapshot" + private const val localReplyPrefix = "$prefix.dynamic-listeners.local-reply-mapper" + + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension( + consul, mapOf( + "$prefix.routing.service-tags.enabled" to true, + "$prefix.routing.service-tags.metadata-key" to "tag", + "$localReplyPrefix.enabled" to true, + "$localReplyPrefix.matchers[0].header-matcher.name" to ":path", + "$localReplyPrefix.matchers[0].header-matcher.exactMatch" to "/api", + "$localReplyPrefix.matchers[0].status-code-to-return" to 510, + "$localReplyPrefix.matchers[0].response-format.json-format" to """{ + "destination":{ + "serviceName":"%REQ(:authority)%", + "serviceTag":"%REQ(x-service-tag)%", + "path":"%REQ(:path)%" + }, + "responseFlags":"%RESPONSE_FLAGS%", + "body":"%LOCAL_REPLY_BODY%", + "path":"%REQ(:path)%" + }""", + "$localReplyPrefix.matchers[1].response-flag-matcher" to listOf( + "NR" + ), + "$localReplyPrefix.matchers[1].status-code-to-return" to 522, + "$localReplyPrefix.matchers[1].body-to-return" to "my-custom no route body", + "$localReplyPrefix.matchers[1].response-format.text-format" to + "Request to service: %REQ(:authority)% responseFlags:%RESPONSE_FLAGS% body: %LOCAL_REPLY_BODY%", + "$localReplyPrefix.response-format.content-type" to "application/envoy+json", + "$localReplyPrefix.response-format.json-format" to """{ + "destination":"service-name: %REQ(:authority)%, service-tag: %REQ(x-service-tag)%", + "responseFlags":"%RESPONSE_FLAGS%", + "body":"%LOCAL_REPLY_BODY%" + }""" + ) + ) + + @JvmField + @RegisterExtension + val service = EchoServiceExtension() + + @JvmField + @RegisterExtension + val envoy = EnvoyExtension(envoyControl, service) + } + + @Test + fun `should return 503 with body in json format`() { + // given + consul.server.operations.registerService(service, name = "service-1") + + // when + untilAsserted { + // when + val response = envoy.egressOperations.callService( + "service-1", headers = mapOf("x-service-tag" to "not-existing") + ) + + assertThat( + response.body()?.string() + ).contains("""{"body":"no healthy upstream","responseFlags":"UH","destination":"service-name: service-1, service-tag: not-existing"}""") + assertThat(response.header("content-type")).isEqualTo("application/envoy+json") + assertThat(response).isUnreachable() + } + } + + @Test + fun `should map no healthy upstream to different json format and rewrite status code to 522`() { + // when + untilAsserted { + // when + val response = envoy.egressOperations.callService("service-2") + + assertThat( + response.body()?.string() + ).contains("Request to service: service-2 responseFlags:NR body: my-custom no route body") + assertThat(response.code()).isEqualTo(522) + } + } + + @Test + fun `should map no healthy upstream to different json format and rewrite status code to 510 when requesting api path`() { + // when + untilAsserted { + // when + val response = envoy.egressOperations.callService("service-2", pathAndQuery = "/api") + + assertThat( + response.body()?.string() + ).contains("""{"destination":{"serviceTag":null,"path":"/api","serviceName":"service-2"},"path":"/api","body":"","responseFlags":"NR"}""") + assertThat(response.code()).isEqualTo(510) + } + } +} From 0ea22d9c0b83cc1844c001682b20c65087f8598b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Dziedziak?= Date: Tue, 2 Feb 2021 11:28:37 +0100 Subject: [PATCH 11/11] Remove request headers on egress (#235) * Remove request headers on egress * Remove unused imports * Added info to docs, changed when to if --- docs/configuration.md | 5 +++-- .../snapshot/SnapshotProperties.kt | 1 + .../routes/EnvoyEgressRoutesFactory.kt | 5 +++++ .../routes/EnvoyEgressRoutesFactoryTest.kt | 19 +++++++++++++++++-- 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index f4f8704dd..307360ee6 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -33,7 +33,7 @@ Property **envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.response-format.text-format** | Text message format with placeholders (refer to [envoy docs](https://www.envoyproxy.io/docs/envoy/latest/configuration/observability/access_log/usage#command-operators)) | "" **envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.response-format.json-format** | JSON message format with placeholders for matched response (refer to [envoy docs](https://www.envoyproxy.io/docs/envoy/latest/configuration/observability/access_log/usage#command-operators)). | "" **envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.response-format.content-type** | Response content-type header value | "" -**envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.matchers.status-code-matcher** | Matcher which handle specific status codes formatted as string e.g.: EQ:400 - equal to status code 400 | "" +**envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.matchers.status-code-matcher** | Matcher which handles specific status codes formatted as string e.g.: EQ:400 - equal to status code 400 | "" **envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.matchers.header-matcher.name** | Header name to match | "" **envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.matchers.header-matcher.exact-match** | Header value to match for specified header (only one of: exactMatch, regexMatch can be specified. If none is specified, header name presence matcher will be used) | "" **envoy-control.envoy.snapshot.dynamic-listeners.local-reply-mapper.matchers.header-matcher.regex-match** | Header value regex to match for specified header (only one of: exactMatch, regexMatch can be specified. If none is specified, header name presence matcher will be used) | "" @@ -61,7 +61,8 @@ Property **envoy-control.envoy.snapshot.egress.handle-internal-redirect** | Handle redirects by Envoy | false **envoy-control.envoy.snapshot.egress.host-header-rewriting.enabled** | Enable rewriting Host header with value from specified header | false **envoy-control.envoy.snapshot.egress.host-header-rewriting.custom-host-header** | Header name which value will override Host header | "x-envoy-original-host" -**envoy-control.envoy.snapshot.ingress.headers-to-remove** | List of headers to sanitize | empty list +**envoy-control.envoy.snapshot.egress.headers-to-remove** | List of headers to sanitize on egress | empty list +**envoy-control.envoy.snapshot.ingress.headers-to-remove** | List of headers to sanitize on ingress | empty list **envoy-control.envoy.snapshot.local-service.idle-timeout** | Idle timeout between client to envoy | 60s **envoy-control.envoy.snapshot.local-service.response-timeout** | Response timeout for localService | 15s **envoy-control.envoy.snapshot.local-service.connection-idle-timeout** | Connection idle timeout for localService | 120s diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt index 99ef76fe1..7a6fcbeb0 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt @@ -230,6 +230,7 @@ class EgressProperties { var commonHttp = CommonHttpProperties() var neverRemoveClusters = true var hostHeaderRewriting = HostHeaderRewritingProperties() + var headersToRemove = mutableListOf() } class IngressProperties { diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyEgressRoutesFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyEgressRoutesFactory.kt index 8db944dae..976069ac6 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyEgressRoutesFactory.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyEgressRoutesFactory.kt @@ -106,6 +106,11 @@ class EnvoyEgressRoutesFactory( ) } } + + if (properties.egress.headersToRemove.isNotEmpty()) { + routeConfiguration.addAllRequestHeadersToRemove(properties.egress.headersToRemove) + } + if (addUpstreamAddressHeader) { routeConfiguration = routeConfiguration.addResponseHeadersToAdd(upstreamAddressHeader) } diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyEgressRoutesFactoryTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyEgressRoutesFactoryTest.kt index 249911d31..9cf6dc398 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyEgressRoutesFactoryTest.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyEgressRoutesFactoryTest.kt @@ -6,11 +6,12 @@ import pl.allegro.tech.servicemesh.envoycontrol.groups.DependencySettings import pl.allegro.tech.servicemesh.envoycontrol.groups.Outgoing import pl.allegro.tech.servicemesh.envoycontrol.groups.hasCustomIdleTimeout import pl.allegro.tech.servicemesh.envoycontrol.groups.hasCustomRequestTimeout -import pl.allegro.tech.servicemesh.envoycontrol.groups.hostRewriteHeaderIsEmpty import pl.allegro.tech.servicemesh.envoycontrol.groups.hasHostRewriteHeader -import pl.allegro.tech.servicemesh.envoycontrol.groups.hasRequestHeaderToAdd import pl.allegro.tech.servicemesh.envoycontrol.groups.hasNoRequestHeaderToAdd +import pl.allegro.tech.servicemesh.envoycontrol.groups.hasRequestHeaderToAdd +import pl.allegro.tech.servicemesh.envoycontrol.groups.hasRequestHeadersToRemove import pl.allegro.tech.servicemesh.envoycontrol.groups.hasResponseHeaderToAdd +import pl.allegro.tech.servicemesh.envoycontrol.groups.hostRewriteHeaderIsEmpty import pl.allegro.tech.servicemesh.envoycontrol.snapshot.RouteSpecification import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SnapshotProperties @@ -138,4 +139,18 @@ internal class EnvoyEgressRoutesFactoryTest { .route .hasHostRewriteHeader(snapshotProperties.egress.hostHeaderRewriting.customHostHeader) } + + @Test + fun `should create route config with headers to remove`() { + // given + val routesFactory = EnvoyEgressRoutesFactory(SnapshotProperties().apply { + egress.headersToRemove = mutableListOf("x-special-case-header", "x-custom") + }) + + // when + val routeConfig = routesFactory.createEgressRouteConfig("client1", clusters, false) + + // then + routeConfig.hasRequestHeadersToRemove(listOf("x-special-case-header", "x-custom")) + } }