Skip to content

Commit

Permalink
Add service_id to filter metadata (#428)
Browse files Browse the repository at this point in the history
* Add service_id to filter metadata
  • Loading branch information
uladzislau-dziuba committed Aug 14, 2024
1 parent fe83881 commit 95ed004
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 33 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
Lists all changes with user impact.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).

## [0.20.20]
### Changed
- Added service_id property to filter metadata

## [0.20.19]
### Changed
- Added http compression filter properties to node metadata
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import pl.allegro.tech.servicemesh.envoycontrol.snapshot.AccessLogFiltersPropert
sealed class Group {
abstract val communicationMode: CommunicationMode
abstract val serviceName: String
abstract val serviceId: Int?
abstract val discoveryServiceName: String?
abstract val proxySettings: ProxySettings
abstract val pathNormalizationConfig: PathNormalizationConfig
Expand All @@ -15,6 +16,7 @@ sealed class Group {
data class ServicesGroup(
override val communicationMode: CommunicationMode,
override val serviceName: String = "",
override val serviceId: Int? = null,
override val discoveryServiceName: String? = null,
override val proxySettings: ProxySettings = ProxySettings(),
override val pathNormalizationConfig: PathNormalizationConfig = PathNormalizationConfig(),
Expand All @@ -25,12 +27,13 @@ data class ServicesGroup(
data class AllServicesGroup(
override val communicationMode: CommunicationMode,
override val serviceName: String = "",
override val serviceId: Int? = null,
override val discoveryServiceName: String? = null,
override val proxySettings: ProxySettings = ProxySettings(),
override val pathNormalizationConfig: PathNormalizationConfig = PathNormalizationConfig(),
override val listenersConfig: ListenersConfig? = null,
override val compressionConfig: CompressionConfig = CompressionConfig(),
) : Group()
) : Group()

data class PathNormalizationConfig(
val normalizationEnabled: Boolean? = null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ class MetadataNodeGroup(
private fun createV3Group(node: NodeV3): Group {
val nodeMetadata = NodeMetadata(node.metadata, properties)
val serviceName = serviceName(nodeMetadata)
val serviceId = nodeMetadata.serviceId
val discoveryServiceName = nodeMetadata.discoveryServiceName
val proxySettings = proxySettings(nodeMetadata)
val listenersConfig = createListenersConfig(node.id, node.metadata, node.userAgentBuildVersion)
Expand All @@ -182,6 +183,7 @@ class MetadataNodeGroup(
AllServicesGroup(
nodeMetadata.communicationMode,
serviceName,
serviceId,
discoveryServiceName,
proxySettings,
nodeMetadata.pathNormalizationConfig,
Expand All @@ -192,6 +194,7 @@ class MetadataNodeGroup(
ServicesGroup(
nodeMetadata.communicationMode,
serviceName,
serviceId,
discoveryServiceName,
proxySettings,
nodeMetadata.pathNormalizationConfig,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class NodeMetadata(metadata: Struct, properties: SnapshotProperties) {
.fieldsMap["service_name"]
?.stringValue

val serviceId: Int? = metadata.fieldsMap["service_id"]?.numberValue?.toInt()

val discoveryServiceName: String? = metadata
.fieldsMap["discovery_service_name"]
?.stringValue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class LuaFilterFactory(private val snapshotProperties: SnapshotProperties) {
)
),
"service_name" to StringPropertyLua(group.serviceName),
"service_id" to StringPropertyLua(group.serviceId?.toString().orEmpty()),
"discovery_service_name" to StringPropertyLua(group.discoveryServiceName ?: ""),
"rbac_headers_to_log" to ListPropertyLua(
snapshotProperties.incomingPermissions.headersToLogInRbac.map(::StringPropertyLua)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -404,15 +404,15 @@ class EnvoySnapshotFactoryTest {
false -> null
}
return ServicesGroup(
mode,
serviceName,
discoveryServiceName,
ProxySettings().with(
communicationMode = mode,
serviceName = serviceName,
discoveryServiceName = discoveryServiceName,
proxySettings = ProxySettings().with(
serviceDependencies = serviceDependencies(*dependencies),
rateLimitEndpoints = rateLimitEndpoints
),
PathNormalizationConfig(),
listenersConfig
pathNormalizationConfig = PathNormalizationConfig(),
listenersConfig = listenersConfig
)
}

Expand All @@ -430,15 +430,15 @@ class EnvoySnapshotFactoryTest {
false -> null
}
return AllServicesGroup(
mode,
serviceName,
discoveryServiceName,
ProxySettings().with(
communicationMode = mode,
serviceName = serviceName,
discoveryServiceName = discoveryServiceName,
proxySettings = ProxySettings().with(
serviceDependencies = serviceDependencies(*dependencies),
defaultServiceSettings = defaultServiceSettings
),
PathNormalizationConfig(),
listenersConfig
pathNormalizationConfig = PathNormalizationConfig(),
listenersConfig = listenersConfig
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,33 @@ class MetadataNodeGroupTest {
)
}

@Test
fun `should set serviceId to group if present`() {
// given
val expectedServiceId = 777
val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true))
val node = nodeV3(serviceId = expectedServiceId)

// when
val group = nodeGroup.hash(node)

// then
assertThat(group.serviceId).isEqualTo(expectedServiceId)
}

@Test
fun `should not set serviceId to group if not present`() {
// given
val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true))
val node = nodeV3()

// when
val group = nodeGroup.hash(node)

// then
assertThat(group.serviceId).isNull()
}

@Test
fun `should not include service settings when incoming permissions are disabled`() {
// given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ fun nodeV3(
serviceDependencies: Set<String> = emptySet(),
ads: Boolean? = null,
serviceName: String? = null,
serviceId: Int? = null,
discoveryServiceName: String? = null,
incomingSettings: Boolean = false,
clients: List<String> = listOf("client1"),
Expand All @@ -30,6 +31,8 @@ fun nodeV3(
meta.putFields("service_name", string(serviceName))
}

serviceId?.let { meta.putFields("service_id", integer(serviceId)) }

discoveryServiceName?.let {
meta.putFields("discovery_service_name", string(discoveryServiceName))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,43 @@ internal class LuaFilterFactoryTest {
assertThat(givenDiscoveryServiceName).isEqualTo(expectedDiscoveryServiceName)
}

@Test
fun `should create filter metadata with serviceId`() {
// given
val expectedServiceId = 777
val group = ServicesGroup(communicationMode = CommunicationMode.XDS, serviceId = expectedServiceId)
val luaFilterFactory = LuaFilterFactory(properties)

// when
val metadata = luaFilterFactory.ingressScriptsMetadata(group, currentZone = "dc1")

val actualServiceId = metadata
.getFilterMetadataOrThrow("envoy.filters.http.lua")
.getFieldsOrThrow("service_id")
.stringValue

// then
assertThat(actualServiceId).isEqualTo(expectedServiceId.toString())
}

@Test
fun `should create filter metadata with empty serviceId`() {
// given
val group = ServicesGroup(communicationMode = CommunicationMode.XDS)
val luaFilterFactory = LuaFilterFactory(properties)

// when
val metadata = luaFilterFactory.ingressScriptsMetadata(group, currentZone = "dc1")

val actualServiceId = metadata
.getFilterMetadataOrThrow("envoy.filters.http.lua")
.getFieldsOrThrow("service_id")
.stringValue

// then
assertThat(actualServiceId).isEmpty()
}

@Test
fun `should create metadata with given customMetadata`() {
// given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,10 @@ internal class EnvoyIngressRoutesFactoryTest {
)
)
val group = ServicesGroup(
CommunicationMode.XDS,
"service_1",
"service_1",
proxySettingsOneEndpoint
communicationMode = CommunicationMode.XDS,
serviceName = "service_1",
discoveryServiceName = "service_1",
proxySettings = proxySettingsOneEndpoint
)

// when
Expand Down Expand Up @@ -203,10 +203,10 @@ internal class EnvoyIngressRoutesFactoryTest {
)
)
val group = ServicesGroup(
CommunicationMode.XDS,
"service_1",
"service_1",
proxySettingsOneEndpoint
communicationMode = CommunicationMode.XDS,
serviceName = "service_1",
discoveryServiceName = "service_1",
proxySettings = proxySettingsOneEndpoint
)

// when
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ fun createServicesGroup(
listenersConfig: ListenersConfig? = createListenersConfig(snapshotProperties)
): ServicesGroup {
return ServicesGroup(
mode,
serviceName,
discoveryServiceName,
ProxySettings().with(
communicationMode = mode,
serviceName = serviceName,
discoveryServiceName = discoveryServiceName,
proxySettings = ProxySettings().with(
serviceDependencies = serviceDependencies(*dependencies),
rateLimitEndpoints = rateLimitEndpoints
),
PathNormalizationConfig(),
listenersConfig
pathNormalizationConfig = PathNormalizationConfig(),
listenersConfig = listenersConfig
)
}

Expand All @@ -50,15 +50,15 @@ fun createAllServicesGroup(
false -> null
}
return AllServicesGroup(
mode,
serviceName,
discoveryServiceName,
ProxySettings().with(
communicationMode = mode,
serviceName = serviceName,
discoveryServiceName = discoveryServiceName,
proxySettings = ProxySettings().with(
serviceDependencies = serviceDependencies(*dependencies),
defaultServiceSettings = defaultServiceSettings
),
PathNormalizationConfig(),
listenersConfig
pathNormalizationConfig = PathNormalizationConfig(),
listenersConfig = listenersConfig
)
}

Expand Down

0 comments on commit 95ed004

Please sign in to comment.