handle TaskNotFoundError where identification comes from params

pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go

@@ -31,6 +31,7 @@ import (
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources"
 	"github.com/tektoncd/pipeline/pkg/remote"
+	"github.com/tektoncd/pipeline/pkg/resolution/resource"
 	"github.com/tektoncd/pipeline/pkg/substitution"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"knative.dev/pkg/apis"
@@ -645,8 +646,12 @@ func resolveTask(
 			case errors.Is(err, remote.ErrRequestInProgress):
 				return rt, err
 			case err != nil:
+				name := pipelineTask.TaskRef.Name
+				if len(strings.TrimSpace(name)) == 0 {
+					name = resource.GenerateErrorLogString(string(pipelineTask.TaskRef.Resolver), pipelineTask.TaskRef.Params)
+				}
 				return rt, &TaskNotFoundError{
-					Name: pipelineTask.TaskRef.Name,
+					Name: name,
 					Msg:  err.Error(),
 				}
 			default:

pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go

@@ -2490,6 +2490,18 @@ func TestResolvePipelineRun_TaskDoesntExist(t *testing.T) {
 				Value: *v1.NewStructuredValues("b", "a", "r"),
 			}},
 		},
+	}, {
+		Name:    "mytask3",
+		TaskRef: &v1.TaskRef{ResolverRef: v1.ResolverRef{Params: v1.Params{{Name: "name", Value: v1.ParamValue{Type: v1.ParamTypeString, StringVal: "foo"}}}}},
+		Matrix: &v1.Matrix{
+			Params: v1.Params{{
+				Name:  "foo",
+				Value: *v1.NewStructuredValues("f", "o", "o"),
+			}, {
+				Name:  "bar",
+				Value: *v1.NewStructuredValues("b", "a", "r"),
+			}},
+		},
 	}}
 
 	// Return an error when the Task is retrieved, as if it didn't exist
@@ -2512,6 +2524,9 @@ func TestResolvePipelineRun_TaskDoesntExist(t *testing.T) {
 			t.Fatalf("Pipeline %s: want error, got nil", p.Name)
 		case errors.As(err, &tnf):
 			// expected error
+			if len(tnf.Name) == 0 {
+				t.Fatalf("Pipeline %s: TaskNotFoundError did not have name set: %s", p.Name, tnf.Error())
+			}
 		default:
 			t.Fatalf("Pipeline %s: Want %T, got %s of type %T", p.Name, tnf, err, err)
 		}

pkg/resolution/resolver/bundle/params.go

@@ -21,6 +21,7 @@ import (
 	"github.com/google/go-containerregistry/pkg/name"
 	pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	"github.com/tektoncd/pipeline/pkg/resolution/resolver/framework"
+	"github.com/tektoncd/pipeline/pkg/resolution/resource"
 )
 
 // ParamImagePullSecret is the parameter defining what secret
@@ -32,7 +33,7 @@ const ParamBundle = "bundle"
 
 // ParamName is the parameter defining what the layer name in the bundle
 // image is.
-const ParamName = "name"
+const ParamName = resource.ParamName
 
 // ParamKind is the parameter defining what the layer kind in the bundle
 // image is.

pkg/resolution/resolver/framework/reconciler.go

@@ -215,13 +215,15 @@ func (r *Reconciler) writeResolvedData(ctx context.Context, rr *v1beta1.Resoluti
 		},
 	})
 	if err != nil {
+		logging.FromContext(ctx).Warnf("writeResolvedData error serializing resource request patch for resolution request %s:%s: %w", rr.Namespace, rr.Name, err)
 		return r.OnError(ctx, rr, &resolutioncommon.UpdatingRequestError{
 			ResolutionRequestKey: fmt.Sprintf("%s/%s", rr.Namespace, rr.Name),
 			Original:             fmt.Errorf("error serializing resource request patch: %w", err),
 		})
 	}
 	_, err = r.resolutionRequestClientSet.ResolutionV1beta1().ResolutionRequests(rr.Namespace).Patch(ctx, rr.Name, types.MergePatchType, patchBytes, metav1.PatchOptions{}, "status")
 	if err != nil {
+		logging.FromContext(ctx).Warnf("writeResolvedData error patching resolution request %s:%s: %w", rr.Namespace, rr.Name, err)
 		return r.OnError(ctx, rr, &resolutioncommon.UpdatingRequestError{
 			ResolutionRequestKey: fmt.Sprintf("%s/%s", rr.Namespace, rr.Name),
 			Original:             err,

pkg/resolution/resolver/git/params.go

@@ -16,9 +16,11 @@ limitations under the License.
 
 package git
 
+import "github.com/tektoncd/pipeline/pkg/resolution/resource"
+
 const (
 	// urlParam is the git repo url when using the anonymous/full clone approach
-	urlParam string = "url"
+	urlParam string = resource.ParamURL
 	// orgParam is the organization to find the repository in when using the SCM API approach
 	orgParam = "org"
 	// repoParam is the repository to use when using the SCM API approach

pkg/resolution/resolver/http/params.go

@@ -13,9 +13,11 @@ limitations under the License.
 
 package http
 
+import "github.com/tektoncd/pipeline/pkg/resolution/resource"
+
 const (
 	// urlParam is the URL to fetch the task from
-	urlParam string = "url"
+	urlParam string = resource.ParamURL
 
 	// httpBasicAuthUsername is the user name to use for basic auth
 	httpBasicAuthUsername string = "http-username"

pkg/resolution/resolver/hub/params.go

@@ -13,6 +13,8 @@ limitations under the License.
 
 package hub
 
+import "github.com/tektoncd/pipeline/pkg/resolution/resource"
+
 // DefaultArtifactHubURL is the default url for the Artifact hub api
 const DefaultArtifactHubURL = "https://artifacthub.io"
 
@@ -30,7 +32,7 @@ const ArtifactHubListTasksEndpoint = "api/v1/packages/tekton-%s/%s/%s"
 
 // ParamName is the parameter defining what the layer name in the bundle
 // image is.
-const ParamName = "name"
+const ParamName = resource.ParamName
 
 // ParamKind is the parameter defining what the layer kind in the bundle
 // image is.

pkg/resolution/resource/name.go

@@ -25,6 +25,16 @@ import (
 	v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 )
 
+const (
+	// ParamName is a param that explicitly assigns a name to the remote object
+	ParamName = "name"
+
+	// ParamURL is a param that hold the URL used for accesing the remote object
+	ParamURL = "url"
+)
+
+//
+
 // nameHasher returns the hash.Hash to use when generating names.
 func nameHasher() hash.Hash {
 	return fnv.New128a()
@@ -69,3 +79,28 @@ func GenerateDeterministicName(prefix, base string, params v1.Params) (string, e
 	}
 	return fmt.Sprintf("%s-%x", prefix, hasher.Sum(nil)), nil
 }
+
+// GenerateErrorLogString makes a best effort attempt to get the name of the task
+// when a resolver error occurred.  The TaskRef name does not have to be set, where
+// the specific resolver gets the name from the parameters.
+func GenerateErrorLogString(resolverType string, params v1.Params) string {
+	paramString := fmt.Sprintf("resolver type %s\n", resolverType)
+	for _, p := range params {
+		if p.Name == ParamName {
+			name := p.Value.StringVal
+			if p.Value.Type != v1.ParamTypeString {
+				asJSON, err := p.Value.MarshalJSON()
+				if err != nil {
+					paramString += fmt.Sprintf("name could not be marshalled: %s\n", err.Error())
+					continue
+				}
+				name = string(asJSON)
+			}
+			paramString += fmt.Sprintf("name = %s\n", name)
+		}
+		if p.Name == ParamURL {
+			paramString += fmt.Sprintf("url = %s\n", p.Value.StringVal)
+		}
+	}
+	return paramString
+}

pkg/resolution/resource/name_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package resource_test
 
 import (
+	"strings"
 	"testing"
 
 	v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
@@ -113,3 +114,97 @@ func TestGenerateDeterministicName(t *testing.T) {
 		})
 	}
 }
+
+func TestGenerateErrorLogString(t *testing.T) {
+	tests := []struct {
+		resolverType string
+		name         string
+		url          string
+		err          string
+		params       []v1.Param
+		isPresent    bool
+	}{
+		{
+			name:         "foo",
+			url:          "https://bar",
+			resolverType: "git",
+			isPresent:    true,
+			params: []v1.Param{
+				{
+					Name: resource.ParamName,
+					Value: v1.ParamValue{
+						Type:      v1.ParamTypeString,
+						StringVal: "foo",
+					},
+				},
+				{
+					Name: resource.ParamURL,
+					Value: v1.ParamValue{
+						Type:      v1.ParamTypeString,
+						StringVal: "https://bar",
+					},
+				},
+			},
+		},
+		{
+			name:         "foo",
+			url:          "https://bar",
+			resolverType: "",
+			err:          "name could not be marshalled",
+			params:       []v1.Param{},
+		},
+		{
+			name:         "goo",
+			resolverType: "bundle",
+			isPresent:    true,
+			params: []v1.Param{
+				{
+					Name: resource.ParamName,
+					Value: v1.ParamValue{
+						Type:      v1.ParamTypeString,
+						StringVal: "goo",
+					},
+				},
+			},
+		},
+		{
+			name:         "hoo",
+			resolverType: "cluster",
+			err:          "name could not be marshalled",
+			isPresent:    true,
+			params: []v1.Param{
+				{
+					Name: resource.ParamName,
+					Value: v1.ParamValue{
+						Type:      v1.ParamTypeString,
+						StringVal: "hoo",
+					},
+				},
+				{
+					Name: resource.ParamName,
+					Value: v1.ParamValue{
+						Type: v1.ParamType("foo"),
+					},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := resource.GenerateErrorLogString(tt.resolverType, tt.params)
+			if strings.Contains(got, tt.name) != tt.isPresent {
+				t.Errorf("name %s presence in %s should be %v", tt.name, got, tt.isPresent)
+			}
+			if strings.Contains(got, tt.url) != tt.isPresent {
+				t.Errorf("url %s presence in %s should be %v", tt.url, got, tt.isPresent)
+			}
+			if strings.Contains(got, tt.err) != tt.isPresent {
+				t.Errorf("err %s presence in %s should be %v", tt.err, got, tt.isPresent)
+			}
+			// should always have resolver type
+			if !strings.Contains(got, tt.resolverType) {
+				t.Errorf("type %s not in %s", tt.resolverType, got)
+			}
+		})
+	}
+}