From b411d79ed07830a72fafbdceb4429c277068bae0 Mon Sep 17 00:00:00 2001 From: Yue Wang Date: Tue, 13 Apr 2021 20:18:15 +0900 Subject: [PATCH] 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 --- pkg/appfile/validate.go | 92 +++++++++++ pkg/appfile/validate_test.go | 148 ++++++++++++++++++ pkg/dsl/definition/template.go | 12 +- pkg/dsl/process/contexthook.go | 47 ++++++ pkg/dsl/process/handle.go | 39 ++++- .../v1alpha2/application/suite_test.go | 19 +++ .../application/validating_handler_test.go | 5 - .../v1alpha2/application/validation.go | 10 +- 8 files changed, 359 insertions(+), 13 deletions(-) create mode 100644 pkg/appfile/validate.go create mode 100644 pkg/appfile/validate_test.go create mode 100644 pkg/dsl/process/contexthook.go diff --git a/pkg/appfile/validate.go b/pkg/appfile/validate.go new file mode 100644 index 000000000..dc73af854 --- /dev/null +++ b/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 + }) +} diff --git a/pkg/appfile/validate_test.go b/pkg/appfile/validate_test.go new file mode 100644 index 000000000..6bc4aad22 --- /dev/null +++ b/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`, + }), + ) +}) diff --git a/pkg/dsl/definition/template.go b/pkg/dsl/definition/template.go index 2e408cad8..41d6feba0 100644 --- a/pkg/dsl/definition/template.go +++ b/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 + } } } diff --git a/pkg/dsl/process/contexthook.go b/pkg/dsl/process/contexthook.go new file mode 100644 index 000000000..fc700d992 --- /dev/null +++ b/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) +} diff --git a/pkg/dsl/process/handle.go b/pkg/dsl/process/handle.go index 529832b45..0cef899fb 100644 --- a/pkg/dsl/process/handle.go +++ b/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 diff --git a/pkg/webhook/core.oam.dev/v1alpha2/application/suite_test.go b/pkg/webhook/core.oam.dev/v1alpha2/application/suite_test.go index acfe17abe..4de888c25 100644 --- a/pkg/webhook/core.oam.dev/v1alpha2/application/suite_test.go +++ b/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()) diff --git a/pkg/webhook/core.oam.dev/v1alpha2/application/validating_handler_test.go b/pkg/webhook/core.oam.dev/v1alpha2/application/validating_handler_test.go index b1ae2ffd2..d3784dc7f 100644 --- a/pkg/webhook/core.oam.dev/v1alpha2/application/validating_handler_test.go +++ b/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()) diff --git a/pkg/webhook/core.oam.dev/v1alpha2/application/validation.go b/pkg/webhook/core.oam.dev/v1alpha2/application/validation.go index 71446b4cd..de4dace1e 100644 --- a/pkg/webhook/core.oam.dev/v1alpha2/application/validation.go +++ b/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 }