Gate the remaining Enterprise-only code behind the variant extensions by caseydavenport · Pull Request #5170 · tigera/operator

caseydavenport · 2026-08-13T15:36:21Z

Description

Prep for splitting the operator across the OSS and Enterprise monorepos. Core code should never name an Enterprise-only resource, and a handful of places still did.

The certificate signing controller now takes its signable certificates, its watches, and its signing-role decision from the variant, rather than hard-coding the Prometheus server certificate and reading the monitoring resource itself.
Dex and Prometheus client certificate watches move into the Enterprise API server hook; the Prometheus and packet capture certificates come out of the cluster connection controller's watches and trusted bundle the same way.
Elasticsearch client helpers move out of the shared controller utilities into the log storage tree, where the only callers live.
The startup check that refuses a contradictory internal/external Elasticsearch configuration becomes an Enterprise startup hook, replacing two near-identical copies.
The Enterprise custom resource getters move out of the shared controller utilities into their own package, along with the conversion from cloud configuration to a tenant.
The kube-controllers render configuration drops the Enterprise resources only the Elasticsearch assembler reads, and the tiers controller stops deciding for itself which namespaces need DNS access when the variant is Enterprise.
The tenant secrets controller moves into a package of its own. It only runs in multi-tenant mode, so the OSS side deletes it wholesale.
A new test fails when anything outside the Enterprise packages imports an Enterprise-only renderer, or names one of the fifteen operator API kinds that only Enterprise installs. Only the operator's main entry point is still on an exception list.

One behavior change worth flagging: with no monitoring resource present, the signing-role decision now falls through to the non-cluster host check instead of returning early. The non-cluster host check was being skipped in that case.

None

Copilot

Pull request overview

This PR continues the variant-extension refactor to keep core operator code variant-blind, moving remaining Enterprise-only behaviors (watches, startup checks, and Elasticsearch helpers) behind pkg/extensions to prepare for the OSS/Enterprise repo split.

Changes:

Add new variant hooks for CSR and startup configuration validation, and route CSR controller behavior/watches through the extension interface.
Move Elasticsearch helpers from shared controller utils into pkg/controller/logstorage/esutils and update callers accordingly.
Add an Enterprise boundary test to prevent core packages from importing Enterprise-only renderers (with a shrinking exception list), and update clusterconnection/apiserver secret watches to be variant-owned.

Reviewed changes

Copilot reviewed 37 out of 42 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pkg/extensions/startup.go	New startup extension interface + noop implementation.
pkg/extensions/extensions.go	Wire `CSR()` and `Startup()` accessors into the extension registry.
pkg/extensions/csr.go	New CSR extension interface + noop implementation.
pkg/extensions/clusterconnection.go	Add `TrustedBundleSecrets()` to clusterconnection extension contract.
pkg/enterprise/startup/extension.go	Enterprise startup hook to validate internal/external ES cert mode.
pkg/enterprise/register.go	Register Enterprise implementations for CSR + startup extensions.
pkg/enterprise/options/options.go	Extend Enterprise options with ES mode fields needed by startup hook.
pkg/enterprise/enterprise_suite_test.go	New Enterprise package ginkgo suite entrypoint.
pkg/enterprise/csr/suite_test.go	New ginkgo suite for Enterprise CSR extension tests.
pkg/enterprise/csr/extension.go	Enterprise CSR behavior (signable assets, needs-role decision, watches).
pkg/enterprise/csr/extension_test.go	Unit tests for Enterprise CSR extension behavior change.
pkg/enterprise/clusterconnection/extension.go	Enterprise Guardian trusted-bundle secret list exposed via extension.
pkg/enterprise/boundary_test.go	New test enforcing no core imports of Enterprise-only render packages.
pkg/enterprise/apiserver/extension.go	Move Dex + Prometheus client cert watches into Enterprise apiserver hook.
pkg/controller/utils/utils.go	Remove ES-specific helpers from shared controller utils.
pkg/controller/utils/utils_test.go	Remove tests for ES helpers that moved out of shared utils.
pkg/controller/manager/manager_controller.go	Switch ES license lookup to `logstorage/esutils`.
pkg/controller/logstorage/users/users_controller.go	Switch ES helpers/types to `logstorage/esutils`.
pkg/controller/logstorage/users/users_controller_test.go	Update tests to use `logstorage/esutils` helpers/types.
pkg/controller/logstorage/linseed/linseed_controller.go	Switch ES readiness lookup to `logstorage/esutils`.
pkg/controller/logstorage/kubecontrollers/es_kube_controllers.go	Switch ES readiness lookup to `logstorage/esutils`.
pkg/controller/logstorage/esutils/test_files/01_put_policy.json	Test fixture for ES ILM policy PUT (case 01).
pkg/controller/logstorage/esutils/test_files/01_get_policy.json	Test fixture for ES ILM policy GET (case 01).
pkg/controller/logstorage/esutils/test_files/02_put_policy.json	Test fixture for ES ILM policy PUT (case 02).
pkg/controller/logstorage/esutils/test_files/02_put_policy_readonly.json	Test fixture for ES ILM policy PUT readonly (case 02).
pkg/controller/logstorage/esutils/test_files/02_get_policy.json	Test fixture for ES ILM policy GET (case 02).
pkg/controller/logstorage/esutils/esutils_suite_test.go	New ginkgo suite entrypoint for `esutils`.
pkg/controller/logstorage/esutils/elasticsearch.go	Move/rename ES helper package to `esutils` and add license + ES getters.
pkg/controller/logstorage/esutils/elasticsearch_test.go	Update package name + add moved tests for ES users + licensing helpers.
pkg/controller/logstorage/esutils/elasticsearch_cloud.go	Update package name to `esutils`.
pkg/controller/logstorage/elastic/mock.go	Update mock interfaces/types to `esutils`.
pkg/controller/logstorage/elastic/elastic_controller.go	Switch ES helpers to `esutils`.
pkg/controller/logstorage/elastic/elastic_controller_test.go	Update test helper signature to `esutils` creator type.
pkg/controller/logstorage/dashboards/dashboards_controller.go	Switch ES readiness lookup to `esutils`.
pkg/controller/intrusiondetection/intrusiondetection_controller.go	Switch ES readiness lookup to `esutils`.
pkg/controller/inputs.go	Add new `CSR` input name constant.
pkg/controller/csr/csr_controller.go	Route watches/assets/role-decision via CSR extension instead of Enterprise hard-codes.
pkg/controller/csr/csr_controller_test.go	Update CSR controller tests to use a stub CSR extension.
pkg/controller/clusterconnection/clusterconnection_controller.go	Build Guardian trusted bundle from core + extension-provided secret list.
pkg/controller/apiserver/apiserver_controller.go	Remove Enterprise-only secret watches from core apiserver controller.
docs/principles.md	Document product-variant principles and the Enterprise render boundary rule.
cmd/main.go	Move startup config validation behind the new startup extension hook.

Suppressed comments (2)

pkg/controller/logstorage/esutils/elasticsearch.go:713

logr.Logger.Info does not do printf-style formatting. Using a "%s" format string here will produce a misleading log entry (and can drop the value as an unmatched key). Use structured key/value logging instead.
pkg/controller/csr/csr_controller.go:110
This error message references "monitor-controller", but this is the CSR controller watching the Installation resource. The mismatched controller name makes startup failures harder to diagnose.

	if err = opts.Extensions.CSR().Watches(c); err != nil {
		return fmt.Errorf("csr-controller failed to set up extension watches: %w", err)
	}

	if err = c.WatchObject(&operatorv1.Installation{}, &handler.EnqueueRequestForObject{}); err != nil {
		return fmt.Errorf("monitor-controller failed to watch primary resource: %w", err)
	}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+// Watches registers the Monitor CR, which decides whether the CSR role is needed.
+func (e *Extension) Watches(c ctrlruntime.Controller) error {
+	if err := c.WatchObject(&operatorv1.Monitor{}, &handler.EnqueueRequestForObject{}); err != nil {
+		return fmt.Errorf("csr-controller failed to watch Monitor: %w", err)
+	}
+	return nil
+}


Copilot

Pull request overview

Copilot reviewed 37 out of 42 changed files in this pull request and generated no new comments.

Suppressed comments (3)

pkg/controller/csr/csr_controller.go:110

The Installation watch error message references "monitor-controller", which is misleading in CSR controller setup and makes troubleshooting harder.

	if err = c.WatchObject(&operatorv1.Installation{}, &handler.EnqueueRequestForObject{}); err != nil {
		return fmt.Errorf("monitor-controller failed to watch primary resource: %w", err)
	}

pkg/controller/logstorage/esutils/elasticsearch.go:713

logr's Info() is not printf-style; passing a format string with a single arg results in an odd key/value pair and a malformed log entry. Use structured logging (or fmt.Sprintf) instead.
pkg/enterprise/csr/extension.go:80
NeedsCSRRole() checks for a NonClusterHost, but Watches() only watches the Monitor CR. If a NonClusterHost is created/removed after startup, the CSR controller may not reconcile to create/delete the CSR ClusterRole before the first CSR is submitted (which can block CSR creation). Add a watch for NonClusterHost to keep the signing-role decision reactive.

Copilot

Pull request overview

Copilot reviewed 55 out of 60 changed files in this pull request and generated no new comments.

Suppressed comments (2)

pkg/controller/logstorage/esutils/elasticsearch.go:712

logr.Logger.Info does not support printf-style formatting. This call will log the literal "%s" rather than the license value, which makes troubleshooting harder.
pkg/enterprise/csr/extension.go:80
NeedsCSRRole() depends on the NonClusterHost CR, but the extension only watches Monitor. Creating/deleting a NonClusterHost will not trigger reconciliation, so the CSR ClusterRole may not be created/removed until some unrelated event occurs.

Copilot

Pull request overview

Copilot reviewed 85 out of 93 changed files in this pull request and generated no new comments.

Suppressed comments (2)

pkg/controller/logstorage/esutils/elasticsearch.go:713

logr.Logger's Info does not support printf-style formatting; passing a single extra argument also results in an odd key/value list. Log the license as a structured field instead.
pkg/enterprise/utils/utils.go:58
Typo in the comment: “AplicationLayer” should be “ApplicationLayer”.

The certificate signing controller, the API server controller and the cluster connection controller take the variant's certificates from pkg/extensions.

The shared controller utils no longer reach into the Elasticsearch renderers.

The exception list names the three files that still do.

The OSS operator will not ship the Enterprise CRDs, so core controllers and render packages must stop referring to their Go types. Seventeen files still do and are on an exception list.

Typha, the istio policy-sync prefix, and the webhooks management-cluster config now take plain values instead of reading Enterprise CRs in core.

The certificate manager only needed to know whether the cluster is multi-tenant, and the two tenant-scoped render helpers moved to their own Enterprise-only file.

The shared controller utils package no longer reads Enterprise-only CRs, and core code importing anything under pkg/enterprise is now a boundary violation too.

The es-kube-controllers assembler now wraps the generic config with the Authentication and ManagementCluster it reads. The Tenant field was dead - its only caller always passed nil.

The cloudconfig package no longer builds an Enterprise CR.

The tiers controller no longer branches on the variant to decide which namespaces need DNS access, nor watches Tenant itself.

It only runs in multi-tenant mode, so it is deleted wholesale on the OSS side. No core file names an Enterprise-only kind now.

Creating a NonClusterHost now triggers a reconcile, so the signing role appears without waiting for an unrelated event. Also fixes a printf-style log call and two misleading strings.

The signable assets, the subject lookup for requests no pod issued, and the requestor authorization all come from the extension now.

The controller now builds the bundle from the core secrets and the extension adds what the variant needs Guardian to trust.

…nfig

Folds the one-function policysync package into pkg/enterprise/utils, names the all-tenants filter, and tightens the boundary test's file matching.

The Linseed namespace and manager service helpers only ever serve Enterprise renderers. pkg/render re-exports the four constants that moved with them, so the existing references stay put until manager.go and logstorage.go follow.

This reverts commit 7d9c8e5.

caseydavenport · 2026-08-13T20:43:22Z

 		Cloud:       isCloudBuild(),
 		ManageCRDs:  manageCRDs,
 		UseV3CRDs:   v3CRDs,
+


Suggested change

caseydavenport · 2026-08-13T20:44:15Z

@@ -589,12 +587,6 @@ admission policy installation; once an Installation exists it is the authority o

 	if isCloudBuild() {


This entire branch, plus the elasticIsMigrating / useExternalElastic are eventually going to need to be inputs to something like operator.New(...) instead of living here. The code in New() will need to be variant agnostic as well.

caseydavenport · 2026-08-13T20:47:24Z

-		}
+	// The webhooks component (v3-CRD mode) needs the management-cluster setup to
+	// register the managed-cluster webhook.
+	managementCluster, err := r.ext.ManagementCluster(ctx, r.client)


This is wrong - the extension interface shouldn't have a ManagementCluster() call on it, nor should the OSS controller know anything about ManagementClusters - this is a violation of the break between OSS and enterprise that we are trying to build.

This needs to be plumbed through the controller extension mechanism.

caseydavenport · 2026-08-13T20:50:01Z


-func WithTenant(t *operatorv1.Tenant) Option {
+// WithMultiTenant tells the manager to use the per-tenant CA secret.
+func WithMultiTenant(multiTenant bool) Option {


Is this change necessary?

caseydavenport · 2026-08-13T20:51:18Z


 	includeSystem := managementClusterConnection.Spec.TLS.CA == operatorv1.CATypePublic

+	bundleSecrets := append([]string{render.CalicoAPIServerTLSSecretName, goldmane.GoldmaneKeyPairSecret},


No need for this to be split across lines, and if it is we should do each argument on its own line, and closing paren on its own line.

caseydavenport · 2026-08-13T21:15:56Z


+	// NonClusterHostEnabled reports whether the variant serves hosts outside the
+	// cluster, which Typha renders a second deployment for.
+	NonClusterHostEnabled(ctx context.Context, c client.Client) (bool, error)


Yep - this is an enterprise concept and shouldn't appear on the generic extensions interface.

caseydavenport · 2026-08-13T21:17:16Z

@@ -79,27 +78,6 @@ type CloudConfig struct {
 	enableMTLS           bool


This whole file also probably belongs in an enterprise/ package of some sort.

caseydavenport · 2026-08-13T21:17:48Z

-	// es-kube-controllers assembler. No base rendering reads them.
-	ManagementCluster           *operatorv1.ManagementCluster
+	// ManagementClusterConnection is an input for the enterprise es-kube-controllers
+	// assembler. No base rendering reads it.


This is a breach. No base rendering needs it -> should be handled in extension.

caseydavenport · 2026-08-13T21:18:48Z

+// LinseedNamespace determine the namespace in which Linseed is running.
+// For management and standalone clusters, this is always the tigera-elasticsearch
+// namespace. For multi-tenant management clusters, this is the tenant namespace
+func LinseedNamespace(tenant *operatorv1.Tenant) string {


This file should also go into an enterprise/ package of some sort - let's make it clear what files are strictly for enterprise.

caseydavenport · 2026-08-13T21:19:34Z

 	TLS               *TyphaNodeTLS
 	MigrateNamespaces bool
 	ClusterDomain     string
-	NonClusterHost    *operatorv1.NonClusterHost


This is lazy - it's not just the type that matters, its' the concept of an NCH altogether. This isn't actually meeting the requirement.

caseydavenport requested a review from a team as a code owner August 13, 2026 15:36

caseydavenport added release-note-not-required docs-not-required labels Aug 13, 2026

Copilot AI lite review requested due to automatic review settings August 13, 2026 15:36

caseydavenport added release-note-not-required docs-not-required labels Aug 13, 2026

marvin-tigera added this to the v1.44.0 milestone Aug 13, 2026

Copilot started reviewing on behalf of caseydavenport August 13, 2026 15:36 View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 13, 2026 15:48

Copilot started reviewing on behalf of caseydavenport August 13, 2026 15:48 View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 13, 2026 16:13

Copilot started reviewing on behalf of caseydavenport August 13, 2026 16:13 View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 13, 2026 16:55

Copilot started reviewing on behalf of caseydavenport August 13, 2026 16:55 View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

caseydavenport added 12 commits August 14, 2026 09:13

Gate the Enterprise TLS assets and secret watches behind extensions

1803d90

The certificate signing controller, the API server controller and the cluster connection controller take the variant's certificates from pkg/extensions.

Move the Elasticsearch helpers into the log storage tree

31c62cf

The shared controller utils no longer reach into the Elasticsearch renderers.

Fail the build if core code imports an Enterprise renderer

ddd4bf7

The exception list names the three files that still do.

Fail the build if core code names an Enterprise-only API kind

afd775c

The OSS operator will not ship the Enterprise CRDs, so core controllers and render packages must stop referring to their Go types. Seventeen files still do and are on an exception list.

Move three Enterprise-only CR reads behind variant extensions

0d69570

Typha, the istio policy-sync prefix, and the webhooks management-cluster config now take plain values instead of reading Enterprise CRs in core.

Take the tenant CR out of the certificate manager and render utils

eb8c362

The certificate manager only needed to know whether the cluster is multi-tenant, and the two tenant-scoped render helpers moved to their own Enterprise-only file.

Move the Enterprise CR getters into pkg/enterprise/utils

1ad6bc8

The shared controller utils package no longer reads Enterprise-only CRs, and core code importing anything under pkg/enterprise is now a boundary violation too.

Move Enterprise CRs off the kube-controllers render config

c50fce6

The es-kube-controllers assembler now wraps the generic config with the Authentication and ManagementCluster it reads. The Tenant field was dead - its only caller always passed nil.

Move the cloud config Tenant conversion into pkg/enterprise/utils

cf266d2

The cloudconfig package no longer builds an Enterprise CR.

Move the apiserver Tenant watch into the Enterprise extension

ac1df5a

Move the tiers controller Enterprise namespaces behind an extension

5b6f69a

The tiers controller no longer branches on the variant to decide which namespaces need DNS access, nor watches Tenant itself.

Move the tenant secrets controller into its own package

fd95c95

It only runs in multi-tenant mode, so it is deleted wholesale on the OSS side. No core file names an Enterprise-only kind now.

caseydavenport added 8 commits August 14, 2026 09:23

Restore the original policy sync path prefix comment

bf4eb7e

Watch NonClusterHost in the CSR extension

5471026

Creating a NonClusterHost now triggers a reconcile, so the signing role appears without waiting for an unrelated event. Also fixes a printf-style log call and two misleading strings.

Move the CSR controller's variant knowledge behind the extension

ccbb54a

The signable assets, the subject lookup for requests no pod issued, and the requestor authorization all come from the extension now.

Move guardian's enterprise trusted certificates behind the extension

fa67df4

The controller now builds the bundle from the core secrets and the extension adds what the variant needs Guardian to trust.

Move the es-kube-controllers managed cluster input out of the core co…

d8ca8bb

…nfig

Move the cloud config helpers into the enterprise packages

5067ad6

Drop the dead management cluster params from the render test helper

1688bd6

Move the namespace helper into the enterprise utils package

5cfcf0a

caseydavenport force-pushed the casey-gate-tls-assets branch from d0be37f to 5cfcf0a Compare August 14, 2026 13:56

caseydavenport added 3 commits August 14, 2026 10:25

Tidy up the extension boundary helpers

10ed891

Folds the one-function policysync package into pkg/enterprise/utils, names the all-tenants filter, and tightens the boundary test's file matching.

Move the tenant render helpers into pkg/enterprise

7d9c8e5

The Linseed namespace and manager service helpers only ever serve Enterprise renderers. pkg/render re-exports the four constants that moved with them, so the existing references stay put until manager.go and logstorage.go follow.

Revert "Move the tenant render helpers into pkg/enterprise"

bfd1b26

This reverts commit 7d9c8e5.

caseydavenport commented Aug 14, 2026

View reviewed changes

		@@ -589,12 +587,6 @@ admission policy installation; once an Installation exists it is the authority o

		if isCloudBuild() {


		includeSystem := managementClusterConnection.Spec.TLS.CA == operatorv1.CATypePublic

		bundleSecrets := append([]string{render.CalicoAPIServerTLSSecretName, goldmane.GoldmaneKeyPairSecret},

Uh oh!

Conversation

caseydavenport commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

caseydavenport commented Aug 13, 2026 •

edited

Loading