Implementation of resource providers and resource types registration #7967

rynowak · 2024-09-26T19:38:39Z

Description

This change implements the API functionality for registering resource providers and related child-types through the Radius API. This is the first step towards making the Radius API surface extensible. For this first step there is no consumer of this data or these APIs. Subsequent changes will consume the data and implement the user-facing functionality.

What's possible in this commit:

CRUDL operations on resource provider and related types like:
- resource type
- api version (child of resource type)
- location

Additionally, this change implements the resource provider "summary" API. The summary API provides a combined view of the most useful data from each resource provider and its children.

For reviewers there are a few important notes:

Routing in UCP is complex, and error prone to work on. I abandoned the existing code and rewrote it in a form that's more native to go-chi. This was a significant improvement and made it much easier to debug the code. UCP has good integration tests so I'm not very concerned about regressions.
This is our first use-case for child resources in Radius. I used the async controllers to implement cascading deletion.
The "summary" API uses cached data. The async controllers for resource provider and other types update the cache. This is our first use of this pattern, but it felt right to me.

Type of change

This pull request adds or changes features of Radius and has an approved issue (issue link required).

Part of: #6688

rynowak · 2024-09-26T19:40:36Z

pkg/armrpc/frontend/server/handler.go

@@ -128,6 +128,25 @@ func HandlerForController(controller ctrl.Controller, operationType v1.Operation
 }
 }

+// CreateHandler creates an http.Handler for the given resource type and operation method.


For reviewers, here's a suggested tour:

Start with the .tsp, converters, and datamodel. That will give you an idea of the API surface being added.

Next look at the frontend changes. This is where I implemented support for child-resources.

Next look at the routing changes in UCP. It was impossible to implement the correct behavior using the current design of routing. UCP's requirements are more complex.

Last take a look at the integration tests + backend conrollers.

rynowak · 2024-09-26T19:40:56Z

pkg/armrpc/frontend/server/handler.go

@@ -128,6 +128,25 @@ func HandlerForController(controller ctrl.Controller, operationType v1.Operation
 }
 }

+// CreateHandler creates an http.Handler for the given resource type and operation method.
+func CreateHandler(ctx context.Context, resourceType string, operationMethod v1.OperationMethod, opts ctrl.Options, factory ControllerFactoryFunc) (http.HandlerFunc, error) {
+ storageClient, err := opts.DataProvider.GetStorageClient(ctx, resourceType)


I needed to extract this logic into its own function to use with UCP routing.

rynowak · 2024-09-26T19:42:25Z

pkg/ucp/datamodel/genericresource.go

@@ -24,8 +24,8 @@ import (
 const (
 // OperationProcess is the operation type for processing a tracked resource.
 OperationProcess = "PROCESS"
- // ResourceType is the resource type for a generic resource.
- ResourceType = "System.Resources/resources"


This was a naming conflict with a bunch of other things.

rynowak · 2024-09-26T19:42:59Z

pkg/ucp/frontend/controller/resourceproviders/getresourceprovidersummary.go

+func (r *GetResourceProviderSummary) Run(ctx context.Context, w http.ResponseWriter, req *http.Request) (armrpc_rest.Response, error) {
+ relativePath := middleware.GetRelativePath(r.Options().PathBase, req.URL.Path)
+
+ scope, name, err := r.extractScopeAndName(relativePath)


This is a special case where the request URL isn't a resource ID. This is why I had to write bespoke controllers for the summary.

rynowak · 2024-09-26T19:44:15Z

pkg/ucp/frontend/radius/routes.go

- resourceGroupResourceRouter := server.NewSubrouter(baseRouter, resourceGroupResourcePath, apiValidator)
-
- handlerOptions := []server.HandlerOptions{
- {


The existing way of registering handlers works decently well for resource providers. UCP has a bunch of specialized functionality which conflicts.

It's always been a pain to work on this code in UCP, and I finally had a need to rewrite it. I think the result is much more maintainable.

rynowak · 2024-09-26T19:45:28Z

pkg/ucp/integrationtests/testserver/testserver.go

+ address := "http://" + server.Listener.Addr().String()
+ connection, err := sdk.NewDirectConnection(address + pathBase)
+ require.NoError(t, err)
+


Needed to reorder some things here to get the URL of the server. The backend controllers call back into UCP to initiate a cascading delete, so the URL is required.

rynowak · 2024-09-26T19:45:54Z

pkg/ucp/integrationtests/testserver/testserver.go

+ if !parsed.IsAbs() {
+ requestUrl = ts.BaseURL + pathAndQuery
+ }
+


This was needed to work with async operations.

rynowak · 2024-09-26T19:46:09Z

pkg/ucp/integrationtests/testserver/testserver.go

@@ -468,6 +485,91 @@ func (tr *TestResponse) EqualsResponse(statusCode int, body []byte) {
 require.Equal(tr.t, statusCode, tr.Raw.StatusCode, "status code did not match expected")
 }

+// EqualsValue compares a TestResponse against an expected status code and an response body.
+//
+// If the systemData propert is present in the response, it will be removed.


Added some new helpers for testing.

This change implements the API functionality for registering resource providers and related child-types through the Radius API. This is the first step towards making the Radius API surface extensible. For this first step there is no consumer of this data or these APIs. Subsequent changes will consume the data and implement the user-facing functionality. What's possible in this commit: - CRUDL operations on resource provider and related types like: - resource type - api version (child of resource type) - location Additionally, this change implements the resource provider "summary" API. The summary API provides a combined view of the most useful data from each resource provider and its children. --- For reviewers there are a few important notes: - Routing in UCP is complex, and error prone to work on. I abandoned the existing code and rewrote it in a form that's more native to go-chi. This was a significant improvement and made it much easier to debug the code. UCP has good integration tests so I'm not very concerned about regressions. - This is our first use-case for child resources in Radius. I used the async controllers to implement cascading deletion. - The "summary" API uses cached data. The async controllers for resource provider and other types update the cache. This is our first use of this pattern, but it felt right to me. Signed-off-by: Ryan Nowak <[email protected]>

radius-functional-tests · 2024-09-27T07:08:29Z

Radius functional test overview

🔍 Go to test action run

Name	Value
Repository	rynowak/radius
Commit ref	`9f6be61`
Unique ID	func02676243a4
Image tag	pr-func02676243a4

Click here to see the list of tools in the current test run

gotestsum 1.12.0
KinD: v0.20.0
Dapr: 1.12.0
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-func02676243a4
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-func02676243a4
controller test image location: ghcr.io/radius-project/dev/controller:pr-func02676243a4
ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-func02676243a4
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 datastoresrp-cloud functional tests...
⌛ Starting ucp-cloud functional tests...
⌛ Starting corerp-cloud functional tests...
✅ datastoresrp-cloud functional tests succeeded
✅ ucp-cloud functional tests succeeded
✅ corerp-cloud functional tests succeeded

codecov · 2024-09-27T07:17:13Z

Codecov Report

Attention: Patch coverage is 68.48485% with 312 lines in your changes missing coverage. Please review.

Project coverage is 61.31%. Comparing base (24be8e6) to head (9f6be61).

Files with missing lines	Patch %	Lines
...oller/resourceproviders/resourceprovider_delete.go	55.17%	26 Missing and 26 partials ⚠️
...resourceproviders/listresourceprovidersummaries.go	0.00%	45 Missing ⚠️
...er/resourceproviders/getresourceprovidersummary.go	23.52%	39 Missing ⚠️
...ontroller/resourceproviders/resourcetype_delete.go	60.00%	14 Missing and 14 partials ⚠️
...g/ucp/backend/controller/resourceproviders/util.go	61.36%	8 Missing and 9 partials ⚠️
pkg/armrpc/frontend/server/handler.go	0.00%	11 Missing ⚠️
...kg/ucp/datamodel/converter/apiversion_converter.go	52.38%	7 Missing and 3 partials ⚠️
pkg/ucp/datamodel/converter/location_converter.go	52.38%	7 Missing and 3 partials ⚠️
.../datamodel/converter/resourceprovider_converter.go	52.38%	7 Missing and 3 partials ⚠️
...del/converter/resourceprovidersummary_converter.go	33.33%	9 Missing and 1 partial ⚠️
... and 21 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7967      +/-   ##
==========================================
+ Coverage   61.13%   61.31%   +0.18%     
==========================================
  Files         533      559      +26     
  Lines       28248    29179     +931     
==========================================
+ Hits        17269    17892     +623     
- Misses       9462     9684     +222     
- Partials     1517     1603      +86

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

rynowak requested review from a team as code owners September 26, 2024 19:38

rynowak commented Sep 26, 2024

View reviewed changes

ytimocin force-pushed the udt-registration branch from e4547fb to 9f6be61 Compare September 27, 2024 07:06

ytimocin temporarily deployed to functional-tests September 27, 2024 07:07 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation of resource providers and resource types registration #7967

Implementation of resource providers and resource types registration #7967

rynowak commented Sep 26, 2024

rynowak Sep 26, 2024

rynowak Sep 26, 2024

rynowak Sep 26, 2024

rynowak Sep 26, 2024

rynowak Sep 26, 2024

rynowak Sep 26, 2024

rynowak Sep 26, 2024

rynowak Sep 26, 2024

radius-functional-tests bot commented Sep 27, 2024 •

edited

Loading

codecov bot commented Sep 27, 2024

Implementation of resource providers and resource types registration #7967

Are you sure you want to change the base?

Implementation of resource providers and resource types registration #7967

Conversation

rynowak commented Sep 26, 2024

Description

Type of change

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

radius-functional-tests bot commented Sep 27, 2024 • edited Loading

Radius functional test overview

Test Status

codecov bot commented Sep 27, 2024

Codecov Report

radius-functional-tests bot commented Sep 27, 2024 •

edited

Loading