Repository navigation
feat(api): add EndpointAttachment to attach addresses to app endpoints - #4324
Timofei Larkin (lllamnyp) wants to merge 1 commit into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
A managed application's tenant-facing Services are its endpoints, and the only way to publish one externally has been the chart boolean `external`, which publishes one chart-chosen endpoint on one address from no particular pool through five different render paths. EndpointAttachment attaches one address, minted from an IPAddressClass or bound from a reserved IPAddressClaim, to one tenant-facing Service by rendering an additive LoadBalancer Service that mirrors the endpoint's selector and ports. The mechanism therefore needs no per-engine render knowledge and never touches the application's own objects, and a reserved address survives every attachment and moves between applications. The endpoint check requires both the lineage labels and the tenantresource marker: lineage alone also covers operator-internal Services. Identity is by owner reference throughout, so a Service carrying the ownership label without the reference is never adopted, and a same-name recreated application does not inherit the previous incarnation's attachments. The VM datapath modes render the cozy-proxy Service contract unchanged and are gated on a platform declaration rather than CNI detection: a proxy skips a Service delegated to a name it does not claim while the load balancer still attracts the address. Exclusivity is keyed on the backend pod in both modes because cozy-proxy's egress map holds one address per pod, and a whole-IP attachment with several ready backends keeps its address dark instead of picking one. The e2e suite is parked as disabled until the address substrate is packaged; the unit tests carry the hold/move and identity proofs. The kind lives in local.sdn.cozystack.io, the CRD-served half of the sdn family the address substrate and cozyplane share, so stock ResourceQuota counts keep working; the proposal text is being corrected from cozystack.io to match. Assisted-by: LLM Signed-off-by: Timofei Larkin <[email protected]>
95da393 to
1d3ab0b
Compare
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
Timofei Larkin (@lllamnyp) NOT LGTM. CI has been red on this since 2026-09-18, and the address pool is open to every tenant.
- The CEL rule for
familyinapi/localsdn/v1alpha1/endpointattachment_types.go:80uses a typographic quote (U+201D) where''belongs. The committed CRD was fixed by hand, socrdcel_test.gopasses, butmake generatewrites the bad quote back. That is why "Verify generated code" and pre-commit are red (run 35341308515), and a CRD generated from the marker would be rejected by the apiserver. - The controller reads
IPAddressClass.spec.loadBalancerClass(substrate.go:159-166), but address-controller has no such field. The class always comes out empty (render.go:152), whiledocs/endpoint-attachments.md:46and the body say it matches. - Tenants get
*on endpointattachments, and the controller creates the IPAddressClaim for anyclassNamewith its own service account (substrate.go:135-153). Neither this PR nor address-controller checks which classes a tenant may use, and there is no default quota. A tenant can take addresses from any pool, and withRetain, creating and deleting attachments in a loop drains a public pool. It needs an allow-list of classes tenants may use, or a default quota that counts retained addresses. - The e2e suite is parked as
chainsaw-test.yaml.disabled, so the integration with the address substrate has never run. I'd land this after #3382 and turn the suite on in the same PR.
There are conflicts with main in the cozystack-controller deployment.yaml and values.yaml. Main added a resources block there, and the new args have to stay under args:, above it.
Not blocking: dual-stack and IPv6 don't set ipFamilyPolicy (render.go:133-173). spec.services in an ApplicationDefinition now also means "can be exposed", and that list includes openbao-*-internal, qdrant-*-headless and etcd, so the website page on application definitions needs an update and the "no trigger matched" line in the body is incomplete.
go test passes for endpointattachment, localsdn and lineagecontrollerwebhook, and helm unittest passes for cozystack-controller, cozystack-basics and platform, also on a trial merge with main.
What this PR does
Implements
EndpointAttachment(local.sdn.cozystack.io/v1alpha1, namespaced) from the design proposal in cozystack/community#45: a tenant-created resource that attaches one external address to one tenant-facing Service of one managed application. It is the Network load-balancing path (a dedicated address per attachment), complementary to SNI exposure through the tenant Gateway, and it implements theloadBalancerunion member only. Nothing in it routes by hostname, touches TLS, or reads chart values.API (
api/localsdn/v1alpha1/endpointattachment_types.go, CRD generated into the cozystack-controller chart). The kind lives inlocal.sdn.cozystack.io, the CRD-served half of the sdn family the address substrate and cozyplane already share, so stockResourceQuotacounts still work; the proposal text is being corrected to say so.spec.applicationRef(group/kind/name, group defaulting toapps.cozystack.io),spec.endpoint.serviceName, andspec.loadBalancerwithclassName|claimName(exclusive),family,ports,method(WholeIP|PortList),allowICMP. CEL freezes the address-identity fields (applicationRef,className,claimName,family) and the member selection, requires exactly one union member (written as a list so agatewaymember can be added without restructuring), and holds the static shape rules (PortListneedsports,allowICMPisPortList-only). Everything cross-object or dynamic is a condition.status.phaseis a projection of theResolvedandProvisionedconditions.Controller (
internal/controller/endpointattachment), one reconcile in four steps:applicationRefandinternal.cozystack.io/tenantresource: "true"; lineage alone is refused (EndpointLineageMismatch/EndpointNotTenantFacing), as is a Service rendered by another attachment. A same-name recreated application (different HelmRelease UID) is reported asApplicationIncarnationChanged, the Service is withdrawn, and the owner reference is never rewritten.claimNamereferences an existingIPAddressClaimand writes nothing to it; otherwise a claim is minted withgenerateNameunder the attachment's controller reference. The substrate kinds are read as unstructured, so the controller carries no module or CRD dependency on it and reportsClaimPendingwhere it is not installed.loadBalancerClassis read from the claim's resolvedIPAddressClassbefore the Service is created and left empty when the class leaves it empty. An address whoseIPAddress.status.associatedTonames another live Service givesClaimInUsewith no contention. Afamilyconflicting with a referenced claim givesFamilyConflict.type: LoadBalancerService viagenerateName, identified by controller owner reference only (a Service with the ownership label and no reference is ignored, never adopted). It mirrors the endpoint's selector and ports (or theportssubset; synthesized ports when the endpoint has only the sentinel port), carries thelocal.sdn.cozystack.io/ip-address-claimannotation,externalTrafficPolicy: Local, andallocateLoadBalancerNodePorts: falseunless the platform says otherwise. It copies no labels or annotations from the endpoint. Re-mirroring is event-driven through watches on Services, EndpointSlices, HelmReleases and, where served at startup, the claim kind.status.addressesmirrors the claim's bound addresses (a list),status.serviceNameandstatus.claimNamename the objects, andProvisioned=Trueasserts address and Service wiring only; its message says so, since reachability additionally depends on the CNI's policy objects.VM datapath modes.
methodrenders the cozy-proxy Service contract exactly (theservice.kubernetes.io/service-proxy-name: cozy-proxylabel,networking.cozystack.io/wholeIP,networking.cozystack.io/allowICMP, the sentinel port for whole-IP without ports) and adds nothing beside it. Rendering is gated on a platform declaration, not CNI detection: the controller flag--cozy-proxy-contract-implementedis threaded from the system bundle, which sets it exactly when it emits the cozy-proxy Package; without it a method-bearing attachment renders no Service and reportsDatapathContractUnavailable, because a load balancer would still attract the address while every proxy skipped the Service. Exclusivity is keyed on the backend pod and counts every delegated Service in either mode (attachments, chart Services rendered withexternal: true, hand-made ones): the oldest delegation wins, ties by name, and the loser reportsBackendAlreadyDelegatedand renders nothing rather than fighting for the slot.WholeIPon an endpoint with more than one ready backend keeps its Service and claim but selects nothing (MultipleBackends), so a rolling update or live migration passes through held-but-dark and recovers without churning the address.Detach semantics. An endpoint Service that disappears withdraws the rendered Service, keeps the claim (minted or referenced), and reports
Detached; the same claim is reused when the endpoint returns. Deleting an attachment collects its Service and its minted claim; a referenced claim is untouched.Wiring. The controller is registered in
cmd/cozystack-controllerbehind two flags (--cozy-proxy-contract-implemented,--endpoint-attachment-allocate-node-ports), the chart exposes them ascozystackController.endpointAttachments.*, the platform system bundle derives both (cozy-proxy emitted, Hetzner RobotLB enabled), the controller ClusterRole gains Service writes, theendpointattachmentskind andipaddressclaimscreate/delete, and the tenant ClusterRoles getendpointattachmentsbesidesecuritygroups(full incozy:tenant:base, read incozy:tenant:view:base, write incozy:tenant:admin:base).Tests.
crdcel_test.gocompiles the shipped CRD and pins the create and update admission matrix.reconciler_test.go(fake client) covers the resolve/deny matrix over both label families, mirroring and port subsetting, the minted-claim path, the two proof sequences (a labelled-but-unowned Service is ignored and a same-name recreated application is not re-acquired; a reserved claim holds across detachment and moves to a different application), detach/reattach, the sentinel-port rule, the capability gate, both modes, held-but-dark on multiple backends, pod-keyed exclusivity, the substrate-absent path and the family conflict. Chart unit tests pin the platform declaration on every bundle variant.hack/e2e-chainsaw/endpointattachmentcarries the postgres hold/move sequence, GC, the internal-Service refusal and the VM mode Test (re-scoped to a single delegated Service per VM, since current cozy-proxy's rule installer keeps one 1:1 slot per pod), parked aschainsaw-test.yaml.disabledbecause the address substrate is not packaged in Cozystack yet; its README states the preconditions.Docs.
docs/endpoint-attachments.mddocuments the resource, the authorization seam, the lifecycle, the conditions' exact meaning, the VM modes and their two rules, and the TLS posture: attaching a database does not turn its TLS on, and the plaintext-on-a-public-address invariant needs a chart-side follow-up shared with the SNI exposure work.Not in this PR, deliberately. The
gatewayunion member (a reserved name; the CRD ships without it and nothing references it). A certSAN write-back for the managed Kubernetes API. Discovery-engine address write-back. The address-preserving migration fromexternal: true. An authoritative exclusivity gate inside cozy-proxy (the attachment-side refusal is advisory); that wants an issue in cozystack/cozy-proxy. The community proposal text still describes thegatewaymember as the SNI trigger, which the API owners have since decided lives with the TLS work; that revision is a separate community PR.Screenshots
Not a UI change.
Downstream repositories
No trigger in the map matched this diff: the cozy-proxy runtime contract is consumed with the same three literal strings and nothing is renamed; the new CRD travels as opaque JSON in the terraform provider; no platform
values.yamlkey, variant, asset or node prerequisite changed. Two follow-ups are worth a human decision rather than a speculative PR: a website page for the new kind once the feature is released, and an issue in cozy-proxy for an authoritative one-delegation-per-pod gate. Left unticked for that reason.Release note