Skip to content

Commit 1b9d32f

Browse files
committed
Addressing coderabbit PR comments
1 parent 1b56db2 commit 1b9d32f

2 files changed

Lines changed: 15 additions & 6 deletions

File tree

pkg/router/controller/route_secret_manager.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -329,12 +329,12 @@ func (p *RouteSecretManager) generateSecretHandler(namespace, routeName string)
329329
if !ok {
330330
tombstone, ok := obj.(cache.DeletedFinalStateUnknown)
331331
if !ok {
332-
log.Error(nil, "Couldn't get object from tombstone", "obj", obj)
332+
log.Error(nil, "Couldn't get object from tombstone", "type", fmt.Sprintf("%T", obj))
333333
return
334334
}
335335
secret, ok = tombstone.Obj.(*kapi.Secret)
336336
if !ok {
337-
log.Error(nil, "Tombstone contained object that is not a secret", "obj", tombstone.Obj)
337+
log.Error(nil, "Tombstone contained object that is not a secret", "type", fmt.Sprintf("%T", tombstone.Obj))
338338
return
339339
}
340340
}
@@ -386,8 +386,8 @@ func (p *RouteSecretManager) validate(route *routev1.Route) error {
386386
// Note: This function performs an in-place update of the route. The caller should be aware that the route's TLS configuration will be modified directly.
387387
func (p *RouteSecretManager) populateRouteTLSFromSecret(route *routev1.Route) error {
388388
// read referenced secret from the informer cache.
389-
// RegisterRoute blocks until cache sync completes, so the secret should be
390-
// available immediately after registration.
389+
// GetSecret attempts to read from the cache and falls back to a direct API
390+
// call if the cache is not synced or the secret is not found.
391391
secret, err := p.secretManager.GetSecret(context.TODO(), route.Namespace, route.Name)
392392
if err != nil {
393393
log.Error(err, "failed to get referenced secret")

pkg/router/controller/shared_secret_manager.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ func (m *SharedSecretManager) RegisterRoute(ctx context.Context, namespace strin
129129
cache.Indexers{},
130130
)
131131

132-
inf.AddEventHandler(cache.ResourceEventHandlerFuncs{
132+
if _, err := inf.AddEventHandler(cache.ResourceEventHandlerFuncs{
133133
AddFunc: func(obj interface{}) {
134134
m.notify(namespace, obj, "Add", nil)
135135
},
@@ -139,7 +139,11 @@ func (m *SharedSecretManager) RegisterRoute(ctx context.Context, namespace strin
139139
DeleteFunc: func(obj interface{}) {
140140
m.notify(namespace, obj, "Delete", nil)
141141
},
142-
})
142+
}); err != nil {
143+
delete(m.registeredRoutes, key)
144+
m.lock.Unlock()
145+
return fmt.Errorf("failed to add secret informer handler for key %s: %w", infKey, err)
146+
}
143147

144148
ctx, cancel := context.WithCancel(context.Background())
145149
infState = &informerState{
@@ -246,6 +250,11 @@ func (m *SharedSecretManager) UnregisterRoute(namespace string, routeName string
246250
return nil
247251
}
248252

253+
// GetSecret returns the secret from the informer cache or falls back to an API call.
254+
// WARNING: To maintain high throughput and reduce GC pressure during route syncs,
255+
// the returned Secret is a direct pointer to the shared informer cache object.
256+
// Callers MUST NOT mutate the returned Secret. If mutation is required, the caller
257+
// must explicitly call secret.DeepCopy() first.
249258
func (m *SharedSecretManager) GetSecret(ctx context.Context, namespace string, routeName string) (*corev1.Secret, error) {
250259
m.lock.RLock()
251260
key := namespace + "/" + routeName

0 commit comments

Comments
 (0)