From 5b6b801a0b4f9785ade1418b78286bfc4aeb587c Mon Sep 17 00:00:00 2001 From: suzi1037 Date: Thu, 21 Aug 2025 22:01:20 +0800 Subject: [PATCH 1/2] refactor(builder): Consolidate validation logic and simplify duplicated watch-related code --- pkg/builder/controller.go | 43 +++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/pkg/builder/controller.go b/pkg/builder/controller.go index 6d906f6e52..015326548b 100644 --- a/pkg/builder/controller.go +++ b/pkg/builder/controller.go @@ -273,6 +273,18 @@ func (blder *TypedBuilder[request]) Build(r reconcile.TypedReconciler[request]) if blder.forInput.err != nil { return nil, blder.forInput.err } + if blder.forInput.object == nil { + if len(blder.ownsInput) > 0 { + return nil, errors.New("Owns() can only be used together with For()") + } + if len(blder.watchesInput) == 0 && len(blder.rawSources) == 0 { + return nil, errors.New("there are no watches configured, controller will never get triggered. Use For(), Owns(), Watches() or WatchesRawSource() to set them up") + } + } else { + if reflect.TypeFor[request]() != reflect.TypeOf(reconcile.Request{}) { + return nil, fmt.Errorf("For() can only be used with reconcile.Request, got %T", *new(request)) + } + } // Set the ControllerManagedBy if err := blder.doController(r); err != nil { @@ -305,6 +317,9 @@ func (blder *TypedBuilder[request]) project(obj client.Object, proj objectProjec } func (blder *TypedBuilder[request]) doWatch() error { + + var sources []source.TypedSource[request] + // Reconcile type if blder.forInput.object != nil { obj, err := blder.project(blder.forInput.object, blder.forInput.objectProjection) @@ -312,24 +327,15 @@ func (blder *TypedBuilder[request]) doWatch() error { return err } - if reflect.TypeFor[request]() != reflect.TypeOf(reconcile.Request{}) { - return fmt.Errorf("For() can only be used with reconcile.Request, got %T", *new(request)) - } - var hdler handler.TypedEventHandler[client.Object, request] reflect.ValueOf(&hdler).Elem().Set(reflect.ValueOf(&handler.EnqueueRequestForObject{})) allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...) allPredicates = append(allPredicates, blder.forInput.predicates...) src := source.TypedKind(blder.mgr.GetCache(), obj, hdler, allPredicates...) - if err := blder.ctrl.Watch(src); err != nil { - return err - } + sources = append(sources, src) } // Watches the managed types - if len(blder.ownsInput) > 0 && blder.forInput.object == nil { - return errors.New("Owns() can only be used together with For()") - } for _, own := range blder.ownsInput { obj, err := blder.project(own.object, own.objectProjection) if err != nil { @@ -349,15 +355,10 @@ func (blder *TypedBuilder[request]) doWatch() error { allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...) allPredicates = append(allPredicates, own.predicates...) src := source.TypedKind(blder.mgr.GetCache(), obj, hdler, allPredicates...) - if err := blder.ctrl.Watch(src); err != nil { - return err - } + sources = append(sources, src) } // Do the watch requests - if len(blder.watchesInput) == 0 && blder.forInput.object == nil && len(blder.rawSources) == 0 { - return errors.New("there are no watches configured, controller will never get triggered. Use For(), Owns(), Watches() or WatchesRawSource() to set them up") - } for _, w := range blder.watchesInput { projected, err := blder.project(w.obj, w.objectProjection) if err != nil { @@ -365,11 +366,13 @@ func (blder *TypedBuilder[request]) doWatch() error { } allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...) allPredicates = append(allPredicates, w.predicates...) - if err := blder.ctrl.Watch(source.TypedKind(blder.mgr.GetCache(), projected, w.handler, allPredicates...)); err != nil { - return err - } + src := source.TypedKind(blder.mgr.GetCache(), projected, w.handler, allPredicates...) + sources = append(sources, src) } - for _, src := range blder.rawSources { + + sources = append(sources, blder.rawSources...) + + for _, src := range sources { if err := blder.ctrl.Watch(src); err != nil { return err } From 292cb5263fd5bcbb7ccf128f03255c0d975f41b2 Mon Sep 17 00:00:00 2001 From: suzi1037 Date: Thu, 21 Aug 2025 22:16:11 +0800 Subject: [PATCH 2/2] refactor(builder): put forInput validation in For() --- pkg/builder/controller.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/builder/controller.go b/pkg/builder/controller.go index 015326548b..7d6ea9d67e 100644 --- a/pkg/builder/controller.go +++ b/pkg/builder/controller.go @@ -93,8 +93,16 @@ type ForInput struct { func (blder *TypedBuilder[request]) For(object client.Object, opts ...ForOption) *TypedBuilder[request] { if blder.forInput.object != nil { blder.forInput.err = fmt.Errorf("For(...) should only be called once, could not assign multiple objects for reconciliation") + } + + if reflect.TypeFor[request]() != reflect.TypeOf(reconcile.Request{}) { + blder.forInput.err = fmt.Errorf("For() can only be used with reconcile.Request, got %T", *new(request)) + } + + if blder.forInput.err != nil { return blder } + input := ForInput{object: object} for _, opt := range opts { opt.ApplyToFor(&input) @@ -280,10 +288,6 @@ func (blder *TypedBuilder[request]) Build(r reconcile.TypedReconciler[request]) if len(blder.watchesInput) == 0 && len(blder.rawSources) == 0 { return nil, errors.New("there are no watches configured, controller will never get triggered. Use For(), Owns(), Watches() or WatchesRawSource() to set them up") } - } else { - if reflect.TypeFor[request]() != reflect.TypeOf(reconcile.Request{}) { - return nil, fmt.Errorf("For() can only be used with reconcile.Request, got %T", *new(request)) - } } // Set the ControllerManagedBy