Support enterprise whisker-backend with Linseed upstream - #4861
Conversation
e7aac2c to
ed02bd2
Compare
hjiawei
added
release-note-not-required
docs-not-required
and removed
docs-pr-required
release-note-required
labels
There was a problem hiding this comment.
Pull request overview
Adds enterprise support to the whisker-backend by wiring it to Linseed (instead of Goldmane) for flow data, including RBAC, mTLS, and network policy plumbing for enterprise variants. Non-enterprise (Calico) paths still render Goldmane env/policy as before.
Changes:
- Render enterprise Whisker
ClusterRole/ClusterRoleBindingand Linseed-specific env vars (LINSEED_URL,LINSEED_CA_PATH,LINSEED_TOKEN_PATH,LINSEED_CLUSTER_ID,LINSEED_CLIENT_CERT/KEY_PATH); split network policy into enterprise (Manager ingress + Linseed/kube-apiserver egress) vs Calico (Goldmane egress). - Controller: skip Goldmane CR requirement for enterprise, watch
TigeraLinseedSecret, switch trusted bundle source to Linseed for enterprise, and add a missingreturnafterSetDegradedin trusted bundle creation. - Wire Manager → Whisker egress and Linseed ← Whisker ingress via new
WhiskerEntityRule/WhiskerSourceEntityRulehelpers; update expected policy JSON fixtures and Manager test egress count.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/render/whisker/component.go | Adds enterprise constants, RBAC objects, Linseed env vars, ingress/egress rules in network policy |
| pkg/render/whisker/component_test.go | Adds enterprise object count, deployment, RBAC, and network policy test cases |
| pkg/render/manager.go | Adds Manager egress rule to Whisker |
| pkg/render/manager_test.go | Updates expected egress rule count from 11 to 12 |
| pkg/render/logstorage/linseed/linseed.go | Adds Whisker as ingress source to Linseed |
| pkg/render/common/networkpolicy/networkpolicy.go | Adds WhiskerEntityRule and WhiskerSourceEntityRule helpers |
| pkg/controller/whisker/controller.go | Skips Goldmane CR requirement for enterprise, watches Linseed secret, switches trusted bundle source, adds missing return on error |
| pkg/render/testutils/expected_policies/manager.json | Adds Manager→Whisker egress entry |
| pkg/render/testutils/expected_policies/manager_ocp.json | Adds Manager→Whisker egress entry |
| pkg/render/testutils/expected_policies/linseed.json | Adds Whisker ingress entry |
| pkg/render/testutils/expected_policies/linseed_ocp.json | Adds Whisker ingress entry |
| pkg/render/testutils/expected_policies/linseed_dpi_enabled.json | Adds Whisker ingress entry |
| pkg/render/testutils/expected_policies/linseed_ocp_dpi_enabled.json | Adds Whisker ingress entry |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cfbff08 to
8bc0e61
Compare
|
This PR is stale because it has been open for 60 days with no activity. |
8bc0e61 to
5172c8a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (2)
pkg/controller/whisker/controller.go:222
- Avoid hard-coding the backend Service name here. Using the shared constant keeps certificate SAN generation aligned with the rendered Service name and prevents drift if the Service name changes in the renderer.
whiskerBackendCertificateNames := dns.GetServiceDNSNames("whisker-backend", whisker.WhiskerNamespace, r.clusterDomain)
pkg/render/manager.go:114
- These Whisker-related constants are duplicated from pkg/render/whisker (notably namespace/service name/ports). This creates a drift risk over time; consider moving the shared Service coordinates into the pkg/render package (or another shared package) so both manager rendering and whisker rendering reference the same single source of truth without an import cycle.
// Whisker-backend Service coordinates, mirroring pkg/render/whisker —
// importing that package here would create an import cycle (it imports
// render).
WhiskerNamespace = "calico-system"
WhiskerDeploymentName = "whisker"
WhiskerBackendServiceName = "whisker-backend"
WhiskerBackendServicePort = 8443
WhiskerBackendTargetPort = 3002
5172c8a to
c31d14c
Compare
c31d14c to
4da86bd
Compare
4da86bd to
9aa8865
Compare
9aa8865 to
b48e64a
Compare
b48e64a to
e917d77
Compare
For Calico Enterprise the Whisker UI is a manager UI module, so only the whisker-backend container is deployed — no SPA container, nginx config, UI Service or nginx TLS key pair. The backend gets the Linseed upstream env, a ClusterRole granting linseed.tigera.io flows access, and Linseed's network policy admits it. The manager wires the module up when a Whisker CR exists: the manager container gets SUPPORTS_FLOW_LOGS for the UI feature flag, Voltron gets a /whisker-backend proxy target pointing at the new whisker-backend Service, and the manager/whisker network policies allow that path.
There was a problem hiding this comment.
🟡 Changes recommended
Managed-cluster cleanup and feature gating are incomplete, and direct OIDC authentication lacks required egress.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 18/18 changed files
- Comments generated: 3
- Review effort level: Balanced
| if mcc != nil { | ||
| reqLogger.V(1).Info("Managed cluster: whisker-backend is not deployed; flow logs are served by the management cluster") | ||
| r.status.ReadyToMonitor() | ||
| r.status.ClearDegraded() | ||
| return reconcile.Result{}, nil |
| Manager: instance, | ||
| Authentication: authenticationCR, | ||
| KibanaEnabled: kibanaEnabled, | ||
| WhiskerEnabled: whiskerCR != nil, |
| if c.cfg.KeyValidatorConfig != nil { | ||
| // Dex-issued tokens are verified against Dex's signing keys. | ||
| egressRules = append(egressRules, v3.Rule{ | ||
| Action: v3.Allow, | ||
| Protocol: &networkpolicy.TCPProtocol, | ||
| Destination: render.DexEntityRule, | ||
| }) |
e917d77 to
7415b51
Compare
pasanw
left a comment
pasanw
left a comment
There was a problem hiding this comment.
The general shape and implementation looks good to me. I audited corner cases with Claude and found some things worth a look
|
|
||
| // Clean up the other variant's objects on a variant switch; GC will not, | ||
| // since the owning Whisker CR still exists. | ||
| if c.isEnterprise() { |
There was a problem hiding this comment.
Claude: These lists cover the objects Objects() renders, but the whisker-key-pair secret is missed on a Calico -> Calico Enterprise upgrade.
The controller now creates that key pair for OSS only, and skips appending its KeyPairOption when nil. Nothing deletes it: certificatemanagement.go:87 just continues on a nil key pair. So the secret is orphaned in both tigera-operator and calico-system.
Smaller and in the other direction: the OSS branch can't delete the KeyValidatorConfig secret/ConfigMap copies enterprise rendered, because cfg.KeyValidatorConfig is nil there.
There was a problem hiding this comment.
Fixed in the monorepo PRs. BackendOnly now puts the UI key pair on the delete list: the copy in calico-system always, and the truth-namespace secret when the operator issued it (BYO() false), so a user-provided pair stays. The KeyValidatorConfig secret and ConfigMap copies are no longer rendered at all (see the thread on line 172), so the OSS branch has nothing to clean up there. Core side in projectcalico/calico#12879 (staleKeyPair in pkg/controller/whisker/controller.go), carried identically in tigera/calico-private#12086.
| Manager: instance, | ||
| Authentication: authenticationCR, | ||
| KibanaEnabled: kibanaEnabled, | ||
| WhiskerEnabled: whiskerCR != nil, |
There was a problem hiding this comment.
Claude: This turns the flow logs module on for every tenant, but there is only one whisker-backend.
In multi-tenant mode this controller reconciles once per tenant namespace. So each tenant's manager gets SUPPORTS_FLOW_LOGS=true and a Voltron route to whisker-backend.calico-system.svc. That backend is shared, and it is not tenant-scoped. It queries the fixed render.ElasticsearchNamespace Linseed, sends no LINSEED_TENANT_ID, and starts its RBAC calculator with an empty tenant namespace.
At best the tab appears and every request fails, because a tenant's flows live in that tenant's own Linseed. At worst a tenant's user is served another tenant's flows.
The line just above already does the right thing for Kibana: kibanaEnabled := !r.opts.MultiTenant. Suggest the same gate here, for example WhiskerEnabled: whiskerCR != nil && !r.opts.MultiTenant. Making whisker genuinely tenant-aware can be separate work.
There was a problem hiding this comment.
Gated as you suggested, plus the managed-cluster case: whiskerEnabled := whiskerCR != nil && managementClusterConnection == nil && !r.opts.MultiTenant. A test in the multi-tenant Describe creates a Whisker CR, reconciles a tenant, and asserts its Voltron gets no whisker route. tigera/calico-private#12086, manager controller commit.
| corev1.EnvVar{Name: "WHISKER_BACKEND_UPSTREAM", Value: "linseed"}, | ||
| corev1.EnvVar{ | ||
| Name: "LINSEED_URL", | ||
| Value: relasticsearch.LinseedEndpoint(c.SupportedOSType(), c.cfg.ClusterDomain, render.ElasticsearchNamespace, false, false), |
There was a problem hiding this comment.
Claude: These coordinates are fixed to the single-tenant layout, so the backend cannot work on a multi-tenant management cluster.
render.ElasticsearchNamespace is always tigera-elasticsearch. In multi-tenant mode Linseed lives in the tenant namespace, which is what render.LinseedNamespace(tenant) returns. MULTI_CLUSTER_FORWARDING_ENDPOINT below uses render.ManagerService(nil), which is likewise the single-tenant manager. LINSEED_TENANT_ID is never set, so Linseed sees an empty tenant.
The whisker controller has no Tenant at all today, so there is nothing to pass in here even if this code accepted it.
Simplest fix for this PR: make the whisker controller a no-op when opts.MultiTenant is set, paired with the gate on the manager side. Then this code is only ever reached in the single-tenant layout it already assumes.
There was a problem hiding this comment.
Took the simplest fix: the Enterprise whisker extension reports Disabled when opts.MultiTenant is set, so nothing is deployed on a multi-tenant management cluster, paired with the manager gate above. The single-tenant coordinates are then only reached in the layout they assume. tigera/calico-private#12086, pkg/enterprise/whisker/extension.go.
| egressRules = append(egressRules, v3.Rule{ | ||
| Action: v3.Allow, | ||
| Protocol: &networkpolicy.TCPProtocol, | ||
| Destination: render.DexEntityRule, |
There was a problem hiding this comment.
Claude: This assumes a non-nil KeyValidatorConfig means Dex, but it can mean an external identity provider instead.
Authentication.spec.oidc.type: Tigera skips Dex and talks straight to the customer's own IdP. In that case eutils.GetKeyValidatorConfig returns a tigerakvc config, so KeyValidatorConfig != nil here, but OIDC_AUTH_JWKSURL is a public URL. eutils.DexEnabled() is false for that type, so the Dex CA is not added to the bundle either.
Two things then break. The whisker pod is deny-all, and its only egress is Linseed, the API server, Voltron, Dex and DNS, so the fetch of the IdP's signing keys is dropped. Even if that were allowed, the bundle is built with includeSystem: false and is mounted over /etc/pki/tls/certs, which hides the public root certificates, so TLS to the IdP would fail. The result is that every user token is rejected and the flow logs UI returns 401 for everyone.
Two options. Branch on the type: keep this rule when eutils.DexEnabled(authenticationCR), otherwise add networkpolicy.GetOIDCEgressRule(parsedIssuerURL) (the pattern used at pkg/enterprise/apiserver/extension.go:752) and build the bundle with system root certificates. Or, much cheaper for this PR: only pass KeyValidatorConfig to this component when Dex is enabled, and degrade with a clear message for type: Tigera until it is supported.
There was a problem hiding this comment.
Took the cheaper option. The key validator config is only built when eutils.DexEnabled(authn); an Authentication with oidc.type: Tigera is rejected with InvalidConfigf naming the reason, so the flow logs UI degrades clearly instead of returning 401 to everyone. The Dex egress rule is therefore the only one needed. tigera/calico-private#12086, pkg/enterprise/whisker/extension.go, with a test for the rejection.
| toCreate = append(toCreate, c.whiskerBackendClusterRole(), c.whiskerBackendClusterRoleBinding(), c.whiskerBackendService()) | ||
| if c.cfg.KeyValidatorConfig != nil { | ||
| toCreate = append(toCreate, secret.ToRuntimeObjects(c.cfg.KeyValidatorConfig.RequiredSecrets(WhiskerNamespace)...)...) | ||
| toCreate = append(toCreate, configmap.ToRuntimeObjects(c.cfg.KeyValidatorConfig.RequiredConfigMaps(WhiskerNamespace)...)...) |
There was a problem hiding this comment.
Claude: For the external-IdP case this renders a ConfigMap the manager already owns, in the same namespace.
RequiredConfigMaps returns something only for oidc.type: Tigera, and the name is fixed: tigera-static-wellknown-jwks. WhiskerNamespace is calico-system. The manager renders that same name into that same namespace (pkg/render/manager.go:330, where ManagerNamespace is also common.CalicoNamespace).
Two controllers then write one object with different owner references, Whisker on one side and Manager on the other. Each reconcile undoes the other, so they keep rewriting it forever. Deleting either CR can also garbage-collect the ConfigMap out from under the other component.
The backend does not need it anyway. That ConfigMap exists so the manager's nginx can serve /.well-known/openid-configuration and /discovery/keys as static files to the browser, which is why the JWKSURL stored inside it is rewritten to a relative path. RequiredVolumeMounts() mounts it under /usr/share/nginx/html, and managerVolumeMounts() is used only by managerContainer(), the nginx container. whisker-backend is a Go binary that fetches the JWKS over the network via oidc.NewRemoteKeySet, and never reads those paths.
Suggest dropping RequiredConfigMaps, RequiredVolumes and RequiredVolumeMounts from this component, and keeping only RequiredEnv and RequiredAnnotations.
There was a problem hiding this comment.
Dropped RequiredConfigMaps, RequiredVolumes, RequiredVolumeMounts and RequiredSecrets from the whisker render; only RequiredEnv and RequiredAnnotations apply, with the reason on the Configuration field. The render test asserts no Secret or ConfigMap is rendered and the volume set is unchanged. Core side in projectcalico/calico#12879, pkg/render/whisker/component.go.
| // changes. The trusted bundle is only used by a client that picks up | ||
| // changes without a restart. | ||
| annotations := map[string]string{ | ||
| c.cfg.WhiskerBackendKeyPair.HashAnnotationKey(): c.cfg.WhiskerBackendKeyPair.HashAnnotationValue(), |
There was a problem hiding this comment.
Claude: The comment above does not hold for the enterprise backend, so a CA rotation breaks it until someone deletes the pod.
These annotations cover the key pair and the key validator config, but not the trusted bundle. I checked whether the Linseed client reloads on its own, since building it once does not by itself prove it ignores later changes. It does not reload:
newHTTPClient(linseed/pkg/client/rest/client.go:93-137) doesos.ReadFile(cfg.CACertPath)once into a freshx509.CertPooland assigns it totlsConfig.RootCAs. The client cert is loaded once withtls.LoadX509KeyPair.- There is no
GetClientCertificate,GetCertificateorVerifyPeerCertificatecallback, and no file watching, anywhere inlinseed/pkg/client/rest/. Those callbacks are the only way a Go TLS client picks up a changed file mid-process. NewClientbuilds thathttp.Clientonce and stores it. Nothing rebuilds the transport.- The same file does hot-reload where it was wanted:
WithTokenPathis documented as loading the token on each request to allow rotation, andToken()re-reads it per request. The token reloads by design; the certificates do not.
So when the operator CA or tigera-secure-linseed-cert rotates, the bundle ConfigMap changes, the pod is not rolled, and every flow query then fails with an x509 error.
Suggest adding maps.Copy(annotations, c.cfg.TrustedCertBundle.HashAnnotations()). That is what the other bundle consumers do, for example pkg/render/intrusion_detection.go:1044, pkg/render/policyrecommendation.go:422 and pkg/render/guardian.go:430.
There was a problem hiding this comment.
Added maps.Copy(annotations, c.cfg.TrustedCertBundle.HashAnnotations()) for the backend-only deployment, and corrected the comment: whisker's own Goldmane client reloads the bundle, another flow source's client may not. A render test checks the annotation is present for backend-only and absent for the full deployment. projectcalico/calico#12879.
| reqLogger.V(1).Info("Managed cluster: whisker-backend is not deployed; flow logs are served by the management cluster") | ||
| r.status.ReadyToMonitor() | ||
| r.status.ClearDegraded() | ||
| return reconcile.Result{}, nil |
There was a problem hiding this comment.
Claude: Returning here deletes nothing, so objects from a previous configuration keep running.
The old enterprise extension did this cleanup. deleteWhisker moved everything whisker rendered onto the delete list, and extensions.Decorate gated only on the variant (pkg/extensions/component.go:32), not on the cluster type, so it covered managed clusters too. This PR removes it and nothing replaces it.
Two paths reach the problem.
- An Enterprise standalone or management cluster becomes managed, which is just an admin creating a
ManagementClusterConnection. The backend Deployment,whisker-backendService, ClusterRole, ClusterRoleBinding and NetworkPolicy all stay. The backend still hasLINSEED_URLpointed at a Linseed that no longer serves this cluster, so it runs and fails, while TigeraStatus reports Available. - An OSS managed cluster is upgraded to Enterprise. OSS managed clusters are supported (
Tenant.ManagedClusterIsCalico()), and the OSS branch above ignores theManagementClusterConnection, so whisker deploys normally there. After the upgrade the whole OSS stack survives: the Deployment with the SPA container,whisker-nginx-config, thewhiskerService, and the Goldmane-egress policy.
A fresh install onto a managed cluster is fine, because there is nothing to clean up.
Two smaller things are also skipped by this return. maintainFinalizer is never called, so a render.WhiskerFinalizer added while the cluster was standalone stays on the Installation and blocks its deletion. And gwHelper.Teardown never runs.
Suggest not returning early. Render the component so everything lands on the delete list (or keep a deleteWhisker-style decorator for the managed case), and let maintainFinalizer and gwHelper.Teardown run as usual.
There was a problem hiding this comment.
The early return is gone. A variant now reports Disabled through RenderData and the reconcile runs on: the render puts everything any configuration could have deployed on the delete list, the controller removes the issued key pairs and the bundle ConfigMap through a deletion passthrough, and maintainFinalizer and gwHelper.Teardown run as before. A controller test does a full OSS-shaped reconcile, then disables, and asserts all eight leftovers (Deployment, Service, ConfigMap, ServiceAccount, both key pairs in both namespaces) are gone with the status not degraded. projectcalico/calico#12879.
| return reconcile.Result{}, err | ||
| } | ||
| if linseedCert == nil { | ||
| r.status.SetDegraded(operatorv1.ResourceNotReady, "Linseed certificate is not available yet, waiting until it becomes available", nil, reqLogger) |
There was a problem hiding this comment.
Claude: This is right while log storage is starting up, but it never ends when log storage is not configured at all.
Log storage is optional in Enterprise. A cluster can run with no LogStorage resource. With the Whisker CR shipping by default (tigera/calico-private#12089), tigera-secure-linseed-cert never appears on such a cluster, so every reconcile lands here. The whisker TigeraStatus then stays Degraded forever, for a perfectly valid configuration. That shows red in the UI, fires alerts, and blocks flows that wait on TigeraStatus.
Before this PR that same cluster was fine: the enterprise extension deleted whisker's objects and the status was Available.
Suggest telling the two cases apart. Read the LogStorage resource: if it is absent, render nothing and report Available; if it exists but the certificate is not ready yet, degrade as this code does now. The manager controller takes a similar approach for Kibana.
There was a problem hiding this comment.
Fixed as you suggested: the extension reads LogStorage first. Absent means Disabled, so nothing is rendered and the status stays Available; present with the Linseed certificate not yet issued still degrades NotReady. LogStorage is watched. tigera/calico-private#12086, pkg/enterprise/whisker/extension.go, with tests for both branches.
| whisker.WhiskerKeyPairSecret, | ||
| goldmane.GoldmaneKeyPairSecret, | ||
| render.TigeraLinseedSecret, | ||
| render.DexTLSSecretName, |
There was a problem hiding this comment.
Claude: Two inputs this controller now reads are not watched, so authentication changes are picked up late.
eutils.GetKeyValidatorConfig reads the IdP secret, via GetSecretOrProviderClass and then GetIDPSecret. That is tigera-oidc-credentials, tigera-ldap-credentials or tigera-openshift-credentials in the operator namespace. A change to clientID there changes OIDC_AUTH_CLIENT_ID in the pod. None of them are watched here.
For oidc.type: Tigera the component also renders tigerakvc.StaticWellKnownJWKSConfigMapName, and that is not watched either.
The manager controller watches both, in its own secret list and at manager_controller.go:197. Without the watches, a change only takes effect on the next periodic reconcile, so logins can use stale settings until then.
Suggest adding the three IdP secret names to this list, plus an AddConfigMapWatch for tigerakvc.StaticWellKnownJWKSConfigMapName. If you also add render.ManagerInternalTLSSecretName to the trusted bundle, it will need a watch here too.
There was a problem hiding this comment.
Added watches for tigera-oidc-credentials, tigera-ldap-credentials and tigera-openshift-credentials, plus LogStorage. The static JWKS ConfigMap is not watched because it is no longer rendered (thread on line 172), and ManagerInternalTLSSecretName is not in the bundle so it needs no watch. tigera/calico-private#12086, Watches in pkg/enterprise/whisker/extension.go.
|
Superseded: the operator now lives in the monorepos, so this change was split and moved there.
Every review comment on this PR is addressed in those commits: the managed-cluster early return (teardown now runs, finalizer and gateway cleanup included), the orphaned UI key pair, the multi-tenant and managed-cluster gates on the manager, the multi-tenant backend, the Dex-only key validator with external IdPs rejected, the key validator files no longer rendered, the bundle-hash roll, the optional-LogStorage degrade, and the missing IdP secret and LogStorage watches. The hardcoded |
Description
New feature: deploy
whisker-backendfor Calico Enterprise, backed by Linseed.For enterprise the Whisker UI is a manager UI module, so only the
whisker-backendcontainer is rendered — no SPA container, nginx config, UI Service or nginx TLS key pair. The backend gets the Linseed upstream env, mTLS client certs, a ClusterRole for Linseed flows plus the RBAC reads it needs (roles/bindings, namespaces, tiers), and Linseed's network policy admits it.Prerequisites and variants
tigera-secure-linseed-cert) before rendering and reportsResourceNotReadyuntil then, so a cluster without log storage degrades instead of running a backend that fails every request.MULTI_CLUSTER_FORWARDING_CA/ENDPOINT, egress to Voltron).OIDC_AUTH_*, Dex CA in the trusted bundle, egress to Dex), and Dex's network policy admits whisker-backend for key fetches. The controller degrades while the Authentication CR is not ready.Manager wires the module up when a Whisker CR exists:
SUPPORTS_FLOW_LOGSfor the UI feature flag, a Voltron/whisker-backendproxy target pointing at the new Service, and the manager/whisker network policies allow that path.Testing. Render and controller unit tests (Dex env/volumes/egress, MCM env, manager egress, Linseed gate degrade/proceed, managed-cluster no-op; Dex policy goldens). Validated live on a standalone enterprise rig: the operator-rendered Deployment carried the MCM and OIDC env, the Dex policy admitted whisker-backend, and a Dex-issued LDAP user token was accepted end to end through Voltron.
Companion: tigera/calico-private#12086 (whisker-backend), tigera/calico-private#12089 (Whisker CR manifests).
Release Note
For PR author
make gen-files(no API changes)make gen-versions(no version changes)AI assistance: Claude Code helped write the rendering, controller gating, tests and this description; every change was reviewed and validated live by the author.