Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions component/ensure_component.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ func (e *EnsureComponentByHash[K, A]) Handle(ctx context.Context) {
ownedObjs := e.List(ctx, e.nn.MustValue(ctx))

newObj := e.newObj(ctx)
// newObj may have signaled a requeue (e.g. RequeueAPIErr) which cancels
// the handler context; bail out before dereferencing the result.
if ctx.Err() != nil {
return
}
hash := e.Hash(newObj)
newObj = newObj.WithAnnotations(map[string]string{e.HashAnnotationKey: hash})

Expand Down
82 changes: 82 additions & 0 deletions component/ensure_component_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package component

import (
"context"
"errors"
"fmt"
"testing"
"time"

"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -174,3 +176,83 @@ func TestEnsureServiceHandler(t *testing.T) {
})
}
}

// TestEnsureComponentByHashStopsWhenNewObjSignalsRequeue guards against a panic
// where a newObj factory both calls a queue operation (e.g. RequeueAPIErr) and
// returns nil. RequeueAPIErr cancels the handler context but does not unwind
// the goroutine, so without a ctx.Err() check the framework would dereference
// the nil to attach a hash annotation and segfault. Reproduces the panic
// observed at authzed/internal#10787.
func TestEnsureComponentByHashStopsWhenNewObjSignalsRequeue(t *testing.T) {
const (
hashKey = "example.com/component-hash"
ownerIndex = "owner"
)

ctx, cancel := context.WithCancel(t.Context())
defer cancel()

serviceGVR := corev1.SchemeGroupVersion.WithResource("services")
scheme := runtime.NewScheme()
require.NoError(t, corev1.AddToScheme(scheme))
client := clientfake.NewSimpleDynamicClient(scheme)
informerFactory := dynamicinformer.NewDynamicSharedInformerFactory(client, 0)
require.NoError(t, informerFactory.ForResource(serviceGVR).Informer().AddIndexers(map[string]cache.IndexFunc{
ownerIndex: func(_ interface{}) ([]string, error) {
return []string{types.NamespacedName{Namespace: "test", Name: "owner"}.String()}, nil
},
}))
informerFactory.Start(ctx.Done())
informerFactory.WaitForCacheSync(ctx.Done())
indexer := typed.NewIndexer[*corev1.Service](informerFactory.ForResource(serviceGVR).Informer().GetIndexer())
ctxOwner := typedctx.WithDefault[types.NamespacedName](types.NamespacedName{Namespace: "test", Name: "owner"})

queueOps := queue.NewQueueOperationsCtx()

// Simulate the manager plumbing: a real Operations whose cancel func
// cancels the handler-scoped context, so RequeueAPIErr propagates via
// ctx.Err() the same way it does in production.
handlerCtx, cancelHandler := context.WithCancel(ctx)
defer cancelHandler()
ops := queue.NewOperations(func() {}, func(time.Duration) {}, cancelHandler)
handlerCtx = queueOps.WithValue(handlerCtx, ops)

simulatedErr := errors.New("simulated upstream failure")
applyCalled := false
deleteCalled := false

h := handler.NewHandler(NewEnsureComponentByHash(
NewHashableComponent[*corev1.Service](
NewIndexedComponent(
indexer,
ownerIndex,
func(_ context.Context) labels.Selector {
return labels.SelectorFromSet(map[string]string{
"example.com/component": "the-main-service-component",
})
}),
hash.NewObjectHash(), hashKey),
ctxOwner,
queueOps,
func(_ context.Context, _ *applycorev1.ServiceApplyConfiguration) (*corev1.Service, error) {
applyCalled = true
return nil, nil
},
func(_ context.Context, _ types.NamespacedName) error {
deleteCalled = true
return nil
},
func(ctx context.Context) *applycorev1.ServiceApplyConfiguration {
queueOps.RequeueAPIErr(ctx, simulatedErr)
return nil
}), "ensureService")

require.NotPanics(t, func() {
h.Handle(handlerCtx)
})

require.False(t, applyCalled, "must not apply when newObj signaled requeue")
require.False(t, deleteCalled, "must not delete when newObj signaled requeue")
require.Equal(t, simulatedErr, ops.Error(), "Operations must record the requeue error")
require.Error(t, handlerCtx.Err(), "handler context must be cancelled by RequeueAPIErr")
}
Loading