Repository: knative/pkg (go)

Issue:
Addressable resolver discard original error when a lister factory fails
<!--
Pro-tip: You can leave this block commented, and it still works!

Select the appropriate areas for your issue:

/area API
/area monitoring
/area networking
/area test-and-release

Classify what kind of issue this is:

/kind question
/kind bug
/kind cleanup
/kind doc
/kind feature
/kind good-first-issue

-->

## Expected Behavior

The error is specific and the original error isn't discarded

## Actual Behavior

I get a generic error like:
`kafkachannels "Lister" not found`

## Additional Info

/kind bug

Pull request:
Don't discard addressable resolver original errors
Errors from the tracker or the addressable resolver factory
are discarded which isn't helpful for debugging since the
error is generic `kafkachannels "Lister" not found`.

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

<!-- Thanks for sending a pull request! -->

# Changes

<!-- 
Describe your changes here- ideally you can get that description straight from
your descriptive commit message(s)! 

- :gift: Add new feature
- :bug: Fix bug
- :broom: Update or clean up current behavior
- :wastebasket: Remove feature or internal logic
-->

- Don't discard addressable resolver original errors

<!--
In addition, categorize the changes you're making using the "/kind" Prow command, example:

/kind <kind>

Supported kinds are: api-change, bug, cleanup, deprecation, removal, documentation, enhancement, performance

-->
/kind bug

<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #2394 

<!-- Please include the 'why' behind your changes if no issue exists -->

Patch:
diff --git a/resolver/addressable_resolver.go b/resolver/addressable_resolver.go
index 0e50a2578..d6995947a 100644
--- a/resolver/addressable_resolver.go
+++ b/resolver/addressable_resolver.go
@@ -23,12 +23,14 @@ import (
 
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/tools/cache"
+
 	"knative.dev/pkg/client/injection/ducks/duck/v1/addressable"
 	"knative.dev/pkg/controller"
 
 	corev1 "k8s.io/api/core/v1"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
+
 	"knative.dev/pkg/apis"
 	pkgapisduck "knative.dev/pkg/apis/duck"
 	duckv1 "knative.dev/pkg/apis/duck/v1"
@@ -169,17 +171,17 @@ func (r *URIResolver) URIFromObjectReference(ctx context.Context, ref *corev1.Ob
 		Namespace:  ref.Namespace,
 		Name:       ref.Name,
 	}, parent); err != nil {
-		return nil, apierrs.NewNotFound(gvr.GroupResource(), ref.Name)
+		return nil, fmt.Errorf("failed to track reference %s %s/%s: %w", gvr.String(), ref.Namespace, ref.Name, err)
 	}
 
 	lister, err := r.listerFactory(gvr)
 	if err != nil {
-		return nil, apierrs.NewNotFound(gvr.GroupResource(), "Lister")
+		return nil, fmt.Errorf("failed to get lister for %s: %w", gvr.String(), err)
 	}
 
 	obj, err := lister.ByNamespace(ref.Namespace).Get(ref.Name)
 	if err != nil {
-		return nil, apierrs.NewNotFound(gvr.GroupResource(), ref.Name)
+		return nil, fmt.Errorf("failed to get object %s/%s: %w", ref.Namespace, ref.Name, err)
 	}
 
 	// K8s Services are special cased. They can be called, even though they do not satisfy the


Test patch:
diff --git a/resolver/addressable_resolver_test.go b/resolver/addressable_resolver_test.go
index 56c195cc8..86526c5a0 100644
--- a/resolver/addressable_resolver_test.go
+++ b/resolver/addressable_resolver_test.go
@@ -29,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
+
 	"knative.dev/pkg/apis"
 	duckv1 "knative.dev/pkg/apis/duck/v1"
 	duckv1alpha1 "knative.dev/pkg/apis/duck/v1alpha1"
@@ -348,7 +349,7 @@ func TestGetURIDestinationV1Beta1(t *testing.T) {
 			wantErr: fmt.Sprintf("address not set for %+v", unaddressableRef()),
 		}, "notFound": {
 			dest:    duckv1beta1.Destination{Ref: unaddressableRef()},
-			wantErr: fmt.Sprintf("%s %q not found", unaddressableResource, unaddressableName),
+			wantErr: fmt.Sprintf("failed to get object %s/%s: %s %q not found", testNS, unaddressableName, unaddressableResource, unaddressableName),
 		}}
 
 	for n, tc := range tests {
@@ -531,10 +532,10 @@ func TestGetURIDestinationV1(t *testing.T) {
 			wantErr: fmt.Sprintf("address not set for %+v", unaddressableRef()),
 		}, "notFound": {
 			dest:    duckv1.Destination{Ref: unaddressableKnativeRef()},
-			wantErr: fmt.Sprintf("%s %q not found", unaddressableResource, unaddressableName),
+			wantErr: fmt.Sprintf("failed to get object %s/%s: %s %q not found", testNS, unaddressableName, unaddressableResource, unaddressableName),
 		}, "notFound k8s service": {
 			dest:    duckv1.Destination{Ref: k8sServiceRef()},
-			wantErr: fmt.Sprintf("services %q not found", addressableName),
+			wantErr: fmt.Sprintf("failed to get object %s/%s: services %q not found", testNS, addressableName, addressableName),
 		}, "with sample resolver": {
 			dest:            duckv1.Destination{Ref: k8sServiceRef()},
 			customResolvers: []resolver.RefResolverFunc{sampleURIResolver},
@@ -595,11 +596,12 @@ func TestURIFromObjectReferenceErrors(t *testing.T) {
 	}{"nil": {
 		wantErr: "ref is nil",
 	}, "fail tracker with bad object": {
-		ref:     invalidObjectRef(),
-		wantErr: `sinks.duck.knative.dev "testsink" not found`,
+		ref: invalidObjectRef(),
+		wantErr: `failed to track reference duck.knative.dev/v1, Resource=sinks -bad/testsink: invalid Reference:
+Namespace: a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name',  or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')`,
 	}, "fail get": {
 		ref:     addressableRef(),
-		wantErr: `sinks.duck.knative.dev "testsink" not found`,
+		wantErr: `failed to get object testnamespace/testsink: sinks.duck.knative.dev "testsink" not found`,
 	}}
 
 	for n, tc := range tests {
@@ -619,8 +621,8 @@ func TestURIFromObjectReferenceErrors(t *testing.T) {
 			if got, want := err2.Error(), tc.wantErr; got != want {
 				t.Errorf("Unexpected error (-want, +got) =\n%s", cmp.Diff(want, got))
 			}
-			if !cmp.Equal(err1, err2) {
-				t.Errorf("Idempotency fail: first err = %v, second err = %v", err1, err2)
+			if err1.Error() != err2.Error() {
+				t.Errorf("Idempotency fail: first err = %+v, second err = %+v", err1, err2)
 			}
 		})
 	}