diff --git a/pkg/controller/reconciler/recipe_webhook.go b/pkg/controller/reconciler/recipe_webhook.go index a41ba6e295..2fcd91f8cb 100644 --- a/pkg/controller/reconciler/recipe_webhook.go +++ b/pkg/controller/reconciler/recipe_webhook.go @@ -22,7 +22,6 @@ import ( "strings" radappiov1alpha3 "github.com/radius-project/radius/pkg/controller/api/radapp.io/v1alpha3" - portableresources "github.com/radius-project/radius/pkg/rp/portableresources" "github.com/radius-project/radius/pkg/ucp/ucplog" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -90,11 +89,10 @@ func (r *RecipeWebhook) validateRecipeType(ctx context.Context, recipe *radappio logger := ucplog.FromContextOrDiscard(ctx) var errList field.ErrorList flPath := field.NewPath("spec").Child("type") - validResourceTypes := strings.Join(portableresources.GetValidPortableResourceTypes(), ", ") logger.Info("Validating Recipe Type %s in Recipe %s", recipe.Spec.Type, recipe.Name) - if !portableresources.IsValidPortableResourceType(recipe.Spec.Type) { - errList = append(errList, field.Invalid(flPath, recipe.Spec.Type, fmt.Sprintf("allowed values are: %s", validResourceTypes))) + if recipe.Spec.Type == "" || strings.Count(recipe.Spec.Type, "/") != 1 { + errList = append(errList, field.Invalid(flPath, recipe.Spec.Type, "must be in the format 'ResourceProvider.Namespace/resourceType'")) return nil, apierrors.NewInvalid( schema.GroupKind{Group: "radapp.io", Kind: "Recipe"}, diff --git a/pkg/controller/reconciler/recipe_webhook_test.go b/pkg/controller/reconciler/recipe_webhook_test.go index 8bb2c0c857..04da8a6f28 100644 --- a/pkg/controller/reconciler/recipe_webhook_test.go +++ b/pkg/controller/reconciler/recipe_webhook_test.go @@ -21,12 +21,10 @@ import ( "fmt" "net" "net/http" - "strings" "testing" "time" radappiov1alpha3 "github.com/radius-project/radius/pkg/controller/api/radapp.io/v1alpha3" - portableresources "github.com/radius-project/radius/pkg/rp/portableresources" "github.com/radius-project/radius/test/testcontext" "github.com/stretchr/testify/require" @@ -44,7 +42,7 @@ import ( const ( defaultNamespace = "default" validResourceType = "Applications.Core/extenders" - invalidResourceType = "Applications.Core/invalidType" + invalidResourceType = "invalidType" webhookConfigName = "recipe-webhook-config" ) @@ -234,8 +232,7 @@ func Test_Webhook_ValidateFunctions(t *testing.T) { } if tr.wantErr { - validResourceTypes := strings.Join(portableresources.GetValidPortableResourceTypes(), ", ") - expectedError := fmt.Sprintf("Recipe.radapp.io \"%s\" is invalid: spec.type: Invalid value: \"%s\": allowed values are: %s", tr.recipeName, tr.typeName, validResourceTypes) + expectedError := fmt.Sprintf("Recipe.radapp.io \"%s\" is invalid: spec.type: Invalid value: \"%s\": must be in the format 'ResourceProvider.Namespace/resourceType'", tr.recipeName, tr.typeName) require.True(t, apierrors.IsInvalid(err)) require.EqualError(t, err, expectedError) diff --git a/pkg/corerp/api/v20231001preview/environment_conversion.go b/pkg/corerp/api/v20231001preview/environment_conversion.go index efeca97f37..93e4795666 100644 --- a/pkg/corerp/api/v20231001preview/environment_conversion.go +++ b/pkg/corerp/api/v20231001preview/environment_conversion.go @@ -26,7 +26,6 @@ import ( "github.com/radius-project/radius/pkg/kubernetes" types "github.com/radius-project/radius/pkg/recipes" - rp_util "github.com/radius-project/radius/pkg/rp/portableresources" rpv1 "github.com/radius-project/radius/pkg/rp/v1" "github.com/radius-project/radius/pkg/to" ) @@ -67,8 +66,8 @@ func (src *EnvironmentResource) ConvertTo() (v1.DataModelInterface, error) { if src.Properties.Recipes != nil { envRecipes := make(map[string]map[string]datamodel.EnvironmentRecipeProperties) for resourceType, recipes := range src.Properties.Recipes { - if !rp_util.IsValidPortableResourceType(resourceType) { - return &datamodel.Environment{}, v1.NewClientErrInvalidRequest(fmt.Sprintf("invalid resource type: %q", resourceType)) + if resourceType == "" || strings.Count(resourceType, "/") != 1 { + return &datamodel.Environment{}, v1.NewClientErrInvalidRequest(fmt.Sprintf("invalid resource type: %q. Must be in the format \"ResourceProvider.Namespace/resourceType\"", resourceType)) } envRecipes[resourceType] = map[string]datamodel.EnvironmentRecipeProperties{} for recipeName, recipeDetails := range recipes { diff --git a/pkg/corerp/api/v20231001preview/environment_conversion_test.go b/pkg/corerp/api/v20231001preview/environment_conversion_test.go index 64ca6c1940..4670f25e4d 100644 --- a/pkg/corerp/api/v20231001preview/environment_conversion_test.go +++ b/pkg/corerp/api/v20231001preview/environment_conversion_test.go @@ -340,7 +340,7 @@ func TestConvertVersionedToDataModel(t *testing.T) { }, { filename: "environmentresource-invalid-resourcetype.json", - err: &v1.ErrClientRP{Code: v1.CodeInvalid, Message: "invalid resource type: \"Applications.Dapr/pubsub\""}, + err: &v1.ErrClientRP{Code: v1.CodeInvalid, Message: "invalid resource type: \"pubsub\". Must be in the format \"ResourceProvider.Namespace/resourceType\""}, }, { filename: "environmentresource-invalid-templatekind.json", diff --git a/pkg/corerp/api/v20231001preview/testdata/environmentresource-invalid-resourcetype.json b/pkg/corerp/api/v20231001preview/testdata/environmentresource-invalid-resourcetype.json index 6739e5212a..665786c200 100644 --- a/pkg/corerp/api/v20231001preview/testdata/environmentresource-invalid-resourcetype.json +++ b/pkg/corerp/api/v20231001preview/testdata/environmentresource-invalid-resourcetype.json @@ -17,7 +17,7 @@ } }, "recipes": { - "Applications.Dapr/pubsub": { + "pubsub": { "cosmos-recipe": { "templateKind": "bicep", "templatePath": "br:ghcr.io/sampleregistry/radius/recipes/pubsub" diff --git a/pkg/corerp/backend/deployment/deploymentprocessor.go b/pkg/corerp/backend/deployment/deploymentprocessor.go index 1e4de863c2..58b4abec52 100644 --- a/pkg/corerp/backend/deployment/deploymentprocessor.go +++ b/pkg/corerp/backend/deployment/deploymentprocessor.go @@ -25,7 +25,6 @@ import ( "strings" v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" - rp_pr "github.com/radius-project/radius/pkg/rp/portableresources" rp_util "github.com/radius-project/radius/pkg/rp/util" rpv1 "github.com/radius-project/radius/pkg/rp/v1" @@ -225,6 +224,9 @@ func (dp *deploymentProcessor) getApplicationAndEnvironmentForResourceID(ctx con // 2. fetch the application properties from the DB app := &corerp_dm.Application{} + if res.AppID == nil { + return nil, nil, fmt.Errorf("application ID is not set for the resource %q", id.String()) + } err = rp_util.FetchScopeResource(ctx, dp.databaseClient, res.AppID.String(), app) if err != nil { return nil, nil, err @@ -575,17 +577,16 @@ func (dp *deploymentProcessor) getResourceDataByID(ctx context.Context, resource func (dp *deploymentProcessor) buildResourceDependency(resourceID resources.ID, applicationID string, resource v1.DataModelInterface, outputResources []rpv1.OutputResource, computedValues map[string]any, secretValues map[string]rpv1.SecretValueReference, recipeData portableresources.RecipeData) (ResourceData, error) { var appID *resources.ID + // Application id is mandatory for some of the core resource types and is a required field. if applicationID != "" { parsedID, err := resources.ParseResource(applicationID) if err != nil { return ResourceData{}, v1.NewClientErrInvalidRequest(fmt.Sprintf("application ID %q for the resource %q is not a valid id. Error: %s", applicationID, resourceID.String(), err.Error())) } appID = &parsedID - } else if rp_pr.IsValidPortableResourceType(resourceID.TypeSegments()[0].Type) { - // Application id is optional for portable resource types - appID = nil } else { - return ResourceData{}, fmt.Errorf("missing required application id for the resource %q", resourceID.String()) + // Application id is optional for portable resource types and UDTs. + appID = nil } return ResourceData{ diff --git a/pkg/corerp/backend/deployment/deploymentprocessor_test.go b/pkg/corerp/backend/deployment/deploymentprocessor_test.go index a0d64f52bf..92a795b9de 100644 --- a/pkg/corerp/backend/deployment/deploymentprocessor_test.go +++ b/pkg/corerp/backend/deployment/deploymentprocessor_test.go @@ -602,7 +602,7 @@ func Test_Render(t *testing.T) { _, err := dp.Render(ctx, resourceID, &testResource) require.Error(t, err) - require.Equal(t, "missing required application id for the resource \"/subscriptions/test-subscription/resourceGroups/test-resource-group/providers/Applications.Core/containers/test-resource\"", err.Error()) + require.Equal(t, "application ID is not set for the resource \"/subscriptions/test-subscription/resourceGroups/test-resource-group/providers/Applications.Core/containers/test-resource\"", err.Error()) }) t.Run("Invalid application resource type", func(t *testing.T) { diff --git a/pkg/rp/portableresources/portableresources.go b/pkg/rp/portableresources/portableresources.go deleted file mode 100644 index a67b3a0c78..0000000000 --- a/pkg/rp/portableresources/portableresources.go +++ /dev/null @@ -1,70 +0,0 @@ -/* -Copyright 2023 The Radius Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package portableresources - -import ( - "sort" - "strings" - - dapr_ctrl "github.com/radius-project/radius/pkg/daprrp/frontend/controller" - ds_ctrl "github.com/radius-project/radius/pkg/datastoresrp/frontend/controller" - msg_ctrl "github.com/radius-project/radius/pkg/messagingrp/frontend/controller" -) - -const ExtendersResourceType = "Applications.Core/extenders" - -// IsValidPortableResourceType checks if the provided resource type is a valid portable resource type. -// Returns true if the resource type is valid, false otherwise. -func IsValidPortableResourceType(resourceType string) bool { - portableResourceTypes := []string{ - dapr_ctrl.DaprPubSubBrokersResourceType, - dapr_ctrl.DaprSecretStoresResourceType, - dapr_ctrl.DaprStateStoresResourceType, - dapr_ctrl.DaprConfigurationStoresResourceType, - msg_ctrl.RabbitMQQueuesResourceType, - ds_ctrl.MongoDatabasesResourceType, - ds_ctrl.RedisCachesResourceType, - ds_ctrl.SqlDatabasesResourceType, - ExtendersResourceType, - } - - for _, s := range portableResourceTypes { - if strings.EqualFold(s, resourceType) { - return true - } - } - - return false -} - -// GetValidPortableResourceTypes returns list of valid portable resource types. -func GetValidPortableResourceTypes() []string { - resourceTypes := []string{ - dapr_ctrl.DaprPubSubBrokersResourceType, - dapr_ctrl.DaprSecretStoresResourceType, - dapr_ctrl.DaprStateStoresResourceType, - dapr_ctrl.DaprConfigurationStoresResourceType, - msg_ctrl.RabbitMQQueuesResourceType, - ds_ctrl.MongoDatabasesResourceType, - ds_ctrl.RedisCachesResourceType, - ds_ctrl.SqlDatabasesResourceType, - ExtendersResourceType, - } - sort.Strings(resourceTypes) - - return resourceTypes -} diff --git a/pkg/rp/portableresources/portableresources_test.go b/pkg/rp/portableresources/portableresources_test.go deleted file mode 100644 index 1a1a0b65ce..0000000000 --- a/pkg/rp/portableresources/portableresources_test.go +++ /dev/null @@ -1,33 +0,0 @@ -/* -Copyright 2023 The Radius Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package portableresources - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestValidPortableResourceType(t *testing.T) { - isValid := IsValidPortableResourceType("Applications.Datastores/mongoDatabases") - require.Equal(t, true, isValid) -} - -func TestInvalidPortableResourceType(t *testing.T) { - isValid := IsValidPortableResourceType("Applications.Dapr/pubSubBroker") - require.Equal(t, false, isValid) -}