From 3652a41bf38c311ade2f04e36fb0afaef80d6fbd Mon Sep 17 00:00:00 2001 From: justinsb Date: Wed, 28 Feb 2024 08:36:43 -0500 Subject: [PATCH] chore: avoid errors.Is, replace with IsFoo errors.Is performs an equality check, which is not normally what we want. --- pkg/controller/lifecyclehandler/handler.go | 7 +- .../lifecyclehandler/handler_test.go | 84 +++++++++++++++++++ pkg/k8s/errors.go | 35 ++++++-- 3 files changed, 117 insertions(+), 9 deletions(-) diff --git a/pkg/controller/lifecyclehandler/handler.go b/pkg/controller/lifecyclehandler/handler.go index 00637da12f..b84534ed45 100644 --- a/pkg/controller/lifecyclehandler/handler.go +++ b/pkg/controller/lifecyclehandler/handler.go @@ -16,7 +16,6 @@ package lifecyclehandler import ( "context" - "errors" "fmt" corekccv1alpha1 "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/apis/core/v1alpha1" @@ -185,11 +184,11 @@ func CausedByUnresolvableDeps(err error) (unwrappedErr error, ok bool) { //nolin func reasonForUnresolvableDeps(err error) (string, error) { switch { - case errors.Is(err, &k8s.ReferenceNotReadyError{}) || errors.Is(err, &k8s.TransitiveDependencyNotReadyError{}): + case k8s.IsReferenceNotReadyError(err) || k8s.IsTransitiveDependencyNotReadyError(err): return k8s.DependencyNotReady, nil - case errors.Is(err, &k8s.ReferenceNotFoundError{}) || errors.Is(err, &k8s.SecretNotFoundError{}) || errors.Is(err, &k8s.TransitiveDependencyNotFoundError{}): + case k8s.IsReferenceNotFoundError(err) || k8s.IsSecretNotFoundError(err) || k8s.IsTransitiveDependencyNotFoundError(err): return k8s.DependencyNotFound, nil - case errors.Is(err, &k8s.KeyInSecretNotFoundError{}): + case k8s.IsKeyInSecretNotFoundError(err): return k8s.DependencyInvalid, nil default: return "", fmt.Errorf("unrecognized error caused by unresolvable dependencies: %w", err) diff --git a/pkg/controller/lifecyclehandler/handler_test.go b/pkg/controller/lifecyclehandler/handler_test.go index 07ce36ed13..a794c21d9f 100644 --- a/pkg/controller/lifecyclehandler/handler_test.go +++ b/pkg/controller/lifecyclehandler/handler_test.go @@ -15,6 +15,7 @@ package lifecyclehandler import ( + "errors" "testing" corekccv1alpha1 "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/apis/core/v1alpha1" @@ -27,6 +28,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/manager" ) @@ -177,6 +179,88 @@ func TestIsOrphaned(t *testing.T) { } } +func Test_reasonForUnresolvableDeps(t *testing.T) { + tests := []struct { + name string + err error + want string + wantErr bool + }{ + { + name: "reference not ready", + err: &k8s.ReferenceNotReadyError{}, + want: k8s.DependencyNotReady, + wantErr: false, + }, + { + name: "NewReferenceNotReadyErrorForResource gives DependencyNotReady", + err: k8s.NewReferenceNotReadyErrorForResource(&k8s.Resource{}), + want: k8s.DependencyNotReady, + wantErr: false, + }, + { + name: "transitive dependency not ready", + err: &k8s.TransitiveDependencyNotReadyError{}, + want: k8s.DependencyNotReady, + wantErr: false, + }, + { + name: "reference not found", + err: &k8s.ReferenceNotFoundError{}, + want: k8s.DependencyNotFound, + wantErr: false, + }, + { + name: "secret not found", + err: &k8s.SecretNotFoundError{}, + want: k8s.DependencyNotFound, + wantErr: false, + }, + { + name: "transitive dependency not found", + err: &k8s.TransitiveDependencyNotFoundError{}, + want: k8s.DependencyNotFound, + wantErr: false, + }, + { + name: "NewTransitiveDependencyNotFoundError gives DependencyNotFound", + err: k8s.NewTransitiveDependencyNotFoundError(schema.GroupVersionKind{}, types.NamespacedName{}), + want: k8s.DependencyNotFound, + wantErr: false, + }, + { + name: "key in secret not found", + err: &k8s.KeyInSecretNotFoundError{}, + want: k8s.DependencyInvalid, + wantErr: false, + }, + { + name: "unrecognized error", + err: errors.New("some error"), + want: "", + wantErr: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, err := reasonForUnresolvableDeps(test.err) + if test.wantErr { + if err == nil { + t.Errorf("reasonForUnresolvableDeps() error = nil, want err") + } + return + } + if err != nil { + t.Errorf("reasonForUnresolvableDeps() error = %v, want nil", err) + return + } + if got != test.want { + t.Errorf("reasonForUnresolvableDeps() got = %v, want %v", got, test.want) + } + }) + } +} + func TestMain(m *testing.M) { testmain.ForUnitTests(m, &mgr) } diff --git a/pkg/k8s/errors.go b/pkg/k8s/errors.go index 5649953864..1763ee9bbc 100644 --- a/pkg/k8s/errors.go +++ b/pkg/k8s/errors.go @@ -52,6 +52,11 @@ func AsReferenceNotReadyError(err error) (unwrappedErr *ReferenceNotReadyError, return unwrappedErr, ok } +func IsReferenceNotReadyError(err error) bool { + _, ok := AsReferenceNotReadyError(err) + return ok +} + type ReferenceNotFoundError struct { RefResourceGVK schema.GroupVersionKind RefResource types.NamespacedName @@ -72,16 +77,16 @@ func NewReferenceNotFoundErrorForResource(r *Resource) *ReferenceNotFoundError { } } -func IsReferenceNotFoundError(err error) bool { - _, ok := AsReferenceNotFoundError(err) - return ok -} - func AsReferenceNotFoundError(err error) (unwrappedErr *ReferenceNotFoundError, ok bool) { ok = errors.As(err, &unwrappedErr) return unwrappedErr, ok } +func IsReferenceNotFoundError(err error) bool { + _, ok := AsReferenceNotFoundError(err) + return ok +} + type SecretNotFoundError struct { Secret types.NamespacedName } @@ -99,6 +104,11 @@ func AsSecretNotFoundError(err error) (unwrappedErr *SecretNotFoundError, ok boo return unwrappedErr, ok } +func IsSecretNotFoundError(err error) bool { + _, ok := AsSecretNotFoundError(err) + return ok +} + type KeyInSecretNotFoundError struct { key string secret types.NamespacedName @@ -117,6 +127,11 @@ func AsKeyInSecretNotFoundError(err error) (unwrappedErr *KeyInSecretNotFoundErr return unwrappedErr, ok } +func IsKeyInSecretNotFoundError(err error) bool { + _, ok := AsKeyInSecretNotFoundError(err) + return ok +} + type TransitiveDependencyNotFoundError struct { ResourceGVK schema.GroupVersionKind Resource types.NamespacedName @@ -135,6 +150,11 @@ func AsTransitiveDependencyNotFoundError(err error) (unwrappedErr *TransitiveDep return unwrappedErr, ok } +func IsTransitiveDependencyNotFoundError(err error) bool { + _, ok := AsTransitiveDependencyNotFoundError(err) + return ok +} + type TransitiveDependencyNotReadyError struct { ResourceGVK schema.GroupVersionKind Resource types.NamespacedName @@ -153,6 +173,11 @@ func AsTransitiveDependencyNotReadyError(err error) (unwrappedErr *TransitiveDep return unwrappedErr, ok } +func IsTransitiveDependencyNotReadyError(err error) bool { + _, ok := AsTransitiveDependencyNotReadyError(err) + return ok +} + type ServerGeneratedIDNotFoundError struct { resourceGVK schema.GroupVersionKind resource types.NamespacedName