diff --git a/charts/vela-core/README.md b/charts/vela-core/README.md index acb8706ef..6d581d56e 100644 --- a/charts/vela-core/README.md +++ b/charts/vela-core/README.md @@ -96,6 +96,7 @@ helm install --create-namespace -n vela-system kubevela kubevela/vela-core --wai | `featureGates.informerCacheFilterUnnecessaryFields` | filter unnecessary fields for informer cache | `true` | | `featureGates.sharedDefinitionStorageForApplicationRevision` | use definition cache to reduce duplicated definition storage for application revision, must be used with InformerCacheFilterUnnecessaryFields | `true` | | `featureGates.disableWorkflowContextConfigMapCache` | disable the workflow context's configmap informer cache | `true` | +| `featureGates.enableCueValidation` | enable the strict cue validation for cue required parameter fields | `false` | ### MultiCluster parameters diff --git a/charts/vela-core/templates/kubevela-controller.yaml b/charts/vela-core/templates/kubevela-controller.yaml index 617dbe42e..b19006b41 100644 --- a/charts/vela-core/templates/kubevela-controller.yaml +++ b/charts/vela-core/templates/kubevela-controller.yaml @@ -309,6 +309,7 @@ spec: - "--feature-gates=InformerCacheFilterUnnecessaryFields={{- .Values.featureGates.informerCacheFilterUnnecessaryFields | toString -}}" - "--feature-gates=SharedDefinitionStorageForApplicationRevision={{- .Values.featureGates.sharedDefinitionStorageForApplicationRevision | toString -}}" - "--feature-gates=DisableWorkflowContextConfigMapCache={{- .Values.featureGates.disableWorkflowContextConfigMapCache | toString -}}" + - "--feature-gates=EnableCueValidation={{- .Values.featureGates.enableCueValidation | toString -}}" {{ if .Values.authentication.enabled }} {{ if .Values.authentication.withUser }} - "--authentication-with-user" diff --git a/charts/vela-core/values.yaml b/charts/vela-core/values.yaml index 296eb42db..46613667a 100644 --- a/charts/vela-core/values.yaml +++ b/charts/vela-core/values.yaml @@ -119,6 +119,7 @@ optimize: ##@param featureGates.informerCacheFilterUnnecessaryFields filter unnecessary fields for informer cache ##@param featureGates.sharedDefinitionStorageForApplicationRevision use definition cache to reduce duplicated definition storage for application revision, must be used with InformerCacheFilterUnnecessaryFields ##@param featureGates.disableWorkflowContextConfigMapCache disable the workflow context's configmap informer cache +##@param featureGates.enableCueValidation enable the strict cue validation for cue required parameter fields ##@param featureGates: gzipResourceTracker: false @@ -134,6 +135,7 @@ featureGates: informerCacheFilterUnnecessaryFields: true sharedDefinitionStorageForApplicationRevision: true disableWorkflowContextConfigMapCache: true + enableCueValidation: false ## @section MultiCluster parameters diff --git a/pkg/cue/definition/template.go b/pkg/cue/definition/template.go index d4659875d..1c8a8083a 100644 --- a/pkg/cue/definition/template.go +++ b/pkg/cue/definition/template.go @@ -39,6 +39,10 @@ import ( "github.com/oam-dev/kubevela/pkg/cue/task" "github.com/oam-dev/kubevela/pkg/oam" "github.com/oam-dev/kubevela/pkg/oam/util" + + utilfeature "k8s.io/apiserver/pkg/util/feature" + + "github.com/oam-dev/kubevela/pkg/features" ) const ( @@ -123,6 +127,14 @@ func (wd *workloadDef) Complete(ctx process.Context, abstractTemplate string, pa return err } + // Strict Cue required field parameter validation + if utilfeature.DefaultMutableFeatureGate.Enabled(features.EnableCueValidation) { + paramCue := val.LookupPath(value.FieldPath(velaprocess.ParameterFieldName)) + if err := paramCue.Validate(cue.Concrete(true)); err != nil { + return errors.WithMessagef(err, "parameter error for %s", wd.name) + } + } + // we will support outputs for workload composition, and it will become trait in AppConfig. outputs := val.LookupPath(value.FieldPath(OutputsFieldName)) if !outputs.Exists() { diff --git a/pkg/cue/definition/template_test.go b/pkg/cue/definition/template_test.go index 9d13a949f..5117d8ead 100644 --- a/pkg/cue/definition/template_test.go +++ b/pkg/cue/definition/template_test.go @@ -28,6 +28,11 @@ import ( "github.com/oam-dev/kubevela/apis/types" "github.com/oam-dev/kubevela/pkg/cue/process" + + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" + + "github.com/oam-dev/kubevela/pkg/features" ) func TestWorkloadTemplateComplete(t *testing.T) { @@ -1565,3 +1570,271 @@ parameter: { assert.Contains(t, err.Error(), v.err) } } + +func TestWorkloadParamsValidations(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(&testing.T{}, utilfeature.DefaultFeatureGate, features.EnableCueValidation, true)() + testCases := map[string]struct { + workloadTemplate string + params map[string]interface{} + expectObj runtime.Object + expAssObjs map[string]runtime.Object + category types.CapabilityCategory + hasCompileErr bool + errorString string + }{ + "Missing Required Param that is used in template": { + workloadTemplate: ` +output:{ + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: context.name + spec: { + replicas: parameter.replicas + host: parameter.requiredParam + } +} +parameter: { + replicas: *1 | int + type: string + requiredParam!: string +} +`, + params: map[string]interface{}{ + "replicas": 2, + "type": "ClusterIP", + }, + expectObj: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{"name": "test"}, + "spec": map[string]interface{}{"replicas": int64(2)}, + }}, + hasCompileErr: true, + errorString: "parameter error for testWorkload: parameter.requiredParam: field is required but not present", + }, + // Missing Required Param that is not used in template + "Missing Required Param that is not used in template": { + workloadTemplate: ` +output:{ + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: context.name + spec: { + replicas: parameter.replicas + } +} +parameter: { + replicas: *1 | int + type: string + requiredParam!: string +} +`, + params: map[string]interface{}{ + "replicas": 2, + "type": "ClusterIP", + }, + expectObj: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{"name": "test"}, + "spec": map[string]interface{}{"replicas": int64(2)}, + }}, + hasCompileErr: true, + errorString: "parameter error for testWorkload: parameter.requiredParam: field is required but not present", + }, + //required param that is nested + "required param that is nested": { + workloadTemplate: ` +output:{ + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: context.name + spec: { + replicas: parameter.replicas + } +} +parameter: { + replicas: *1 | int + type: string + host: requiredParam!: string +} +`, + params: map[string]interface{}{ + "replicas": 2, + "type": "ClusterIP", + "host": map[string]string{}, + }, + expectObj: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{"name": "test"}, + "spec": map[string]interface{}{"replicas": int64(2)}, + }}, + hasCompileErr: true, + errorString: "parameter error for testWorkload: parameter.host.requiredParam: field is required but not present", + }, + //required params that are provided + "required params that are provided": { + workloadTemplate: ` +output:{ + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: context.name + spec: { + replicas: parameter.replicas + host: parameter.host.requiredParam + } +} +parameter: { + replicas: *1 | int + type: string + host: requiredParam!: string + param1!: string +} +`, + params: map[string]interface{}{ + "replicas": 2, + "type": "ClusterIP", + "host": map[string]interface{}{"requiredParam": "example.com"}, + "param1": "newparam", + }, + expectObj: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{"name": "test"}, + "spec": map[string]interface{}{"replicas": int64(2), "host": "example.com"}, + }}, + hasCompileErr: false, + errorString: "", + }, + //optional and regular param with default value should not give error + "optional and regular param with default value should not give error": { + workloadTemplate: ` +output:{ + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: context.name + spec: { + replicas: parameter.replicas + } +} +parameter: { + replicas: *1 | int + type: string + requiredParam!: string + optionalParam?: string + regularParam: string | *"" +} +`, + params: map[string]interface{}{ + "replicas": 2, + "type": "ClusterIP", + "requiredParam": "example.com", + }, + expectObj: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{"name": "test"}, + "spec": map[string]interface{}{"replicas": int64(2)}, + }}, + hasCompileErr: false, + errorString: "", + }, + // regular param should give error + "regular param should give error": { + workloadTemplate: ` +output:{ + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: context.name + spec: { + replicas: parameter.replicas + } +} +parameter: { + replicas: *1 | int + type: string + requiredParam!: string + regularParam: string +} +`, + params: map[string]interface{}{ + "replicas": 2, + "type": "ClusterIP", + "requiredParam": "example.com", + }, + expectObj: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{"name": "test"}, + "spec": map[string]interface{}{"replicas": int64(2)}, + }}, + hasCompileErr: true, + errorString: "parameter error for testWorkload: parameter.regularParam: incomplete value string", + }, + + // multiple errors + "multiple errors": { + workloadTemplate: ` +output:{ + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: context.name + spec: { + replicas: parameter.replicas + } +} +parameter: { + replicas: *1 | int + type: string + requiredParam!: string + regularParam: string +} +`, + params: map[string]interface{}{ + "replicas": 2, + "type": "ClusterIP", + }, + expectObj: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{"name": "test"}, + "spec": map[string]interface{}{"replicas": int64(2)}, + }}, + hasCompileErr: true, + errorString: "parameter error for testWorkload: parameter.requiredParam: field is required but not present (and 1 more errors)", + }, + } + + for _, v := range testCases { + ctx := process.NewContext(process.ContextData{ + AppName: "myapp", + CompName: "test", + Namespace: "default", + AppRevisionName: "myapp-v1", + ClusterVersion: types.ClusterVersion{Minor: "19+"}, + }) + wt := NewWorkloadAbstractEngine("testWorkload") + err := wt.Complete(ctx, v.workloadTemplate, v.params) + hasError := err != nil + assert.Equal(t, v.hasCompileErr, hasError) + if v.hasCompileErr { + if err != nil { + assert.Equal(t, err.Error(), v.errorString) + } + continue + } + base, assists := ctx.Output() + assert.Equal(t, len(v.expAssObjs), len(assists)) + assert.NotNil(t, base) + baseObj, err := base.Unstructured() + assert.Equal(t, nil, err) + assert.Equal(t, v.expectObj, baseObj) + for _, ss := range assists { + assert.Equal(t, AuxiliaryWorkload, ss.Type) + got, err := ss.Ins.Unstructured() + assert.NoError(t, err) + assert.Equal(t, got, v.expAssObjs[ss.Name]) + } + } +} diff --git a/pkg/features/controller_features.go b/pkg/features/controller_features.go index 811e95fb7..8aa600ae4 100644 --- a/pkg/features/controller_features.go +++ b/pkg/features/controller_features.go @@ -111,6 +111,9 @@ const ( // DisableWorkflowContextConfigMapCache disable the workflow context's configmap informer cache DisableWorkflowContextConfigMapCache = "DisableWorkflowContextConfigMapCache" + + // EnableCueValidation enable strict cue validation fields for the required parameter field verification + EnableCueValidation = "EnableCueValidation" ) var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ @@ -135,6 +138,7 @@ var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ InformerCacheFilterUnnecessaryFields: {Default: true, PreRelease: featuregate.Alpha}, SharedDefinitionStorageForApplicationRevision: {Default: true, PreRelease: featuregate.Alpha}, DisableWorkflowContextConfigMapCache: {Default: true, PreRelease: featuregate.Alpha}, + EnableCueValidation: {Default: false, PreRelease: featuregate.Beta}, } func init() {