Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove validation of resource types against the static list in recipes codebase #8391

Merged
merged 14 commits into from
Mar 7, 2025
6 changes: 2 additions & 4 deletions pkg/controller/reconciler/recipe_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"},
Expand Down
7 changes: 2 additions & 5 deletions pkg/controller/reconciler/recipe_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -44,7 +42,7 @@ import (
const (
defaultNamespace = "default"
validResourceType = "Applications.Core/extenders"
invalidResourceType = "Applications.Core/invalidType"
invalidResourceType = "invalidType"
webhookConfigName = "recipe-webhook-config"
)

Expand Down Expand Up @@ -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)

Expand Down
5 changes: 2 additions & 3 deletions pkg/corerp/api/v20231001preview/environment_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
}
},
"recipes": {
"Applications.Dapr/pubsub": {
"pubsub": {
"cosmos-recipe": {
"templateKind": "bicep",
"templatePath": "br:ghcr.io/sampleregistry/radius/recipes/pubsub"
Expand Down
11 changes: 6 additions & 5 deletions pkg/corerp/backend/deployment/deploymentprocessor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
70 changes: 0 additions & 70 deletions pkg/rp/portableresources/portableresources.go

This file was deleted.

33 changes: 0 additions & 33 deletions pkg/rp/portableresources/portableresources_test.go

This file was deleted.

Loading