From 94308c2460241138b0060079bab6db80ebf148f7 Mon Sep 17 00:00:00 2001 From: slonka Date: Sun, 8 Nov 2020 17:43:48 +0100 Subject: [PATCH 1/3] Add metrics per incoming endpoint --- .../snapshot/SnapshotProperties.kt | 1 + .../routes/EnvoyIngressRoutesFactory.kt | 67 ++++++++- .../config/envoy/EgressOperations.kt | 22 ++- .../MetricsPerIncomingEndpointTest.kt | 139 ++++++++++++++++++ 4 files changed, 223 insertions(+), 6 deletions(-) create mode 100644 envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/MetricsPerIncomingEndpointTest.kt 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 e765ae2e2..9acf8621a 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 @@ -65,6 +65,7 @@ typealias Client = String class IncomingPermissionsProperties { var enabled = false + var metricsPerEndpointEnabled = false var clientIdentityHeaders = listOf("x-service-name") var serviceNameHeader = "x-service-name" var sourceIpAuthentication = SourceIpAuthenticationProperties() diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyIngressRoutesFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyIngressRoutesFactory.kt index 309539363..f59547156 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyIngressRoutesFactory.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyIngressRoutesFactory.kt @@ -12,6 +12,7 @@ import io.envoyproxy.envoy.api.v2.route.RouteMatch import io.envoyproxy.envoy.api.v2.route.VirtualCluster import io.envoyproxy.envoy.api.v2.route.VirtualHost import io.envoyproxy.envoy.type.matcher.RegexMatcher +import pl.allegro.tech.servicemesh.envoycontrol.groups.IncomingEndpoint import pl.allegro.tech.servicemesh.envoycontrol.groups.PathMatchingType import pl.allegro.tech.servicemesh.envoycontrol.groups.ProxySettings import pl.allegro.tech.servicemesh.envoycontrol.protocol.HttpMethod @@ -24,7 +25,6 @@ class EnvoyIngressRoutesFactory( private val properties: SnapshotProperties, envoyHttpFilters: EnvoyHttpFilters = EnvoyHttpFilters.emptyFilters ) { - private val filterMetadata = envoyHttpFilters.ingressMetadata private fun clusterRouteAction( responseTimeout: Duration?, @@ -144,8 +144,71 @@ class EnvoyIngressRoutesFactory( routeAction: RouteAction.Builder ) = routeAction.clone().setRetryPolicy(retryPolicy) + private fun createVirtualClustersForIncomingPermissions(endpoints: List): List { + if (!properties.incomingPermissions.metricsPerEndpointEnabled) { + return listOf() + } + + return endpoints.flatMap { endpoint -> + val pathMatcher = HeaderMatcher.newBuilder() + val virtualClusterName = createVirtualClusterName(endpoint) + when (endpoint.pathMatchingType) { + PathMatchingType.PATH_PREFIX -> { + pathMatcher.setName(":path").setPrefixMatch(endpoint.path) + } + PathMatchingType.PATH -> { + pathMatcher.setName(":path").setExactMatch(endpoint.path) + } + PathMatchingType.PATH_REGEX -> { + pathMatcher.setName(":path").setRe2Match(endpoint.path) + } + } + + val methodMatcher = if (endpoint.methods.isNotEmpty()) { + HeaderMatcher.newBuilder().setName(":method").setRe2Match(endpoint.methods.joinToString("|")).build() + } else { + null + } + + endpoint.clients.map { client -> + val clientMatcher = HeaderMatcher.newBuilder() + .setName(properties.incomingPermissions.serviceNameHeader) + .setExactMatch(client.compositeName()) + .build() + + VirtualCluster.newBuilder() + .addAllHeaders(listOfNotNull(clientMatcher, pathMatcher.build(), methodMatcher)) + .setName("${virtualClusterName}_client_${client.compositeName()}") + .build() + } + VirtualCluster.newBuilder() + .addAllHeaders(listOfNotNull(pathMatcher.build(), methodMatcher)) + .setName("${virtualClusterName}_other_clients") + .build() + } + } + + private fun createVirtualClusterName(endpoint: IncomingEndpoint): String { + return when (endpoint.pathMatchingType) { + PathMatchingType.PATH_PREFIX -> { + endpoint.path + "*" + } + PathMatchingType.PATH -> { + endpoint.path + } + PathMatchingType.PATH_REGEX -> { + "regex_${endpoint.path}" + } + } + "_" + if (endpoint.methods.isNotEmpty()) { + endpoint.methods.joinToString("|") + } else { + "ALL_HTTP_METHODS" + } + } + fun createSecuredIngressRouteConfig(proxySettings: ProxySettings): RouteConfiguration { - val virtualClusters = when (statusRouteVirtualClusterEnabled()) { + val incomingEndpointsVirtualClusters = + createVirtualClustersForIncomingPermissions(proxySettings.incoming.endpoints) + val virtualClusters = incomingEndpointsVirtualClusters + when (statusRouteVirtualClusterEnabled()) { true -> { statusClusters + endpoints } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EgressOperations.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EgressOperations.kt index e420f6e16..cf09cbcd1 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EgressOperations.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EgressOperations.kt @@ -3,8 +3,10 @@ package pl.allegro.tech.servicemesh.envoycontrol.config.envoy import okhttp3.HttpUrl import okhttp3.OkHttpClient import okhttp3.Request +import okhttp3.RequestBody import okhttp3.Response import org.assertj.core.api.Assertions.assertThat +import pl.allegro.tech.discovery.consul.recipes.internal.http.MediaType import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension import java.time.Duration @@ -16,8 +18,12 @@ class EgressOperations(val envoy: EnvoyContainer) { .readTimeout(Duration.ofSeconds(20)) .build() - fun callService(service: String, headers: Map = mapOf(), pathAndQuery: String = "") = - callWithHostHeader(service, headers, pathAndQuery) + fun callService( + service: String, + headers: Map = mapOf(), + pathAndQuery: String = "", + method: String = "GET" + ) = callWithHostHeader(service, headers, pathAndQuery, method) fun callServiceRepeatedly( service: String, @@ -56,10 +62,18 @@ class EgressOperations(val envoy: EnvoyContainer) { "" ) - private fun callWithHostHeader(host: String, moreHeaders: Map, pathAndQuery: String): Response { + private fun callWithHostHeader( + host: String, + moreHeaders: Map, + pathAndQuery: String, + method: String = "GET" + ): Response { + val body = if (method == "POST") { + RequestBody.create(MediaType.JSON_MEDIA_TYPE, "{}") + } else null return client.newCall( Request.Builder() - .get() + .method(method, body) .header("Host", host) .apply { moreHeaders.forEach { name, value -> header(name, value) } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/MetricsPerIncomingEndpointTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/MetricsPerIncomingEndpointTest.kt new file mode 100644 index 000000000..3aab5b523 --- /dev/null +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/permissions/MetricsPerIncomingEndpointTest.kt @@ -0,0 +1,139 @@ +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.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 +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.snapshot.EndpointMatch +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension + +internal class MetricsPerIncomingEndpointTest { + + companion object { + + private val ecProperties = mapOf( + "envoy-control.envoy.snapshot.incoming-permissions.enabled" to true, + "envoy-control.envoy.snapshot.incoming-permissions.metricsPerEndpointEnabled" 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, + "envoy-control.envoy.snapshot.routes.status.path-prefix" to "/status/", + // Round robin gives much more predictable results in tests than LEAST_REQUEST + "envoy-control.envoy.snapshot.load-balancing.policy" to "ROUND_ROBIN" + ) + + // language=yaml + private val echo1EnvoyConfig = Echo1EnvoyAuthConfig.copy(configOverride = """ + node: + metadata: + proxy_settings: + outgoing: + dependencies: + - service: "echo2" + """.trimIndent()) + // language=yaml + private val echo2EnvoyConfig = Echo2EnvoyAuthConfig.copy(configOverride = """ + node: + metadata: + proxy_settings: + incoming: + endpoints: + - path: "/secured_endpoint" + methods: ["GET"] + clients: ["echo"] + unlistedClientsPolicy: "log" + - path: "/secured_endpoint" + clients: ["echo"] + """.trimIndent()) + + // language=yaml + private val echo3EnvoyConfig = Echo3EnvoyAuthConfig.copy(configOverride = """ + node: + metadata: + proxy_settings: + outgoing: + dependencies: + - service: "echo2" + """.trimIndent()) + + @JvmField + @RegisterExtension + val consul = ConsulExtension() + + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension(consul, ecProperties) + + @JvmField + @RegisterExtension + val service1 = EchoServiceExtension() + + @JvmField + @RegisterExtension + val service2 = EchoServiceExtension() + + @JvmField + @RegisterExtension + val echo1Envoy = EnvoyExtension(envoyControl, service1, echo1EnvoyConfig) + + @JvmField + @RegisterExtension + val echo2Envoy = EnvoyExtension(envoyControl, service2, echo2EnvoyConfig) + + @JvmField + @RegisterExtension + val echo3Envoy = EnvoyExtension(envoyControl, service2, echo3EnvoyConfig) + + private fun registerEcho2WithEnvoyOnIngress() { + consul.server.operations.registerService( + id = "echo2", + name = "echo2", + address = echo2Envoy.container.ipAddress(), + port = EnvoyContainer.INGRESS_LISTENER_CONTAINER_PORT, + tags = listOf("mtls:enabled") + ) + } + } + + @BeforeEach + fun setup() { + registerEcho2WithEnvoyOnIngress() + } + + @Test + fun `should produce stats per incoming endpoint`() { + untilAsserted { + // when + val getRequest = echo1Envoy.egressOperations.callService("echo2", pathAndQuery = "/secured_endpoint") + + // then + val getRequestCount = echo2Envoy.container.admin().statValue("vhost.secured_local_service.vcluster./secured_endpoint_GET_client_echo.upstream_rq_200")?.toInt() + assertThat(getRequestCount).isGreaterThan(0) + assertThat(getRequest).isOk().isFrom(service2) + + // when + val notListedMethodRequest = echo1Envoy.egressOperations.callService("echo2", pathAndQuery = "/secured_endpoint", method = "POST") + // then + val notListedMethodRequestCount = echo2Envoy.container.admin().statValue("vhost.secured_local_service.vcluster./secured_endpoint_ALL_HTTP_METHODS_client_echo.upstream_rq_200")?.toInt() + assertThat(notListedMethodRequestCount).isGreaterThan(0) + assertThat(notListedMethodRequest).isOk().isFrom(service2) + + // when + val notListedClientRequest = echo3Envoy.egressOperations.callService("echo2", pathAndQuery = "/secured_endpoint") + // then + val notListedClientRequestCount = echo2Envoy.container.admin().statValue("vhost.secured_local_service.vcluster./secured_endpoint_GET_other_clients.upstream_rq_200")?.toInt() + assertThat(notListedClientRequestCount).isGreaterThan(0) + assertThat(notListedClientRequest).isOk().isFrom(service2) + } + } +} From 5d7d1f133f9ab8c8be5127862444bd2fb82844cf Mon Sep 17 00:00:00 2001 From: slonka Date: Sun, 8 Nov 2020 18:04:13 +0100 Subject: [PATCH 2/3] Enable metrics per incoming endpoint by default --- .../servicemesh/envoycontrol/snapshot/SnapshotProperties.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 9acf8621a..2857b29a1 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 @@ -65,7 +65,7 @@ typealias Client = String class IncomingPermissionsProperties { var enabled = false - var metricsPerEndpointEnabled = false + var metricsPerEndpointEnabled = true var clientIdentityHeaders = listOf("x-service-name") var serviceNameHeader = "x-service-name" var sourceIpAuthentication = SourceIpAuthenticationProperties() From d35237970ecc48435d3483c1fee1105b19bdebb3 Mon Sep 17 00:00:00 2001 From: slonka Date: Mon, 9 Nov 2020 10:23:07 +0100 Subject: [PATCH 3/3] Add docs --- docs/configuration.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/configuration.md b/docs/configuration.md index e318a5278..125b8ff2a 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 that called 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.metrics-per-endpoint-enabled** | Enable metrics per incoming permissions endpoint. | true **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 **envoy-control.envoy.snapshot.incoming-permissions.source-ip-authentication.ip-from-service-discovery.enabled-for-incoming-services** | Enable source ip based authentication for selected services | empty list **envoy-control.envoy.snapshot.incoming-permissions.source-ip-authentication.ip-from-range** | Enable source ip based authentication for selected clients using static IP ranges | empty map