design-proposal: application endpoints and EndpointAttachment

[Timofei Larkin (lllamnyp)]

What this proposal is

Managed applications already have endpoints — the Services their operators create (Postgres -rw/-ro, a tenant cluster's API Service, Redis master) — and, with #35, addresses are becoming first-class resources that can be held, moved, and quota'd. Nothing connects the two: the only exposure mechanism is the chart-level external: true|false, which publishes one chart-chosen endpoint on one unchooseable address through five structurally different render paths.

This proposal adds EndpointAttachment (cozystack.io/v1alpha1, namespaced, tenant-created): attach an external address to one endpoint of one application — as many attachments per application as needed, each with an independent lifecycle, none of them ever mutating the application's own Services or values. An attachment either mints an IPAddressClaim from a class (ephemeral, dies with the attachment) or binds a pre-reserved claim (the elastic-IP hold/move experience — the address survives detachment). The controller is engine-agnostic: it renders one additive type: LoadBalancer Service mirroring the endpoint Service's selector and ports, and consumes the address through the substrate's claim annotation, so no per-chart render logic is needed. Attachments are garbage-collected with their application.

Relationship to prior proposals

• design-proposal: IP addresses as a first-class resource (IPAddress / IPAddressClaim / IPAddressClass) #35 (IP addresses as a first-class resource) is the allocation half; this is the exposure half. They meet at exactly one point: an attachment names or mints a claim and consumes it via the substrate's Service-annotation contract.
• design-proposal: structured, additive external exposure for managed applications #29 (structured external exposure, closed) established the goals — additive, multi-endpoint, per-listener exposure — but anchored them to chart-rendered Services, which its review showed impossible uniformly, and left the discovery-engine address write-back without an owner. Both points are answered structurally here: a controller that mirrors operator-owned Services, and an explicit deferred-engines section naming the future owner.
• cozystack#3218 (removal of ServiceExposure): the Alternatives considered section addresses why this is not that mistake repeated — N-per-app attachment lifecycle, claim hold/move semantics, and an authorization seam over which Services may be published do not reduce to a loadBalancerClass field on an existing Service.

external: true keeps its exact behavior; its sunset is explicitly gated on an address-preserving migration via the substrate's adoption path.

Out of scope

Hostnames/DNS/TLS/SANs (unified-TLS effort, #19), source-IP ACL (SecurityGroup, cozystack#2922), the discovery-engine write-back loop (Kafka / Mongo replica sets / NATS — deferred, stated plainly), Gateway API (a future union member the API shape leaves room for), and vm-instance whole-IP semantics.

Summary by CodeRabbit

• Documentation
  • Added a design proposal for exposing tenant application endpoints through externally reachable addresses.
  • Documents support for dynamically provisioned or pre-reserved IP addresses and additive load balancer services, without modifying application-owned services.
  • Defines address reporting, resource lifecycle and cleanup behavior, endpoint disappearance handling, migration considerations, security scenarios, and testing expectations.
  • Records open questions around port selection, dual-stack support, and dashboard presentation.

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c9b03c33-4333-4332-adeb-0da4ce279ce5

📝 Walkthrough

Walkthrough

Adds a design proposal for a namespaced EndpointAttachment resource that resolves tenant endpoint Services, manages IP address claims, creates additive LoadBalancer Services, reports status, and defines lifecycle, compatibility, validation, and rollout behavior.

Changes

EndpointAttachment design proposal

Layer / File(s)	Summary
Proposal scope and context design-proposals/endpoint-attachments/README.md	Documents the current external: true mechanism, proposal scope, goals, non-goals, and related work.
Resource and reconciliation design design-proposals/endpoint-attachments/README.md	Defines the EndpointAttachment spec and status, endpoint resolution, IP address claim handling, additive LoadBalancer Service rendering, status reporting, ownership, and garbage collection.
Compatibility and rollout details design-proposals/endpoint-attachments/README.md	Describes compatibility with existing exposure behavior, failure cases, validation and testing scenarios, rollout phases, open questions, and alternatives.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• cozystack/community#35: Defines the related IPAddressClass and IPAddressClaim resource model used by the proposal.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the proposal’s main topic: application endpoints and the new EndpointAttachment resource.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch design/endpoint-attachments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@design-proposals/endpoint-attachments/README.md`:
- Around line 78-80: Update the application identity used by authorization to
include the API group alongside kind and name, and ensure the resolver compares
all three fields. Apply this consistently to the applicationRef examples and
related authorization flow, or explicitly define and validate a single allowed
group; prefer binding to the application UID to prevent delete/recreate reuse.
- Around line 86-94: Resolve the unattainable Attached phase value in the status
definition by either documenting the lifecycle transition and finalizer behavior
that emits Detached, or removing Detached from the phase enum and its Pending |
Attached | Detached comment. Keep endpoint-loss behavior represented through
Resolved=False and resource deletion semantics consistent.
- Around line 104-105: Update the Render design to make the attachment UID in
the controller owner reference the authoritative identity for the generated
LoadBalancer Service. Use the ownership label only for indexing or diagnostics,
and require reconciliation to verify the Service’s controller owner reference
points to the attachment before adopting or updating it; otherwise ignore it and
create or rediscover the correctly owned Service.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: dd691aa7-3010-4ff4-b440-29580c3c0eca

📥 Commits

Reviewing files that changed from the base of the PR and between c484063 and cb00c12.

📒 Files selected for processing (1)

• design-proposals/endpoint-attachments/README.md

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Include the API group in the application identity used for authorization.

applicationRef contains only kind and name, and the resolver compares only those two fields, while the proposal says lineage labels also include .group. Unless a single application group is hard-coded and validated, same-kind/name resources from another group could satisfy the authorization check. Add the group to the reference—or explicitly constrain it—and compare all identity fields; binding to the application UID would also prevent reuse after delete/recreate.

Also applies to: 102-103, 159-162

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@design-proposals/endpoint-attachments/README.md` around lines 78 - 80, Update
the application identity used by authorization to include the API group
alongside kind and name, and ensure the resolver compares all three fields.
Apply this consistently to the applicationRef examples and related authorization
flow, or explicitly define and validate a single allowed group; prefer binding
to the application UID to prevent delete/recreate reuse.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Define or remove the Detached phase.

No lifecycle path emits Detached: deletion removes the resource, while endpoint loss leaves it present with Resolved=False. Specify the transition/finalizer semantics or remove this unreachable status value.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@design-proposals/endpoint-attachments/README.md` around lines 86 - 94,
Resolve the unattainable Attached phase value in the status definition by either
documenting the lifecycle transition and finalizer behavior that emits Detached,
or removing Detached from the phase enum and its Pending | Attached | Detached
comment. Keep endpoint-loss behavior represented through Resolved=False and
resource deletion semantics consistent.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Use the owner reference as the rendered-Service identity.

The design says the Service is owned by the attachment but rediscovered through an ownership label. Labels are mutable and can collide, so a stale or tenant-created Service could be selected, while label removal could cause duplicate LoadBalancer Services. Reconcile only a Service whose controller owner reference points to the attachment UID; use the label only as an index or diagnostic.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@design-proposals/endpoint-attachments/README.md` around lines 104 - 105,
Update the Render design to make the attachment UID in the controller owner
reference the authoritative identity for the generated LoadBalancer Service. Use
the ownership label only for indexing or diagnostics, and require reconciliation
to verify the Service’s controller owner reference points to the attachment
before adopting or updating it; otherwise ignore it and create or rediscover the
correctly owned Service.

[myasnikovdaniil]

Hi Timofei Larkin (@lllamnyp)!
I've read your design and find some issues,

Main blocker for now - this design not mention accepted database exposure design that may contradicts with yours at some places, take a look please.

Non-blocking follow-ups

1. Mutation semantics of spec.loadBalancer are undefined (line 78-83). spec.applicationRef is immutable via CEL and className/claimName are mutually exclusive, but nothing states what happens on className → a different class, claimName → a different claim, or a switch between the two members. Each of those silently changes the address a client may have allow-listed — the exact failure named in What exists today ("Turning external off and on may change the address a customer has allow-listed"). Either declare spec.loadBalancer immutable alongside applicationRef, or specify the re-mint and release semantics.

2. The API group choice is what buys the quota claim; say so. count/endpointattachments.cozystack.io (line 153) works because cozystack.io is CRD-served by the kube-apiserver (packages/system/cozystack-controller/definitions/cozystack.io_workloadmonitors.yaml), matching the existing count/certificates.cert-manager.io pattern documented at packages/core/platform/values.yaml:327. SecurityGroup — the resource this one is modelled beside — instead lives in sdn.cozystack.io, served by the aggregated cozystack-api (pkg/registry/sdn/securitygroup/rest.go, no CRD under definitions/), where the kube-apiserver's ResourceQuota admission never runs. A later move to sdn.cozystack.io for group consistency would silently drop stock quota support, so the coupling deserves a sentence.

3. Does any platform path delete and recreate an application's HelmRelease under the same name? Lifecycle and garbage collection hangs attachment GC on an ownerReference to the application's HelmRelease. Kubernetes GC matches owners by UID, so a same-name recreate orphans and collects every dependent — here, tearing down tenant exposure and releasing minted addresses. The platform already uses delete-and-let-the-chart-recreate for system releases (packages/core/platform/images/migrations/migrations/20, .../25:15-17); I could not find that pattern applied to a tenant application HelmRelease, so this is a question rather than a finding. If it is reachable, the design needs a finalizer or a name-based fallback.

4. spec.applicationRef should carry the group, per existing precedent. The precedent: SecurityGroup's ApplicationReference already resolves on all three of group, kind and name (pkg/registry/sdn/securitygroup/rest.go:67-69), and the proposal's own resolver text names only .kind and .name while the labels it reads include .group.

5. status.phase and status.conditions is duplicating info? how conditions list managed if it not have timestamps or some identification for list items?

[myasnikovdaniil]

You are identifying spec.endpoint.serviceName here in comment as struct but example shows plain string.

[myasnikovdaniil]

I think it is charts part to address (do not expose mixed services that serve data+ui or data+metrics), not this endpoint attachments responsibility.

[myasnikovdaniil]

Looks like this endpoint attachment resources is not application - it can't be created by itself without other app to attach. So it is first option or maybe define new category of resources, something like "applicationaddons"

[myasnikovdaniil]

You did not mentioned already accepted proposal for external exposure of databases. How do you think both designs will coexist? Accepted design address #29 as owner of exposure mechanics, and you superseded it, so now you need to resolve claims made to it. Also I see problem in idea: you making app-agnostic additive exposure, accepted design making app based exposure - certificates for tls sni based routing and encryption must be emitted by app itself, so it is not additive.

[myasnikovdaniil]

internal/lineagecontrollerwebhook/webhook.go:159-174 stamps apps.cozystack.io/application.{group,kind,name} on every object resolving to a managed HelmRelease. ApplicationDefinition.spec.services feeds a different label at lines 180-189 — internal.cozystack.io/tenantresource — and that is the tenant-facing marker (default-deny, matcher.go:65-73). Since the allowlists are strict subsets (kafka declares exactly one Service name, clickhouse one), the specified check would admit operator-internal and headless Services to a public address. Fix is a one-word change: gate on internal.cozystack.io/tenantresource: "true".

[myasnikovdaniil]

Here is the correct wording for the L102 labels problem

[myasnikovdaniil]

Forgot to request changes in previous review message. Doing now

[Andrei Kvapil (kvaps)]

The document is thorough, and the two structural choices are right: mirroring the endpoint Service's selector and ports is what makes the mechanism engine-agnostic without per-chart render logic, and the lineage-label check is a real authorization seam rather than a stated intention. Alternatives considered is honest about why this is not ServiceExposure repeated, which was the obvious objection.

myasnikovdaniil's blocker is correct, and I want to sharpen it, because it is more than a missing cross-reference. Then a concrete requirement from our side that I think changes the scope.

1. The accepted design this collides with is community #20, and it is merged

design-proposals/external-database-exposure opens with the problem statement: "Today every managed database a tenant exposes externally gets its own LoadBalancer Service, and therefore its own public IP." Its goal is to collapse a tenant's databases — including several instances of the same engine — onto the single LoadBalancer IP of the tenant Gateway, routed by SNI.

EndpointAttachment renders one additive LoadBalancer Service per attachment, each drawing its own address. That is the pattern #20 was accepted to replace. Scope and related proposals lists #35, #29, #3218, #2922, unified-TLS and Gateway API — and not #20.

I do not think these conflict as designs: SNI consolidation for the common case, a dedicated address for the tenant whose client allow-lists an IP or whose driver cannot do SNI. But the document has to say so, because as written it reads as the general answer to external exposure, and the general answer was decided differently.

2. This proposal inherited an obligation from #29 and does not know it

#20's User-facing changes says its trigger is "an external-adjacent toggle to select passthrough/SNI mode (expected to become an expose entry once the structured-exposure model lands)". #20 was written expecting #29 to be its user-facing surface. This proposal declares itself #29's successor — so it is now that surface.

The practical consequence today: a tenant cannot request SNI exposure at all. #29 is closed, and the Gateway union member here is "future work, not designed here". The merged design has no trigger. That reframes the union member: it is not speculative, it is specified elsewhere and waiting.

3. The hostname non-goal points past the accepted answer

Non-goals sends hostnames, DNS and certificates to the unified-TLS effort. For databases, #20 already settled the SNI hostname scheme and the end-to-end TLS story. Deferring to unified-TLS is right for the PKI half and misses that the naming half for this exact case is decided.

4. A question about the mirroring contract

Failure and edge cases says a changed endpoint selector is "mirrored on the next reconcile". That is safe where an operator expresses instance role as a pod label and keeps the Service selector static — CloudNativePG's -rw works this way, so the flagship case is fine. It is not safe for an operator that rewrites the Service selector on failover: for the reconcile window, external write traffic would land on a demoted primary. Worth stating the assumption and which engines were checked against it.

5. VMs are a second first-class consumer, not future work

Non-goals defers vm-instance ("the existing externalMethod contract stays as-is; folding it into the union is future work"). We have a live requirement that does not fit that deferral: attaching one or more public IPs to a VM.

That requirement is exactly this proposal's N-per-application shape, and more of the machinery already fits than the document assumes. On a live cluster, a vm-instance Service already carries everything the resolve step needs:

labels:
  apps.cozystack.io/application.group: apps.cozystack.io
  apps.cozystack.io/application.kind: VMInstance
  apps.cozystack.io/application.name: whmcs
  apps.cozystack.io/user-service: "true"
  service.kubernetes.io/service-proxy-name: cozy-proxy

The lineage labels are stamped, and the selector is a normal pod selector that mirrors fine. Three things block it:

• Ports must be in the attachment spec. A VM's non-external Service is headless with a single sentinel port 65535, so there is nothing meaningful to mirror, and externalPorts has no counterpart. This answers your open question 1 — a ports filter is required, not optional.
• A whole-IP mode. The rendered Service needs the service.kubernetes.io/service-proxy-name: cozy-proxy label and the networking.cozystack.io/wholeIP / allowICMP annotations. The datapath already exists; only the render is missing.
• Egress with several addresses is a real design question. WholeIP today means both "all ports in" and "the VM egresses as this address". With two attachments that is ambiguous. Either one attachment is marked primary for egress, or egress stays on the application's own Service, or wholeIP is permitted on at most one attachment. This is the one part that is not a field.

Related, the deprecation sequence has an internal tension: step 3 removes external from chart schemas "with a validation error pointing at attachments", while Non-goals says vm-instance's contract stays as-is. For VMs there is currently nothing to point at.

6. A granularity mismatch worth settling before implementation

We are mapping a portal-side model onto this: a standalone PublicIP resource (a clean fit for #35's IPAddressClaim) attached through an instance's NIC list, where each NIC carries its own security groups. So one VM can have a management address with SSH open and a public address with only HTTPS.

Cozystack's SecurityGroup.spec.attachments is a list of ApplicationReference, and the controller stamps a membership label onto the pods of the referenced application (pkg/apis/sdn/v1alpha1/securitygroup_types.go). The granularity is per-application, so two addresses on the same VM necessarily share one rule set.

Per-attachment ports covers part of that in practice, but it is port filtering rather than source-IP ACL. Since Non-goals hands ACL to #2922 wholesale, it is worth saying explicitly whether per-address ACL is intended to become expressible, or whether consumers should expect application-level granularity permanently.

---

Points 1–3 are a few lines each. Point 5 is the one that changes scope, and I would rather raise it now than after the MVP ships database-only.

Separately: your five non-blocking points from 2026-08-05 have had no response and the PR has not moved in three and a half weeks. Points 1 (mutation semantics of spec.loadBalancer) and 3 (same-name HelmRelease recreate orphaning attachments by UID) look load-bearing to me, not nits.

Not adding a second CHANGES_REQUESTED on top of myasnikovdaniil's — one block is enough.