Change type for serviceId

allegro · Aug 9, 2024 · 486cdb6 · 486cdb6
1 parent 2a5c800
commit 486cdb6
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 11 deletions.
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
@@ -5,7 +5,7 @@ import pl.allegro.tech.servicemesh.envoycontrol.snapshot.AccessLogFiltersPropert
 sealed class Group {
     abstract val communicationMode: CommunicationMode
     abstract val serviceName: String
-    abstract val serviceId: String?
+    abstract val serviceId: Int?
     abstract val discoveryServiceName: String?
     abstract val proxySettings: ProxySettings
     abstract val pathNormalizationConfig: PathNormalizationConfig
@@ -16,7 +16,7 @@ sealed class Group {
 data class ServicesGroup(
     override val communicationMode: CommunicationMode,
     override val serviceName: String = "",
-    override val serviceId: String? = null,
+    override val serviceId: Int? = null,
     override val discoveryServiceName: String? = null,
     override val proxySettings: ProxySettings = ProxySettings(),
     override val pathNormalizationConfig: PathNormalizationConfig = PathNormalizationConfig(),
@@ -27,7 +27,7 @@ data class ServicesGroup(
 data class AllServicesGroup(
     override val communicationMode: CommunicationMode,
     override val serviceName: String = "",
-    override val serviceId: String? = null,
+    override val serviceId: Int? = null,
     override val discoveryServiceName: String? = null,
     override val proxySettings: ProxySettings = ProxySettings(),
     override val pathNormalizationConfig: PathNormalizationConfig = PathNormalizationConfig(),

diff --git a/...trol-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadata.kt b/...trol-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadata.kt
@@ -28,7 +28,7 @@ class NodeMetadata(metadata: Struct, properties: SnapshotProperties) {
         .fieldsMap["service_name"]
         ?.stringValue
 
-    val serviceId: String? = metadata.fieldsMap["service_id"]?.stringValue
+    val serviceId: Int? = metadata.fieldsMap["service_id"]?.numberValue?.toInt()
 
     val discoveryServiceName: String? = metadata
         .fieldsMap["discovery_service_name"]

diff --git a/...gro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/LuaFilterFactory.kt b/...gro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/LuaFilterFactory.kt
@@ -78,7 +78,7 @@ class LuaFilterFactory(private val snapshotProperties: SnapshotProperties) {
                 )
             ),
             "service_name" to StringPropertyLua(group.serviceName),
-            "service_id" to StringPropertyLua(group.serviceId ?: ""),
+            "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)

diff --git a/.../src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroupTest.kt b/.../src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/MetadataNodeGroupTest.kt
@@ -160,6 +160,32 @@ class MetadataNodeGroupTest {
         )
     }
 
+    @Test
+    fun `should set serviceId to group if present`() {
+        // given
+        val nodeGroup = MetadataNodeGroup(createSnapshotProperties(outgoingPermissions = true))
+        val node = nodeV3(serviceId = 777)
+
+        // when
+        val group = nodeGroup.hash(node)
+
+        // then
+        assertThat(group.serviceId).isEqualTo(777)
+    }
+
+    @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

diff --git a/...l-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/TestNodeFactory.kt b/...l-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/TestNodeFactory.kt
@@ -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"),
@@ -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))
     }

diff --git a/...tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/LuaFilterFactoryTest.kt b/...tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/LuaFilterFactoryTest.kt
@@ -21,11 +21,9 @@ internal class LuaFilterFactoryTest {
         // given
         val expectedServiceName = "service-1"
         val expectedDiscoveryServiceName = "consul-service-1"
-        val expectedServiceId = "777"
         val group: Group = ServicesGroup(
             communicationMode = CommunicationMode.XDS,
             serviceName = expectedServiceName,
-            serviceId = expectedServiceId,
             discoveryServiceName = expectedDiscoveryServiceName
         )
         val factory = LuaFilterFactory(properties)
@@ -42,15 +40,47 @@ internal class LuaFilterFactoryTest {
             .getFieldsOrThrow("discovery_service_name")
             .stringValue
 
-        val givenServiceId = metadata
+        // then
+        assertThat(givenServiceName).isEqualTo(expectedServiceName)
+        assertThat(givenDiscoveryServiceName).isEqualTo(expectedDiscoveryServiceName)
+    }
+
+    @Test
+    fun `should create metadata with serviceId`() {
+        // given
+        val expectedServiceId = 777
+
+        val group: Group = ServicesGroup(communicationMode = CommunicationMode.XDS, serviceId = expectedServiceId)
+        val factory = LuaFilterFactory(properties)
+
+        // when
+        val metadata = factory.ingressScriptsMetadata(group, currentZone = "dc1")
+
+        val actualServiceId = metadata
             .getFilterMetadataOrThrow("envoy.filters.http.lua")
             .getFieldsOrThrow("service_id")
             .stringValue
 
         // then
-        assertThat(givenServiceName).isEqualTo(expectedServiceName)
-        assertThat(givenDiscoveryServiceName).isEqualTo(expectedDiscoveryServiceName)
-        assertThat(givenServiceId).isEqualTo(expectedServiceId)
+        assertThat(actualServiceId).isEqualTo(expectedServiceId.toString())
+    }
+
+    @Test
+    fun `should create metadata with empty serviceId`() {
+        // given
+        val group: Group = ServicesGroup(communicationMode = CommunicationMode.XDS)
+        val factory = LuaFilterFactory(properties)
+
+        // when
+        val metadata = factory.ingressScriptsMetadata(group, currentZone = "dc1")
+
+        val actualServiceId = metadata
+            .getFilterMetadataOrThrow("envoy.filters.http.lua")
+            .getFieldsOrThrow("service_id")
+            .stringValue
+
+        // then
+        assertThat(actualServiceId).isEmpty()
     }
 
     @Test