Repository navigation
refactor(bridge): move 3P wrapper reconciliation out of reconciler.go - #1011
Merged
VLukiantsev merged 1 commit intoOct 2, 2026
Merged
Conversation
Third of four pure-move layers splitting pkg/components/bridge/reconciler.go by responsibility, following the file layout agreed in the #992 review. - wrapper_reconciler.go: the 3P wrapper and operator-less (Profile 3) path: resolveAndValidateWrapperRef, resolveOperatorlessEndpoint, reconcile3PWrapper, reconcile3PWrapperFallback, reconcileOperatorlessWrapper, verifyEndpointHealth, handle3PUnreadyStatus, handle3PReadyStatus and populateAccessEnvVars. - wrapper_reconciler_test.go: TestReconcileComponent_3PWrapperAdvanced, TestReconcile3PWrapper_CrossNamespaceForbidden, TestPopulateAccessEnvVars, TestReconcile_OperatorlessProfile3 and TestResolveOperatorlessEndpointAndIPv6 (moved intact; it also covers two prober endpoint helpers). - Drop the now-unused reflect import from reconciler_test.go. No renames, no behaviour or public API changes. Review with: git diff --color-moved=dimmed-zebra TAG=agy CONV=25ae2278-aa0f-44db-af8a-4f9deda0d399
There was a problem hiding this comment.
Code Review
This pull request refactors the codebase by moving third-party wrapper reconciliation logic and its associated tests from reconciler.go and reconciler_test.go into dedicated files, wrapper_reconciler.go and wrapper_reconciler_test.go. The feedback highlights two improvement opportunities in the newly created wrapper_reconciler.go: simplifying a redundant endpoint check in reconcile3PWrapper and removing dead code in reconcile3PWrapperFallback where a nil check is performed on a prober that is already guaranteed to be nil.
AntonTyb
approved these changes
Sep 29, 2026
Collaborator
There was a problem hiding this comment.
as usual - feels like more logs could be added...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request
31B4
is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part 3/4 of the pure-move stack splitting
pkg/components/bridge/reconciler.go. This PR moves the 3P wrapper and operator-less (Profile 3) reconciliation path intowrapper_reconciler.go, along with its tests. There are no behaviour, API or naming changes.Motivation & Context
This implements the
wrapper_reconciler.gofile from the layout agreed in the #992 review.Review with:
git diff --color-moved=dimmed-zebra origin/refactor/bridge-split-cms-prober...origin/refactor/bridge-split-wrapperKey Changes
wrapper_reconciler.go:resolveAndValidateWrapperRef,resolveOperatorlessEndpoint,reconcile3PWrapper,reconcile3PWrapperFallback,reconcileOperatorlessWrapper,verifyEndpointHealth,handle3PUnreadyStatus,handle3PReadyStatus,populateAccessEnvVars.wrapper_reconciler_test.go:TestReconcileComponent_3PWrapperAdvanced,TestReconcile3PWrapper_CrossNamespaceForbidden,TestPopulateAccessEnvVars,TestReconcile_OperatorlessProfile3,TestResolveOperatorlessEndpointAndIPv6.reflectimport fromreconciler_test.go, which the move left unused.Assumptions
TestResolveOperatorlessEndpointAndIPv6moved intact, even though it also asserts twobridge_prober.gohelpers (resolveProbeEndpoint,resolveConnectionEndpoint). Splitting it would change test code, which is out of scope for a pure move — impact if wrong: a small follow-up to split it.verify3PStatusDetectorAssertionsandassertCoordinateEnvVarsstay inreconciler_test.gobecause only the mixedTestReconcileComponent_Scenariostable uses them.Verification & Testing
bazel test //pkg/components/bridge:bridge_test,./.githooks/pre-commitRelated Issues / Bugs
Follow-up to the #992 review.
TAG=agy
CONV=25ae2278-aa0f-44db-af8a-4f9deda0d399