Adding design for automatically registering core RRTs#11610
Adding design for automatically registering core RRTs#11610kachawla wants to merge 1 commit intoradius-project:mainfrom
Conversation
kachawla
force-pushed
the
rrt-auto-registration
branch
from
f165ae2 to
4177f2d
Compare
There was a problem hiding this comment.
Pull request overview
Adds a design note proposing automated default registration of Radius resource types by embedding selected manifests from resource-types-contrib into the Radius binary and registering them during UCP initialization.
Changes:
- Introduces a design for using
resource-types-contribas a Go module dependency and embedding default manifests viago:embed. - Proposes a centralized
defaults.yaml+go generateworkflow to control which manifests ship as defaults. - Describes initializer/runtime behavior, error handling, and a test/CI validation plan for generated embed lists.
| # Automated Default Registration of Resource Types from resource-types-contrib | ||
|
|
||
| * **Author**: Karishma Chawla (@kachawla) | ||
|
|
||
| ## Overview |
There was a problem hiding this comment.
PR description still contains placeholder Fixes: #issue_number and the Type of change / checklist items aren’t selected. Please update the PR description to link the approved issue/design-doc PR (or mark items N/A) so reviewers can validate scope and tracking.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11610 +/- ##
=======================================
Coverage 51.38% 51.38%
=======================================
Files 699 699
Lines 44114 44114
=======================================
Hits 22666 22666
Misses 19279 19279
Partials 2169 2169 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
kachawla
force-pushed
the
rrt-auto-registration
branch
2 times, most recently
from
06da8d4 to
369e713
Compare
Signed-off-by: Karishma Chawla <kachawla@microsoft.com>
kachawla
force-pushed
the
rrt-auto-registration
branch
from
369e713 to
9262bfb
Compare
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |
| ```yaml | ||
| defaultRegistration: | ||
| - Networking/loadBalancers/loadBalancers.yaml | ||
| ``` |
There was a problem hiding this comment.
Is the defaults.yaml in radius repo?
There was a problem hiding this comment.
in resource-types-contrib
| 2. **Defaults key format**: Should `defaults.yaml` entries be file paths (e.g., `Compute/containers/containers.yaml`) or logical resource type names (e.g., `Radius.Compute/containers`)? | ||
|
|
||
| - **Option A: File paths (proposed).** Directly resolvable by `go:embed` and `fs.ReadFile` with no lookup step. Breakage on renames is mitigated by `go generate` failing immediately on missing files, making stale paths easy to catch. This is simpler to implement and aligns with how `go:embed` patterns work. | ||
| - **Option B: Logical resource type names.** Uses the canonical `<namespace>/<typeName>` format (e.g., `Radius.Compute/containers`) which is stable across file renames and consistent with how resource types are referenced elsewhere in Radius (CLI, API, logs). The `go generate` script resolves names to file paths by scanning the directory tree for matching `namespace` and type entries. This adds generator complexity and couples the generator to the manifest schema format. |
There was a problem hiding this comment.
I'm actually leaning towards this as preferred when combined with a version. I'd expect like Radius.Compute/containers@v1 and a file path would look weird.
There was a problem hiding this comment.
I agree, for now I wanted to keep this simple because I believe @Reshrahim is planning to revisit how we surface different stages or resource types. If we think this file is going to be the longer term approach then option B is definitely much better
There was a problem hiding this comment.
I like Option B as well because I can totally see someone moving a .yaml manifest file but forgetting to update the file path in defaults.yaml
There was a problem hiding this comment.
going with option B based on the discussion
| Remaining files ( `radius_core.yaml`, `microsoft_resources.yaml`) stay because they are not included in resource-types-contrib. | ||
|
|
||
| ### Error Handling | ||
|
|
There was a problem hiding this comment.
is there any risk of issues with upgrades? e.g. adding a new resource that's only available in the new version of radius?
There was a problem hiding this comment.
We have added new resource types in new releases in the past. Have we run into any issues with it so far?
There was a problem hiding this comment.
discussed that this is not expected to happen but we will make sure that proper error handling is done and a test is added for this scenario
|
|
||
| **Manifest YAML files** remain unchanged: no `location` field, no `defaultRegistration` field. They contain only `namespace` and `types`. | ||
|
|
||
| #### UCP |
There was a problem hiding this comment.
Quick question I recently refactored all the type registration to use internal methods not http calls and just want to validate this persists the same pattern
There was a problem hiding this comment.
yeah thanks for the reminder. I started this design before your changes, I will review your changes and update this accordingly
|
|
||
| #### Platform engineer updates a resource type schema | ||
|
|
||
| A platform engineer updates the schema for `Radius.Compute/containers` in `resource-types-contrib`. The change flows to Radius when a maintainer bumps the dependency by running `go get -u github.com/radius-project/resource-types-contrib` and merging the `go.mod` change. No file copying or sync scripts are needed. |
There was a problem hiding this comment.
The change flows to Radius when a maintainer bumps the dependency by running
go get -u github.com/radius-project/resource-types-contriband merging thego.modchange.
Is the expectation that this gets done at each release? Would it be feasible to build this into a workflow so that it's automated?
There was a problem hiding this comment.
I'm hoping dependabot would take care of upgrades here once we add tagged releases to resource-types-contrib, which I'm taking on as a next step after initial implementation.
There was a problem hiding this comment.
action item to update release process to include this step till we implement tagged releases
|
|
||
| ### High Level Design | ||
|
|
||
| The design introduces `resource-types-contrib` as a Go module dependency of `radius`. Resource type manifests are embedded into the Radius binary using Go's `embed.FS` mechanism. A central `defaults.yaml` file in `resource-types-contrib` lists which manifests should be embedded and registered by default. |
There was a problem hiding this comment.
Naming nit: default-types.yaml might be more appropriate if the design is to only store default type definitions in the file.
However, if additional default items (e.g. default recipe packs @nithyatsu 👀) may be stored in the same config file, then defaults.yaml makes sense.
brooke-hamilton
left a comment
brooke-hamilton
left a comment
There was a problem hiding this comment.
🚀 I really like how this proposal sets up resource-types-contrib as a clear dependency of Radius via a Golang module dependecy.
|
|
||
| ### 3. Tagged releases and automated dependency updates for `resource-types-contrib` | ||
|
|
||
| `resource-types-contrib` does not have a formal release or tagging process today. Without tagged releases, Radius depends on Go pseudo-versions (e.g., `v0.0.0-20260408153021-abc123def456`), and dependency updates require a maintainer to manually run `go get -u`. This limits automation and makes it harder to track what changed between versions. |
There was a problem hiding this comment.
+1 on creating GitHub releases for the generated go code. Having releases would allow for idiomatic consumption of the releases in the Radius repo, and trigger dependabot updates when a new version is available.
If there is a need for more granular releases for specific resource types, then each resource type could be separated into its own go module and released separately. I don't think this would be required, but it's an option if we find ourselves updating the individual types in different cycles.
nellshamrell
left a comment
nellshamrell
left a comment
There was a problem hiding this comment.
Excellent work, @kachawla!
|
|
||
| Today, resource type manifests for default registration in Radius are manually duplicated from the `resource-types-contrib` repository into the `radius` repository under `deploy/manifest/built-in-providers/`. This creates a maintenance burden - when a resource type schema is updated in `resource-types-contrib`, the corresponding file in `radius` must be manually updated, leading to schema drift, stale definitions, and duplicated effort. | ||
|
|
||
| This design introduces a mechanism to automatically embed resource type manifests from `resource-types-contrib` as a Go module dependency of `radius`. A central configuration file (`defaults.yaml`) in `resource-types-contrib` declares which resource types should be default-registered. At build time, only those manifests are embedded into the Radius binary via `go:embed`. At startup, the UCP initializer reads the embedded manifests and registers them alongside any existing directory-based manifests. |
There was a problem hiding this comment.
I like this - it lets us define recipes that require extra scrutiny when reviewing (we can also use CI to flag when recipes on the defaults.yml are changed and require additional review). This makes the threat of malicious contributions easier to manage (malicious contributions are a risk in any OSS project).
| - **Runtime fetching of manifests**: Manifests are embedded at build time, not downloaded at runtime. This avoids network dependencies during startup. | ||
| - **Migrating non-dynamic-rp providers**: Resource types served by `applications-rp` or the deployment engine (e.g., `Applications.Core`, `Microsoft.Resources`) require explicit `location` addresses and remain as directory-based manifests in `radius`. Migrating them is out of scope. | ||
| - **Recipe registration**: This design covers resource type schema registration only, not recipe registration or recipe pack management. | ||
| - **Release process for `resource-types-contrib`**: This design assumes a Radius maintainer manually bumps the `resource-types-contrib` dependency in `go.mod` to pick up changes. Establishing a formal release/tagging process for `resource-types-contrib` is out of scope. |
There was a problem hiding this comment.
This was going to be my next question - whether there is a release/tagging process which would prevent a bug that was accidentally merged into a default resource type from manifesting in Radius. I agree it is out of scope for this proposal, but would be good to consider in the future.
| - Networking/loadBalancers/loadBalancers.yaml | ||
| ``` | ||
| 3. They run `go generate` and commit `defaults.yaml` along with the auto-generated `manifests_gen.go` (which contains the `//go:embed` directives that tell the Go compiler which files to embed in the binary). | ||
| 4. A Radius maintainer manually bumps the dependency by running `go get -u github.com/radius-project/resource-types-contrib` in the `radius` repository and merging the resulting `go.mod` change. Since `resource-types-contrib` does not have tagged releases today, Go resolves a pseudo-version based on the latest commit (e.g., `v0.0.0-20260408153021-abc123def456`). |
There was a problem hiding this comment.
ah - excellent use of the commit hash! This should not be used as a long-term solution as I believe git by default uses SHA-1 for its hashes, which is not very strong and can be broken. More to consider when we think about tagged releases in the future, but not a blocker to this proposal.
| - **Discoverability**: A single file shows all defaults at a glance. | ||
| - **Reviewability**: PR diffs for `defaults.yaml` clearly show what's being added or removed. | ||
| - **No parser coupling**: `resource-types-contrib` metadata stays out of the Radius manifest parser. | ||
| - **Extensible**: Works for any directory structure; new top-level directories (e.g., `Networking/`) work without changing Go code. |
|
|
||
| ## Security | ||
|
|
||
| No changes to the security model. The embedded manifests are static YAML files compiled into the binary at build time, so there is no new attack surface for injection or tampering beyond what exists for any compiled-in resource. The `defaults.yaml` file is validated at startup, and invalid entries cause a clear startup failure. |
There was a problem hiding this comment.
Agreed - to me this adds no more significant risks than manually copying and pasting resource types does. Like all OSS projects, we need to consider ways to do more extensive security filtering/testing of pull requests, but this design does not add additional risk.
|
|
||
| 1. **`go generate` enforcement**: Should `resource-types-contrib` CI block merges if `manifests_gen.go` is out of date, or should CI auto-regenerate and commit? | ||
|
|
||
| - **Option A: CI blocks merges (proposed).** CI runs `go generate` and `git diff --exit-code manifests_gen.go`. If the file is stale, the PR fails. Contributors must run `go generate` locally before pushing. This keeps generated files explicitly reviewed in PRs and avoids hidden auto-commits. |
Description
Please explain the changes you've made.
Type of change
Fixes: #issue_number
Contributor checklist
Please verify that the PR meets the following requirements, where applicable: