From 7fa5a34c9dca926890fa6b72e6306e8e078607fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=A4=A9=E5=85=83?= Date: Fri, 9 Oct 2020 20:26:52 +0800 Subject: [PATCH] add mutating for appConfig to support name/properties fomart of trait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 天元 --- .../templates/webhook.yaml | 16 + .../applicationconfiguration/render_test.go | 2 +- pkg/oam/labels.go | 5 + pkg/oam/types.go | 3 - pkg/oam/util/helper.go | 16 +- pkg/oam/util/helper_test.go | 35 +- pkg/webhook/v1alpha2/admit.go | 3 +- .../applicationconfiguration/handler_test.go | 306 ++++++++++++++++++ .../applicationconfiguration/helper.go | 4 +- .../applicationconfiguration/helper_test.go | 2 +- .../mutating_handler.go | 174 ++++++++++ .../applicationconfiguration/suite_test.go | 91 ++++++ ...configuration.go => validating_handler.go} | 59 +++- ...ion_test.go => validating_handler_test.go} | 36 ++- 14 files changed, 711 insertions(+), 41 deletions(-) create mode 100644 pkg/webhook/v1alpha2/applicationconfiguration/handler_test.go create mode 100644 pkg/webhook/v1alpha2/applicationconfiguration/mutating_handler.go create mode 100644 pkg/webhook/v1alpha2/applicationconfiguration/suite_test.go rename pkg/webhook/v1alpha2/applicationconfiguration/{applicationconfiguration.go => validating_handler.go} (65%) rename pkg/webhook/v1alpha2/applicationconfiguration/{applicationconfiguration_test.go => validating_handler_test.go} (91%) diff --git a/charts/oam-kubernetes-runtime/templates/webhook.yaml b/charts/oam-kubernetes-runtime/templates/webhook.yaml index c2df7109..4a6d1838 100644 --- a/charts/oam-kubernetes-runtime/templates/webhook.yaml +++ b/charts/oam-kubernetes-runtime/templates/webhook.yaml @@ -47,6 +47,22 @@ metadata: labels: {{- include "oam-kubernetes-runtime.selectorLabels" . | nindent 4 }} webhooks: + - name: "mutate.applicationconfigurations.core.oam.dev" + clientConfig: + service: + name: {{ template "oam-kubernetes-runtime.name" . }}-webhook + namespace: {{.Release.Namespace}} + path: /mutating-core-oam-dev-v1alpha2-applicationconfigurations + caBundle: "{{.Values.certificate.caBundle}}" + rules: + - apiGroups: ["core.oam.dev"] + apiVersions: ["v1alpha2"] + operations: ["CREATE", "UPDATE"] + resources: ["applicationconfigurations"] + scope: "Namespaced" + admissionReviewVersions: ["v1beta1"] + failurePolicy: Fail + timeoutSeconds: 5 - name: "mutate.component.core.oam.dev" clientConfig: service: diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go index 4309fb29..b1fa0b2c 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go @@ -749,7 +749,7 @@ func TestGetDefinitionName(t *testing.T) { } for name, ti := range tests { t.Run(name, func(t *testing.T) { - got := util.GetDefinitionName(ti.u) + got := util.GetDefinitionName(ti.u, "") if got != ti.exp { t.Errorf("%s getCRDName want %s got %s ", ti.reason, ti.exp, got) } diff --git a/pkg/oam/labels.go b/pkg/oam/labels.go index 97b72447..1f30174d 100644 --- a/pkg/oam/labels.go +++ b/pkg/oam/labels.go @@ -27,6 +27,11 @@ const ( LabelAppComponentRevision = "app.oam.dev/revision" // LabelOAMResourceType whether a CR is workload or trait LabelOAMResourceType = "app.oam.dev/resourceType" + + // WorkloadTypeLabel indicates the type of the workloadDefinition + WorkloadTypeLabel = "workload.oam.dev/type" + // TraitTypeLabel indicates the type of the traitDefinition + TraitTypeLabel = "trait.oam.dev/type" ) const ( diff --git a/pkg/oam/types.go b/pkg/oam/types.go index 5cc3be19..456c96c2 100644 --- a/pkg/oam/types.go +++ b/pkg/oam/types.go @@ -27,9 +27,6 @@ import ( runtimev1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" ) -// WorkloadTypeLabel indicates the type of the workloadDefinition -const WorkloadTypeLabel = "workload.oam.dev/type" - // ScopeKind contains the type metadata for a kind of an OAM scope resource. type ScopeKind schema.GroupVersionKind diff --git a/pkg/oam/util/helper.go b/pkg/oam/util/helper.go index 4fa18fae..7a547c48 100644 --- a/pkg/oam/util/helper.go +++ b/pkg/oam/util/helper.go @@ -115,7 +115,8 @@ func FetchWorkload(ctx context.Context, c client.Client, mLog logr.Logger, oamTr func FetchScopeDefinition(ctx context.Context, r client.Reader, scope *unstructured.Unstructured) (*v1alpha2.ScopeDefinition, error) { // The name of the scopeDefinition CR is the CRD name of the scope - spName := GetDefinitionName(scope) + // TODO(wonderflow): we haven't support scope definition label type yet. + spName := GetDefinitionName(scope, "") // the scopeDefinition crd is cluster scoped nn := types.NamespacedName{Name: spName} // Fetch the corresponding scopeDefinition CR @@ -130,7 +131,7 @@ func FetchScopeDefinition(ctx context.Context, r client.Reader, func FetchTraitDefinition(ctx context.Context, r client.Reader, trait *unstructured.Unstructured) (*v1alpha2.TraitDefinition, error) { // The name of the traitDefinition CR is the CRD name of the trait - trName := GetDefinitionName(trait) + trName := GetDefinitionName(trait, oam.TraitTypeLabel) // the traitDefinition crd is cluster scoped nn := types.NamespacedName{Name: trName} // Fetch the corresponding traitDefinition CR @@ -145,7 +146,7 @@ func FetchTraitDefinition(ctx context.Context, r client.Reader, func FetchWorkloadDefinition(ctx context.Context, r client.Reader, workload *unstructured.Unstructured) (*v1alpha2.WorkloadDefinition, error) { // The name of the workloadDefinition CR is the CRD name of the component - wldName := GetDefinitionName(workload) + wldName := GetDefinitionName(workload, oam.WorkloadTypeLabel) // the workloadDefinition crd is cluster scoped nn := types.NamespacedName{Name: wldName} // Fetch the corresponding workloadDefinition CR @@ -227,10 +228,11 @@ func PassLabelAndAnnotation(parentObj oam.Object, childObj labelAnnotationObject // GetDefinitionName return the Definition name of any resources // the format of the definition of a resource is . // Now the definition name of a resource could also be defined as `definition.oam.dev/name` in `metadata.annotations` -func GetDefinitionName(u *unstructured.Unstructured) string { - if labels := u.GetLabels(); labels != nil { - if resourceType, ok := labels[oam.LabelOAMResourceType]; ok && resourceType == "WORKLOAD" { - if definitionName, ok := labels[oam.WorkloadTypeLabel]; ok { +// typeLabel specified which Definition it is, if specified, will directly get definition from label. +func GetDefinitionName(u *unstructured.Unstructured, typeLabel string) string { + if typeLabel != "" { + if labels := u.GetLabels(); labels != nil { + if definitionName, ok := labels[typeLabel]; ok { return definitionName } } diff --git a/pkg/oam/util/helper_test.go b/pkg/oam/util/helper_test.go index 2703e20f..85e48da6 100644 --- a/pkg/oam/util/helper_test.go +++ b/pkg/oam/util/helper_test.go @@ -698,8 +698,9 @@ var _ = Describe("Test workload related helper utils", func() { var _ = Describe("Test unstructured related helper utils", func() { It("Test get CRD name from an unstructured object", func() { tests := map[string]struct { - u *unstructured.Unstructured - exp string + u *unstructured.Unstructured + typeLabel string + exp string }{ "native resource": { u: &unstructured.Unstructured{Object: map[string]interface{}{ @@ -715,11 +716,37 @@ var _ = Describe("Test unstructured related helper utils", func() { }}, exp: "simplerollouttraits.extend.oam.dev", }, + "trait": { + u: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "extend.oam.dev/v1alpha2", + "kind": "SimpleRolloutTrait", + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + oam.TraitTypeLabel: "rollout", + }, + }, + }}, + typeLabel: oam.TraitTypeLabel, + exp: "rollout", + }, + "workload": { + u: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + oam.WorkloadTypeLabel: "deploy", + }, + }, + }}, + typeLabel: oam.WorkloadTypeLabel, + exp: "deploy", + }, } for name, ti := range tests { - got := util.GetDefinitionName(ti.u) + got := util.GetDefinitionName(ti.u, ti.typeLabel) By(fmt.Sprint("Running test: ", name)) - Expect(ti.exp).Should(Equal(got)) + Expect(got).Should(Equal(ti.exp)) } }) }) diff --git a/pkg/webhook/v1alpha2/admit.go b/pkg/webhook/v1alpha2/admit.go index ca7cb374..c6925030 100644 --- a/pkg/webhook/v1alpha2/admit.go +++ b/pkg/webhook/v1alpha2/admit.go @@ -9,7 +9,8 @@ import ( // Add will be called in main and register all validation handlers func Add(mgr manager.Manager) { - applicationconfiguration.Register(mgr) + applicationconfiguration.RegisterValidatingHandler(mgr) + applicationconfiguration.RegisterMutatingHandler(mgr) component.RegisterMutatingHandler(mgr) component.RegisterValidatingHandler(mgr) } diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/handler_test.go b/pkg/webhook/v1alpha2/applicationconfiguration/handler_test.go new file mode 100644 index 00000000..3f51ebbe --- /dev/null +++ b/pkg/webhook/v1alpha2/applicationconfiguration/handler_test.go @@ -0,0 +1,306 @@ +package applicationconfiguration + +import ( + "context" + "fmt" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "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" + "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" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" +) + +var _ = Describe("ApplicationConfiguration Admission controller Test", func() { + var appConfig v1alpha2.ApplicationConfiguration + var appConfigName, namespace string + var label map[string]string + BeforeEach(func() { + namespace = "appconfig-test" + label = map[string]string{"test": "test-appConfig"} + // Create a appConfig definition + appConfigName = "example-app" + appConfig = v1alpha2.ApplicationConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: appConfigName, + Namespace: namespace, + Labels: label, + }, + Spec: v1alpha2.ApplicationConfigurationSpec{ + Components: []v1alpha2.ApplicationConfigurationComponent{ + { + ComponentName: "example-comp", + Traits: make([]v1alpha2.ComponentTrait, 1), + }, + }, + }, + } + }) + + Context("Test Mutation Webhook", func() { + var handler admission.Handler = &MutatingHandler{} + var traitDef v1alpha2.TraitDefinition + var traitTypeName = "test-trait" + var baseTrait unstructured.Unstructured + + BeforeEach(func() { + decoderInjector := handler.(admission.DecoderInjector) + decoderInjector.InjectDecoder(decoder) + // define workloadDefinition + traitDef = v1alpha2.TraitDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: traitTypeName, + Labels: label, + }, + Spec: v1alpha2.TraitDefinitionSpec{ + Reference: v1alpha2.DefinitionReference{ + Name: "foos.example.com", + }, + }, + } + // the base trait + baseTrait = unstructured.Unstructured{} + baseTrait.SetAPIVersion("example.com/v1") + baseTrait.SetKind("Foo") + baseTrait.SetName("traitName") + unstructured.SetNestedField(baseTrait.Object, "test", "spec", "key") + Expect(len(crd.Spec.Versions)).Should(Equal(1)) + Expect(appConfig.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()) + }) + + It("Test noop mutate admission handle", func() { + appConfig.Spec.Components[0].Traits[0].Trait = runtime.RawExtension{Raw: util.JSONMarshal(baseTrait)} + + req := admission.Request{ + AdmissionRequest: admissionv1beta1.AdmissionRequest{ + Operation: admissionv1beta1.Create, + Resource: reqResource, + Object: runtime.RawExtension{Raw: util.JSONMarshal(appConfig)}, + }, + } + resp := handler.Handle(context.TODO(), req) + Expect(resp.Allowed).Should(BeTrue()) + }) + + It("Test mutate function", func() { + // the trait that uses type to refer to the traitDefinition + traitWithType := unstructured.Unstructured{} + typeContent := make(map[string]interface{}) + typeContent[TraitTypeField] = traitTypeName + typeContent[TraitSpecField] = map[string]interface{}{ + "key": "test", + } + traitWithType.SetUnstructuredContent(typeContent) + traitWithType.SetName("traitName") + traitWithType.SetLabels(label) + // set up the bad type + traitWithBadType := traitWithType.DeepCopy() + traitWithBadType.Object[TraitTypeField] = traitDef + // set up the result + mutatedTrait := baseTrait.DeepCopy() + mutatedTrait.SetNamespace(appConfig.GetNamespace()) + mutatedTrait.SetLabels(util.MergeMap(label, map[string]string{oam.TraitTypeLabel: traitTypeName})) + tests := map[string]struct { + client client.Client + trait interface{} + errMsg string + wanted []byte + }{ + "bad trait case": { + trait: "bad trait", + errMsg: "cannot unmarshal string", + }, + "bad trait type case": { + trait: traitWithBadType, + errMsg: "name of trait should be string instead of map[string]interface {}", + }, + "no op case": { + trait: baseTrait, + wanted: util.JSONMarshal(baseTrait), + }, + "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.TraitDefinition: + return fmt.Errorf("does not exist") + } + return nil + }, + }, + trait: traitWithType.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.TraitDefinition: + Expect(key.Name).Should(BeEquivalentTo(typeContent[TraitTypeField])) + *o = traitDef + case *crdv1.CustomResourceDefinition: + Expect(key.Name).Should(Equal(traitDef.Spec.Reference.Name)) + *o = crd + } + return nil + }, + }, + trait: traitWithType.DeepCopyObject(), + wanted: util.JSONMarshal(mutatedTrait), + }, + } + for testCase, test := range tests { + By(fmt.Sprintf("start test : %s", testCase)) + appConfig.Spec.Components[0].Traits[0].Trait = runtime.RawExtension{Raw: util.JSONMarshal(test.trait)} + injc := handler.(inject.Client) + injc.InjectClient(test.client) + mutatingHandler := handler.(*MutatingHandler) + err := mutatingHandler.Mutate(&appConfig) + if len(test.errMsg) == 0 { + Expect(err).Should(BeNil()) + Expect(appConfig.Spec.Components[0].Traits[0].Trait.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 trait") + validTrait := unstructured.Unstructured{} + validTrait.SetAPIVersion("validAPI") + validTrait.SetKind("validKind") + By("Creating invalid trait with type") + traitWithType := validTrait.DeepCopy() + typeContent := make(map[string]interface{}) + typeContent[TraitTypeField] = "should not be here" + traitWithType.SetUnstructuredContent(typeContent) + By("Creating invalid trait without kind") + noKindTrait := validTrait.DeepCopy() + noKindTrait.SetKind("") + var traitTypeName = "test-trait" + traitDef := v1alpha2.TraitDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: traitTypeName, + Labels: label, + }, + Spec: v1alpha2.TraitDefinitionSpec{ + Reference: v1alpha2.DefinitionReference{ + Name: "foos.example.com", + }, + }, + } + clientInstance := &test.MockClient{ + MockGet: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { + switch o := obj.(type) { + case *v1alpha2.TraitDefinition: + *o = traitDef + case *crdv1.CustomResourceDefinition: + Expect(key.Name).Should(Equal(traitDef.Spec.Reference.Name)) + *o = crd + } + return nil + }, + } + tests := map[string]struct { + trait interface{} + client client.Client + operation admissionv1beta1.Operation + pass bool + reason string + }{ + "valid create case": { + trait: validTrait.DeepCopyObject(), + operation: admissionv1beta1.Create, + pass: true, + reason: "", + client: clientInstance, + }, + "valid update case": { + trait: validTrait.DeepCopyObject(), + operation: admissionv1beta1.Update, + pass: true, + reason: "", + client: clientInstance, + }, + "malformat appConfig": { + trait: "bad format", + operation: admissionv1beta1.Create, + pass: false, + reason: "the trait is malformed", + client: clientInstance, + }, + "trait still has type": { + trait: traitWithType.DeepCopyObject(), + operation: admissionv1beta1.Create, + pass: false, + reason: "the trait contains 'name' info", + client: clientInstance, + }, + "no kind trait appConfig": { + trait: noKindTrait.DeepCopyObject(), + operation: admissionv1beta1.Update, + pass: false, + reason: "the trait data missing GVK", + client: clientInstance, + }, + } + for testCase, test := range tests { + By(fmt.Sprintf("start test : %s", testCase)) + appConfig.Spec.Components[0].Traits[0].Trait = runtime.RawExtension{Raw: util.JSONMarshal(test.trait)} + req := admission.Request{ + AdmissionRequest: admissionv1beta1.AdmissionRequest{ + Operation: test.operation, + Resource: reqResource, + Object: runtime.RawExtension{Raw: util.JSONMarshal(appConfig)}, + }, + } + injc := handler.(inject.Client) + injc.InjectClient(test.client) + 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/applicationconfiguration/helper.go b/pkg/webhook/v1alpha2/applicationconfiguration/helper.go index 56f61582..168e9c0c 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/helper.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/helper.go @@ -41,7 +41,7 @@ func checkComponentVersionEnabled(ctx context.Context, client client.Reader, acc return false, nil } -// checkParams will check whethter exist parameter assigning value to workload name +// checkParams will check whethter exist parameter assigning value to trait name func checkParams(cp []v1alpha2.ComponentParameter, cpv []v1alpha2.ComponentParameterValue) (bool, string) { targetParams := make(map[string]bool) for _, v := range cp { @@ -54,7 +54,7 @@ func checkParams(cp []v1alpha2.ComponentParameter, cpv []v1alpha2.ComponentParam } for _, v := range cpv { if targetParams[v.Name] { - // check fails if get parameter to overwrite workload name + // check fails if get parameter to overwrite trait name return false, v.Value.StrVal } } diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/helper_test.go b/pkg/webhook/v1alpha2/applicationconfiguration/helper_test.go index 2043d52e..202b2318 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/helper_test.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/helper_test.go @@ -147,7 +147,7 @@ func TestCheckParams(t *testing.T) { expectParamValue: wlNameValue, }, { - caseName: "not found workload name params", + caseName: "not found trait name params", cps: []v1alpha2.ComponentParameter{mockParam}, cpvs: []v1alpha2.ComponentParameterValue{mockParamValue}, expectResult: true, diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/mutating_handler.go b/pkg/webhook/v1alpha2/applicationconfiguration/mutating_handler.go new file mode 100644 index 00000000..d6537cb6 --- /dev/null +++ b/pkg/webhook/v1alpha2/applicationconfiguration/mutating_handler.go @@ -0,0 +1,174 @@ +package applicationconfiguration + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "reflect" + + "github.com/davecgh/go-spew/spew" + 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/types" + "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" + + "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" +) + +const ( + // TraitTypeField is the special field indicate the type of the traitDefinition + TraitTypeField = "name" + // TraitSpecField indicate the spec of the trait in ApplicationConfiguration + TraitSpecField = "properties" +) + +// MutatingHandler handles Component +type MutatingHandler struct { + Client client.Client + + // Decoder decodes objects + Decoder *admission.Decoder +} + +// log is for logging in this package. +var mutatelog = logf.Log.WithName("applicationconfiguration mutate webhook") + +var _ admission.Handler = &MutatingHandler{} + +// Handle handles admission requests. +func (h *MutatingHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + obj := &v1alpha2.ApplicationConfiguration{} + + err := h.Decoder.Decode(req, obj) + if err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + // mutate the object + if err := h.Mutate(obj); err != nil { + mutatelog.Error(err, "failed to mutate the applicationConfiguration", "name", obj.Name) + return admission.Errored(http.StatusBadRequest, err) + } + mutatelog.Info("Print the mutated obj", "obj name", obj.Name, "mutated obj", spew.Sdump(obj.Spec)) + + marshalled, err := json.Marshal(obj) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + resp := admission.PatchResponseFromRaw(req.AdmissionRequest.Object.Raw, marshalled) + if len(resp.Patches) > 0 { + mutatelog.Info("admit ApplicationConfiguration", + "namespace", obj.Namespace, "name", obj.Name, "patches", util.JSONMarshal(resp.Patches)) + } + return resp +} + +// Mutate sets all the default value for the Component +func (h *MutatingHandler) Mutate(obj *v1alpha2.ApplicationConfiguration) error { + mutatelog.Info("mutate", "name", obj.Name) + + for compIdx, comp := range obj.Spec.Components { + var updated bool + for idx, tr := range comp.Traits { + var content map[string]interface{} + if err := json.Unmarshal(tr.Trait.Raw, &content); err != nil { + return err + } + rawByte, mutated, err := h.mutateTrait(content, comp.ComponentName, obj.GetNamespace()) + if err != nil { + return err + } + if !mutated { + continue + } + tr.Trait.Raw = rawByte + comp.Traits[idx] = tr + updated = true + } + if updated { + obj.Spec.Components[compIdx] = comp + } + } + + return nil +} + +func (h *MutatingHandler) mutateTrait(content map[string]interface{}, compName, namespace string) ([]byte, bool, error) { + if content[TraitTypeField] == nil { + return nil, false, nil + } + traitType, ok := content[TraitTypeField].(string) + if !ok { + return nil, false, fmt.Errorf("name of trait should be string instead of %s", reflect.TypeOf(content[TraitTypeField])) + } + mutatelog.Info("the trait refers to traitDefinition by name", "compName", compName, "trait name", traitType) + // Fetch the corresponding traitDefinition CR, the traitDefinition crd is cluster scoped + traitDefinition := &v1alpha2.TraitDefinition{} + if err := h.Client.Get(context.TODO(), types.NamespacedName{Name: traitType}, traitDefinition); err != nil { + return nil, false, err + } + // fetch the CRDs definition + customResourceDefinition := &crdv1.CustomResourceDefinition{} + if err := h.Client.Get(context.TODO(), types.NamespacedName{Name: traitDefinition.Spec.Reference.Name}, customResourceDefinition); err != nil { + return nil, false, err + } + // reconstruct the trait CR + delete(content, TraitTypeField) + + if content[TraitSpecField] != nil { + content["spec"] = content[TraitSpecField] + delete(content, TraitSpecField) + } + + trait := unstructured.Unstructured{ + Object: content, + } + // find out the GVK from the CRD definition and set + apiVersion := metav1.GroupVersion{ + Group: customResourceDefinition.Spec.Group, + Version: customResourceDefinition.Spec.Versions[0].Name, + }.String() + trait.SetAPIVersion(apiVersion) + trait.SetKind(customResourceDefinition.Spec.Names.Kind) + mutatelog.Info("Set the trait GVK", "trait api version", trait.GetAPIVersion(), "trait Kind", trait.GetKind()) + // copy namespace/label/annotation to the trait and add traitType label + trait.SetNamespace(namespace) + trait.SetLabels(util.MergeMap(trait.GetLabels(), map[string]string{oam.TraitTypeLabel: traitType})) + // copy back the object + rawBye, err := json.Marshal(trait.Object) + if err != nil { + return nil, false, err + } + return rawBye, true, nil +} + +var _ inject.Client = &MutatingHandler{} + +// InjectClient injects the client into the ComponentMutatingHandler +func (h *MutatingHandler) InjectClient(c client.Client) error { + h.Client = c + return nil +} + +var _ admission.DecoderInjector = &MutatingHandler{} + +// InjectDecoder injects the decoder into the ComponentMutatingHandler +func (h *MutatingHandler) InjectDecoder(d *admission.Decoder) error { + h.Decoder = d + return nil +} + +// RegisterMutatingHandler will register component mutation handler to the webhook +func RegisterMutatingHandler(mgr manager.Manager) { + server := mgr.GetWebhookServer() + server.Register("/mutating-core-oam-dev-v1alpha2-applicationconfigurations", &webhook.Admission{Handler: &MutatingHandler{}}) +} diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/suite_test.go b/pkg/webhook/v1alpha2/applicationconfiguration/suite_test.go new file mode 100644 index 00000000..7f18ac26 --- /dev/null +++ b/pkg/webhook/v1alpha2/applicationconfiguration/suite_test.go @@ -0,0 +1,91 @@ +package applicationconfiguration + +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 TestApplicationConfigurationWebHandler(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "ApplicationConfiguration Web handler") +} + +var _ = BeforeSuite(func(done Done) { + By("Bootstrapping test environment") + ctrl.SetLogger(zap.New(func(o *zap.Options) { + o.Development = true + o.DestWritter = os.Stdout + })) + By("Setup scheme") + err := core.AddToScheme(scheme) + Expect(err).Should(BeNil()) + err = clientgoscheme.AddToScheme(scheme) + Expect(err).Should(BeNil()) + // the crd we will refer to + crd = crdv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo.example.com", + }, + 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{ + "spec": { + Type: "object", + Properties: map[string]crdv1.JSONSchemaProps{ + "key": {Type: "string"}, + }, + }, + "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: "applicationconfigurations"} + 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/applicationconfiguration/applicationconfiguration.go b/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler.go similarity index 65% rename from pkg/webhook/v1alpha2/applicationconfiguration/applicationconfiguration.go rename to pkg/webhook/v1alpha2/applicationconfiguration/validating_handler.go index c2e7129d..b6b2853b 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/applicationconfiguration.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler.go @@ -6,6 +6,9 @@ import ( "fmt" "net/http" + apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" + "k8s.io/apimachinery/pkg/util/validation/field" + "github.com/crossplane/crossplane-runtime/pkg/fieldpath" "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" @@ -22,13 +25,13 @@ import ( ) const ( - reasonFmtWorkloadNameNotEmpty = "Versioning-enabled component's workload name MUST NOT be assigned. Expect workload name %q to be empty." + reasonFmtWorkloadNameNotEmpty = "Versioning-enabled component's trait name MUST NOT be assigned. Expect trait name %q to be empty." - errFmtCheckWorkloadName = "Error occurs when checking workload name. %q" + errFmtCheckWorkloadName = "Error occurs when checking trait name. %q" - errFmtUnmarshalWorkload = "Error occurs when unmarshal workload of component %q error: %q" + errFmtUnmarshalWorkload = "Error occurs when unmarshal trait of component %q error: %q" - // WorkloadNamePath indicates field path of workload name + // WorkloadNamePath indicates field path of trait name WorkloadNamePath = "metadata.name" ) @@ -65,18 +68,56 @@ func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) a if err != nil { return admission.Errored(http.StatusBadRequest, err) } + if allErrs := ValidateTraitObject(obj); len(allErrs) > 0 { + klog.Info("create or update failed", "name", obj.Name, "errMsg", allErrs.ToAggregate().Error()) + return admission.Denied(allErrs.ToAggregate().Error()) + } if pass, reason := checkRevisionName(obj); !pass { return admission.ValidationResponse(false, reason) } - // TODO(wonderflow): Add more validation logic here. - if pass, reason := checkWorkloadNameForVersioning(ctx, h.Client, obj); !pass { return admission.ValidationResponse(false, reason) } + // TODO(wonderflow): Add more validation logic here. } return admission.ValidationResponse(true, "") } +// ValidateTraitObject validates the ApplicationConfiguration on creation/update +func ValidateTraitObject(obj *v1alpha2.ApplicationConfiguration) field.ErrorList { + klog.Info("validate applicationConfiguration", "name", obj.Name) + allErrs := apimachineryvalidation.ValidateObjectMeta(&obj.ObjectMeta, true, + apimachineryvalidation.NameIsDNSSubdomain, field.NewPath("metadata")) + for cidx, comp := range obj.Spec.Components { + for idx, tr := range comp.Traits { + fldPath := field.NewPath("spec").Child("components").Index(cidx).Child("traits").Index(idx).Child("trait") + var content map[string]interface{} + if err := json.Unmarshal(tr.Trait.Raw, &content); err != nil { + allErrs = append(allErrs, field.Invalid(fldPath, string(tr.Trait.Raw), + "the trait is malformed")) + return allErrs + } + if content[TraitTypeField] != nil { + allErrs = append(allErrs, field.Invalid(fldPath, string(tr.Trait.Raw), + "the trait contains 'name' info that should be mutated to GVK")) + } + if content[TraitSpecField] != nil { + allErrs = append(allErrs, field.Invalid(fldPath, string(tr.Trait.Raw), + "the trait contains 'properties' info that should be mutated to spec")) + } + trait := unstructured.Unstructured{ + Object: content, + } + if len(trait.GetAPIVersion()) == 0 || len(trait.GetKind()) == 0 { + allErrs = append(allErrs, field.Invalid(fldPath, content, + fmt.Sprintf("the trait data missing GVK, api = %s, kind = %s,", trait.GetAPIVersion(), trait.GetKind()))) + } + } + } + + return allErrs +} + func checkRevisionName(appConfig *v1alpha2.ApplicationConfiguration) (bool, string) { for _, v := range appConfig.Spec.Components { if v.ComponentName != "" && v.RevisionName != "" { @@ -86,7 +127,7 @@ func checkRevisionName(appConfig *v1alpha2.ApplicationConfiguration) (bool, stri return true, "" } -// checkWorkloadNameForVersioning check whether versioning-enabled component workload name is empty +// checkWorkloadNameForVersioning check whether versioning-enabled component trait name is empty func checkWorkloadNameForVersioning(ctx context.Context, client client.Reader, appConfig *v1alpha2.ApplicationConfiguration) (bool, string) { for _, v := range appConfig.Spec.Components { acc := v @@ -136,8 +177,8 @@ func (h *ValidatingHandler) InjectDecoder(d *admission.Decoder) error { return nil } -// Register will register application configuration validation to webhook -func Register(mgr manager.Manager) { +// RegisterValidatingHandler will register application configuration validation to webhook +func RegisterValidatingHandler(mgr manager.Manager) { server := mgr.GetWebhookServer() server.Register("/validating-core-oam-dev-v1alpha2-applicationconfigurations", &webhook.Admission{Handler: &ValidatingHandler{}}) } diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/applicationconfiguration_test.go b/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler_test.go similarity index 91% rename from pkg/webhook/v1alpha2/applicationconfiguration/applicationconfiguration_test.go rename to pkg/webhook/v1alpha2/applicationconfiguration/validating_handler_test.go index b64aa55a..0fdf7d1a 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/applicationconfiguration_test.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler_test.go @@ -57,17 +57,27 @@ func TestApplicationConfigurationValidation(t *testing.T) { dec, _ := admission.NewDecoder(scheme) decoder.InjectDecoder(dec) - app1, _ := json.Marshal(v1alpha2.ApplicationConfiguration{Spec: v1alpha2.ApplicationConfigurationSpec{Components: []v1alpha2.ApplicationConfigurationComponent{ - { - RevisionName: "r1", - ComponentName: "c1", + app1, _ := json.Marshal(v1alpha2.ApplicationConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test-ns", }, - }}}) - app2, _ := json.Marshal(v1alpha2.ApplicationConfiguration{Spec: v1alpha2.ApplicationConfigurationSpec{Components: []v1alpha2.ApplicationConfigurationComponent{ - { - RevisionName: "r1", + Spec: v1alpha2.ApplicationConfigurationSpec{Components: []v1alpha2.ApplicationConfigurationComponent{ + { + RevisionName: "r1", + ComponentName: "c1", + }, + }}}) + app2, _ := json.Marshal(v1alpha2.ApplicationConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test-ns", }, - }}}) + Spec: v1alpha2.ApplicationConfigurationSpec{Components: []v1alpha2.ApplicationConfigurationComponent{ + { + RevisionName: "r1", + }, + }}}) tests := []struct { req admission.Request @@ -150,7 +160,7 @@ func TestCheckWorkloadNameForVersioning(t *testing.T) { expectReason string }{ { - caseName: "Test validation fails for workload name fixed in component", + caseName: "Test validation fails for trait name fixed in component", appConfig: v1alpha2.ApplicationConfiguration{ Spec: v1alpha2.ApplicationConfigurationSpec{ Components: []v1alpha2.ApplicationConfigurationComponent{ @@ -175,7 +185,7 @@ func TestCheckWorkloadNameForVersioning(t *testing.T) { expectReason: fmt.Sprintf(reasonFmtWorkloadNameNotEmpty, workloadName), }, { - caseName: "Test validation fails for workload name assigned by parameter", + caseName: "Test validation fails for trait name assigned by parameter", appConfig: v1alpha2.ApplicationConfiguration{ Spec: v1alpha2.ApplicationConfigurationSpec{ Components: []v1alpha2.ApplicationConfigurationComponent{ @@ -322,7 +332,7 @@ func TestCheckWorkloadNameForVersioning(t *testing.T) { return nil }, expectResult: false, - expectReason: "Error occurs when checking workload name. \"cannot get component \\\"c\\\": get error\"", + expectReason: "Error occurs when checking trait name. \"cannot get component \\\"c\\\": get error\"", }, { caseName: "Test unmarshalWorkload error occurs during validation", @@ -360,7 +370,7 @@ func TestCheckWorkloadNameForVersioning(t *testing.T) { return nil }, expectResult: false, - expectReason: "Error occurs when unmarshal workload of component \"\" error: \"unexpected end of JSON input\"", + expectReason: "Error occurs when unmarshal trait of component \"\" error: \"unexpected end of JSON input\"", }, } for _, tc := range tests {