add webhook validation on CUE template outputs name (#1460)

add hooks for process.Context to do validation

add unit test

Signed-off-by: roy wang <seiwy2010@gmail.com>

pkg/appfile/validate.go

@@ -0,0 +1,92 @@
+/*
+Copyright 2021 The KubeVela Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package appfile
+
+import (
+	"fmt"
+
+	"github.com/pkg/errors"
+
+	"github.com/oam-dev/kubevela/apis/types"
+	"github.com/oam-dev/kubevela/pkg/dsl/process"
+)
+
+// ValidateCUESchematicAppfile validates CUE schematic workloads in an Appfile
+func (p *Parser) ValidateCUESchematicAppfile(a *Appfile) error {
+	for _, wl := range a.Workloads {
+		// because helm & kube schematic has no CUE template
+		// it only validates CUE schematic workload
+		if wl.CapabilityCategory != types.CUECategory {
+			continue
+		}
+
+		pCtx, err := newValidationProcessContext(wl, a.Name, a.RevisionName, a.Namespace)
+		if err != nil {
+			return errors.WithMessage(err, "cannot create validationg process context")
+		}
+		for _, tr := range wl.Traits {
+			if tr.CapabilityCategory != types.CUECategory {
+				continue
+			}
+			if err := tr.EvalContext(pCtx); err != nil {
+				return errors.WithMessagef(err, "cannot evaluate trait %q", tr.Name)
+			}
+		}
+	}
+	return nil
+}
+
+func newValidationProcessContext(wl *Workload, appName, revisionName, ns string) (process.Context, error) {
+	baseHooks := []process.BaseHook{
+		// add more hook funcs here to validate CUE base
+	}
+	auxiliaryHooks := []process.AuxiliaryHook{
+		// add more hook funcs here to validate CUE auxiliaries
+		validateAuxiliaryNameUnique(),
+	}
+
+	pCtx := process.NewContextWithHooks(ns, wl.Name, appName, revisionName, baseHooks, auxiliaryHooks)
+	pCtx.InsertSecrets(wl.OutputSecretName, wl.RequiredSecrets)
+	if len(wl.UserConfigs) > 0 {
+		pCtx.SetConfigs(wl.UserConfigs)
+	}
+	if err := wl.EvalContext(pCtx); err != nil {
+		return nil, errors.Wrapf(err, "evaluate base template app=%s in namespace=%s", appName, ns)
+	}
+	return pCtx, nil
+}
+
+// validateAuxiliaryNameUnique validates the name of each outputs item which is
+// called auxiliary in vela CUE-based DSL.
+// Each capability definition can have arbitrary number of outputs and each
+// outputs can have more than one auxiliaries.
+// Auxiliaries can be referenced by other cap's template to pass data
+// within a workload, so their names must be unique.
+func validateAuxiliaryNameUnique() process.AuxiliaryHook {
+	return process.AuxiliaryHookFn(func(c process.Context, a []process.Auxiliary) error {
+		_, existingAuxs := c.Output()
+		for _, newAux := range a {
+			for _, existingAux := range existingAuxs {
+				if existingAux.Name == newAux.Name {
+					return errors.Wrap(fmt.Errorf("auxiliary %q already exits", newAux.Name),
+						"outputs item name must be unique")
+				}
+			}
+		}
+		return nil
+	})
+}

pkg/appfile/validate_test.go

@@ -0,0 +1,148 @@
+/*
+Copyright 2021 The KubeVela Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package appfile
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/gomega"
+
+	"github.com/oam-dev/kubevela/apis/types"
+	"github.com/oam-dev/kubevela/pkg/dsl/definition"
+)
+
+var _ = Describe("Test validate CUE schematic Appfile", func() {
+	type SubTestCase struct {
+		compDefTmpl   string
+		traitDefTmpl1 string
+		traitDefTmpl2 string
+		wantErrMsg    string
+	}
+
+	DescribeTable("Test validate outputs name unique", func(tc SubTestCase) {
+		Expect("").Should(BeEmpty())
+		wl := &Workload{
+			Name:               "myweb",
+			Type:               "worker",
+			CapabilityCategory: types.CUECategory,
+			Traits: []*Trait{
+				{
+					Name:               "myscaler",
+					CapabilityCategory: types.CUECategory,
+					Template:           tc.traitDefTmpl1,
+					engine:             definition.NewTraitAbstractEngine("myscaler", pd),
+				},
+				{
+					Name:               "myingress",
+					CapabilityCategory: types.CUECategory,
+					Template:           tc.traitDefTmpl2,
+					engine:             definition.NewTraitAbstractEngine("myingress", pd),
+				},
+			},
+			FullTemplate: &Template{
+				TemplateStr: tc.compDefTmpl,
+			},
+			engine: definition.NewWorkloadAbstractEngine("myweb", pd),
+		}
+		pCtx, err := newValidationProcessContext(wl, "myapp", "myapp-v1", "test-ns")
+		Expect(err).Should(BeNil())
+		Eventually(func() string {
+			for _, tr := range wl.Traits {
+				if err := tr.EvalContext(pCtx); err != nil {
+					return err.Error()
+				}
+			}
+			return ""
+		}).Should(ContainSubstring(tc.wantErrMsg))
+	},
+		Entry("Succeed", SubTestCase{
+			compDefTmpl: `
+			output: {
+				apiVersion: "apps/v1" 
+				kind: "Deployment"
+				}
+			outputs: mysvc: {
+				apiVersion: "v1"
+				kind: "Service"
+			}
+			`,
+			traitDefTmpl1: `
+			outputs: mysvc1: {
+				apiVersion: "v1"
+				kind: "Service"
+			}
+			`,
+			traitDefTmpl2: `
+			outputs: mysvc2: {
+				apiVersion: "v1"
+				kind: "Service"
+			}
+			`,
+			wantErrMsg: "",
+		}),
+		Entry("CompDef and TraitDef have same outputs", SubTestCase{
+			compDefTmpl: `
+			output: {
+				apiVersion: "apps/v1" 
+				kind: "Deployment"
+				}
+			outputs: mysvc1: {
+				apiVersion: "v1"
+				kind: "Service"
+			}
+			`,
+			traitDefTmpl1: `
+			outputs: mysvc1: {
+				apiVersion: "v1"
+				kind: "Service"
+			}
+			`,
+			traitDefTmpl2: `
+			outputs: mysvc2: {
+				apiVersion: "v1"
+				kind: "Service"
+			}
+			`,
+			wantErrMsg: `auxiliary "mysvc1" already exits`,
+		}),
+		Entry("TraitDefs have same outputs", SubTestCase{
+			compDefTmpl: `
+			output: {
+				apiVersion: "apps/v1" 
+				kind: "Deployment"
+				}
+			outputs: mysvc: {
+				apiVersion: "v1"
+				kind: "Service"
+			}
+			`,
+			traitDefTmpl1: `
+			outputs: mysvc1: {
+				apiVersion: "v1"
+				kind: "Service"
+			}
+			`,
+			traitDefTmpl2: `
+			outputs: mysvc1: {
+				apiVersion: "v1"
+				kind: "Service"
+			}
+			`,
+			wantErrMsg: `auxiliary "mysvc1" already exits`,
+		}),
+	)
+})

pkg/dsl/definition/template.go

@@ -117,7 +117,9 @@ func (wd *workloadDef) Complete(ctx process.Context, abstractTemplate string, pa
 	if err != nil {
 		return errors.WithMessagef(err, "invalid output of workload %s", wd.name)
 	}
-	ctx.SetBase(base)
+	if err := ctx.SetBase(base); err != nil {
+		return err
+	}
 
 	// we will support outputs for workload composition, and it will become trait in AppConfig.
 	outputs := inst.Lookup(OutputsFieldName)
@@ -137,7 +139,9 @@ func (wd *workloadDef) Complete(ctx process.Context, abstractTemplate string, pa
 		if err != nil {
 			return errors.WithMessagef(err, "invalid outputs(%s) of workload %s", fieldInfo.Name, wd.name)
 		}
-		ctx.AppendAuxiliaries(process.Auxiliary{Ins: other, Type: AuxiliaryWorkload, Name: fieldInfo.Name})
+		if err := ctx.AppendAuxiliaries(process.Auxiliary{Ins: other, Type: AuxiliaryWorkload, Name: fieldInfo.Name}); err != nil {
+			return err
+		}
 	}
 	return nil
 }
@@ -314,7 +318,9 @@ func (td *traitDef) Complete(ctx process.Context, abstractTemplate string, param
 			if err != nil {
 				return errors.WithMessagef(err, "invalid outputs(resource=%s) of trait %s", fieldInfo.Name, td.name)
 			}
-			ctx.AppendAuxiliaries(process.Auxiliary{Ins: other, Type: td.name, Name: fieldInfo.Name})
+			if err := ctx.AppendAuxiliaries(process.Auxiliary{Ins: other, Type: td.name, Name: fieldInfo.Name}); err != nil {
+				return err
+			}
 		}
 	}
 

pkg/dsl/process/contexthook.go

@@ -0,0 +1,47 @@
+/*
+Copyright 2021 The KubeVela Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package process
+
+import "github.com/oam-dev/kubevela/pkg/dsl/model"
+
+// BaseHook defines function to be invoked before setting base to a
+// process.Context
+type BaseHook interface {
+	Exec(Context, model.Instance) error
+}
+
+// BaseHookFn implements BaseHook interface
+type BaseHookFn func(Context, model.Instance) error
+
+// Exec will be invoked before settiing 'base' into ctx.Base
+func (fn BaseHookFn) Exec(ctx Context, base model.Instance) error {
+	return fn(ctx, base)
+}
+
+// AuxiliaryHook defines function to be invoked before appending auxiliaries to
+// a process.Context
+type AuxiliaryHook interface {
+	Exec(Context, []Auxiliary) error
+}
+
+// AuxiliaryHookFn implements AuxiliaryHook interface
+type AuxiliaryHookFn func(Context, []Auxiliary) error
+
+// Exec will be invoked before appending 'auxs' into ctx.Auxiliaries
+func (fn AuxiliaryHookFn) Exec(ctx Context, auxs []Auxiliary) error {
+	return fn(ctx, auxs)
+}

pkg/dsl/process/handle.go

@@ -22,6 +22,8 @@ import (
 	"strings"
 	"unicode"
 
+	"github.com/pkg/errors"
+
 	"github.com/oam-dev/kubevela/pkg/dsl/model"
 )
 
@@ -48,8 +50,8 @@ const (
 
 // Context defines Rendering Context Interface
 type Context interface {
-	SetBase(base model.Instance)
-	AppendAuxiliaries(auxiliaries ...Auxiliary)
+	SetBase(base model.Instance) error
+	AppendAuxiliaries(auxiliaries ...Auxiliary) error
 	Output() (model.Instance, []Auxiliary)
 	BaseContextFile() string
 	ExtendedContextFile() string
@@ -87,6 +89,9 @@ type templateContext struct {
 	outputSecretName string
 	// requiredSecrets is used to store all secret names which are generated by cloud resource components and required by current component
 	requiredSecrets []RequiredSecrets
+
+	baseHooks      []BaseHook
+	auxiliaryHooks []AuxiliaryHook
 }
 
 // RequiredSecrets is used to store all secret names which are generated by cloud resource components and required by current component
@@ -109,19 +114,45 @@ func NewContext(namespace, name, appName, appRevision string) Context {
 	}
 }
 
+// NewContextWithHooks create render templateContext with hooks for validation
+func NewContextWithHooks(namespace, name, appName, appRevision string, baseHooks []BaseHook, auxHooks []AuxiliaryHook) Context {
+	return &templateContext{
+		name:           name,
+		appName:        appName,
+		appRevision:    appRevision,
+		configs:        []map[string]string{},
+		auxiliaries:    []Auxiliary{},
+		namespace:      namespace,
+		baseHooks:      baseHooks,
+		auxiliaryHooks: auxHooks,
+	}
+}
+
 // SetBase set templateContext base model
 func (ctx *templateContext) SetConfigs(configs []map[string]string) {
 	ctx.configs = configs
 }
 
 // SetBase set templateContext base model
-func (ctx *templateContext) SetBase(base model.Instance) {
+func (ctx *templateContext) SetBase(base model.Instance) error {
+	for _, hook := range ctx.baseHooks {
+		if err := hook.Exec(ctx, base); err != nil {
+			return errors.Wrap(err, "cannot set base into context")
+		}
+	}
 	ctx.base = base
+	return nil
 }
 
 // AppendAuxiliaries add Assist model to templateContext
-func (ctx *templateContext) AppendAuxiliaries(auxiliaries ...Auxiliary) {
+func (ctx *templateContext) AppendAuxiliaries(auxiliaries ...Auxiliary) error {
+	for _, hook := range ctx.auxiliaryHooks {
+		if err := hook.Exec(ctx, auxiliaries); err != nil {
+			return errors.Wrap(err, "cannot append auxiliaries into context")
+		}
+	}
 	ctx.auxiliaries = append(ctx.auxiliaries, auxiliaries...)
+	return nil
 }
 
 // BaseContextFile return cue format string of templateContext

pkg/webhook/core.oam.dev/v1alpha2/application/suite_test.go

@@ -41,6 +41,8 @@ import (
 	"sigs.k8s.io/yaml"
 
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1alpha2"
+	"github.com/oam-dev/kubevela/pkg/dsl/definition"
+	"github.com/oam-dev/kubevela/pkg/oam/discoverymapper"
 	// +kubebuilder:scaffold:imports
 )
 
@@ -52,6 +54,10 @@ var k8sClient client.Client
 var testEnv *envtest.Environment
 var testScheme = runtime.NewScheme()
 var decoder *admission.Decoder
+var dm discoverymapper.DiscoveryMapper
+var pd *definition.PackageDiscover
+var ctx = context.Background()
+var handler *ValidatingHandler
 
 func TestAPIs(t *testing.T) {
 
@@ -94,6 +100,19 @@ var _ = BeforeSuite(func(done Done) {
 	Expect(err).ToNot(HaveOccurred())
 	Expect(k8sClient).ToNot(BeNil())
 
+	dm, err = discoverymapper.New(cfg)
+	Expect(err).ToNot(HaveOccurred())
+	Expect(dm).ToNot(BeNil())
+
+	pd, err = definition.NewPackageDiscover(cfg)
+	Expect(err).ToNot(HaveOccurred())
+	Expect(pd).ToNot(BeNil())
+
+	handler = &ValidatingHandler{
+		dm: dm,
+		pd: pd,
+	}
+
 	decoder, err = admission.NewDecoder(testScheme)
 	Expect(err).Should(BeNil())
 	Expect(decoder).ToNot(BeNil())

pkg/webhook/core.oam.dev/v1alpha2/application/validating_handler_test.go

@@ -17,8 +17,6 @@ limitations under the License.
 package application
 
 import (
-	"context"
-
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
@@ -28,9 +26,6 @@ import (
 )
 
 var _ = Describe("Test Application Validator", func() {
-	ctx := context.Background()
-	handler := &ValidatingHandler{}
-
 	BeforeEach(func() {
 		Expect(handler.InjectClient(k8sClient)).Should(BeNil())
 		Expect(handler.InjectDecoder(decoder)).Should(BeNil())

pkg/webhook/core.oam.dev/v1alpha2/application/validation.go

@@ -30,9 +30,17 @@ func (h *ValidatingHandler) ValidateCreate(ctx context.Context, app *v1beta1.App
 	var componentErrs field.ErrorList
 	// try to generate an app file
 	appParser := appfile.NewApplicationParser(h.Client, h.dm, h.pd)
-	if _, err := appParser.GenerateAppFile(ctx, app); err != nil {
+
+	af, err := appParser.GenerateAppFile(ctx, app)
+	if err != nil {
 		componentErrs = append(componentErrs, field.Invalid(field.NewPath("spec"), app, err.Error()))
+		// cannot generate appfile, no need to validate further
+		return componentErrs
 	}
+	if err := appParser.ValidateCUESchematicAppfile(af); err != nil {
+		componentErrs = append(componentErrs, field.Invalid(field.NewPath("schematic"), app, err.Error()))
+	}
+
 	return componentErrs
 }