Add tests for plane-scoped API scenarios #5620

rynowak · 2023-05-31T23:53:29Z

Description

This change includes tests for the "plane-scoped" list API scenarios. The existing code in the list shared operation mostly already works for this scenario, but we need to toggle the query to be recursive. We don't do this by default because it would be incorrect and slow.

Also updated some of the cosmosdb storage tests to better cover this scenario. Everything was working as expected, but the tests themselves weren't actually covering this scenario.

Issue reference

Part of: radius-project/core-team#1197

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

Code compiles correctly
Adds necessary unit tests for change
Adds necessary E2E tests for change
Unit tests passing
Extended the documentation / Created issue for it

Auto-generated summary

`🤖 Generated by Copilot at 687d5d6`

Summary

📄🧪🔎

This pull request adds support for plane-scoped queries in the armrpc frontend and the cosmosdb store, and improves the test coverage and specificity for the ListResources operation. It modifies the query construction logic and the request headers in the defaultoperation package, and adds new test cases and test data files.

ListResources grows
Adding recursive queries
Winter of cosmos

Walkthrough

Add RecursiveQuery field to ListResources struct and initialize it to false in NewListResources function (link,link)
Set ScopeRecursive field of store query to RecursiveQuery field of ListResources instance in Run method (link)
Add import statement for v1 package in listresources_test.go file (link)
Add headerFile field to listEnvsCases slice and a new test case for plane-scoped list operation in TestListResourcesRun function (link)
Extract ARMRequestContext from context and add expectedQuery variable to each test case in TestListResourcesRun function (link,link)
Set RecursiveQuery field of ListResources instance to planeScope field of test case in TestListResourcesRun function (link)
Add resource_planescope_requestheaders.json file to testdata folder with request headers for plane-scoped list operation test case (link)
Add test cases for querying by plane scope with recursive and non-recursive queries in TestConstructCosmosDBQuery function (link,link)
Update comment for TestQuery function to include plane-scoped query scenarios (link)
Rename subscriptionID field to rootScope and modify its values to include plane scope in subscriptionIDCases map in TestQuery function (link,link,link)
Fix typos in descriptions of test cases in subscriptionIDCases map in TestQuery function (link,link)
Modify query construction logic to use rootScope field instead of subscriptionID field in TestQuery function (link)
Add assertion to check that each item in query result has an ID that starts with rootScope in TestQuery function (link)

Part of: #5609 This change includes tests for the "plane-scoped" list API scenarios. The existing code in the list shared operation mostly already works for this scenario, but we need to toggle the query to be recursive. We don't do this by default because it would be incorrect and slow. Also updated some of the cosmosdb storage tests to better cover this scenario. Everything was working as expected, but the tests themselves weren't actually covering this scenario.

rynowak · 2023-05-31T23:53:41Z

/cc @youngbupark @ytimocin

github-actions · 2023-05-31T23:54:37Z

Radius functional test overview

🔍 Go to test action run

Name	Value
Repository	project-radius/radius
Commit ref	`687d5d6`
Unique ID	25d1e8325d
Image tag	pr-25d1e8325d

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

KinD: v0.18.0
Dapr: 1.10.0
Azure KeyVault CSI driver: 1.4.2
Azure Workload identity webhook: 1.0.0
recipe location radiusdev.azurecr.io/test/functional/corerp/recipes/<name>:pr-25d1e8325d
appcore-rp test image location: radiusdev.azurecr.io/appcore-rp:pr-25d1e8325d
ucp test image location: radiusdev.azurecr.io/ucpd:pr-25d1e8325d

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 ucp functional tests...
⌛ Starting corerp functional tests...
⌛ Starting samples functional tests...
✅ ucp functional tests succeeded
✅ samples functional tests succeeded
✅ corerp functional tests succeeded

rynowak · 2023-05-31T23:56:17Z

pkg/armrpc/frontend/defaultoperation/listresources.go

-		ResourceType: serviceCtx.ResourceID.Type(),
+		RootScope:      serviceCtx.ResourceID.RootScope(),
+		ResourceType:   serviceCtx.ResourceID.Type(),
+		ScopeRecursive: e.RecursiveQuery,


We need to have different behaviors for this controller between the plane-scope and resource-group-scope usecases. Basically any time there's a mismatch of the query scope and the resource scope we need recursive.

This seemed like a reasonable way to toggle the behavior, open to suggestions.

rynowak · 2023-05-31T23:57:19Z

pkg/armrpc/frontend/defaultoperation/listresources_test.go

+		{"list-envs-more-items-than-top", resourceTestHeaderFile, 10, 5, "5", true, false},
+		{"list-envs-less-items-than-top", resourceTestHeaderFile, 5, 5, "10", false, false},
+		{"list-envs-no-top", resourceTestHeaderFile, 5, 5, "", false, false},
+		{"list-envs-plane-scope-more-items-than-top", "resource_planescope_requestheaders.json", 5, 5, "", false, false},


We didn't have a test for this case, so I added one.

rynowak · 2023-05-31T23:58:27Z

pkg/armrpc/frontend/defaultoperation/testdata/resource_planescope_requestheaders.json

+    "Accept-Language": "en-US",
+    "Content-Length": "305",
+    "Content-Type": "application/json; charset=utf-8",
+    "Referer": "https://radius.dev/planes/radius/local/providers/applications.core/environments/env0?api-version=2022-03-15-privatepreview",


Needed another test file, because we need the root scope to be different. This is actually kinda wierd, but it matches what the exisiting test is doing (getting a URL for a resource get, but testing for list).

rynowak · 2023-05-31T23:59:04Z

pkg/ucp/store/cosmosdb/cosmosdbstorageclient_test.go

+			rootScope:    "/planes/radius/local",
+			resourceType: "",
+			filters:      []store.QueryFilter{},
+			expected:     3,


Looking for input on this, I think the existing tests were just wrong. They were only covering the subscription ID case.

github-actions · 2023-06-01T00:03:27Z

Test Results

2 748 tests +3 2 741 ✔️ +3 1m 58s ⏱️ ±0s
  242 suites ±0       7 💤 ±0
      1 files ±0       0 ❌ ±0

Results for commit f323dff. ± Comparison against base commit 4d124b3.

This pull request removes 2 and adds 5 tests. Note that renamed tests count towards both.

github.com/project-radius/radius/pkg/armrpc/frontend/controller ‑ TestValidateEtag_IfMatch/87731c05-de08-47b7-a3f9-08bec4396237
github.com/project-radius/radius/pkg/armrpc/frontend/controller ‑ TestValidateEtag_IfMatch/87731c05-de08-47b7-a3f9-08bec4396237#01

github.com/project-radius/radius/pkg/armrpc/frontend/controller ‑ TestValidateEtag_IfMatch/28b7458e-e747-4446-9199-3d795b7a73d7
github.com/project-radius/radius/pkg/armrpc/frontend/controller ‑ TestValidateEtag_IfMatch/28b7458e-e747-4446-9199-3d795b7a73d7#01
github.com/project-radius/radius/pkg/armrpc/frontend/defaultoperation ‑ TestListResourcesRun/list-envs-plane-scope-more-items-than-top
github.com/project-radius/radius/pkg/ucp/store/cosmosdb ‑ TestConstructCosmosDBQuery/root-scope-plane
github.com/project-radius/radius/pkg/ucp/store/cosmosdb ‑ TestConstructCosmosDBQuery/root-scope-plane-and-resource-group

♻️ This comment has been updated with latest results.

github-actions · 2023-06-01T00:03:39Z

For the detailed report, please go to Checks tab, click Build and Test, and then download unit_test_coverage artifact at the bottom of build page.

Your PR branch coverage: 64.7 %
main branch coverage: 64.7 %
diff coverage: 0 %

The coverage result does not include the functional test coverage.

youngbupark

LGTM.

youngbupark · 2023-06-01T15:20:14Z

pkg/ucp/store/cosmosdb/cosmosdbstorageclient_test.go

@@ -503,40 +526,40 @@ func TestQuery(t *testing.T) {
 		})
 	}

-	// Query with subscriptionID
+	// Query with subscriptionID or plane - These are recursive queries


This test will be skipped unless we set the following envvars.

https://github.com/project-radius/radius/blob/4d124b35166ca662830dbced0f2bda11c0f80b63/pkg/ucp/store/cosmosdb/cosmosdbstorageclient_test.go#L54-L56

If you didn't validate it, it is fine.

I will do it when I migrate 3rd-party cosmosdb to the official cosmosdb lib. (SDK team finally announced the official cosmosdb lib :) )

https://github.com/Azure/azure-sdk-for-go/tree/main/sdk/data/azcosmos

I did validate it. I create a cosmos DB and ran the tests

I did validate it. I create a cosmos DB and ran the tests

Thanks! :) I may use cosmosdb simulator when I migrate to the official sdk.

github-actions · 2023-06-01T16:10:26Z

Radius functional test overview

🔍 Go to test action run

Name	Value
Repository	project-radius/radius
Commit ref	`f323dff`
Unique ID	32191293a3
Image tag	pr-32191293a3

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

KinD: v0.18.0
Dapr: 1.10.0
Azure KeyVault CSI driver: 1.4.2
Azure Workload identity webhook: 1.0.0
recipe location radiusdev.azurecr.io/test/functional/corerp/recipes/<name>:pr-32191293a3
appcore-rp test image location: radiusdev.azurecr.io/appcore-rp:pr-32191293a3
ucp test image location: radiusdev.azurecr.io/ucpd:pr-32191293a3

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 functional tests...
⌛ Starting samples functional tests...
⌛ Starting ucp functional tests...
✅ samples functional tests succeeded
✅ ucp functional tests succeeded
✅ corerp functional tests succeeded

github-actions · 2023-06-01T16:19:26Z

For the detailed report, please go to Checks tab, click Build and Test, and then download unit_test_coverage artifact at the bottom of build page.

Your PR branch coverage: 64.9 %
main branch coverage: 64.9 %
diff coverage: 0 %

The coverage result does not include the functional test coverage.

ytimocin · 2023-06-01T16:23:05Z

pkg/armrpc/frontend/defaultoperation/listresources.go

+	// RecursiveQuery specifies whether the store query should be recursive or not. This should be set when the
+	// scope of the list operation does not match the scope of the underlying resource type.
+	RecursiveQuery bool


Do we need this or can we understand this from the request?

We need this right now. Let me explain what do and do not know from the request.

We know the scope of the query - that's the part of the URL path before providers

We know the resource type - that's the part of the URL path immediately after providers

We do not know the scope of the resource being queried - this is the info we need to decide if the query is recursive.

rynowak requested a review from a team as a code owner May 31, 2023 23:53

rynowak commented May 31, 2023

View reviewed changes

youngbupark approved these changes Jun 1, 2023

View reviewed changes

Merge branch 'main' into rynowak/plane-scoped-apis

f323dff

ytimocin approved these changes Jun 1, 2023

View reviewed changes

rynowak merged commit 2228304 into main Jun 1, 2023

rynowak deleted the rynowak/plane-scoped-apis branch June 1, 2023 16:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tests for plane-scoped API scenarios #5620

Add tests for plane-scoped API scenarios #5620

rynowak commented May 31, 2023 •

edited by ghost

Loading

rynowak commented May 31, 2023

github-actions bot commented May 31, 2023 •

edited

Loading

rynowak May 31, 2023

rynowak May 31, 2023

rynowak May 31, 2023

rynowak May 31, 2023

github-actions bot commented Jun 1, 2023 •

edited

Loading

github-actions bot commented Jun 1, 2023

youngbupark left a comment

youngbupark Jun 1, 2023

rynowak Jun 1, 2023

youngbupark Jun 1, 2023

github-actions bot commented Jun 1, 2023 •

edited

Loading

github-actions bot commented Jun 1, 2023

ytimocin Jun 1, 2023

rynowak Jun 1, 2023

Add tests for plane-scoped API scenarios #5620

Add tests for plane-scoped API scenarios #5620

Conversation

rynowak commented May 31, 2023 • edited by ghost Loading

Description

Issue reference

Checklist

Auto-generated summary

🤖 Generated by Copilot at 687d5d6

Summary

Walkthrough

rynowak commented May 31, 2023

github-actions bot commented May 31, 2023 • edited Loading

Radius functional test overview

Test Status

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

github-actions bot commented Jun 1, 2023 • edited Loading

Test Results

github-actions bot commented Jun 1, 2023

youngbupark left a 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

github-actions bot commented Jun 1, 2023 • edited Loading

Radius functional test overview

Test Status

github-actions bot commented Jun 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rynowak commented May 31, 2023 •

edited by ghost

Loading

`🤖 Generated by Copilot at 687d5d6`

github-actions bot commented May 31, 2023 •

edited

Loading

github-actions bot commented Jun 1, 2023 •

edited

Loading

github-actions bot commented Jun 1, 2023 •

edited

Loading