Validate Dapr install in Dapr links #4856
Conversation
Part of: #2952 This change validates the installation status of Dapr inside links RP for Dapr resources. This is step one in removing Dapr from Radius' default installation. After we have a good experience with Dapr missing, we can remove it. This validation is only applied on PUT calls to be more friendly to API consumers. We still allow clients to get and list dapr link resources without Dapr installed. The pattern I followed is similar to the validation we perform for the AKV CSI driver.
| @@ -56,7 +56,7 @@ func TestValidateRequest(t *testing.T) { | |||
| }) | |||
|
|
|||
| // Create default Kubernetes fake client. | |||
| defaultFakeClient := rptesting.NewFakeKubeClient(nil) | |||
| defaultFakeClient := rptesting.NewFakeKubeClient(crdScheme) | |||
There was a problem hiding this comment.
The bug in volume validation had a matching bug in the tests. Since the scheme was not registered, it was failing due to not understanding what a CRD is.
| // Licensed under the MIT License. | ||
| // ------------------------------------------------------------ | ||
|
|
||
| package datamodel |
There was a problem hiding this comment.
I needed a good place to share code among 4 controllers. Open to suggestions on where to place this very small amount of code if datamodel is not right.
| } else if !isSupported { | ||
| return rest.NewBadRequestResponse(datamodel.DaprMissingError), nil | ||
| } | ||
|
|
There was a problem hiding this comment.
It seemed right to make this the first thing that happens in the controller (before validating data, before looking in the database, etc).
| defer mctrl.Finish() | ||
|
|
||
| mStorageClient := store.NewMockStorageClient(mctrl) | ||
| mDeploymentProcessor := deployment.NewMockDeploymentProcessor(mctrl) |
There was a problem hiding this comment.
I've seen this bug appear in a lot of our tests that use both mocks and subtests. We should always initialize the mock controller inside t.Run() in that case.
The problem with this approach is that the mock controller only does validation when all subtests have run, so if one of the subtests was wrong, you can't find out which one. You're sharing the state of the mock across subtests.
If you initialize the controller inside t.Run() then the validations will be performed as part of each subtest.
There was a problem hiding this comment.
Also since go version 1.X (kinda old version I think) defer mctrl.Finish() is called for you.
| @@ -53,13 +51,15 @@ func TestCreateOrUpdateDaprInvokeHttpRoute_20220315PrivatePreview(t *testing.T) | |||
| headerKey string | |||
| headerValue string | |||
| resourceETag string | |||
| daprMissing bool | |||
There was a problem hiding this comment.
Changes to tests were all boilerplate copy-paste.
Part of: #2952
This change validates the installation status of Dapr inside links RP for Dapr resources. This is step one in removing Dapr from Radius' default installation. After we have a good experience with Dapr missing, we can remove it.
This validation is only applied on PUT calls to be more friendly to API consumers. We still allow clients to get and list dapr link resources without Dapr installed.
The pattern I followed is similar to the validation we perform for the AKV CSI driver.