Dynamic Namespaces with restricted license gate#9569
Conversation
|
| Status | Scan Engine | Total (0) | ||||
|---|---|---|---|---|---|---|
| Open Source Security | 0 | 0 | 0 | 0 | See details | |
| Licenses | 0 | 0 | 0 | 0 | See details |
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.
🔍 Preview links for changed docs |
✅ Elastic Docs Style Checker (Vale)No issues found on modified lines! The Vale linter checks documentation changes against the Elastic Docs style guide. To use Vale locally or report issues, refer to Elastic style guide for Vale. |
…ke telemetry dynamic-namespace aware.
1c349c6 to
07438e9
Compare
|
buildkite test this -f p=kind,t=TestNamespaceSelector -m s=9.4.2 |
barkbay
left a comment
There was a problem hiding this comment.
Quick, AI-assisted review (Claude Code), so worth a human sanity-check, especially the non-suggestion comments. Scope: the namespace-selector / license-gate machinery.
Most items are minor and inline below. Two cross-cutting notes that don't map to a single line:
- The six
namespaceFlipRequestsmappers (association, autoops, license, trial, remotecluster, stackconfigpolicy) share a list-cluster-wide, extract, build-requests skeleton; a shared helper taking a per-item mapper would dedupe them while keeping the "use the cache, not the FilterClient" invariant in one place. - Cross-namespace flip behavior (e.g. an in-scope Kibana referencing an ES whose namespace flips out) has unit coverage on the mappers but no integration/e2e; the current e2e only exercises standalone-ES scope-in. An envtest for cross-namespace plus scope-out, and one e2e for the runtime association break, would be worth adding.
Nothing here is a hard blocker.
| if !ok { | ||
| const msg = "Dynamic namespace selector is an enterprise feature. Enterprise features are disabled" | ||
| log.V(1).Info(msg) | ||
| license.EmitEnterpriseFeatureEvent(r.recorder, r.parameters.OperatorNamespace, msg) |
There was a problem hiding this comment.
This emits the event on the operator Pod rather than on the affected resource, so kubectl describe elasticsearch <name> gives no hint why it stopped reconciling. Volume is fine (events aggregate on the single Pod), but discoverability suffers. A status condition on the resource (writable by the ungated license controller) would surface it where users look; worth considering as a follow-up, not a blocker.
| # - key: environment | ||
| # operator: In | ||
| # values: [production, staging] | ||
| namespaceSelector: {} |
There was a problem hiding this comment.
nit: namespaceSelector collides with the existing webhook.namespaceSelector in this same file (different scope) and is asymmetric with its sibling managedNamespaces. Consider managedNamespaceSelector to pair with managedNamespaces and disambiguate from the webhook selector. Cheap to change before it ships as public API.
pkoutsovasilis
left a comment
There was a problem hiding this comment.
Did a first pass, utilised also 🤖 , need to do some manual testing but here are my comments so far
| func (r *ReconcileAgent) onDelete(ctx context.Context, obj types.NamespacedName) error { | ||
| // OnNamespaceOutOfScope releases all controller-local state associated with the given Agent | ||
| // resource when its namespace no longer matches the operator's namespace selector. | ||
| func (r *ReconcileAgent) OnNamespaceOutOfScope(obj types.NamespacedName) { |
There was a problem hiding this comment.
here I think we need to call also this
r.dynamicWatches.Secrets.RemoveHandlerForKey(certificates.CertificateWatchKey(agent.Namer, obj.Name))
and the same happen
There was a problem hiding this comment.
Was that missing in the implementation before this pr?
There was a problem hiding this comment.
no it wasn't so this is a drive-by fix, but sure it can come as a follow-up independent PR
| r.expectations.RemoveCluster(obj) | ||
| // OnNamespaceOutOfScope releases all controller-local state associated with the given Logstash | ||
| // resource when its namespace no longer matches the operator's namespace selector. | ||
| func (r *ReconcileLogstash) OnNamespaceOutOfScope(obj types.NamespacedName) { |
There was a problem hiding this comment.
same comment as the elastic-agent one removing the dynamic watches for http certificates
Co-authored-by: Michael Morello <michael.morello@gmail.com>
Co-authored-by: Michael Morello <michael.morello@gmail.com>
pkoutsovasilis
left a comment
There was a problem hiding this comment.
LGTM, did run the following manual test scenarios and this works as expected, nice!
| # | Scenario | Result |
|---|---|---|
| 1 | ES in labeled vs unlabeled namespace | In-scope reached health=green; out-of-scope had zero status fields, no pods/secrets ever created. |
| 2 | Namespace flip OUT/IN (Kibana + ES association) | Flip out: spec change bumped generation but observedGeneration stayed stuck 30s+ (no new pod). Flip in: caught up within ~5s. OnNamespaceOutOfScope only drops watches, doesn't tear down child resources. |
| 3 | Flip IN, cross-namespace association reference | Kibana (in scope) → ES in a namespace that just came into scope: association went Pending→Established in ~8s with no event on the ES itself — proves association controller's cluster-wide "case B" flip matching. |
| 4 | StackConfigPolicy, 2 ES clusters (1 in/1 out) | Baseline resources=1 readyCount=1/1. Flip out-of-scope in → resources=2, then readyCount=2/2. Flip both out → readyCount=0/0, status.details cleared; ES pods keep running untouched. |
| 5 | Two SCPs, same ES, different weights | High-weight SCP (100, co-located, starts out of scope) vs low-weight SCP (10, operator namespace, always in scope). After flip-in, both report 1/1, but the merged <es>-es-file-settings secret shows the weight-100 value (77mb), higher weight wins. |
| 6 | AutoOpsAgentPolicy, 2 ES clusters (1 in/1 out of scope) | One AutoOpsAgentPolicy in the operator namespace targeting two ES clusters by label (cluster-wide selector, no same-namespace restriction, unlike StackConfigPolicy). Baseline: resources=1 — only the in-scope cluster matched. Flip the out-of-scope one in → resources=2 immediately, and a second AutoOps agent Deployment appears once that cluster finishes provisioning. Flip both out → resources=0 readyCount=0/0, and both AutoOps Deployments get garbage-collected (unlike ES/Kibana's own out-of-scope handling, which just stops reconciling and leaves child resources in place). |
| 7 | Enterprise license removal/recovery (deletion, not time-based expiry — functionally equivalent: both leave CurrentEnterpriseLicense returning nil) |
Deleted eck-license → gated resource's observedGeneration froze, 4x Warning InvalidLicense events fired with the exact expected message. Restored the license → recovered within ~5-10s via an already-in-flight reconcile trigger (no dedicated license-secret watch, so worst case is the 5-minute RequeueAfter). |
|
buildkite test this -f p=kind,t=TestNamespaceSelector -m s=9.4.2 |
|
buildkite test this -f p=kind,t=TestNamespaceSelector -m s=9.4.2 |
What this PR does
Introduces label-selector-based namespace scoping for ECK: instead of a static list of namespace names, operators can now configure a
metav1.LabelSelectorthat the operator evaluates against namespace labels at runtime to determine which namespaces it manages.When
namespaceSelectoris configured, the operator runs in dynamic mode:DefaultNamespacesset).NamespaceMatcherevaluates the configured selector against a namespace's current labels, read live from the informer cache. There is no in-memory match-state to maintain, seed, or replicate: every lookup reflects the cache's present view.watches.NamespacedKind), so events from non-matching namespaces are dropped before they reach a handler.Namespaceobjects and reacts when a namespace flips in or out of the selector's scope (watches.WatchNamespaceScopeChange): resources are picked up or cleaned up immediately — no operator restart required.common.NewNamespacedController, which enforces the enterprise license gate and skips (with local-state cleanup) requests whose namespace has left the scope.FilterClientwraps the manager's client to filterGet/Listresults by namespace, catching any code path that bypasses the event-driven filtering.Because matching is evaluated live from each replica's own cache, all operator replicas — leader or not — answer scope questions identically. Webhooks served by a non-leader pod behave exactly like the leader's, and failover has no warm-up window.
Changes by area
pkg/controller/common/nsmatch— core packageNamespaceMatcher(ns.go)A small, stateless component: it holds the parsed
labels.Selector, the set of always-managed namespaces (the operator's own namespace, plus the empty string so cluster-scoped objects always pass), and a reference to the manager's cache (wired in viaSetCacheafter manager creation).NewNamespaceMatcher(sel, operatorNS)nilselector → matcher disabled (legacy / static mode).SelectorEnabled()NamespaceMatches(ns)corev1.Namespace's labels. Always-managed namespaces short-circuit totrue.NamespaceNameMatches(ctx, name)falsewhen the namespace cannot be read (conservative).MatchingNamespaces(ctx)No-op when disabled: with a
nilselector every check returnstrue. Callers need no conditional logic; the matcher can be wired unconditionally.FilterClient(filter_client.go)A
client.Clientwrapper injected as the manager's client (viaopts.NewClient) whennamespaceSelectoris configured.Get: when the requested key's namespace does not match, returns a syntheticNotFounderror without querying the delegate — the object is invisible to callers. TheGroupResourcein the error is best-effort resolved from the scheme/RESTMapper so it renders like a real API-serverNotFound; as with a realNotFound,objis left untouched. Cluster-scoped objects have an emptykey.Namespace, which always matches.List: filters out items in non-matching namespaces after the underlyingListreturns. Fast path: when the list is already scoped to a single namespace (client.InNamespace), one match check decides the whole result.This is the safety net behind the event-driven filtering: reconcilers reading through the manager's client cannot see out-of-scope objects even if a request slips through.
pkg/controller/common/watches— additionsNamespacedKind(namespaced_kind.go)Drop-in replacement for
source.Kindthat prepends a predicate callingNamespaceNameMatches(obj.GetNamespace()), so events from non-matching namespaces are filtered before reaching the handler. When the matcher is disabled it delegates directly tosource.Kindwith no overhead. Used for every namespaced resource watch across all controllers; cluster-scoped watches (webhook configurations) keepsource.Kind.To keep this enforced going forward, a new forbidigo lint rule (
.golangci.yml) forbids directsource.Kindcalls; the few deliberate uses (cluster-scoped watches, the wrapper itself, the Namespace watch) carry an explanatory//nolint:forbidigo, and test files are exempt.The
WatchPodsandWatchSoftOwnedSecretshelpers now take the matcher and useNamespacedKindinternally.WatchNamespaceScopeChange(namespace_flip.go)Registers a watch on
corev1.Namespaceobjects (a no-op when the selector is disabled) with predicates that only let scope changes through:ObjectOldandObjectNew— i.e. the label edit crossed the selector boundary. Irrelevant label churn is dropped.The flip detection is a pure comparison of the old and new objects carried by the event itself — no stored state, hence no seeding step and no startup race to defend against.
On a flip, a caller-provided mapper (
func(ctx, *corev1.Namespace) []reconcile.Request) enumerates what to re-enqueue.ReconcileObjectsInNamespaceis the default mapper: it lists the controller's kind in the flipped namespace and enqueues one request per object. It lists via the cache, not theFilterClient: when a namespace is being de-scoped, theFilterClientwould hide the very objects whose cleanup reconciles need to be enqueued.pkg/controller/common/namespaced_controller.go— newcommon.NewNamespacedControlleris the registration entry point for every controller whose reconciliation must be restricted to the selector's scope. When the selector is disabled it behaves exactly likecommon.NewController. When enabled, it:EnterpriseFeaturesEnabled; while disabled, a Warning event is emitted on the operator Pod and the request is requeued after 5 minutes;OnNamespaceOutOfScopeis invoked instead, letting the reconciler release any state it holds for the resource.WatchNamespaceScopeChangeflip watch with the mapper supplied by the controller.The new
NamespacedReconcilerinterface isreconcile.Reconcilerplus:Per-kind controllers (ES, Kibana, APM, Agent, Beat, Logstash, Enterprise Search, Maps, Package Registry, ES autoscaling)
Every resource controller receives the same three changes:
common.NewControllertocommon.NewNamespacedController, passingwatches.ReconcileObjectsInNamespacefor its primary CRD list type (e.g.&esv1.ElasticsearchList{}) as the flip mapper.addWatches: allsource.Kindcalls for namespaced resources (primary CRDs, StatefulSets, Deployments, Pods, Services, Secrets, ConfigMaps, PodDisruptionBudgets) are replaced withwatches.NamespacedKind.OnNamespaceOutOfScopeis implemented on the reconciler, releasing controller-local state — stopping observers, removing dynamic watch handler keys for secrets/configmaps — without touching the resource itself.onDeletenow callsOnNamespaceOutOfScopeplus the deletion-only steps (expectations removal, soft-owned secret GC).Note on the controllers that track reconciliation expectations (Elasticsearch, Logstash):
OnNamespaceOutOfScopedeliberately does not clear expectations. The resource and its children still exist, and the namespace may move back into scope before the controller cache observes recent StatefulSet/Pod changes — keeping expectations preserves the usual stale-cache safety across short namespace-label flaps. Expectations are only cleared on CR deletion or operator restart.Association controllers (
pkg/controller/association) — cross-namespace reference re-enqueueAssociations are the one place where a namespace flip must trigger reconciliation of resources living outside the flipped namespace: the associated resource (e.g. a Kibana in namespace A) can reference a resource in a different namespace (e.g. an Elasticsearch in namespace B).
The scenario this handles: namespace B is de-scoped while namespace A stays in scope. Kibana A's association still points at ES B, but nothing would re-enqueue Kibana A:
NamespacedKindpredicate on the referenced-resource watch drops events from the de-scoped namespace B.Establishedassociation returns no requeue, so Kibana A would keep a stale association conf pointing at a now-invisible Elasticsearch indefinitely.The fix: the association controllers pass a custom flip mapper. On a flip it lists the associated kind cluster-wide from the cache (a new
AssociatedObjListTemplatefield onAssociationInfoprovides the list type) and enqueues:AssociationTypewhoseAssociationRef()points into the flipped namespace.AssociationRef()already resolves an empty ref namespace to the associated resource's own namespace, so the comparison is direct.Once Kibana A is enqueued, the existing reconcile logic does the rest: the
FilterClientmakes ES B read asNotFound, so the association conf is removed and the status transitions toPending. The inverse direction works the same way — when namespace B (re-)gains eligibility, Kibana A is re-enqueued and the association is re-established immediately instead of waiting for thePendingrequeue backoff.Known limitation: transitive references (e.g. Agent → Fleet Server → Elasticsearch, each in a different namespace) are matched on the direct reference only; a flip of the transitive Elasticsearch namespace does not re-enqueue the Agent.
Other controllers with custom flip mappers (AutoOps, StackConfigPolicy, RemoteCluster)
Three more controllers use
NewNamespacedControllerwith their own mapper, because the resources a flip affects are not simply the ones living in the flipped namespace. All of them list from the cache rather than theFilterClientfor the same de-scoping reason as above.AutoOpsAgentPolicyselects its Elasticsearch clusters cluster-wide by label (ResourceSelector), so a flip of any namespace can change any policy's set of matching clusters — policies in the flipped namespace must be picked up or cleaned up, and policies elsewhere must deploy agents for newly scoped clusters or clean up agents and API keys for de-scoped ones. The mapper re-enqueues all policies, mirroring the controller's Elasticsearch watch (reconcileRequestForAllAutoOpsPolicies).Spec.RemoteClustersreference a cluster in the flipped namespace.Reconcilere-evaluates all of a cluster's remote-cluster relationships in both directions (getExpectedRemoteClientsFor), so enqueueing the counterparts is enough to establish or tear down trust; theFilterClientthen decides what remains visible.License and trial controllers — namespace-scoped but deliberately not license-gated
Both controllers keep
common.NewControllerand are not wrapped byNewNamespacedController, each for its own reason:NamespacedKind, and a flip watch re-enqueues clusters when a namespace moves in or out of scope. Its flip mapper: if the flipped namespace holds an operator license secret, the licensing outcome may change for every cluster, so all currently in-scope clusters are re-enqueued (listed through the filtering client, which at that point yields exactly the clusters in scope); otherwise only the clusters in the flipped namespace (listed via the cache, so a de-scoped namespace's contents remain visible for the decision).NamespacedKind, and its flip mapper re-enqueues the trial-license/trial-status secrets living in the flipped namespace.Webhooks (
pkg/controller/common/webhook/resource_validator.go,cmd/manager/validation.go)ResourceValidator[T]gains anamespaceMatcherfield set viaWithNamespaceMatcher(m).preValidatenow branches:SelectorEnabled(): skip validation when!NamespaceNameMatches(ctx, ns)(dynamic mode).managedNamespacescheck (legacy mode).The webhook server receives admission requests for every namespace in the cluster: several operator instances, each watching a different set of namespaces, may be installed side by side, but only one of their webhooks is actually registered. Namespaces that do not match — including ones that cannot be read from the cache — are silently let through instead of denied, so an operator never rejects a request for a namespace it doesn't manage. Because the matcher reads the namespace live from the local cache, this works identically on any replica, including non-leaders serving webhook traffic.
All
RegisterResourceWebhook/RegisterWebhookcall sites invalidation.gopass the matcher.The webhook certificates controller (
pkg/controller/webhook/webhook_certificates_controller.go) also takes the matcher now, applyingwatches.NamespacedKindto its webhook-cert Secret watch. Its watch on theValidating/MutatingWebhookConfigurationitself stays on plainsource.Kind, since those are cluster-scoped resources with no namespace to filter on.pkg/controller/common/license/event.go— new fileEmitEnterpriseFeatureEvent(recorder, operatorNS, msg): emits a Warning event on the operator's own Pod when an enterprise feature is used without a valid license. Pod name is resolved fromos.Hostname()(Kubernetes always sets this to the pod name); a no-op if the hostname cannot be determined.cmd/manager/main.goand operator configurationNamespaceSelectorFlag = "namespace-selector"constant (config-file only; no CLI flag, as the value is a structuredmetav1.LabelSelectorobject).parseNSSelector: reads the viper value, round-trips throughyaml.Marshal/yaml.Unmarshalintometav1.LabelSelector, then converts viametav1.LabelSelectorAsSelector.namespacesandnamespace-selectorare mutually exclusive; supplying both is a fatal error.DefaultNamespacesis left empty, widening the cache to cluster scope, andopts.NewClientinjectsnsmatch.NewFilterClientso all manager-level client calls are namespace-filtered.nsmatch.NewNamespaceMatcheris constructed once, given the manager's cache viaSetCache(mgr.GetCache()), and threaded intooperator.Parameters.NamespaceMatcher.MatchingNamespacesin dynamic mode instead of the (empty) static list.Telemetry (
pkg/telemetry/telemetry.go)telemetry.NewReportertakes theNamespaceMatcher. A newgetNamespaceshelper decides which namespaces to scan:MatchingNamespaces(ctx)in dynamic mode (the staticmanagedNamespaceslist is empty/meaningless under a selector), the existing static list otherwise. BothgetResourceStatsandreportuse it, so telemetry follows namespaces as they move in and out of scope at runtime, the same way reconciliation does.Helm chart (
deploy/eck-operator/)values.yaml:configmap.yamlemits thenamespace-selector:block under operator config when the value is non-empty.validate-chart.yamlfails rendering whennamespaceSelectoris combined withmanagedNamespaces, or withcreateClusterScopedResources: false.End-to-end test (
test/e2e/namespace_selector/namespace_selector_test.go)TestNamespaceSelectorDynamicLabelChange(skipped unless a test enterprise license is available):ns1witheck-visible=true, and leavesns2unlabeled.matchLabels: {eck-visible: "true"}selector and waits for the config-change restart.ns1(normal reconciliation) and that an Elasticsearch created inns2gets no pods.ns2, then asserts the ES cluster there is reconciled to health and that the operator did not restart — the namespace was picked up dynamically.t.Cleanuprestores the labels and original operator config.Shared e2e helpers were factored out into
test/e2e/test/helper(UpdateOperatorConfig,GetOperatorConfigValue,OperatorRestartCount,SetNamespaceLabel/DeleteNamespaceLabel);global_ca_test.gonow uses them instead of its private copies.Unit/integration coverage:
filter_client_test.go,license_controller_test.go, associationcontroller_test.go,autoops/namespace_flip_test.go,stackconfigpolicy/namespace_flip_test.go,remotecluster/watches_test.go, and telemetry tests for the dynamic path.License gating
The dynamic namespace selector is an enterprise feature, enforced per-reconciliation by the
NewNamespacedControllerwrapper:Backward compatibility
namespaceSelectoris not set theNamespaceMatcheris constructed with anilselector: every check returnstrue,NewNamespacedControllerfalls through toNewController,NamespacedKinddegrades tosource.Kind, andWatchNamespaceScopeChangeis a no-op. Existing behavior is unchanged.FilterClientis only injected whennamespaceSelectoris configured; all other modes use the default client.namespaceslist and the newnamespaceSelectorare mutually exclusive, validated both at chart-render time and at operator startup.What is not in scope
createClusterScopedResources: false): dynamic mode requires a cluster-wide cache and therefore a ClusterRole. The namespace-scoped profile is not supported.LabelSelectorAND-s across keys; OR within a single key is available viaIn).