diff --git a/go.mod b/go.mod index ad39ec95..16c1d350 100644 --- 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 index 3627ae9b..8beebe5a 100644 --- 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 index 67db0170..b981d1be 100644 --- 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 diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/helper_test.go b/pkg/webhook/v1alpha2/applicationconfiguration/helper_test.go index 3234c119..740d1ffc 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/helper_test.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/helper_test.go @@ -1,116 +1,16 @@ package applicationconfiguration import ( - "context" "fmt" "testing" - "github.com/crossplane/crossplane-runtime/pkg/test" "github.com/stretchr/testify/assert" - json "github.com/json-iterator/go" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" - "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock" ) -func TestCheckComponentVersionEnabled(t *testing.T) { - ctx := context.Background() - mockClient := test.NewMockClient() - traitRevisionEnabled, _ := json.Marshal(v1alpha2.ManualScalerTrait{ - TypeMeta: v1.TypeMeta{ - Kind: "ManualScalerTrait", - APIVersion: "core.oam.dev", - }, - }) - - tests := []struct { - caseName string - mockGetFun test.MockGetFn - acc v1alpha2.ApplicationConfigurationComponent - result bool - }{ - { - caseName: "Versioning Disabled", - acc: v1alpha2.ApplicationConfigurationComponent{ - ComponentName: "compName", - }, - result: false, - }, - { - caseName: "Versioning Enabled With RevisionName", - acc: v1alpha2.ApplicationConfigurationComponent{ - RevisionName: "revisionName", - }, - result: true, - }, - { - caseName: "Versioning Enabled With RevisionEnabled Trait", - mockGetFun: func(_ context.Context, _ types.NamespacedName, obj runtime.Object) error { - if o, ok := obj.(*v1alpha2.TraitDefinition); ok { - *o = v1alpha2.TraitDefinition{ - Spec: v1alpha2.TraitDefinitionSpec{ - RevisionEnabled: true, - }, - } - return nil - } - return nil - }, - acc: v1alpha2.ApplicationConfigurationComponent{ - ComponentName: "compName", - Traits: []v1alpha2.ComponentTrait{ - { - Trait: runtime.RawExtension{ - Raw: traitRevisionEnabled, - }, - }, - }, - }, - result: true, - }, - { - caseName: "Unmarshal error occurs", - mockGetFun: func(_ context.Context, _ types.NamespacedName, obj runtime.Object) error { - if o, ok := obj.(*v1alpha2.TraitDefinition); ok { - *o = v1alpha2.TraitDefinition{ - Spec: v1alpha2.TraitDefinitionSpec{ - RevisionEnabled: true, - }, - } - return nil - } - return nil - }, - acc: v1alpha2.ApplicationConfigurationComponent{ - ComponentName: "compName", - Traits: []v1alpha2.ComponentTrait{ - { - Trait: runtime.RawExtension{ - Raw: nil, - }, - }, - }, - }, - result: false, - }, - } - mapper := mock.NewMockDiscoveryMapper() - - for _, tv := range tests { - func(t *testing.T) { - mockClient.MockGet = tv.mockGetFun - result, _ := checkComponentVersionEnabled(ctx, mockClient, mapper, &tv.acc) - assert.Equal(t, tv.result, result, fmt.Sprintf("Test case: %q", tv.caseName)) - }(t) - } - -} - func TestCheckParams(t *testing.T) { wlNameValue := "wlName" pName := "wlnameParam" diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler.go b/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler.go index cbb13daa..32b9b7b3 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler.go @@ -2,17 +2,16 @@ package applicationconfiguration import ( "context" - "encoding/json" "fmt" "net/http" + "strings" "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/discoverymapper" - "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" - "github.com/crossplane/crossplane-runtime/pkg/fieldpath" admissionv1beta1 "k8s.io/api/admission/v1beta1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/klog" "sigs.k8s.io/controller-runtime/pkg/client" @@ -23,11 +22,11 @@ import ( ) const ( - reasonFmtWorkloadNameNotEmpty = "Versioning-enabled component's workload name MUST NOT be assigned. Expect workload name %q to be empty." + errFmtWorkloadNameNotEmpty = "versioning-enabled component's workload name MUST NOT be assigned, expect workload name %q to be empty." - errFmtCheckWorkloadName = "Error occurs when checking workload name. %q" + errFmtRevisionName = "componentName %q and revisionName %q are mutually exclusive, you can only specify one of them" - errFmtUnmarshalWorkload = "Error occurs when unmarshal workload of component %q error: %q" + errFmtUnappliableTrait = "the trait %q cannot apply to workload %q of component %q (appliable: %q)" // WorkloadNamePath indicates field path of workload name WorkloadNamePath = "metadata.name" @@ -35,6 +34,19 @@ const ( var appConfigResource = v1alpha2.SchemeGroupVersion.WithResource("applicationconfigurations") +// AppConfigValidator provides functions to validate ApplicationConfiguration +type AppConfigValidator interface { + Validate(context.Context, ValidatingAppConfig) []error +} + +// AppConfigValidateFunc implements function to validate ApplicationConfiguration +type AppConfigValidateFunc func(context.Context, ValidatingAppConfig) []error + +// Validate validates ApplicationConfiguration +func (fn AppConfigValidateFunc) Validate(ctx context.Context, v ValidatingAppConfig) []error { + return fn(ctx, v) +} + // ValidatingHandler handles CloneSet type ValidatingHandler struct { Client client.Client @@ -42,6 +54,8 @@ type ValidatingHandler struct { // Decoder decodes objects Decoder *admission.Decoder + + Validators []AppConfigValidator } var _ admission.Handler = &ValidatingHandler{} @@ -67,97 +81,123 @@ 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()) + vAppConfig := &ValidatingAppConfig{} + if err := vAppConfig.PrepareForValidation(ctx, h.Client, h.Mapper, obj); err != nil { + klog.Info("failed init appConfig before validation ", " name: ", obj.Name, " errMsg: ", err.Error()) + return admission.Denied(err.Error()) } - if pass, reason := checkRevisionName(obj); !pass { - return admission.ValidationResponse(false, reason) - } - if pass, reason := checkWorkloadNameForVersioning(ctx, h.Client, h.Mapper, obj); !pass { - return admission.ValidationResponse(false, reason) + for _, validator := range h.Validators { + if allErrs := validator.Validate(ctx, *vAppConfig); utilerrors.NewAggregate(allErrs) != nil { + // utilerrors.NewAggregate can remove nil from allErrs + klog.Info("validation failed ", " name: ", obj.Name, " errMsgi: ", utilerrors.NewAggregate(allErrs).Error()) + return admission.Denied(utilerrors.NewAggregate(allErrs).Error()) + } } - // 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) +// ValidateTraitObjectFn validates the ApplicationConfiguration on creation/update +func ValidateTraitObjectFn(_ context.Context, v ValidatingAppConfig) []error { + klog.Info("validate applicationConfiguration", "name", v.appConfig.Name) var allErrs field.ErrorList - for cidx, comp := range obj.Spec.Components { - for idx, tr := range comp.Traits { + for cidx, comp := range v.validatingComps { + for idx, tr := range comp.validatingTraits { 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 - } + content := tr.traitContent.Object if content[TraitTypeField] != nil { - allErrs = append(allErrs, field.Invalid(fldPath, string(tr.Trait.Raw), + allErrs = append(allErrs, field.Invalid(fldPath, string(tr.componentTrait.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), + allErrs = append(allErrs, field.Invalid(fldPath, string(tr.componentTrait.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 { + if len(tr.traitContent.GetAPIVersion()) == 0 || len(tr.traitContent.GetKind()) == 0 { allErrs = append(allErrs, field.Invalid(fldPath, content, - fmt.Sprintf("the trait data missing GVK, api = %s, kind = %s,", trait.GetAPIVersion(), trait.GetKind()))) + fmt.Sprintf("the trait data missing GVK, api = %s, kind = %s,", + tr.traitContent.GetAPIVersion(), tr.traitContent.GetKind()))) } } } - - return allErrs + if len(allErrs) > 0 { + return allErrs.ToAggregate().Errors() + } + return nil } -func checkRevisionName(appConfig *v1alpha2.ApplicationConfiguration) (bool, string) { - for _, v := range appConfig.Spec.Components { - if v.ComponentName != "" && v.RevisionName != "" { - return false, "componentName and revisionName are mutually exclusive, you can only specify one of them" +// ValidateRevisionNameFn validates revisionName and componentName are assigned both. +func ValidateRevisionNameFn(_ context.Context, v ValidatingAppConfig) []error { + klog.Info("validate revisionName in applicationConfiguration", "name", v.appConfig.Name) + var allErrs []error + for _, c := range v.validatingComps { + if c.appConfigComponent.ComponentName != "" && c.appConfigComponent.RevisionName != "" { + allErrs = append(allErrs, fmt.Errorf(errFmtRevisionName, + c.appConfigComponent.ComponentName, c.appConfigComponent.RevisionName)) } } - return true, "" + return allErrs } -// checkWorkloadNameForVersioning check whether versioning-enabled component workload name is empty -func checkWorkloadNameForVersioning(ctx context.Context, client client.Reader, dm discoverymapper.DiscoveryMapper, - appConfig *v1alpha2.ApplicationConfiguration) (bool, string) { - for _, v := range appConfig.Spec.Components { - acc := v - vEnabled, err := checkComponentVersionEnabled(ctx, client, dm, &acc) - if err != nil { - return false, fmt.Sprintf(errFmtCheckWorkloadName, err.Error()) - } - if !vEnabled { - continue - } - c, _, err := util.GetComponent(ctx, client, acc, appConfig.GetNamespace()) - if err != nil { - return false, fmt.Sprintf(errFmtCheckWorkloadName, err.Error()) - } - - if ok, workloadName := checkParams(c.Spec.Parameters, acc.ParameterValues); !ok { - return false, fmt.Sprintf(reasonFmtWorkloadNameNotEmpty, workloadName) +// ValidateWorkloadNameForVersioningFn validates workload name for version-enabled component +func ValidateWorkloadNameForVersioningFn(_ context.Context, v ValidatingAppConfig) []error { + var allErrs []error + for _, c := range v.validatingComps { + isVersionEnabled := false + for _, t := range c.validatingTraits { + if t.traitDefinition.Spec.RevisionEnabled { + isVersionEnabled = true + break + } } - - w := &fieldpath.Paved{} - if err := json.Unmarshal(c.Spec.Workload.Raw, w); err != nil { - return false, fmt.Sprintf(errFmtUnmarshalWorkload, c.GetName(), err.Error()) + if isVersionEnabled { + if ok, workloadName := checkParams(c.component.Spec.Parameters, c.appConfigComponent.ParameterValues); !ok { + allErrs = append(allErrs, fmt.Errorf(errFmtWorkloadNameNotEmpty, workloadName)) + } + if workloadName := c.workloadContent.GetName(); workloadName != "" { + allErrs = append(allErrs, fmt.Errorf(errFmtWorkloadNameNotEmpty, workloadName)) + } } - workload := unstructured.Unstructured{Object: w.UnstructuredContent()} - workloadName := workload.GetName() + } + return allErrs +} - if len(workloadName) != 0 { - return false, fmt.Sprintf(reasonFmtWorkloadNameNotEmpty, workloadName) +// ValidateTraitAppliableToWorkloadFn validates whether a trait is allowed to apply to the workload. +func ValidateTraitAppliableToWorkloadFn(_ context.Context, v ValidatingAppConfig) []error { + klog.Info("validate trait is appliable to workload", "name", v.appConfig.Name) + var allErrs []error + for _, c := range v.validatingComps { + workloadType := c.component.GetLabels()[oam.WorkloadTypeLabel] + workloadDefRefName := c.workloadDefinition.Spec.Reference.Name + // TODO(roywang) consider a CRD group could have multiple versions + // and maybe we need to specify the minimum version here in the future + workloadGroup := c.workloadDefinition.GetObjectKind().GroupVersionKind().Group + ValidateApplyTo: + for _, t := range c.validatingTraits { + if len(t.traitDefinition.Spec.AppliesToWorkloads) == 0 { + // AppliesToWorkloads is empty, the trait can be applied to ANY workload + continue + } + for _, applyTo := range t.traitDefinition.Spec.AppliesToWorkloads { + if applyTo == "*" { + // "*" means the trait can be applied to ANY workload + continue ValidateApplyTo + } + if strings.HasPrefix(applyTo, "*.") && workloadGroup == applyTo[2:] { + continue ValidateApplyTo + } + if workloadType == applyTo || + workloadDefRefName == applyTo { + continue ValidateApplyTo + } + } + allErrs = append(allErrs, fmt.Errorf(errFmtUnappliableTrait, + t.traitDefinition.GetObjectKind().GroupVersionKind().String(), + c.workloadDefinition.GetObjectKind().GroupVersionKind().String(), + c.compName, t.traitDefinition.Spec.AppliesToWorkloads)) } } - return true, "" + return allErrs } var _ inject.Client = &ValidatingHandler{} @@ -185,6 +225,13 @@ func RegisterValidatingHandler(mgr manager.Manager) error { } server.Register("/validating-core-oam-dev-v1alpha2-applicationconfigurations", &webhook.Admission{Handler: &ValidatingHandler{ Mapper: mapper, + Validators: []AppConfigValidator{ + AppConfigValidateFunc(ValidateTraitObjectFn), + AppConfigValidateFunc(ValidateRevisionNameFn), + AppConfigValidateFunc(ValidateWorkloadNameForVersioningFn), + AppConfigValidateFunc(ValidateTraitAppliableToWorkloadFn), + // TODO(wonderflow): Add more validation logic here. + }, }}) return nil } diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler_test.go b/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler_test.go index 8bd09dfd..52439006 100644 --- a/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler_test.go +++ b/pkg/webhook/v1alpha2/applicationconfiguration/validating_handler_test.go @@ -5,194 +5,170 @@ import ( "fmt" "testing" - "github.com/pkg/errors" "github.com/stretchr/testify/assert" - "github.com/crossplane/oam-kubernetes-runtime/apis/core" "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" - "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" - "github.com/crossplane/crossplane-runtime/pkg/test" - json "github.com/json-iterator/go" - admissionv1beta1 "k8s.io/api/admission/v1beta1" - appsv1 "k8s.io/api/apps/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/intstr" - "sigs.k8s.io/controller-runtime/pkg/runtime/inject" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) -func TestApplicationConfigurationValidation(t *testing.T) { - var handler admission.Handler = &ValidatingHandler{} - - cwRaw, _ := json.Marshal(v1alpha2.ContainerizedWorkload{ - ObjectMeta: metav1.ObjectMeta{ - Name: "", - }, - }) +var ( + ctx = context.Background() +) - mgr := &mock.Manager{ - Client: &test.MockClient{ - MockGet: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { - o, _ := obj.(*appsv1.ControllerRevision) - *o = appsv1.ControllerRevision{ - Data: runtime.RawExtension{Object: &v1alpha2.Component{ - Spec: v1alpha2.ComponentSpec{ - Workload: runtime.RawExtension{ - Raw: cwRaw, - }, - }}}} - return nil +func TestValidateRevisionNameFn(t *testing.T) { + tests := []struct { + caseName string + validatingAppConfig ValidatingAppConfig + want []error + }{ + { + caseName: "componentName and revisionName are both assigned", + validatingAppConfig: ValidatingAppConfig{ + validatingComps: []ValidatingComponent{ + { + appConfigComponent: v1alpha2.ApplicationConfigurationComponent{ + ComponentName: "example-comp", + RevisionName: "example-comp-v1", + }, + }, + }, }, - }, - } - resource := metav1.GroupVersionResource{Group: "core.oam.dev", Version: "v1alpha2", Resource: "applicationconfigurations"} - injc := handler.(inject.Client) - injc.InjectClient(mgr.GetClient()) - decoder := handler.(admission.DecoderInjector) - var scheme = runtime.NewScheme() - _ = core.AddToScheme(scheme) - dec, _ := admission.NewDecoder(scheme) - decoder.InjectDecoder(dec) - - app1, _ := json.Marshal(v1alpha2.ApplicationConfiguration{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "test-ns", - }, - Spec: v1alpha2.ApplicationConfigurationSpec{Components: []v1alpha2.ApplicationConfigurationComponent{ - { - RevisionName: "r1", - ComponentName: "c1", + want: []error{ + fmt.Errorf(errFmtRevisionName, "example-comp", "example-comp-v1"), }, - }}}) - 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 - pass bool - reason string - }{ { - req: admission.Request{ - AdmissionRequest: admissionv1beta1.AdmissionRequest{ - Resource: resource, - Object: runtime.RawExtension{Raw: app1}, + caseName: "componentName is assigned", + validatingAppConfig: ValidatingAppConfig{ + validatingComps: []ValidatingComponent{ + { + appConfigComponent: v1alpha2.ApplicationConfigurationComponent{ + ComponentName: "example-comp", + }, + }, }, }, - pass: false, - reason: "componentName and revisionName are mutually exclusive, you can only specify one of them", + want: nil, }, { - req: admission.Request{ - AdmissionRequest: admissionv1beta1.AdmissionRequest{ - Resource: resource, - Object: runtime.RawExtension{Raw: app2}, + caseName: "revisionName is assigned", + validatingAppConfig: ValidatingAppConfig{ + validatingComps: []ValidatingComponent{ + { + appConfigComponent: v1alpha2.ApplicationConfigurationComponent{ + RevisionName: "example-comp-v1", + }, + }, }, }, - pass: true, + want: nil, }, } - for _, tv := range tests { - resp := handler.Handle(context.Background(), tv.req) - if tv.pass != resp.Allowed { - t.Errorf("expect %v but got %v from validation", tv.pass, resp.Allowed) - } - if tv.reason != "" { - if tv.reason != string(resp.Result.Reason) { - t.Errorf("\nvalidation should fail by reason: %v \ninstead of by reason: %v ", tv.reason, resp.Result.Reason) - } - } + + for _, tc := range tests { + result := ValidateRevisionNameFn(ctx, tc.validatingAppConfig) + assert.Equal(t, tc.want, result, fmt.Sprintf("Test case: %q", tc.caseName)) } } -func TestCheckWorkloadNameForVersioning(t *testing.T) { - ctx := context.Background() - mockClient := test.NewMockClient() +func TestValidateTraitObjectFn(t *testing.T) { + traitWithName := unstructured.Unstructured{ + Object: make(map[string]interface{}), + } + unstructured.SetNestedField(traitWithName.Object, "test", TraitTypeField) - revisionName := "r" - componentName := "c" - workloadName := "WorkloadName" - paramName := "workloadName" - paramValue := workloadName + traitWithProperties := unstructured.Unstructured{ + Object: make(map[string]interface{}), + } + unstructured.SetNestedField(traitWithProperties.Object, "test", TraitSpecField) - getErr := errors.New("get error") + traitWithoutGVK := unstructured.Unstructured{} + traitWithoutGVK.SetAPIVersion("") + traitWithoutGVK.SetKind("") - cwRaw, _ := json.Marshal(v1alpha2.ContainerizedWorkload{ - ObjectMeta: metav1.ObjectMeta{ - Name: "", + tests := []struct { + caseName string + traitContent unstructured.Unstructured + want string + }{ + { + caseName: "the trait contains 'name' info that should be mutated to GVK", + traitContent: traitWithName, + want: "the trait contains 'name' info", }, - }) - cwRawWithWorkloadName, _ := json.Marshal(v1alpha2.ContainerizedWorkload{ - ObjectMeta: metav1.ObjectMeta{ - Name: workloadName, + { + caseName: "the trait contains 'properties' info that should be mutated to spec", + traitContent: traitWithProperties, + want: "the trait contains 'properties' info", }, - }) - - kind := "ManualScalerTrait" - version := "core.oam.dev" - tName := "ms" - msTraitRaw, _ := json.Marshal(v1alpha2.ManualScalerTrait{ - TypeMeta: metav1.TypeMeta{ - Kind: kind, - APIVersion: version, + { + caseName: "the trait data missing GVK", + traitContent: traitWithoutGVK, + want: "the trait data missing GVK", }, - ObjectMeta: metav1.ObjectMeta{ - Name: tName, - }}) + } - mapper := mock.NewMockDiscoveryMapper() + for _, tc := range tests { + vAppConfig := ValidatingAppConfig{ + validatingComps: []ValidatingComponent{ + { + validatingTraits: []ValidatingTrait{ + { + traitContent: tc.traitContent, + }, + }, + }, + }, + } + allErrs := ValidateTraitObjectFn(ctx, vAppConfig) + result := utilerrors.NewAggregate(allErrs).Error() + assert.Contains(t, result, tc.want, fmt.Sprintf("Test case: %q", tc.caseName)) + } +} + +func TestValidateWorkloadNameForVersioningFn(t *testing.T) { + workloadName := "wl-name" + wlWithName := unstructured.Unstructured{} + wlWithName.SetName(workloadName) + paramName := "workloadName" + paramValue := workloadName tests := []struct { - caseName string - appConfig v1alpha2.ApplicationConfiguration - mockGetFunc test.MockGetFn - expectResult bool - expectReason string + caseName string + validatingAppConfig ValidatingAppConfig + want []error }{ { - caseName: "Test validation fails for workload name fixed in component", - appConfig: v1alpha2.ApplicationConfiguration{ - Spec: v1alpha2.ApplicationConfigurationSpec{ - Components: []v1alpha2.ApplicationConfigurationComponent{ - { - RevisionName: revisionName, + caseName: "validation fails for workload name fixed in component", + validatingAppConfig: ValidatingAppConfig{ + validatingComps: []ValidatingComponent{ + { + compName: "example-comp", + workloadContent: wlWithName, + validatingTraits: []ValidatingTrait{ + {traitDefinition: v1alpha2.TraitDefinition{ + Spec: v1alpha2.TraitDefinitionSpec{RevisionEnabled: true}, + }}, }, }, }, }, - mockGetFunc: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { - o, _ := obj.(*appsv1.ControllerRevision) - *o = appsv1.ControllerRevision{ - Data: runtime.RawExtension{Object: &v1alpha2.Component{ - Spec: v1alpha2.ComponentSpec{ - Workload: runtime.RawExtension{ - Raw: cwRawWithWorkloadName, - }, - }}}} - return nil + want: []error{ + fmt.Errorf(errFmtWorkloadNameNotEmpty, workloadName), }, - expectResult: false, - expectReason: fmt.Sprintf(reasonFmtWorkloadNameNotEmpty, workloadName), }, { - caseName: "Test validation fails for workload name assigned by parameter", - appConfig: v1alpha2.ApplicationConfiguration{ - Spec: v1alpha2.ApplicationConfigurationSpec{ - Components: []v1alpha2.ApplicationConfigurationComponent{ - { - RevisionName: revisionName, + caseName: "validation fails for workload name assigned by parameter", + validatingAppConfig: ValidatingAppConfig{ + validatingComps: []ValidatingComponent{ + { + compName: "example-comp", + appConfigComponent: v1alpha2.ApplicationConfigurationComponent{ ParameterValues: []v1alpha2.ComponentParameterValue{ { Name: paramName, @@ -200,187 +176,197 @@ func TestCheckWorkloadNameForVersioning(t *testing.T) { }, }, }, - }, - }, - }, - mockGetFunc: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { - o, _ := obj.(*appsv1.ControllerRevision) - *o = appsv1.ControllerRevision{ - Data: runtime.RawExtension{Object: &v1alpha2.Component{ - Spec: v1alpha2.ComponentSpec{ - Workload: runtime.RawExtension{ - Raw: cwRaw, - }, - Parameters: []v1alpha2.ComponentParameter{ - { - Name: paramName, - FieldPaths: []string{WorkloadNamePath}, - }, - }, - }}}} - return nil - }, - expectResult: false, - expectReason: fmt.Sprintf(reasonFmtWorkloadNameNotEmpty, workloadName), - }, - { - caseName: "Test validation success", - appConfig: v1alpha2.ApplicationConfiguration{ - Spec: v1alpha2.ApplicationConfigurationSpec{ - Components: []v1alpha2.ApplicationConfigurationComponent{ - { - ComponentName: componentName, - Traits: []v1alpha2.ComponentTrait{ - { - Trait: runtime.RawExtension{ - Raw: msTraitRaw, + component: v1alpha2.Component{ + Spec: v1alpha2.ComponentSpec{ + Parameters: []v1alpha2.ComponentParameter{ + { + Name: paramName, + FieldPaths: []string{WorkloadNamePath}, }, }, }, }, - { - ComponentName: componentName, + validatingTraits: []ValidatingTrait{ + {traitDefinition: v1alpha2.TraitDefinition{ + Spec: v1alpha2.TraitDefinitionSpec{RevisionEnabled: true}, + }}, }, }, }, }, - mockGetFunc: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { - if o, ok := obj.(*v1alpha2.TraitDefinition); ok { - *o = v1alpha2.TraitDefinition{ - Spec: v1alpha2.TraitDefinitionSpec{ - RevisionEnabled: true, - }, - } - } - if o, ok := obj.(*v1alpha2.Component); ok { - *o = v1alpha2.Component{ - Spec: v1alpha2.ComponentSpec{ - Workload: runtime.RawExtension{ - Raw: cwRaw, - }, + want: []error{ + fmt.Errorf(errFmtWorkloadNameNotEmpty, workloadName), + }, + }, + { + caseName: "validation succeeds", + validatingAppConfig: ValidatingAppConfig{ + validatingComps: []ValidatingComponent{ + { + compName: "example-comp", + validatingTraits: []ValidatingTrait{ + {traitDefinition: v1alpha2.TraitDefinition{ + Spec: v1alpha2.TraitDefinitionSpec{RevisionEnabled: true}, + }}, }, - } - } - return nil + }, + }, }, - expectResult: true, - expectReason: "", + want: nil, }, + } + + for _, tc := range tests { + result := ValidateWorkloadNameForVersioningFn(ctx, tc.validatingAppConfig) + assert.Equal(t, tc.want, result, fmt.Sprintf("Test case: %q", tc.caseName)) + } + +} + +func TestValidateTraitAppliableToWorkloadFn(t *testing.T) { + tests := []struct { + caseName string + validatingAppConfig ValidatingAppConfig + want []error + }{ { - caseName: "Test checkVersionEnbled error occurs during validation", - appConfig: v1alpha2.ApplicationConfiguration{ - Spec: v1alpha2.ApplicationConfigurationSpec{ - Components: []v1alpha2.ApplicationConfigurationComponent{ - { - ComponentName: componentName, - Traits: []v1alpha2.ComponentTrait{ - { - Trait: runtime.RawExtension{ - Raw: msTraitRaw, - }, + caseName: "apply trait to any workload", + validatingAppConfig: ValidatingAppConfig{ + validatingComps: []ValidatingComponent{ + { + workloadDefinition: v1alpha2.WorkloadDefinition{ + Spec: v1alpha2.WorkloadDefinitionSpec{ + Reference: v1alpha2.DefinitionReference{ + Name: "TestWorkload", }, }, }, + validatingTraits: []ValidatingTrait{ + {traitDefinition: v1alpha2.TraitDefinition{ + Spec: v1alpha2.TraitDefinitionSpec{ + AppliesToWorkloads: []string{"*"}, + }, + }}, + {traitDefinition: v1alpha2.TraitDefinition{ + Spec: v1alpha2.TraitDefinitionSpec{ + AppliesToWorkloads: []string{}, + }, + }}, + }, }, }, }, - mockGetFunc: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { - if _, ok := obj.(*v1alpha2.TraitDefinition); ok { - return getErr - } - if o, ok := obj.(*v1alpha2.Component); ok { - *o = v1alpha2.Component{ - Spec: v1alpha2.ComponentSpec{ - Workload: runtime.RawExtension{ - Raw: cwRaw, - }, + want: nil, + }, + { + caseName: "apply trait to workload with specific type", + validatingAppConfig: ValidatingAppConfig{ + validatingComps: []ValidatingComponent{ + { + component: v1alpha2.Component{ObjectMeta: v1.ObjectMeta{ + Labels: map[string]string{oam.WorkloadTypeLabel: "TestWorkload"}, + }}, + validatingTraits: []ValidatingTrait{ + {traitDefinition: v1alpha2.TraitDefinition{ + Spec: v1alpha2.TraitDefinitionSpec{ + AppliesToWorkloads: []string{"TestWorkload"}, + }, + }}, }, - } - } - return nil + }, + }, }, - expectResult: false, - expectReason: fmt.Sprintf(errFmtCheckWorkloadName, errors.Wrapf(getErr, errFmtGetTraitDefinition, version, kind, tName).Error()), + want: nil, }, { - caseName: "Test getComponent error occurs during validation", - appConfig: v1alpha2.ApplicationConfiguration{ - Spec: v1alpha2.ApplicationConfigurationSpec{ - Components: []v1alpha2.ApplicationConfigurationComponent{ - { - ComponentName: componentName, - Traits: []v1alpha2.ComponentTrait{ - { - Trait: runtime.RawExtension{ - Raw: msTraitRaw, - }, + caseName: "apply trait to workload with specific definition reference name", + validatingAppConfig: ValidatingAppConfig{ + validatingComps: []ValidatingComponent{ + { + workloadDefinition: v1alpha2.WorkloadDefinition{ + Spec: v1alpha2.WorkloadDefinitionSpec{ + Reference: v1alpha2.DefinitionReference{ + Name: "TestWorkload", }, }, }, + validatingTraits: []ValidatingTrait{ + {traitDefinition: v1alpha2.TraitDefinition{ + Spec: v1alpha2.TraitDefinitionSpec{ + AppliesToWorkloads: []string{"TestWorkload"}, + }, + }}, + }, }, }, }, - mockGetFunc: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { - if o, ok := obj.(*v1alpha2.TraitDefinition); ok { - *o = v1alpha2.TraitDefinition{ - Spec: v1alpha2.TraitDefinitionSpec{ - RevisionEnabled: true, - }, - } - } - if _, ok := obj.(*v1alpha2.Component); ok { - return getErr - } - return nil - }, - expectResult: false, - expectReason: "Error occurs when checking workload name. \"cannot get component \\\"c\\\": get error\"", + want: nil, }, { - caseName: "Test unmarshalWorkload error occurs during validation", - appConfig: v1alpha2.ApplicationConfiguration{ - Spec: v1alpha2.ApplicationConfigurationSpec{ - Components: []v1alpha2.ApplicationConfigurationComponent{ - { - ComponentName: componentName, - Traits: []v1alpha2.ComponentTrait{ - { - Trait: runtime.RawExtension{ - Raw: msTraitRaw, - }, - }, + caseName: "apply trait to workload with specific group", + validatingAppConfig: ValidatingAppConfig{ + validatingComps: []ValidatingComponent{ + { + workloadDefinition: v1alpha2.WorkloadDefinition{ + TypeMeta: v1.TypeMeta{ + APIVersion: "example.com/v1", + Kind: "TestWorkload", }, }, + validatingTraits: []ValidatingTrait{ + {traitDefinition: v1alpha2.TraitDefinition{ + Spec: v1alpha2.TraitDefinitionSpec{ + AppliesToWorkloads: []string{"*.example.com"}, + }, + }}, + }, }, }, }, - mockGetFunc: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { - if o, ok := obj.(*v1alpha2.TraitDefinition); ok { - *o = v1alpha2.TraitDefinition{ - Spec: v1alpha2.TraitDefinitionSpec{ - RevisionEnabled: true, + want: nil, + }, + { + caseName: "apply trait to unappliable workload", + validatingAppConfig: ValidatingAppConfig{ + validatingComps: []ValidatingComponent{ + { + compName: "example-comp", + component: v1alpha2.Component{ObjectMeta: v1.ObjectMeta{ + Labels: map[string]string{oam.WorkloadTypeLabel: "TestWorkload0"}, + }}, + workloadDefinition: v1alpha2.WorkloadDefinition{ + TypeMeta: v1.TypeMeta{ + APIVersion: "unknown.group/v1", + Kind: "TestWorkload1", + }, + Spec: v1alpha2.WorkloadDefinitionSpec{ + Reference: v1alpha2.DefinitionReference{ + Name: "TestWorkload2", + }, + }, }, - } - } - if o, ok := obj.(*v1alpha2.Component); ok { - *o = v1alpha2.Component{ - Spec: v1alpha2.ComponentSpec{ - Workload: runtime.RawExtension{}, + validatingTraits: []ValidatingTrait{ + {traitDefinition: v1alpha2.TraitDefinition{ + TypeMeta: v1.TypeMeta{ + APIVersion: "example.com/v1", + Kind: "TestTrait", + }, + Spec: v1alpha2.TraitDefinitionSpec{ + AppliesToWorkloads: []string{"example.com", "TestWorkload"}, + }, + }}, }, - } - } - return nil + }, + }, }, - expectResult: false, - expectReason: "Error occurs when unmarshal workload of component \"\" error: \"unexpected end of JSON input\"", + want: []error{fmt.Errorf(errFmtUnappliableTrait, + "example.com/v1, Kind=TestTrait", "unknown.group/v1, Kind=TestWorkload1", "example-comp", + []string{"example.com", "TestWorkload"})}, }, } + for _, tc := range tests { - func(t *testing.T) { - mockClient.MockGet = tc.mockGetFunc - result, reason := checkWorkloadNameForVersioning(ctx, mockClient, mapper, &tc.appConfig) - assert.Equal(t, tc.expectResult, result, fmt.Sprintf("Test case: %q", tc.caseName)) - assert.Equal(t, tc.expectReason, reason, fmt.Sprintf("Test case: %q", tc.caseName)) - }(t) + result := ValidateTraitAppliableToWorkloadFn(ctx, tc.validatingAppConfig) + assert.Equal(t, tc.want, result, fmt.Sprintf("Test case: %q", tc.caseName)) } }