feat: watch owned resources and user-provided TLS secrets for prompt reconciliation#142
Open
viniciusdc wants to merge 1 commit into
Open
feat: watch owned resources and user-provided TLS secrets for prompt reconciliation#142viniciusdc wants to merge 1 commit into
viniciusdc wants to merge 1 commit into
Conversation
…reconciliation The controller only watched NebariApp (via For) plus cert-manager Certificates, so drift in the resources it manages was only repaired on the periodic requeue (up to a minute). Add event-driven watches. Owned resources (#30): - SetupWithManager now Owns HTTPRoute, SecurityPolicy, and Secret, so external edits or deletions of those trigger reconciliation immediately. HTTPRoutes and SecurityPolicies already carry controller owner references; the OIDC client Secret did not, so storeClientSecret now sets one (and adopts pre-existing secrets on the next sync). KeycloakProvider gains a Scheme field, wired from the manager in main.go, for SetControllerReference; a nil Scheme skips the owner reference so existing tests without a scheme still pass. User-provided TLS secrets (#115): - Add a filtered Watches on Secrets in the Gateway namespace mapped back to any NebariApp referencing them via routing.tls.secretName. Creating or fixing a referenced secret now flips TLSReady promptly instead of on the periodic requeue. The map function lists NebariApps from the controller cache, so the per-event cost is negligible. Tests: - TestSecretToNebariApp covers the map function (match, no spurious requeues). - TestKeycloakProvider_StoreClientSecret_SetsOwnerReference asserts the secret is created with a controller owner reference to the NebariApp. Closes #30 Closes #115
Docker Images BuiltImages pushed to Quay.io for branch
Test the operator: kubectl apply -k https://github.com/nebari-dev/nebari-operator.git/config/default?ref=feat/owned-resource-watches
kubectl set image deployment/nebari-operator-controller-manager manager=quay.io/nebari/nebari-operator:feat-owned-resource-watches -n nebari-operator-system |
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
PR 3 of the routing/SecurityPolicy consolidation (#139). The controller only watched
NebariApp(viaFor) plus cert-managerCertificates, so drift in the resources it manages was only repaired on the periodic requeue (up to ~1 minute). This adds event-driven watches.Owned resources (closes #30)
SetupWithManagernowOwnsHTTPRoute,SecurityPolicy, andSecret, so external edits or deletions of those trigger reconciliation immediately.HTTPRoutes andSecurityPolicys already carry controller owner references; the OIDC clientSecretdid not.storeClientSecretnow sets one viacontrollerutil.SetControllerReference(and adopts pre-existing secrets on the next sync by writing the owner reference on update too).KeycloakProvidergains aSchemefield, wired frommgr.GetScheme()inmain.go. A nilSchemeskips the owner reference, so existing provider tests that don't wire a scheme keep passing.gatewayapiv1.Install,egv1alpha1.AddToScheme, core), so no scheme changes were needed.User-provided TLS secrets (closes #115)
WatchesonSecrets in the Gateway namespace (envoy-gateway-system), mapped back to any NebariApp referencing them viarouting.tls.secretName. Creating or fixing a referenced secret now flipsTLSReadypromptly instead of on the periodic requeue.Ownsabove, this watch is for the unowned, user-managed secret.Test plan
go build ./...,go vet ./...,gofmt -lclean.go test ./internal/controller/reconcilers/auth/...passes, including the newTestKeycloakProvider_StoreClientSecret_SetsOwnerReference.go test -run TestSecretToNebariApp ./internal/controller/passes (placed in a standalone test file so it runs without the envtest suite).internal/controllerGinkgo/envtest suite fails locally on a pre-existing scheme-registration gap (also fails on cleanmain; needsmake test's CRD setup) — unrelated to this change.Notes / follow-ups
Owns(&corev1.Secret{})makes the manager cache Secrets in watched namespaces. That is what enhancement: Add watches on owned resources (HTTPRoute, SecurityPolicy, Secret) #30 asks for; if the cluster-wide Secret cache becomes a concern, a label-selector cache is a reasonable later optimization.Ownswiring is the standard controller-runtime pattern and the owner-reference unit test covers the piece that was missing (the Secret owner ref).