Merge pull request #310 from captainroy-hy/trait-apply-to

implement trait.ApplyTo through AppConfig validating webhook
crossplane · Nov 25, 2020 · 4b190d1 · 4b190d1
2 parents 7c702eb + b0f5416
commit 4b190d1
Show file tree

Hide file tree

Showing 6 changed files with 539 additions and 565 deletions.
diff --git a/go.mod b/go.mod
@@ -13,7 +13,6 @@ require (
 	github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6 // indirect
 	github.com/google/go-cmp v0.4.0
 	github.com/gophercloud/gophercloud v0.6.0 // indirect
-	github.com/json-iterator/go v1.1.10
 	github.com/kr/pretty v0.2.0 // indirect
 	github.com/onsi/ginkgo v1.12.1
 	github.com/onsi/gomega v1.10.1

diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/handler_test.go b/pkg/webhook/v1alpha2/applicationconfiguration/handler_test.go
@@ -194,113 +194,88 @@ var _ = Describe("ApplicationConfiguration Admission controller Test", func() {
 		var handler admission.Handler = &ValidatingHandler{Mapper: mapper}
 		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,
+
+		testWorkload := unstructured.Unstructured{}
+		testWorkload.SetAPIVersion("example.com/v1")
+		testWorkload.SetKind("TestWorkload")
+
+		testComponent := v1alpha2.Component{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: "example.com/v1",
+				Kind:       "TestComponent",
+			},
+			Spec: v1alpha2.ComponentSpec{
+				Workload: runtime.RawExtension{
+					Raw: util.JSONMarshal(testWorkload.Object),
+				},
+			},
+			Status: v1alpha2.ComponentStatus{
+				LatestRevision: &v1alpha2.Revision{
+					Name: "example-comp-v1",
+				},
+			},
+		}
+
+		testWorkloadDef := v1alpha2.WorkloadDefinition{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: "example.com/v1",
+				Kind:       "TestWorkload",
 			},
-			Spec: v1alpha2.TraitDefinitionSpec{
-				Reference: v1alpha2.DefinitionReference{
-					Name: "foos.example.com",
+		}
+		testTrait := unstructured.Unstructured{}
+		testTrait.SetAPIVersion("example.com/v1")
+		testTrait.SetKind("TestTrait")
+		appConfig.Spec.Components[0] = v1alpha2.ApplicationConfigurationComponent{
+			ComponentName: "example-comp",
+			Traits: []v1alpha2.ComponentTrait{
+				{
+					Trait: runtime.RawExtension{Raw: util.JSONMarshal(testTrait.Object)},
 				},
 			},
 		}
+		testTraitDef := v1alpha2.TraitDefinition{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: "example.com/v1",
+				Kind:       "TestTrait",
+			},
+		}
+
 		clientInstance := &test.MockClient{
 			MockGet: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error {
 				switch o := obj.(type) {
+				case *v1alpha2.Component:
+					*o = testComponent
+				case *v1alpha2.WorkloadDefinition:
+					*o = testWorkloadDef
 				case *v1alpha2.TraitDefinition:
-					*o = traitDef
-				case *crdv1.CustomResourceDefinition:
-					Expect(key.Name).Should(Equal(traitDef.Spec.Reference.Name))
-					*o = crd
+					*o = testTraitDef
 				}
 				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,
+
+		req := admission.Request{
+			AdmissionRequest: admissionv1beta1.AdmissionRequest{
+				Operation: admissionv1beta1.Create,
+				Resource:  reqResource,
+				Object:    runtime.RawExtension{Raw: util.JSONMarshal(appConfig)},
 			},
 		}
-		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))
-			}
-		}
+		injc := handler.(inject.Client)
+		injc.InjectClient(clientInstance)
+		resp := handler.Handle(context.TODO(), req)
+		By(string(resp.Result.Reason))
+		Expect(resp.Allowed).Should(BeTrue())
+
 		By("Test bad admission request format")
-		req := admission.Request{
+		req = admission.Request{
 			AdmissionRequest: admissionv1beta1.AdmissionRequest{
 				Operation: admissionv1beta1.Create,
 				Resource:  reqResource,
 				Object:    runtime.RawExtension{Raw: []byte("bad request")},
 			},
 		}
-		resp := handler.Handle(context.TODO(), req)
+		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
@@ -6,7 +6,6 @@ import (
 	"strings"
 
 	"github.com/pkg/errors"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -16,32 +15,100 @@ import (
 )
 
 const (
-	errUnmarshalTrait        = "cannot unmarshal trait"
-	errFmtGetTraitDefinition = "cannot find trait definition %q %q %q"
+	errFmtGetComponent          = "cannot get component %q"
+	errFmtGetTraitDefinition    = "cannot get trait definition in component %q"
+	errFmtUnmarshalWorkload     = "cannot unmarshal workload of component %q"
+	errFmtUnmarshalTrait        = "cannot unmarshal trait of component %q"
+	errFmtGetWorkloadDefinition = "cannot get workload definition of component %q"
 )
 
-// checkComponentVersionEnabled check whethter a component is versioning mechanism enabled
-func checkComponentVersionEnabled(ctx context.Context, client client.Reader, dm discoverymapper.DiscoveryMapper,
-	acc *v1alpha2.ApplicationConfigurationComponent) (bool, error) {
-	if acc.RevisionName != "" {
-		return true, nil
-	}
-	for _, ct := range acc.Traits {
-		ut := &unstructured.Unstructured{}
-		if err := json.Unmarshal(ct.Trait.Raw, ut); err != nil {
-			return false, errors.Wrap(err, errUnmarshalTrait)
+// ValidatingAppConfig is used for validating ApplicationConfiguration
+type ValidatingAppConfig struct {
+	appConfig       v1alpha2.ApplicationConfiguration
+	validatingComps []ValidatingComponent
+}
+
+// ValidatingComponent is used for validatiing ApplicationConfigurationComponent
+type ValidatingComponent struct {
+	appConfigComponent v1alpha2.ApplicationConfigurationComponent
+
+	// below data is convenient for validation
+	compName           string
+	component          v1alpha2.Component
+	workloadDefinition v1alpha2.WorkloadDefinition
+	workloadContent    unstructured.Unstructured
+	validatingTraits   []ValidatingTrait
+}
+
+// ValidatingTrait is used for validating Trait
+type ValidatingTrait struct {
+	componentTrait v1alpha2.ComponentTrait
+
+	// below data is convenient for validation
+	traitDefinition v1alpha2.TraitDefinition
+	traitContent    unstructured.Unstructured
+}
+
+// PrepareForValidation prepares data for validations to avoiding repetitive GET/unmarshal operations
+func (v *ValidatingAppConfig) PrepareForValidation(ctx context.Context, c client.Reader, dm discoverymapper.DiscoveryMapper, ac *v1alpha2.ApplicationConfiguration) error {
+	v.appConfig = *ac
+	v.validatingComps = make([]ValidatingComponent, 0, len(ac.Spec.Components))
+	for _, acc := range ac.Spec.Components {
+		tmp := ValidatingComponent{}
+		tmp.appConfigComponent = acc
+
+		if acc.ComponentName != "" {
+			tmp.compName = acc.ComponentName
+		} else {
+			tmp.compName = acc.RevisionName
 		}
-		td, err := util.FetchTraitDefinition(ctx, client, dm, ut)
-		if err != nil && !apierrors.IsNotFound(err) {
-			return false, errors.Wrapf(err, errFmtGetTraitDefinition, ut.GetAPIVersion(), ut.GetKind(), ut.GetName())
+		comp, _, err := util.GetComponent(ctx, c, acc, ac.Namespace)
+		if err != nil {
+			return errors.Wrapf(err, errFmtGetComponent, tmp.compName)
 		}
-		if td.Spec.RevisionEnabled {
-			// if any traitDefinition's RevisionEnabled is true
-			// then the component is versioning enabled
-			return true, nil
+		tmp.component = *comp
+
+		// get worload content from raw
+		var wlContentObject map[string]interface{}
+		if err := json.Unmarshal(comp.Spec.Workload.Raw, &wlContentObject); err != nil {
+			return errors.Wrapf(err, errFmtUnmarshalWorkload, tmp.compName)
+		}
+		wl := unstructured.Unstructured{
+			Object: wlContentObject,
+		}
+		tmp.workloadContent = wl
+
+		// get workload definition
+		wlDef, err := util.FetchWorkloadDefinition(ctx, c, dm, &wl)
+		if err != nil {
+			return errors.Wrapf(err, errFmtGetWorkloadDefinition, tmp.compName)
+		}
+		tmp.workloadDefinition = *wlDef
+
+		tmp.validatingTraits = make([]ValidatingTrait, 0, len(acc.Traits))
+		for _, t := range acc.Traits {
+			tmpT := ValidatingTrait{}
+			tmpT.componentTrait = t
+			// get trait content from raw
+			var tContentObject map[string]interface{}
+			if err := json.Unmarshal(t.Trait.Raw, &tContentObject); err != nil {
+				return errors.Wrapf(err, errFmtUnmarshalTrait, tmp.compName)
+			}
+			tContent := unstructured.Unstructured{
+				Object: tContentObject,
+			}
+			// get trait definition
+			tDef, err := util.FetchTraitDefinition(ctx, c, dm, &tContent)
+			if err != nil {
+				return errors.Wrapf(err, errFmtGetTraitDefinition, tmp.compName)
+			}
+			tmpT.traitContent = tContent
+			tmpT.traitDefinition = *tDef
+			tmp.validatingTraits = append(tmp.validatingTraits, tmpT)
 		}
+		v.validatingComps = append(v.validatingComps, tmp)
 	}
-	return false, nil
+	return nil
 }
 
 // checkParams will check whether exist parameter assigning value to workload name