Extract shared WorkloadReference helpers into controllerutil#4520
Merged
ChrisJBurns merged 4 commits intomainfrom Apr 3, 2026
Merged
Extract shared WorkloadReference helpers into controllerutil#4520ChrisJBurns merged 4 commits intomainfrom
ChrisJBurns merged 4 commits intomainfrom
Conversation
The ReferencingWorkloads pattern was duplicated across four config controllers with inconsistent sorting behavior: MCPTelemetryConfig and MCPExternalAuthConfig sorted refs, but MCPToolConfig and MCPOIDCConfig did not. This caused unnecessary API server writes when the same set of workloads was discovered in a different list order across reconcile runs. Extract SortWorkloadRefs, WorkloadRefsEqual, and FindWorkloadRefsFromMCPServers into the shared controllerutil package so all controllers use deterministic, consistent ordering. Each controller's findReferencingWorkloads now delegates to the shared helper, removing ~50 lines of duplicated list-filter-build logic. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4520 +/- ##
==========================================
+ Coverage 69.02% 69.10% +0.07%
==========================================
Files 502 502
Lines 52008 51997 -11
==========================================
+ Hits 35899 35933 +34
+ Misses 13320 13276 -44
+ Partials 2789 2788 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
JAORMX
previously approved these changes
Apr 3, 2026
Define WorkloadKindMCPServer, WorkloadKindVirtualMCPServer, and WorkloadKindMCPRemoteProxy constants in the API types and use them across all controllers. This fixes a goconst lint violation where "MCPServer" appeared as a raw string literal 4+ times. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 tasks
rdimitrov
approved these changes
Apr 3, 2026
ChrisJBurns
added a commit
that referenced
this pull request
Apr 3, 2026
Replace hardcoded "MCPRemoteProxy" strings with the shared constant from PR #4520 for consistency with all other controllers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
MatteoManzoni
pushed a commit
to DocPlanner/toolhive
that referenced
this pull request
Apr 4, 2026
…k#4520) * Extract shared WorkloadReference helpers into controllerutil The ReferencingWorkloads pattern was duplicated across four config controllers with inconsistent sorting behavior: MCPTelemetryConfig and MCPExternalAuthConfig sorted refs, but MCPToolConfig and MCPOIDCConfig did not. This caused unnecessary API server writes when the same set of workloads was discovered in a different list order across reconcile runs. Extract SortWorkloadRefs, WorkloadRefsEqual, and FindWorkloadRefsFromMCPServers into the shared controllerutil package so all controllers use deterministic, consistent ordering. Each controller's findReferencingWorkloads now delegates to the shared helper, removing ~50 lines of duplicated list-filter-build logic. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add WorkloadKind constants to replace string literals Define WorkloadKindMCPServer, WorkloadKindVirtualMCPServer, and WorkloadKindMCPRemoteProxy constants in the API types and use them across all controllers. This fixes a goconst lint violation where "MCPServer" appeared as a raw string literal 4+ times. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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 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
SortWorkloadRefs,CompareWorkloadRefs,WorkloadRefsEqual, andFindWorkloadRefsFromMCPServersinto the sharedcontrollerutilpackage. All four controllers now delegate to these helpers, ensuring consistent deterministic ordering and removing ~50 lines of duplicated code.Type of change
Test plan
task test)task lint-fix)Changes
cmd/thv-operator/pkg/controllerutil/config.goSortWorkloadRefs,CompareWorkloadRefs,WorkloadRefsEqual,FindWorkloadRefsFromMCPServersshared helperscmd/thv-operator/pkg/controllerutil/config_test.gocmd/thv-operator/controllers/toolconfig_controller.gofindReferencingWorkloadsto shared helper; add sorting inhandleConfigHashChangecmd/thv-operator/controllers/mcpexternalauthconfig_controller.gofindReferencingWorkloadswith shared helperscmd/thv-operator/controllers/mcpoidcconfig_controller.gocmd/thv-operator/controllers/mcptelemetryconfig_controller.goworkloadRefsEqual, andfindReferencingWorkloadswith shared helpersSpecial notes for reviewers
handleDeletionpatterns are also duplicated but require more involved generalization (interface/generic approaches for different config list types). Those are better addressed as part of the shared config resource lifecycle work in toolhive-rfcs#65.goconstlint issue for"MCPServer"string inmcpexternalauthconfig_controller.gois not addressed here — it predates this change.Generated with Claude Code