Skip to content

feat(spectre): implement Spectre dCP operator (Phases 0-D) - #506

Open
xCatalitY wants to merge 89 commits into
mainfrom
feat/spectre
Open

feat(spectre): implement Spectre dCP operator (Phases 0-D)#506
xCatalitY wants to merge 89 commits into
mainfrom
feat/spectre

Conversation

@xCatalitY

@xCatalitY xCatalitY commented Jul 24, 2026

Copy link
Copy Markdown

Summary

Implements the Spectre operator for the distributed Control Plane, enabling API traffic capture via gateway-jumper integration.

Scope:

  • Phase 0: Scaffold fixes (API group, go.mod, Makefile, real CRDs)
  • Phase A: Gateway RouteListener CRD + FeatureBuilder (jumper_config injection + /listener path switch)
  • Phase B: Spectre operator (ListenerHandler + SpectreApplicationHandler)
  • Phase C: Rover producer (spec.listeners → SpectreApplication + Listener CRs)
  • Phase D: E2E validation (14 scenarios, 2365 LOC)
  • D1–D5: Callback ACL identity gap fixes, Consumer owner index, appId guard

Modules touched: spectre/ (new), gateway/, rover/, rover-ctl/, rover-server/, event/

Key design decisions:

  • Direct-to-pubsub (bypasses EventType CRD admission that requires .vN suffix)
  • RouteListener = gateway domain, authored by Spectre
  • Peer domain pattern (Spectre owns its SSE routes, like api/ and admin/ do)
  • Single approval gate at Listener level

Bug fixes included:

  • Gateway controller was setting Kong service to /proxy instead of /listener when RouteListeners are present
  • Event controller: create missing gateway Consumer for callback clients, derive ACL from mesh client spec, render Routes independently of event backend, index and own the Consumer CR
  • Spectre: prevent Listener provisioning with unresolved application ID

Verified on real infrastructure:

  • spectre-test vCluster: verify-spectre-e2e.sh 37/38 pass (1 skip: no SSE backend)
  • Full event chain: Jumper → Starlight → Galaxy → Comet callbacks delivered

Note for merge: Re-render the pre-built bundles — clusters/awsd-dev-distcp1 and clusters/dedicated-non-prod-0 need gateway.cp.ei.telekom.de/consumers in event-manager-role.

Test plan

  • cd spectre && make test — all pass (3 packages)
  • cd event && make test — 87 tests pass
  • cd rover && make test — all pass
  • cd rover-server && make test — 31 tests pass
  • spectre-test vCluster E2E — 37/38 pass
  • Full-chain deploy with latest commits (pending image build)
  • CI green after rebase onto main

@github-advanced-security

Copy link
Copy Markdown

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

@xCatalitY
xCatalitY marked this pull request as ready for review August 19, 2026 09:22
Copilot AI lite review requested due to automatic review settings August 19, 2026 09:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@xCatalitY xCatalitY closed this Aug 19, 2026
@xCatalitY xCatalitY reopened this Aug 19, 2026
@xCatalitY
xCatalitY force-pushed the feat/spectre branch 2 times, most recently from 729230e to 80ec377 Compare September 2, 2026 05:46
ivankrylow and others added 20 commits September 3, 2026 09:18
The kubebuilder scaffold doubled the API group to
spectre.ei.telekom.de.cp.ei.telekom.de because the --group flag
was passed as "spectre.ei.telekom.de" instead of just "spectre".

This corrects the group across all source and config files:
- groupversion_info.go: fix +groupName marker and GroupVersion var
- PROJECT: fix group field from "spectre.ei.telekom.de" to "spectre"
- listener_types.go: align with controller-runtime scheme.Builder
- controller RBAC markers: fix apiGroups
- config/rbac/*.yaml: fix apiGroups
- config/crd/kustomization.yaml: fix CRD filename reference
- config/samples: rename file and fix apiVersion
- Makefile: fix boilerplate paths to use ../hack/ (repo root)
Add local replace directives for all monorepo dependencies (common,
common-server, admin/api, application/api, approval/api, gateway/api,
identity/api, pubsub/api, secret-manager, event/api) matching the
pattern used by the event module. Also bumps go directive to 1.26.5
as required by the common module.
Replace spectre-system with controlplane-system in kustomization and
e2e tests to match the shared controlplane namespace convention used
by all other domain modules (event, gateway, etc.).
…Application CRDs

Replace the scaffolded placeholder ListenerSpec (with single Foo field)
with the full Listener and SpectreApplication CRD types from the design
doc. Both types implement the common types.Object interface (GetConditions,
SetCondition) and have compile-time assertions.

Changes:
- ListenerSpec: consumer/provider TypedObjectRefs, optional ApiListener
  and EventListener with filter support
- ListenerStatus: conditions, routeListener, eventSubscriptions,
  providerApproval, consumerApproval refs
- SpectreApplicationSpec: application ref, deliveryType enum with CEL
  XValidation for callback field
- SpectreApplicationStatus: conditions, id, publisher, subscriber,
  listenerRoute, proxyRoute refs
- schema.go: RegisterSchemesOrDie for all cross-domain API types
- SpectreApplication controller + handler stubs
- cmd/main.go: wires both controllers via RegisterSchemesOrDie
- Test updated to initialize controller properly with required spec fields
Implement handler utility package for the Spectre operator:

- GetListeningZone: ports legacy ListenerUtil preference logic
  (listener zone > provider zone > consumer zone > blocked)
- GetEventConfig: resolves EventConfig for a zone via field index
- Naming helpers: MakePublisherName, MakeSubscriberName,
  MakeRouteListenerName, MakeBridgeSubscriberId,
  BuildListenerEventType, BuildBridgeCallbackURL
- Constants: PublisherID ("gateway"), GenericEventType

27 Ginkgo specs with 96.8% coverage.
Bug 1: RouteListener Spec.Route referenced itself (routeListenerName)
instead of the actual gateway Route CR. Added findRouteByPath helper
that resolves the Route by matching Spec.Paths against apiBasePath,
and returns a BlockedError if no Route exists yet.

Bug 2: GatewayClient credentials (ClientId, Issuer) were not propagated
to the JumperConfig RouteListenerEntry. Extended the struct and updated
the Apply() method to copy them from the RouteListener spec.

Bug 3: RouteListener listing was placed outside the !PassThrough guard,
causing unnecessary listing for passthrough routes that skip security
features entirely. Moved inside the guard block.

Bug 4: Route controller did not watch RouteListener changes, so
RouteListener creation/update would not trigger Route re-reconciliation.
Added Watches() with mapRouteListenerToRoute mapping function.
The branch still carried the old per-module ci.yaml structure from
before #626 redesigned CI. Two rebased commits re-introduced the
old format. Reset to main's version — module config now lives in
.github/ci/modules.yaml.
RouteListener and bridge Subscribers live in the zone namespace while
the Listener lives in the team namespace. Kubernetes does not support
cross-namespace ownerReferences, so these children had no durable
ownership marker — cleanup relied solely on refs in Listener.status.

Stamp cp.ei.telekom.de/owner.uid on each child in its CreateOrUpdate
mutator and add label-based Cleanup calls in Delete as a fallback for
children whose status ref was never recorded. Matches the pattern used
by permission and admin/zone for cross-namespace children.
Upgrades google.golang.org/grpc from v1.82.1 to v1.83.1 across all 14
modules. v1.82.1 is vulnerable to heap exhaustion through fragmented
HTTP/2 DATA frames; v1.83.1 contains the receive-buffer compaction fix.
…changes

The Listener controller had no watch on SpectreApplication, so when a
Listener reconciled before its SpectreApplication set Status.Id, it
returned a BlockedError and waited 30-51 minutes for the next requeue.

Add a mapped Watches that enqueues every Listener referencing a changed
SpectreApplication, using ResourceVersionChangedPredicate (status
updates don't increment generation). Follows the same pattern as
eventconfig watching zones.
…dependencies

- Add webhook validation rejecting unsupported listener inputs (event types,
  filters, callback without URL, SSE on proxy zones, Spectre feature disabled)
- Resolve consumer/provider Application refs by exact name lookup instead of
  hardcoding rover namespace, handling cross-team namespaces correctly
- Replace non-deterministic List+Items[0] EventStore resolution with
  EventConfig.Status.EventStore direct-Get path, rejecting duplicate configs
- Tighten Spectre Listener CRD: apiListener required, eventListener/filters
  blocked via XValidation until implemented
- Align rover-server OpenAPI to supported input (apiBasePath required, remove
  unsupported eventType/filter fields), regenerate server.gen.go
- Update SSE proxy guard message to clarify local-zone-only constraint
Replace the constant-hash approval with a content-bound authorization
fingerprint derived from the full capture intent (consumer, provider,
application, path, directions, delivery mode, callback target, and
filter state). The fingerprint is:

- passed to WithHashValue so a changed intent creates a new
  ApprovalRequest (forces re-approval)
- applied as a label to RouteListeners and bridge Subscribers so
  stale children can be identified

Before evaluating the replacement grant, children whose fingerprint
differs from the current intent (or is missing, for pre-migration
resources) are deleted — RouteListeners first so no new traffic is
captured while the replacement approval is pending.

Each approval state is now handled explicitly:
- Granted: provision children, janitor cleanup removes extras
- Denied: delete all owner-labelled children, clear status
- Pending: do not provision (stale already removed)
- RequestDenied: retain same-intent children per builder contract

Approval properties now carry consumer/provider ApplicationRefs and
the full capture intent map for human-readable review.
Pass-through routes skip authentication (the route handler wraps
RouteListener collection in `if !route.Spec.PassThrough`), and
failover routes overwrite the /listener upstream to /proxy
(priority 109 > 103). Both silently break listener capture.

Spectre handler: resolve the gateway Route before approval
evaluation and reject pass-through/failover modes with a
BlockedError after cleaning up any existing capture children.

Gateway RouteListener handler: report NotReady/Blocked when the
referenced Route is pass-through or has failover config.

Gateway Route handler: skip RouteListener listing when the route
has failover config, preventing an invalid RouteListener from
breaking normal failover traffic.

RouteListenerFeature: defensive rejection if invoked directly on
an unsupported route mode via another builder path.
… usage

Listener Delete now runs in explicit phases:
1. Resolve publisher namespace before clearing status (status refs,
   then owner-label fallback, then topology).
2. Delete RouteListener first (stop new capture).
3. Delete bridge Subscribers (status + owner-label).
4. Fresh-list Subscribers — retry with RetryableWithDelayErrorf while
   any remain (finalizers still running).
5. Only after Subscribers gone, check generic Publisher orphan.

The generic Publisher refcount now lists Subscribers in the zone namespace
and checks whether any reference the Publisher, instead of globally
counting Listeners. Terminating Subscribers still count.

SpectreApplication Delete now phases: Subscribers first (with fresh-list
retry), then Publishers, then Routes — each combining status ref and
owner-label deletion.
- pubsub: remove inner ctx shadow in publisher/subscriber controller
  tests (govet lint blocker)
- rover: wrap stale-object-prone Update in Eventually+re-fetch loop
  in rover_controller_test.go (resource-version conflict)
- rover: validate that listener.Consumer == rover.Name in webhook
  (prevents impersonation via same-team auto-approval)
- spectre: add defense-in-depth check that Listener consumer matches
  SpectreApplication's Application in listener_handler.go
- spectre: use application.Status.ClientId (team-scoped) instead of
  application.Name for appId in spectreapplication_handler.go
- tests: add webhook + handler tests for consumer identity enforcement;
  update fixtures for ClientId-based appId
When two Spectre listener applications observe the same consumer+provider
route, each creates its own RouteListener CR. Previously the gateway
feature rejected the second as a conflict unconditionally.

Now entries with identical Issue and ServiceOwner are recognized as
duplicates from different observers and silently skipped. Only entries
that actually differ (different Issue or ServiceOwner) produce an error.

The GatewayClient is similarly guarded against redundant overwrites.
Add a CEL XValidation rule on EventTypeSpec that rejects event types
starting with 'de.telekom.ei.listener'. This prefix is used internally
by Spectre for listener event types and must not collide with
user-created event types.

The same check is added to the Rover webhook for both event exposures
and event subscriptions, catching the reserved prefix before the
EventType CR is even created.

Includes CRD regeneration and a test verifying the rule is present
in the generated CRD YAML.
When a SpectreApplication's zone is a proxy zone (EventConfig has
Spec.Proxy set), SSE delivery previously returned a BlockedError.

Now Spectre creates two Routes:
- A primary SSE Route in the backend (local) zone where Pulsar serves SSE
- A proxy SSE Route in the app's own zone that forwards to the backend
  zone's gateway

The primary route trusts both the backend zone's IDP issuer and the app
zone's LMS issuer so proxy-forwarded requests are accepted. The proxy
route trusts the app zone's IDP issuer for direct subscriber access.

Also removes the admission-time IsProxy rejection from the Rover webhook
since proxy-zone SSE is now supported.
Register SpectreApplication and Listener controllers with the envtest
manager so watches fire automatically. Add 7 scenarios that verify
watch-driven reconciliation without calling Reconcile() directly:

1. Approval grant provisions children (RouteListener + Subscribers)
2. Approval suspension removes capture
3. Route creation unblocks a blocked Listener
4. Route pass-through deprovisions capture
5. EventStore readiness requeues SpectreApplication
6. Child readiness updates parent status
7. SpectreApplication status.Id requeues Listeners

Scenario 8 (Rover→Spectre chain) is documented as needing a separate
rover test suite and skipped.

Adapt the existing listener_controller_test to tolerate concurrent
manager-driven reconciliation (conflict retry, NotFound in cleanup).
- Extend EventConfig mappers (both SpectreApplication and Listener
  controllers) to follow proxy targets: when a backend zone's
  EventConfig changes, SpectreApplications and Listeners in proxy
  zones that reference it are also enqueued for reconciliation.

- Add Realm watch to the Listener controller so that IssuerUrl
  changes in a Realm trigger re-reconciliation of Listeners whose
  Applications are in Zones referencing that Realm. The RBAC marker
  already existed.

- Add condition.EnsureReady(targetZone) in resolveSSEBackendZone
  so proxy resolution blocks when the target zone is not ready.

- Add mapper tests for proxy-aware EventConfig mapping and Realm
  mapping.
…orting Ready

The scoped client's AllReady() uses IsStatusConditionFalse, which treats
children with no conditions (freshly created, not yet reconciled by
downstream controllers) as ready. This allowed Spectre parents to report
Ready=True before children were confirmed ready.

Add ensureChildReady() helper that re-fetches each status-ref child and
verifies it carries an actual Ready=True condition via condition.EnsureReady().
Applied after the existing AnyChanged()/AllReady() blocks in both
SpectreApplicationHandler and ListenerHandler.

Also includes:
- Revert of common/pkg/client/client.go comment (the !IsStatusConditionTrue
  change broke all envtest suites; fix is at handler level instead)
- Fix common/pkg/client/client_test.go assertion to match reverted behavior
- Fix rover spectre watch test: manually set children Ready in envtest
  (no downstream controllers) and increase timeout
…eady

The ensureChildReady check now requires explicit Ready=True on
RouteListeners and Subscribers. In envtest without downstream
controllers, children must be manually set Ready.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants