PROM-6026 | x-envoy-upstream-service-tags header for all

allegro · Feb 27, 2025 · 250603b · 250603b
1 parent 12e5ee0
commit 250603b
Show file tree

Hide file tree

Showing 10 changed files with 91 additions and 142 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,12 @@
 Lists all changes with user impact.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
+## [0.22.9]
+### Changed
+- changes for `x-envoy-upstream-service-tags` response header:
+  - add configuration property to enable it for all envoys
+  - remove support for enabling it per envoy 
+
 ## [0.22.8]
 ### Changed
 - added tests for missing and malformed JWT token scenarios

diff --git a/docs/configuration.md b/docs/configuration.md
@@ -153,7 +153,8 @@ Property
  **(...).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                     
+  **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                     
+**envoy-control.envoy.snapshot.routing.service-tags.add-upstream-service-tags-header** | Whether to add a `x-envoy-upstream-service-tags` header to an egress response (example value: `["tag1", "tag2"]`)                                                                 | false                    
 
 ## Outlier detection
 Property                                                                                         | Description                                                                                                                                                                               | Default value

diff --git a/docs/index.md b/docs/index.md
@@ -16,9 +16,9 @@ Data Plane that is platform agnostic.
 * [Local reply modification](features/local_reply_mapper.md)
 
 ## Why another Control Plane?
-Our use case for Service Mesh is running 800 microservices on [Mesos](https://mesos.apache.org/) / [Marathon](https://mesosphere.github.io/marathon/) stack.
-Some of these services are run on Virtual Machines using the [OpenStack](https://www.openstack.org/) platform.
-Most current solutions on the market assume that the platform is [Kubernetes](https://kubernetes.io/).
+In the past, our use case for Service Mesh was running 800 microservices on [Mesos](https://mesos.apache.org/) / Marathon stack.
+Some of these services were run on Virtual Machines using the [OpenStack](https://www.openstack.org/) platform.
+Most solutions on the market at that time assumed that the platform is [Kubernetes](https://kubernetes.io/).
 After evaluating current solutions on the market we decided to build our own Control Plane.
 [See comparision](ec_vs_other_software.md) with other popular alternatives.
 

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
@@ -63,7 +63,6 @@ data class ListenersConfig(
     val enableLuaScript: Boolean = defaultEnableLuaScript,
     val accessLogPath: String = defaultAccessLogPath,
     val addUpstreamExternalAddressHeader: Boolean = defaultAddUpstreamExternalAddressHeader,
-    val addUpstreamServiceTags: AddUpstreamServiceTagsCondition = AddUpstreamServiceTagsCondition.NEVER,
     val addJwtFailureStatus: Boolean = true,
     val accessLogFilterSettings: AccessLogFilterSettings,
     val hasStaticSecretsDefined: Boolean = defaultHasStaticSecretsDefined,
@@ -78,8 +77,6 @@ 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
 
@@ -91,10 +88,6 @@ data class ListenersConfig(
             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
@@ -5,7 +5,6 @@ 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
@@ -125,11 +124,6 @@ class MetadataNodeGroup(
             logger.warn("Node $id has access log enabled without filters configurations.")
         }
 
-        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
@@ -149,28 +143,13 @@ class MetadataNodeGroup(
             enableLuaScript,
             accessLogPath,
             addUpstreamExternalAddressHeader,
-            addUpstreamServiceTags,
             addJwtFailureStatus,
             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)

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
@@ -321,8 +321,7 @@ class EnvoySnapshotFactory(
                 egressRoutesFactory.createEgressRouteConfig(
                     serviceName = group.serviceName,
                     routes = egressRouteSpecification,
-                    addUpstreamAddressHeader = group.listenersConfig.orDefault().addUpstreamExternalAddressHeader,
-                    addUpstreamServiceTagsHeader = group.listenersConfig.orDefault().addUpstreamServiceTags
+                    addUpstreamAddressHeader = group.listenersConfig.orDefault().addUpstreamExternalAddressHeader
                 )
             )
         }

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

diff --git a/...llegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyEgressRoutesFactory.kt b/...llegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyEgressRoutesFactory.kt
@@ -24,7 +24,6 @@ import io.envoyproxy.envoy.extensions.retry.host.omit_canary_hosts.v3.OmitCanary
 import io.envoyproxy.envoy.extensions.retry.host.omit_host_metadata.v3.OmitHostMetadataConfig
 import io.envoyproxy.envoy.extensions.retry.host.previous_hosts.v3.PreviousHostsPredicate
 import io.envoyproxy.envoy.type.matcher.v3.RegexMatcher
-import pl.allegro.tech.servicemesh.envoycontrol.groups.ListenersConfig.AddUpstreamServiceTagsCondition
 import pl.allegro.tech.servicemesh.envoycontrol.groups.RateLimitedRetryBackOff
 import pl.allegro.tech.servicemesh.envoycontrol.groups.RetryBackOff
 import pl.allegro.tech.servicemesh.envoycontrol.groups.RetryHostPredicate
@@ -108,13 +107,12 @@ class EnvoyEgressRoutesFactory(
         serviceName: String,
         routes: Collection<RouteSpecification>,
         addUpstreamAddressHeader: Boolean,
-        addUpstreamServiceTagsHeader: AddUpstreamServiceTagsCondition = AddUpstreamServiceTagsCondition.NEVER,
         routeName: String = "default_routes"
     ): RouteConfiguration {
         val virtualHosts = routes
             .filter { it.routeDomains.isNotEmpty() }
             .map { routeSpecification ->
-                buildEgressVirtualHost(routeSpecification, addUpstreamServiceTagsHeader)
+                buildEgressVirtualHost(routeSpecification)
             }
 
         val routeConfiguration = RouteConfiguration.newBuilder()
@@ -142,17 +140,14 @@ class EnvoyEgressRoutesFactory(
             routeConfiguration.addResponseHeadersToAdd(upstreamAddressHeader)
         }
 
-        if (addUpstreamServiceTagsHeader == AddUpstreamServiceTagsCondition.ALWAYS) {
+        if (properties.routing.serviceTags.addUpstreamServiceTagsHeader) {
             routeConfiguration.addResponseHeadersToAdd(upstreamTagsHeader)
         }
 
         return routeConfiguration.build()
     }
 
-    private fun buildEgressVirtualHost(
-        routeSpecification: RouteSpecification,
-        addUpstreamServiceTagsHeader: AddUpstreamServiceTagsCondition
-    ): VirtualHost {
+    private fun buildEgressVirtualHost(routeSpecification: RouteSpecification): VirtualHost {
         val virtualHost = VirtualHost.newBuilder()
             .setName(routeSpecification.clusterName)
             .addAllDomains(routeSpecification.routeDomains)
@@ -170,7 +165,7 @@ class EnvoyEgressRoutesFactory(
         }
 
         if (properties.routing.serviceTags.isAutoServiceTagEffectivelyEnabled()) {
-            addServiceTagHeaders(routeSpecification, virtualHost, addUpstreamServiceTagsHeader)
+            addServiceTagHeaders(routeSpecification, virtualHost)
         }
 
         return virtualHost.build()
@@ -179,7 +174,6 @@ class EnvoyEgressRoutesFactory(
     private fun addServiceTagHeaders(
         routeSpecification: RouteSpecification,
         virtualHost: VirtualHost.Builder,
-        addUpstreamServiceTagsHeader: AddUpstreamServiceTagsCondition
     ) {
         val routingPolicy = routeSpecification.settings.routingPolicy
         if (routingPolicy.autoServiceTag) {
@@ -194,9 +188,6 @@ class EnvoyEgressRoutesFactory(
                     .setAppendAction(HeaderValueOption.HeaderAppendAction.OVERWRITE_IF_EXISTS_OR_ADD)
                     .setKeepEmptyValue(false)
             )
-            if (addUpstreamServiceTagsHeader == AddUpstreamServiceTagsCondition.WHEN_SERVICE_TAG_PREFERENCE_IS_USED) {
-                virtualHost.addResponseHeadersToAdd(upstreamTagsHeader)
-            }
         }
     }
 

diff --git a/.../src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroupTest.kt b/.../src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroupTest.kt
@@ -3,11 +3,7 @@ package pl.allegro.tech.servicemesh.envoycontrol.groups
 import com.google.protobuf.Struct
 import com.google.protobuf.Value
 import com.google.protobuf.util.Durations
-import com.google.protobuf.util.Values
 import io.envoyproxy.envoy.config.accesslog.v3.ComparisonFilter
-import io.envoyproxy.envoy.config.core.v3.BuildVersion
-import io.envoyproxy.envoy.config.core.v3.Node
-import io.envoyproxy.envoy.type.v3.SemanticVersion
 import io.grpc.Status
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Assertions.assertFalse
@@ -18,7 +14,6 @@ import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.CsvSource
 import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode.ADS
 import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode.XDS
-import pl.allegro.tech.servicemesh.envoycontrol.groups.ListenersConfig.AddUpstreamServiceTagsCondition
 import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SnapshotProperties
 import pl.allegro.tech.servicemesh.envoycontrol.snapshot.serviceDependencies
 import io.envoyproxy.envoy.config.core.v3.Node as NodeV3
@@ -384,39 +379,6 @@ class MetadataNodeGroupTest {
         assertFalse(v2dot21 < v1dot24)
     }
 
-    @Test
-    fun `should set addUpstreamServiceTag condition according to client flag and envoy version`() {
-        // given
-        val nodeGroup = MetadataNodeGroup(createSnapshotProperties())
-
-        fun assertThat(node: Node, has: AddUpstreamServiceTagsCondition) {
-            // when
-            val group = nodeGroup.hash(node)
-
-            // then
-            assertThat(group.listenersConfig.orDefault().addUpstreamServiceTags).isEqualTo(has)
-        }
-
-        val emptyNode = NodeV3.newBuilder().build()
-        val basicNode = NodeV3.newBuilder().setMetadata(createMetadataBuilderWithDefaults()).build()
-        val newEnvoyNode = basicNode.with(envoyVersion(1, 24))
-        val oldEnvoyNode = basicNode.with(envoyVersion(1, 23))
-        val enabledNewEnvoyNode = newEnvoyNode.withMetadata("add_upstream_service_tags", Values.of(true))
-        val enabledOldEnvoyNode = oldEnvoyNode.withMetadata("add_upstream_service_tags", Values.of(true))
-        val disabledNewEnvoyNode = newEnvoyNode.withMetadata("add_upstream_service_tags", Values.of(false))
-        val disabledOldEnvoyNode = oldEnvoyNode.withMetadata("add_upstream_service_tags", Values.of(false))
-
-        // expect
-        assertThat(node = emptyNode, has = AddUpstreamServiceTagsCondition.NEVER)
-        assertThat(node = basicNode, has = AddUpstreamServiceTagsCondition.NEVER)
-        assertThat(node = newEnvoyNode, has = AddUpstreamServiceTagsCondition.WHEN_SERVICE_TAG_PREFERENCE_IS_USED)
-        assertThat(node = oldEnvoyNode, has = AddUpstreamServiceTagsCondition.NEVER)
-        assertThat(node = enabledNewEnvoyNode, has = AddUpstreamServiceTagsCondition.ALWAYS)
-        assertThat(node = enabledOldEnvoyNode, has = AddUpstreamServiceTagsCondition.NEVER)
-        assertThat(node = disabledNewEnvoyNode, has = AddUpstreamServiceTagsCondition.NEVER)
-        assertThat(node = disabledOldEnvoyNode, has = AddUpstreamServiceTagsCondition.NEVER)
-    }
-
     private fun createMetadataBuilderWithDefaults(): Struct.Builder? {
         val metadata = NodeV3.newBuilder().metadataBuilder
         metadata.putFields("ingress_host", Value.newBuilder().setStringValue("127.0.0.1").build())
@@ -437,12 +399,4 @@ class MetadataNodeGroupTest {
         snapshotProperties.incomingPermissions.enabled = incomingPermissions
         return snapshotProperties
     }
-
-    private fun NodeV3.with(version: SemanticVersion) = toBuilder()
-        .setUserAgentBuildVersion(BuildVersion.newBuilder().setVersion(version))
-        .build()
-
-    private fun NodeV3.withMetadata(key: String, value: Value) = toBuilder()
-        .apply { metadataBuilder.putFields(key, value) }
-        .build()
 }