From 19935b54abc638b4d8fc91be97c1cf140746fe54 Mon Sep 17 00:00:00 2001 From: Max Brauer Date: Sun, 8 May 2022 17:15:16 +0200 Subject: [PATCH] Include reconciler name in validation errors --- reconcilers/reconcilers.go | 28 +++++++++---------- reconcilers/reconcilers_validate_test.go | 34 +++++++++++++++--------- 2 files changed, 36 insertions(+), 26 deletions(-) diff --git a/reconcilers/reconcilers.go b/reconcilers/reconcilers.go index e377a66..98c26ba 100644 --- a/reconcilers/reconcilers.go +++ b/reconcilers/reconcilers.go @@ -396,11 +396,11 @@ func (r *SyncReconciler) validate(ctx context.Context) error { // func(ctx context.Context, parent client.Object) error // func(ctx context.Context, parent client.Object) (ctrl.Result, error) if r.Sync == nil { - return fmt.Errorf("SyncReconciler must implement Sync") + return fmt.Errorf("SyncReconciler %q must implement Sync", r.Name) } else { castParentType := RetrieveCastParentType(ctx) fn := reflect.TypeOf(r.Sync) - err := fmt.Errorf("SyncReconciler must implement Sync: func(context.Context, %s) error | func(context.Context, %s) (ctrl.Result, error), found: %s", reflect.TypeOf(castParentType), reflect.TypeOf(castParentType), fn) + err := fmt.Errorf("SyncReconciler %q must implement Sync: func(context.Context, %s) error | func(context.Context, %s) (ctrl.Result, error), found: %s", r.Name, reflect.TypeOf(castParentType), reflect.TypeOf(castParentType), fn) if fn.NumIn() != 2 || !reflect.TypeOf((*context.Context)(nil)).Elem().AssignableTo(fn.In(0)) || !reflect.TypeOf(castParentType).AssignableTo(fn.In(1)) { @@ -428,7 +428,7 @@ func (r *SyncReconciler) validate(ctx context.Context) error { if r.Finalize != nil { castParentType := RetrieveCastParentType(ctx) fn := reflect.TypeOf(r.Finalize) - err := fmt.Errorf("SyncReconciler must implement Finalize: nil | func(context.Context, %s) error | func(context.Context, %s) (ctrl.Result, error), found: %s", reflect.TypeOf(castParentType), reflect.TypeOf(castParentType), fn) + err := fmt.Errorf("SyncReconciler %q must implement Finalize: nil | func(context.Context, %s) error | func(context.Context, %s) (ctrl.Result, error), found: %s", r.Name, reflect.TypeOf(castParentType), reflect.TypeOf(castParentType), fn) if fn.NumIn() != 2 || !reflect.TypeOf((*context.Context)(nil)).Elem().AssignableTo(fn.In(0)) || !reflect.TypeOf(castParentType).AssignableTo(fn.In(1)) { @@ -697,7 +697,7 @@ func (r *ChildReconciler) validate(ctx context.Context) error { // validate DesiredChild function signature: // func(ctx context.Context, parent client.Object) (client.Object, error) if r.DesiredChild == nil { - return fmt.Errorf("ChildReconciler must implement DesiredChild") + return fmt.Errorf("ChildReconciler %q must implement DesiredChild", r.Name) } else { fn := reflect.TypeOf(r.DesiredChild) if fn.NumIn() != 2 || fn.NumOut() != 2 || @@ -705,21 +705,21 @@ func (r *ChildReconciler) validate(ctx context.Context) error { !reflect.TypeOf(castParentType).AssignableTo(fn.In(1)) || !reflect.TypeOf(r.ChildType).AssignableTo(fn.Out(0)) || !reflect.TypeOf((*error)(nil)).Elem().AssignableTo(fn.Out(1)) { - return fmt.Errorf("ChildReconciler must implement DesiredChild: func(context.Context, %s) (%s, error), found: %s", reflect.TypeOf(castParentType), reflect.TypeOf(r.ChildType), fn) + return fmt.Errorf("ChildReconciler %q must implement DesiredChild: func(context.Context, %s) (%s, error), found: %s", r.Name, reflect.TypeOf(castParentType), reflect.TypeOf(r.ChildType), fn) } } // validate ReflectChildStatusOnParent function signature: // func(parent, child client.Object, err error) if r.ReflectChildStatusOnParent == nil { - return fmt.Errorf("ChildReconciler must implement ReflectChildStatusOnParent") + return fmt.Errorf("ChildReconciler %q must implement ReflectChildStatusOnParent", r.Name) } else { fn := reflect.TypeOf(r.ReflectChildStatusOnParent) if fn.NumIn() != 3 || fn.NumOut() != 0 || !reflect.TypeOf(castParentType).AssignableTo(fn.In(0)) || !reflect.TypeOf(r.ChildType).AssignableTo(fn.In(1)) || !reflect.TypeOf((*error)(nil)).Elem().AssignableTo(fn.In(2)) { - return fmt.Errorf("ChildReconciler must implement ReflectChildStatusOnParent: func(%s, %s, error), found: %s", reflect.TypeOf(castParentType), reflect.TypeOf(r.ChildType), fn) + return fmt.Errorf("ChildReconciler %q must implement ReflectChildStatusOnParent: func(%s, %s, error), found: %s", r.Name, reflect.TypeOf(castParentType), reflect.TypeOf(r.ChildType), fn) } } @@ -731,34 +731,34 @@ func (r *ChildReconciler) validate(ctx context.Context) error { if fn.NumIn() != 2 || fn.NumOut() != 0 || !reflect.TypeOf(r.ChildType).AssignableTo(fn.In(0)) || !reflect.TypeOf(r.ChildType).AssignableTo(fn.In(1)) { - return fmt.Errorf("ChildReconciler must implement HarmonizeImmutableFields: nil | func(%s, %s), found: %s", reflect.TypeOf(r.ChildType), reflect.TypeOf(r.ChildType), fn) + return fmt.Errorf("ChildReconciler %q must implement HarmonizeImmutableFields: nil | func(%s, %s), found: %s", r.Name, reflect.TypeOf(r.ChildType), reflect.TypeOf(r.ChildType), fn) } } // validate MergeBeforeUpdate function signature: // func(current, desired client.Object) if r.MergeBeforeUpdate == nil { - return fmt.Errorf("ChildReconciler must implement MergeBeforeUpdate") + return fmt.Errorf("ChildReconciler %q must implement MergeBeforeUpdate", r.Name) } else { fn := reflect.TypeOf(r.MergeBeforeUpdate) if fn.NumIn() != 2 || fn.NumOut() != 0 || !reflect.TypeOf(r.ChildType).AssignableTo(fn.In(0)) || !reflect.TypeOf(r.ChildType).AssignableTo(fn.In(1)) { - return fmt.Errorf("ChildReconciler must implement MergeBeforeUpdate: nil | func(%s, %s), found: %s", reflect.TypeOf(r.ChildType), reflect.TypeOf(r.ChildType), fn) + return fmt.Errorf("ChildReconciler %q must implement MergeBeforeUpdate: nil | func(%s, %s), found: %s", r.Name, reflect.TypeOf(r.ChildType), reflect.TypeOf(r.ChildType), fn) } } // validate SemanticEquals function signature: // func(a1, a2 client.Object) bool if r.SemanticEquals == nil { - return fmt.Errorf("ChildReconciler must implement SemanticEquals") + return fmt.Errorf("ChildReconciler %q must implement SemanticEquals", r.Name) } else { fn := reflect.TypeOf(r.SemanticEquals) if fn.NumIn() != 2 || fn.NumOut() != 1 || !reflect.TypeOf(r.ChildType).AssignableTo(fn.In(0)) || !reflect.TypeOf(r.ChildType).AssignableTo(fn.In(1)) || fn.Out(0).Kind() != reflect.Bool { - return fmt.Errorf("ChildReconciler must implement SemanticEquals: nil | func(%s, %s) bool, found: %s", reflect.TypeOf(r.ChildType), reflect.TypeOf(r.ChildType), fn) + return fmt.Errorf("ChildReconciler %q must implement SemanticEquals: nil | func(%s, %s) bool, found: %s", r.Name, reflect.TypeOf(r.ChildType), reflect.TypeOf(r.ChildType), fn) } } @@ -771,7 +771,7 @@ func (r *ChildReconciler) validate(ctx context.Context) error { !reflect.TypeOf(castParentType).AssignableTo(fn.In(0)) || !reflect.TypeOf(r.ChildType).AssignableTo(fn.In(1)) || fn.Out(0).Kind() != reflect.Bool { - return fmt.Errorf("ChildReconciler must implement OurChild: nil | func(%s, %s) bool, found: %s", reflect.TypeOf(castParentType), reflect.TypeOf(r.ChildType), fn) + return fmt.Errorf("ChildReconciler %q must implement OurChild: nil | func(%s, %s) bool, found: %s", r.Name, reflect.TypeOf(castParentType), reflect.TypeOf(r.ChildType), fn) } } @@ -782,7 +782,7 @@ func (r *ChildReconciler) validate(ctx context.Context) error { fn := reflect.TypeOf(r.Sanitize) if fn.NumIn() != 1 || fn.NumOut() != 1 || !reflect.TypeOf(r.ChildType).AssignableTo(fn.In(0)) { - return fmt.Errorf("ChildReconciler must implement Sanitize: nil | func(%s) interface{}, found: %s", reflect.TypeOf(r.ChildType), fn) + return fmt.Errorf("ChildReconciler %q must implement Sanitize: nil | func(%s) interface{}, found: %s", r.Name, reflect.TypeOf(r.ChildType), fn) } } diff --git a/reconcilers/reconcilers_validate_test.go b/reconcilers/reconcilers_validate_test.go index e05de7c..c5f6dd3 100644 --- a/reconcilers/reconcilers_validate_test.go +++ b/reconcilers/reconcilers_validate_test.go @@ -25,8 +25,8 @@ func TestSyncReconciler_validate(t *testing.T) { { name: "empty", parent: &corev1.ConfigMap{}, - reconciler: &SyncReconciler{}, - shouldErr: "SyncReconciler must implement Sync", + reconciler: &SyncReconciler{Name: "empty"}, + shouldErr: `SyncReconciler "empty" must implement Sync`, }, { name: "valid", @@ -50,50 +50,55 @@ func TestSyncReconciler_validate(t *testing.T) { name: "Sync num in", parent: &corev1.ConfigMap{}, reconciler: &SyncReconciler{ + Name: "Sync num in", Sync: func() error { return nil }, }, - shouldErr: "SyncReconciler must implement Sync: func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func() error", + shouldErr: `SyncReconciler "Sync num in" must implement Sync: func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func() error`, }, { name: "Sync in 1", parent: &corev1.ConfigMap{}, reconciler: &SyncReconciler{ + Name: "Sync in 1", Sync: func(ctx context.Context, parent *corev1.Secret) error { return nil }, }, - shouldErr: "SyncReconciler must implement Sync: func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func(context.Context, *v1.Secret) error", + shouldErr: `SyncReconciler "Sync in 1" must implement Sync: func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func(context.Context, *v1.Secret) error`, }, { name: "Sync num out", parent: &corev1.ConfigMap{}, reconciler: &SyncReconciler{ + Name: "Sync num out", Sync: func(ctx context.Context, parent *corev1.ConfigMap) { }, }, - shouldErr: "SyncReconciler must implement Sync: func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func(context.Context, *v1.ConfigMap)", + shouldErr: `SyncReconciler "Sync num out" must implement Sync: func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func(context.Context, *v1.ConfigMap)`, }, { name: "Sync out 1", parent: &corev1.ConfigMap{}, reconciler: &SyncReconciler{ + Name: "Sync out 1", Sync: func(ctx context.Context, parent *corev1.ConfigMap) string { return "" }, }, - shouldErr: "SyncReconciler must implement Sync: func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func(context.Context, *v1.ConfigMap) string", + shouldErr: `SyncReconciler "Sync out 1" must implement Sync: func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func(context.Context, *v1.ConfigMap) string`, }, { name: "Sync result out 1", parent: &corev1.ConfigMap{}, reconciler: &SyncReconciler{ + Name: "Sync result out 1", Sync: func(ctx context.Context, parent *corev1.ConfigMap) (ctrl.Result, string) { return ctrl.Result{}, "" }, }, - shouldErr: "SyncReconciler must implement Sync: func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func(context.Context, *v1.ConfigMap) (reconcile.Result, string)", + shouldErr: `SyncReconciler "Sync result out 1" must implement Sync: func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func(context.Context, *v1.ConfigMap) (reconcile.Result, string)`, }, { name: "valid Finalize", @@ -123,6 +128,7 @@ func TestSyncReconciler_validate(t *testing.T) { name: "Finalize num in", parent: &corev1.ConfigMap{}, reconciler: &SyncReconciler{ + Name: "Finalize num in", Sync: func(ctx context.Context, parent *corev1.ConfigMap) error { return nil }, @@ -130,12 +136,13 @@ func TestSyncReconciler_validate(t *testing.T) { return nil }, }, - shouldErr: "SyncReconciler must implement Finalize: nil | func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func() error", + shouldErr: `SyncReconciler "Finalize num in" must implement Finalize: nil | func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func() error`, }, { name: "Finalize in 1", parent: &corev1.ConfigMap{}, reconciler: &SyncReconciler{ + Name: "Finalize in 1", Sync: func(ctx context.Context, parent *corev1.ConfigMap) error { return nil }, @@ -143,24 +150,26 @@ func TestSyncReconciler_validate(t *testing.T) { return nil }, }, - shouldErr: "SyncReconciler must implement Finalize: nil | func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func(context.Context, *v1.Secret) error", + shouldErr: `SyncReconciler "Finalize in 1" must implement Finalize: nil | func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func(context.Context, *v1.Secret) error`, }, { name: "Finalize num out", parent: &corev1.ConfigMap{}, reconciler: &SyncReconciler{ + Name: "Finalize num out", Sync: func(ctx context.Context, parent *corev1.ConfigMap) error { return nil }, Finalize: func(ctx context.Context, parent *corev1.ConfigMap) { }, }, - shouldErr: "SyncReconciler must implement Finalize: nil | func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func(context.Context, *v1.ConfigMap)", + shouldErr: `SyncReconciler "Finalize num out" must implement Finalize: nil | func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func(context.Context, *v1.ConfigMap)`, }, { name: "Finalize out 1", parent: &corev1.ConfigMap{}, reconciler: &SyncReconciler{ + Name: "Finalize out 1", Sync: func(ctx context.Context, parent *corev1.ConfigMap) error { return nil }, @@ -168,12 +177,13 @@ func TestSyncReconciler_validate(t *testing.T) { return "" }, }, - shouldErr: "SyncReconciler must implement Finalize: nil | func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func(context.Context, *v1.ConfigMap) string", + shouldErr: `SyncReconciler "Finalize out 1" must implement Finalize: nil | func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func(context.Context, *v1.ConfigMap) string`, }, { name: "Finalize result out 1", parent: &corev1.ConfigMap{}, reconciler: &SyncReconciler{ + Name: "Finalize result out 1", Sync: func(ctx context.Context, parent *corev1.ConfigMap) error { return nil }, @@ -181,7 +191,7 @@ func TestSyncReconciler_validate(t *testing.T) { return ctrl.Result{}, "" }, }, - shouldErr: "SyncReconciler must implement Finalize: nil | func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func(context.Context, *v1.ConfigMap) (reconcile.Result, string)", + shouldErr: `SyncReconciler "Finalize result out 1" must implement Finalize: nil | func(context.Context, *v1.ConfigMap) error | func(context.Context, *v1.ConfigMap) (ctrl.Result, error), found: func(context.Context, *v1.ConfigMap) (reconcile.Result, string)`, }, }