github/kubevela
1
0
Fork 0
mirror of https://github.com/kubevela/kubevela.git synced 2026-08-18 20:17:04 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix minor lint issues

Browse Source 
add unit tests

Signed-off-by: roy wang <seiwy2010@gmail.com>
This commit is contained in:
roy wang
2020-11-30 16:11:48 +09:00
parent 8b27ade1cb
commit 12a2eb8491
7 changed files with 172 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/applicationconfiguration.go
+1 -1
View File
@@ -86,7 +86,7 @@ const (
func Setup(mgr ctrl.Manager, args core.Args, l logging.Logger) error {
dm, err := discoverymapper.New(mgr.GetConfig())
if err != nil {
return fmt.Errorf("create discovery dm fail %v", err)
return fmt.Errorf("create discovery dm fail %w", err)
}
name := "oam/" + strings.ToLower(v1alpha2.ApplicationConfigurationGroupKind)
pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/apply.go
+3
View File
@@ -84,6 +84,7 @@ type workloads struct {
dm discoverymapper.DiscoveryMapper
}
//nolint:errorlint
func (a *workloads) Apply(ctx context.Context, status []v1alpha2.WorkloadStatus, w []Workload, ao ...resource.ApplyOption) error {
// they are all in the same namespace
var namespace = w[0].Workload.GetNamespace()
@@ -91,6 +92,7 @@ func (a *workloads) Apply(ctx context.Context, status []v1alpha2.WorkloadStatus,
if !wl.HasDep {
err := a.patchingClient.Apply(ctx, wl.Workload, ao...)
if err != nil {
// TODO(roywang) use errors.As() insteand of type assertion on error
if _, ok := err.(*GenerationUnchanged); !ok {
// GenerationUnchanged only aborts applying current workload
// but not blocks the whole reconciliation through returning an error
@@ -104,6 +106,7 @@ func (a *workloads) Apply(ctx context.Context, status []v1alpha2.WorkloadStatus,
}
t := trait.Object
if err := a.updatingClient.Apply(ctx, &trait.Object, ao...); err != nil {
// TODO(roywang) use errors.As() insteand of type assertion on error
if _, ok := err.(*GenerationUnchanged); !ok {
// GenerationUnchanged only aborts applying current trait
// but not blocks the whole reconciliation through returning an error
pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/render.go
+1 -1
View File
@@ -615,7 +615,7 @@ func getExpectVal(m v1alpha2.ConditionRequirement, ac *fieldpath.Paved) (string,
var err error
value, err := ac.GetString(m.ValueFrom.FieldPath)
if err != nil {
return "", fmt.Errorf("get valueFrom.fieldPath fail: %v", err)
return "", fmt.Errorf("get valueFrom.fieldPath fail: %w", err)
}
return value, nil
}
pkg/oam/util/test_utils.go
+4
View File
@@ -82,6 +82,8 @@ func (matcher ErrorMatcher) Match(actual interface{}) (success bool, err error)
}
// FailureMessage builds an error message.
//nolint:errorlint
// TODO(roywang) use errors.As() instead of type assertion on error
func (matcher ErrorMatcher) FailureMessage(actual interface{}) (message string) {
actualError, actualOK := actual.(error)
expectedError, expectedOK := matcher.ExpectedError.(error)
@@ -102,6 +104,8 @@ func (matcher ErrorMatcher) FailureMessage(actual interface{}) (message string)
}
// NegatedFailureMessage builds an error message.
//nolint:errorlint
// TODO(roywang) use errors.As() instead of type assertion on error
func (matcher ErrorMatcher) NegatedFailureMessage(actual interface{}) (message string) {
actualError, actualOK := actual.(error)
expectedError, expectedOK := matcher.ExpectedError.(error)
pkg/webhook/core.oam.dev/v1alpha2/applicationconfiguration/handler_test.go
+56
View File
@@ -268,6 +268,17 @@ var _ = Describe("ApplicationConfiguration Admission controller Test", func() {
By(string(resp.Result.Reason))
Expect(resp.Allowed).Should(BeTrue())
By("Test delete operation request")
req = admission.Request{
AdmissionRequest: admissionv1beta1.AdmissionRequest{
Operation: admissionv1beta1.Delete,
Resource: reqResource,
Object: runtime.RawExtension{Raw: util.JSONMarshal(appConfig)},
},
}
resp = handler.Handle(context.TODO(), req)
Expect(resp.Allowed).Should(BeTrue())
By("Test bad admission request format")
req = admission.Request{
AdmissionRequest: admissionv1beta1.AdmissionRequest{
@@ -278,5 +289,50 @@ var _ = Describe("ApplicationConfiguration Admission controller Test", func() {
}
resp = handler.Handle(context.TODO(), req)
Expect(resp.Allowed).Should(BeFalse())
By("Prepare for a bad admission resource")
badReqResource := metav1.GroupVersionResource{Group: "core.oam.dev", Version: "v1alpha2", Resource: "foo"}
req = admission.Request{
AdmissionRequest: admissionv1beta1.AdmissionRequest{
Operation: admissionv1beta1.Create,
Resource: badReqResource,
Object: runtime.RawExtension{Raw: util.JSONMarshal(appConfig)},
},
}
resp = handler.Handle(context.TODO(), req)
Expect(resp.Allowed).Should(BeFalse())
By("reject the request for error occurs when prepare data for validation")
errClientInstance := &test.MockClient{
MockGet: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error {
return fmt.Errorf("cannot prepare data for validation")
},
}
req = admission.Request{
AdmissionRequest: admissionv1beta1.AdmissionRequest{
Operation: admissionv1beta1.Create,
Resource: reqResource,
Object: runtime.RawExtension{Raw: util.JSONMarshal(appConfig)},
},
}
injc.InjectClient(errClientInstance)
resp = handler.Handle(context.TODO(), req)
Expect(resp.Allowed).Should(BeFalse())
By("reject the request for validation fails")
var rejectHandler admission.Handler = &ValidatingHandler{
Mapper: mapper,
Validators: []AppConfigValidator{
AppConfigValidateFunc(func(c context.Context, vac ValidatingAppConfig) []error {
return []error{fmt.Errorf("validation fails")}
}),
},
}
rejectDeecoderInjector := rejectHandler.(admission.DecoderInjector)
rejectDeecoderInjector.InjectDecoder(decoder)
rejectClientInjector := rejectHandler.(inject.Client)
rejectClientInjector.InjectClient(clientInstance)
resp = rejectHandler.Handle(context.TODO(), req)
Expect(resp.Allowed).Should(BeFalse())
})
})
pkg/webhook/core.oam.dev/v1alpha2/applicationconfiguration/validating_handler.go
+8 -6
View File
@@ -170,13 +170,15 @@ func ValidateTraitAppliableToWorkloadFn(_ context.Context, v ValidatingAppConfig
for _, c := range v.validatingComps {
// TODO(roywang) consider a CRD group could have multiple versions
// and maybe we need to specify the minimum version here in the future
workloadDefRefName := c.workloadDefinition.Spec.Reference.Name
workloadDefName := c.workloadDefinition.GetName()
workloadGroup := schema.ParseGroupResource(workloadDefRefName).Group
// according to OAM convention, Spec.Reference.Name in workloadDefinition is CRD name
crdName := c.workloadDefinition.Spec.Reference.Name
// according to OAM convention, name of workloadDefinition is the workload type.
workloadTypeName := c.workloadDefinition.GetName()
workloadGroup := schema.ParseGroupResource(crdName).Group
klog.Info("validate trait is appliable to workload: ",
fmt.Sprintf("workloadDefRefName:%s, workloadDefName(type):%s, workloadGroup:%s",
workloadDefRefName, workloadDefName, workloadGroup))
crdName, workloadTypeName, workloadGroup))
ValidateApplyTo:
for _, t := range c.validatingTraits {
klog.Info("validate trait is appliable to workload: ",
@@ -194,8 +196,8 @@ func ValidateTraitAppliableToWorkloadFn(_ context.Context, v ValidatingAppConfig
if strings.HasPrefix(applyTo, "*.") && workloadGroup == applyTo[2:] {
continue ValidateApplyTo
}
if workloadDefRefName == applyTo ||
workloadDefName == applyTo {
if crdName == applyTo ||
workloadTypeName == applyTo {
continue ValidateApplyTo
}
}
pkg/webhook/core.oam.dev/v1alpha2/applicationconfiguration/validating_handler_test.go
+99 -11
View File
@@ -229,7 +229,7 @@ func TestValidateTraitAppliableToWorkloadFn(t *testing.T) {
want []error
}{
{
caseName: "apply trait to any workload",
caseName: "validate succeed: apply trait to any workload",
validatingAppConfig: ValidatingAppConfig{
validatingComps: []ValidatingComponent{
{
@@ -243,12 +243,12 @@ func TestValidateTraitAppliableToWorkloadFn(t *testing.T) {
validatingTraits: []ValidatingTrait{
{traitDefinition: v1alpha2.TraitDefinition{
Spec: v1alpha2.TraitDefinitionSpec{
AppliesToWorkloads: []string{"*"},
AppliesToWorkloads: []string{"*"}, // "*" means apply to any
},
}},
{traitDefinition: v1alpha2.TraitDefinition{
Spec: v1alpha2.TraitDefinitionSpec{
AppliesToWorkloads: []string{},
AppliesToWorkloads: []string{}, // empty means apply to any
},
}},
},
@@ -258,12 +258,12 @@ func TestValidateTraitAppliableToWorkloadFn(t *testing.T) {
want: nil,
},
{
caseName: "apply trait to workload with specific workloadDefinition name",
caseName: "validate succeed: apply trait to workload with specific workloadDefinition name",
validatingAppConfig: ValidatingAppConfig{
validatingComps: []ValidatingComponent{
{
workloadDefinition: v1alpha2.WorkloadDefinition{
ObjectMeta: v1.ObjectMeta{Name: "TestWorkload"},
ObjectMeta: v1.ObjectMeta{Name: "TestWorkload"}, // matched workload def(type) nmae
},
validatingTraits: []ValidatingTrait{
{traitDefinition: v1alpha2.TraitDefinition{
@@ -278,14 +278,14 @@ func TestValidateTraitAppliableToWorkloadFn(t *testing.T) {
want: nil,
},
{
caseName: "apply trait to workload with specific definition reference name",
caseName: "validate succeed: apply trait to workload with specific definition reference name",
validatingAppConfig: ValidatingAppConfig{
validatingComps: []ValidatingComponent{
{
workloadDefinition: v1alpha2.WorkloadDefinition{
Spec: v1alpha2.WorkloadDefinitionSpec{
Reference: v1alpha2.DefinitionReference{
Name: "TestWorkload",
Name: "TestWorkload", // matched CRD name
},
},
},
@@ -302,14 +302,14 @@ func TestValidateTraitAppliableToWorkloadFn(t *testing.T) {
want: nil,
},
{
caseName: "apply trait to workload with specific group",
caseName: "validate succeed: apply trait to workload with specific group",
validatingAppConfig: ValidatingAppConfig{
validatingComps: []ValidatingComponent{
{
workloadDefinition: v1alpha2.WorkloadDefinition{
Spec: v1alpha2.WorkloadDefinitionSpec{
Reference: v1alpha2.DefinitionReference{
Name: "testworkloads.example.com",
Name: "testworkloads.example.com", // matched CRD group
},
},
},
@@ -342,7 +342,7 @@ func TestValidateTraitAppliableToWorkloadFn(t *testing.T) {
want: nil,
},
{
caseName: "apply trait to unappliable workload",
caseName: "validate fail: apply trait to unappliable workload",
validatingAppConfig: ValidatingAppConfig{
validatingComps: []ValidatingComponent{
{
@@ -351,7 +351,7 @@ func TestValidateTraitAppliableToWorkloadFn(t *testing.T) {
ObjectMeta: v1.ObjectMeta{Name: "TestWorkload"},
Spec: v1alpha2.WorkloadDefinitionSpec{
Reference: v1alpha2.DefinitionReference{
Name: "TestWorkload1",
Name: "TestWorkload1.example.foo",
},
},
},
@@ -370,6 +370,94 @@ func TestValidateTraitAppliableToWorkloadFn(t *testing.T) {
"TestTrait", "TestWorkload", "example-comp",
[]string{"example.com", "TestWorkload2"})},
},
{
caseName: "validate fail: applyTo has CRD group but not match workload",
validatingAppConfig: ValidatingAppConfig{
validatingComps: []ValidatingComponent{
{
compName: "example-comp",
workloadDefinition: v1alpha2.WorkloadDefinition{
ObjectMeta: v1.ObjectMeta{
Name: "TestWorkload",
},
Spec: v1alpha2.WorkloadDefinitionSpec{
Reference: v1alpha2.DefinitionReference{
Name: "testworkloads.example.foo", // dismatched CRD group
},
},
},
validatingTraits: []ValidatingTrait{
{traitDefinition: v1alpha2.TraitDefinition{
ObjectMeta: v1.ObjectMeta{Name: "TestTrait"},
Spec: v1alpha2.TraitDefinitionSpec{
AppliesToWorkloads: []string{"*.example.com"},
},
}},
},
},
},
},
want: []error{fmt.Errorf(errFmtUnappliableTrait,
"TestTrait", "TestWorkload", "example-comp",
[]string{"*.example.com"})},
},
{
caseName: "validate fail: applyTo has CRD name but not match",
validatingAppConfig: ValidatingAppConfig{
validatingComps: []ValidatingComponent{
{
compName: "example-comp",
workloadDefinition: v1alpha2.WorkloadDefinition{
ObjectMeta: v1.ObjectMeta{
Name: "TestWorkload",
},
Spec: v1alpha2.WorkloadDefinitionSpec{
Reference: v1alpha2.DefinitionReference{
Name: "bar.example.com", // dismatched CRD name
},
},
},
validatingTraits: []ValidatingTrait{
{traitDefinition: v1alpha2.TraitDefinition{
ObjectMeta: v1.ObjectMeta{Name: "TestTrait"},
Spec: v1alpha2.TraitDefinitionSpec{
AppliesToWorkloads: []string{"foo.example.com"},
},
}},
},
},
},
},
want: []error{fmt.Errorf(errFmtUnappliableTrait,
"TestTrait", "TestWorkload", "example-comp",
[]string{"foo.example.com"})},
},
{
caseName: "validate fail: applyTo has definition name but not match",
validatingAppConfig: ValidatingAppConfig{
validatingComps: []ValidatingComponent{
{
compName: "example-comp",
workloadDefinition: v1alpha2.WorkloadDefinition{
ObjectMeta: v1.ObjectMeta{
Name: "bar", // dismatched workload def(type) name
},
},
validatingTraits: []ValidatingTrait{
{traitDefinition: v1alpha2.TraitDefinition{
ObjectMeta: v1.ObjectMeta{Name: "TestTrait"},
Spec: v1alpha2.TraitDefinitionSpec{
AppliesToWorkloads: []string{"foo"},
},
}},
},
},
},
},
want: []error{fmt.Errorf(errFmtUnappliableTrait,
"TestTrait", "bar", "example-comp",
[]string{"foo"})},
},
}
for _, tc := range tests {