Feat: add validation for invalid definition attribute (#6029)

* Feat: add validation for invalid definition attribute

Signed-off-by: Somefive <yd219913@alibaba-inc.com>

* fix flaky test

Signed-off-by: Somefive <yd219913@alibaba-inc.com>

---------

Signed-off-by: Somefive <yd219913@alibaba-inc.com>

pkg/appfile/parser_test.go

@@ -24,6 +24,7 @@ import (
 	"testing"
 
 	"github.com/crossplane/crossplane-runtime/pkg/test"
+	"github.com/kubevela/pkg/util/slices"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/stretchr/testify/assert"
@@ -660,7 +661,11 @@ func TestParser_parseScopes(t *testing.T) {
 			},
 			wantErr: assert.NoError,
 			validateFunc: func(w *Workload) bool {
-				return w != nil && len(w.Scopes) == 2 && w.Scopes[0].Name == "namespace1" && w.Scopes[1].Name == "namespace2"
+				if w == nil {
+					return false
+				}
+				scopes := slices.Map(w.Scopes, func(scope Scope) string { return scope.Name })
+				return len(scopes) == 2 && slices.Contains(scopes, "namespace1") && slices.Contains(scopes, "namespace2")
 			},
 		},
 	}

pkg/definition/definition.go

@@ -20,6 +20,7 @@ package definition
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"strings"
 
@@ -46,6 +47,7 @@ import (
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
 	velacue "github.com/oam-dev/kubevela/pkg/cue"
 	"github.com/oam-dev/kubevela/pkg/oam"
+	"github.com/oam-dev/kubevela/pkg/utils"
 	"github.com/oam-dev/kubevela/pkg/utils/filters"
 )
 
@@ -56,12 +58,16 @@ const (
 	AliasKey = "definition.oam.dev/alias"
 	// UserPrefix defines the prefix of user customized label or annotation
 	UserPrefix = "custom.definition.oam.dev/"
-	// DefinitionAlias is alias of definition
-	DefinitionAlias = "alias.config.oam.dev"
-	// DefinitionType marks definition's usage type, like image-registry
-	DefinitionType = "type.config.oam.dev"
-	// ConfigCatalog marks definition is a catalog
-	ConfigCatalog = "catalog.config.oam.dev"
+)
+
+// the names for different type of definition
+const (
+	componentDefType    = "component"
+	traitDefType        = "trait"
+	policyDefType       = "policy"
+	workflowStepDefType = "workflow-step"
+	workloadDefType     = "workload"
+	scopeDefType        = "scope"
 )
 
 var (
@@ -69,23 +75,23 @@ var (
 	DefinitionTemplateKeys = []string{"spec", "schematic", "cue", "template"}
 	// DefinitionTypeToKind maps the definition types to corresponding kinds
 	DefinitionTypeToKind = map[string]string{
-		"component":     v1beta1.ComponentDefinitionKind,
-		"trait":         v1beta1.TraitDefinitionKind,
-		"policy":        v1beta1.PolicyDefinitionKind,
-		"workload":      v1beta1.WorkloadDefinitionKind,
-		"scope":         v1beta1.ScopeDefinitionKind,
-		"workflow-step": v1beta1.WorkflowStepDefinitionKind,
+		componentDefType:    v1beta1.ComponentDefinitionKind,
+		traitDefType:        v1beta1.TraitDefinitionKind,
+		policyDefType:       v1beta1.PolicyDefinitionKind,
+		workloadDefType:     v1beta1.WorkloadDefinitionKind,
+		scopeDefType:        v1beta1.ScopeDefinitionKind,
+		workflowStepDefType: v1beta1.WorkflowStepDefinitionKind,
 	}
 	// StringToDefinitionType converts user input to DefinitionType used in DefinitionRevisions
 	StringToDefinitionType = map[string]common.DefinitionType{
 		// component
-		"component": common.ComponentType,
+		componentDefType: common.ComponentType,
 		// trait
-		"trait": common.TraitType,
+		traitDefType: common.TraitType,
 		// policy
-		"policy": common.PolicyType,
+		policyDefType: common.PolicyType,
 		// workflow-step
-		"workflow-step": common.WorkflowStepType,
+		workflowStepDefType: common.WorkflowStepType,
 	}
 	// DefinitionKindToNameLabel records DefinitionRevision types and labels to search its name
 	DefinitionKindToNameLabel = map[common.DefinitionType]string{
@@ -96,12 +102,12 @@ var (
 	}
 	// DefinitionKindToType maps the definition kinds to a shorter type
 	DefinitionKindToType = map[string]string{
-		v1beta1.ComponentDefinitionKind:    "component",
-		v1beta1.TraitDefinitionKind:        "trait",
-		v1beta1.PolicyDefinitionKind:       "policy",
-		v1beta1.WorkloadDefinitionKind:     "workload",
-		v1beta1.ScopeDefinitionKind:        "scope",
-		v1beta1.WorkflowStepDefinitionKind: "workflow-step",
+		v1beta1.ComponentDefinitionKind:    componentDefType,
+		v1beta1.TraitDefinitionKind:        traitDefType,
+		v1beta1.PolicyDefinitionKind:       policyDefType,
+		v1beta1.WorkloadDefinitionKind:     workloadDefType,
+		v1beta1.ScopeDefinitionKind:        scopeDefType,
+		v1beta1.WorkflowStepDefinitionKind: workflowStepDefType,
 	}
 )
 
@@ -331,10 +337,36 @@ func (def *Definition) FromCUE(val *cue.Value, templateString string) error {
 	if err := unstructured.SetNestedField(spec, templateString, DefinitionTemplateKeys[1:]...); err != nil {
 		return err
 	}
+	if err = validateSpec(spec, def.GetType()); err != nil {
+		return fmt.Errorf("invalid definition spec: %w", err)
+	}
 	def.Object["spec"] = spec
 	return nil
 }
 
+func validateSpec(spec map[string]interface{}, t string) error {
+	bs, err := json.Marshal(spec)
+	if err != nil {
+		return err
+	}
+	var tpl interface{}
+	switch t {
+	case componentDefType:
+		tpl = &v1beta1.ComponentDefinitionSpec{}
+	case traitDefType:
+		tpl = &v1beta1.TraitDefinitionSpec{}
+	case policyDefType:
+		tpl = &v1beta1.PolicyDefinitionSpec{}
+	case workflowStepDefType:
+		tpl = &v1beta1.WorkflowStepDefinitionSpec{}
+	default:
+	}
+	if tpl != nil {
+		return utils.StrictUnmarshal(bs, tpl)
+	}
+	return nil
+}
+
 func encodeDeclsToString(decls []ast.Decl) (string, error) {
 	bs, err := format.Node(&ast.File{Decls: decls}, format.Simplify())
 	if err != nil {
@@ -590,7 +622,7 @@ func GetDefinitionDefaultSpec(kind string) map[string]interface{} {
 			"appliesToWorkloads": []interface{}{},
 			"conflictsWith":      []interface{}{},
 			"workloadRefPath":    "",
-			"definitionRef":      "",
+			"definitionRef":      map[string]interface{}{},
 			"podDisruptive":      false,
 			"schematic": map[string]interface{}{
 				"cue": map[string]interface{}{

pkg/definition/definition_test.go

@@ -25,6 +25,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -218,3 +219,46 @@ func TestDefinitionRevisionSearch(t *testing.T) {
 	_, err = GetDefinitionFromDefinitionRevision(&defrev)
 	assert.NoError(t, err)
 }
+
+func TestValidateSpec(t *testing.T) {
+	testcases := map[string]struct {
+		Input  string
+		Type   string
+		HasErr bool
+	}{
+		"comp": {
+			Input: `{"podSpecPath": "a"}`,
+			Type:  "component",
+		},
+		"trait": {
+			Input: `{"appliesToWorkloads":["deployments"]}`,
+			Type:  "trait",
+		},
+		"workflow-step": {
+			Input: `{"definitionRef":{"name":"v"}}`,
+			Type:  "workflow-step",
+		},
+		"bad-policy": {
+			Input:  `{"definitionRef":{"invalid":5}}`,
+			Type:   "policy",
+			HasErr: true,
+		},
+		"unknown": {
+			Input:  `{}`,
+			Type:   "unknown",
+			HasErr: false,
+		},
+	}
+	for name, tt := range testcases {
+		t.Run(name, func(t *testing.T) {
+			spec := map[string]interface{}{}
+			require.NoError(t, json.Unmarshal([]byte(tt.Input), &spec))
+			err := validateSpec(spec, tt.Type)
+			if tt.HasErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}