From 4ef8cb3f12b7baac36b9460b890cf92027c13efb Mon Sep 17 00:00:00 2001 From: yangsoon Date: Thu, 13 May 2021 21:44:08 +0800 Subject: [PATCH] fix bug: Use stricter syntax check for CUE (#1643) * fix cue render * fix e2e-api-test * add test --- e2e/plugin/plugin_test.go | 9 ++- pkg/dsl/definition/template.go | 4 +- pkg/dsl/definition/template_test.go | 73 ++++++++++++++++++++++- test/e2e-test/definition_revision_test.go | 10 ++-- 4 files changed, 85 insertions(+), 11 deletions(-) diff --git a/e2e/plugin/plugin_test.go b/e2e/plugin/plugin_test.go index e298caf72..decc8d58b 100644 --- a/e2e/plugin/plugin_test.go +++ b/e2e/plugin/plugin_test.go @@ -160,7 +160,7 @@ spec: properties: image: crccheck/hello-world port: 5000 - cpu: 0.5 + cpu: "0.5" traits: - type: test-ingress properties: @@ -535,7 +535,7 @@ var livediffResult = `--- - - name: express-server + - name: new-express-server properties: -+ cpu: 0.5 ++ cpu: "0.5" image: crccheck/hello-world - port: 80 + port: 5000 @@ -671,6 +671,11 @@ var livediffResult = `--- + name: new-express-server + ports: + - containerPort: 5000 ++ resources: ++ limits: ++ cpu: "0.5" ++ requests: ++ cpu: "0.5" + status: + observedGeneration: 0 diff --git a/pkg/dsl/definition/template.go b/pkg/dsl/definition/template.go index 41d6feba0..a0169bc88 100644 --- a/pkg/dsl/definition/template.go +++ b/pkg/dsl/definition/template.go @@ -109,7 +109,7 @@ func (wd *workloadDef) Complete(ctx process.Context, abstractTemplate string, pa return err } - if err := inst.Value().Err(); err != nil { + if err := inst.Value().Validate(); err != nil { return errors.WithMessagef(err, "invalid cue template of workload %s after merge parameter and context", wd.name) } output := inst.Lookup(OutputFieldName) @@ -294,7 +294,7 @@ func (td *traitDef) Complete(ctx process.Context, abstractTemplate string, param return err } - if err := inst.Value().Err(); err != nil { + if err := inst.Value().Validate(); err != nil { return errors.WithMessagef(err, "invalid template of trait %s after merge with parameter and context", td.name) } processing := inst.Lookup("processing") diff --git a/pkg/dsl/definition/template_test.go b/pkg/dsl/definition/template_test.go index 95ad8bf06..8d65ade57 100644 --- a/pkg/dsl/definition/template_test.go +++ b/pkg/dsl/definition/template_test.go @@ -34,6 +34,7 @@ func TestWorkloadTemplateComplete(t *testing.T) { expectObj runtime.Object expAssObjs map[string]runtime.Object category types.CapabilityCategory + hasCompileErr bool }{ "only contain an output": { workloadTemplate: ` @@ -60,6 +61,7 @@ parameter: { "metadata": map[string]interface{}{"name": "test"}, "spec": map[string]interface{}{"replicas": int64(2)}, }}, + hasCompileErr: false, }, "contain output and outputs": { workloadTemplate: ` @@ -122,6 +124,7 @@ parameter: { }, }, }, + hasCompileErr: false, }, "output needs context appRevision": { workloadTemplate: ` @@ -156,6 +159,7 @@ parameter: { }, }, }, + hasCompileErr: false, }, "output needs context replicas": { workloadTemplate: ` @@ -180,13 +184,46 @@ parameter: { }, }, }, + hasCompileErr: false, + }, + "parameter type doesn't match will raise error": { + workloadTemplate: ` +output:{ + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: context.name + spec: replicas: parameter.replicas +} +parameter: { + replicas: *1 | int + type: string + host: string +} +`, + params: map[string]interface{}{ + "replicas": "2", + "type": "ClusterIP", + "host": "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, }, } for _, v := range testCases { ctx := process.NewContext("default", "test", "myapp", "myapp-v1") wt := NewWorkloadAbstractEngine("testWorkload", &PackageDiscover{}) - assert.NoError(t, wt.Complete(ctx, v.workloadTemplate, v.params)) + err := wt.Complete(ctx, v.workloadTemplate, v.params) + hasError := err != nil + assert.Equal(t, v.hasCompileErr, hasError) + if v.hasCompileErr { + continue + } base, assists := ctx.Output() assert.Equal(t, len(v.expAssObjs), len(assists)) assert.NotNil(t, base) @@ -211,6 +248,7 @@ func TestTraitTemplateComplete(t *testing.T) { params map[string]interface{} expWorkload *unstructured.Unstructured expAssObjs map[string]runtime.Object + hasCompileErr bool }{ "patch trait": { traitTemplate: ` @@ -691,6 +729,32 @@ parameter: { "spec": map[string]interface{}{"maxReplicas": int64(10), "minReplicas": int64(1)}}}, }, }, + "parameter type doesn't match will raise error": { + traitTemplate: ` + parameter: { + exposePort: int + } + // trait template can have multiple outputs in one trait + outputs: service: { + apiVersion: "v1" + kind: "Service" + spec: { + selector: + app: context.name + ports: [ + { + port: parameter.exposePort + targetPort: parameter.exposePort + } + ] + } + } +`, + params: map[string]interface{}{ + "exposePort": "1080", + }, + hasCompileErr: true, + }, } for cassinfo, v := range tds { @@ -761,7 +825,12 @@ parameter: { return } td := NewTraitAbstractEngine(v.traitName, &PackageDiscover{}) - assert.NoError(t, td.Complete(ctx, v.traitTemplate, v.params)) + err := td.Complete(ctx, v.traitTemplate, v.params) + hasError := err != nil + assert.Equal(t, v.hasCompileErr, hasError) + if v.hasCompileErr { + continue + } base, assists := ctx.Output() assert.Equal(t, len(v.expAssObjs), len(assists), cassinfo) assert.NotNil(t, base) diff --git a/test/e2e-test/definition_revision_test.go b/test/e2e-test/definition_revision_test.go index a361f7eee..a66fb5a80 100644 --- a/test/e2e-test/definition_revision_test.go +++ b/test/e2e-test/definition_revision_test.go @@ -81,7 +81,7 @@ var _ = Describe("Test application of the specified definition version", func() return fmt.Errorf("error defRevison number wants %d, actually %d", 2, len(labelDefRevList.Items)) } return nil - }, 20*time.Second, time.Second).Should(BeNil()) + }, 40*time.Second, time.Second).Should(BeNil()) }) @@ -144,7 +144,7 @@ var _ = Describe("Test application of the specified definition version", func() return fmt.Errorf("error defRevison number wants %d, actually %d", 2, len(workerDefRevList.Items)) } return nil - }, 20*time.Second, time.Second).Should(BeNil()) + }, 40*time.Second, time.Second).Should(BeNil()) webserviceV1 := webServiceWithNoTemplate.DeepCopy() webserviceV1.Spec.Schematic.CUE.Template = webServiceV1Template @@ -175,7 +175,7 @@ var _ = Describe("Test application of the specified definition version", func() return fmt.Errorf("error defRevison number wants %d, actually %d", 2, len(webserviceDefRevList.Items)) } return nil - }, 20*time.Second, time.Second).Should(BeNil()) + }, 40*time.Second, time.Second).Should(BeNil()) app := v1beta1.Application{ ObjectMeta: metav1.ObjectMeta{ @@ -405,7 +405,7 @@ var _ = Describe("Test application of the specified definition version", func() return fmt.Errorf("error defRevison number wants %d, actually %d", 2, len(helmworkerDefRevList.Items)) } return nil - }, 20*time.Second, time.Second).Should(BeNil()) + }, 40*time.Second, time.Second).Should(BeNil()) app := v1beta1.Application{ ObjectMeta: metav1.ObjectMeta{ @@ -584,7 +584,7 @@ var _ = Describe("Test application of the specified definition version", func() return fmt.Errorf("error defRevison number wants %d, actually %d", 2, len(kubeworkerDefRevList.Items)) } return nil - }, 20*time.Second, time.Second).Should(BeNil()) + }, 40*time.Second, time.Second).Should(BeNil()) app := v1beta1.Application{ ObjectMeta: metav1.ObjectMeta{