Extract variant branches into a generic extension mechanism by caseydavenport · Pull Request #4871 · tigera/operator

caseydavenport · 2026-05-29T21:09:53Z

This is phase 1 of prepping the operator for the monorepo merge, where the Calico and Calico Enterprise code paths eventually live apart. Today they share one codebase with IsEnterprise() checks sprinkled through the render and controller code, and that coupling is the thing that makes the split hard.

This PR pulls the enterprise-specific behavior out of the core code and behind a generic extension mechanism, so the enterprise build registers its own additions and the core operator stays variant-blind. After this, core render and controller code has no idea enterprise exists.

How it works

Enterprise registers extensions against a set keyed by variant. There are two extension points, one per phase of a reconcile:

Controller phase: a per-controller hook does the variant's reconcile-time work (validating config, creating certificates, assembling the trusted bundle, registering extra watches) and hands the result to the render phase.
Render phase: a per-component modifier runs at the single point where the operator writes a component's objects, wrapping the component and adjusting its output. A separate image-override registry lets a variant swap an image without core branching.

The core operator registers nothing and runs the base path. All the enterprise wiring lives in pkg/enterprise, one subpackage per component. After the split, that package is what the enterprise build constructs and the core build drops.

Every extracted component (node, typha, guardian, windows, apiserver, kube-controllers) and the clusterconnection controller now run their enterprise behavior through this mechanism, with no IsEnterprise() left in their core paths. Behavior is unchanged: the test gate is the existing core tests plus the relocated enterprise tests, which now run against the real extension set.

A few shared-code cleanups and ergonomic refactors are left as follow-ups, tracked in CORE-13042.

None

…n keying

Add WithContext/ComponentHandlerOption to NewComponentHandler (variadic, backward-compatible) and call operator.ApplyPatches in CreateOrUpdateOrDelete for components implementing render.Named.

Pulls the enterprise RBAC extra-rules and MULTI_INTERFACE_MODE env branches out of pkg/render/typha.go into a new pkg/enterprise package. The enterprise package registers a patch via operator.Patch on startup; pkg/render/typha.go now has zero IsEnterprise branches.

Calls enterprise.Register() at startup so the typha modifier is wired in. Builds an operator.Context in the installation reconciler and passes it to the component handler so registered modifiers receive reconcile-derived state.

Extracts the image override registry into a leaf pkg/imageoverride package (no render/operator transitive deps) to avoid the render→operator import cycle. operator.OverrideImage/ResolveImage now delegate there. Registers the enterprise node image override in pkg/enterprise. Removes the IsEnterprise image switch from render/node.go; FIPS handling is preserved via a post-resolve check.

…sion The OSS installation controller no longer directly creates the node-prometheus keypair or fetches the prometheus/esgw certs. Those are now handled by a registered InstallationExtension in pkg/enterprise. Port value derivation and the kube-controller TLS block remain in the OSS controller unchanged.

Moves the calico-node-metrics Service out of OSS node render and into the enterprise node modifier, where it derives ports from ctx.FelixConfiguration. Also exports NodeBGPReporterPort so the modifier can reference it.

…se in tests

# Conflicts: # pkg/controller/installation/core_controller_test.go # pkg/controller/utils/component.go # pkg/render/node.go

The registry package is renamed to extensions. The installation controller builds the render context through a registered factory, and the componentHandler applies registered modifiers to component output. The node and typha variant branches now live in enterprise modifiers, and the calico log directory is mounted for both variants.

Drop the functional-options builder for Inputs (one call site, all fields always set) in favor of a plain struct literal, and replace the single-method RenderContextFactory interface with a registered builder func. All three extension seams now register a func.

Register modifiers, image overrides, and the render context builder per variant. The registries now gate on the installation variant, so the enterprise funcs drop their self-gate guards (the IsEnterprise checks the PR set out to remove) and the image override drops its decline bool - it only runs for its own variant. Move the node prometheus reporter keypair mounting (volume, mount, cert-management init container, pod hash annotation) into the node modifier, and remove NodeConfiguration.PrometheusServerTLS along with the round-trip through the installation controller. Core node render no longer carries a prometheus mount; in calico the keypair is never created. Rename Extensible.Name() to ModifierKey() so an unrelated Name() method can't make a component modifier-eligible by accident.

Merge the per-component image override and modifier into a single Extension{Image, Modify} registered once per (variant, component) via extensions.Register, so all of a component's variance lives in one place. The image half still lands in the imageoverride leaf so render resolves it without an import cycle; the fan-out is internal to Register. Rename the variant-level render context builder to Setup (RegisterSetup/RunSetup). That names the two phases a reader has to hold: Setup is the controller-side work that builds the RenderContext baton, and Extension hooks are the pure render-time funcs. Three registries with three key schemes become two concepts split by when they run.

modCtx read like "modifier context"; the value is an extensions.RenderContext, so name it for what it is.

Add a package doc that lays out the two-phase model (Setup vs Extension) so the whole seam is legible from `go doc`. Fix two comments that still called the setup a render context builder.

Add a per-component context channel: a component implements render.ExtensionContextProvider to hand its modifier config a modifier can't derive from the shared RenderContext (config only the component's controller has). The componentHandler reads it into RenderContext.Component before applying the modifier. node's setup-produced keypair keeps its own field; this is for component-config-derived inputs. Move windows's enterprise branches into a pkg/enterprise extension: the two windows image overrides, the node-metrics Service, the calico log volume (swapped in for the OSS cni-log mount), the enterprise felix env, the trusted DNS servers for openshift/rke2, and the prometheus reporter keypair mount. The windows component exposes its reporter port, keypair, and trusted bundle via ExtensionContext; the windows controller wires the render context into its handler. Core windows render is now OSS-only.

A modifier used to be registered against a bare component name, with its inputs type inferred from the modifier itself. Nothing tied the two together, so a modifier written for one component registered happily against another and only failed at render time, as a BUG log and a silently unmodified component. Registration now takes the component's ModifierKey, which pins the type of the inputs its modifier receives. Cfg comes from the key rather than from the modifier, so a mismatch does not compile. Keys can only be declared in render, next to the component they belong to. Every extension point gets its own inputs type, empty ones included - sharing a single empty type between components would let the compiler accept a modifier registered against the wrong one. That also means Variant.Modify and RegisterModifier collapse into one generic extensions.Modify.

A modifier finds the container it layers onto by name, and both sides were writing that name out independently: render built the container from a literal and the modifier matched on its own copy. install-cni existed three times over (render, the modifier, and the modifier's test fixture) with nothing tying them together, so renaming it in render left the modifier compiling and silently not matching. Render now declares those names and uses them at the container it names, and the modifiers match on the same constants. Picking containers out of a PodSpec also moves behind render.Container / render.Containers, which log rather than skip quietly when nothing matches - a modifier asking for a container that is no longer rendered is a bug on one side or the other. The modifier tests that ran against hand-built DaemonSets now run against real render output. A fixture only ever matches the shape render had when it was written, which is what let this go unnoticed: with the container renamed at the creation site, every affected enterprise suite now fails.

…sions Conflict in pkg/render/apiserver_test.go: master added two specs while this branch moved the Calico Enterprise block of that file out to pkg/enterprise/apiserver. The spec master added to the Calico block merges as is. The one it added to the enterprise block is ported over, covering the half that was not already there: the existing v3-CRD spec asserts the deployment survives, so this adds the APIService assertion. It runs post-modifier, since the base render is variant-blind here and queues the deployment for deletion until the enterprise modifier takes it back out.

Several were paragraphs restating what the declaration below them already said, which makes the code harder to scan rather than easier. No behavior change.

A modifier that can't find the container it is written against means render and the modifier have drifted apart. That is not recoverable, so render.MustContainer and extensions.Modify panic rather than logging and leaving the object half-modified. Not every container is guaranteed, though. The windows confd container only renders when BGP is enabled, so treating it as required would panic on every BGP-disabled Windows cluster. render.Container stays for that case and reports whether it found one. Also drops the render.ContainerName type: only three of the ten container-name constants used it, which forced a cast wherever they met a plain string.

It was embedded, so the field was called Inputs and construction read Inputs: render.Inputs{...}. Naming it RenderInputs says where the field came from at the point of use, at the cost of the promoted accesses getting longer.

Every modifier key now pins a distinct inputs type, so a modifier registered against the wrong key fails to compile.

The Calico Cloud gate landed on master in code this branch had already moved, so the cloud-gated UISettings/lma RBAC now lives in the enterprise apiserver extension and the es-kube-controllers cluster role verbs in pkg/enterprise/kubecontrollers.

The base kube-controllers render was reading cfg.TenantID, which only the es-kube-controllers controller ever sets.

Only the enterprise apiserver modifier reads it, so the API server render configuration should not carry it. The flag comes from the build rather than the cluster, so main hands it to enterprise.New.

Master resolves the variant at startup, so the extension wiring and the reconciler gates key on opts.Variant instead of the EnterpriseCRDExists flag it replaced.

The variant is now a startup fact, so the extension Set no longer needs to route by it. It becomes a registry built for the one variant the operator runs as: lookups are by controller name and return a no-op when nothing is registered, so the nil-Set guards and the handler's forgotten-wiring warning go away. Behavior moves onto the ControllerExtension, Watcher and decorator interfaces, and the component handler now takes a decorator interface it declares itself, which drops its dependency on the extensions package. The variant's controller-phase options are computed once and held by the hooks that need them, so controller.Inputs loses its opaque Options field.

The no-op contract was explained in four places; it belongs on Registry alone.

A controller called Validate and ExtendInputs back to back, and the API server extension read the same two CRs in each. Validation now happens inside ExtendInputs, which rejects unsupported configuration with an error matching ErrInvalidConfig so the controller still degrades with a validation reason rather than a create reason.

caseydavenport

Claude: A pass looking only for behavior that changed in the extraction, skipping the structural and naming ground already covered in the existing comments. Everything below was checked against origin/master rather than inferred.

The first three are RBAC and watches that master has and this branch doesn't. All three were already absent before today's master merges, so they came from earlier rounds: a merge hit a region the branch had deleted, took our side, and quietly dropped what master had added inside it. Worth a systematic diff of master's other extracted components against their pkg/enterprise copies before this lands. I did that sweep for the two API server user roles and the rule vocabulary comes back with exactly the two clusters below and nothing else.

The last two, on the decorator and the reporter gating, are judgement calls rather than defects - flagging for a decision, not asking for a change.

The API server user roles lost their Gateway API read rules and the rbacUI role management rules, the installation extension lost its Manager watch, and the typha modifier only reached the cluster Typha, not the non-cluster-host one. Also pass the clusterconnection handler the inputs ExtendInputs returned, match removeByRef on the concrete type rather than an always-empty Kind, gate the node prometheus mount and its env on the same condition, keep the GatewayAPI read diagnostic, and skip decoration when the Installation asks for a variant this process didn't boot as, matching how the image overrides already resolve.

The product version reported in status comes from a ProductVersion companion the variant implements. The controller watches the Calico CRDs whatever it is running as, and the enterprise extension watches the ones it adds. The management cluster CRs are read by the hook rather than the controller, which means the base kube-controllers render no longer decides between reaching the manager directly or through Guardian, nor whether to bind the managed-cluster watch role - both move to the modifiers. The kube-controllers image resolves through the override registry, so Calico Cloud registers its image instead of the render branching on a flag.

A controller took the whole registry and looked its extension up on every reconcile. It now resolves once when it is built, so the call sites read r.ext.ExtendInputs and nothing downstream carries the registry. The resolved surface is a struct whose zero value is inert, which replaces the per-lookup null objects.

Master removed the Compliance feature, which included four Compliance ingress rules from the guardian calico-system policy. That policy body now lives in the enterprise guardian extension, so drop them there too.

caseydavenport added 14 commits May 28, 2026 16:08

operator: add extension Context type

9499353

operator: add patch registry

a5767f2

operator: add image-override registry

b448227

render: add Named interface and component name constants for extensio…

e651d7d

…n keying

utils: apply registered render patches in componentHandler

c1cd817

Add WithContext/ComponentHandlerOption to NewComponentHandler (variadic, backward-compatible) and call operator.ApplyPatches in CreateOrUpdateOrDelete for components implementing render.Named.

operator: add installation controller extension registry

cc62d3c

render: typha implements Named

e8c588f

render: fix node.go import ordering

0506f4c

enterprise: render node metrics service via a modifier

6e426cb

Moves the calico-node-metrics Service out of OSS node render and into the enterprise node modifier, where it derives ports from ctx.FelixConfiguration. Also exports NodeBGPReporterPort so the modifier can reference it.

variant-extensions: gofmt and replace deprecated TigeraSecureEnterpri…

c74b106

…se in tests

marvin-tigera added this to the v1.43.0 milestone May 29, 2026

marvin-tigera added docs-pr-required release-note-required labels May 29, 2026

caseydavenport added docs-not-required release-note-not-required labels May 29, 2026

marvin-tigera removed docs-pr-required release-note-required labels May 29, 2026

Merge remote-tracking branch 'origin/master' into pr-4871

6aea4fd

# Conflicts: # pkg/controller/installation/core_controller_test.go # pkg/controller/utils/component.go # pkg/render/node.go

caseydavenport commented Jun 3, 2026

View reviewed changes

caseydavenport added 7 commits June 3, 2026 15:31

Rename modCtx to renderCtx

9197ec9

modCtx read like "modifier context"; the value is an extensions.RenderContext, so name it for what it is.

Document the extensions package and drop stale builder wording

6375a34

Add a package doc that lays out the two-phase model (Setup vs Extension) so the whole seam is legible from `go doc`. Fix two comments that still called the setup a render context builder.

caseydavenport force-pushed the casey-variant-extensions branch from 39de688 to 03e702b Compare July 27, 2026 20:33

caseydavenport added 6 commits July 27, 2026 14:34

Trim the comments on the extension mechanism

3b51691

Several were paragraphs restating what the declaration below them already said, which makes the code harder to scan rather than easier. No behavior change.

Name the render inputs field on controller.Inputs

d048a39

It was embedded, so the field was called Inputs and construction read Inputs: render.Inputs{...}. Naming it RenderInputs says where the field came from at the point of use, at the cost of the promoted accesses getting longer.

caseydavenport force-pushed the casey-variant-extensions branch from 1897453 to d048a39 Compare July 27, 2026 23:02

Brian-McM reviewed Jul 28, 2026

View reviewed changes

Comment thread pkg/render/apiserver.go Outdated

Give the API server policy extension point its own inputs type

0d1a79b

Every modifier key now pins a distinct inputs type, so a modifier registered against the wrong key fails to compile.

Brian-McM reviewed Jul 29, 2026

View reviewed changes

Comment thread pkg/extensions/set.go Outdated

caseydavenport added 6 commits August 4, 2026 07:18

Emit the cloud TENANT_ID from the enterprise es assembler

50d6f8d

The base kube-controllers render was reading cfg.TenantID, which only the es-kube-controllers controller ever sets.

Move the Calico Cloud flag onto the enterprise options

19ba9c9

Only the enterprise apiserver modifier reads it, so the API server render configuration should not carry it. The flag comes from the build rather than the cluster, so main hands it to enterprise.New.

Merge master into casey-variant-extensions

4cb7f15

Master resolves the variant at startup, so the extension wiring and the reconciler gates key on opts.Variant instead of the EnterpriseCRDExists flag it replaced.

Trim the extension mechanism comments

277c2b4

The no-op contract was explained in four places; it belongs on Registry alone.