Skip to content

Conversation

stephank
Copy link
Contributor

@stephank stephank commented Apr 16, 2025

An attempt at #7.

This generalizes the informer code so adding more informers doesn't involve a bunch of boilerplate. It then adds the Service and EndpointSlice informers necessary to keep the upstreams updated.

This also includes some work on secrets, to try keep them out of memory, because the informer normally builds a cache of everything. Especially without -namespace, we were just kinda keeping around every secret anywhere in the cluster in memory.

@stephank
Copy link
Contributor Author

After some more testing, I noticed the informer cache shifts results every now and then. I suspect periodic reconciliation. This caused the output to shift as well, and frequent reloads. I now sort the informer results before processing.

I also added a simple helper to limit logging of warning messages generated by plugins, so it is safe to log from plugins even though they run quite often.

Copy link

codecov bot commented May 15, 2025

Codecov Report

Attention: Patch coverage is 26.00473% with 313 lines in your changes missing coverage. Please review.

Project coverage is 21.97%. Comparing base (7db5cd4) to head (1bd8fb3).

Files with missing lines Patch % Lines
internal/caddy/global/tls.go 22.05% 51 Missing and 2 partials ⚠️
internal/controller/queued_event_handler.go 0.00% 52 Missing ⚠️
pkg/store/store.go 59.78% 34 Missing and 3 partials ⚠️
internal/caddy/ingress/reverseproxy.go 50.81% 24 Missing and 6 partials ⚠️
internal/controller/action_configmap.go 0.00% 26 Missing ⚠️
internal/controller/controller.go 0.00% 26 Missing ⚠️
internal/controller/action_ingress.go 0.00% 25 Missing ⚠️
internal/controller/action_endpointslice.go 0.00% 17 Missing ⚠️
internal/controller/action_service.go 0.00% 17 Missing ⚠️
internal/controller/diagnostics.go 0.00% 16 Missing ⚠️
... and 4 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #310      +/-   ##
==========================================
+ Coverage   20.11%   21.97%   +1.85%     
==========================================
  Files          30       33       +3     
  Lines        1397     1502     +105     
==========================================
+ Hits          281      330      +49     
- Misses       1114     1159      +45     
- Partials        2       13      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

stephank added 3 commits May 15, 2025 11:35
This replaces our own Store.Ingresses list with a method Ingresses()
that derives the list from the informer cache.

This also adds a ResourceEventHandler implementation that automatically
submits via the queue. The intention is to reduce boilerplate as we add
more informers to the code.
The isManagedTLSSecret helper relied on ingresses, but wasn't checked on
an update to ingresses. This instead moves all checks to the plugin.

In addition, creating a secret informer builds a cache of all secrets
in memory. This is undesirable, because it means we keep around
sensitive data from completely unrelated secrets.

Instead, a transform is installed on the informer to throw away all
secret data, so we only keep metadata. The plugin is then responsible
for fetching actual secret data for only the secrets we need.

In practice, we are still dependant on the Go garbage collector.
@stephank
Copy link
Contributor Author

I rebased this on main and fixed the conflicts. Did some testing and still good in my setup.

stephank added 3 commits June 4, 2025 15:58
This ensures output is consistent when there are no changes, and fixes
spurious reloads.
@stephank
Copy link
Contributor Author

stephank commented Jun 4, 2025

I didn't notice the failing test, but it was simply missing permissions for watching EndpointSlices in the chart. That's fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant