From e8a39719776f537fa6363963961edd44c862b946 Mon Sep 17 00:00:00 2001 From: Kamil Smigielski Date: Fri, 11 Nov 2022 17:01:11 +0100 Subject: [PATCH 1/7] remove xDS support --- CHANGELOG.md | 1 + .../servicemesh/envoycontrol/groups/Groups.kt | 3 - .../envoycontrol/groups/MetadataNodeGroup.kt | 2 - .../envoycontrol/groups/NodeMetadata.kt | 17 --- .../groups/NodeMetadataValidator.kt | 12 -- .../snapshot/EnvoySnapshotFactory.kt | 7 +- .../snapshot/SnapshotProperties.kt | 6 - .../envoycontrol/snapshot/SnapshotUpdater.kt | 30 ++--- .../resource/clusters/EnvoyClustersFactory.kt | 42 +------ .../filters/HttpConnectionManagerFactory.kt | 15 +-- .../envoycontrol/EnvoySnapshotFactoryTest.kt | 5 - .../groups/MetadataNodeGroupTest.kt | 66 ++-------- .../groups/NodeMetadataValidatorTest.kt | 76 ----------- .../envoycontrol/groups/TestNodeFactory.kt | 5 - .../metrics/ThreadPoolMetricTest.kt | 1 + .../snapshot/SnapshotUpdaterTest.kt | 107 ++++------------ .../snapshot/SnapshotsVersionsTest.kt | 4 +- .../listeners/filters/JwtFilterFactoryTest.kt | 9 +- .../listeners/filters/LuaFilterFactoryTest.kt | 3 - .../filters/RateLimitFilterFactoryTest.kt | 3 +- .../rbac/RBACFilterFactoryTestUtils.kt | 2 - .../routes/EnvoyIngressRoutesFactoryTest.kt | 3 - .../snapshot/debug/SnapshotDebugController.kt | 9 +- .../snapshot/debug/SnapshotDebugService.kt | 24 +--- .../EndpointMetadataMergingTests.kt | 4 +- .../envoycontrol/EnvoyControlHttp2Test.kt | 36 ------ .../envoycontrol/EnvoyControlSmokeTest.kt | 30 ----- .../envoycontrol/EnvoyControlTest.kt | 36 ------ .../MetricsDiscoveryServerCallbacksTest.kt | 57 --------- .../envoycontrol/OutgoingPermissionsTest.kt | 32 ----- .../servicemesh/envoycontrol/RateLimitTest.kt | 39 ------ .../envoycontrol/RetryPolicyTest.kt | 4 +- .../envoycontrol/SnapshotDebugTest.kt | 29 ++--- .../config/EnvoyControlTestConfiguration.kt | 13 +- .../config/envoy/EnvoyContainer.kt | 8 +- .../config/envoy/EnvoyExtension.kt | 2 +- .../envoycontrol/EnvoyControlTestApp.kt | 7 +- .../src/main/resources/envoy/config_xds.yaml | 119 ------------------ 38 files changed, 98 insertions(+), 770 deletions(-) delete mode 100644 envoy-control-tests/src/main/resources/envoy/config_xds.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 1af53a675..aad3d2cef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). ## [Unreleased] ### Changed +- Remove xds support ## [0.19.24] 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 ef6eff51b..a0b89e3a2 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 @@ -1,7 +1,6 @@ package pl.allegro.tech.servicemesh.envoycontrol.groups sealed class Group { - abstract val communicationMode: CommunicationMode abstract val serviceName: String abstract val discoveryServiceName: String? abstract val proxySettings: ProxySettings @@ -9,7 +8,6 @@ sealed class Group { } data class ServicesGroup( - override val communicationMode: CommunicationMode, override val serviceName: String = "", override val discoveryServiceName: String? = null, override val proxySettings: ProxySettings = ProxySettings(), @@ -17,7 +15,6 @@ data class ServicesGroup( ) : Group() data class AllServicesGroup( - override val communicationMode: CommunicationMode, override val serviceName: String = "", override val discoveryServiceName: String? = null, override val proxySettings: ProxySettings = ProxySettings(), 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 f25228335..69bb60bcb 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 @@ -138,7 +138,6 @@ class MetadataNodeGroup( return when { hasAllServicesDependencies(nodeMetadata) -> AllServicesGroup( - nodeMetadata.communicationMode, serviceName, discoveryServiceName, proxySettings, @@ -146,7 +145,6 @@ class MetadataNodeGroup( ) else -> ServicesGroup( - nodeMetadata.communicationMode, serviceName, discoveryServiceName, proxySettings, 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 369dd3475..802affbee 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 @@ -28,8 +28,6 @@ class NodeMetadata(metadata: Struct, properties: SnapshotProperties) { .fieldsMap["discovery_service_name"] ?.stringValue - val communicationMode = getCommunicationMode(metadata.fieldsMap["ads"]) - val proxySettings: ProxySettings = ProxySettings(metadata.fieldsMap["proxy_settings"], properties) } @@ -63,17 +61,6 @@ data class ProxySettings( ) } -private fun getCommunicationMode(proto: Value?): CommunicationMode { - val ads = proto - ?.boolValue - ?: false - - return when (ads) { - true -> CommunicationMode.ADS - else -> CommunicationMode.XDS - } -} - fun Value?.toComparisonFilter(default: String? = null): ComparisonFilterSettings? { return (this?.stringValue ?: default)?.let { AccessLogFilterParser.parseComparisonFilter(it.uppercase()) @@ -673,10 +660,6 @@ enum class PathMatchingType { PATH, PATH_PREFIX, PATH_REGEX } -enum class CommunicationMode { - ADS, XDS -} - data class OAuth( val provider: String = "", val verification: Verification = Verification.OFFLINE, 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 feb5e20c2..20a700b2b 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,8 +1,6 @@ package pl.allegro.tech.servicemesh.envoycontrol.groups import io.envoyproxy.controlplane.server.DiscoveryServerCallbacks -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 import pl.allegro.tech.servicemesh.envoycontrol.protocol.HttpMethod import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SnapshotProperties @@ -84,7 +82,6 @@ class NodeMetadataValidator( validateDependencies(metadata) validateIncomingEndpoints(metadata) validateIncomingRateLimitEndpoints(metadata) - validateConfigurationMode(metadata) } private fun validateServiceName(metadata: NodeMetadata) { @@ -170,13 +167,4 @@ class NodeMetadataValidator( private fun isAllowedToHaveAllServiceDependencies(metadata: NodeMetadata) = properties .outgoingPermissions.servicesAllowedToUseWildcard.contains(metadata.serviceName) - - private fun validateConfigurationMode(metadata: NodeMetadata) { - if (metadata.communicationMode == ADS && !properties.enabledCommunicationModes.ads) { - throw ConfigurationModeNotSupportedException(metadata.serviceName, "ADS") - } - if (metadata.communicationMode == XDS && !properties.enabledCommunicationModes.xds) { - throw ConfigurationModeNotSupportedException(metadata.serviceName, "XDS") - } - } } diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt index 1357b6eac..ecdcff284 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt @@ -9,7 +9,6 @@ import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.Secret import io.micrometer.core.instrument.MeterRegistry import io.micrometer.core.instrument.Timer import pl.allegro.tech.servicemesh.envoycontrol.groups.AllServicesGroup -import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode import pl.allegro.tech.servicemesh.envoycontrol.groups.DependencySettings import pl.allegro.tech.servicemesh.envoycontrol.groups.Group import pl.allegro.tech.servicemesh.envoycontrol.groups.IncomingRateLimitEndpoint @@ -41,14 +40,12 @@ class EnvoySnapshotFactory( fun newSnapshot( servicesStates: MultiClusterState, - clusterConfigurations: Map, - communicationMode: CommunicationMode + clusterConfigurations: Map ): GlobalSnapshot { val sample = Timer.start(meterRegistry) val clusters = clustersFactory.getClustersForServices( - clusterConfigurations.values, - communicationMode + clusterConfigurations.values ) val securedClusters = clustersFactory.getSecuredClusters(clusters) 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 a5221c042..467a84625 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 @@ -26,7 +26,6 @@ class SnapshotProperties { var staticClusterConnectionTimeout: Duration = Duration.ofSeconds(2) var trustedCaFile = "/etc/ssl/certs/ca-certificates.crt" var dynamicListeners = ListenersFactoryProperties() - var enabledCommunicationModes = EnabledCommunicationModes() var shouldSendMissingEndpoints = false var metrics: MetricsProperties = MetricsProperties() var dynamicForwardProxy = DynamicForwardProxyProperties() @@ -296,11 +295,6 @@ class Http2Properties { var tagName = "envoy" } -class EnabledCommunicationModes { - var ads = true - var xds = true -} - class HostHeaderRewritingProperties { var enabled = false var customHostHeader = "x-envoy-original-host" diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotUpdater.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotUpdater.kt index 92ef0be9b..f41fc189b 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotUpdater.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotUpdater.kt @@ -4,8 +4,6 @@ import io.envoyproxy.controlplane.cache.SnapshotCache import io.envoyproxy.controlplane.cache.v3.Snapshot import io.micrometer.core.instrument.MeterRegistry import io.micrometer.core.instrument.Timer -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.Group import pl.allegro.tech.servicemesh.envoycontrol.logger import pl.allegro.tech.servicemesh.envoycontrol.services.MultiClusterState @@ -59,8 +57,7 @@ class SnapshotUpdater( UpdateResult( action = newUpdate.action, groups = newUpdate.groups, - adsSnapshot = newUpdate.adsSnapshot ?: previous.adsSnapshot, - xdsSnapshot = newUpdate.xdsSnapshot ?: previous.xdsSnapshot + snapshot = newUpdate.snapshot ?: previous.snapshot ) } // concat map guarantees sequential processing (unlike flatMap) @@ -74,7 +71,7 @@ class SnapshotUpdater( // step 4: update the snapshot for either all groups (if services changed) // or specific groups (groups changed). // TODO(dj): on what occasion can this be false? - if (result.adsSnapshot != null || result.xdsSnapshot != null) { + if (result.snapshot != null) { // Stateful operation! This is the meat of this processing. updateSnapshotForGroups(groups, result) } else { @@ -111,19 +108,9 @@ class SnapshotUpdater( .name("snapshot-updater-services-published").metrics() .createClusterConfigurations() .map { (states, clusters) -> - var lastXdsSnapshot: GlobalSnapshot? = null - var lastAdsSnapshot: GlobalSnapshot? = null - - if (properties.enabledCommunicationModes.xds) { - lastXdsSnapshot = snapshotFactory.newSnapshot(states, clusters, XDS) - } - if (properties.enabledCommunicationModes.ads) { - lastAdsSnapshot = snapshotFactory.newSnapshot(states, clusters, ADS) - } val updateResult = UpdateResult( action = Action.ALL_SERVICES_GROUP_ADDED, - adsSnapshot = lastAdsSnapshot, - xdsSnapshot = lastXdsSnapshot + snapshot = snapshotFactory.newSnapshot(states, clusters), ) globalSnapshot = updateResult updateResult @@ -169,13 +156,11 @@ class SnapshotUpdater( versions.retainGroups(cache.groups()) val results = Flux.fromIterable(groups) .doOnNextScheduledOn(groupSnapshotScheduler) { group -> - if (result.adsSnapshot != null && group.communicationMode == ADS) { - updateSnapshotForGroup(group, result.adsSnapshot) - } else if (result.xdsSnapshot != null && group.communicationMode == XDS) { - updateSnapshotForGroup(group, result.xdsSnapshot) + if (result.snapshot != null) { + updateSnapshotForGroup(group, result.snapshot) } else { meterRegistry.counter("snapshot-updater.communication-mode.errors").increment() - logger.error("Requested snapshot for ${group.communicationMode.name} mode, but it is not here. " + + logger.error("Requested snapshot, but it is not here. " + "Handling Envoy with not supported communication mode should have been rejected before." + " Please report this to EC developers.") } @@ -212,6 +197,5 @@ enum class Action { data class UpdateResult( val action: Action, val groups: List = listOf(), - val adsSnapshot: GlobalSnapshot? = null, - val xdsSnapshot: GlobalSnapshot? = null + val snapshot: GlobalSnapshot? = null ) diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt index 1a25e5a7f..ad3829270 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt @@ -10,11 +10,9 @@ import io.envoyproxy.envoy.config.cluster.v3.Cluster import io.envoyproxy.envoy.config.cluster.v3.OutlierDetection import io.envoyproxy.envoy.config.core.v3.Address import io.envoyproxy.envoy.config.core.v3.AggregatedConfigSource -import io.envoyproxy.envoy.config.core.v3.ApiConfigSource import io.envoyproxy.envoy.config.core.v3.ApiVersion import io.envoyproxy.envoy.config.core.v3.ConfigSource import io.envoyproxy.envoy.config.core.v3.DataSource -import io.envoyproxy.envoy.config.core.v3.GrpcService import io.envoyproxy.envoy.config.core.v3.Http2ProtocolOptions import io.envoyproxy.envoy.config.core.v3.HttpProtocolOptions import io.envoyproxy.envoy.config.core.v3.RoutingPriority @@ -36,9 +34,6 @@ import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.SdsSecretConfig import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.TlsParameters import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext import pl.allegro.tech.servicemesh.envoycontrol.groups.AllServicesGroup -import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode -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.DependencySettings import pl.allegro.tech.servicemesh.envoycontrol.groups.DomainDependency import pl.allegro.tech.servicemesh.envoycontrol.groups.Group @@ -77,10 +72,9 @@ class EnvoyClustersFactory( } fun getClustersForServices( - services: Collection, - communicationMode: CommunicationMode + services: Collection ): List { - return services.map { edsCluster(it, communicationMode) } + return services.map { edsCluster(it) } } fun getSecuredClusters(insecureClusters: List): List { @@ -351,8 +345,7 @@ class EnvoyClustersFactory( } private fun edsCluster( - clusterConfiguration: ClusterConfiguration, - communicationMode: CommunicationMode + clusterConfiguration: ClusterConfiguration ): Cluster { val clusterBuilder = Cluster.newBuilder() @@ -366,32 +359,9 @@ class EnvoyClustersFactory( .setConnectTimeout(Durations.fromMillis(properties.edsConnectionTimeout.toMillis())) .setEdsClusterConfig( Cluster.EdsClusterConfig.newBuilder().setEdsConfig( - when (communicationMode) { - // here we do not have group information - ADS -> ConfigSource.newBuilder() - .setResourceApiVersion(ApiVersion.V3) - .setAds(AggregatedConfigSource.newBuilder()) - XDS -> - ConfigSource.newBuilder() - .setResourceApiVersion(ApiVersion.V3) - .setApiConfigSource( - ApiConfigSource.newBuilder() - .setApiType( - if (properties.deltaXdsEnabled) { - ApiConfigSource.ApiType.DELTA_GRPC - } else { - ApiConfigSource.ApiType.GRPC - } - ) - .setTransportApiVersion(ApiVersion.V3) - .addGrpcServices( - 0, GrpcService.newBuilder().setEnvoyGrpc( - GrpcService.EnvoyGrpc.newBuilder() - .setClusterName(properties.xdsClusterName) - ) - ) - ) - } + ConfigSource.newBuilder() + .setResourceApiVersion(ApiVersion.V3) + .setAds(AggregatedConfigSource.newBuilder()) ).setServiceName(clusterConfiguration.serviceName) ) .setLbPolicy(properties.loadBalancing.policy) 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 336ecc73d..e6e9034a2 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 @@ -3,7 +3,6 @@ package pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.fil import com.google.protobuf.BoolValue import com.google.protobuf.Duration import com.google.protobuf.util.Durations -import io.envoyproxy.envoy.config.core.v3.AggregatedConfigSource import io.envoyproxy.envoy.config.core.v3.ApiConfigSource import io.envoyproxy.envoy.config.core.v3.ApiVersion import io.envoyproxy.envoy.config.core.v3.ConfigSource @@ -12,7 +11,6 @@ import io.envoyproxy.envoy.config.core.v3.Http1ProtocolOptions import io.envoyproxy.envoy.config.core.v3.HttpProtocolOptions import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.Rds -import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode import pl.allegro.tech.servicemesh.envoycontrol.groups.Group import pl.allegro.tech.servicemesh.envoycontrol.snapshot.GlobalSnapshot import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SnapshotProperties @@ -53,7 +51,7 @@ class HttpConnectionManagerFactory( val connectionManagerBuilder = HttpConnectionManager.newBuilder() .setStatPrefix(statPrefix) - .setRds(setupRds(group.communicationMode, initialFetchTimeout, routeConfigName)) + .setRds(setupRds(initialFetchTimeout, routeConfigName)) .setGenerateRequestId(BoolValue.newBuilder().setValue(listenersConfig.generateRequestId).build()) .setPreserveExternalRequestId(listenersConfig.preserveExternalRequestId) @@ -106,18 +104,13 @@ class HttpConnectionManagerFactory( } private fun setupRds( - communicationMode: CommunicationMode, rdsInitialFetchTimeout: Duration, routeConfigName: String ): Rds { val configSource = ConfigSource.newBuilder() .setInitialFetchTimeout(rdsInitialFetchTimeout) - configSource.resourceApiVersion = ApiVersion.V3 - - when (communicationMode) { - CommunicationMode.ADS -> configSource.ads = AggregatedConfigSource.getDefaultInstance() - CommunicationMode.XDS -> configSource.apiConfigSource = defaultApiConfigSourceV3 - } + .setResourceApiVersion(ApiVersion.V3) + .setApiConfigSource(defaultApiConfigSourceV3) return Rds.newBuilder() .setRouteConfigName(routeConfigName) @@ -148,7 +141,7 @@ class HttpConnectionManagerFactory( GrpcService.newBuilder() .setEnvoyGrpc( GrpcService.EnvoyGrpc.newBuilder() - .setClusterName("envoy-control-xds") + .setClusterName(snapshotProperties.xdsClusterName) ) ).build() } diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoySnapshotFactoryTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoySnapshotFactoryTest.kt index 41fe92da5..a65f7310d 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoySnapshotFactoryTest.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoySnapshotFactoryTest.kt @@ -16,7 +16,6 @@ import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test import pl.allegro.tech.servicemesh.envoycontrol.groups.AccessLogFilterSettings import pl.allegro.tech.servicemesh.envoycontrol.groups.AllServicesGroup -import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode import pl.allegro.tech.servicemesh.envoycontrol.groups.DependencySettings import pl.allegro.tech.servicemesh.envoycontrol.groups.Group import pl.allegro.tech.servicemesh.envoycontrol.groups.IncomingRateLimitEndpoint @@ -279,7 +278,6 @@ class EnvoySnapshotFactoryTest { ), "v1").resources()) private fun createServicesGroup( - mode: CommunicationMode = CommunicationMode.XDS, serviceName: String = DEFAULT_SERVICE_NAME, discoveryServiceName: String = DEFAULT_DISCOVERY_SERVICE_NAME, dependencies: Array> = emptyArray(), @@ -292,7 +290,6 @@ class EnvoySnapshotFactoryTest { false -> null } return ServicesGroup( - mode, serviceName, discoveryServiceName, ProxySettings().with( @@ -304,7 +301,6 @@ class EnvoySnapshotFactoryTest { } private fun createAllServicesGroup( - mode: CommunicationMode = CommunicationMode.XDS, serviceName: String = DEFAULT_SERVICE_NAME, discoveryServiceName: String = DEFAULT_DISCOVERY_SERVICE_NAME, dependencies: Array> = emptyArray(), @@ -317,7 +313,6 @@ class EnvoySnapshotFactoryTest { false -> null } return AllServicesGroup( - mode, serviceName, discoveryServiceName, ProxySettings().with( diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroupTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroupTest.kt index 1b3fc0203..d03e4fd5b 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroupTest.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroupTest.kt @@ -10,8 +10,6 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows 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.snapshot.SnapshotProperties import pl.allegro.tech.servicemesh.envoycontrol.snapshot.serviceDependencies import io.envoyproxy.envoy.config.core.v3.Node as NodeV3 @@ -22,7 +20,7 @@ class MetadataNodeGroupTest { fun `should assign to group with all dependencies`() { // given val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true)) - val node = nodeV3(serviceDependencies = setOf("*", "a", "b", "c"), ads = false) + val node = nodeV3(serviceDependencies = setOf("*", "a", "b", "c")) // when val group = nodeGroup.hash(node) @@ -33,7 +31,6 @@ class MetadataNodeGroupTest { // we have to preserve all services even if wildcard is present, // because service may define different settings for different dependencies (for example endpoints, which // will be implemented in https://github.com/allegro/envoy-control/issues/6 - communicationMode = XDS, proxySettings = ProxySettings().with( serviceDependencies = serviceDependencies("a", "b", "c"), allServicesDependencies = true @@ -53,8 +50,7 @@ class MetadataNodeGroupTest { // then assertThat(group).isEqualTo( ServicesGroup( - proxySettings = ProxySettings().with(serviceDependencies = setOf()), - communicationMode = XDS + proxySettings = ProxySettings().with(serviceDependencies = setOf()) ) ) } @@ -63,7 +59,7 @@ class MetadataNodeGroupTest { fun `should assign to group with listed dependencies`() { // given val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true)) - val node = nodeV3(serviceDependencies = setOf("a", "b", "c"), ads = false) + val node = nodeV3(serviceDependencies = setOf("a", "b", "c")) // when val group = nodeGroup.hash(node) @@ -71,44 +67,7 @@ class MetadataNodeGroupTest { // then assertThat(group).isEqualTo( ServicesGroup( - proxySettings = ProxySettings().with(serviceDependencies = serviceDependencies("a", "b", "c")), - communicationMode = XDS - ) - ) - } - - @Test - fun `should assign to group with all dependencies on ads`() { - // given - val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true)) - val node = nodeV3(serviceDependencies = setOf("*"), ads = true) - - // when - val group = nodeGroup.hash(node) - - // then - assertThat(group).isEqualTo( - AllServicesGroup( - communicationMode = ADS, - proxySettings = ProxySettings().with(allServicesDependencies = true) - ) - ) - } - - @Test - fun `should assign to group with listed dependencies on ads`() { - // given - val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true)) - val node = nodeV3(serviceDependencies = setOf("a", "b", "c"), ads = true) - - // when - val group = nodeGroup.hash(node) - - // then - assertThat(group).isEqualTo( - ServicesGroup( - proxySettings = ProxySettings().with(serviceDependencies = serviceDependencies("a", "b", "c")), - communicationMode = ADS + proxySettings = ProxySettings().with(serviceDependencies = serviceDependencies("a", "b", "c")) ) ) } @@ -117,7 +76,7 @@ class MetadataNodeGroupTest { fun `should assign to group with all dependencies when outgoing-permissions is not enabled`() { // given val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = false)) - val node = nodeV3(serviceDependencies = setOf("a", "b", "c"), ads = true) + val node = nodeV3(serviceDependencies = setOf("a", "b", "c")) // when val group = nodeGroup.hash(node) @@ -127,7 +86,6 @@ class MetadataNodeGroupTest { AllServicesGroup( // we have to preserve all services even if outgoingPermissions is disabled, // because service may define different settings for different dependencies (for example retry config) - communicationMode = ADS, proxySettings = ProxySettings().with(serviceDependencies = serviceDependencies("a", "b", "c")) ) ) @@ -139,7 +97,7 @@ class MetadataNodeGroupTest { val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true)) val node = nodeV3( serviceDependencies = setOf("a", "b", "c"), - ads = false, serviceName = "app1", + serviceName = "app1", incomingSettings = true ) @@ -150,7 +108,6 @@ class MetadataNodeGroupTest { assertThat(group).isEqualTo( ServicesGroup( proxySettings = ProxySettings().with(serviceDependencies = serviceDependencies("a", "b", "c")), - communicationMode = XDS, serviceName = "app1" ) ) @@ -160,7 +117,7 @@ class MetadataNodeGroupTest { fun `should not include service settings when incoming permissions are disabled for all dependencies`() { // given val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true)) - val node = nodeV3(serviceDependencies = setOf("*"), ads = false, serviceName = "app1", incomingSettings = true) + val node = nodeV3(serviceDependencies = setOf("*"), serviceName = "app1", incomingSettings = true) // when val group = nodeGroup.hash(node) @@ -177,7 +134,6 @@ class MetadataNodeGroupTest { ) val node = nodeV3( serviceDependencies = setOf("a", "b"), - ads = true, serviceName = "app1", incomingSettings = true ) @@ -188,7 +144,6 @@ class MetadataNodeGroupTest { // then assertThat(group).isEqualTo( ServicesGroup( - communicationMode = ADS, serviceName = "app1", proxySettings = addedProxySettings.with(serviceDependencies = serviceDependencies("a", "b")) ) @@ -201,7 +156,7 @@ class MetadataNodeGroupTest { val nodeGroup = MetadataNodeGroup( createSnapshotProperties(outgoingPermissions = true, incomingPermissions = true) ) - val node = nodeV3(serviceDependencies = setOf("*"), ads = false, serviceName = "app1", incomingSettings = true) + val node = nodeV3(serviceDependencies = setOf("*"), serviceName = "app1", incomingSettings = true) // when val group = nodeGroup.hash(node) @@ -209,7 +164,6 @@ class MetadataNodeGroupTest { // then assertThat(group).isEqualTo( AllServicesGroup( - communicationMode = XDS, serviceName = "app1", proxySettings = addedProxySettings.with(allServicesDependencies = true) ) @@ -221,7 +175,7 @@ class MetadataNodeGroupTest { // when val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true)) val node = nodeV3( - serviceDependencies = setOf("*"), ads = true, incomingSettings = true, + serviceDependencies = setOf("*"), incomingSettings = true, responseTimeout = "777s", idleTimeout = "13.33s" ) @@ -238,7 +192,7 @@ class MetadataNodeGroupTest { // when val nodeGroup = MetadataNodeGroup(createSnapshotProperties(incomingPermissions = true)) val node = nodeV3( - serviceDependencies = setOf("*"), ads = true, incomingSettings = true, + serviceDependencies = setOf("*"), incomingSettings = true, healthCheckPath = "/status/ping", healthCheckClusterName = "local_service_health_check" ) 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 76451d45c..7862ce976 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 @@ -6,9 +6,6 @@ import org.assertj.core.api.Assertions.assertThatExceptionOfType import org.assertj.core.api.Assertions.catchThrowable import org.junit.jupiter.api.Assertions.assertDoesNotThrow import org.junit.jupiter.api.Test -import org.junit.jupiter.params.ParameterizedTest -import org.junit.jupiter.params.provider.CsvSource -import pl.allegro.tech.servicemesh.envoycontrol.snapshot.EnabledCommunicationModes import pl.allegro.tech.servicemesh.envoycontrol.snapshot.IncomingPermissionsProperties import pl.allegro.tech.servicemesh.envoycontrol.snapshot.OutgoingPermissionsProperties import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SnapshotProperties @@ -140,72 +137,6 @@ class NodeMetadataValidatorTest { assertDoesNotThrow { permissionsDisabledValidator.onV3StreamRequest(123, request = request) } } - @ParameterizedTest - @CsvSource( - "false, true, true, ADS", - "true, false, false, XDS", - "false, false, false, XDS", - "false, false, true, ADS" - ) - fun `should fail if service wants to use mode which server doesn't support`( - adsSupported: Boolean, - xdsSupported: Boolean, - ads: Boolean, - modeNotSupportedName: String - ) { - // given - val configurationModeValidator = NodeMetadataValidator(SnapshotProperties().apply { - enabledCommunicationModes = createCommunicationMode(ads = adsSupported, xds = xdsSupported) - }) - - val node = nodeV3( - ads = ads, - serviceDependencies = setOf("a", "b", "c"), - serviceName = "regular-1" - ) - val request = DiscoveryRequestV3.newBuilder().setNode(node).build() - - // when - val exception = - catchThrowable { configurationModeValidator.onV3StreamRequest(streamId = 123, request = request) } - - // expects - assertThat(exception).isInstanceOf(ConfigurationModeNotSupportedException::class.java) - val validationException = exception as ConfigurationModeNotSupportedException - assertThat(validationException.status.description) - .isEqualTo("Blocked service regular-1 from receiving updates. $modeNotSupportedName is not supported by server.") - assertThat(validationException.status.code) - .isEqualTo(Status.Code.INVALID_ARGUMENT) - } - - @ParameterizedTest - @CsvSource( - "true, true, true", - "true, false, true", - "false, true, false", - "true, true, false" - ) - fun `should do nothing if service wants to use mode supported by the server`( - adsSupported: Boolean, - xdsSupported: Boolean, - ads: Boolean - ) { - // given - val configurationModeValidator = NodeMetadataValidator(SnapshotProperties().apply { - enabledCommunicationModes = createCommunicationMode(ads = adsSupported, xds = xdsSupported) - }) - - val node = nodeV3( - ads = ads, - serviceDependencies = setOf("a", "b", "c"), - serviceName = "regular-1" - ) - val request = DiscoveryRequestV3.newBuilder().setNode(node).build() - - // then - assertDoesNotThrow { configurationModeValidator.onV3StreamRequest(123, request = request) } - } - @Test fun `should fail when service name is empty`() { // given @@ -283,11 +214,4 @@ class NodeMetadataValidatorTest { outgoingPermissions.servicesAllowedToUseWildcard = servicesAllowedToUseWildcard return outgoingPermissions } - - private fun createCommunicationMode(ads: Boolean = true, xds: Boolean = true): EnabledCommunicationModes { - val enabledCommunicationModes = EnabledCommunicationModes() - enabledCommunicationModes.ads = ads - enabledCommunicationModes.xds = xds - return enabledCommunicationModes - } } 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 98b1c28ca..b56a72d1d 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 @@ -11,7 +11,6 @@ import io.envoyproxy.envoy.config.core.v3.Node as NodeV3 fun nodeV3( serviceDependencies: Set = emptySet(), - ads: Boolean? = null, serviceName: String? = null, discoveryServiceName: String? = null, incomingSettings: Boolean = false, @@ -33,10 +32,6 @@ fun nodeV3( meta.putFields("discovery_service_name", string(discoveryServiceName)) } - ads?.let { - meta.putFields("ads", Value.newBuilder().setBoolValue(ads).build()) - } - if (incomingSettings || serviceDependencies.isNotEmpty()) { meta.putFields( "proxy_settings", diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/metrics/ThreadPoolMetricTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/metrics/ThreadPoolMetricTest.kt index 2a91824e1..5fdeec6fe 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/metrics/ThreadPoolMetricTest.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/metrics/ThreadPoolMetricTest.kt @@ -15,6 +15,7 @@ class ThreadPoolMetricTest { // given val meterRegistry = SimpleMeterRegistry() val envoyControlProperties = EnvoyControlProperties().also { + it.server.port = 50050 it.server.executorGroup.type = ExecutorType.PARALLEL it.server.groupSnapshotUpdateScheduler.type = ExecutorType.PARALLEL } diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotUpdaterTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotUpdaterTest.kt index 85f5105a2..6298e9290 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotUpdaterTest.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotUpdaterTest.kt @@ -25,9 +25,6 @@ import org.junit.jupiter.params.provider.MethodSource import org.mockito.Mockito import pl.allegro.tech.servicemesh.envoycontrol.groups.AccessLogFilterSettings import pl.allegro.tech.servicemesh.envoycontrol.groups.AllServicesGroup -import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode -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.DependencySettings import pl.allegro.tech.servicemesh.envoycontrol.groups.DomainDependency import pl.allegro.tech.servicemesh.envoycontrol.groups.Group @@ -76,11 +73,6 @@ import pl.allegro.tech.servicemesh.envoycontrol.groups.RetryPolicy as EnvoyContr class SnapshotUpdaterTest { companion object { - @JvmStatic - fun configurationModeNotSupported() = listOf( - Arguments.of(false, false, ADS, "ADS not supported by server"), - Arguments.of(false, false, XDS, "XDS not supported by server") - ) @JvmStatic fun tapConfiguration() = listOf( @@ -92,7 +84,6 @@ class SnapshotUpdaterTest { } val groupWithProxy = AllServicesGroup( - communicationMode = ADS, serviceName = "service" ) val groupWithServiceName = groupOf( @@ -124,7 +115,7 @@ class SnapshotUpdaterTest { val cache = MockCache() val allServicesGroup = AllServicesGroup( - communicationMode = XDS, proxySettings = ProxySettings( + proxySettings = ProxySettings( outgoing = Outgoing( serviceDependencies = listOf( ServiceDependency( @@ -173,7 +164,7 @@ class SnapshotUpdaterTest { val snapshotAuditor = Mockito.mock(SnapshotChangeAuditor::class.java) Mockito.`when`(snapshotAuditor.audit(any(UpdateResult::class.java), any(UpdateResult::class.java))) .thenReturn(Mono.empty()) - val groups = listOf(groupWithProxy, groupWithServiceName, AllServicesGroup(communicationMode = ADS)) + val groups = listOf(groupWithProxy, groupWithServiceName, AllServicesGroup()) groups.forEach { cache.setSnapshot(it, uninitializedSnapshot) } @@ -217,7 +208,7 @@ class SnapshotUpdaterTest { methods = setOf("POST") ) val allServicesGroup = AllServicesGroup( - communicationMode = XDS, proxySettings = ProxySettings( + proxySettings = ProxySettings( outgoing = Outgoing( serviceDependencies = listOf( ServiceDependency( @@ -271,7 +262,7 @@ class SnapshotUpdaterTest { // groups are generated foreach element in SnapshotCache.groups(), so we need to initialize them val groups = listOf( - AllServicesGroup(communicationMode = XDS), groupWithProxy, groupWithServiceName, + AllServicesGroup(), groupWithProxy, groupWithServiceName, groupOf(services = serviceDependencies("existingService1")), groupOf(services = serviceDependencies("existingService2")) ) @@ -300,7 +291,7 @@ class SnapshotUpdaterTest { updater.startWithServices("existingService1", "existingService2") // then - hasSnapshot(cache, AllServicesGroup(communicationMode = XDS)) + hasSnapshot(cache, AllServicesGroup()) .hasOnlyClustersFor("existingService1", "existingService2") hasSnapshot(cache, groupWithProxy) @@ -336,7 +327,7 @@ class SnapshotUpdaterTest { // groups are generated foreach element in SnapshotCache.groups(), so we need to initialize them val groups = listOf( - AllServicesGroup(communicationMode = XDS), groupWithProxy, groupWithServiceName, + AllServicesGroup(), groupWithProxy, groupWithServiceName, groupOf(services = serviceDependencies("existingService1")), groupOf(services = serviceDependencies("existingService2")) ) @@ -365,7 +356,7 @@ class SnapshotUpdaterTest { updater.startWithServices("existingService1", "existingService2") // then - hasSnapshot(cache, AllServicesGroup(communicationMode = XDS)) + hasSnapshot(cache, AllServicesGroup()) .tapConfigurationVerifier() hasSnapshot(cache, groupWithProxy) @@ -388,34 +379,6 @@ class SnapshotUpdaterTest { ).tapConfigurationVerifier() } - @ParameterizedTest - @MethodSource("configurationModeNotSupported") - fun `should not generate group snapshots for modes not supported by the server`( - adsSupported: Boolean, - xdsSupported: Boolean, - mode: CommunicationMode - ) { - val allServiceGroup = AllServicesGroup(communicationMode = mode) - - val cache = MockCache() - cache.setSnapshot(allServiceGroup, uninitializedSnapshot) - - val updater = snapshotUpdater( - cache = cache, - properties = SnapshotProperties().apply { - enabledCommunicationModes.ads = adsSupported; enabledCommunicationModes.xds = xdsSupported - } - ) - - // when - updater.start( - Flux.just(MultiClusterState.empty()) - ).blockFirst() - - // should not generate snapshot - assertThat(cache.getSnapshot(allServiceGroup)).isNull() - } - @Test fun `should generate snapshot with empty clusters and endpoints version and one route`() { // given @@ -451,7 +414,6 @@ class SnapshotUpdaterTest { fun `should not crash on bad snapshot generation`() { // given val servicesGroup = AllServicesGroup( - communicationMode = ADS, serviceName = "example-service" ) val cache = FailingMockCache() @@ -506,17 +468,15 @@ class SnapshotUpdaterTest { // then assertThat(results.size).isEqualTo(2) - results[0].adsSnapshot!!.hasHttp2Cluster("service") - results[0].xdsSnapshot!!.hasHttp2Cluster("service") - results[1].adsSnapshot!!.hasHttp2Cluster("service") - results[1].xdsSnapshot!!.hasHttp2Cluster("service") + results[0].snapshot!!.hasHttp2Cluster("service") + results[1].snapshot!!.hasHttp2Cluster("service") } @Test fun `should not change CDS version when service order in remote cluster is different`() { // given val cache = MockCache() - val allServicesGroup = AllServicesGroup(communicationMode = ADS) + val allServicesGroup = AllServicesGroup() val groups = listOf(allServicesGroup) val updater = snapshotUpdater( cache = cache, @@ -566,7 +526,7 @@ class SnapshotUpdaterTest { fun `should not change EDS when remote doesn't have state of service`() { // given val cache = MockCache() - val allServicesGroup = AllServicesGroup(communicationMode = ADS) + val allServicesGroup = AllServicesGroup() val groups = listOf(allServicesGroup) val updater = snapshotUpdater( cache = cache, @@ -639,22 +599,14 @@ class SnapshotUpdaterTest { .collectList().block()!! // then - resultsRemoteWithBothServices[0].adsSnapshot!! - .hasTheSameClusters(resultsRemoteWithJustOneService[0].adsSnapshot!!) - .hasTheSameEndpoints(resultsRemoteWithJustOneService[0].adsSnapshot!!) - .hasTheSameSecuredClusters(resultsRemoteWithJustOneService[0].adsSnapshot!!) - resultsRemoteWithBothServices[0].xdsSnapshot!! - .hasTheSameClusters(resultsRemoteWithJustOneService[0].xdsSnapshot!!) - .hasTheSameEndpoints(resultsRemoteWithJustOneService[0].xdsSnapshot!!) - .hasTheSameSecuredClusters(resultsRemoteWithJustOneService[0].xdsSnapshot!!) - resultsRemoteWithBothServices[1].adsSnapshot!! - .hasTheSameClusters(resultsRemoteWithJustOneService[1].adsSnapshot!!) - .hasTheSameEndpoints(resultsRemoteWithJustOneService[1].adsSnapshot!!) - .hasTheSameSecuredClusters(resultsRemoteWithJustOneService[1].adsSnapshot!!) - resultsRemoteWithBothServices[1].xdsSnapshot!! - .hasTheSameClusters(resultsRemoteWithJustOneService[1].xdsSnapshot!!) - .hasTheSameEndpoints(resultsRemoteWithJustOneService[1].xdsSnapshot!!) - .hasTheSameSecuredClusters(resultsRemoteWithJustOneService[1].xdsSnapshot!!) + resultsRemoteWithBothServices[0].snapshot!! + .hasTheSameClusters(resultsRemoteWithJustOneService[0].snapshot!!) + .hasTheSameEndpoints(resultsRemoteWithJustOneService[0].snapshot!!) + .hasTheSameSecuredClusters(resultsRemoteWithJustOneService[0].snapshot!!) + resultsRemoteWithBothServices[1].snapshot!! + .hasTheSameClusters(resultsRemoteWithJustOneService[1].snapshot!!) + .hasTheSameEndpoints(resultsRemoteWithJustOneService[1].snapshot!!) + .hasTheSameSecuredClusters(resultsRemoteWithJustOneService[1].snapshot!!) } @Test @@ -687,17 +639,11 @@ class SnapshotUpdaterTest { // then assertThat(results.size).isEqualTo(2) - results[0].adsSnapshot!! - .hasHttp2Cluster("service") - .hasAnEndpoint("service", "127.0.0.3", 4444) - results[0].xdsSnapshot!! + results[0].snapshot!! .hasHttp2Cluster("service") .hasAnEndpoint("service", "127.0.0.3", 4444) - results[1].adsSnapshot!! - .hasHttp2Cluster("service") - .hasAnEmptyLocalityEndpointsList("service", setOf("cluster")) - results[1].xdsSnapshot!! + results[1].snapshot!! .hasHttp2Cluster("service") .hasAnEmptyLocalityEndpointsList("service", setOf("cluster")) } @@ -707,7 +653,7 @@ class SnapshotUpdaterTest { // given val cache = MockCache() - val allServicesGroup = AllServicesGroup(communicationMode = ADS) + val allServicesGroup = AllServicesGroup() val groupWithBlacklistedDependency = groupOf(services = serviceDependencies("mock-service")) val groups = listOf(allServicesGroup, groupWithBlacklistedDependency) @@ -744,19 +690,19 @@ class SnapshotUpdaterTest { hasSnapshot(cache, allServicesGroup) .hasOnlyClustersFor(*expectedWhitelistedServices) .hasOnlyEndpointsFor(*expectedWhitelistedServices) - .hasOnlyEgressRoutesForClusters(expected = *expectedDomainsWhitelistedServices) + .hasOnlyEgressRoutesForClusters(expected = expectedDomainsWhitelistedServices) hasSnapshot(cache, groupWithBlacklistedDependency) .hasOnlyClustersFor(*expectedBlacklistedServices) .hasOnlyEndpointsFor(*expectedBlacklistedServices) - .hasOnlyEgressRoutesForClusters(expected = *expectedDomainsBlacklistedServices) + .hasOnlyEgressRoutesForClusters(expected = expectedDomainsBlacklistedServices) } @Test fun `should set snapshot in parallel`() { // given val cache = MockCache() - val groups = listOf(groupWithProxy, groupWithServiceName, AllServicesGroup(communicationMode = ADS)) + val groups = listOf(groupWithProxy, groupWithServiceName, AllServicesGroup()) groups.forEach { cache.setSnapshot(it, uninitializedSnapshot) } @@ -776,7 +722,7 @@ class SnapshotUpdaterTest { updater.startWithServices("existingService1", "existingService2") // then - hasSnapshot(cache, AllServicesGroup(communicationMode = ADS)) + hasSnapshot(cache, AllServicesGroup()) .hasOnlyClustersFor("existingService1", "existingService2") hasSnapshot(cache, groupWithProxy) .hasOnlyClustersFor("existingService1", "existingService2") @@ -1256,7 +1202,6 @@ class SnapshotUpdaterTest { domains: Set = emptySet(), listenerConfig: ListenersConfig? = null ) = ServicesGroup( - communicationMode = XDS, listenersConfig = listenerConfig, proxySettings = ProxySettings().with( serviceDependencies = services, domainDependencies = domains diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotsVersionsTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotsVersionsTest.kt index e3a9def2a..1b5499b10 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotsVersionsTest.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotsVersionsTest.kt @@ -11,7 +11,6 @@ import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test import pl.allegro.tech.servicemesh.envoycontrol.groups.AllServicesGroup import pl.allegro.tech.servicemesh.envoycontrol.groups.ClientWithSelector -import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode.XDS import pl.allegro.tech.servicemesh.envoycontrol.groups.Incoming import pl.allegro.tech.servicemesh.envoycontrol.groups.IncomingEndpoint import pl.allegro.tech.servicemesh.envoycontrol.groups.Outgoing @@ -23,7 +22,7 @@ internal class SnapshotsVersionsTest { private val snapshotsVersions = SnapshotsVersions() - private val group = AllServicesGroup(communicationMode = XDS) + private val group = AllServicesGroup() private val clusters = listOf(cluster(name = "service1")) private val endpoints = listOf(endpoints(clusterName = "service1", instances = 1)) @@ -137,7 +136,6 @@ internal class SnapshotsVersionsTest { private fun createGroup(endpointPath: String): AllServicesGroup { return AllServicesGroup( - communicationMode = XDS, serviceName = "name", proxySettings = ProxySettings( incoming = Incoming( diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/JwtFilterFactoryTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/JwtFilterFactoryTest.kt index 3941082f4..be6f7db64 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/JwtFilterFactoryTest.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/JwtFilterFactoryTest.kt @@ -7,7 +7,6 @@ import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3 import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test import pl.allegro.tech.servicemesh.envoycontrol.groups.ClientWithSelector -import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode import pl.allegro.tech.servicemesh.envoycontrol.groups.Group import pl.allegro.tech.servicemesh.envoycontrol.groups.Incoming import pl.allegro.tech.servicemesh.envoycontrol.groups.IncomingEndpoint @@ -38,7 +37,7 @@ internal class JwtFilterFactoryTest { ) private val noProviderJwtFilterFactory = JwtFilterFactory(JwtFilterProperties().also { it.providers = emptyMap() }) - private val emptyGroup: Group = ServicesGroup(CommunicationMode.ADS) + private val emptyGroup: Group = ServicesGroup() @Test fun `should not create JWT filter when no providers are defined`() { @@ -160,7 +159,7 @@ internal class JwtFilterFactoryTest { } private fun createGroup(pathToProvider: Map, policy: OAuth.Policy) = ServicesGroup( - CommunicationMode.ADS, proxySettings = ProxySettings( + proxySettings = ProxySettings( Incoming( pathToProvider.map { (path, provider) -> IncomingEndpoint( @@ -173,12 +172,12 @@ internal class JwtFilterFactoryTest { ) private fun createGroup(incomingEndpoints: List) = ServicesGroup( - CommunicationMode.ADS, proxySettings = ProxySettings( + proxySettings = ProxySettings( Incoming(incomingEndpoints) ) ) private fun createGroupWithClientWithSelector(pathToProvider: Map) = ServicesGroup( - CommunicationMode.ADS, proxySettings = ProxySettings( + proxySettings = ProxySettings( Incoming( pathToProvider.map { (path, _) -> IncomingEndpoint( diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/LuaFilterFactoryTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/LuaFilterFactoryTest.kt index 901b5dd5a..08bbc880b 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/LuaFilterFactoryTest.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/LuaFilterFactoryTest.kt @@ -2,7 +2,6 @@ package pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.fil import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test -import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode import pl.allegro.tech.servicemesh.envoycontrol.groups.Group import pl.allegro.tech.servicemesh.envoycontrol.groups.ServicesGroup import pl.allegro.tech.servicemesh.envoycontrol.snapshot.IncomingPermissionsProperties @@ -20,7 +19,6 @@ internal class LuaFilterFactoryTest { val expectedServiceName = "service-1" val expectedDiscoveryServiceName = "consul-service-1" val group: Group = ServicesGroup( - communicationMode = CommunicationMode.XDS, serviceName = expectedServiceName, discoveryServiceName = expectedDiscoveryServiceName ) @@ -58,7 +56,6 @@ internal class LuaFilterFactoryTest { val expectedServiceName = "service-1" val expectedDiscoveryServiceName = "consul-service-1" val group: Group = ServicesGroup( - communicationMode = CommunicationMode.XDS, serviceName = expectedServiceName, discoveryServiceName = expectedDiscoveryServiceName ) diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RateLimitFilterFactoryTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RateLimitFilterFactoryTest.kt index 114c7bae6..37a3ebb5e 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RateLimitFilterFactoryTest.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RateLimitFilterFactoryTest.kt @@ -4,7 +4,6 @@ import io.envoyproxy.envoy.extensions.filters.http.lua.v3.Lua import io.envoyproxy.envoy.extensions.filters.http.ratelimit.v3.RateLimit import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test -import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode import pl.allegro.tech.servicemesh.envoycontrol.groups.Group import pl.allegro.tech.servicemesh.envoycontrol.groups.Incoming import pl.allegro.tech.servicemesh.envoycontrol.groups.IncomingRateLimitEndpoint @@ -96,7 +95,7 @@ class RateLimitFilterFactoryTest { } private fun createGroupWithIncomingRateLimits(serviceName: String, vararg rateLimitEndpoints: IncomingRateLimitEndpoint): Group = - ServicesGroup(CommunicationMode.ADS, + ServicesGroup( serviceName = serviceName, proxySettings = ProxySettings(incoming = Incoming( rateLimitEndpoints = rateLimitEndpoints.toList() diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/rbac/RBACFilterFactoryTestUtils.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/rbac/RBACFilterFactoryTestUtils.kt index 3b84eca33..739a88f34 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/rbac/RBACFilterFactoryTestUtils.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/rbac/RBACFilterFactoryTestUtils.kt @@ -4,7 +4,6 @@ import com.google.protobuf.Any import com.google.protobuf.util.JsonFormat import io.envoyproxy.envoy.extensions.filters.http.rbac.v3.RBAC import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.HttpFilter -import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode import pl.allegro.tech.servicemesh.envoycontrol.groups.Incoming import pl.allegro.tech.servicemesh.envoycontrol.groups.ProxySettings import pl.allegro.tech.servicemesh.envoycontrol.groups.ServicesGroup @@ -131,7 +130,6 @@ interface RBACFilterFactoryTestUtils { incomingPermission: Incoming? = null, serviceName: String = "some-service" ) = ServicesGroup( - communicationMode = CommunicationMode.ADS, serviceName = serviceName, proxySettings = ProxySettings( incoming = incomingPermission ?: Incoming() diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyIngressRoutesFactoryTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyIngressRoutesFactoryTest.kt index b7270887b..1bdea5bcf 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyIngressRoutesFactoryTest.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/routes/EnvoyIngressRoutesFactoryTest.kt @@ -3,7 +3,6 @@ package pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.routes import com.google.protobuf.util.Durations import org.junit.jupiter.api.Test import pl.allegro.tech.servicemesh.envoycontrol.groups.ClientWithSelector -import pl.allegro.tech.servicemesh.envoycontrol.groups.CommunicationMode import pl.allegro.tech.servicemesh.envoycontrol.groups.HealthCheck import pl.allegro.tech.servicemesh.envoycontrol.groups.Incoming import pl.allegro.tech.servicemesh.envoycontrol.groups.Incoming.TimeoutPolicy @@ -117,7 +116,6 @@ internal class EnvoyIngressRoutesFactoryTest { ) ) val group = ServicesGroup( - CommunicationMode.XDS, "service_1", "service_1", proxySettingsOneEndpoint @@ -196,7 +194,6 @@ internal class EnvoyIngressRoutesFactoryTest { ) ) val group = ServicesGroup( - CommunicationMode.XDS, "service_1", "service_1", proxySettingsOneEndpoint diff --git a/envoy-control-runner/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/debug/SnapshotDebugController.kt b/envoy-control-runner/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/debug/SnapshotDebugController.kt index e831fbb10..19415d833 100644 --- a/envoy-control-runner/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/debug/SnapshotDebugController.kt +++ b/envoy-control-runner/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/debug/SnapshotDebugController.kt @@ -54,9 +54,9 @@ class SnapshotDebugController(val debugService: SnapshotDebugService) { } @GetMapping("/snapshot-global") - fun globalSnapshot(@RequestParam(defaultValue = "false") xds: Boolean): ResponseEntity { + fun globalSnapshot(): ResponseEntity { return ResponseEntity( - debugService.globalSnapshot(xds), + debugService.globalSnapshot(), HttpStatus.OK ) } @@ -65,17 +65,16 @@ class SnapshotDebugController(val debugService: SnapshotDebugService) { fun globalSnapshot( @PathVariable service: String, @RequestParam dc: String?, - @RequestParam(defaultValue = "false") xds: Boolean ): ResponseEntity { return ResponseEntity( - debugService.globalSnapshot(service, dc, xds), + debugService.globalSnapshot(service, dc), HttpStatus.OK ) } @JsonComponent class ProtoSerializer : JsonSerializer() { - final val typeRegistry: TypeRegistry = TypeRegistry.newBuilder() + private final val typeRegistry: TypeRegistry = TypeRegistry.newBuilder() .add(HttpConnectionManager.getDescriptor()) .add(Config.getDescriptor()) .add(BoolValue.getDescriptor()) diff --git a/envoy-control-runner/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/debug/SnapshotDebugService.kt b/envoy-control-runner/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/debug/SnapshotDebugService.kt index 346a74245..b3e7ed870 100644 --- a/envoy-control-runner/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/debug/SnapshotDebugService.kt +++ b/envoy-control-runner/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/debug/SnapshotDebugService.kt @@ -30,30 +30,18 @@ class SnapshotDebugService( } } - fun globalSnapshot(xds: Boolean): SnapshotDebugInfo { + fun globalSnapshot(): SnapshotDebugInfo { val globalSnapshot = snapshotUpdater.getGlobalSnapshot() - if (xds) { - return if (globalSnapshot?.xdsSnapshot == null) { - throw GlobalSnapshotNotFoundException("Xds global snapshot missing") - } else { - SnapshotDebugInfo(globalSnapshot.xdsSnapshot!!) - } - } - return if (globalSnapshot?.adsSnapshot == null) { - throw GlobalSnapshotNotFoundException("Ads global snapshot missing") + return if (globalSnapshot?.snapshot == null) { + throw GlobalSnapshotNotFoundException("Global snapshot missing") } else { - SnapshotDebugInfo(globalSnapshot.adsSnapshot!!) + SnapshotDebugInfo(globalSnapshot.snapshot!!) } } - fun globalSnapshot(service: String, dc: String?, xds: Boolean): EndpointInfoList { + fun globalSnapshot(service: String, dc: String?): EndpointInfoList { val updateResult = snapshotUpdater.getGlobalSnapshot() - val globalSnapshot = if (xds) { - updateResult?.xdsSnapshot - } else { - updateResult?.adsSnapshot - } - val endpoints = extractEndpoints(globalSnapshot, service) + val endpoints = extractEndpoints(updateResult?.snapshot, service) val endpointInfos = getEndpointsInfo(endpoints, dc) return EndpointInfoList(endpointInfos) } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt index c281cf7e1..42248eef3 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt @@ -54,7 +54,7 @@ open class EndpointMetadataMergingTests { protected open fun callEchoServiceRepeatedly( repeat: Int, - tag: String? = null, + tag: String, assertNoErrors: Boolean = true ): CallStats { val stats = CallStats(listOf(service)) @@ -63,7 +63,7 @@ open class EndpointMetadataMergingTests { stats = stats, minRepeat = repeat, maxRepeat = repeat, - headers = tag?.let { mapOf("x-service-tag" to it) } ?: emptyMap(), + headers = mapOf("x-service-tag" to tag), assertNoErrors = assertNoErrors ) return stats diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlHttp2Test.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlHttp2Test.kt index 273454cdb..e18707667 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlHttp2Test.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlHttp2Test.kt @@ -7,48 +7,12 @@ import pl.allegro.tech.servicemesh.envoycontrol.assertions.isFrom import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted import pl.allegro.tech.servicemesh.envoycontrol.config.Ads -import pl.allegro.tech.servicemesh.envoycontrol.config.Xds import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyContainer import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension -class XdsEnvoyControlHttp2Test : EnvoyControlHttp2Test { - companion object { - - @JvmField - @RegisterExtension - val consul = ConsulExtension() - - @JvmField - @RegisterExtension - val envoyControl = EnvoyControlExtension(consul) - - @JvmField - @RegisterExtension - val service = EchoServiceExtension() - - @JvmField - @RegisterExtension - val envoy = EnvoyExtension(envoyControl, service, config = Xds) - - @JvmField - @RegisterExtension - val secondEnvoy = EnvoyExtension(envoyControl, service, config = Xds) - } - - override fun consul() = consul - - override fun envoyControl() = envoyControl - - override fun service() = service - - override fun envoy() = envoy - - override fun secondEnvoy() = secondEnvoy -} - class AdsEnvoyControlHttp2Test : EnvoyControlHttp2Test { companion object { diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlSmokeTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlSmokeTest.kt index cf7410a17..abf0aca63 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlSmokeTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlSmokeTest.kt @@ -8,7 +8,6 @@ import pl.allegro.tech.servicemesh.envoycontrol.config.Ads import pl.allegro.tech.servicemesh.envoycontrol.config.AdsWithNoDependencies import pl.allegro.tech.servicemesh.envoycontrol.config.AdsWithStaticListeners import pl.allegro.tech.servicemesh.envoycontrol.config.DeltaAds -import pl.allegro.tech.servicemesh.envoycontrol.config.Xds import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension @@ -130,35 +129,6 @@ class DeltaAdsEnvoyControlSmokeTest : EnvoyControlSmokeTest { override fun envoy() = envoy } -class XdsEnvoyControlSmokeTest : EnvoyControlSmokeTest { - companion object { - - @JvmField - @RegisterExtension - val consul = ConsulExtension() - - @JvmField - @RegisterExtension - val envoyControl = EnvoyControlExtension(consul) - - @JvmField - @RegisterExtension - val service = EchoServiceExtension() - - @JvmField - @RegisterExtension - val envoy = EnvoyExtension(envoyControl, service, config = Xds) - } - - override fun consul() = consul - - override fun envoyControl() = envoyControl - - override fun service() = service - - override fun envoy() = envoy -} - interface EnvoyControlSmokeTest { fun consul(): ConsulExtension diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlTest.kt index 2f5a4ad42..9da0de858 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlTest.kt @@ -8,7 +8,6 @@ import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted import pl.allegro.tech.servicemesh.envoycontrol.config.Ads import pl.allegro.tech.servicemesh.envoycontrol.config.AdsWithStaticListeners -import pl.allegro.tech.servicemesh.envoycontrol.config.Xds import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension @@ -84,41 +83,6 @@ class AdsEnvoyControlTest : EnvoyControlTest { override fun envoy() = envoy } -class XdsEnvoyControlTest : EnvoyControlTest { - companion object { - - @JvmField - @RegisterExtension - val consul = ConsulExtension() - - @JvmField - @RegisterExtension - val envoyControl = EnvoyControlExtension(consul) - - @JvmField - @RegisterExtension - val service = EchoServiceExtension() - - @JvmField - @RegisterExtension - val redeployedService = EchoServiceExtension() - - @JvmField - @RegisterExtension - val envoy = EnvoyExtension(envoyControl, config = Xds) - } - - override fun consul() = consul - - override fun envoyControl() = envoyControl - - override fun service() = service - - override fun redeployedService() = redeployedService - - override fun envoy() = envoy -} - interface EnvoyControlTest { fun consul(): ConsulExtension diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/MetricsDiscoveryServerCallbacksTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/MetricsDiscoveryServerCallbacksTest.kt index beffcee16..b496c7b13 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/MetricsDiscoveryServerCallbacksTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/MetricsDiscoveryServerCallbacksTest.kt @@ -7,7 +7,6 @@ import org.junit.jupiter.api.extension.RegisterExtension import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted import pl.allegro.tech.servicemesh.envoycontrol.config.Ads import pl.allegro.tech.servicemesh.envoycontrol.config.DeltaAds -import pl.allegro.tech.servicemesh.envoycontrol.config.Xds import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension @@ -21,62 +20,6 @@ import pl.allegro.tech.servicemesh.envoycontrol.server.callbacks.MetricsDiscover import pl.allegro.tech.servicemesh.envoycontrol.server.callbacks.MetricsDiscoveryServerCallbacks.StreamType.SDS import pl.allegro.tech.servicemesh.envoycontrol.server.callbacks.MetricsDiscoveryServerCallbacks.StreamType.UNKNOWN -class XdsMetricsDiscoveryServerCallbacksTest : MetricsDiscoveryServerCallbacksTest { - companion object { - - @JvmField - @RegisterExtension - val consul = ConsulExtension() - - @JvmField - @RegisterExtension - val envoyControl = EnvoyControlExtension(consul) - - @JvmField - @RegisterExtension - val service = EchoServiceExtension() - - @JvmField - @RegisterExtension - val envoy = EnvoyExtension(envoyControl, service, config = Xds) - } - - override fun consul() = consul - - override fun envoyControl() = envoyControl - - override fun service() = service - - override fun envoy() = envoy - - override fun expectedGrpcConnectionsGaugeValues() = mapOf( - CDS to 1, - EDS to 2, // separate streams for consul and echo - LDS to 1, - RDS to 2, // default_routes - SDS to 0, - ADS to 0, - UNKNOWN to 0 - ) - - override fun expectedGrpcRequestsCounterValues() = mapOf( - CDS.name.toLowerCase() to isGreaterThanZero(), - EDS.name.toLowerCase() to isGreaterThanZero(), - LDS.name.toLowerCase() to isGreaterThanZero(), - RDS.name.toLowerCase() to isGreaterThanZero(), - SDS.name.toLowerCase() to isNull(), - ADS.name.toLowerCase() to isNull(), - UNKNOWN.name.toLowerCase() to isNull(), - "${CDS.name.toLowerCase()}.delta" to isNull(), - "${EDS.name.toLowerCase()}.delta" to isNull(), - "${LDS.name.toLowerCase()}.delta" to isNull(), - "${RDS.name.toLowerCase()}.delta" to isNull(), - "${SDS.name.toLowerCase()}.delta" to isNull(), - "${ADS.name.toLowerCase()}.delta" to isNull(), - "${UNKNOWN.name.toLowerCase()}.delta" to isNull() - ) -} - class AdsMetricsDiscoveryServerCallbackTest : MetricsDiscoveryServerCallbacksTest { companion object { diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/OutgoingPermissionsTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/OutgoingPermissionsTest.kt index 73f898820..52513e179 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/OutgoingPermissionsTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/OutgoingPermissionsTest.kt @@ -8,7 +8,6 @@ import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk import pl.allegro.tech.servicemesh.envoycontrol.assertions.isUnreachable import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted import pl.allegro.tech.servicemesh.envoycontrol.config.Ads -import pl.allegro.tech.servicemesh.envoycontrol.config.Xds import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension @@ -45,37 +44,6 @@ class AdsOutgoingPermissionsTest : OutgoingPermissionsTest { override fun envoy() = envoy } -class XdsOutgoingPermissionsTest : OutgoingPermissionsTest { - - companion object { - - @JvmField - @RegisterExtension - val consul = ConsulExtension() - - @JvmField - @RegisterExtension - val envoyControl = EnvoyControlExtension(consul, mapOf( - "envoy-control.envoy.snapshot.outgoing-permissions.enabled" to true, - "envoy-control.envoy.snapshot.egress.domains" to mutableListOf(".test.domain", ".domain") - )) - - @JvmField - @RegisterExtension - val service = EchoServiceExtension() - - @JvmField - @RegisterExtension - val envoy = EnvoyExtension(envoyControl, config = Xds) - } - - override fun consul() = consul - - override fun service() = service - - override fun envoy() = envoy -} - interface OutgoingPermissionsTest { fun consul(): ConsulExtension diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/RateLimitTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/RateLimitTest.kt index 0931cb000..29ce50986 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/RateLimitTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/RateLimitTest.kt @@ -6,7 +6,6 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.RegisterExtension import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted import pl.allegro.tech.servicemesh.envoycontrol.config.Ads -import pl.allegro.tech.servicemesh.envoycontrol.config.Xds import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension @@ -57,44 +56,6 @@ open class AdsRateLimitTest : RateLimitTest { override fun rateLimitService() = rateLimitService } -open class XdsRateLimitTest : RateLimitTest { - companion object { - @JvmField - @RegisterExtension - val consul = ConsulExtension() - - @JvmField - @RegisterExtension - val envoyControl = EnvoyControlExtension(consul) - - @JvmField - @RegisterExtension - val service = EchoServiceExtension() - - @JvmField - @RegisterExtension - val envoy = EnvoyExtension(envoyControl, service, config = Xds.copy(configOverride = RATE_LIMIT_CONFIG)) - - @JvmField - @RegisterExtension - val redis = GenericServiceExtension(RedisContainer()) - - @JvmField - @RegisterExtension - val rateLimitService = GenericServiceExtension(RedisBasedRateLimitContainer(redis.container())) - } - - override fun consul() = consul - - override fun envoyControl() = envoyControl - - override fun service() = service - - override fun envoy() = envoy - - override fun rateLimitService() = rateLimitService -} - val RATE_LIMIT_CONFIG = """ node: metadata: diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/RetryPolicyTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/RetryPolicyTest.kt index 6fbb3627c..0f736ef84 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/RetryPolicyTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/RetryPolicyTest.kt @@ -7,7 +7,7 @@ import org.junit.jupiter.api.extension.RegisterExtension import pl.allegro.tech.servicemesh.envoycontrol.assertions.isFrom import pl.allegro.tech.servicemesh.envoycontrol.assertions.isOk import pl.allegro.tech.servicemesh.envoycontrol.assertions.untilAsserted -import pl.allegro.tech.servicemesh.envoycontrol.config.Xds +import pl.allegro.tech.servicemesh.envoycontrol.config.Ads import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension @@ -59,7 +59,7 @@ node: @JvmField @RegisterExtension - val envoy = EnvoyExtension(envoyControl, service, config = Xds.copy(configOverride = RETRY_SETTINGS_CONFIG)) + val envoy = EnvoyExtension(envoyControl, service, config = Ads.copy(configOverride = RETRY_SETTINGS_CONFIG)) } @Test diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/SnapshotDebugTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/SnapshotDebugTest.kt index 2bfd0463b..53b01612c 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/SnapshotDebugTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/SnapshotDebugTest.kt @@ -184,35 +184,22 @@ interface SnapshotDebugTest { assertThat(snapshot.found).isFalse() } - @Test - fun `should return global snapshot debug info from xds`() { - untilAsserted { - // when - val snapshot = envoyControl().app.getGlobalSnapshot(xds = true) - - // then - assertThat(snapshot.snapshot!!["clusters"]).isNotEmpty() - assertThat(snapshot.snapshot["endpoints"]).isNotEmpty() - assertThat(snapshot.snapshot["clusters"].first()["edsClusterConfig"]["edsConfig"].toString()).contains("envoy-control-xds") - } - } - @Test fun `should return global snapshot debug info from ads`() { untilAsserted { // when - val snapshotXdsNull = envoyControl().app.getGlobalSnapshot(xds = null) - val snapshotXdsFalse = envoyControl().app.getGlobalSnapshot(xds = false) + val snapshotNull = envoyControl().app.getGlobalSnapshot() + val snapshotFalse = envoyControl().app.getGlobalSnapshot() // then - assertThat(snapshotXdsNull.snapshot!!["clusters"]).isNotEmpty() - assertThat(snapshotXdsNull.snapshot["endpoints"]).isNotEmpty() - assertThat(snapshotXdsFalse.snapshot!!["clusters"]).isNotEmpty() - assertThat(snapshotXdsFalse.snapshot["endpoints"]).isNotEmpty() - assertThat(snapshotXdsNull.snapshot["clusters"].first()["edsClusterConfig"]["edsConfig"].toString()).contains( + assertThat(snapshotNull.snapshot!!["clusters"]).isNotEmpty() + assertThat(snapshotNull.snapshot["endpoints"]).isNotEmpty() + assertThat(snapshotFalse.snapshot!!["clusters"]).isNotEmpty() + assertThat(snapshotFalse.snapshot["endpoints"]).isNotEmpty() + assertThat(snapshotNull.snapshot["clusters"].first()["edsClusterConfig"]["edsConfig"].toString()).contains( "ads" ) - assertThat(snapshotXdsFalse.snapshot["clusters"].first()["edsClusterConfig"]["edsConfig"].toString()).contains( + assertThat(snapshotFalse.snapshot["clusters"].first()["edsClusterConfig"]["edsConfig"].toString()).contains( "ads" ) } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/EnvoyControlTestConfiguration.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/EnvoyControlTestConfiguration.kt index 9a968b2be..8b66e1b8c 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/EnvoyControlTestConfiguration.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/EnvoyControlTestConfiguration.kt @@ -68,8 +68,7 @@ val Echo3EnvoyAuthConfig = Echo1EnvoyAuthConfig.copy( val AdsWithDisabledEndpointPermissions = EnvoyConfig("envoy/config_ads_disabled_endpoint_permissions.yaml") val AdsWithStaticListeners = EnvoyConfig("envoy/config_ads_static_listeners.yaml") val AdsWithNoDependencies = EnvoyConfig("envoy/config_ads_no_dependencies.yaml") -val Xds = EnvoyConfig("envoy/config_xds.yaml") -val RandomConfigFile = listOf(Ads, Xds, DeltaAds).random() +val RandomConfigFile = listOf(Ads, DeltaAds).random() val OAuthEnvoyConfig = EnvoyConfig("envoy/config_oauth.yaml") @Deprecated("use extension approach instead, e.g. RetryPolicyTest") @@ -171,15 +170,15 @@ abstract class EnvoyControlTestConfiguration : BaseEnvoyTest() { localServiceIp: String = localServiceContainer.ipAddress(), envoyImage: String = EnvoyContainer.DEFAULT_IMAGE ): EnvoyContainer { - val envoyControl1XdsPort = envoyConnectGrpcPort ?: envoyControl1.grpcPort - val envoyControl2XdsPort = if (envoyControls == 2 && instancesInSameDc) { + val envoyControl1Port = envoyConnectGrpcPort ?: envoyControl1.grpcPort + val envoyControl2Port = if (envoyControls == 2 && instancesInSameDc) { envoyConnectGrpcPort2 ?: envoyControl2.grpcPort - } else envoyControl1XdsPort + } else envoyControl1Port return EnvoyContainer( config = envoyConfig, localServiceIp = { localServiceIp }, - envoyControl1XdsPort = envoyControl1XdsPort, - envoyControl2XdsPort = envoyControl2XdsPort, + envoyControl1Port = envoyControl1Port, + envoyControl2Port = envoyControl2Port, image = envoyImage ).withNetwork(network) } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyContainer.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyContainer.kt index 99fe91166..d276f589d 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyContainer.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyContainer.kt @@ -13,8 +13,8 @@ import pl.allegro.tech.servicemesh.envoycontrol.logger as loggerDelegate class EnvoyContainer( private val config: EnvoyConfig, private val localServiceIp: () -> String, - private val envoyControl1XdsPort: Int, - private val envoyControl2XdsPort: Int = envoyControl1XdsPort, + private val envoyControl1Port: Int, + private val envoyControl2Port: Int = envoyControl1Port, private val logLevel: String = "info", image: String = DEFAULT_IMAGE ) : SSLGenericContainer( @@ -59,8 +59,8 @@ class EnvoyContainer( withCommand( "/bin/sh", "/usr/local/bin/launch_envoy.sh", - Integer.toString(envoyControl1XdsPort), - Integer.toString(envoyControl2XdsPort), + envoyControl1Port.toString(), + envoyControl2Port.toString(), CONFIG_DEST, localServiceIp(), config.trustedCa, diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt index cc5f22468..07f686ec9 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoy/EnvoyExtension.kt @@ -18,7 +18,7 @@ import pl.allegro.tech.servicemesh.envoycontrol.logger class EnvoyExtension( private val envoyControl: EnvoyControlExtensionBase, private val localService: ServiceExtension<*>? = null, - private val config: EnvoyConfig = RandomConfigFile + config: EnvoyConfig = RandomConfigFile ) : BeforeAllCallback, AfterAllCallback, AfterEachCallback { companion object { diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoycontrol/EnvoyControlTestApp.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoycontrol/EnvoyControlTestApp.kt index 2bc55e721..e51fe1a1e 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoycontrol/EnvoyControlTestApp.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/config/envoycontrol/EnvoyControlTestApp.kt @@ -33,7 +33,7 @@ interface EnvoyControlTestApp { fun isHealthy(): Boolean fun getState(): ServicesState fun getSnapshot(nodeJson: String): SnapshotDebugResponse - fun getGlobalSnapshot(xds: Boolean?): SnapshotDebugResponse + fun getGlobalSnapshot(): SnapshotDebugResponse fun getHealthStatus(): Health fun postChaosFaultRequest( username: String = "user", @@ -129,11 +129,8 @@ class EnvoyControlRunnerTestApp( ?.copy(found = true) ?: throw SnapshotDebugResponseMissingException() } - override fun getGlobalSnapshot(xds: Boolean?): SnapshotDebugResponse { + override fun getGlobalSnapshot(): SnapshotDebugResponse { var url = "http://localhost:$appPort/snapshot-global" - if (xds != null) { - url += "?xds=$xds" - } val response = httpClient.newCall( Request.Builder() .get() diff --git a/envoy-control-tests/src/main/resources/envoy/config_xds.yaml b/envoy-control-tests/src/main/resources/envoy/config_xds.yaml deleted file mode 100644 index 511b28e73..000000000 --- a/envoy-control-tests/src/main/resources/envoy/config_xds.yaml +++ /dev/null @@ -1,119 +0,0 @@ -admin: - access_log_path: /dev/null - address: - socket_address: { address: 0.0.0.0, port_value: 10000 } -dynamic_resources: - lds_config: - resource_api_version: V3 - api_config_source: - api_type: GRPC - transport_api_version: V3 - grpc_services: - envoy_grpc: - cluster_name: envoy-control-xds - cds_config: - resource_api_version: V3 - api_config_source: - api_type: GRPC - transport_api_version: V3 - grpc_services: - envoy_grpc: - cluster_name: envoy-control-xds -node: - cluster: test-cluster - id: test-id - metadata: - ingress_host: "0.0.0.0" - ingress_port: 5001 - egress_host: "0.0.0.0" - egress_port: 5000 - use_remote_address: true - generate_request_id: true - preserve_external_request_id: true - access_log_enabled: false - add_upstream_external_address_header: true - resources_dir: "/etc/envoy/extra" - service_name: "echo2" - proxy_settings: - incoming: - endpoints: - - path: "/endpoint" - clients: ["authorizedClient"] - outgoing: - dependencies: - - service: "service-1" - - service: "service-2" - - service: "service-3" - - service: "service-4" - - service: "service-5" - - service: "echo" - timeoutPolicy: - requestTimeout: "15s" - - service: "consul" - timeoutPolicy: - requestTimeout: "15s" - - service: "proxy1" - - service: "proxy2" - - service: "service-redirect" - handleInternalRedirect: true - - service: "host-rewrite-service" - rewriteHostHeader: true - - domain: "https://my.example.com" - - domain: "https://bad.host.example.com" - - domain: "https://www.example.com" - - domain: "https://www.example-redirect.com" - handleInternalRedirect: true - -static_resources: - clusters: - - connect_timeout: 1s - load_assignment: - cluster_name: envoy-control-xds - endpoints: - - lb_endpoints: - - endpoint: - address: - socket_address: - address: HOST_IP - port_value: HOST_PORT - - endpoint: - address: - socket_address: - address: HOST_IP - port_value: HOST2_PORT - http2_protocol_options: {} - name: envoy-control-xds - - name: envoy-original-destination - type: ORIGINAL_DST - lb_policy: CLUSTER_PROVIDED - original_dst_lb_config: - use_http_header: true - connect_timeout: - seconds: 1 - http_protocol_options: - allow_absolute_url: true - - name: local_service - type: STATIC - load_assignment: - cluster_name: local_service - endpoints: - - lb_endpoints: - - endpoint: - address: - socket_address: - address: LOCAL_SERVICE_IP - port_value: 5678 - connect_timeout: 1s - - name: this_admin - type: STATIC - load_assignment: - cluster_name: this_admin - endpoints: - - lb_endpoints: - - endpoint: - address: - socket_address: - address: 127.0.0.1 - port_value: 10000 - connect_timeout: - seconds: 1 From 9fd2ce32944137ac78b16430c31ab81f9433e0a4 Mon Sep 17 00:00:00 2001 From: Kamil Smigielski Date: Fri, 11 Nov 2022 19:19:38 +0100 Subject: [PATCH 2/7] fix tests --- .../MetricsDiscoveryServerCallbacks.kt | 6 +- .../snapshot/EnvoySnapshotFactory.kt | 2 +- .../resource/clusters/EnvoyClustersFactory.kt | 2 +- .../filters/HttpConnectionManagerFactory.kt | 40 ++------- .../groups/NodeMetadataValidatorTest.kt | 16 +--- .../EndpointMetadataMergingTests.kt | 3 +- .../envoycontrol/EnvoyControlHttp2Test.kt | 46 +++------- .../MetricsDiscoveryServerCallbacksTest.kt | 84 ++++++++++--------- .../envoycontrol/OutgoingPermissionsTest.kt | 41 +++------ .../servicemesh/envoycontrol/RateLimitTest.kt | 82 +++++++----------- .../envoycontrol/ServiceTagsAndCanaryTest.kt | 35 -------- .../ServiceTagsAndCanaryTestBase.kt | 46 ++++++---- .../src/main/resources/envoy/bad_config.yaml | 3 - .../src/main/resources/envoy/config_ads.yaml | 3 - .../envoy/config_ads_all_dependencies.yaml | 3 - .../envoy/config_ads_custom_health_check.yaml | 5 -- ...fig_ads_disabled_endpoint_permissions.yaml | 3 - .../config_ads_dynamic_forward_proxy.yaml | 3 - .../envoy/config_ads_no_dependencies.yaml | 3 - .../envoy/config_ads_static_listeners.yaml | 5 -- .../src/main/resources/envoy/config_auth.yaml | 3 - .../main/resources/envoy/config_oauth.yaml | 3 - tools/envoy/envoy-template.yaml | 1 - 23 files changed, 144 insertions(+), 294 deletions(-) delete mode 100644 envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsAndCanaryTest.kt diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/server/callbacks/MetricsDiscoveryServerCallbacks.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/server/callbacks/MetricsDiscoveryServerCallbacks.kt index c0f65410b..6110002e2 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/server/callbacks/MetricsDiscoveryServerCallbacks.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/server/callbacks/MetricsDiscoveryServerCallbacks.kt @@ -36,7 +36,7 @@ class MetricsDiscoveryServerCallbacks(private val meterRegistry: MeterRegistry) meterRegistry.gauge("grpc.all-connections", connections) connectionsByType.forEach { (type, typeConnections) -> - meterRegistry.gauge("grpc.connections.${type.name.toLowerCase()}", typeConnections) + meterRegistry.gauge("grpc.connections.${type.name.lowercase()}", typeConnections) } } @@ -51,7 +51,7 @@ class MetricsDiscoveryServerCallbacks(private val meterRegistry: MeterRegistry) } override fun onV3StreamRequest(streamId: Long, request: V3DiscoveryRequest) { - meterRegistry.counter("grpc.requests.${StreamType.fromTypeUrl(request.typeUrl).name.toLowerCase()}") + meterRegistry.counter("grpc.requests.${StreamType.fromTypeUrl(request.typeUrl).name.lowercase()}") .increment() } @@ -59,7 +59,7 @@ class MetricsDiscoveryServerCallbacks(private val meterRegistry: MeterRegistry) streamId: Long, request: V3DeltaDiscoveryRequest ) { - meterRegistry.counter("grpc.requests.${StreamType.fromTypeUrl(request.typeUrl).name.toLowerCase()}.delta") + meterRegistry.counter("grpc.requests.${StreamType.fromTypeUrl(request.typeUrl).name.lowercase()}.delta") .increment() } diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt index ecdcff284..cd4d7ce9d 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt @@ -315,7 +315,7 @@ class EnvoySnapshotFactory( routes.add( egressRoutesFactory.createEgressDomainRoutes( it.value, - it.key.port.toString().toLowerCase() + it.key.port.toString().lowercase() ) ) } diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt index ad3829270..d39c54d31 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt @@ -464,7 +464,7 @@ class EnvoyClustersFactory( thresholdsBuilder.maxPendingRequests = UInt32Value.of(threshold.maxPendingRequests) thresholdsBuilder.maxRequests = UInt32Value.of(threshold.maxRequests) thresholdsBuilder.maxRetries = UInt32Value.of(threshold.maxRetries) - when (threshold.priority.toUpperCase()) { + when (threshold.priority.uppercase()) { "DEFAULT" -> thresholdsBuilder.priority = RoutingPriority.DEFAULT "HIGH" -> thresholdsBuilder.priority = RoutingPriority.HIGH else -> thresholdsBuilder.priority = RoutingPriority.UNRECOGNIZED 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 e6e9034a2..80be1b8d8 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 @@ -3,10 +3,9 @@ package pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.fil import com.google.protobuf.BoolValue import com.google.protobuf.Duration import com.google.protobuf.util.Durations -import io.envoyproxy.envoy.config.core.v3.ApiConfigSource +import io.envoyproxy.envoy.config.core.v3.AggregatedConfigSource import io.envoyproxy.envoy.config.core.v3.ApiVersion import io.envoyproxy.envoy.config.core.v3.ConfigSource -import io.envoyproxy.envoy.config.core.v3.GrpcService import io.envoyproxy.envoy.config.core.v3.Http1ProtocolOptions import io.envoyproxy.envoy.config.core.v3.HttpProtocolOptions import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager @@ -34,7 +33,6 @@ class HttpConnectionManagerFactory( snapshotProperties.dynamicForwardProxy ).filter - private val defaultApiConfigSourceV3: ApiConfigSource = apiConfigSource() private val accessLogFilter = AccessLogFilter(snapshotProperties) @SuppressWarnings("LongParameterList") @@ -93,7 +91,7 @@ class HttpConnectionManagerFactory( connectionManagerBuilder.addAccessLog( accessLogFilter.createFilter( listenersConfig.accessLogPath, - direction.name.toLowerCase(), + direction.name.lowercase(), listenersConfig.accessLogFilterSettings ) ) @@ -106,19 +104,16 @@ class HttpConnectionManagerFactory( private fun setupRds( rdsInitialFetchTimeout: Duration, routeConfigName: String - ): Rds { - val configSource = ConfigSource.newBuilder() - .setInitialFetchTimeout(rdsInitialFetchTimeout) - .setResourceApiVersion(ApiVersion.V3) - .setApiConfigSource(defaultApiConfigSourceV3) - - return Rds.newBuilder() + ): Rds = Rds.newBuilder() .setRouteConfigName(routeConfigName) .setConfigSource( - configSource.build() + ConfigSource.newBuilder() + .setInitialFetchTimeout(rdsInitialFetchTimeout) + .setResourceApiVersion(ApiVersion.V3) + .setAds(AggregatedConfigSource.getDefaultInstance()) + .build() ) .build() - } private fun ingressHttp1ProtocolOptions(serviceName: String): Http1ProtocolOptions? { return Http1ProtocolOptions.newBuilder() @@ -127,25 +122,6 @@ class HttpConnectionManagerFactory( .build() } - private fun apiConfigSource(): ApiConfigSource { - return ApiConfigSource.newBuilder() - .setApiType( - if (snapshotProperties.deltaXdsEnabled) { - ApiConfigSource.ApiType.DELTA_GRPC - } else { - ApiConfigSource.ApiType.GRPC - } - ) - .setTransportApiVersion(ApiVersion.V3) - .addGrpcServices( - GrpcService.newBuilder() - .setEnvoyGrpc( - GrpcService.EnvoyGrpc.newBuilder() - .setClusterName(snapshotProperties.xdsClusterName) - ) - ).build() - } - private fun addHttpFilters( connectionManagerBuilder: HttpConnectionManager.Builder, filterFactories: List, 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 7862ce976..76c3af900 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 @@ -152,12 +152,8 @@ class NodeMetadataValidatorTest { // expects assertThatExceptionOfType(ServiceNameNotProvidedException::class.java) .isThrownBy { requireServiceNameValidator.onV3StreamRequest(streamId = 123, request = request) } - .satisfies { - assertThat(it.status.description).isEqualTo( - "Service name has not been provided." - ) - assertThat(it.status.code).isEqualTo(Status.Code.INVALID_ARGUMENT) - } + .matches { it.status.description == "Service name has not been provided." } + .matches { it.status.code == Status.Code.INVALID_ARGUMENT } } @Test @@ -187,12 +183,8 @@ class NodeMetadataValidatorTest { // then assertThatExceptionOfType(RateLimitIncorrectValidationException::class.java) .isThrownBy { validator.onV3StreamRequest(123, request = request) } - .satisfies { - assertThat(it.status.description).isEqualTo( - "Rate limit value: 0/j is incorrect." - ) - assertThat(it.status.code).isEqualTo(Status.Code.INVALID_ARGUMENT) - } + .matches { it.status.description == "Rate limit value: 0/j is incorrect." } + .matches { it.status.code == Status.Code.INVALID_ARGUMENT } } private fun createIncomingPermissions( diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt index 42248eef3..2d04d10d5 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt @@ -55,7 +55,6 @@ open class EndpointMetadataMergingTests { protected open fun callEchoServiceRepeatedly( repeat: Int, tag: String, - assertNoErrors: Boolean = true ): CallStats { val stats = CallStats(listOf(service)) envoy.egressOperations.callServiceRepeatedly( @@ -64,7 +63,7 @@ open class EndpointMetadataMergingTests { minRepeat = repeat, maxRepeat = repeat, headers = mapOf("x-service-tag" to tag), - assertNoErrors = assertNoErrors + assertNoErrors = true ) return stats } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlHttp2Test.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlHttp2Test.kt index e18707667..6a4bafd32 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlHttp2Test.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlHttp2Test.kt @@ -13,7 +13,8 @@ import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension -class AdsEnvoyControlHttp2Test : EnvoyControlHttp2Test { +class EnvoyControlHttp2Test { + companion object { @JvmField @@ -37,63 +38,40 @@ class AdsEnvoyControlHttp2Test : EnvoyControlHttp2Test { val secondEnvoy = EnvoyExtension(envoyControl, service, config = Ads) } - override fun consul() = consul - - override fun envoyControl() = envoyControl - - override fun service() = service - - override fun envoy() = envoy - - override fun secondEnvoy() = secondEnvoy -} - -interface EnvoyControlHttp2Test { - - fun consul(): ConsulExtension - - fun envoyControl(): EnvoyControlExtension - - fun service(): EchoServiceExtension - - fun envoy(): EnvoyExtension - - fun secondEnvoy(): EnvoyExtension - @Test fun `should establish http2 connection between envoys`() { // given - consul().server.operations.registerService( + consul.server.operations.registerService( name = "proxy1", - address = secondEnvoy().container.ipAddress(), + address = secondEnvoy.container.ipAddress(), port = EnvoyContainer.INGRESS_LISTENER_CONTAINER_PORT, tags = listOf("envoy") ) untilAsserted { // when - val response = envoy().egressOperations.callService("proxy1") + val response = envoy.egressOperations.callService("proxy1") assertThat(response).isOk() // then - assertDidNotUseHttp1(envoy()) - assertUsedHttp2(envoy()) + assertDidNotUseHttp1(envoy) + assertUsedHttp2(envoy) } } @Test fun `should establish http1 connection between envoy and a service by default`() { // given - consul().server.operations.registerService(service(), name = "echo") + consul.server.operations.registerService(service, name = "echo") untilAsserted { // when - val response = envoy().egressOperations.callService("echo") - assertThat(response).isOk().isFrom(service()) + val response = envoy.egressOperations.callService("echo") + assertThat(response).isOk().isFrom(service) // then - assertUsedHttp1(envoy()) - assertDidNotUseHttp2(envoy()) + assertUsedHttp1(envoy) + assertDidNotUseHttp2(envoy) } } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/MetricsDiscoveryServerCallbacksTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/MetricsDiscoveryServerCallbacksTest.kt index b496c7b13..c54d2fb67 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/MetricsDiscoveryServerCallbacksTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/MetricsDiscoveryServerCallbacksTest.kt @@ -1,6 +1,5 @@ package pl.allegro.tech.servicemesh.envoycontrol -import io.micrometer.core.instrument.MeterRegistry import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.RegisterExtension @@ -19,6 +18,7 @@ import pl.allegro.tech.servicemesh.envoycontrol.server.callbacks.MetricsDiscover import pl.allegro.tech.servicemesh.envoycontrol.server.callbacks.MetricsDiscoveryServerCallbacks.StreamType.RDS import pl.allegro.tech.servicemesh.envoycontrol.server.callbacks.MetricsDiscoveryServerCallbacks.StreamType.SDS import pl.allegro.tech.servicemesh.envoycontrol.server.callbacks.MetricsDiscoveryServerCallbacks.StreamType.UNKNOWN +import java.time.Duration class AdsMetricsDiscoveryServerCallbackTest : MetricsDiscoveryServerCallbacksTest { companion object { @@ -59,20 +59,20 @@ class AdsMetricsDiscoveryServerCallbackTest : MetricsDiscoveryServerCallbacksTes ) override fun expectedGrpcRequestsCounterValues() = mapOf( - CDS.name.toLowerCase() to isGreaterThanZero(), - EDS.name.toLowerCase() to isGreaterThanZero(), - LDS.name.toLowerCase() to isGreaterThanZero(), - RDS.name.toLowerCase() to isGreaterThanZero(), - SDS.name.toLowerCase() to isNull(), - ADS.name.toLowerCase() to isNull(), - UNKNOWN.name.toLowerCase() to isNull(), - "${CDS.name.toLowerCase()}.delta" to isNull(), - "${EDS.name.toLowerCase()}.delta" to isNull(), - "${LDS.name.toLowerCase()}.delta" to isNull(), - "${RDS.name.toLowerCase()}.delta" to isNull(), - "${SDS.name.toLowerCase()}.delta" to isNull(), - "${ADS.name.toLowerCase()}.delta" to isNull(), - "${UNKNOWN.name.toLowerCase()}.delta" to isNull() + CDS.name.lowercase() to isGreaterThanZero(), + EDS.name.lowercase() to isGreaterThanZero(), + LDS.name.lowercase() to isGreaterThanZero(), + RDS.name.lowercase() to isGreaterThanZero(), + SDS.name.lowercase() to isNull(), + ADS.name.lowercase() to isNull(), + UNKNOWN.name.lowercase() to isNull(), + "${CDS.name.lowercase()}.delta" to isNull(), + "${EDS.name.lowercase()}.delta" to isNull(), + "${LDS.name.lowercase()}.delta" to isNull(), + "${RDS.name.lowercase()}.delta" to isNull(), + "${SDS.name.lowercase()}.delta" to isNull(), + "${ADS.name.lowercase()}.delta" to isNull(), + "${UNKNOWN.name.lowercase()}.delta" to isNull() ) } @@ -105,7 +105,7 @@ class DeltaAdsMetricsDiscoveryServerCallbackTest : MetricsDiscoveryServerCallbac override fun envoy() = envoy override fun expectedGrpcConnectionsGaugeValues() = mapOf( - CDS to 0, + CDS to 1, EDS to 0, LDS to 0, RDS to 0, @@ -115,20 +115,20 @@ class DeltaAdsMetricsDiscoveryServerCallbackTest : MetricsDiscoveryServerCallbac ) override fun expectedGrpcRequestsCounterValues() = mapOf( - CDS.name.toLowerCase() to isNull(), - EDS.name.toLowerCase() to isNull(), - LDS.name.toLowerCase() to isNull(), - RDS.name.toLowerCase() to isNull(), - SDS.name.toLowerCase() to isNull(), - ADS.name.toLowerCase() to isNull(), - UNKNOWN.name.toLowerCase() to isNull(), - "${CDS.name.toLowerCase()}.delta" to isGreaterThanZero(), - "${EDS.name.toLowerCase()}.delta" to isGreaterThanZero(), - "${LDS.name.toLowerCase()}.delta" to isGreaterThanZero(), - "${RDS.name.toLowerCase()}.delta" to isGreaterThanZero(), - "${SDS.name.toLowerCase()}.delta" to isNull(), - "${ADS.name.toLowerCase()}.delta" to isNull(), - "${UNKNOWN.name.toLowerCase()}.delta" to isNull() + CDS.name.lowercase() to isNull(), + EDS.name.lowercase() to isNull(), + LDS.name.lowercase() to isNull(), + RDS.name.lowercase() to isNull(), + SDS.name.lowercase() to isNull(), + ADS.name.lowercase() to isNull(), + UNKNOWN.name.lowercase() to isNull(), + "${CDS.name.lowercase()}.delta" to isGreaterThanZero(), + "${EDS.name.lowercase()}.delta" to isGreaterThanZero(), + "${LDS.name.lowercase()}.delta" to isGreaterThanZero(), + "${RDS.name.lowercase()}.delta" to isGreaterThanZero(), + "${SDS.name.lowercase()}.delta" to isNull(), + "${ADS.name.lowercase()}.delta" to isNull(), + "${UNKNOWN.name.lowercase()}.delta" to isNull() ) } @@ -146,9 +146,7 @@ interface MetricsDiscoveryServerCallbacksTest { fun expectedGrpcRequestsCounterValues(): Map Boolean> - fun MeterRegistry.counterValue(name: String) = this.find(name).counter()?.count()?.toInt() - - fun isGreaterThanZero() = { x: Int? -> x!! > 0 } + fun isGreaterThanZero() = { x: Int? -> x != null && x > 0 } fun isNull() = { x: Int? -> x == null } @@ -159,11 +157,14 @@ interface MetricsDiscoveryServerCallbacksTest { consul().server.operations.registerService(service(), name = "echo") // expect - untilAsserted { + untilAsserted(wait = Duration.ofSeconds(5)) { expectedGrpcConnectionsGaugeValues().forEach { (type, value) -> - val metric = "grpc.connections.${type.name.toLowerCase()}" - assertThat(meterRegistry.find(metric).gauge()).isNotNull - assertThat(meterRegistry.get(metric).gauge().value().toInt()).isEqualTo(value) + val metric = "grpc.connections.${type.name.lowercase()}" + assertThat(meterRegistry.find(metric).gauge()) + .withFailMessage("Metric $metric should not be null") + .isNotNull + .withFailMessage("Value of metric $metric should be $value") + .matches { it.value().toInt() == value } } } } @@ -175,11 +176,12 @@ interface MetricsDiscoveryServerCallbacksTest { consul().server.operations.registerService(service(), name = "echo") // expect - untilAsserted { + untilAsserted(wait = Duration.ofSeconds(5)) { expectedGrpcRequestsCounterValues().forEach { (type, condition) -> - val counterValue = meterRegistry.counterValue("grpc.requests.$type") - println("$type $counterValue") - assertThat(counterValue).satisfies { condition(it) } + val metric = "grpc.requests.$type" + assertThat(meterRegistry.find(metric).counter()?.count()?.toInt()) + .withFailMessage("Metric $metric does not meet the condition") + .matches(condition) } } } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/OutgoingPermissionsTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/OutgoingPermissionsTest.kt index 52513e179..9680dfb16 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/OutgoingPermissionsTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/OutgoingPermissionsTest.kt @@ -13,7 +13,7 @@ import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtens import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension -class AdsOutgoingPermissionsTest : OutgoingPermissionsTest { +class OutgoingPermissionsTest { companion object { @@ -37,42 +37,27 @@ class AdsOutgoingPermissionsTest : OutgoingPermissionsTest { val envoy = EnvoyExtension(envoyControl, config = Ads) } - override fun consul() = consul - - override fun service() = service - - override fun envoy() = envoy -} - -interface OutgoingPermissionsTest { - - fun consul(): ConsulExtension - - fun service(): EchoServiceExtension - - fun envoy(): EnvoyExtension - @Test fun `should only allow access to resources from node_metadata_dependencies`() { // given - consul().server.operations.registerService(service(), name = "not-accessible") - consul().server.operations.registerService(service(), name = "echo") + consul.server.operations.registerService(service, name = "not-accessible") + consul.server.operations.registerService(service, name = "echo") untilAsserted { // when - val unreachableResponse = envoy().egressOperations.callService("not-accessible") - val unregisteredResponse = envoy().egressOperations.callService("unregistered") - val reachableResponse = envoy().egressOperations.callService("echo") - val reachableResponseEchoWithDomain = envoy().egressOperations.callService("echo.test.domain") - val reachableResponseEchoWithDomain2 = envoy().egressOperations.callService("echo.domain") - val reachableDomainResponse = envoy().egressOperations.callDomain("www.example.com") - val unreachableDomainResponse = envoy().egressOperations.callDomain("www.another-example.com") + val unreachableResponse = envoy.egressOperations.callService("not-accessible") + val unregisteredResponse = envoy.egressOperations.callService("unregistered") + val reachableResponse = envoy.egressOperations.callService("echo") + val reachableResponseEchoWithDomain = envoy.egressOperations.callService("echo.test.domain") + val reachableResponseEchoWithDomain2 = envoy.egressOperations.callService("echo.domain") + val reachableDomainResponse = envoy.egressOperations.callDomain("www.example.com") + val unreachableDomainResponse = envoy.egressOperations.callDomain("www.another-example.com") // then - assertThat(reachableResponse).isOk().isFrom(service()) + assertThat(reachableResponse).isOk().isFrom(service) assertThat(reachableDomainResponse).isOk() - assertThat(reachableResponseEchoWithDomain).isOk().isFrom(service()) - assertThat(reachableResponseEchoWithDomain2).isOk().isFrom(service()) + assertThat(reachableResponseEchoWithDomain).isOk().isFrom(service) + assertThat(reachableResponseEchoWithDomain2).isOk().isFrom(service) assertThat(unreachableDomainResponse).isUnreachable() assertThat(unreachableResponse).isUnreachable() assertThat(unregisteredResponse).isUnreachable() diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/RateLimitTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/RateLimitTest.kt index 29ce50986..4fe9f09e3 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/RateLimitTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/RateLimitTest.kt @@ -11,13 +11,12 @@ import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension import pl.allegro.tech.servicemesh.envoycontrol.config.service.GenericServiceExtension -import pl.allegro.tech.servicemesh.envoycontrol.config.service.HttpContainer import pl.allegro.tech.servicemesh.envoycontrol.config.service.RedisBasedRateLimitContainer import pl.allegro.tech.servicemesh.envoycontrol.config.service.RedisContainer -import pl.allegro.tech.servicemesh.envoycontrol.config.service.ServiceExtension import java.time.Duration -open class AdsRateLimitTest : RateLimitTest { +class RateLimitTest { + companion object { @JvmField @@ -45,58 +44,11 @@ open class AdsRateLimitTest : RateLimitTest { val rateLimitService = GenericServiceExtension(RedisBasedRateLimitContainer(redis.container())) } - override fun consul() = consul - - override fun envoyControl() = envoyControl - - override fun service() = service - - override fun envoy() = envoy - - override fun rateLimitService() = rateLimitService -} - -val RATE_LIMIT_CONFIG = """ - node: - metadata: - proxy_settings: - incoming: - rateLimitEndpoints: - - path: /banned - rateLimit: 0/s - - pathPrefix: /limited-5-h - rateLimit: 5/h - - pathRegex: /one/.*/three - rateLimit: 25/m - - path: /limited-30-m - rateLimit: 30/m - - path: /limited-15-s - rateLimit: 15/s - - path: /banned-for-harry-potter - clients: ["harry-potter"] - rateLimit: 0/s - - path: /banned-for-all - clients: ["*"] - rateLimit: 0/s - """.trimIndent() - -interface RateLimitTest { - - fun consul(): ConsulExtension - - fun envoyControl(): EnvoyControlExtension - - fun service(): EchoServiceExtension - - fun envoy(): EnvoyExtension - - fun rateLimitService(): ServiceExtension - @Test fun `should limit using rateLimit`() { // given - val extension = rateLimitService() - consul().server.operations.registerService(extension = extension, + val extension = rateLimitService + consul.server.operations.registerService(extension = extension, name = "ratelimit-grpc", tags = listOf("envoy"), // this turns on HTTP2 beforeRegistration = { service -> @@ -169,7 +121,7 @@ interface RateLimitTest { var counter = 0 untilAsserted(poll = poll) { counter++ - val response = envoy().ingressOperations.callLocalService(endpoint, + val response = envoy.ingressOperations.callLocalService(endpoint, clientServiceName?.let { headersOf("x-service-name", clientServiceName) } ?: headersOf()) assertThat(response.code).isEqualTo(429) @@ -178,3 +130,27 @@ interface RateLimitTest { resultFn.invoke(counter) } } + +val RATE_LIMIT_CONFIG = """ + node: + metadata: + proxy_settings: + incoming: + rateLimitEndpoints: + - path: /banned + rateLimit: 0/s + - pathPrefix: /limited-5-h + rateLimit: 5/h + - pathRegex: /one/.*/three + rateLimit: 25/m + - path: /limited-30-m + rateLimit: 30/m + - path: /limited-15-s + rateLimit: 15/s + - path: /banned-for-harry-potter + clients: ["harry-potter"] + rateLimit: 0/s + - path: /banned-for-all + clients: ["*"] + rateLimit: 0/s + """.trimIndent() diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsAndCanaryTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsAndCanaryTest.kt deleted file mode 100644 index b4deb8033..000000000 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsAndCanaryTest.kt +++ /dev/null @@ -1,35 +0,0 @@ -package pl.allegro.tech.servicemesh.envoycontrol - -import org.junit.jupiter.api.extension.RegisterExtension -import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension -import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulExtension -import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension - -class ServiceTagsAndCanaryTest : ServiceTagsAndCanaryTestBase { - companion object { - - @JvmField - @RegisterExtension - val consul = ConsulExtension() - - @JvmField - @RegisterExtension - val envoyControl = EnvoyControlExtension(consul, mapOf( - "envoy-control.envoy.snapshot.routing.service-tags.enabled" to true, - "envoy-control.envoy.snapshot.routing.service-tags.metadata-key" to "tag", - "envoy-control.envoy.snapshot.load-balancing.canary.enabled" to true, - "envoy-control.envoy.snapshot.load-balancing.canary.metadata-key" to "canary", - "envoy-control.envoy.snapshot.load-balancing.canary.metadata-value" to "1" - )) - - @JvmField - @RegisterExtension - val envoy = EnvoyExtension(envoyControl) - } - - override fun consul() = consul - - override fun envoyControl() = envoyControl - - override fun envoy() = envoy -} diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsAndCanaryTestBase.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsAndCanaryTestBase.kt index e2135383e..33c22491b 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsAndCanaryTestBase.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ServiceTagsAndCanaryTestBase.kt @@ -11,7 +11,7 @@ import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtens import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.CallStats import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension -interface ServiceTagsAndCanaryTestBase { +class ServiceTagsAndCanaryTestBase { companion object { @JvmField @@ -25,18 +25,30 @@ interface ServiceTagsAndCanaryTestBase { @JvmField @RegisterExtension val ipsumRegularService = EchoServiceExtension() - } - fun consul(): ConsulExtension + @JvmField + @RegisterExtension + val consul = ConsulExtension() - fun envoyControl(): EnvoyControlExtension + @JvmField + @RegisterExtension + val envoyControl = EnvoyControlExtension(consul, mapOf( + "envoy-control.envoy.snapshot.routing.service-tags.enabled" to true, + "envoy-control.envoy.snapshot.routing.service-tags.metadata-key" to "tag", + "envoy-control.envoy.snapshot.load-balancing.canary.enabled" to true, + "envoy-control.envoy.snapshot.load-balancing.canary.metadata-key" to "canary", + "envoy-control.envoy.snapshot.load-balancing.canary.metadata-value" to "1" + )) - fun envoy(): EnvoyExtension + @JvmField + @RegisterExtension + val envoy = EnvoyExtension(envoyControl) + } - fun registerServices() { - consul().server.operations.registerService(loremRegularService, name = "echo", tags = listOf("lorem")) - consul().server.operations.registerService(loremCanaryService, name = "echo", tags = listOf("lorem", "canary")) - consul().server.operations.registerService(ipsumRegularService, name = "echo", tags = listOf("ipsum")) + private fun registerServices() { + consul.server.operations.registerService(loremRegularService, name = "echo", tags = listOf("lorem")) + consul.server.operations.registerService(loremCanaryService, name = "echo", tags = listOf("lorem", "canary")) + consul.server.operations.registerService(ipsumRegularService, name = "echo", tags = listOf("ipsum")) } @Test @@ -104,19 +116,19 @@ interface ServiceTagsAndCanaryTestBase { assertThat(stats.ipsumRegularHits).isEqualTo(0) } - fun waitForReadyServices(vararg serviceNames: String) { + private fun waitForReadyServices(vararg serviceNames: String) { serviceNames.forEach { untilAsserted { - envoy().egressOperations.callService(it).also { + envoy.egressOperations.callService(it).also { assertThat(it).isOk() } } } } - fun callStats() = CallStats(listOf(loremCanaryService, loremRegularService, ipsumRegularService)) + private fun callStats() = CallStats(listOf(loremCanaryService, loremRegularService, ipsumRegularService)) - fun callEchoServiceRepeatedly( + private fun callEchoServiceRepeatedly( repeat: Int, tag: String? = null, canary: Boolean, @@ -126,7 +138,7 @@ interface ServiceTagsAndCanaryTestBase { val tagHeader = tag?.let { mapOf("x-service-tag" to it) } ?: emptyMap() val canaryHeader = if (canary) mapOf("x-canary" to "1") else emptyMap() - envoy().egressOperations.callServiceRepeatedly( + envoy.egressOperations.callServiceRepeatedly( service = "echo", stats = stats, minRepeat = repeat, @@ -137,10 +149,10 @@ interface ServiceTagsAndCanaryTestBase { return stats } - val CallStats.loremCanaryHits: Int + private val CallStats.loremCanaryHits: Int get() = this.hits(loremCanaryService) - val CallStats.loremRegularHits: Int + private val CallStats.loremRegularHits: Int get() = this.hits(loremRegularService) - val CallStats.ipsumRegularHits: Int + private val CallStats.ipsumRegularHits: Int get() = this.hits(ipsumRegularService) } diff --git a/envoy-control-tests/src/main/resources/envoy/bad_config.yaml b/envoy-control-tests/src/main/resources/envoy/bad_config.yaml index bbebcfee6..614e22a3f 100644 --- a/envoy-control-tests/src/main/resources/envoy/bad_config.yaml +++ b/envoy-control-tests/src/main/resources/envoy/bad_config.yaml @@ -4,10 +4,8 @@ admin: socket_address: { address: 0.0.0.0, port_value: 10000 } dynamic_resources: lds_config: - resource_api_version: V3 ads: {} cds_config: - resource_api_version: V3 ads: {} ads_config: transport_api_version: V3 @@ -19,7 +17,6 @@ node: cluster: test-cluster id: test-id metadata: - ads: true proxy_settings: incoming: endpoints: diff --git a/envoy-control-tests/src/main/resources/envoy/config_ads.yaml b/envoy-control-tests/src/main/resources/envoy/config_ads.yaml index 5eb15b086..d53bdc90f 100644 --- a/envoy-control-tests/src/main/resources/envoy/config_ads.yaml +++ b/envoy-control-tests/src/main/resources/envoy/config_ads.yaml @@ -4,10 +4,8 @@ admin: socket_address: { address: 0.0.0.0, port_value: 10000 } dynamic_resources: lds_config: - resource_api_version: V3 ads: {} cds_config: - resource_api_version: V3 ads: {} ads_config: transport_api_version: V3 @@ -20,7 +18,6 @@ node: id: test-id metadata: service_name: "echo2" - ads: true ingress_host: "0.0.0.0" ingress_port: 5001 egress_host: "0.0.0.0" diff --git a/envoy-control-tests/src/main/resources/envoy/config_ads_all_dependencies.yaml b/envoy-control-tests/src/main/resources/envoy/config_ads_all_dependencies.yaml index b7e90b3b5..442241122 100644 --- a/envoy-control-tests/src/main/resources/envoy/config_ads_all_dependencies.yaml +++ b/envoy-control-tests/src/main/resources/envoy/config_ads_all_dependencies.yaml @@ -4,10 +4,8 @@ admin: socket_address: { address: 0.0.0.0, port_value: 10000 } dynamic_resources: lds_config: - resource_api_version: V3 ads: {} cds_config: - resource_api_version: V3 ads: {} ads_config: transport_api_version: V3 @@ -19,7 +17,6 @@ node: cluster: test-cluster id: test-id metadata: - ads: true ingress_host: "0.0.0.0" ingress_port: 5001 egress_host: "0.0.0.0" diff --git a/envoy-control-tests/src/main/resources/envoy/config_ads_custom_health_check.yaml b/envoy-control-tests/src/main/resources/envoy/config_ads_custom_health_check.yaml index c3fb0006a..84042c56f 100644 --- a/envoy-control-tests/src/main/resources/envoy/config_ads_custom_health_check.yaml +++ b/envoy-control-tests/src/main/resources/envoy/config_ads_custom_health_check.yaml @@ -4,10 +4,8 @@ admin: socket_address: { address: 0.0.0.0, port_value: 10000 } dynamic_resources: lds_config: - resource_api_version: V3 ads: {} cds_config: - resource_api_version: V3 ads: {} ads_config: transport_api_version: V3 @@ -19,7 +17,6 @@ node: cluster: test-cluster id: test-id metadata: - ads: true proxy_settings: incoming: healthCheck: @@ -100,7 +97,6 @@ static_resources: rds: route_config_name: default_routes config_source: - resource_api_version: V3 ads: {} http_filters: - name: envoy.filters.http.router @@ -118,7 +114,6 @@ static_resources: rds: route_config_name: ingress_secured_routes config_source: - resource_api_version: V3 ads: {} http_filters: - name: envoy.filters.http.router diff --git a/envoy-control-tests/src/main/resources/envoy/config_ads_disabled_endpoint_permissions.yaml b/envoy-control-tests/src/main/resources/envoy/config_ads_disabled_endpoint_permissions.yaml index ddee4bdbc..5a2817bac 100644 --- a/envoy-control-tests/src/main/resources/envoy/config_ads_disabled_endpoint_permissions.yaml +++ b/envoy-control-tests/src/main/resources/envoy/config_ads_disabled_endpoint_permissions.yaml @@ -4,10 +4,8 @@ admin: socket_address: { address: 0.0.0.0, port_value: 10000 } dynamic_resources: lds_config: - resource_api_version: V3 ads: {} cds_config: - resource_api_version: V3 ads: {} ads_config: transport_api_version: V3 @@ -19,7 +17,6 @@ node: cluster: test-cluster id: test-id metadata: - ads: true ingress_host: "0.0.0.0" ingress_port: 5001 egress_host: "0.0.0.0" diff --git a/envoy-control-tests/src/main/resources/envoy/config_ads_dynamic_forward_proxy.yaml b/envoy-control-tests/src/main/resources/envoy/config_ads_dynamic_forward_proxy.yaml index 21062f4cf..7fa40b36e 100644 --- a/envoy-control-tests/src/main/resources/envoy/config_ads_dynamic_forward_proxy.yaml +++ b/envoy-control-tests/src/main/resources/envoy/config_ads_dynamic_forward_proxy.yaml @@ -4,10 +4,8 @@ admin: socket_address: { address: 0.0.0.0, port_value: 10000 } dynamic_resources: lds_config: - resource_api_version: V3 ads: {} cds_config: - resource_api_version: V3 ads: {} ads_config: transport_api_version: V3 @@ -19,7 +17,6 @@ node: cluster: test-cluster id: test-id metadata: - ads: true ingress_host: "0.0.0.0" ingress_port: 5001 egress_host: "0.0.0.0" diff --git a/envoy-control-tests/src/main/resources/envoy/config_ads_no_dependencies.yaml b/envoy-control-tests/src/main/resources/envoy/config_ads_no_dependencies.yaml index 3f34c2520..2c7f391fb 100644 --- a/envoy-control-tests/src/main/resources/envoy/config_ads_no_dependencies.yaml +++ b/envoy-control-tests/src/main/resources/envoy/config_ads_no_dependencies.yaml @@ -4,10 +4,8 @@ admin: socket_address: { address: 0.0.0.0, port_value: 10000 } dynamic_resources: lds_config: - resource_api_version: V3 ads: {} cds_config: - resource_api_version: V3 ads: {} ads_config: transport_api_version: V3 @@ -19,7 +17,6 @@ node: cluster: test-cluster id: test-id metadata: - ads: true ingress_host: "0.0.0.0" ingress_port: 5001 egress_host: "0.0.0.0" diff --git a/envoy-control-tests/src/main/resources/envoy/config_ads_static_listeners.yaml b/envoy-control-tests/src/main/resources/envoy/config_ads_static_listeners.yaml index 459f75ea4..2d694b8b0 100644 --- a/envoy-control-tests/src/main/resources/envoy/config_ads_static_listeners.yaml +++ b/envoy-control-tests/src/main/resources/envoy/config_ads_static_listeners.yaml @@ -4,10 +4,8 @@ admin: socket_address: { address: 0.0.0.0, port_value: 10000 } dynamic_resources: lds_config: - resource_api_version: V3 ads: {} cds_config: - resource_api_version: V3 ads: {} ads_config: transport_api_version: V3 @@ -19,7 +17,6 @@ node: cluster: test-cluster id: test-id metadata: - ads: true proxy_settings: outgoing: dependencies: @@ -85,7 +82,6 @@ static_resources: rds: route_config_name: default_routes config_source: - resource_api_version: V3 ads: {} http_filters: - name: envoy.filters.http.router @@ -103,7 +99,6 @@ static_resources: rds: route_config_name: ingress_secured_routes config_source: - resource_api_version: V3 ads: {} http_filters: - name: envoy.filters.http.router diff --git a/envoy-control-tests/src/main/resources/envoy/config_auth.yaml b/envoy-control-tests/src/main/resources/envoy/config_auth.yaml index b6c8c5ff8..8e9b8d660 100644 --- a/envoy-control-tests/src/main/resources/envoy/config_auth.yaml +++ b/envoy-control-tests/src/main/resources/envoy/config_auth.yaml @@ -4,10 +4,8 @@ admin: socket_address: { address: 0.0.0.0, port_value: 10000 } dynamic_resources: lds_config: - resource_api_version: V3 ads: {} cds_config: - resource_api_version: V3 ads: {} ads_config: transport_api_version: V3 @@ -20,7 +18,6 @@ node: id: test-id metadata: service_name: "SERVICE_NAME" - ads: true ingress_host: "0.0.0.0" ingress_port: 5001 egress_host: "0.0.0.0" diff --git a/envoy-control-tests/src/main/resources/envoy/config_oauth.yaml b/envoy-control-tests/src/main/resources/envoy/config_oauth.yaml index 70fea9b1f..93f7a4005 100644 --- a/envoy-control-tests/src/main/resources/envoy/config_oauth.yaml +++ b/envoy-control-tests/src/main/resources/envoy/config_oauth.yaml @@ -4,10 +4,8 @@ admin: socket_address: { address: 0.0.0.0, port_value: 10000 } dynamic_resources: lds_config: - resource_api_version: V3 ads: {} cds_config: - resource_api_version: V3 ads: {} ads_config: transport_api_version: V3 @@ -20,7 +18,6 @@ node: id: test-id metadata: service_name: "SERVICE_NAME" - ads: true ingress_host: "0.0.0.0" ingress_port: 5001 egress_host: "0.0.0.0" diff --git a/tools/envoy/envoy-template.yaml b/tools/envoy/envoy-template.yaml index 232a2ccba..f2ae27373 100644 --- a/tools/envoy/envoy-template.yaml +++ b/tools/envoy/envoy-template.yaml @@ -1,7 +1,6 @@ --- node: metadata: - ads: true service_name: docker ingress_host: "0.0.0.0" ingress_port: {{.IngressListenerPort}} From d7eb127462d4acd82aa6165a9598d1d49e6898ac Mon Sep 17 00:00:00 2001 From: Kamil Smigielski Date: Sat, 12 Nov 2022 20:23:27 +0100 Subject: [PATCH 3/7] fix tests --- .../envoycontrol/MetricsDiscoveryServerCallbacksTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/MetricsDiscoveryServerCallbacksTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/MetricsDiscoveryServerCallbacksTest.kt index c54d2fb67..cb7f96dbc 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/MetricsDiscoveryServerCallbacksTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/MetricsDiscoveryServerCallbacksTest.kt @@ -105,7 +105,7 @@ class DeltaAdsMetricsDiscoveryServerCallbackTest : MetricsDiscoveryServerCallbac override fun envoy() = envoy override fun expectedGrpcConnectionsGaugeValues() = mapOf( - CDS to 1, + CDS to 0, EDS to 0, LDS to 0, RDS to 0, From c85e8e65c9448e9f0752e0ce4fa7aa5379f38014 Mon Sep 17 00:00:00 2001 From: Kamil Smigielski Date: Sat, 12 Nov 2022 21:23:00 +0100 Subject: [PATCH 4/7] fix flaky test --- .../envoycontrol/EndpointMetadataMergingTests.kt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt index 2d04d10d5..343e011f8 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt @@ -39,7 +39,12 @@ open class EndpointMetadataMergingTests { consul.server.operations.registerService(name = "echo", extension = service, tags = listOf("ipsum")) consul.server.operations.registerService(name = "echo", extension = service, tags = listOf("lorem", "dolom")) - envoy.waitForReadyServices("echo") + //TODO: flaky test. I'm not sure why, but it fails time to time. + // Theoretically after one call service should be ready, + // but for some reason sometimes is not and returns 503. + repeat(3) { + envoy.waitForReadyServices("echo") + } // when val ipsumStats = callEchoServiceRepeatedly(repeat = 1, tag = "ipsum") From 8ba1a9d91f38dce263683deeb16e98227c791233 Mon Sep 17 00:00:00 2001 From: Kamil Smigielski Date: Sat, 12 Nov 2022 21:42:33 +0100 Subject: [PATCH 5/7] fix lint --- .../envoycontrol/EndpointMetadataMergingTests.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt index 343e011f8..2944cd5df 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt @@ -39,9 +39,9 @@ open class EndpointMetadataMergingTests { consul.server.operations.registerService(name = "echo", extension = service, tags = listOf("ipsum")) consul.server.operations.registerService(name = "echo", extension = service, tags = listOf("lorem", "dolom")) - //TODO: flaky test. I'm not sure why, but it fails time to time. - // Theoretically after one call service should be ready, - // but for some reason sometimes is not and returns 503. + // TODO: flaky test. I'm not sure why, but it fails time to time. + // Theoretically after one call service should be ready, + // but for some reason sometimes is not and returns 503. repeat(3) { envoy.waitForReadyServices("echo") } From 3417e63887b2420ffc0341038dfed67cf4a80186 Mon Sep 17 00:00:00 2001 From: Kamil Smigielski Date: Wed, 16 Nov 2022 18:23:59 +0100 Subject: [PATCH 6/7] clean docs --- docs/configuration.md | 4 +--- docs/features/permissions.md | 1 - docs/integrations/envoy.md | 10 ---------- docs/performance.md | 9 --------- 4 files changed, 1 insertion(+), 23 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 04f586a49..74e57d1ca 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -79,9 +79,7 @@ Property **envoy-control.envoy.snapshot.routes.status.endpoints** | List of endpoints with path or prefix of status routes | /status **envoy-control.envoy.snapshot.routes.status.create-virtual-cluster** | Create virtual cluster for status route | false **envoy-control.envoy.snapshot.state-sample-duration** | Duration of state sampling (this is used to prevent surges in consul events overloading control plane) | 1s -**envoy-control.envoy.snapshot.xds-cluster-name** | Name of cluster for xDS operations | envoy-control-xds -**envoy-control.envoy.snapshot.enabled-communication-modes.ads** | Enable or disable support for ADS communication mode | true -**envoy-control.envoy.snapshot.enabled-communication-modes.xds** | Enable or disable support for XDS communication mode | true +**envoy-control.envoy.snapshot.xds-cluster-name** | Name of cluster for xDS operations | envoy-control-xds | true **envoy-control.envoy.snapshot.should-send-missing-endpoints** | Enable sending missing Endpoints - when Envoy requests for not existing cluster in snapshot control-plane will respond with empty Endpoint definition | false **envoy-control.envoy.snapshot.cluster-name** | Dynamic forward proxy cluster name | dynamic_forward_proxy_cluster **envoy-control.envoy.snapshot.dns-lookup-family** | DNS lookup address family | V4_ONLY diff --git a/docs/features/permissions.md b/docs/features/permissions.md index 06ee90807..8ad31572a 100644 --- a/docs/features/permissions.md +++ b/docs/features/permissions.md @@ -16,7 +16,6 @@ An example configuration: ```yaml metadata: - ads: true proxy_settings: outgoing: dependencies: diff --git a/docs/integrations/envoy.md b/docs/integrations/envoy.md index d0c09f327..26c3c3b20 100644 --- a/docs/integrations/envoy.md +++ b/docs/integrations/envoy.md @@ -24,16 +24,6 @@ and the IP can be put in `x-envoy-original-dst-host` header (`x-envoy-original-d By default, Envoy will respond with `404` status code when it receives a request for a cluster that does not exist. The behavior is changed so that the `503` status code is returned. -## ADS Support - -By default, the xDS is used instead of -[Aggregated Discovery Service](https://www.envoyproxy.io/docs/envoy/latest/configuration/overview/xds_api#aggregated-discovery-service) -(ADS). To use ADS for given node put the -``` -ads: true -``` -in Envoy metadata config. Envoy Control will pick it up and use ADS for this node. - ## Outlier detection You can configure global diff --git a/docs/performance.md b/docs/performance.md index bc9e649ea..724822062 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -20,15 +20,6 @@ Additionally, the network usage is significantly higher. With 1,000 clusters, we Assuming that a new snapshot is generated every second. 1,000 Envoys with 1,000 clusters can generate a load of 300 MB/s. When following only a few services, the snapshot is about 5 KB and it's sent much less frequently. -### Use ADS - -With xDS, Envoy set up a gRPC stream to Envoy Control per cluster. Let's say there are 1,000 Envoys and 1,000 clusters. -Envoy Control will have to handle 1,000,000 open gRPC streams. This puts pressure on memory, which converts to more -frequent GC runs and higher CPU usage. - -With ADS, each Envoy sets up a single gRPC stream for all clusters. With 1,000 Envoys, there are 1,000 streams which -reduces memory usage dramatically. - ### Sampling Envoy Control by default follows changes from the discovery service, batches them and sends to Envoys at most once every second. From 6a9f16d5e2926fac0b119186516e77b8c77ff45f Mon Sep 17 00:00:00 2001 From: Kamil Smigielski Date: Fri, 17 Feb 2023 11:41:21 +0100 Subject: [PATCH 7/7] RBAC logs - add if token is present, add posibility to specify custom headres to log --- .../resources/lua/ingress_rbac_logging.lua | 182 ++++++++++-------- .../lua_spec/ingress_rbac_logging_spec.lua | 159 ++++++++++----- 2 files changed, 218 insertions(+), 123 deletions(-) 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 index c526d5b0a..5ff3470ff 100644 --- a/envoy-control-core/src/main/resources/lua/ingress_rbac_logging.lua +++ b/envoy-control-core/src/main/resources/lua/ingress_rbac_logging.lua @@ -1,62 +1,70 @@ function envoy_on_request(handle) - local path = handle:headers():get(":path") - local method = handle:headers():get(":method") - local authority = handle:headers():get(":authority") or "" - local lua_destination_authority = handle:headers():get("x-lua-destination-authority") or "" - local xff_header = handle:headers():get("x-forwarded-for") - local metadata = handle:streamInfo():dynamicMetadata() - local client_identity_header_names = handle:metadata():get("client_identity_headers") or {} - local clients_allowed_to_all_endpoints = handle:metadata():get("clients_allowed_to_all_endpoints") or {} - local request_id_header_names = handle:metadata():get("request_id_headers") or {} - local request_id = first_header_value_from_list(request_id_header_names, handle) - local trusted_header_name = handle:metadata():get("trusted_client_identity_header") or "" - local client_name = "" + local metadata = handle:metadata() + local client_identity_header_names = metadata:get('client_identity_headers') or {} + local clients_allowed_to_all_endpoints = metadata:get('clients_allowed_to_all_endpoints') or {} + local request_id_header_names = metadata:get('request_id_headers') or {} + local trusted_header_name = metadata:get('trusted_client_identity_header') or '' + + local headers = handle:headers() + local path = headers:get(':path') + local method = headers:get(':method') + local authority = headers:get(':authority') or '' + local lua_destination_authority = headers:get('x-lua-destination-authority') or '' + local xff_header = headers:get('x-forwarded-for') + + local request_id = first_header_value_from_list(request_id_header_names, headers) + local client_name = '' local allowed_client = false local trusted_client = false - if trusted_header_name ~= "" then - client_name = handle:headers():get(trusted_header_name) or "" + if trusted_header_name ~= '' then + client_name = headers:get(trusted_header_name) or '' allowed_client = is_allowed_client(client_name, clients_allowed_to_all_endpoints) - if client_name ~= "" then + if client_name ~= '' then trusted_client = true end end - if client_name == "" then - client_name = first_header_value_from_list(client_identity_header_names, handle) + if client_name == '' then + client_name = first_header_value_from_list(client_identity_header_names, headers) allowed_client = is_allowed_client(client_name, clients_allowed_to_all_endpoints) - if trusted_header_name ~= "" and client_name ~= "" and handle:connection():ssl() ~= nil then - client_name = client_name .. " (not trusted)" + if trusted_header_name ~= '' and client_name ~= '' and handle:connection():ssl() ~= nil then + client_name = client_name .. ' (not trusted)' end end - metadata:set("envoy.filters.http.lua", "request.info.path", path) - metadata:set("envoy.filters.http.lua", "request.info.method", method) - metadata:set("envoy.filters.http.lua", "request.info.xff_header", xff_header) - metadata:set("envoy.filters.http.lua", "request.info.client_name", client_name) - metadata:set("envoy.filters.http.lua", "request.info.trusted_client", trusted_client) - metadata:set("envoy.filters.http.lua", "request.info.allowed_client", allowed_client) - metadata:set("envoy.filters.http.lua", "request.info.request_id", request_id) - metadata:set("envoy.filters.http.lua", "request.info.authority", authority) - metadata:set("envoy.filters.http.lua", "request.info.lua_destination_authority", lua_destination_authority) + local dynamic_metadata = handle:streamInfo():dynamicMetadata() + dynamic_metadata:set('envoy.filters.http.lua', 'request.info.path', path) + dynamic_metadata:set('envoy.filters.http.lua', 'request.info.method', method) + dynamic_metadata:set('envoy.filters.http.lua', 'request.info.xff_header', xff_header) + dynamic_metadata:set('envoy.filters.http.lua', 'request.info.client_name', client_name) + dynamic_metadata:set('envoy.filters.http.lua', 'request.info.trusted_client', trusted_client) + dynamic_metadata:set('envoy.filters.http.lua', 'request.info.allowed_client', allowed_client) + dynamic_metadata:set('envoy.filters.http.lua', 'request.info.request_id', request_id) + dynamic_metadata:set('envoy.filters.http.lua', 'request.info.authority', authority) + dynamic_metadata:set('envoy.filters.http.lua', 'request.info.lua_destination_authority', lua_destination_authority) + local headers_to_log = metadata:get('rbac_headers_to_log') or {} + for _, header in ipairs(headers_to_log) do + dynamic_metadata:set('envoy.filters.http.lua', 'request.info.headers.'..header, headers:get(header)) + end end -function first_header_value_from_list(header_list, handle) +function first_header_value_from_list(header_list, headers) for _,h in ipairs(header_list) do - local value = handle:headers():get(h) or "" - if value ~= "" then + local value = headers:get(h) or '' + if value ~= '' then return value end end - return "" + return '' end function is_allowed_client(client_name, clients_allowed_to_all_endpoints) - if client_name == "" then + if client_name == '' then return false end for _,h in ipairs(clients_allowed_to_all_endpoints) do - if h ~= "" and client_name == h then + if h ~= '' and client_name == h then return true end end @@ -65,58 +73,80 @@ function is_allowed_client(client_name, clients_allowed_to_all_endpoints) end function envoy_on_response(handle) - local rbacMetadata = handle:streamInfo():dynamicMetadata():get("envoy.filters.http.rbac") or {} - local is_shadow_denied = (rbacMetadata["shadow_engine_result"] or "") == "denied" + local dynamic_metadata = handle:streamInfo():dynamicMetadata() + local rbacMetadata = dynamic_metadata:get('envoy.filters.http.rbac') or {} + local is_shadow_denied = (rbacMetadata['shadow_engine_result'] or '') == 'denied' if is_shadow_denied then - local lua_metadata = handle:streamInfo():dynamicMetadata():get("envoy.filters.http.lua") or {} - local upstream_request_time = handle:headers():get("x-envoy-upstream-service-time") - local status_code = handle:headers():get(":status") - local rbac_action = "shadow_denied" - if upstream_request_time == nil and status_code == "403" then - rbac_action = "denied" + local headers = handle:headers() + local lua_metadata = dynamic_metadata:get('envoy.filters.http.lua') or {} + local jwt_status = (dynamic_metadata:get('envoy.filters.http.header_to_metadata') or {})['jwt-status'] or 'missing' + local upstream_request_time = headers:get('x-envoy-upstream-service-time') + local status_code = headers:get(':status') + local rbac_action = 'shadow_denied' + if upstream_request_time == nil and status_code == '403' then + rbac_action = 'denied' end - log_request(lua_metadata, handle, rbac_action) + log_request(handle, lua_metadata, jwt_status, rbac_action) end end -function log_request(lua_metadata, handle, rbac_action) - local client_name = lua_metadata["request.info.client_name"] or "" - local trusted_client = lua_metadata["request.info.trusted_client"] or false - local path = lua_metadata["request.info.path"] or "" - local protocol = handle:connection():ssl() == nil and "http" or "https" - local method = lua_metadata["request.info.method"] or "" - local xff_header = lua_metadata["request.info.xff_header"] or "" - local source_ip = string.match(xff_header, '[^,]+$') or "" - local request_id = lua_metadata["request.info.request_id"] or "" - local status_code = handle:headers():get(":status") or "0" - local allowed_client = lua_metadata["request.info.allowed_client"] or false - local authority = lua_metadata["request.info.authority"] or "" - local lua_destination_authority = lua_metadata["request.info.lua_destination_authority"] or "" - handle:logInfo("\nINCOMING_PERMISSIONS { \"method\": \""..method.. - "\", \"path\": \""..path.. - "\", \"clientIp\": \""..source_ip.. - "\", \"clientName\": \""..escape(client_name).. - "\", \"trustedClient\": "..tostring(trusted_client).. - ", \"authority\": \""..escape(authority).. - "\", \"luaDestinationAuthority\": \""..escape(lua_destination_authority).. - "\", \"clientAllowedToAllEndpoints\": "..tostring(allowed_client).. - ", \"protocol\": \""..protocol.. - "\", \"requestId\": \""..escape(request_id).. - "\", \"statusCode\": "..status_code.. - ", \"rbacAction\": \""..rbac_action.."\" }") +function log_request(handle, lua_metadata, jwt_status, rbac_action) + local client_name = lua_metadata['request.info.client_name'] or '' + local trusted_client = lua_metadata['request.info.trusted_client'] or false + local path = lua_metadata['request.info.path'] or '' + local protocol = handle:connection():ssl() == nil and 'http' or 'https' + local method = lua_metadata['request.info.method'] or '' + local xff_header = lua_metadata['request.info.xff_header'] or '' + local source_ip = string.match(xff_header, '[^,]+$') or '' + local request_id = lua_metadata['request.info.request_id'] or '' + local status_code = handle:headers():get(':status') or '0' + local allowed_client = lua_metadata['request.info.allowed_client'] or false + local authority = lua_metadata['request.info.authority'] or '' + local lua_destination_authority = lua_metadata['request.info.lua_destination_authority'] or '' + + local message = { + '\nINCOMING_PERMISSIONS {"method":"', method, + '","path":"', path, + '","clientIp":"', source_ip, + '","clientName":"', escape(client_name), + '","trustedClient":', tostring(trusted_client), + ',"authority":"', escape(authority), + '","luaDestinationAuthority":"', escape(lua_destination_authority), + '","clientAllowedToAllEndpoints":', tostring(allowed_client), + ',"protocol":"', protocol, + '","requestId":"', escape(request_id), + '","statusCode":', status_code, + ',"rbacAction":"', rbac_action, '"' , + ',"jwtTokenStatus":"' , jwt_status, '"', + } + + local headers_to_log = handle:metadata():get('rbac_headers_to_log') or {} + for _, header in ipairs(headers_to_log) do + local value = lua_metadata['request.info.headers.'..header] + if value then + table.insert(message, ',"') + table.insert(message, header) + table.insert(message, '":"') + table.insert(message, value) + table.insert(message, '"') + end + end + table.insert(message, '}') + + handle:logInfo(table.concat(message, '')) end escapeList = { - ["\\"] = "\\\\", - ["\""] = "\\\"", - ["\b"] = "\\b", - ["\f"] = "\\f", - ["\n"] = "\\n", - ["\r"] = "\\r", - ["\t"] = "\\t", + ['\\'] = '\\\\', + ['\"'] = '\\\"', + ['\b'] = '\\b', + ['\f'] = '\\f', + ['\n'] = '\\n', + ['\r'] = '\\r', + ['\t'] = '\\t', } function escape(val) - return string.gsub(val, "[\\\"\b\f\n\r\t]", escapeList) + return string.gsub(val, '[\\\"\b\f\n\r\t]', escapeList) end diff --git a/envoy-control-tests/src/main/resources/lua_spec/ingress_rbac_logging_spec.lua b/envoy-control-tests/src/main/resources/lua_spec/ingress_rbac_logging_spec.lua index 212cf3d49..f8a4ed494 100644 --- a/envoy-control-tests/src/main/resources/lua_spec/ingress_rbac_logging_spec.lua +++ b/envoy-control-tests/src/main/resources/lua_spec/ingress_rbac_logging_spec.lua @@ -1,23 +1,32 @@ -require("ingress_rbac_logging") +require('ingress_rbac_logging') local _ = match._ local contains = function(substring) return match.matches(substring, nil, true) end -local function formatLog(method, path, source_ip, client_name, protocol, request_id, status_code, trusted_client, allowed_client, rbac_action, authority, lua_authority) - return "\nINCOMING_PERMISSIONS { \"method\": \""..method.. - "\", \"path\": \""..path.. - "\", \"clientIp\": \""..source_ip.. - "\", \"clientName\": \""..escape(client_name).. - "\", \"trustedClient\": "..tostring(trusted_client).. - ", \"authority\": \""..escape(authority).. - "\", \"luaDestinationAuthority\": \""..escape(lua_authority).. - "\", \"clientAllowedToAllEndpoints\": "..tostring(allowed_client).. - ", \"protocol\": \""..protocol.. - "\", \"requestId\": \""..escape(request_id).. - "\", \"statusCode\": "..status_code.. - ", \"rbacAction\": \""..rbac_action.."\" }" +local function formatLog(method, path, source_ip, client_name, protocol, request_id, status_code, trusted_client, allowed_client, rbac_action, authority, lua_authority, jwt_token_status, headers_to_log) + local message = "\nINCOMING_PERMISSIONS {\"method\":\""..method.. + "\",\"path\":\""..path.. + "\",\"clientIp\":\""..source_ip.. + "\",\"clientName\":\""..escape(client_name).. + "\",\"trustedClient\":"..tostring(trusted_client).. + ",\"authority\":\""..escape(authority).. + "\",\"luaDestinationAuthority\":\""..escape(lua_authority).. + "\",\"clientAllowedToAllEndpoints\":"..tostring(allowed_client).. + ",\"protocol\":\""..protocol.. + "\",\"requestId\":\""..escape(request_id).. + "\",\"statusCode\":"..status_code.. + ",\"rbacAction\":\""..rbac_action.. + "\",\"jwtTokenStatus\":\""..jwt_token_status.. "\"" + + if headers_to_log then + for k,v in pairs(headers_to_log) do + message = message..',"'..k..'":"'..v..'"' + end + end + + return message..'}' end -local function handlerMock(headers, dynamic_metadata, https, filter_metadata) +local function handlerMock(headers, dynamic_metadata, https, filter_metadata, logs) local metadata_mock = mock({ set = function() end, get = function(_, key) return dynamic_metadata[key] end @@ -331,18 +340,38 @@ describe("envoy_on_request:", function() assert.spy(dynamic_metadata.set).was_called_with(_, "envoy.filters.http.lua", "request.info.path", "/path") assert.spy(dynamic_metadata.set).was_called_with(_, "envoy.filters.http.lua", "request.info.method", "GET") end) + + it('should set dynamic metadata for custom headers', function() + -- given + local headers = { + ['test-header'] = 'header-value' + } + local filter_metadata = { + ['rbac_headers_to_log'] = {'test-header'} + } + + local handle = handlerMock(headers, {}, nil, filter_metadata) + local metadata = handle:streamInfo():dynamicMetadata() + + -- when + envoy_on_request(handle) + + -- then + assert.spy(metadata.set).was_called_with(_, 'envoy.filters.http.lua', 'request.info.headers.test-header', 'header-value') + end) end) describe("envoy_on_response:", function() local headers - local metadata + local dynamic_metadata local ssl + local metadata before_each(function () headers = { [':status'] = '403' } - metadata = { + dynamic_metadata = { ['envoy.filters.http.rbac'] = { ['shadow_engine_result'] = 'denied' }, @@ -357,13 +386,14 @@ describe("envoy_on_response:", function() } } ssl = true + metadata = {} end) describe("should log unauthorized requests:", function () it("https request", function () -- given - local handle = handlerMock(headers, metadata, ssl) + local handle = handlerMock(headers, dynamic_metadata, ssl, metadata) -- when envoy_on_response(handle) @@ -381,7 +411,8 @@ describe("envoy_on_response:", function() false, "denied", "authority", - "lua_authority" + "lua_authority", + "missing" )) assert.spy(handle.logInfo).was_called(1) end) @@ -389,7 +420,7 @@ describe("envoy_on_response:", function() it("http request", function () -- given ssl = false - local handle = handlerMock(headers, metadata, ssl) + local handle = handlerMock(headers, dynamic_metadata, ssl, metadata) -- when envoy_on_response(handle) @@ -407,7 +438,8 @@ describe("envoy_on_response:", function() false, "denied", "authority", - "lua_authority" + "lua_authority", + "missing" )) assert.spy(handle.logInfo).was_called(1) end) @@ -415,7 +447,7 @@ describe("envoy_on_response:", function() it("as logged when status code is different than 403", function () -- given headers[':status'] = '503' - local handle = handlerMock(headers, metadata, ssl) + local handle = handlerMock(headers, dynamic_metadata, ssl, metadata) -- when envoy_on_response(handle) @@ -433,7 +465,8 @@ describe("envoy_on_response:", function() false, "shadow_denied", "authority", - "lua_authority" + "lua_authority", + "missing" )) assert.spy(handle.logInfo).was_called(1) end) @@ -442,7 +475,7 @@ describe("envoy_on_response:", function() -- given headers[':status'] = '200' headers['x-envoy-upstream-service-time'] = '10' - local handle = handlerMock(headers, metadata, ssl) + local handle = handlerMock(headers, dynamic_metadata, ssl, metadata) -- when envoy_on_response(handle) @@ -460,16 +493,17 @@ describe("envoy_on_response:", function() false, "shadow_denied", "authority", - "lua_authority" + "lua_authority", + "missing" )) assert.spy(handle.logInfo).was_called(1) end) it("request with no lua filter metadata fields saved", function () -- given - metadata['envoy.filters.http.lua'] = {} + dynamic_metadata['envoy.filters.http.lua'] = {} headers = {} - local handle = handlerMock(headers, metadata, ssl) + local handle = handlerMock(headers, dynamic_metadata, ssl, metadata) -- when envoy_on_response(handle) @@ -487,16 +521,17 @@ describe("envoy_on_response:", function() false, "shadow_denied", "", - "" + "", + "missing" )) assert.spy(handle.logInfo).was_called(1) end) it("request with no lua filter metadata saved", function () -- given - metadata['envoy.filters.http.lua'] = nil + dynamic_metadata['envoy.filters.http.lua'] = nil headers = {} - local handle = handlerMock(headers, metadata, ssl) + local handle = handlerMock(headers, dynamic_metadata, ssl, metadata) -- when envoy_on_response(handle) @@ -514,15 +549,16 @@ describe("envoy_on_response:", function() false, "shadow_denied", "", - "" + "", + "missing" )) assert.spy(handle.logInfo).was_called(1) end) it("request with empty path", function () -- given - metadata['envoy.filters.http.lua']['request.info.path'] = '' - local handle = handlerMock(headers, metadata, ssl) + dynamic_metadata['envoy.filters.http.lua']['request.info.path'] = '' + local handle = handlerMock(headers, dynamic_metadata, ssl, metadata) -- when envoy_on_response(handle) @@ -540,7 +576,37 @@ describe("envoy_on_response:", function() false, "denied", "authority", - "lua_authority" + "lua_authority", + "missing" + )) + assert.spy(handle.logInfo).was_called(1) + end) + + it('with populated header values', function() + -- given + dynamic_metadata['envoy.filters.http.lua']['request.info.headers.test-header'] = 'test-value' + metadata['rbac_headers_to_log'] = { 'test-header' } + local handle = handlerMock(headers, dynamic_metadata, ssl, metadata) + + -- when + envoy_on_response(handle) + + -- then + assert.spy(handle.logInfo).was_called_with(_, formatLog( + "POST", + "/path?query=val", + "127.1.1.3", + "service-first", + "https", + "", + "403", + false, + false, + "denied", + "authority", + "lua_authority", + "missing", + {['test-header'] = 'test-value'} )) assert.spy(handle.logInfo).was_called(1) end) @@ -550,10 +616,10 @@ describe("envoy_on_response:", function() it("with globally allowed client", function () -- given - metadata['envoy.filters.http.rbac']['shadow_engine_result'] = 'denied' - metadata['envoy.filters.http.lua']['request.info.allowed_client'] = true + dynamic_metadata['envoy.filters.http.rbac']['shadow_engine_result'] = 'denied' + dynamic_metadata['envoy.filters.http.lua']['request.info.allowed_client'] = true headers['x-envoy-upstream-service-time'] = '10' - local handle = handlerMock(headers, metadata, ssl) + local handle = handlerMock(headers, dynamic_metadata, ssl, metadata) -- when envoy_on_response(handle) @@ -571,7 +637,8 @@ describe("envoy_on_response:", function() true, "shadow_denied", "authority", - "lua_authority" + "lua_authority", + "missing" )) assert.spy(handle.logInfo).was_called(1) end) @@ -581,9 +648,8 @@ describe("envoy_on_response:", function() it("request with no rbac metadata", function() -- given - metadata = {} - local handle = handlerMock(headers, metadata, ssl) - local metadataMock = handle:streamInfo():dynamicMetadata() + dynamic_metadata = {} + local handle = handlerMock(headers, dynamic_metadata, ssl, metadata) -- when envoy_on_response(handle) @@ -594,9 +660,8 @@ describe("envoy_on_response:", function() it("authorized request", function() -- given - metadata['envoy.filters.http.rbac']['shadow_engine_result'] = 'allowed' - local handle = handlerMock(headers, metadata, ssl) - local metadataMock = handle:streamInfo():dynamicMetadata() + dynamic_metadata['envoy.filters.http.rbac']['shadow_engine_result'] = 'allowed' + local handle = handlerMock(headers, dynamic_metadata, ssl, metadata) -- when envoy_on_response(handle) @@ -621,14 +686,14 @@ describe("envoy_on_response:", function() it("'"..xff.."' -> '"..expected_client_ip.."'", function () -- given - metadata['envoy.filters.http.lua']['request.info.xff_header'] = xff - local handle = handlerMock(headers, metadata, ssl) + dynamic_metadata['envoy.filters.http.lua']['request.info.xff_header'] = xff + local handle = handlerMock(headers, dynamic_metadata, ssl, metadata) -- when envoy_on_response(handle) -- then - assert.spy(handle.logInfo).was_called_with(_, contains("\"clientIp\": \""..expected_client_ip.."\"")) + assert.spy(handle.logInfo).was_called_with(_, contains('"clientIp":"'..expected_client_ip..'"')) end) end end)