Add upstream service tag response header; reject requests with servic…

…e-tag duplicates (#366) * "add_upstream_service_tags" envoy flag * reject-requests-with-duplicated-auto-service-tag feature flag
allegro · Apr 5, 2023 · bda5220 · bda5220
1 parent 6aa18ff
commit bda5220
Show file tree

Hide file tree

Showing 19 changed files with 425 additions and 102 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,10 +3,12 @@
 Lists all changes with user impact.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
-
-## [Unreleased]
+## [0.19.32]
 
 ### Changed
+- reject request service-tag if it duplicates auto service-tag preference
+- add debug information to an egress response: upstream service-tags.
+  Enabled by default if auto service-tags feature is used
 - decrease log level for no clients in incoming-endpoint
 
 ## [0.19.31]

diff --git a/docs/configuration.md b/docs/configuration.md
@@ -139,17 +139,19 @@ 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
-**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
+
+ 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.reject-requests-with-duplicated-auto-service-tag** | Return 400 for requests with service-tag which duplicates auto service-tag preference                                                                                             | true                     
 
 ## Outlier detection
 Property                                                                                         | Description                                                                                                                                                                               | Default value

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
@@ -1,5 +1,7 @@
 package pl.allegro.tech.servicemesh.envoycontrol.groups
 
+import pl.allegro.tech.servicemesh.envoycontrol.snapshot.AccessLogFiltersProperties
+
 sealed class Group {
     abstract val communicationMode: CommunicationMode
     abstract val serviceName: String
@@ -36,6 +38,7 @@ data class ListenersConfig(
     val enableLuaScript: Boolean = defaultEnableLuaScript,
     val accessLogPath: String = defaultAccessLogPath,
     val addUpstreamExternalAddressHeader: Boolean = defaultAddUpstreamExternalAddressHeader,
+    val addUpstreamServiceTags: AddUpstreamServiceTagsCondition = AddUpstreamServiceTagsCondition.NEVER,
     val accessLogFilterSettings: AccessLogFilterSettings,
     val hasStaticSecretsDefined: Boolean = defaultHasStaticSecretsDefined,
     val useTransparentProxy: Boolean = defaultUseTransparentProxy
@@ -49,7 +52,23 @@ data class ListenersConfig(
         const val defaultAccessLogEnabled = false
         const val defaultEnableLuaScript = false
         const val defaultAddUpstreamExternalAddressHeader = false
+        val defaultAddUpstreamServiceTagsIfSupported =
+            AddUpstreamServiceTagsCondition.WHEN_SERVICE_TAG_PREFERENCE_IS_USED
         const val defaultHasStaticSecretsDefined: Boolean = false
         const val defaultUseTransparentProxy: Boolean = false
+
+        val DEFAULT = ListenersConfig(
+            "",
+            0,
+            "",
+            0,
+            accessLogFilterSettings = AccessLogFilterSettings(null, AccessLogFiltersProperties())
+        )
+    }
+
+    enum class AddUpstreamServiceTagsCondition {
+        NEVER, WHEN_SERVICE_TAG_PREFERENCE_IS_USED, ALWAYS
     }
 }
+
+fun ListenersConfig?.orDefault() = this ?: ListenersConfig.DEFAULT
diff --git a/...core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroup.kt b/...core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroup.kt
@@ -3,10 +3,16 @@ package pl.allegro.tech.servicemesh.envoycontrol.groups
 import com.google.protobuf.Struct
 import com.google.protobuf.Value
 import io.envoyproxy.controlplane.cache.NodeGroup
+import io.envoyproxy.envoy.config.core.v3.BuildVersion
+import io.envoyproxy.envoy.type.v3.SemanticVersion
+import pl.allegro.tech.servicemesh.envoycontrol.groups.ListenersConfig.AddUpstreamServiceTagsCondition
 import pl.allegro.tech.servicemesh.envoycontrol.logger
 import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SnapshotProperties
 import io.envoyproxy.envoy.config.core.v3.Node as NodeV3
 
+@Suppress("MagicNumber")
+val MIN_ENVOY_VERSION_SUPPORTING_UPSTREAM_METADATA = envoyVersion(1, 24)
+
 class MetadataNodeGroup(
     val properties: SnapshotProperties
 ) : NodeGroup<Group> {
@@ -68,7 +74,7 @@ class MetadataNodeGroup(
         return ListenersHostPortConfig(ingressHost, ingressPort, egressHost, egressPort)
     }
 
-    private fun createListenersConfig(id: String, metadata: Struct): ListenersConfig? {
+    private fun createListenersConfig(id: String, metadata: Struct, envoyVersion: BuildVersion): ListenersConfig? {
         val ingressHostValue = metadata.fieldsMap["ingress_host"]
         val ingressPortValue = metadata.fieldsMap["ingress_port"]
         val egressHostValue = metadata.fieldsMap["egress_host"]
@@ -105,6 +111,12 @@ class MetadataNodeGroup(
             ?: ListenersConfig.defaultAccessLogPath
         val addUpstreamExternalAddressHeader = metadata.fieldsMap["add_upstream_external_address_header"]?.boolValue
             ?: ListenersConfig.defaultAddUpstreamExternalAddressHeader
+
+        val addUpstreamServiceTags = mapAddUpstreamServiceTags(
+            addUpstreamServiceTagsFlag = metadata.fieldsMap["add_upstream_service_tags"]?.boolValue,
+            envoyVersion = envoyVersion
+        )
+
         val hasStaticSecretsDefined = metadata.fieldsMap["has_static_secrets_defined"]?.boolValue
             ?: ListenersConfig.defaultHasStaticSecretsDefined
         val useTransparentProxy = metadata.fieldsMap["use_transparent_proxy"]?.boolValue
@@ -122,18 +134,33 @@ class MetadataNodeGroup(
             enableLuaScript,
             accessLogPath,
             addUpstreamExternalAddressHeader,
+            addUpstreamServiceTags,
             accessLogFilterSettings,
             hasStaticSecretsDefined,
             useTransparentProxy
         )
     }
 
+    private fun mapAddUpstreamServiceTags(
+        addUpstreamServiceTagsFlag: Boolean?,
+        envoyVersion: BuildVersion
+    ): AddUpstreamServiceTagsCondition {
+        if (envoyVersion.version < MIN_ENVOY_VERSION_SUPPORTING_UPSTREAM_METADATA) {
+            return AddUpstreamServiceTagsCondition.NEVER
+        }
+        return when (addUpstreamServiceTagsFlag) {
+            true -> AddUpstreamServiceTagsCondition.ALWAYS
+            false -> AddUpstreamServiceTagsCondition.NEVER
+            null -> ListenersConfig.defaultAddUpstreamServiceTagsIfSupported
+        }
+    }
+
     private fun createV3Group(node: NodeV3): Group {
         val nodeMetadata = NodeMetadata(node.metadata, properties)
         val serviceName = serviceName(nodeMetadata)
         val discoveryServiceName = nodeMetadata.discoveryServiceName
         val proxySettings = proxySettings(nodeMetadata)
-        val listenersConfig = createListenersConfig(node.id, node.metadata)
+        val listenersConfig = createListenersConfig(node.id, node.metadata, node.userAgentBuildVersion)
 
         return when {
             hasAllServicesDependencies(nodeMetadata) ->
@@ -182,3 +209,9 @@ data class ListenersHostPortConfig(
     val egressHost: String,
     val egressPort: Int
 )
+
+fun envoyVersion(major: Int, minor: Int): SemanticVersion =
+    SemanticVersion.newBuilder().setMajorNumber(major).setMinorNumber(minor).build()
+
+private val semanticVersionComparator = compareBy<SemanticVersion>({ it.majorNumber }, { it.minorNumber })
+operator fun SemanticVersion.compareTo(other: SemanticVersion): Int = semanticVersionComparator.compare(this, other)
diff --git a/...src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt b/...src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt
@@ -14,6 +14,7 @@ import pl.allegro.tech.servicemesh.envoycontrol.groups.DependencySettings
 import pl.allegro.tech.servicemesh.envoycontrol.groups.Group
 import pl.allegro.tech.servicemesh.envoycontrol.groups.IncomingRateLimitEndpoint
 import pl.allegro.tech.servicemesh.envoycontrol.groups.ServicesGroup
+import pl.allegro.tech.servicemesh.envoycontrol.groups.orDefault
 import pl.allegro.tech.servicemesh.envoycontrol.services.MultiClusterState
 import pl.allegro.tech.servicemesh.envoycontrol.services.ServiceInstance
 import pl.allegro.tech.servicemesh.envoycontrol.services.ServiceInstances
@@ -264,8 +265,10 @@ class EnvoySnapshotFactory(
         } else {
             routes.add(
                 egressRoutesFactory.createEgressRouteConfig(
-                    group.serviceName, egressRouteSpecification,
-                    group.listenersConfig?.addUpstreamExternalAddressHeader ?: false
+                    serviceName = group.serviceName,
+                    routes = egressRouteSpecification,
+                    addUpstreamAddressHeader = group.listenersConfig.orDefault().addUpstreamExternalAddressHeader,
+                    addUpstreamServiceTagsHeader = group.listenersConfig.orDefault().addUpstreamServiceTags
                 )
             )
         }
@@ -306,18 +309,19 @@ class EnvoySnapshotFactory(
         routes.add(
             egressRoutesFactory.createEgressRouteConfig(
                 group.serviceName, emptyList(),
-                group.listenersConfig?.addUpstreamExternalAddressHeader ?: false
+                group.listenersConfig.orDefault().addUpstreamExternalAddressHeader
             )
         )
         // routes for listener on port http = 80
         routes.add(
             egressRoutesFactory.createEgressRouteConfig(
-                group.serviceName, egressRouteSpecification +
+                serviceName = group.serviceName,
+                routes = egressRouteSpecification +
                     egressDomainRouteSpecifications.getOrDefault(
                         DomainRoutesGrouper(DEFAULT_HTTP_PORT, false), emptyList()
                     ),
-                group.listenersConfig?.addUpstreamExternalAddressHeader ?: false,
-                DEFAULT_HTTP_PORT.toString()
+                addUpstreamAddressHeader = group.listenersConfig.orDefault().addUpstreamExternalAddressHeader,
+                routeName = DEFAULT_HTTP_PORT.toString()
             )
         )
 

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
@@ -241,6 +241,7 @@ class ServiceTagsProperties {
     var routingExcludedTags: MutableList<StringMatcher> = mutableListOf()
     var allowedTagsCombinations: MutableList<ServiceTagsCombinationsProperties> = mutableListOf()
     var autoServiceTagEnabled = false
+    var rejectRequestsWithDuplicatedAutoServiceTag = true
 
     fun isAutoServiceTagEffectivelyEnabled() = enabled && autoServiceTagEnabled
 }

diff --git a/.../tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/EnvoyDefaultFilters.kt b/.../tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/EnvoyDefaultFilters.kt
@@ -26,14 +26,15 @@ class EnvoyDefaultFilters(
     private val rateLimitFilterFactory = RateLimitFilterFactory(
         snapshotProperties.rateLimit
     )
-
-    private val defaultServiceTagFilterRules = ServiceTagFilter.serviceTagFilterRules(
-        snapshotProperties.routing.serviceTags.header,
-        snapshotProperties.routing.serviceTags.metadataKey
+    private val serviceTagFilterFactory = ServiceTagFilterFactory(
+        properties = snapshotProperties.routing.serviceTags
     )
-    private val defaultHeaderToMetadataConfig = headerToMetadataConfig(defaultServiceTagFilterRules)
+
+    private val defaultServiceTagHeaderToMetadataFilterRules = serviceTagFilterFactory.headerToMetadataFilterRules()
+    private val defaultHeaderToMetadataConfig = headerToMetadataConfig(defaultServiceTagHeaderToMetadataFilterRules)
     private val headerToMetadataHttpFilter = headerToMetadataHttpFilter(defaultHeaderToMetadataConfig)
     private val defaultHeaderToMetadataFilter = { _: Group, _: GlobalSnapshot -> headerToMetadataHttpFilter }
+    private val defaultServiceTagFilter = { _: Group, _: GlobalSnapshot -> serviceTagFilterFactory.luaEgressFilter() }
     private val envoyRouterHttpFilter = envoyRouterHttpFilter()
 
     /**
@@ -63,7 +64,9 @@ class EnvoyDefaultFilters(
     val defaultAuthorizationHeaderFilter = { _: Group, _: GlobalSnapshot ->
         authorizationHeaderToMetadataFilter()
     }
-    val defaultEgressFilters = listOf(defaultHeaderToMetadataFilter, defaultEnvoyRouterHttpFilter)
+    val defaultEgressFilters = listOf(
+        defaultHeaderToMetadataFilter, defaultServiceTagFilter, defaultEnvoyRouterHttpFilter
+    )
 
     /**
      * Order matters:

diff --git a/...gro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/LuaFilterFactory.kt b/...gro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/LuaFilterFactory.kt
@@ -10,8 +10,8 @@ import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3
 import pl.allegro.tech.servicemesh.envoycontrol.groups.Group
 import pl.allegro.tech.servicemesh.envoycontrol.snapshot.IncomingPermissionsProperties
 import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters.LuaMetadataProperty.ListPropertyLua
-import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters.LuaMetadataProperty.StructPropertyLua
 import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters.LuaMetadataProperty.StringPropertyLua
+import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters.LuaMetadataProperty.StructPropertyLua
 
 class LuaFilterFactory(private val incomingPermissionsProperties: IncomingPermissionsProperties) {
 

diff --git a/...gro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/ServiceTagFilter.kt b/...gro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/ServiceTagFilter.kt