github/kubevela
1
0
Fork 0
mirror of https://github.com/kubevela/kubevela.git synced 2026-08-19 04:26:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

Feat(#6505): Add support for Cue Required field feature (#6622)

Browse Source 
* Fix: e2e-tests and unit-tests

- Remove gomega from workflow e2e-test step
- Change the app phase to WorkFlowFailed when there is an error in workflow
- Change the app10.yaml file

Signed-off-by: Chaitanyareddy0702 <chaitanyareddy0702@gmail.com>

Author: VibhorChinda <vibhorchinda@gmail.com>

* Feat: Add strict cue required field parameter validation

Signed-off-by: Chaitanya Reddy Onteddu <co@guidewire.com>

---------

Signed-off-by: Chaitanya Reddy Onteddu <co@guidewire.com>
Co-authored-by: VibhorChinda <vibhorchinda@gmail.com>
This commit is contained in:
Chaitanyareddy0702
2024-09-20 20:28:44 +05:30
committed by GitHub
co-authored by VibhorChinda
parent 3f87c6f2e7
commit 613174384a
6 changed files with 293 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
charts/vela-core/README.md
+1
View File
@@ -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
charts/vela-core/templates/kubevela-controller.yaml
+1
View File
@@ -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"
charts/vela-core/values.yaml
+2
View File
@@ -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
pkg/cue/definition/template.go
+12
View File
@@ -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() {
pkg/cue/definition/template_test.go
+273
View File
@@ -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])
}
}
}
pkg/features/controller_features.go
+4
View File
@@ -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() {