[release/v1.2] Cherry pick v1.2.3 (#4810)

* use a waitGroup instead of an enabled channel in the status updater (#4809) use a waitGroup instead of a channel in the status updater * use a waitGroup to synchronize to the `Send` method that the status updater is enabled and ready for updates Signed-off-by: Arko Dasgupta <arko@tetrate.io> (cherry picked from commit 36939dc) Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix: remove the default retry policy for jwks fetch (#4802) * remove the default retry policy for jwks fetch Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix gen Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * Update release-notes/current.yaml Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com> Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> --------- Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com> (cherry picked from commit 526a05f) Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> --------- Signed-off-by: Arko Dasgupta <arko@tetrate.io> Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com>
envoyproxy · Nov 29, 2024 · ae2837b · ae2837b
1 parent 4901ba0
commit ae2837b
Show file tree

Hide file tree

Showing 16 changed files with 20 additions and 69 deletions.
diff --git a/internal/cmd/egctl/testdata/translate/out/jwt-single-route-single-match-to-xds.all.json b/internal/cmd/egctl/testdata/translate/out/jwt-single-route-single-match-to-xds.all.json
@@ -514,8 +514,7 @@
                                         "cluster": "raw_githubusercontent_com_443",
                                         "timeout": "10s",
                                         "uri": "https://raw.githubusercontent.com/envoyproxy/gateway/main/examples/kubernetes/jwt/jwks.json"
-                                      },
-                                      "retryPolicy": {}
+                                      }
                                     }
                                   }
                                 },

diff --git a/internal/cmd/egctl/testdata/translate/out/jwt-single-route-single-match-to-xds.all.yaml b/internal/cmd/egctl/testdata/translate/out/jwt-single-route-single-match-to-xds.all.yaml
@@ -306,7 +306,6 @@ xds:
                               cluster: raw_githubusercontent_com_443
                               timeout: 10s
                               uri: https://raw.githubusercontent.com/envoyproxy/gateway/main/examples/kubernetes/jwt/jwks.json
-                            retryPolicy: {}
                       requirementMap:
                         httproute/envoy-gateway-system/backend/rule/0/match/0/www_example_com:
                           providerName: httproute/envoy-gateway-system/backend/rule/0/match/0/www_example_com/example

diff --git a/internal/cmd/egctl/testdata/translate/out/jwt-single-route-single-match-to-xds.listener.yaml b/internal/cmd/egctl/testdata/translate/out/jwt-single-route-single-match-to-xds.listener.yaml
@@ -61,7 +61,6 @@ xds:
                             cluster: raw_githubusercontent_com_443
                             timeout: 10s
                             uri: https://raw.githubusercontent.com/envoyproxy/gateway/main/examples/kubernetes/jwt/jwks.json
-                          retryPolicy: {}
                     requirementMap:
                       httproute/envoy-gateway-system/backend/rule/0/match/0/www_example_com:
                         providerName: httproute/envoy-gateway-system/backend/rule/0/match/0/www_example_com/example

diff --git a/internal/provider/kubernetes/status_updater.go b/internal/provider/kubernetes/status_updater.go
@@ -7,7 +7,7 @@ package kubernetes
 
 import (
 	"context"
-	"errors"
+	"sync"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -57,26 +57,21 @@ func (m MutatorFunc) Mutate(old client.Object) client.Object {
 type UpdateHandler struct {
 	log           logr.Logger
 	client        client.Client
-	sendUpdates   chan struct{}
 	updateChannel chan Update
-	writer        *UpdateWriter
+	wg            *sync.WaitGroup
 }
 
 func NewUpdateHandler(log logr.Logger, client client.Client) *UpdateHandler {
-	sendUpdates := make(chan struct{})
-	updateChannel := make(chan Update, 100)
-	return &UpdateHandler{
+	u := &UpdateHandler{
 		log:           log,
 		client:        client,
-		sendUpdates:   sendUpdates,
-		updateChannel: updateChannel,
-		writer: &UpdateWriter{
-			log:                 log,
-			enabled:             sendUpdates,
-			updateChannel:       updateChannel,
-			eventsBeforeEnabled: make(chan Update, 1000),
-		},
+		updateChannel: make(chan Update, 1000),
+		wg:            new(sync.WaitGroup),
 	}
+
+	u.wg.Add(1)
+
+	return u
 }
 
 func (u *UpdateHandler) apply(update Update) {
@@ -140,8 +135,7 @@ func (u *UpdateHandler) Start(ctx context.Context) error {
 	defer u.log.Info("stopped status update handler")
 
 	// Enable Updaters to start sending updates to this handler.
-	close(u.sendUpdates)
-	u.writer.handleEventsReceivedBeforeEnabled()
+	u.wg.Done()
 
 	for {
 		select {
@@ -158,7 +152,10 @@ func (u *UpdateHandler) Start(ctx context.Context) error {
 
 // Writer retrieves the interface that should be used to write to the UpdateHandler.
 func (u *UpdateHandler) Writer() Updater {
-	return u.writer
+	return &UpdateWriter{
+		updateChannel: u.updateChannel,
+		wg:            u.wg,
+	}
 }
 
 // Updater describes an interface to send status updates somewhere.
@@ -168,40 +165,15 @@ type Updater interface {
 
 // UpdateWriter takes status updates and sends these to the UpdateHandler via a channel.
 type UpdateWriter struct {
-	log           logr.Logger
-	enabled       <-chan struct{}
 	updateChannel chan<- Update
-	// a temporary buffer to store events received before the Updater is enabled.
-	// These events will be sent to the update channel once the Updater is enabled.
-	eventsBeforeEnabled chan Update
+	wg            *sync.WaitGroup
 }
 
 // Send sends the given Update off to the update channel for writing by the UpdateHandler.
 func (u *UpdateWriter) Send(update Update) {
-	// Non-blocking receive to see if we should pass along update.
-	select {
-	case <-u.enabled:
-		u.updateChannel <- update
-	default:
-		if len(u.eventsBeforeEnabled) < cap(u.eventsBeforeEnabled) {
-			u.log.Info("received a status update while disabled, storing for later", "event", update.NamespacedName)
-			u.eventsBeforeEnabled <- update
-		} else {
-			// If the buffer is full, drop the event to avoid blocking the sender.
-			u.log.Error(errors.New("dropping status update, buffer full"), "event", update.NamespacedName)
-		}
-	}
-}
-
-// handleEventsReceivedBeforeEnabled sends the events received before the Updater was enabled to the update channel.
-func (u *UpdateWriter) handleEventsReceivedBeforeEnabled() {
-	go func() {
-		for e := range u.eventsBeforeEnabled {
-			u.log.Info("sending stored status update", "event", e.NamespacedName)
-			u.updateChannel <- e
-		}
-		close(u.eventsBeforeEnabled)
-	}()
+	// Wait until updater is ready
+	u.wg.Wait()
+	u.updateChannel <- update
 }
 
 // isStatusEqual checks if two objects have equivalent status.

diff --git a/internal/xds/translator/jwt.go b/internal/xds/translator/jwt.go
@@ -120,7 +120,6 @@ func buildJWTAuthn(irListener *ir.HTTPListener) (*jwtauthnv3.JwtAuthentication,
 					},
 					CacheDuration: &durationpb.Duration{Seconds: 5 * 60},
 					AsyncFetch:    &jwtauthnv3.JwksAsyncFetch{},
-					RetryPolicy:   &corev3.RetryPolicy{},
 				},
 			}
 

diff --git a/internal/xds/translator/testdata/out/xds-ir/authorization-jwt-claim.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/authorization-jwt-claim.listeners.yaml
@@ -35,7 +35,6 @@
                     cluster: two_example_com_443
                     timeout: 10s
                     uri: https://two.example.com/jwt/public-key/jwks.json
-                  retryPolicy: {}
               httproute/default/httproute-2/rule/0/match/0/www_example_com/example1:
                 audiences:
                 - one.foo.com
@@ -52,7 +51,6 @@
                     cluster: one_example_com_443
                     timeout: 10s
                     uri: https://one.example.com/jwt/public-key/jwks.json
-                  retryPolicy: {}
             requirementMap:
               httproute/default/httproute-1/rule/0/match/0/www_example_com:
                 providerName: httproute/default/httproute-1/rule/0/match/0/www_example_com/example1

diff --git a/internal/xds/translator/testdata/out/xds-ir/authorization-jwt-scope.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/authorization-jwt-scope.listeners.yaml
@@ -35,7 +35,6 @@
                     cluster: two_example_com_443
                     timeout: 10s
                     uri: https://two.example.com/jwt/public-key/jwks.json
-                  retryPolicy: {}
               httproute/default/httproute-2/rule/0/match/0/www_example_com/example1:
                 audiences:
                 - one.foo.com
@@ -52,7 +51,6 @@
                     cluster: one_example_com_443
                     timeout: 10s
                     uri: https://one.example.com/jwt/public-key/jwks.json
-                  retryPolicy: {}
             requirementMap:
               httproute/default/httproute-1/rule/0/match/0/www_example_com:
                 providerName: httproute/default/httproute-1/rule/0/match/0/www_example_com/example1

diff --git a/internal/xds/translator/testdata/out/xds-ir/custom-filter-order.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/custom-filter-order.listeners.yaml
@@ -85,7 +85,6 @@
                     cluster: one_example_com_443
                     timeout: 10s
                     uri: https://one.example.com/jwt/public-key/jwks.json
-                  retryPolicy: {}
               httproute/envoy-gateway/httproute-1/rule/0/match/0/www_example_com/example2:
                 audiences:
                 - two.foo.com
@@ -105,7 +104,6 @@
                     cluster: two_example_com_80
                     timeout: 10s
                     uri: http://two.example.com/jwt/public-key/jwks.json
-                  retryPolicy: {}
             requirementMap:
               httproute/envoy-gateway/httproute-1/rule/0/match/0/www_example_com:
                 requiresAny:

diff --git a/internal/xds/translator/testdata/out/xds-ir/jwt-custom-extractor.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/jwt-custom-extractor.listeners.yaml
@@ -42,7 +42,6 @@
                     cluster: localhost_443
                     timeout: 10s
                     uri: https://localhost/jwt/public-key/jwks.json
-                  retryPolicy: {}
             requirementMap:
               first-route:
                 providerName: first-route/example

diff --git a/internal/xds/translator/testdata/out/xds-ir/jwt-multi-route-multi-provider.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/jwt-multi-route-multi-provider.listeners.yaml
@@ -38,7 +38,6 @@
                     cluster: localhost_80
                     timeout: 10s
                     uri: http://localhost/jwt/public-key/jwks.json
-                  retryPolicy: {}
               first-route-www.test.com/example2:
                 audiences:
                 - one.foo.com
@@ -62,7 +61,6 @@
                     cluster: "192_168_1_250_8080"
                     timeout: 10s
                     uri: https://192.168.1.250:8080/jwt/public-key/jwks.json
-                  retryPolicy: {}
               second-route-www.test.com/example:
                 audiences:
                 - foo.com
@@ -82,7 +80,6 @@
                     cluster: localhost_80
                     timeout: 10s
                     uri: http://localhost/jwt/public-key/jwks.json
-                  retryPolicy: {}
               second-route-www.test.com/example2:
                 audiences:
                 - one.foo.com
@@ -100,7 +97,6 @@
                     cluster: "192_168_1_250_8080"
                     timeout: 10s
                     uri: https://192.168.1.250:8080/jwt/public-key/jwks.json
-                  retryPolicy: {}
             requirementMap:
               first-route-www.test.com:
                 requiresAny:

diff --git a/internal/xds/translator/testdata/out/xds-ir/jwt-multi-route-single-provider.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/jwt-multi-route-single-provider.listeners.yaml
@@ -60,7 +60,6 @@
                     cluster: localhost_443
                     timeout: 10s
                     uri: https://localhost/jwt/public-key/jwks.json
-                  retryPolicy: {}
               second-route/example:
                 audiences:
                 - foo.com
@@ -77,7 +76,6 @@
                     cluster: localhost_443
                     timeout: 10s
                     uri: https://localhost/jwt/public-key/jwks.json
-                  retryPolicy: {}
             requirementMap:
               first-route:
                 providerName: first-route/example

diff --git a/internal/xds/translator/testdata/out/xds-ir/jwt-optional.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/jwt-optional.listeners.yaml
@@ -42,7 +42,6 @@
                     cluster: localhost_443
                     timeout: 10s
                     uri: https://localhost/jwt/public-key/jwks.json
-                  retryPolicy: {}
             requirementMap:
               first-route:
                 requiresAny:

diff --git a/internal/xds/translator/testdata/out/xds-ir/jwt-ratelimit.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/jwt-ratelimit.listeners.yaml
@@ -35,7 +35,6 @@
                     cluster: "192_168_1_250_443"
                     timeout: 10s
                     uri: https://192.168.1.250/jwt/public-key/jwks.json
-                  retryPolicy: {}
             requirementMap:
               first-route:
                 providerName: first-route/example

diff --git a/internal/xds/translator/testdata/out/xds-ir/jwt-single-route-single-match.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/jwt-single-route-single-match.listeners.yaml
@@ -35,7 +35,6 @@
                     cluster: localhost_443
                     timeout: 10s
                     uri: https://localhost/jwt/public-key/jwks.json
-                  retryPolicy: {}
             requirementMap:
               first-route:
                 providerName: first-route/example

diff --git a/...rnal/xds/translator/testdata/out/xds-ir/securitypolicy-with-oidc-jwt-authz.listeners.yaml b/...rnal/xds/translator/testdata/out/xds-ir/securitypolicy-with-oidc-jwt-authz.listeners.yaml
@@ -81,7 +81,6 @@
                     cluster: oidc_example_com_443
                     timeout: 10s
                     uri: https://oidc.example.com/auth/realms/example/protocol/openid-connect/certs
-                  retryPolicy: {}
             requirementMap:
               httproute/default/httproute-1/rule/0/match/0/www_example_com:
                 providerName: httproute/default/httproute-1/rule/0/match/0/www_example_com/exjwt

diff --git a/release-notes/current.yaml b/release-notes/current.yaml
@@ -14,7 +14,7 @@ new features: |
 
 # Fixes for bugs identified in previous versions.
 bug fixes: |
-  Add a bug fix here
+  Disabled the retry policy for the JWT provider to reduce requests sent to the JWKS endpoint. Failed async fetches will retry every 1s.
 
 # Enhancements that improve performance.
 performance improvements: |
-Original file line number
+Diff line change
@@ Expand Up @@
     					},
     					CacheDuration: &durationpb.Duration{Seconds: 5 * 60},
     					AsyncFetch:    &jwtauthnv3.JwksAsyncFetch{},
-    					RetryPolicy:   &corev3.RetryPolicy{},
     				},
     			}
@@ Expand Down @@