validating webhook for TraitDefinition

add unit test

fix lint issues

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

charts/vela-core/templates/webhook.yaml

@@ -142,6 +142,28 @@ webhooks:
     admissionReviewVersions: 
       - v1beta1
     timeoutSeconds: 5
+  - clientConfig:
+      caBundle: Cg==
+      service:
+        name: {{ template "kubevela.name" . }}-webhook
+        namespace: {{ .Release.Namespace }}
+        path: /validating-core-oam-dev-v1alpha2-traitdefinitions
+    failurePolicy: Fail
+    name: validating.core.oam.dev.v1alpha2.traitdefinitions
+    rules:
+      - apiGroups:
+          - core.oam.dev
+        apiVersions:
+          - v1alpha2
+        operations:
+          - CREATE
+          - UPDATE
+        resources:
+          - traitdefinitions
+        scope: Cluster
+    admissionReviewVersions: 
+      - v1beta1
+    timeoutSeconds: 5
   - clientConfig:
       caBundle: Cg==
       service:

pkg/webhook/core.oam.dev/register.go

@@ -7,6 +7,7 @@ import (
 	"github.com/oam-dev/kubevela/pkg/webhook/core.oam.dev/v1alpha2/applicationconfiguration"
 	"github.com/oam-dev/kubevela/pkg/webhook/core.oam.dev/v1alpha2/applicationdeployment"
 	"github.com/oam-dev/kubevela/pkg/webhook/core.oam.dev/v1alpha2/component"
+	"github.com/oam-dev/kubevela/pkg/webhook/core.oam.dev/v1alpha2/traitdefinition"
 )
 
 // Register will be called in main and register all validation handlers
@@ -17,6 +18,9 @@ func Register(mgr manager.Manager) error {
 	if err := applicationconfiguration.RegisterValidatingHandler(mgr); err != nil {
 		return err
 	}
+	if err := traitdefinition.RegisterValidatingHandler(mgr); err != nil {
+		return err
+	}
 	applicationconfiguration.RegisterMutatingHandler(mgr)
 	applicationdeployment.RegisterMutatingHandler(mgr)
 	if err := component.RegisterMutatingHandler(mgr); err != nil {

pkg/webhook/core.oam.dev/v1alpha2/traitdefinition/validating_handler.go

@@ -0,0 +1,140 @@
+package traitdefinition
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"net/http"
+
+	admissionv1beta1 "k8s.io/api/admission/v1beta1"
+	"k8s.io/klog"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
+	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	"github.com/pkg/errors"
+
+	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1alpha2"
+	"github.com/oam-dev/kubevela/pkg/oam/discoverymapper"
+)
+
+const (
+	errValidateDefRef = "error occurs when validating definition reference"
+
+	failInfoDefRefOmitted = "if definition reference is omitted, patch or output with GVK is required"
+)
+
+var traitDefGVR = v1alpha2.SchemeGroupVersion.WithResource("traitdefinitions")
+
+// ValidatingHandler handles validation of trait definition
+type ValidatingHandler struct {
+	Client client.Client
+	Mapper discoverymapper.DiscoveryMapper
+
+	// Decoder decodes object
+	Decoder *admission.Decoder
+	// Validators validate objects
+	Validators []TraitDefValidator
+}
+
+// TraitDefValidator validate trait definition
+type TraitDefValidator interface {
+	Validate(context.Context, v1alpha2.TraitDefinition) error
+}
+
+// TraitDefValidatorFn implements TraitDefValidator
+type TraitDefValidatorFn func(context.Context, v1alpha2.TraitDefinition) error
+
+// Validate implements TraitDefValidator method
+func (fn TraitDefValidatorFn) Validate(ctx context.Context, td v1alpha2.TraitDefinition) error {
+	return fn(ctx, td)
+}
+
+var _ admission.Handler = &ValidatingHandler{}
+
+// Handle validate trait definition
+func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
+	obj := &v1alpha2.TraitDefinition{}
+	if req.Resource.String() != traitDefGVR.String() {
+		return admission.Errored(http.StatusBadRequest, fmt.Errorf("expect resource to be %s", traitDefGVR))
+	}
+
+	if req.Operation == admissionv1beta1.Create || req.Operation == admissionv1beta1.Update {
+		err := h.Decoder.Decode(req, obj)
+		if err != nil {
+			return admission.Errored(http.StatusBadRequest, err)
+		}
+		klog.Info("validating ", " name: ", obj.Name, " operation: ", string(req.Operation))
+		for _, validator := range h.Validators {
+			if err := validator.Validate(ctx, *obj); err != nil {
+				klog.Info("validation failed ", " name: ", obj.Name, " errMsgi: ", err.Error())
+				return admission.Denied(err.Error())
+			}
+		}
+		klog.Info("validation passed ", " name: ", obj.Name, " operation: ", string(req.Operation))
+	}
+	return admission.ValidationResponse(true, "")
+}
+
+var _ inject.Client = &ValidatingHandler{}
+
+// InjectClient injects the client into the ValidatingHandler
+func (h *ValidatingHandler) InjectClient(c client.Client) error {
+	h.Client = c
+	return nil
+}
+
+var _ admission.DecoderInjector = &ValidatingHandler{}
+
+// InjectDecoder injects the decoder into the ValidatingHandler
+func (h *ValidatingHandler) InjectDecoder(d *admission.Decoder) error {
+	h.Decoder = d
+	return nil
+}
+
+// RegisterValidatingHandler will register TraitDefinition validation to webhook
+func RegisterValidatingHandler(mgr manager.Manager) error {
+	server := mgr.GetWebhookServer()
+	mapper, err := discoverymapper.New(mgr.GetConfig())
+	if err != nil {
+		return err
+	}
+	server.Register("/validating-core-oam-dev-v1alpha2-traitdefinitions", &webhook.Admission{Handler: &ValidatingHandler{
+		Mapper: mapper,
+		Validators: []TraitDefValidator{
+			TraitDefValidatorFn(ValidateDefinitionReference),
+			// add more validators here
+		},
+	}})
+	return nil
+}
+
+// ValidateDefinitionReference validates whether the trait definition is valid if
+// its `.spec.reference` field is unset.
+// It's valid if
+// it has at least one output, and all outputs must have GVK
+// or it has no output but has a patch
+// or it has a patch and outputs, and all outputs must have GVK
+// TODO(roywang) currently we only validate whether it contains CUE template.
+// Further validation, e.g., output with GVK, valid patch, etc, remains to be done.
+func ValidateDefinitionReference(_ context.Context, td v1alpha2.TraitDefinition) error {
+	if len(td.Spec.Reference.Name) > 0 {
+		return nil
+	}
+
+	if td.Spec.Extension == nil || len(td.Spec.Extension.Raw) < 1 {
+		return errors.New(failInfoDefRefOmitted)
+	}
+
+	tmp := map[string]interface{}{}
+	if err := json.Unmarshal(td.Spec.Extension.Raw, &tmp); err != nil {
+		return errors.Wrap(err, errValidateDefRef)
+	}
+	template, ok := tmp["template"]
+	if !ok || len(fmt.Sprint(template)) < 1 {
+		return errors.New(failInfoDefRefOmitted)
+	}
+	return nil
+}

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

@@ -0,0 +1,123 @@
+package traitdefinition
+
+import (
+	"context"
+	"encoding/json"
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/pkg/errors"
+
+	admissionv1beta1 "k8s.io/api/admission/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1alpha2"
+)
+
+var handler ValidatingHandler
+var req admission.Request
+var reqResource metav1.GroupVersionResource
+var decoder *admission.Decoder
+var td v1alpha2.TraitDefinition
+var tdRaw []byte
+var scheme = runtime.NewScheme()
+
+func TestTraitdefinition(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Traitdefinition Suite")
+}
+
+var _ = BeforeSuite(func(done Done) {
+	td = v1alpha2.TraitDefinition{}
+	td.SetGroupVersionKind(v1alpha2.TraitDefinitionGroupVersionKind)
+	tdRaw, _ = json.Marshal(td)
+
+	var err error
+	decoder, err = admission.NewDecoder(scheme)
+	Expect(err).Should(BeNil())
+
+	close(done)
+})
+
+var _ = Describe("Test TraitDefinition validating handler", func() {
+	BeforeEach(func() {
+		reqResource = metav1.GroupVersionResource{
+			Group:    v1alpha2.Group,
+			Version:  v1alpha2.Version,
+			Resource: "traitdefinitions"}
+		handler = ValidatingHandler{}
+		handler.InjectDecoder(decoder)
+	})
+
+	It("Test wrong resource of admission request", func() {
+		wrongReqResource := metav1.GroupVersionResource{
+			Group:    v1alpha2.Group,
+			Version:  v1alpha2.Version,
+			Resource: "foos"}
+		req = admission.Request{
+			AdmissionRequest: admissionv1beta1.AdmissionRequest{
+				Operation: admissionv1beta1.Create,
+				Resource:  wrongReqResource,
+				Object:    runtime.RawExtension{Raw: []byte("")},
+			},
+		}
+		resp := handler.Handle(context.TODO(), req)
+		Expect(resp.Allowed).Should(BeFalse())
+	})
+
+	It("Test bad admission request", func() {
+		req = admission.Request{
+			AdmissionRequest: admissionv1beta1.AdmissionRequest{
+				Operation: admissionv1beta1.Create,
+				Resource:  reqResource,
+				Object:    runtime.RawExtension{Raw: []byte("bad request")},
+			},
+		}
+		resp := handler.Handle(context.TODO(), req)
+		Expect(resp.Allowed).Should(BeFalse())
+	})
+
+	Context("Test create/update operation admission request", func() {
+		var mockValidator TraitDefValidatorFn
+		It("Test validation passed", func() {
+			// mock a validator that always validates successfully
+			mockValidator = func(_ context.Context, _ v1alpha2.TraitDefinition) error {
+				return nil
+			}
+			handler.Validators = []TraitDefValidator{
+				TraitDefValidatorFn(mockValidator),
+			}
+			req = admission.Request{
+				AdmissionRequest: admissionv1beta1.AdmissionRequest{
+					Operation: admissionv1beta1.Create,
+					Resource:  reqResource,
+					Object:    runtime.RawExtension{Raw: tdRaw},
+				},
+			}
+			resp := handler.Handle(context.TODO(), req)
+			Expect(resp.Allowed).Should(BeTrue())
+		})
+		It("Test validation failed", func() {
+			// mock a validator that always failed
+			mockValidator = func(_ context.Context, _ v1alpha2.TraitDefinition) error {
+				return errors.New("mock validator error")
+			}
+			handler.Validators = []TraitDefValidator{
+				TraitDefValidatorFn(mockValidator),
+			}
+			req = admission.Request{
+				AdmissionRequest: admissionv1beta1.AdmissionRequest{
+					Operation: admissionv1beta1.Create,
+					Resource:  reqResource,
+					Object:    runtime.RawExtension{Raw: tdRaw},
+				},
+			}
+			resp := handler.Handle(context.TODO(), req)
+			Expect(resp.Allowed).Should(BeFalse())
+			Expect(resp.Result.Reason).Should(Equal(metav1.StatusReason("mock validator error")))
+		})
+	})
+})

pkg/webhook/core.oam.dev/v1alpha2/traitdefinition/validator_test.go

@@ -0,0 +1,69 @@
+package traitdefinition
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/crossplane/crossplane-runtime/pkg/test"
+	"github.com/google/go-cmp/cmp"
+	"github.com/pkg/errors"
+
+	"github.com/oam-dev/kubevela/pkg/oam/util"
+)
+
+func TestValidateDefinitionReference(t *testing.T) {
+	cases := map[string]struct {
+		reason   string
+		template string
+		want     error
+	}{
+		"NoExtension": {
+			reason:   "An error should be returned if extension is omitted",
+			template: "",
+			want:     errors.New(failInfoDefRefOmitted),
+		},
+		"HaveExtentsion_NoTemplate": {
+			reason: "An error should be returned if extension template is omitted",
+			template: `
+  extension:
+    notemplate: |-
+      fakefield: fakefieldvalue`,
+			want: errors.New(failInfoDefRefOmitted),
+		},
+		"HaveExtension_HaveTemplate": {
+			reason: "No error should be returned if have CUE template",
+			template: `
+  extension:
+    template: |-
+      patch: {
+       spec: replicas: parameter.replicas
+      }`,
+			want: nil,
+		},
+	}
+
+	for caseName, tc := range cases {
+		t.Run(caseName, func(t *testing.T) {
+			tdStr := traitDefStringWithTemplate(tc.template)
+			td, err := util.UnMarshalStringToTraitDefinition(tdStr)
+			if err != nil {
+				t.Fatal("error occurs in generating TraitDefinition string", err.Error())
+			}
+			err = ValidateDefinitionReference(context.Background(), *td)
+			if diff := cmp.Diff(tc.want, err, test.EquateErrors()); diff != "" {
+				t.Errorf("\n%s\nValidateDefinitionReference: -want , +got \n%s\n", tc.reason, diff)
+			}
+		})
+	}
+}
+
+func traitDefStringWithTemplate(t string) string {
+	return fmt.Sprintf(`
+apiVersion: core.oam.dev/v1alpha2
+kind: TraitDefinition
+metadata:
+  name: scaler
+spec:
+%s`, t)
+}