Conversation
scotwells
left a comment
There was a problem hiding this comment.
Can we use YAML manifests to express the api design instead of bulleted list of fields? That helps make the api design more clear.
| - `spec.resources[]`: list of references to ConfigMaps/Secrets that contain YAML/JSON | ||
| manifests (multiple keys; multiple YAML documents per key supported). |
There was a problem hiding this comment.
Have you considered embedding the resource directly here? Loading raw yaml through ConfigMaps / Secrets will be error prone.
| - **Resource conflicts** when multiple sets try to manage the same object: | ||
| - Mitigation: define deterministic ordering or explicit priority; ensure conflicts | ||
| are surfaced in binding status. |
There was a problem hiding this comment.
This mentions we're mitigating the issue through deterministic ordering / explicit priority but I don't see that represented in the proposed API design.
| - **Consistency and policy**: Organizations often want consistent defaults across | ||
| all Projects they own. |
There was a problem hiding this comment.
The wording here makes it sound like this resource is something that consumers would have access to in order to manage default resources created in their project. I'd expect this concept to be something that service providers are responsible for managing.
| - **Extensibility**: new services should not require editing the Project controller | ||
| to install their defaults into every Project. |
There was a problem hiding this comment.
Seems like this is restating the same thing the "separation of concerns" bullet point is stating.
| - Provide an API surface that supports Organization-scoped policy (e.g. different | ||
| defaults per Organization) without requiring per-service code changes. |
There was a problem hiding this comment.
What's the use-case for needing different default resources to be created per organization?
| 2. A controller in the Milo management plane matches Projects using selectors and | ||
| applies the manifests into each Project control plane using the existing per-Project | ||
| API routing (`/projects/<id>/control-plane`). |
There was a problem hiding this comment.
We should use the fully qualified path /apis/resourcemanager.miloapis.com/v1alpha1/projects/<project-id>/control-plane.
| - **Accidental wide blast radius** (e.g. selector matches too many Projects): | ||
| - Mitigation: require non-empty selectors; optionally gate with feature flags and | ||
| RBAC so only privileged actors can create/modify sets. | ||
| - Mitigation: record all applications in `ProjectResourceSetBinding` for audit and | ||
| rollback planning. |
There was a problem hiding this comment.
I'm not understanding the risk / mitigation here. What would the typical use-case look like if we required non-empty selectors? I'd think in almost all cases, a resource set will apply to all projects, not a subset.
| - Create dynamic/discovery clients | ||
| - Apply manifests into that Project control plane | ||
|
|
||
| The controller should gate application on the Project being Ready. |
There was a problem hiding this comment.
I originally interpreted "application" as a software application. Seems like this is more about application of resources.
Might be better worded as "The controller must wait until the project is ready before applying resources configured in the resource set".
| ### Selection model: labels vs Organization spec vs Project spec | ||
|
|
||
| **Recommendation (initial / alpha): selection via labels (no Project/Organization API changes).** | ||
|
|
||
| Rationale: | ||
|
|
||
| - Projects already have an immutable organization label set by the Project admission webhook | ||
| (`resourcemanager.miloapis.com/organization-name`). | ||
| - Label selectors are the simplest, most Kubernetes-native way to express “apply to all | ||
| Projects in org X” or “apply to Projects matching capability Y”. | ||
| - This keeps the initial footprint small and avoids immediate API migrations. | ||
|
|
||
| **How Organization-specific defaults work with labels:** | ||
|
|
||
| - Create a `ProjectResourceSet` per Organization (or per Organization class), with | ||
| `spec.projectSelector.matchLabels` including the organization label. |
There was a problem hiding this comment.
The heading of this section is oddly worded. "Labels vs Organization Spec vs Project Spec" makes it sound like there's going to be a comparison of various approaches.
Label selectors are the simplest, most Kubernetes-native way to express “apply to all Projects in org X” or “apply to Projects matching capability Y”.
CEL expressions are also becoming increasingly common as a filtering method for resources in Kubernetes. The wording "Projects matching capability Y" would require us to surface capabilities through project labels, but not sure what capabilities are in this context.
Create a
ProjectResourceSetper Organization (or per Organization class), withspec.projectSelector.matchLabelsincluding the organization label.
What is "Organization class" referencing? That's not a concept that exists today.
| - [ ] Feature gate | ||
| - Feature gate name: `ProjectResourceSets` | ||
| - Components depending on the feature gate: milo-controller-manager (PRS controller) |
There was a problem hiding this comment.
Why does this need to be feature gated?
Adds an enhancement/design doc proposing
ProjectResourceSet/ProjectResourceSetBinding(inspired by Cluster API’s ClusterResourceSet) to declaratively apply Kubernetes manifests into each Project’s virtual control plane. The initial target is replacing the Project controller’s hard-coded default installers forGatewayClassandDNSZoneClass, with org-scoped policy via label selectors and clear status/traceability via bindings.