diff --git a/CHANGELOG.md b/CHANGELOG.md index 65e5bed63..6ad8bd4fd 100644 --- a/CHANGELOG.md +++ b/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 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 ef23e5ec7..fef3b2c3e 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 @@ -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,7 +16,8 @@ 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( @@ -23,9 +25,16 @@ data class AllServicesGroup( 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, 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 153319669..b60a06edb 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 @@ -172,7 +172,6 @@ class MetadataNodeGroup( val discoveryServiceName = nodeMetadata.discoveryServiceName val proxySettings = proxySettings(nodeMetadata) val listenersConfig = createListenersConfig(node.id, node.metadata, node.userAgentBuildVersion) - return when { hasAllServicesDependencies(nodeMetadata) -> AllServicesGroup( @@ -180,6 +179,7 @@ class MetadataNodeGroup( serviceName, discoveryServiceName, proxySettings, + nodeMetadata.pathNormalizationConfig, listenersConfig ) else -> @@ -188,6 +188,7 @@ class MetadataNodeGroup( serviceName, discoveryServiceName, proxySettings, + nodeMetadata.pathNormalizationConfig, listenersConfig ) } 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 50627d0dc..71eaa8e6e 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 @@ -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,6 +494,7 @@ 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) } @@ -480,6 +502,7 @@ fun Value.toDuration(): Duration? { throw NodeMetadataValidationException("Timeout definition has incorrect format: ${ex.message}") } } + else -> null } } diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataValidator.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataValidator.kt index 5607bc5d2..fd7ea7e41 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataValidator.kt +++ b/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()) { 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 0a2c76202..5046aff1a 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 @@ -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 } diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/HttpConnectionManagerFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/HttpConnectionManagerFactory.kt index 4490b50a1..1a092c2b2 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/HttpConnectionManagerFactory.kt +++ b/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, 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 6ef2480e1..6a92d5336 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 @@ -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 diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataValidatorTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataValidatorTest.kt index 59197755e..034e756fe 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataValidatorTest.kt +++ b/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 = mutableSetOf() diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/TestNodeFactory.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/TestNodeFactory.kt index c12ed00d1..5658ee227 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/TestNodeFactory.kt +++ b/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 = 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,