diff --git a/component/ensure_component.go b/component/ensure_component.go index ad5ff6a..c70db2c 100644 --- a/component/ensure_component.go +++ b/component/ensure_component.go @@ -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}) diff --git a/component/ensure_component_test.go b/component/ensure_component_test.go index de31ae8..06c1a24 100644 --- a/component/ensure_component_test.go +++ b/component/ensure_component_test.go @@ -2,8 +2,10 @@ package component import ( "context" + "errors" "fmt" "testing" + "time" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -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") +}