fix jwt impacting lds cache (#425)

* fix jwt impacting lds cache * update CHANGELOG.md
allegro · Jul 25, 2024 · 1a64a5f · 1a64a5f
1 parent 72b542b
commit 1a64a5f
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,7 +4,8 @@ Lists all changes with user impact.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 ## [0.20.17]
-### Changed
+### Fixed
+- Fix JWT provider configuration to not impact lds cache
 - Add missing methods in lua scripts to remove logs about it
 
 ## [0.20.16]

diff --git a/...gro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/JwtFilterFactory.kt b/...gro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/JwtFilterFactory.kt
@@ -25,20 +25,20 @@ class JwtFilterFactory(
     private val properties: JwtFilterProperties
 ) {
 
-    private val jwtProviderBuilders: Map<ProviderName, JwtProvider.Builder> = getJwtProviderBuilders()
+    private val jwtProviders: Map<ProviderName, JwtProvider> = getJwtProviders(failedStatusInMetadataEnabled = false)
+    private val jwtProvidersWithJwtStatusMetadata: Map<ProviderName, JwtProvider> =
+        getJwtProviders(failedStatusInMetadataEnabled = true)
     private val clientToOAuthProviderName: Map<String, String> =
         properties.providers.entries.flatMap { (providerName, provider) ->
             provider.matchings.keys.map { client -> client to providerName }
         }.toMap()
 
     fun createJwtFilter(group: Group): HttpFilter? {
-        val configuredJwtProviders =
+        val selectedJwtProviders =
             if (group.listenersConfig?.addJwtFailureStatus != false && properties.failedStatusInMetadataEnabled) {
-                jwtProviderBuilders.mapValues {
-                    it.value.setFailedStatusInMetadata(properties.failedStatusInMetadata).build()
-                }
+                jwtProvidersWithJwtStatusMetadata
             } else {
-                jwtProviderBuilders.mapValues { it.value.clearFailedStatusInMetadata().build() }
+                jwtProviders
             }
 
         return if (shouldCreateFilter(group)) {
@@ -47,7 +47,7 @@ class JwtFilterFactory(
                 .setTypedConfig(
                     Any.pack(
                         JwtAuthentication.newBuilder().putAllProviders(
-                            configuredJwtProviders
+                            selectedJwtProviders
                         )
                             .addAllRules(createRules(group.proxySettings.incoming.endpoints))
                             .build()
@@ -68,26 +68,34 @@ class JwtFilterFactory(
     private fun containsClientsWithSelector(it: IncomingEndpoint) =
         clientToOAuthProviderName.keys.intersect(it.clients.map { it.name }).isNotEmpty()
 
-    private fun getJwtProviderBuilders(): Map<ProviderName, JwtProvider.Builder> =
+    private fun getJwtProviders(failedStatusInMetadataEnabled: Boolean): Map<ProviderName, JwtProvider> =
         properties.providers.entries.associate {
-            it.key to createProviderBuilder(it.value)
+            it.key to createProvider(it.value, failedStatusInMetadataEnabled)
         }
 
-    private fun createProviderBuilder(provider: OAuthProvider) = JwtProvider.newBuilder()
-        .setRemoteJwks(
-            RemoteJwks.newBuilder().setHttpUri(
-                HttpUri.newBuilder()
-                    .setUri(provider.jwksUri.toString())
-                    .setCluster(provider.clusterName)
-                    .setTimeout(
-                        Durations.fromMillis(provider.connectionTimeout.toMillis())
-                    ).build()
+    private fun createProvider(provider: OAuthProvider, failedStatusInMetadataEnabled: Boolean): JwtProvider {
+        val jwtProvider = JwtProvider.newBuilder()
+            .setRemoteJwks(
+                RemoteJwks.newBuilder().setHttpUri(
+                    HttpUri.newBuilder()
+                        .setUri(provider.jwksUri.toString())
+                        .setCluster(provider.clusterName)
+                        .setTimeout(
+                            Durations.fromMillis(provider.connectionTimeout.toMillis())
+                        ).build()
+                )
+                    .setCacheDuration(Durations.fromMillis(provider.cacheDuration.toMillis()))
             )
-                .setCacheDuration(Durations.fromMillis(provider.cacheDuration.toMillis()))
-        )
-        .setForward(properties.forwardJwt)
-        .setForwardPayloadHeader(properties.forwardPayloadHeader)
-        .setPayloadInMetadata(properties.payloadInMetadata)
+            .setForward(properties.forwardJwt)
+            .setForwardPayloadHeader(properties.forwardPayloadHeader)
+            .setPayloadInMetadata(properties.payloadInMetadata)
+
+        if (failedStatusInMetadataEnabled) {
+            jwtProvider.setFailedStatusInMetadata(properties.failedStatusInMetadata)
+        }
+
+        return jwtProvider.build()
+    }
 
     private fun createRules(endpoints: List<IncomingEndpoint>): Set<RequirementRule> {
         return endpoints.mapNotNull(this::createRuleForEndpoint).toSet()
@@ -144,7 +152,7 @@ class JwtFilterFactory(
     }
 
     private val requirementsForProviders: Map<ProviderName, JwtRequirement> =
-        jwtProviderBuilders.keys.associateWith { JwtRequirement.newBuilder().setProviderName(it).build() }
+        jwtProviders.keys.associateWith { JwtRequirement.newBuilder().setProviderName(it).build() }
 
     private val allowMissingOrFailedRequirement =
         JwtRequirement.newBuilder().setAllowMissingOrFailed(Empty.getDefaultInstance()).build()