Ensure list is retried when object resolution fails

In order for the error producer and the error consumer can work
together, we extract the error type and the error predicate in a
dedicated package and define a constructor for the error

The error producer must use the constructor (as the error type is
package visible), and the consumer must use the predefined predicate (as
it cannot create a pointer to the type).

This way we guarantee that no type mismatch will happen

Co-authored-by: Danail Branekov <[email protected]>

Author: georgethebeatle

api/repositories/k8sklient/descriptors/errors/resolution.go

@@ -0,0 +1,28 @@
+package errors
+
+import (
+	"errors"
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+type objectResolutionError struct {
+	guid string
+	gvk  schema.GroupVersionKind
+}
+
+func (e objectResolutionError) Error() string {
+	return fmt.Sprintf("item not found: guid %q, gvk %q", e.guid, e.gvk)
+}
+
+func NewObjectResolutionError(guid string, gvk schema.GroupVersionKind) error {
+	return objectResolutionError{
+		guid: guid,
+		gvk:  gvk,
+	}
+}
+
+func IsObjectResolutionError(err error) bool {
+	return errors.As(err, new(objectResolutionError))
+}

api/repositories/k8sklient/descriptors/errors/resolution_test.go

@@ -0,0 +1,49 @@
+package errors_test
+
+import (
+	"errors"
+
+	desc_errs "code.cloudfoundry.org/korifi/api/repositories/k8sklient/descriptors/errors"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+var _ = Describe("IsObjectResolutionError", func() {
+	var (
+		isObjectResolutionErr bool
+		err                   error
+	)
+
+	BeforeEach(func() {
+		err = nil
+	})
+
+	JustBeforeEach(func() {
+		isObjectResolutionErr = desc_errs.IsObjectResolutionError(err)
+	})
+
+	It("returns false", func() {
+		Expect(isObjectResolutionErr).To(BeFalse())
+	})
+
+	When("the error is a ObjectResulutionError", func() {
+		BeforeEach(func() {
+			err = desc_errs.NewObjectResolutionError("1234", schema.GroupVersionKind{})
+		})
+
+		It("returns true", func() {
+			Expect(isObjectResolutionErr).To(BeTrue())
+		})
+	})
+
+	When("the error is a random error", func() {
+		BeforeEach(func() {
+			err = errors.New("foo")
+		})
+
+		It("returns false", func() {
+			Expect(isObjectResolutionErr).To(BeFalse())
+		})
+	})
+})

api/repositories/k8sklient/descriptors/errors/suite_test.go

@@ -0,0 +1,13 @@
+package errors_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestErrors(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Errors Suite")
+}

api/repositories/k8sklient/descriptors/mapper.go

@@ -6,6 +6,7 @@ import (
 	"reflect"
 
 	"code.cloudfoundry.org/korifi/api/authorization"
+	"code.cloudfoundry.org/korifi/api/repositories/k8sklient/descriptors/errors"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -60,15 +61,6 @@ func (m *ObjectListMapper) GUIDsToObjectList(ctx context.Context, listObjectGVK
 	return order(orderedGUIDs, listObjectGVK, list)
 }
 
-type ObjectResolutionError struct {
-	guid string
-	gvk  schema.GroupVersionKind
-}
-
-func (e ObjectResolutionError) Error() string {
-	return fmt.Sprintf("item not found: guid %q, gvk %q", e.guid, e.gvk)
-}
-
 func order(orderedGUIDs []string, listObjectGVK schema.GroupVersionKind, list client.ObjectList) (client.ObjectList, error) {
 	resultList, err := scheme.Scheme.New(listObjectGVK)
 	if err != nil {
@@ -84,10 +76,7 @@ func order(orderedGUIDs []string, listObjectGVK schema.GroupVersionKind, list cl
 	for _, guid := range orderedGUIDs {
 		item, ok := itemsIndex[guid]
 		if !ok {
-			return nil, &ObjectResolutionError{
-				guid: guid,
-				gvk:  listObjectGVK,
-			}
+			return nil, errors.NewObjectResolutionError(guid, listObjectGVK)
 		}
 		orderedObjects = append(orderedObjects, item)
 	}

api/repositories/k8sklient/k8sklient_suite_test.go

@@ -2,10 +2,13 @@ package k8sklient_test
 
 import (
 	"context"
+	"log"
 	"testing"
 
 	"code.cloudfoundry.org/korifi/api/authorization"
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
+	"github.com/go-logr/logr"
+	"github.com/go-logr/stdr"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -27,5 +30,6 @@ var _ = BeforeEach(func() {
 		Token: "i-am-user",
 	}
 	ctx = authorization.NewContext(context.Background(), &authInfo)
+	ctx = logr.NewContext(ctx, stdr.New(log.New(GinkgoWriter, ">>>", log.LstdFlags)))
 	utilruntime.Must(korifiv1alpha1.AddToScheme(scheme.Scheme))
 })

api/repositories/k8sklient/lister.go

@@ -2,11 +2,11 @@ package k8sklient
 
 import (
 	"context"
-	"errors"
 	"fmt"
 
 	"code.cloudfoundry.org/korifi/api/repositories"
 	"code.cloudfoundry.org/korifi/api/repositories/k8sklient/descriptors"
+	"code.cloudfoundry.org/korifi/api/repositories/k8sklient/descriptors/errors"
 	"code.cloudfoundry.org/korifi/tools/k8s"
 	"github.com/go-logr/logr"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -57,7 +57,7 @@ func (k *DescriptorsBasedLister) retryList(ctx context.Context, listObjectGVK sc
 		err        error
 	)
 
-	err = retry.OnError(k8s.NewDefaultBackoff(), isObjectResolutionError, func() error {
+	err = retry.OnError(k8s.NewDefaultBackoff(), errors.IsObjectResolutionError, func() error {
 		listResult, pageInfo, err = k.descriptorsBasedList(ctx, listObjectGVK, listOpts)
 		if err != nil {
 			logger.Info("failed to resolve objects, retrying", "error", err)
@@ -68,16 +68,6 @@ func (k *DescriptorsBasedLister) retryList(ctx context.Context, listObjectGVK sc
 	return listResult, pageInfo, err
 }
 
-func isObjectResolutionError(err error) bool {
-	if err == nil {
-		return false
-	}
-	if errors.As(err, new(descriptors.ObjectResolutionError)) {
-		return true
-	}
-	return false
-}
-
 func (k *DescriptorsBasedLister) descriptorsBasedList(ctx context.Context, listObjectGVK schema.GroupVersionKind, listOpts repositories.ListOptions) (client.ObjectList, descriptors.PageInfo, error) {
 	objectGUIDs, err := k.fetchObjectGUIDs(ctx, listObjectGVK, listOpts)
 	if err != nil {

api/repositories/k8sklient/lister_test.go

@@ -11,6 +11,7 @@ import (
 	"code.cloudfoundry.org/korifi/api/repositories"
 	"code.cloudfoundry.org/korifi/api/repositories/k8sklient"
 	"code.cloudfoundry.org/korifi/api/repositories/k8sklient/descriptors"
+	desc_errs "code.cloudfoundry.org/korifi/api/repositories/k8sklient/descriptors/errors"
 	descfake "code.cloudfoundry.org/korifi/api/repositories/k8sklient/descriptors/fake"
 	"code.cloudfoundry.org/korifi/api/repositories/k8sklient/fake"
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
@@ -163,7 +164,7 @@ var _ = Describe("Lister", func() {
 				},
 			}
 
-			objectListMapper.GUIDsToObjectListReturnsOnCall(0, nil, descriptors.ObjectResolutionError{})
+			objectListMapper.GUIDsToObjectListReturnsOnCall(0, nil, desc_errs.NewObjectResolutionError("1234", schema.GroupVersionKind{}))
 			objectListMapper.GUIDsToObjectListReturns(appList, nil)
 		})