diff --git a/pkg/oam/util/helper.go b/pkg/oam/util/helper.go index dd365f9f..9878b375 100644 --- a/pkg/oam/util/helper.go +++ b/pkg/oam/util/helper.go @@ -272,10 +272,10 @@ func Object2Map(obj interface{}) (map[string]interface{}, error) { return res, err } -// DumpJSON returns the JSON encoding -func DumpJSON(o interface{}) string { +// JSONMarshal returns the JSON encoding +func JSONMarshal(o interface{}) []byte { j, _ := json.Marshal(o) - return string(j) + return j } // GenTraitName generate trait name diff --git a/pkg/webhook/v1alpha2/component/component_suite_test.go b/pkg/webhook/v1alpha2/component/component_suite_test.go index 4645e28d..61e0f6a2 100644 --- a/pkg/webhook/v1alpha2/component/component_suite_test.go +++ b/pkg/webhook/v1alpha2/component/component_suite_test.go @@ -1,30 +1,86 @@ package component_test import ( + "os" "testing" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + crdv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/crossplane/oam-kubernetes-runtime/apis/core" ) var scheme = runtime.NewScheme() +var crd crdv1.CustomResourceDefinition +var reqResource metav1.GroupVersionResource +var decoder *admission.Decoder -func TestMetrics(t *testing.T) { +func TestComponentWebHandler(t *testing.T) { RegisterFailHandler(Fail) - RunSpecs(t, "Metrics Suite") + RunSpecs(t, "Component Web handler") } var _ = BeforeSuite(func(done Done) { By("Bootstrapping test environment") - err := clientgoscheme.AddToScheme(scheme) + ctrl.SetLogger(zap.New(func(o *zap.Options) { + o.Development = true + o.DestWritter = os.Stderr + })) + By("Setup scheme") + err := core.AddToScheme(scheme) Expect(err).Should(BeNil()) - err = core.AddToScheme(scheme) + err = clientgoscheme.AddToScheme(scheme) Expect(err).Should(BeNil()) - err = crdv1.AddToScheme(scheme) + // the crd we will refer to + crd = crdv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo.example.com", + Labels: map[string]string{"crd": "dependency"}, + }, + Spec: crdv1.CustomResourceDefinitionSpec{ + Group: "example.com", + Names: crdv1.CustomResourceDefinitionNames{ + Kind: "Foo", + ListKind: "FooList", + Plural: "foo", + Singular: "foo", + }, + Versions: []crdv1.CustomResourceDefinitionVersion{ + { + Name: "v1", + Served: true, + Storage: true, + Schema: &crdv1.CustomResourceValidation{ + OpenAPIV3Schema: &crdv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]crdv1.JSONSchemaProps{ + "status": { + Type: "object", + Properties: map[string]crdv1.JSONSchemaProps{ + "key": {Type: "string"}, + }, + }, + }, + }, + }, + }, + }, + Scope: crdv1.NamespaceScoped, + }, + } + By("Prepare for the admission resource") + reqResource = metav1.GroupVersionResource{Group: "core.oam.dev", Version: "v1alpha2", Resource: "components"} + By("Prepare for the admission decoder") + decoder, err = admission.NewDecoder(scheme) Expect(err).Should(BeNil()) + By("Finished test bootstrap") + close(done) }) diff --git a/pkg/webhook/v1alpha2/component/component_test.go b/pkg/webhook/v1alpha2/component/component_test.go index 2a177322..2f80435f 100644 --- a/pkg/webhook/v1alpha2/component/component_test.go +++ b/pkg/webhook/v1alpha2/component/component_test.go @@ -1,98 +1,36 @@ package component_test import ( + "context" + "fmt" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" + "github.com/crossplane/crossplane-runtime/pkg/test" + admissionv1beta1 "k8s.io/api/admission/v1beta1" crdv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" utilpointer "k8s.io/utils/pointer" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" + . "github.com/crossplane/oam-kubernetes-runtime/pkg/webhook/v1alpha2/component" ) -var _ = Describe("Metrics Admission controller Test", func() { +var _ = Describe("Component Admission controller Test", func() { var component v1alpha2.Component - var crd crdv1.CustomResourceDefinition - var componentName, namespace, workloadName string + var componentName, namespace string + var label map[string]string BeforeEach(func() { - crd = crdv1.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo.example.com", - Labels: map[string]string{"crd": "dependency"}, - }, - Spec: crdv1.CustomResourceDefinitionSpec{ - Group: "example.com", - Names: crdv1.CustomResourceDefinitionNames{ - Kind: "Foo", - ListKind: "FooList", - Plural: "foo", - Singular: "foo", - }, - Versions: []crdv1.CustomResourceDefinitionVersion{ - { - Name: "v1", - Served: true, - Storage: true, - Schema: &crdv1.CustomResourceValidation{ - OpenAPIV3Schema: &crdv1.JSONSchemaProps{ - Type: "object", - Properties: map[string]crdv1.JSONSchemaProps{ - "status": { - Type: "object", - Properties: map[string]crdv1.JSONSchemaProps{ - "key": {Type: "string"}, - }, - }, - }, - }, - }, - }, - }, - Scope: crdv1.NamespaceScoped, - }, - } - label := map[string]string{"workload": "deployment"} - - wl := appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Name: workloadName, - }, - Spec: appsv1.DeploymentSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: label, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Labels: label, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "wordpress", - Image: "wordpress:4.6.1-apache", - Ports: []corev1.ContainerPort{ - { - Name: "wordpress", - HostPort: 80, - ContainerPort: 8080, - }, - }, - }, - }, - }, - }, - }, - } - // reflect workload gvk from scheme - gvks, _, _ := scheme.ObjectKinds(&wl) - wl.APIVersion = gvks[0].GroupVersion().String() - wl.Kind = gvks[0].Kind + namespace = "component-test" + label = map[string]string{"workload": "test-component"} // Create a component definition componentName = "example-deployment-workload" component = v1alpha2.Component{ @@ -102,9 +40,6 @@ var _ = Describe("Metrics Admission controller Test", func() { Labels: label, }, Spec: v1alpha2.ComponentSpec{ - Workload: runtime.RawExtension{ - Object: &wl, - }, Parameters: []v1alpha2.ComponentParameter{ { Name: "image", @@ -117,18 +52,220 @@ var _ = Describe("Metrics Admission controller Test", func() { }) Context("Test Mutation Webhook", func() { - It("Test with fill in the gvk", func() { - Expect(crd.Spec.Versions).Should(Equal(1)) + var handler admission.Handler = &MutatingHandler{} + var workloadDef v1alpha2.WorkloadDefinition + var workloadTypeName string + var baseWorkload unstructured.Unstructured + + BeforeEach(func() { + decoderInjector := handler.(admission.DecoderInjector) + decoderInjector.InjectDecoder(decoder) + // define workloadDefinition + workloadDef = v1alpha2.WorkloadDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: workloadTypeName, + Labels: label, + }, + Spec: v1alpha2.WorkloadDefinitionSpec{ + Reference: v1alpha2.DefinitionReference{ + Name: "foos.example.com", + }, + }, + } + // the base workload + baseWorkload = unstructured.Unstructured{} + baseWorkload.SetAPIVersion("example.com/v1") + baseWorkload.SetKind("Foo") + baseWorkload.SetName("workloadName") + Expect(len(crd.Spec.Versions)).Should(Equal(1)) Expect(component.Spec).NotTo(BeNil()) }) - }) + It("Test bad admission request format", 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 Validating Webhook", func() { - It("Test validating CR", func() { + It("Test noop mutate admission handle", func() { + component.Spec.Workload = runtime.RawExtension{Raw: util.JSONMarshal(baseWorkload)} + req := admission.Request{ + AdmissionRequest: admissionv1beta1.AdmissionRequest{ + Operation: admissionv1beta1.Create, + Resource: reqResource, + Object: runtime.RawExtension{Raw: util.JSONMarshal(component)}, + }, + } + resp := handler.Handle(context.TODO(), req) + Expect(resp.Allowed).Should(BeTrue()) + }) + + It("Test mutate function", func() { + // the workload that uses type to refer to the workloadDefinition + workloadWithType := unstructured.Unstructured{} + typeContent := make(map[string]interface{}) + typeContent[TypeField] = workloadTypeName + workloadWithType.SetUnstructuredContent(typeContent) + workloadWithType.SetName("workloadName") + // set up the bad type + workloadWithBadType := workloadWithType.DeepCopy() + workloadWithBadType.Object[TypeField] = workloadDef + // set up the result + mutatedWorkload := baseWorkload.DeepCopy() + mutatedWorkload.SetNamespace(component.GetNamespace()) + mutatedWorkload.SetLabels(util.MergeMap(label, map[string]string{WorkloadTypeLabel: workloadTypeName})) + tests := map[string]struct { + client client.Client + workload interface{} + errMsg string + wanted []byte + }{ + "bad workload case": { + workload: "bad workload", + errMsg: "cannot unmarshal string", + }, + "bad workload type case": { + workload: workloadWithBadType, + errMsg: "workload content has an unknown type", + }, + "no op case": { + workload: baseWorkload, + wanted: util.JSONMarshal(baseWorkload), + }, + "update gvk get failed case": { + client: &test.MockClient{ + MockGet: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { + switch obj.(type) { + case *v1alpha2.WorkloadDefinition: + return fmt.Errorf("does not exist") + } + return nil + }, + }, + workload: workloadWithType.DeepCopyObject(), + errMsg: "does not exist", + }, + "update gvk and label case": { + client: &test.MockClient{ + MockGet: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { + switch o := obj.(type) { + case *v1alpha2.WorkloadDefinition: + Expect(key.Name).Should(BeEquivalentTo(typeContent[TypeField])) + *o = workloadDef + case *crdv1.CustomResourceDefinition: + Expect(key.Name).Should(Equal(workloadDef.Spec.Reference.Name)) + *o = crd + } + return nil + }, + }, + workload: workloadWithType.DeepCopyObject(), + wanted: util.JSONMarshal(mutatedWorkload), + }, + } + for testCase, test := range tests { + By(fmt.Sprintf("start test : %s", testCase)) + component.Spec.Workload = runtime.RawExtension{Raw: util.JSONMarshal(test.workload)} + injc := handler.(inject.Client) + injc.InjectClient(test.client) + mutatingHandler := handler.(*MutatingHandler) + err := mutatingHandler.Mutate(&component) + if len(test.errMsg) == 0 { + Expect(err).Should(BeNil()) + Expect(component.Spec.Workload.Raw).Should(BeEquivalentTo(test.wanted)) + } else { + Expect(err.Error()).Should(ContainSubstring(test.errMsg)) + } + } }) + }) + It("Test validating handler", func() { + var handler admission.Handler = &ValidatingHandler{} + decoderInjector := handler.(admission.DecoderInjector) + decoderInjector.InjectDecoder(decoder) + By("Creating valid workload") + validWorkload := unstructured.Unstructured{} + validWorkload.SetAPIVersion("validAPI") + validWorkload.SetKind("validKind") + By("Creating invalid workload with type") + workloadWithType := validWorkload.DeepCopy() + typeContent := make(map[string]interface{}) + typeContent[TypeField] = "should not be here" + workloadWithType.SetUnstructuredContent(typeContent) + By("Creating invalid workload without kind") + noKindWorkload := validWorkload.DeepCopy() + noKindWorkload.SetKind("") + tests := map[string]struct { + workload interface{} + operation admissionv1beta1.Operation + pass bool + reason string + }{ + "valid create case": { + workload: validWorkload.DeepCopyObject(), + operation: admissionv1beta1.Create, + pass: true, + reason: "", + }, + "valid update case": { + workload: validWorkload.DeepCopyObject(), + operation: admissionv1beta1.Update, + pass: true, + reason: "", + }, + "malformat component": { + workload: "bad format", + operation: admissionv1beta1.Create, + pass: false, + reason: "the workload is malformat", + }, + "workload still has type": { + workload: workloadWithType.DeepCopyObject(), + operation: admissionv1beta1.Create, + pass: false, + reason: "the workload contains type info", + }, + "no kind workload component": { + workload: noKindWorkload.DeepCopyObject(), + operation: admissionv1beta1.Update, + pass: false, + reason: "the workload data missing GVK", + }, + } + for testCase, test := range tests { + By(fmt.Sprintf("start test : %s", testCase)) + component.Spec.Workload = runtime.RawExtension{Raw: util.JSONMarshal(test.workload)} + req := admission.Request{ + AdmissionRequest: admissionv1beta1.AdmissionRequest{ + Operation: test.operation, + Resource: reqResource, + Object: runtime.RawExtension{Raw: util.JSONMarshal(component)}, + }, + } + resp := handler.Handle(context.TODO(), req) + Expect(resp.Allowed).Should(Equal(test.pass)) + if !test.pass { + Expect(string(resp.Result.Reason)).Should(ContainSubstring(test.reason)) + } + } + By("Test bad admission request format") + 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()) }) }) diff --git a/pkg/webhook/v1alpha2/component/mutating_handler.go b/pkg/webhook/v1alpha2/component/mutating_handler.go index 1fe9e4dc..8068dd1c 100644 --- a/pkg/webhook/v1alpha2/component/mutating_handler.go +++ b/pkg/webhook/v1alpha2/component/mutating_handler.go @@ -41,11 +41,11 @@ const ( // TypeField is the special field indicate the type of the workloadDefinition TypeField = "type" - // WorkloadTypeLabel is the default metrics path we support + // WorkloadTypeLabel indicates the type of the workloadDefinition WorkloadTypeLabel = "workload.oam.dev/type" ) -// MutatingHandler handles MetricsTrait +// MutatingHandler handles Component type MutatingHandler struct { Client client.Client @@ -81,7 +81,7 @@ func (h *MutatingHandler) Handle(ctx context.Context, req admission.Request) adm resp := admission.PatchResponseFromRaw(req.AdmissionRequest.Object.Raw, marshalled) if len(resp.Patches) > 0 { mutatelog.Info("admit Component", - "namespace", obj.Namespace, "name", obj.Name, "patches", util.DumpJSON(resp.Patches)) + "namespace", obj.Namespace, "name", obj.Name, "patches", util.JSONMarshal(resp.Patches)) } return resp } @@ -122,10 +122,10 @@ func (h *MutatingHandler) Mutate(obj *v1alpha2.Component) error { workload.SetAPIVersion(apiVersion) workload.SetKind(customResourceDefinition.Spec.Names.Kind) mutatelog.Info("Set the component workload GVK", "workload api version", workload.GetAPIVersion(), "workload Kind", workload.GetKind()) - // copy label/annotation to the workload - // add workloadType label - workload.SetLabels(util.MergeMap(obj.GetLabels(), map[string]string{WorkloadTypeLabel: workloadType})) + // copy namespace/label/annotation to the workload and add workloadType label workload.SetNamespace(obj.GetNamespace()) + workload.SetLabels(util.MergeMap(obj.GetLabels(), map[string]string{WorkloadTypeLabel: workloadType})) + workload.SetAnnotations(obj.GetAnnotations()) // copy back the object rawBye, err := json.Marshal(workload.Object) if err != nil { diff --git a/pkg/webhook/v1alpha2/component/validating_handler.go b/pkg/webhook/v1alpha2/component/validating_handler.go index 1aa22b02..3541c114 100644 --- a/pkg/webhook/v1alpha2/component/validating_handler.go +++ b/pkg/webhook/v1alpha2/component/validating_handler.go @@ -20,16 +20,13 @@ import ( "context" "encoding/json" "fmt" - "net/http" admissionv1beta1 "k8s.io/api/admission/v1beta1" apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/validation/field" - "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" "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" @@ -38,7 +35,11 @@ import ( // ValidatingHandler handles Component type ValidatingHandler struct { - Client client.Client + // To use the client, you need to do the following: + // - uncomment it + // - import sigs.k8s.io/controller-runtime/pkg/client + // - uncomment the InjectClient method at the bottom of this file. + // Client client.Client // Decoder decodes objects Decoder *admission.Decoder @@ -57,28 +58,24 @@ func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) a if err != nil { validatelog.Error(err, "decoder failed", "req operation", req.AdmissionRequest.Operation, "req", req.AdmissionRequest) - return admission.Errored(http.StatusBadRequest, err) + return admission.Denied(err.Error()) } switch req.AdmissionRequest.Operation { case admissionv1beta1.Create: if allErrs := ValidateCreate(obj); len(allErrs) > 0 { - validatelog.Info("create failed", "name", obj.Name, "err", allErrs.ToAggregate().Error()) - return admission.Errored(http.StatusUnprocessableEntity, allErrs.ToAggregate()) + validatelog.Info("create failed", "name", obj.Name, "errMsg", allErrs.ToAggregate().Error()) + return admission.Denied(allErrs.ToAggregate().Error()) } case admissionv1beta1.Update: oldObj := &v1alpha2.Component{} - if err := h.Decoder.DecodeRaw(req.AdmissionRequest.OldObject, oldObj); err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - if allErrs := ValidateUpdate(obj, oldObj); len(allErrs) > 0 { - validatelog.Info("update failed", "name", obj.Name, "err", allErrs.ToAggregate().Error()) - return admission.Errored(http.StatusUnprocessableEntity, allErrs.ToAggregate()) + validatelog.Info("update failed", "name", obj.Name, "errMsg", allErrs.ToAggregate().Error()) + return admission.Denied(allErrs.ToAggregate().Error()) } } - return admission.ValidationResponse(true, "") + return admission.Allowed("") } // ValidateCreate validates the Component on creation @@ -89,20 +86,20 @@ func ValidateCreate(obj *v1alpha2.Component) field.ErrorList { fldPath := field.NewPath("spec") var content map[string]interface{} if err := json.Unmarshal(obj.Spec.Workload.Raw, &content); err != nil { - allErrs = append(allErrs, field.Invalid(fldPath.Child("workload"), obj.Spec.Workload, - fmt.Sprintf("the workload `%+v` is malformat", obj.Spec.Workload))) + allErrs = append(allErrs, field.Invalid(fldPath.Child("workload"), string(obj.Spec.Workload.Raw), + "the workload is malformat")) return allErrs } if content[TypeField] != nil { - allErrs = append(allErrs, field.Invalid(fldPath.Child("workload"), obj.Spec.Workload, - fmt.Sprintf("the workload `%+v` is not a CR", obj.Spec.Workload))) + allErrs = append(allErrs, field.Invalid(fldPath.Child("workload"), string(obj.Spec.Workload.Raw), + "the workload contains type info")) } workload := unstructured.Unstructured{ Object: content, } if len(workload.GetAPIVersion()) == 0 || len(workload.GetKind()) == 0 { - allErrs = append(allErrs, field.Invalid(fldPath.Child("workload"), obj.Spec.Workload, - fmt.Sprintf("the data `%+v` missing GVK", obj.Spec.Workload))) + allErrs = append(allErrs, field.Invalid(fldPath.Child("workload"), content, + fmt.Sprintf("the workload data missing GVK, api = %s, kind = %s,", workload.GetAPIVersion(), workload.GetKind()))) } return allErrs } @@ -113,12 +110,7 @@ func ValidateUpdate(r *v1alpha2.Component, _ *v1alpha2.Component) field.ErrorLis return ValidateCreate(r) } -// ValidateDelete validates the Component on delete -func ValidateDelete(r *v1alpha2.Component) field.ErrorList { - validatelog.Info("validate delete", "name", r.Name) - return nil -} - +/* var _ inject.Client = &ValidatingHandler{} // InjectClient injects the client into the ComponentValidatingHandler @@ -126,7 +118,7 @@ func (h *ValidatingHandler) InjectClient(c client.Client) error { h.Client = c return nil } - +*/ var _ admission.DecoderInjector = &ValidatingHandler{} // InjectDecoder injects the decoder into the ComponentValidatingHandler