Merge branch 'master' into remove_xds_support

# Conflicts: # CHANGELOG.md # envoy-control-core/src/main/resources/lua/ingress_rbac_logging.lua # envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/routing/ServiceTagsAndCanaryTest.kt
allegro · Mar 22, 2023 · 7feaa29 · 7feaa29
2 parents 6a9f16d + 072d286
commit 7feaa29
Show file tree

Hide file tree

Showing 19 changed files with 279 additions and 46 deletions.
diff --git a/.github/workflows/ci-min-envoy-version.yaml b/.github/workflows/ci-min-envoy-version.yaml
@@ -0,0 +1,11 @@
+name: CI - min envoy version
+
+on:
+  workflow_dispatch:
+
+jobs:
+  ci:
+    uses: ./.github/workflows/ci.yaml
+    with:
+      envoyVersion: min
+    secrets: inherit
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -2,11 +2,22 @@ name: CI
 
 on:
   workflow_dispatch:
+    inputs:
+      envoyVersion:
+        type: string
+        description: "envoy version to run tests on, e.g. 'v1.24.0'. Special values: 'max' - max supported version, 'min' - min supported version"
+        default: max
 
   push:
     paths-ignore:
       - 'readme.md'
 
+  workflow_call:
+    inputs:
+      envoyVersion:
+        type: string
+        default: max
+
 jobs:
   ci:
     name: CI
@@ -37,8 +48,8 @@ jobs:
           restore-keys: |
             ${{ runner.os }}-gradle-
 
-      - name: Test with Gradle
-        run: ./gradlew clean check
+      - name: Test with Gradle (envoyVersion=${{ inputs.envoyVersion }})
+        run: ./gradlew clean check -PenvoyVersion=${{ inputs.envoyVersion }}
 
       - name: Junit report
         uses: mikepenz/action-junit-report@v2

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,20 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 ### Changed
 - Remove xds support
 
+## [0.19.31]
+
+### Changed
+- move min & max envoy versions inside artifact to be accessible for dependant projects
+- add x-service-tag-preference header to upstream request
+
+## [0.19.30]
+
+### Changed
+- add possibility to log custom header in RBAC
+- add token information to RBAC logs
+- specify min and max supported envoy version
+- add option to run tests on specific envoy version, including min and max supported version
+
 ## [0.19.29]
 
 ### Changed

diff --git a/docs/configuration.md b/docs/configuration.md
@@ -137,16 +137,17 @@ Property
 **envoy-control.envoy.snapshot.load-balancing.use-keys-subset-fallback-policy**             | KEYS_SUBSET fallback policy is used by default when canary and service-tags are enabled. It is not supported in Envoy <= 1.12.x. Set to false for compatibility with Envoy 1.12.x         | true
 
 ## Routing
-Property                                                                                    | Description                                                                                                                                                                      | Default value
-------------------------------------------------------------------------------------------- |----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| ---------
-**envoy-control.envoy.snapshot.routing.service-tags.enabled**                               | If set to true, service tags routing will be enabled                                                                                                                             | false
-**envoy-control.envoy.snapshot.routing.service-tags.metadata-key**                          | What key to use in endpoint metadata to store its service tags                                                                                                                   | tag
-**envoy-control.envoy.snapshot.routing.service-tags.header**                                | What header to use in service tag rules                                                                                                                                          | x-service-tag
+Property                                                                                    | Description                                                                                                                                                                       | Default value
+------------------------------------------------------------------------------------------- |-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| ---------
+**envoy-control.envoy.snapshot.routing.service-tags.enabled**                               | If set to true, service tags routing will be enabled                                                                                                                              | false
+**envoy-control.envoy.snapshot.routing.service-tags.metadata-key**                          | What key to use in endpoint metadata to store its service tags                                                                                                                    | tag
+**envoy-control.envoy.snapshot.routing.service-tags.header**                                | What header to use in service tag rules                                                                                                                                           | x-service-tag
+**envoy-control.envoy.snapshot.routing.service-tags.preference-header**                     | What header to use for service tag preference list. Used for sending info to upstream if 'auto service tags' is in force. In the future also read from downstream request.        | x-service-tag-preference
 **envoy-control.envoy.snapshot.routing.service-tags.routing-excluded-tags**                 | List of tags predicates that cannot be used for routing. This supports an exact matching (just "string" - EXACT matching) prefixes (PREFIX matching) and regexes (REGEX matching) | empty list 
-**envoy-control.envoy.snapshot.routing.service-tags.allowed-tags-combinations**             | List of rules, which tags can be conbined together and requested together. Details below                                                                                         | empty list
-**(...).allowed-tags-combinations[].service-name**                                          | The rule will apply only for this service                                                                                                                                        | ""
-**(...).allowed-tags-combinations[].tags**                                                  | List of tag patterns, that can be combined and requested together                                                                                                                | empty list
-**envoy-control.envoy.snapshot.routing.service-tags.auto-service-tag-enabled**              | Enable auto service tag feature. (`enabled` needs also be true)                                                                                                                  | false
+**envoy-control.envoy.snapshot.routing.service-tags.allowed-tags-combinations**             | List of rules, which tags can be conbined together and requested together. Details below                                                                                          | empty list
+**(...).allowed-tags-combinations[].service-name**                                          | The rule will apply only for this service                                                                                                                                         | ""
+**(...).allowed-tags-combinations[].tags**                                                  | List of tag patterns, that can be combined and requested together                                                                                                                 | empty list
+**envoy-control.envoy.snapshot.routing.service-tags.auto-service-tag-enabled**              | Enable auto service tag feature. (`enabled` needs also be true)                                                                                                                   | false
 
 ## Outlier detection
 Property                                                                                         | Description                                                                                                                                                                               | Default value

diff --git a/...e/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt b/...e/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt
@@ -96,6 +96,7 @@ class IncomingPermissionsProperties {
     var clientsAllowedToAllEndpoints = mutableListOf<String>()
     var clientsLists = ClientsListsProperties()
     var overlappingPathsFix = false // TODO: to be removed when proved it did not mess up anything
+    var headersToLogInRbac: List<String> = emptyList()
 }
 
 class SelectorMatching {
@@ -235,9 +236,12 @@ class ServiceTagsProperties {
     var enabled = false
     var metadataKey = "tag"
     var header = "x-service-tag"
+    var preferenceHeader = "x-service-tag-preference"
     var routingExcludedTags: MutableList<StringMatcher> = mutableListOf()
     var allowedTagsCombinations: MutableList<ServiceTagsCombinationsProperties> = mutableListOf()
     var autoServiceTagEnabled = false
+
+    fun isAutoServiceTagEffectivelyEnabled() = enabled && autoServiceTagEnabled
 }
 
 class StringMatcher {

diff --git a/...llegro/tech/servicemesh/envoycontrol/snapshot/resource/endpoints/EnvoyEndpointsFactory.kt b/...llegro/tech/servicemesh/envoycontrol/snapshot/resource/endpoints/EnvoyEndpointsFactory.kt
@@ -52,7 +52,7 @@ class EnvoyEndpointsFactory(
         clusterLoadAssignment: ClusterLoadAssignment,
         routingPolicy: RoutingPolicy
     ): ClusterLoadAssignment {
-        if (!routingPolicy.autoServiceTag || !isAutoServiceTagEnabled()) {
+        if (!routingPolicy.autoServiceTag || !properties.routing.serviceTags.isAutoServiceTagEffectivelyEnabled()) {
             return clusterLoadAssignment
         }
 
@@ -67,8 +67,6 @@ class EnvoyEndpointsFactory(
         }
     }
 
-    private fun isAutoServiceTagEnabled() = properties.routing.serviceTags.run { enabled && autoServiceTagEnabled }
-
     private fun filterEndpoints(loadAssignment: ClusterLoadAssignment, tag: String): ClusterLoadAssignment? {
         var allEndpointMatched = true
         val filteredEndpoints = loadAssignment.endpointsList.mapNotNull { localityLbEndpoint ->

diff --git a/...gro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/LuaFilterFactory.kt b/...gro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/LuaFilterFactory.kt
@@ -64,7 +64,9 @@ class LuaFilterFactory(private val incomingPermissionsProperties: IncomingPermis
             ),
             "service_name" to StringPropertyLua(group.serviceName),
             "discovery_service_name" to StringPropertyLua(group.discoveryServiceName ?: ""),
-
+            "rbac_headers_to_log" to ListPropertyLua(
+                incomingPermissionsProperties.headersToLogInRbac.map(::StringPropertyLua)
+            ),
             ) + customLuaMetadata
         return Metadata.newBuilder()
             .putFilterMetadata("envoy.filters.http.lua", metadata.toValue().structValue)

diff --git a/...llegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyEgressRoutesFactory.kt b/...llegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyEgressRoutesFactory.kt
@@ -139,6 +139,24 @@ class EnvoyEgressRoutesFactory(
                     .build()
             )
         }
+
+        if (properties.routing.serviceTags.isAutoServiceTagEffectivelyEnabled()) {
+            val routingPolicy = routeSpecification.settings.routingPolicy
+            if (routingPolicy.autoServiceTag) {
+                val tagsPreferenceJoined = routingPolicy.serviceTagPreference.joinToString("|")
+                virtualHost.addRequestHeadersToAdd(
+                    HeaderValueOption.newBuilder()
+                        .setHeader(
+                            HeaderValue.newBuilder()
+                                .setKey(properties.routing.serviceTags.preferenceHeader)
+                                .setValue(tagsPreferenceJoined)
+                        )
+                        .setAppendAction(HeaderValueOption.HeaderAppendAction.OVERWRITE_IF_EXISTS_OR_ADD)
+                        .setKeepEmptyValue(false)
+                )
+            }
+        }
+
         return virtualHost.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
@@ -128,7 +128,7 @@ function log_request(handle, lua_metadata, jwt_status, rbac_action)
             table.insert(message, ',"')
             table.insert(message, header)
             table.insert(message, '":"')
-            table.insert(message, value)
+            table.insert(message, tostring(escape(value)))
             table.insert(message, '"')
         end
     end

diff --git a/envoy-control-tests/build.gradle b/envoy-control-tests/build.gradle
@@ -39,3 +39,7 @@ task reliabilityTest(type: Test) {
     }
     testClassesDirs = project.sourceSets.main.output.classesDirs
 }
+
+tasks.withType(Test).configureEach {
+    project.findProperty("envoyVersion")?.with { systemProperty("pl.allegro.tech.servicemesh.envoyVersion", it) }
+}
diff --git a/...ts/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/assertions/EnvoyAssertions.kt b/...ts/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/assertions/EnvoyAssertions.kt
@@ -18,7 +18,8 @@ private class RbacLog(
     val clientIp: String? = null,
     val statusCode: String? = null,
     val requestId: String? = null,
-    val rbacAction: String? = null
+    val rbacAction: String? = null,
+    val jwtTokenStatus: String? = null,
 )
 
 private const val RBAC_LOG_PREFIX = "INCOMING_PERMISSIONS"
@@ -190,6 +191,7 @@ private fun ObjectAssert<String>.matchesRbacAccessDeniedLog(logPredicate: RbacLo
     assertEqualProperty(parsed, logPredicate, RbacLog::requestId)
     assertEqualProperty(parsed, logPredicate, RbacLog::rbacAction)
     assertEqualProperty(parsed, logPredicate, RbacLog::statusCode)
+    assertEqualProperty(parsed, logPredicate, RbacLog::jwtTokenStatus)
 }
 
 private fun <T> assertEqualProperty(actual: RbacLog, expected: RbacLog, supplier: RbacLog.() -> T) {

diff --git a/...s/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyContainer.kt b/...s/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyContainer.kt
@@ -36,8 +36,21 @@ class EnvoyContainer(
         const val ENVOY_UID_ENV_NAME = "ENVOY_UID"
         const val EGRESS_LISTENER_CONTAINER_PORT = 5000
         const val INGRESS_LISTENER_CONTAINER_PORT = 5001
-        const val DEFAULT_IMAGE = "envoyproxy/envoy:v1.24.0"
         private const val ADMIN_PORT = 10000
+
+        private const val MIN_SUPPORTED_ENVOY_VERSION = "v1.22.7"
+        private const val MAX_SUPPORTED_ENVOY_VERSION = "v1.24.0"
+
+        val DEFAULT_IMAGE = run {
+            val version =
+                when (val versionArg = System.getProperty("pl.allegro.tech.servicemesh.envoyVersion").orEmpty()) {
+                    "max", "" -> MAX_SUPPORTED_ENVOY_VERSION
+                    "min" -> MIN_SUPPORTED_ENVOY_VERSION
+                    else -> versionArg
+                }
+            logger.info("Using envoy version: $version")
+            "envoyproxy/envoy:$version"
+        }
     }
 
     override fun configure() {

diff --git a/...s/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt b/...s/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt
@@ -14,6 +14,7 @@ import pl.allegro.tech.servicemesh.envoycontrol.config.RandomConfigFile
 import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtensionBase
 import pl.allegro.tech.servicemesh.envoycontrol.config.service.ServiceExtension
 import pl.allegro.tech.servicemesh.envoycontrol.logger
+import java.time.Duration
 
 class EnvoyExtension(
     private val envoyControl: EnvoyControlExtensionBase,
@@ -82,6 +83,38 @@ class EnvoyExtension(
         }
     }
 
+    fun waitForClusterEndpointHealthy(
+        cluster: String,
+        endpointIp: String
+    ) {
+        untilAsserted(wait = Duration.ofSeconds(5)) {
+            assertThat(container.admin().isEndpointHealthy(cluster, endpointIp))
+                .withFailMessage {
+                    "Expected to see healthy endpoint of cluster '$cluster' with address " +
+                        "'$endpointIp' in envoy ${container.adminUrl()}/clusters, " +
+                        "but it's not present. Found following endpoints: " +
+                        "${container.admin().endpointsAddress(cluster)}"
+                }
+                .isTrue()
+        }
+    }
+
+    fun waitForClusterEndpointNotHealthy(
+        cluster: String,
+        endpointIp: String
+    ) {
+        untilAsserted(wait = Duration.ofSeconds(5)) {
+            assertThat(container.admin().isEndpointHealthy(cluster, endpointIp))
+                .withFailMessage {
+                    "Expected to not see endpoint of cluster '$cluster' with address " +
+                        "'$endpointIp' in envoy ${container.adminUrl()}/clusters, " +
+                        "but it's still present. Found following endpoints: " +
+                        "${container.admin().endpointsAddress(cluster)}"
+                }
+                .isFalse()
+        }
+    }
+
     fun recordRBACLogs() {
         container.logRecorder.recordLogs(::isRbacAccessLog)
     }

diff --git a/...h/envoycontrol/CanaryLoadBalancingTest.kt → ...ontrol/routing/CanaryLoadBalancingTest.kt b/...h/envoycontrol/CanaryLoadBalancingTest.kt → ...ontrol/routing/CanaryLoadBalancingTest.kt
@@ -1,4 +1,4 @@
-package pl.allegro.tech.servicemesh.envoycontrol
+package pl.allegro.tech.servicemesh.envoycontrol.routing
 
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Test

diff --git a/...icemesh/envoycontrol/RoutingPolicyTest.kt → ...envoycontrol/routing/RoutingPolicyTest.kt b/...icemesh/envoycontrol/RoutingPolicyTest.kt → ...envoycontrol/routing/RoutingPolicyTest.kt
@@ -1,4 +1,4 @@
-package pl.allegro.tech.servicemesh.envoycontrol
+package pl.allegro.tech.servicemesh.envoycontrol.routing
 
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Test
@@ -306,35 +306,18 @@ class RoutingPolicyTest {
         serviceInstance: EchoServiceExtension,
         envoy: EnvoyExtension
     ) {
-        untilAsserted(wait = Duration.ofSeconds(5)) {
-            assertThat(envoy.container.admin().isEndpointHealthy(serviceName, serviceInstance.container().ipAddress()))
-                .withFailMessage {
-                    "Expected to see healthy endpoint of cluster '$serviceName' with address " +
-                        "'${serviceInstance.container().address()}' in envoy " +
-                        "${serviceInstance.container().address()}/clusters, " +
-                        "but it's not present. Found following endpoints: " +
-                        "${envoy.container.admin().endpointsAddress(serviceName)}"
-                }
-                .isTrue()
-        }
+        envoy.waitForClusterEndpointHealthy(cluster = serviceName, endpointIp = serviceInstance.container().ipAddress())
     }
 
     private fun waitForEndpointRemoved(
         serviceName: String,
         serviceInstance: EchoServiceExtension,
         envoy: EnvoyExtension
     ) {
-        untilAsserted(wait = Duration.ofSeconds(5)) {
-            assertThat(envoy.container.admin().isEndpointHealthy(serviceName, serviceInstance.container().ipAddress()))
-                .withFailMessage {
-                    "Expected to not see endpoint of cluster '$serviceName' with address " +
-                        "'${serviceInstance.container().address()}' in envoy " +
-                        "${serviceInstance.container().address()}/clusters, " +
-                        "but it's still present. Found following endpoints: " +
-                        "${envoy.container.admin().endpointsAddress(serviceName)}"
-                }
-                .isFalse()
-        }
+        envoy.waitForClusterEndpointNotHealthy(
+            cluster = serviceName,
+            endpointIp = serviceInstance.container().ipAddress()
+        )
     }
 
     private fun callStats() = CallStats(listOf(ipsumEchoService, loremEchoService, otherEchoService))