trust-manager icon indicating copy to clipboard operation
trust-manager copied to clipboard

Split Bundle controller into multiple controllers

Open erikgb opened this issue 7 months ago • 2 comments

We currently have a single controller in trust-manager, and this controller has a lot of responsibilities. I can identify at least the following:

  • Prepare target configmap/secret content from sources (buildSourceBundle)
  • Create/delete target configmaps/secrets in namespaces matching namespaceSelector
  • Ensure target configmaps/secrets CA bundle content is up-to-date

Since we always reconcile bundles, this creates a long and quite complex reconcile process. I have been looking a implementing https://github.com/cert-manager/trust-manager/issues/58, and that would a least require a new controller. Introducing a new controller is doable, but we would need a lot of the logic from the existing bundle controller. And waste compute resources by building everything from scratch.

I would suggest splitting the single existing controller into:

  • Controller for bundles + watching sources configmaps/secrets (in trust namespace). The output from this controller should be a configmap/secret in trust namespace acting as a prototype for all target configmaps/secrets.
  • Controller for bundles (namespaceSelector) + watching namespaces. This controller will be responsible for creating/deleting target configmaps/secrets in namespaces matching/not-matching namespaceSelector. New targets will be created from the bundle target prototype resource.
  • Controller for bundle target prototype configmaps/secrets. It will ensure target configmaps/secrets are good copies of their prototype. Note: This controller should not create/delete target configmaps/secrets.
  • Controller for target configmaps/secrets. It should simply ensure the target configmap/secret is a good copy of it's prototype.

This might seem complex, and I am happy to discuss adjustments. 😃 I think the first controller is the most important, as it will ensure we process bundle sources once per bundle.

I see the following benefits from this refactoring:

  • More efficient processing, avoiding doing potentially heavy processing over and over again.
  • No need to have conditional operations requiring helpers with dedicated tests, ref. https://github.com/cert-manager/trust-manager/pull/236
  • Better separation of concern. If you want to read the current bundle reconcile from start to finish you have to look through most of the code in trust-manager. 😉
  • Easier testing - as it will only be the first controller that needs to work with X.509 stuff. The rest is just technical Kubernetes controller operations - that can be tested with dummy trust bundle content.

A potential downside/challenge, is maintaining the Bundle Synced condition. But do we really need it? What about heading for the kstatus approach, where the idea is to add conditions when something is wrong.

CC @inteon @SgtCoDFish

erikgb avatar Nov 26 '23 12:11 erikgb

Hello, If you need help with this bunch of tasks, I kindly help you with that!

arsenalzp avatar Dec 15 '23 12:12 arsenalzp

If you need help with this bunch of tasks, I kindly help you with that!

Thanks @arsenalzp, but we have discussed this a bit and decided to put it a bit on hold. It will increase the complexity of our controller machinery. So not sure if we ever want to do this.

erikgb avatar Dec 15 '23 12:12 erikgb