Adding design for automatically registering core RRTs#11610
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.
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
kachawla
force-pushed
the
rrt-auto-registration
branch
from
369e713 to
9262bfb
Compare
| 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
|
|
||
| #### 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.
There was a problem hiding this comment.
I think we would include recipes corresponding to each type in this file as well. What do you think @nithyatsu?
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).
There was a problem hiding this comment.
that's a good point to use CI for flagging changes. I'll add that. Thanks for reviewing
| - **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. |
kachawla
force-pushed
the
rrt-auto-registration
branch
2 times, most recently
from
ce6d3d4 to
133e092
Compare
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |
kachawla
force-pushed
the
rrt-auto-registration
branch
from
133e092 to
8044493
Compare
Signed-off-by: Karishma Chawla <kachawla@microsoft.com>
Signed-off-by: Karishma Chawla <kachawla@microsoft.com>
Signed-off-by: Karishma Chawla <kachawla@microsoft.com>
kachawla
force-pushed
the
rrt-auto-registration
branch
from
8044493 to
c834f40
Compare
Description
Certain resource types defined in the resource-types-contrib repo are expected to be registered with Radius by default for every installation. The way we enable it today is by manually copying over the manifests into Radius repo and maintaining duplicate files. This design automates propagating the changes to the Radius repo.
Type of change
Fixes: #11108
Contributor checklist
Please verify that the PR meets the following requirements, where applicable: