Skip to content

Fix resource types missing from rad resource-type list with per-type manifest files#11933

Merged
kachawla merged 8 commits into
mainfrom
kachawla/fix-namespace-merge-upstream
May 19, 2026
Merged

Fix resource types missing from rad resource-type list with per-type manifest files#11933
kachawla merged 8 commits into
mainfrom
kachawla/fix-namespace-merge-upstream

Conversation

@kachawla
Copy link
Copy Markdown
Member

@kachawla kachawla commented May 18, 2026

Background

PR #11911 introduced automated synchronization of default resource type manifests from resource-types-contrib. As part of that change, the manifest file layout in deploy/manifest/built-in-providers/ changed from one file per namespace (e.g., radius_compute.yaml containing containers, routes, and persistentVolumes) to one file per type (e.g., containers.yaml, routes.yaml, persistentVolumes.yaml). This matches the per-type layout in resource-types-contrib and keeps diffs scoped to the type that changed.

However, the initializer's registerResourceProviderDirect function was not designed for multiple files sharing the same namespace. It saves the Location and ResourceProviderSummary per namespace on each file, and each save was a full overwrite rather than a merge.

Problem

When multiple manifest files share the same namespace (e.g., containers.yaml and persistentVolumes.yaml both under Radius.Compute), the initializer was overwriting the Location and ResourceProviderSummary on each file, causing earlier types to disappear from rad resource-type list. Only the last file's types (alphabetically) were visible.

The individual ResourceType records were saved correctly (one per type), but the Location and Summary - which UCP's routing layer and rad resource-type list read from - only contained the last file's types.

Fix

registerResourceProviderDirect now reads the existing Location and Summary from the database before saving, merging in new types alongside existing ones. If no existing record is found (first file for this namespace), it behaves as before. Only ErrNotFound is treated as "no existing record"; other errors (database outage, decode failure) cause the initializer to fail fast.

The fix also preserves the existing location address when a later manifest for the same namespace does not specify one.

Changes

  • pkg/ucp/initializer/service.go - Read-merge-write for Location and ResourceProviderSummary, with ErrNotFound-only error handling and address preservation
  • pkg/ucp/integrationtests/resourceproviders/resourceproviders_test.go - Updated Test_ResourceProvider_RegisterManifests_NoLocation to verify both types from two files sharing a namespace are registered and present in the location's resourceTypes map. Added Test_ResourceProvider_DefaultsRegistered that reads defaults.yaml, loads real manifests, and verifies all types are registered with correct location entries.
  • testdata/manifests-no-location/persistentVolumes.yaml - Second test manifest sharing the Radius.Compute namespace
  • test/functional-portable/dynamicrp/noncloud/resources/default_containers_test.go - End-to-end functional test deploying Radius.Compute/containers and Radius.Compute/routes (two types from the same namespace) using the default recipes
  • test/functional-portable/dynamicrp/noncloud/resources/testdata/default-containers-test.bicep - Bicep template for the e2e test

Testing

  • Integration test (Test_ResourceProvider_RegisterManifests_NoLocation): Two test manifests sharing Radius.Compute namespace, verifies both types appear in the location's resourceTypes map.
  • Integration test (Test_ResourceProvider_DefaultsRegistered): Reads defaults.yaml, loads manifests from built-in-providers/dev/, verifies all types are registered with correct location entries.
  • Functional test (Test_DefaultContainers_Deploy): End-to-end deployment of Radius.Compute/containers and Radius.Compute/routes using default recipes.

…manifest files

When multiple manifest files share the same namespace, the initializer
now merges types into the existing Location and ResourceProviderSummary
instead of overwriting them. Also preserves existing location address
when a later manifest does not specify one.

Adds integration tests verifying namespace merge behavior with real
manifests and a functional test deploying Radius.Compute/containers
and Radius.Compute/routes end-to-end.

Signed-off-by: Karishma Chawla <kachawla@microsoft.com>
Copilot AI review requested due to automatic review settings May 18, 2026 22:27
@kachawla kachawla requested review from a team as code owners May 18, 2026 22:27
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 18, 2026

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a regression in UCP manifest initialization where per-type manifest files that share a namespace (e.g., Radius.Compute) would overwrite previously-registered Location and ResourceProviderSummary entries, causing earlier types to disappear from rad resource-type list.

Changes:

  • Update registerResourceProviderDirect to read existing Location/Summary records and merge in newly discovered resource types (and preserve location address when omitted).
  • Expand UCP integration coverage to validate namespace-level merging and ensure all defaults.yaml types are registered from real built-in manifests.
  • Add a portable functional test that deploys two default Radius.Compute types end-to-end using the default recipe pack.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/ucp/initializer/service.go Implements read-merge-write for Location and ResourceProviderSummary during manifest registration.
pkg/ucp/integrationtests/resourceproviders/resourceproviders_test.go Adds/updates integration tests to verify merged registration across multiple files per namespace and validate defaults registration.
pkg/ucp/integrationtests/resourceproviders/testdata/manifests-no-location/persistentVolumes.yaml Adds a second no-location manifest sharing Radius.Compute namespace for merge regression coverage.
test/functional-portable/dynamicrp/noncloud/resources/default_containers_test.go Adds an e2e functional test deploying default containers and routes types from the same namespace.
test/functional-portable/dynamicrp/noncloud/resources/testdata/default-containers-test.bicep Bicep template used by the new e2e functional test.

Comment thread pkg/ucp/initializer/service.go Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 18, 2026

Unit Tests

    2 files  ±0    423 suites  ±0   7m 26s ⏱️ +3s
5 140 tests +3  5 138 ✅ +3  2 💤 ±0  0 ❌ ±0 
6 178 runs  +3  6 176 ✅ +3  2 💤 ±0  0 ❌ ±0 

Results for commit f001dc9. ± Comparison against base commit 9017ff2.

♻️ This comment has been updated with latest results.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

❌ Patch coverage is 77.77778% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.71%. Comparing base (9017ff2) to head (f001dc9).

Files with missing lines Patch % Lines
pkg/ucp/initializer/service.go 77.77% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11933      +/-   ##
==========================================
+ Coverage   51.69%   51.71%   +0.01%     
==========================================
  Files         724      724              
  Lines       45508    45522      +14     
==========================================
+ Hits        23525    23541      +16     
+ Misses      19763    19762       -1     
+ Partials     2220     2219       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

kachawla added 4 commits May 18, 2026 15:51
Instead of read-merge-write on Location and Summary per file, merge all
manifest files by namespace in memory first, then register once per
namespace. This makes the on-disk manifest directory the source of truth
and avoids accumulating stale types from previous database state.

registerResourceProviderDirect is reverted to the simple overwrite
behavior since it now receives a single merged provider per namespace
with all types already combined.

Signed-off-by: Karishma Chawla <kachawla@microsoft.com>
The Radius.Compute/containers recipe resolves the application via
Applications.Core, which fails when the app is Radius.Core/applications
referencing an Applications.Core/environments. Fix by creating a
self-contained Radius.Core/environments with its own K8s namespace.

Signed-off-by: Karishma Chawla <kachawla@microsoft.com>
Signed-off-by: Karishma Chawla <kachawla@microsoft.com>
Comment thread pkg/ucp/initializer/service.go
Comment thread pkg/ucp/initializer/service.go Outdated
kachawla added 2 commits May 18, 2026 17:32
Signed-off-by: Karishma Chawla <kachawla@microsoft.com>
Signed-off-by: Karishma Chawla <kachawla@microsoft.com>
Signed-off-by: Karishma Chawla <kachawla@microsoft.com>
@radius-functional-tests
Copy link
Copy Markdown

radius-functional-tests Bot commented May 19, 2026

Radius functional test overview

🔍 Go to test action run

Click here to see the test run details
Name Value
Repository radius-project/radius
Commit ref f001dc9
Unique ID funcd46c194100
Image tag pr-funcd46c194100
  • gotestsum 1.13.0
  • KinD: v0.29.0
  • Dapr: 1.14.4
  • Azure KeyVault CSI driver: 1.4.2
  • Azure Workload identity webhook: 1.3.0
  • Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:pr-funcd46c194100
  • Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
  • applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-funcd46c194100
  • dynamic-rp test image location: ghcr.io/radius-project/dev/dynamic-rp:pr-funcd46c194100
  • controller test image location: ghcr.io/radius-project/dev/controller:pr-funcd46c194100
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-funcd46c194100
  • deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting corerp-cloud functional tests...
⌛ Starting ucp-cloud functional tests...
✅ ucp-cloud functional tests succeeded
✅ corerp-cloud functional tests succeeded

@kachawla kachawla merged commit 41193c2 into main May 19, 2026
59 checks passed
@kachawla kachawla deleted the kachawla/fix-namespace-merge-upstream branch May 19, 2026 16:43
zachcasper pushed a commit to zachcasper/radius that referenced this pull request May 19, 2026
…manifest files (radius-project#11933)

## Background

PR radius-project#11911 introduced automated synchronization of default resource type
manifests from `resource-types-contrib`. As part of that change, the
manifest file layout in `deploy/manifest/built-in-providers/` changed
from one file per namespace (e.g., `radius_compute.yaml` containing
`containers`, `routes`, and `persistentVolumes`) to one file per type
(e.g., `containers.yaml`, `routes.yaml`, `persistentVolumes.yaml`). This
matches the per-type layout in `resource-types-contrib` and keeps diffs
scoped to the type that changed.

However, the initializer's `registerResourceProviderDirect` function was
not designed for multiple files sharing the same namespace. It saves the
Location and ResourceProviderSummary per namespace on each file, and
each save was a full overwrite rather than a merge.

## Problem

When multiple manifest files share the same namespace (e.g.,
`containers.yaml` and `persistentVolumes.yaml` both under
`Radius.Compute`), the initializer was overwriting the Location and
ResourceProviderSummary on each file, causing earlier types to disappear
from `rad resource-type list`. Only the last file's types
(alphabetically) were visible.

The individual ResourceType records were saved correctly (one per type),
but the Location and Summary - which UCP's routing layer and `rad
resource-type list` read from - only contained the last file's types.

## Fix

`registerResourceProviderDirect` now reads the existing Location and
Summary from the database before saving, merging in new types alongside
existing ones. If no existing record is found (first file for this
namespace), it behaves as before. Only `ErrNotFound` is treated as "no
existing record"; other errors (database outage, decode failure) cause
the initializer to fail fast.

The fix also preserves the existing location address when a later
manifest for the same namespace does not specify one.

## Changes

- `pkg/ucp/initializer/service.go` - Read-merge-write for Location and
ResourceProviderSummary, with ErrNotFound-only error handling and
address preservation
- `pkg/ucp/integrationtests/resourceproviders/resourceproviders_test.go`
- Updated `Test_ResourceProvider_RegisterManifests_NoLocation` to verify
both types from two files sharing a namespace are registered and present
in the location's resourceTypes map. Added
`Test_ResourceProvider_DefaultsRegistered` that reads `defaults.yaml`,
loads real manifests, and verifies all types are registered with correct
location entries.
- `testdata/manifests-no-location/persistentVolumes.yaml` - Second test
manifest sharing the `Radius.Compute` namespace
-
`test/functional-portable/dynamicrp/noncloud/resources/default_containers_test.go`
- End-to-end functional test deploying `Radius.Compute/containers` and
`Radius.Compute/routes` (two types from the same namespace) using the
default recipes
-
`test/functional-portable/dynamicrp/noncloud/resources/testdata/default-containers-test.bicep`
- Bicep template for the e2e test

## Testing

- **Integration test**
(`Test_ResourceProvider_RegisterManifests_NoLocation`): Two test
manifests sharing `Radius.Compute` namespace, verifies both types appear
in the location's resourceTypes map.
- **Integration test** (`Test_ResourceProvider_DefaultsRegistered`):
Reads `defaults.yaml`, loads manifests from `built-in-providers/dev/`,
verifies all types are registered with correct location entries.
- **Functional test** (`Test_DefaultContainers_Deploy`): End-to-end
deployment of `Radius.Compute/containers` and `Radius.Compute/routes`
using default recipes.

---------

Signed-off-by: Karishma Chawla <kachawla@microsoft.com>
Signed-off-by: Zach Casper <zachcasper@microsoft.com>
zachcasper pushed a commit to zachcasper/radius that referenced this pull request May 19, 2026
…manifest files (radius-project#11933)

## Background

PR radius-project#11911 introduced automated synchronization of default resource type
manifests from `resource-types-contrib`. As part of that change, the
manifest file layout in `deploy/manifest/built-in-providers/` changed
from one file per namespace (e.g., `radius_compute.yaml` containing
`containers`, `routes`, and `persistentVolumes`) to one file per type
(e.g., `containers.yaml`, `routes.yaml`, `persistentVolumes.yaml`). This
matches the per-type layout in `resource-types-contrib` and keeps diffs
scoped to the type that changed.

However, the initializer's `registerResourceProviderDirect` function was
not designed for multiple files sharing the same namespace. It saves the
Location and ResourceProviderSummary per namespace on each file, and
each save was a full overwrite rather than a merge.

## Problem

When multiple manifest files share the same namespace (e.g.,
`containers.yaml` and `persistentVolumes.yaml` both under
`Radius.Compute`), the initializer was overwriting the Location and
ResourceProviderSummary on each file, causing earlier types to disappear
from `rad resource-type list`. Only the last file's types
(alphabetically) were visible.

The individual ResourceType records were saved correctly (one per type),
but the Location and Summary - which UCP's routing layer and `rad
resource-type list` read from - only contained the last file's types.

## Fix

`registerResourceProviderDirect` now reads the existing Location and
Summary from the database before saving, merging in new types alongside
existing ones. If no existing record is found (first file for this
namespace), it behaves as before. Only `ErrNotFound` is treated as "no
existing record"; other errors (database outage, decode failure) cause
the initializer to fail fast.

The fix also preserves the existing location address when a later
manifest for the same namespace does not specify one.

## Changes

- `pkg/ucp/initializer/service.go` - Read-merge-write for Location and
ResourceProviderSummary, with ErrNotFound-only error handling and
address preservation
- `pkg/ucp/integrationtests/resourceproviders/resourceproviders_test.go`
- Updated `Test_ResourceProvider_RegisterManifests_NoLocation` to verify
both types from two files sharing a namespace are registered and present
in the location's resourceTypes map. Added
`Test_ResourceProvider_DefaultsRegistered` that reads `defaults.yaml`,
loads real manifests, and verifies all types are registered with correct
location entries.
- `testdata/manifests-no-location/persistentVolumes.yaml` - Second test
manifest sharing the `Radius.Compute` namespace
-
`test/functional-portable/dynamicrp/noncloud/resources/default_containers_test.go`
- End-to-end functional test deploying `Radius.Compute/containers` and
`Radius.Compute/routes` (two types from the same namespace) using the
default recipes
-
`test/functional-portable/dynamicrp/noncloud/resources/testdata/default-containers-test.bicep`
- Bicep template for the e2e test

## Testing

- **Integration test**
(`Test_ResourceProvider_RegisterManifests_NoLocation`): Two test
manifests sharing `Radius.Compute` namespace, verifies both types appear
in the location's resourceTypes map.
- **Integration test** (`Test_ResourceProvider_DefaultsRegistered`):
Reads `defaults.yaml`, loads manifests from `built-in-providers/dev/`,
verifies all types are registered with correct location entries.
- **Functional test** (`Test_DefaultContainers_Deploy`): End-to-end
deployment of `Radius.Compute/containers` and `Radius.Compute/routes`
using default recipes.

---------

Signed-off-by: Karishma Chawla <kachawla@microsoft.com>
Signed-off-by: Zach Casper <zachcasper@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants