Merge pull request GoogleCloudPlatform#1306 from justinsb/fix_errors_as

chore: avoid errors.Is, replace with IsFoo
gemmahou · Feb 28, 2024 · 8313dcc · 8313dcc
2 parents bea1e19 + 3652a41
commit 8313dcc
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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