configurable path normalization

CHANGELOG.md

@@ -2,7 +2,17 @@
 
 Lists all changes with user impact.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
-## [0.20.4]
+## [0.20.9]
+### Changed
+- Configurable path normalization
+
+## [0.20.6 - 0.20.8]
+
+### Changed
+- Merge slashes in http request
+- Feature flag for auditing global snapshot
+
+## [0.20.5]
 
 ### Changed
 - Added possibility to add response header for weighted secondary cluster 

envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/Groups.kt

@@ -7,6 +7,7 @@ sealed class Group {
     abstract val serviceName: String
     abstract val discoveryServiceName: String?
     abstract val proxySettings: ProxySettings
+    abstract val pathNormalizationConfig: PathNormalizationConfig
     abstract val listenersConfig: ListenersConfig?
 }
 
@@ -15,17 +16,25 @@ data class ServicesGroup(
     override val serviceName: String = "",
     override val discoveryServiceName: String? = null,
     override val proxySettings: ProxySettings = ProxySettings(),
-    override val listenersConfig: ListenersConfig? = null
+    override val pathNormalizationConfig: PathNormalizationConfig,
+    override val listenersConfig: ListenersConfig? = null,
 ) : Group()
 
 data class AllServicesGroup(
     override val communicationMode: CommunicationMode,
     override val serviceName: String = "",
     override val discoveryServiceName: String? = null,
     override val proxySettings: ProxySettings = ProxySettings(),
+    override val pathNormalizationConfig: PathNormalizationConfig,
     override val listenersConfig: ListenersConfig? = null
 ) : Group()
 
+data class PathNormalizationConfig(
+    val normalizationEnabled: Boolean,
+    val mergeSlashes: Boolean,
+    val pathWithEscapedSlashesAction: String
+)
+
 data class ListenersConfig(
     val ingressHost: String,
     val ingressPort: Int,

envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroup.kt

@@ -172,14 +172,14 @@ class MetadataNodeGroup(
         val discoveryServiceName = nodeMetadata.discoveryServiceName
         val proxySettings = proxySettings(nodeMetadata)
         val listenersConfig = createListenersConfig(node.id, node.metadata, node.userAgentBuildVersion)
-
         return when {
             hasAllServicesDependencies(nodeMetadata) ->
                 AllServicesGroup(
                     nodeMetadata.communicationMode,
                     serviceName,
                     discoveryServiceName,
                     proxySettings,
+                    nodeMetadata.pathNormalizationConfig,
                     listenersConfig
                 )
             else ->
@@ -188,6 +188,7 @@ class MetadataNodeGroup(
                     serviceName,
                     discoveryServiceName,
                     proxySettings,
+                    nodeMetadata.pathNormalizationConfig,
                     listenersConfig
                 )
         }

envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadata.kt

@@ -33,6 +33,7 @@ class NodeMetadata(metadata: Struct, properties: SnapshotProperties) {
 
     val communicationMode = getCommunicationMode(metadata.fieldsMap["ads"])
 
+    val pathNormalizationConfig = getPathNormalization(metadata.fieldsMap["path_normalization"], properties)
     val proxySettings: ProxySettings = ProxySettings(metadata.fieldsMap["proxy_settings"], properties)
 }
 
@@ -68,6 +69,22 @@ data class ProxySettings(
     )
 }
 
+fun getPathNormalization(proto: Value?, snapshotProperties: SnapshotProperties): PathNormalizationConfig {
+    val defaultNormalizationConfig = PathNormalizationConfig(
+        snapshotProperties.pathNormalization.enabled,
+        snapshotProperties.pathNormalization.mergeSlashes,
+        snapshotProperties.pathNormalization.pathWithEscapedSlashesAction
+    )
+    if (proto == null) {
+        return defaultNormalizationConfig
+    }
+    return PathNormalizationConfig(
+        normalizationEnabled = proto.field("enabled")?.boolValue ?: defaultNormalizationConfig.normalizationEnabled,
+        mergeSlashes = proto.field("merge_slashes")?.boolValue ?: defaultNormalizationConfig.mergeSlashes,
+        pathWithEscapedSlashesAction = proto.field("path_with_escaped_slashes_action")?.stringValue ?: defaultNormalizationConfig.pathWithEscapedSlashesAction
+    )
+}
+
 private fun getCommunicationMode(proto: Value?): CommunicationMode {
     val ads = proto
         ?.boolValue
@@ -366,9 +383,11 @@ fun Value.toIncomingEndpoint(properties: SnapshotProperties): IncomingEndpoint {
         pathPrefix != null -> IncomingEndpoint(
             pathPrefix, PathMatchingType.PATH_PREFIX, methods, clients, unlistedClientsPolicy, oauth
         )
+
         pathRegex != null -> IncomingEndpoint(
             pathRegex, PathMatchingType.PATH_REGEX, methods, clients, unlistedClientsPolicy, oauth
         )
+
         else -> throw NodeMetadataValidationException("One of 'path', 'pathPrefix' or 'pathRegex' field is required")
     }
 }
@@ -391,9 +410,11 @@ fun Value.toIncomingRateLimitEndpoint(): IncomingRateLimitEndpoint {
         pathPrefix != null -> IncomingRateLimitEndpoint(
             pathPrefix, PathMatchingType.PATH_PREFIX, methods, clients, rateLimit
         )
+
         pathRegex != null -> IncomingRateLimitEndpoint(
             pathRegex, PathMatchingType.PATH_REGEX, methods, clients, rateLimit
         )
+
         else -> throw NodeMetadataValidationException("One of 'path', 'pathPrefix' or 'pathRegex' field is required")
     }
 }
@@ -473,13 +494,15 @@ fun Value.toDuration(): Duration? {
             "Timeout definition has number format" +
                 " but should be in string format and ends with 's'"
         )
+
         Value.KindCase.STRING_VALUE -> {
             try {
                 this.stringValue?.takeIf { it.isNotBlank() }?.let { Durations.parse(it) }
             } catch (ex: ParseException) {
                 throw NodeMetadataValidationException("Timeout definition has incorrect format: ${ex.message}")
             }
         }
+
         else -> null
     }
 }

envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataValidator.kt

@@ -1,6 +1,7 @@
 package pl.allegro.tech.servicemesh.envoycontrol.groups
 
 import io.envoyproxy.controlplane.server.DiscoveryServerCallbacks
+import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
 import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode.ADS
 import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode.XDS
 import pl.allegro.tech.servicemesh.envoycontrol.logger
@@ -40,6 +41,9 @@ class ServiceNameNotProvidedException : NodeMetadataValidationException(
     "Service name has not been provided."
 )
 
+class InvalidPathWithEscapedSlashesAction(action: String) : NodeMetadataValidationException(
+    "$action is invalid value for pathWithEscapedSlashesAction."
+)
 class RateLimitIncorrectValidationException(rateLimit: String?) : NodeMetadataValidationException(
     "Rate limit value: $rateLimit is incorrect."
 )
@@ -82,11 +86,22 @@ class NodeMetadataValidator(
     private fun validateMetadata(metadata: NodeMetadata) {
         validateServiceName(metadata)
         validateDependencies(metadata)
+        validatePathNormalization(metadata)
         validateIncomingEndpoints(metadata)
         validateIncomingRateLimitEndpoints(metadata)
         validateConfigurationMode(metadata)
     }
 
+    private fun validatePathNormalization(metadata: NodeMetadata) {
+        val action = metadata.pathNormalizationConfig.pathWithEscapedSlashesAction
+
+        val actionIsValidEnumValue = HttpConnectionManager.PathWithEscapedSlashesAction.values()
+            .any { it.name.uppercase() == action.uppercase() }
+        if (actionIsValidEnumValue) {
+            throw InvalidPathWithEscapedSlashesAction(action)
+        }
+    }
+
     private fun validateServiceName(metadata: NodeMetadata) {
         if (properties.requireServiceName) {
             if (metadata.serviceName.isNullOrEmpty()) {

envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt

@@ -13,6 +13,7 @@ import java.time.Duration
 class SnapshotProperties {
     var routes = RoutesProperties()
     var localService = LocalServiceProperties()
+    var pathNormalization = PathNormalizationProperties()
     var egress = EgressProperties()
     var ingress = IngressProperties()
     var incomingPermissions = IncomingPermissionsProperties()
@@ -39,6 +40,11 @@ class SnapshotProperties {
     var shouldAuditGlobalSnapshot: Boolean = true
 }
 
+class PathNormalizationProperties {
+    var enabled = true
+    var mergeSlashes = true
+    var pathWithEscapedSlashesAction = "KEEP_UNCHANGED"
+}
 class MetricsProperties {
     var cacheSetSnapshot = false
 }

envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/HttpConnectionManagerFactory.kt

@@ -51,6 +51,7 @@ class HttpConnectionManagerFactory(
     ): HttpConnectionManager? {
         val listenersConfig = group.listenersConfig!!
 
+        val normalizationConfig = group.pathNormalizationConfig
         val connectionManagerBuilder = HttpConnectionManager.newBuilder()
             .setStatPrefix(statPrefix)
             .setRds(setupRds(group.communicationMode, initialFetchTimeout, routeConfigName))
@@ -66,9 +67,9 @@ class HttpConnectionManagerFactory(
                     .setUseRemoteAddress(BoolValue.newBuilder().setValue(listenersConfig.useRemoteAddress).build())
                     .setDelayedCloseTimeout(Duration.newBuilder().setSeconds(0).build())
                     .setCommonHttpProtocolOptions(httpProtocolOptions)
-                    .setNormalizePath(BoolValue.newBuilder().setValue(true).build())
-                    .setPathWithEscapedSlashesAction(HttpConnectionManager.PathWithEscapedSlashesAction.KEEP_UNCHANGED)
-                    .setMergeSlashes(true)
+                    .setNormalizePath(BoolValue.newBuilder().setValue(normalizationConfig.normalizationEnabled).build())
+                    .setPathWithEscapedSlashesAction(normalizationConfig.pathWithEscapedSlashesAction.toPathWithEscapedSlashesActionEnum())
+                    .setMergeSlashes(normalizationConfig.mergeSlashes)
                     .setCodecType(HttpConnectionManager.CodecType.AUTO)
                     .setHttpProtocolOptions(ingressHttp1ProtocolOptions(group.serviceName))
                 if (listenersConfig.useRemoteAddress) {
@@ -77,6 +78,7 @@ class HttpConnectionManagerFactory(
                     )
                 }
             }
+
             Direction.EGRESS -> {
                 connectionManagerBuilder
                     .setHttpProtocolOptions(
@@ -108,6 +110,12 @@ class HttpConnectionManagerFactory(
         return connectionManagerBuilder.build()
     }
 
+    private fun String.toPathWithEscapedSlashesActionEnum(): HttpConnectionManager.PathWithEscapedSlashesAction {
+        return HttpConnectionManager.PathWithEscapedSlashesAction.values()
+            .find { it.name.uppercase() == this.uppercase() }
+            ?: HttpConnectionManager.PathWithEscapedSlashesAction.UNRECOGNIZED
+    }
+
     private fun setupRds(
         communicationMode: CommunicationMode,
         rdsInitialFetchTimeout: Duration,

envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataTest.kt

@@ -1194,6 +1194,33 @@ class NodeMetadataTest {
         assertThat(exception.status.code).isEqualTo(Status.Code.INVALID_ARGUMENT)
     }
 
+    @Test
+    fun `should parse normalization config`() {
+        // given
+        val proto = pathNormalizationProto(normalizationEnabled = true, mergeSlashes = false, pathWithEscapedSlashesAction = "UNESCAPE_AND_REDIRECT")
+
+        // when
+        val value = getPathNormalization(proto, snapshotProperties())
+        // expects
+
+        assertThat(value.normalizationEnabled).isTrue
+        assertThat(value.mergeSlashes).isFalse
+        assertThat(value.pathWithEscapedSlashesAction).isEqualTo("UNESCAPE_AND_REDIRECT")
+    }
+    @Test
+    fun `should use default values when there is no normalization config`() {
+        // given
+        val proto = pathNormalizationProto(normalizationEnabled = null, mergeSlashes = null, pathWithEscapedSlashesAction = null)
+
+        // when
+        val value = getPathNormalization(proto, snapshotProperties())
+        // expects
+
+        assertThat(value.normalizationEnabled).isTrue
+        assertThat(value.mergeSlashes).isTrue()
+        assertThat(value.pathWithEscapedSlashesAction).isEqualTo("KEEP_UNCHANGED")
+    }
+
     @Test
     fun `should use default routing policy`() {
         // given

envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataValidatorTest.kt

@@ -261,6 +261,25 @@ class NodeMetadataValidatorTest {
             })
     }
 
+    @Test
+    fun `should throw an exception when PathWithEscapedSlashesAction is invalid`() {
+        // given
+        val node = nodeV3(
+            pathNormalization = PathNormalizationConfig(pathWithEscapedSlashesAction = "foo", normalizationEnabled = true, mergeSlashes = true)
+        )
+        val request = DiscoveryRequestV3.newBuilder()
+            .setNode(node)
+            .build()
+
+        // then
+        assertThatExceptionOfType(InvalidPathWithEscapedSlashesAction::class.java)
+            .isThrownBy { validator.onV3StreamRequest(123, request = request) }
+            .satisfies(Consumer {
+                assertThat(it.status.description).isEqualTo("foo is invalid value for pathWithEscapedSlashesAction.")
+                assertThat(it.status.code).isEqualTo(Status.Code.INVALID_ARGUMENT)
+            })
+    }
+
     private fun createIncomingPermissions(
         enabled: Boolean = false,
         servicesAllowedToUseWildcard: MutableSet<String> = mutableSetOf()

envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/TestNodeFactory.kt

@@ -21,7 +21,8 @@ fun nodeV3(
     connectionIdleTimeout: String? = null,
     healthCheckPath: String? = null,
     healthCheckClusterName: String? = null,
-    rateLimit: String? = null
+    rateLimit: String? = null,
+    pathNormalization: PathNormalizationConfig? = null
 ): NodeV3 {
     val meta = NodeV3.newBuilder().metadataBuilder
 
@@ -54,6 +55,16 @@ fun nodeV3(
             )
         )
     }
+    if (pathNormalization != null) {
+        meta.putFields(
+            "path_normalization",
+            pathNormalizationProto(
+                pathNormalization.normalizationEnabled,
+                pathNormalization.mergeSlashes,
+                pathNormalization.pathWithEscapedSlashesAction
+            )
+        )
+    }
 
     return NodeV3.newBuilder()
         .setMetadata(meta)
@@ -112,6 +123,29 @@ fun accessLogFilterProto(value: String? = null, fieldName: String): Value = stru
     }
 }
 
+fun pathNormalizationProto(
+    normalizationEnabled: Boolean?,
+    mergeSlashes: Boolean?,
+    pathWithEscapedSlashesAction: String?
+) = struct {
+        normalizationEnabled?.let {
+            putFields(
+                "enabled", boolean(it)
+            )
+        }
+        mergeSlashes?.let {
+            putFields(
+                "merge_slashes", boolean(it)
+            )
+        }
+        pathWithEscapedSlashesAction?.let {
+            putFields(
+                "path_with_escaped_slashes_action", string(it)
+            )
+        }
+    }
+
+
 fun accessLogBooleanFilterProto(value: Boolean? = null, fieldName: String): Value = struct {
     when {
         value != null -> putFields(fieldName, boolean(value))
@@ -186,6 +220,7 @@ data class RetryBackOffInput(
     val baseInterval: String? = null,
     val maxInterval: String? = null
 )
+
 data class RetryHostPredicateInput(
     val name: String?
 )
@@ -216,7 +251,13 @@ class OutgoingDependenciesProtoScope {
         serviceDependencies: List<String> = emptyList(),
         idleTimeout: String? = null,
         responseTimeout: String? = null
-    ) = serviceDependencies.forEach { withService(it, idleTimeout, responseTimeout) } // TODO: responseTimeout as connectionIdleTimeout, is this correct?
+    ) = serviceDependencies.forEach {
+        withService(
+            it,
+            idleTimeout,
+            responseTimeout
+        )
+    } // TODO: responseTimeout as connectionIdleTimeout, is this correct?
 
     fun withService(
         serviceName: String,