Introduction of Glob Patterns in extension of API related to URL paths (

#417) * Extended API for the 'paths' field. --------- Co-authored-by: radoslaw.chrzanowski <[email protected]> Co-authored-by: jan.kozlowski <[email protected]>
allegro · Aug 19, 2024 · 10ec6c7 · 10ec6c7
1 parent 95ed004
commit 10ec6c7
Show file tree

Hide file tree

Showing 9 changed files with 199 additions and 47 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 ### Changed
 - Add JWT failure reason to metadata and use it in jwt-status field on denied requests
 
+## [0.21.0]
+### Changed
+- Added `paths` field in API to support Glob Patterns
+
 ## [0.20.15]
 ### Changed
 - Java-control-plane update to 1.0.45

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
@@ -398,9 +398,11 @@ fun Value.toIncomingEndpoint(properties: SnapshotProperties): IncomingEndpoint {
     val pathPrefix = this.field("pathPrefix")?.stringValue
     val path = this.field("path")?.stringValue
     val pathRegex = this.field("pathRegex")?.stringValue
+    val paths = this.field("paths")?.list().orEmpty().map { it.stringValue }.toSet()
 
-    if (isMoreThanOnePropertyDefined(path, pathPrefix, pathRegex)) {
-        throw NodeMetadataValidationException("Precisely one of 'path', 'pathPrefix' or 'pathRegex' field is allowed")
+    if (isMoreThanOnePropertyDefined(paths, path, pathPrefix, pathRegex)) {
+        throw NodeMetadataValidationException(
+            "Precisely one of 'paths', 'path', 'pathPrefix' or 'pathRegex' field is allowed")
     }
 
     val methods = this.field("methods")?.list().orEmpty().map { it.stringValue }.toSet()
@@ -409,16 +411,20 @@ fun Value.toIncomingEndpoint(properties: SnapshotProperties): IncomingEndpoint {
     val oauth = properties.let { this.field("oauth")?.toOAuth(it) }
 
     return when {
-        path != null -> IncomingEndpoint(path, PathMatchingType.PATH, methods, clients, unlistedClientsPolicy, oauth)
+        paths.isNotEmpty() -> IncomingEndpoint(
+            paths, "", PathMatchingType.PATH, methods, clients, unlistedClientsPolicy, oauth)
+        path != null -> IncomingEndpoint(
+            paths, path, PathMatchingType.PATH, methods, clients, unlistedClientsPolicy, oauth)
         pathPrefix != null -> IncomingEndpoint(
-            pathPrefix, PathMatchingType.PATH_PREFIX, methods, clients, unlistedClientsPolicy, oauth
+            paths, pathPrefix, PathMatchingType.PATH_PREFIX, methods, clients, unlistedClientsPolicy, oauth
         )
 
         pathRegex != null -> IncomingEndpoint(
-            pathRegex, PathMatchingType.PATH_REGEX, methods, clients, unlistedClientsPolicy, oauth
+            paths, pathRegex, PathMatchingType.PATH_REGEX, methods, clients, unlistedClientsPolicy, oauth
         )
 
-        else -> throw NodeMetadataValidationException("One of 'path', 'pathPrefix' or 'pathRegex' field is required")
+        else -> throw NodeMetadataValidationException(
+            "One of 'paths', 'path', 'pathPrefix' or 'pathRegex' field is required")
     }
 }
 
@@ -449,7 +455,16 @@ fun Value.toIncomingRateLimitEndpoint(): IncomingRateLimitEndpoint {
     }
 }
 
-fun isMoreThanOnePropertyDefined(vararg properties: String?): Boolean = properties.filterNotNull().count() > 1
+fun isMoreThanOnePropertyDefined(vararg properties: Any?): Boolean =
+    countNonNullAndNotEmptyProperties(properties.toList()) > 1
+
+private fun countNonNullAndNotEmptyProperties(props: List<Any?>): Int = props.filterNotNull().count {
+    if (it is Set<*>) {
+        it.isNotEmpty()
+    } else {
+        true
+    }
+}
 
 private fun Value?.toIncomingTimeoutPolicy(): Incoming.TimeoutPolicy {
     val idleTimeout: Duration? = this?.field("idleTimeout")?.toDuration()
@@ -786,6 +801,7 @@ data class ClientWithSelector private constructor(
 }
 
 data class IncomingEndpoint(
+    override val paths: Set<String> = emptySet(),
     override val path: String = "",
     override val pathMatchingType: PathMatchingType = PathMatchingType.PATH,
     override val methods: Set<String> = emptySet(),
@@ -826,6 +842,7 @@ data class OAuth(
 }
 
 interface EndpointBase {
+    val paths: Set<String>
     val path: String
     val pathMatchingType: PathMatchingType
     val methods: Set<String>

diff --git a/...gro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/JwtFilterFactory.kt b/...gro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/JwtFilterFactory.kt
@@ -4,6 +4,7 @@ import com.google.protobuf.Any
 import com.google.protobuf.Empty
 import com.google.protobuf.util.Durations
 import io.envoyproxy.envoy.config.core.v3.HttpUri
+import io.envoyproxy.envoy.config.core.v3.TypedExtensionConfig
 import io.envoyproxy.envoy.config.route.v3.HeaderMatcher
 import io.envoyproxy.envoy.config.route.v3.RouteMatch
 import io.envoyproxy.envoy.extensions.filters.http.jwt_authn.v3.JwtAuthentication
@@ -13,6 +14,7 @@ import io.envoyproxy.envoy.extensions.filters.http.jwt_authn.v3.JwtRequirementOr
 import io.envoyproxy.envoy.extensions.filters.http.jwt_authn.v3.RemoteJwks
 import io.envoyproxy.envoy.extensions.filters.http.jwt_authn.v3.RequirementRule
 import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.HttpFilter
+import io.envoyproxy.envoy.extensions.path.match.uri_template.v3.UriTemplateMatchConfig
 import io.envoyproxy.envoy.type.matcher.v3.RegexMatcher
 import pl.allegro.tech.servicemesh.envoycontrol.groups.Group
 import pl.allegro.tech.servicemesh.envoycontrol.groups.IncomingEndpoint
@@ -98,10 +100,10 @@ class JwtFilterFactory(
     }
 
     private fun createRules(endpoints: List<IncomingEndpoint>): Set<RequirementRule> {
-        return endpoints.mapNotNull(this::createRuleForEndpoint).toSet()
+        return endpoints.flatMap(this::createRulesForEndpoint).toSet()
     }
 
-    private fun createRuleForEndpoint(endpoint: IncomingEndpoint): RequirementRule? {
+    private fun createRulesForEndpoint(endpoint: IncomingEndpoint): Set<RequirementRule> {
         val providers = mutableSetOf<String>()
 
         if (endpoint.oauth != null) {
@@ -111,11 +113,44 @@ class JwtFilterFactory(
         providers.addAll(endpoint.clients.filter { it.name in clientToOAuthProviderName.keys }
             .mapNotNull { clientToOAuthProviderName[it.name] })
 
-        return if (providers.isNotEmpty()) {
-            requirementRuleWithPathMatching(endpoint.path, endpoint.pathMatchingType, endpoint.methods, providers)
+        if (providers.isEmpty()) {
+            return emptySet()
+        }
+
+        return if (endpoint.paths.isNotEmpty()) {
+            endpoint.paths.map {
+                requirementRuleWithURITemplateMatching(it, endpoint.methods, providers)
+            }.toSet()
         } else {
-            null
+            setOf(requirementRuleWithPathMatching(
+                endpoint.path, endpoint.pathMatchingType, endpoint.methods, providers))
+        }
+    }
+
+    private fun requirementRuleWithURITemplateMatching(
+        pathGlobPattern: String,
+        methods: Set<String>,
+        providers: MutableSet<String>
+    ): RequirementRule {
+        val pathMatching = RouteMatch.newBuilder().setPathMatchPolicy(
+            TypedExtensionConfig.newBuilder()
+                .setName("envoy.path.match.uri_template.uri_template_matcher")
+                .setTypedConfig(
+                    Any.pack(
+                        UriTemplateMatchConfig.newBuilder()
+                            .setPathTemplate(pathGlobPattern)
+                            .build()
+                    )
+                ).build()
+        )
+        if (methods.isNotEmpty()) {
+            pathMatching.addHeaders(createHeaderMatcherBuilder(methods))
         }
+
+        return RequirementRule.newBuilder()
+            .setMatch(pathMatching)
+            .setRequires(createJwtRequirement(providers))
+            .build()
     }
 
     private fun requirementRuleWithPathMatching(
@@ -135,22 +170,25 @@ class JwtFilterFactory(
                 )
         }
         if (methods.isNotEmpty()) {
-            val methodsRegexp = methods.joinToString("|")
-            val headerMatcher = HeaderMatcher.newBuilder()
-                .setName(":method")
-                .setSafeRegexMatch(
-                    RegexMatcher.newBuilder().setRegex(methodsRegexp).setGoogleRe2(
-                        RegexMatcher.GoogleRE2.getDefaultInstance()
-                    ).build()
-                )
-            pathMatching.addHeaders(headerMatcher)
+            pathMatching.addHeaders(createHeaderMatcherBuilder(methods))
         }
+
         return RequirementRule.newBuilder()
             .setMatch(pathMatching)
             .setRequires(createJwtRequirement(providers))
             .build()
     }
 
+    private fun createHeaderMatcherBuilder(methods: Set<String>): HeaderMatcher.Builder {
+        return HeaderMatcher.newBuilder()
+            .setName(":method")
+            .setSafeRegexMatch(
+                RegexMatcher.newBuilder().setRegex(methods.joinToString("|")).setGoogleRe2(
+                    RegexMatcher.GoogleRE2.getDefaultInstance()
+                ).build()
+            )
+    }
+
     private val requirementsForProviders: Map<ProviderName, JwtRequirement> =
         jwtProviders.keys.associateWith { JwtRequirement.newBuilder().setProviderName(it).build() }
 

diff --git a/...ech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterPermissions.kt b/...ech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/RBACFilterPermissions.kt
@@ -1,7 +1,10 @@
 package pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters
 
+import com.google.protobuf.Any
+import io.envoyproxy.envoy.config.core.v3.TypedExtensionConfig
 import io.envoyproxy.envoy.config.rbac.v3.Permission
 import io.envoyproxy.envoy.config.route.v3.HeaderMatcher
+import io.envoyproxy.envoy.extensions.path.match.uri_template.v3.UriTemplateMatchConfig
 import io.envoyproxy.envoy.type.matcher.v3.PathMatcher
 import io.envoyproxy.envoy.type.matcher.v3.RegexMatcher
 import io.envoyproxy.envoy.type.matcher.v3.StringMatcher
@@ -11,8 +14,12 @@ import pl.allegro.tech.servicemesh.envoycontrol.groups.PathMatchingType
 class RBACFilterPermissions {
     fun createCombinedPermissions(incomingEndpoint: IncomingEndpoint): Permission.Builder {
         val permissions = listOfNotNull(
-            createPathPermissionForEndpoint(incomingEndpoint),
-            createMethodPermissions(incomingEndpoint)
+            if (incomingEndpoint.paths.isNotEmpty()) {
+                createPathTemplatesPermissionForEndpoint(incomingEndpoint)
+            } else {
+                createPathPermissionForEndpoint(incomingEndpoint)
+            },
+            createMethodPermissions(incomingEndpoint),
         )
             .map { it.build() }
 
@@ -21,6 +28,22 @@ class RBACFilterPermissions {
         )
     }
 
+    private fun createPathTemplatesPermissionForEndpoint(incomingEndpoint: IncomingEndpoint): Permission.Builder {
+        return permission()
+            .setOrRules(Permission.Set.newBuilder().addAllRules(
+                incomingEndpoint.paths.map(this::createPathTemplate)))
+    }
+
+    private fun createPathTemplate(path: String): Permission {
+        return permission().setUriTemplate(TypedExtensionConfig.newBuilder()
+            .setName("envoy.path.match.uri_template.uri_template_matcher")
+            .setTypedConfig(Any.pack(
+                UriTemplateMatchConfig.newBuilder()
+                    .setPathTemplate(path)
+                    .build()
+            ))).build()
+    }
+
     fun createPathPermission(path: String, matchingType: PathMatchingType): Permission.Builder {
         return permission().setUrlPath(createPathMatcher(path, matchingType))
     }

diff --git a/...-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataTest.kt b/...-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataTest.kt
@@ -119,7 +119,7 @@ class NodeMetadataTest {
 
         // expects
         val exception = assertThrows<NodeMetadataValidationException> { proto.toIncomingEndpoint(snapshotProperties()) }
-        assertThat(exception.status.description).isEqualTo("Precisely one of 'path', 'pathPrefix' or 'pathRegex' field is allowed")
+        assertThat(exception.status.description).isEqualTo("Precisely one of 'paths', 'path', 'pathPrefix' or 'pathRegex' field is allowed")
         assertThat(exception.status.code).isEqualTo(Status.Code.INVALID_ARGUMENT)
     }
 
@@ -135,18 +135,18 @@ class NodeMetadataTest {
 
         // expects
         val exception = assertThrows<NodeMetadataValidationException> { proto.toIncomingEndpoint(snapshotProperties()) }
-        assertThat(exception.status.description).isEqualTo("Precisely one of 'path', 'pathPrefix' or 'pathRegex' field is allowed")
+        assertThat(exception.status.description).isEqualTo("Precisely one of 'paths', 'path', 'pathPrefix' or 'pathRegex' field is allowed")
         assertThat(exception.status.code).isEqualTo(Status.Code.INVALID_ARGUMENT)
     }
 
     @Test
-    fun `should reject endpoint with no path or pathPrefix or pathRegex defined`() {
+    fun `should reject endpoint with empty paths or without path or pathPrefix or pathRegex defined`() {
         // given
         val proto = incomingEndpointProto(path = null, pathPrefix = null, pathRegex = null)
 
         // expects
         val exception = assertThrows<NodeMetadataValidationException> { proto.toIncomingEndpoint(snapshotProperties()) }
-        assertThat(exception.status.description).isEqualTo("One of 'path', 'pathPrefix' or 'pathRegex' field is required")
+        assertThat(exception.status.description).isEqualTo("One of 'paths', 'path', 'pathPrefix' or 'pathRegex' field is required")
         assertThat(exception.status.code).isEqualTo(Status.Code.INVALID_ARGUMENT)
     }
 

diff --git a/...tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/JwtFilterFactoryTest.kt b/...tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/JwtFilterFactoryTest.kt
@@ -164,7 +164,7 @@ internal class JwtFilterFactoryTest {
             Incoming(
                 pathToProvider.map { (path, provider) ->
                     IncomingEndpoint(
-                        path,
+                        path = path,
                         oauth = OAuth(provider, policy = policy)
                     )
                 }
@@ -182,7 +182,7 @@ internal class JwtFilterFactoryTest {
             Incoming(
                 pathToProvider.map { (path, _) ->
                     IncomingEndpoint(
-                        path,
+                        path = path,
                         clients = setOf(ClientWithSelector.create("oauth", "client")),
                         oauth = null
                     )

diff --git a/...icemesh/envoycontrol/snapshot/resource/listeners/filters/rbac/RBACFilterFactoryJwtTest.kt b/...icemesh/envoycontrol/snapshot/resource/listeners/filters/rbac/RBACFilterFactoryJwtTest.kt
@@ -71,6 +71,7 @@ internal class RBACFilterFactoryJwtTest : RBACFilterFactoryTestUtils {
             permissionsEnabled = true,
             endpoints = listOf(
                 IncomingEndpoint(
+                    emptySet(),
                     "/oauth-protected",
                     PathMatchingType.PATH,
                     setOf("GET"),
@@ -110,6 +111,7 @@ internal class RBACFilterFactoryJwtTest : RBACFilterFactoryTestUtils {
             permissionsEnabled = true,
             endpoints = listOf(
                 IncomingEndpoint(
+                    emptySet(),
                     "/oauth-protected",
                     PathMatchingType.PATH,
                     setOf("GET"),
@@ -152,6 +154,7 @@ internal class RBACFilterFactoryJwtTest : RBACFilterFactoryTestUtils {
             permissionsEnabled = true,
             endpoints = listOf(
                 IncomingEndpoint(
+                    emptySet(),
                     "/oauth-protected",
                     PathMatchingType.PATH,
                     setOf("GET"),
@@ -178,6 +181,7 @@ internal class RBACFilterFactoryJwtTest : RBACFilterFactoryTestUtils {
             permissionsEnabled = true,
             endpoints = listOf(
                 IncomingEndpoint(
+                    emptySet(),
                     "/oauth-protected",
                     PathMatchingType.PATH,
                     setOf("GET"),
@@ -219,6 +223,7 @@ internal class RBACFilterFactoryJwtTest : RBACFilterFactoryTestUtils {
             permissionsEnabled = true,
             endpoints = listOf(
                 IncomingEndpoint(
+                    emptySet(),
                     "/oauth-protected",
                     PathMatchingType.PATH,
                     setOf("GET"),
@@ -264,6 +269,7 @@ internal class RBACFilterFactoryJwtTest : RBACFilterFactoryTestUtils {
             permissionsEnabled = true,
             endpoints = listOf(
                 IncomingEndpoint(
+                    emptySet(),
                     "/oauth-protected",
                     PathMatchingType.PATH,
                     setOf("GET"),
@@ -336,7 +342,7 @@ internal class RBACFilterFactoryJwtTest : RBACFilterFactoryTestUtils {
     ) = """
         {
           "policies": {
-            "IncomingEndpoint(path=/oauth-protected, pathMatchingType=PATH, methods=[GET], clients=[$clientsWithSelector], unlistedClientsPolicy=$unlistedClientsPolicy, oauth=$oauth)": {
+            "IncomingEndpoint(paths=[], path=/oauth-protected, pathMatchingType=PATH, methods=[GET], clients=[$clientsWithSelector], unlistedClientsPolicy=$unlistedClientsPolicy, oauth=$oauth)": {
               "permissions": [
                 {
                   "and_rules": {