feat: RBAC pre-flight check, containerlab fixes, controller refactor#131
Merged
Conversation
- Add RBAC pre-flight check at startup to surface missing list+watch permissions before manager starts (cmd/galactic-router/main.go) - Fix containerlab IPv6 kubectl TLS skip and lo-galactic interface setup for IPv6-only clusters - Update RBAC from legacy galactic-agent/BGPInstance to galactic-router with full CRD permissions (BGPRouter, BGPPeer, BGPAdvertisement, BGPPolicy, BGPVRFInstance, Node, Secret) - Remove ASN maximum patch from install-overlay (no longer needed) - Add BGP_LOCAL_ADDRESS env for iad-rr tenant router - Add route-map SET_SRC in FRR underlay to set src address for EVPN - Update cosmos API: Prefix type changed from AdvertisedPrefix.CIDR to direct Prefix string; add int32→uint32 conversion for LOCAL_PREF - Refactor BGPAdvertisement controller: move Watches into BGPRouter SetupWithManager, remove redundant reconciler - Fix field index name typos in status update list calls Co-Authored-By: Claude <noreply@anthropic.com>
scotwells
reviewed
Jun 25, 2026
Comment on lines
+218
to
+223
| if err := c.List(ctx, objList, client.Limit(1)); err != nil { | ||
| if apierrors.IsForbidden(err) { | ||
| logger.Error(err, "missing list+watch RBAC", | ||
| "detail", "informer cache will not sync; add resource to ServiceAccount ClusterRole and restart") | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
I'd recommend doing a SelfSubjectAccessReview check here. It's a simple permission check instead having to list resources. You can also check for the Watch permission when this is only checking the list permission.
scotwells
previously approved these changes
Jun 25, 2026
Contributor
|
Assume we should have also have alerts that catch these situations? E.g. the controller is erroring when making API requests |
- Replace List() calls with SelfSubjectAccessReview to check watch permission without reading cluster data - Check the watch verb explicitly instead of relying on list API behavior - Remove unused corev1 and apierrors imports - Use plain resource name structs instead of CRD List types
privateip
force-pushed
the
feat/galactic-router-updates
branch
from
2198ed1 to
877920b
Compare
Contributor
Author
yes, i have held off putting alerts into galactic until the code settled down. Im very close to starting that work now across the board |
scotwells
approved these changes
Jun 25, 2026
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
2 participants
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
This is a set of patches to fix the containerlab deployment to work with the latest CRDs published by Cosmos. It also adds a new function to log RBAC failures before attempting to start
galactic-router. Prior to this patch,galatic-routerwould look like it starts fine but never established peer connections.Changes
checkWatchPermissions()issues a test list for every resource the manager watches at startup. If any return Forbidden, logs a clear actionable message so cache sync failures are immediately visible rather than silent.lo-galacticdummy interface with IPv6 address for transit mesh routing.galactic-agent→galactic-router; update apiGroups fromproviders.bgp.miloapis.com→bgp.miloapis.com; add permissions for all new CRDs (BGPRouter, BGPPeer, BGPAdvertisement, BGPPolicy, BGPVRFInstance) plus Node and Secret.BGP_LOCAL_ADDRESSenv for iad-rr tenant; add FRR route-map SET_SRC to set src address for EVPN routes.fix-asn-maximum.shno longer needed after cosmos API update.Prefixtype changed fromAdvertisedPrefix.CIDRto directPrefixstring; added int32→uint32 conversion for LOCAL_PREF (cosmos uses int32, GoBGP uses uint32 per RFC 4271).BGPAdvertisementWatches handler intoBGPRouter.SetupWithManager; removed redundant empty reconciler.BGPPeerByRouterName→BGPAdvByRouterNameandBGPPeerByRouterName→BGPPolicyByRouterNamein status update list calls.