Remote Resolution Refactor

This PR hides the implementation of the internally used API in an internal package. This allows us to update the internal interfaces freely without breaking users.

NOTE that this is a migration of what I think is only internally used. Our docs on using the resolver framework do not use any of these and neither do our built-in resolvers.

docs/resolver-template/cmd/demoresolver/main.go

@@ -19,7 +19,7 @@ import (
 
 	pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	"github.com/tektoncd/pipeline/pkg/apis/resolution/v1beta1"
-	"github.com/tektoncd/pipeline/pkg/resolution/common"
+	"github.com/tektoncd/pipeline/pkg/resolution/resolver/common"
 	"github.com/tektoncd/pipeline/pkg/resolution/resolver/framework"
 	filteredinformerfactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"
 	"knative.dev/pkg/injection/sharedmain"

docs/resolver-template/cmd/demoresolver/main_test.go

@@ -23,7 +23,7 @@ import (
 
 	"github.com/tektoncd/pipeline/pkg/apis/resolution/v1beta1"
 	ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing"
-	resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common"
+	resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/resolver/common"
 	frtesting "github.com/tektoncd/pipeline/pkg/resolution/resolver/framework/testing"
 	"github.com/tektoncd/pipeline/test"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

pkg/remote/oci/resolver.go → internal/remote/oci/resolver.go

@@ -29,9 +29,9 @@ import (
 	imgname "github.com/google/go-containerregistry/pkg/name"
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 	ociremote "github.com/google/go-containerregistry/pkg/v1/remote"
+	"github.com/tektoncd/pipeline/internal/remote"
 	pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	"github.com/tektoncd/pipeline/pkg/client/clientset/versioned/scheme"
-	"github.com/tektoncd/pipeline/pkg/remote"
 	"k8s.io/apimachinery/pkg/runtime"
 )
 

pkg/remote/oci/resolver_test.go → internal/remote/oci/resolver_test.go

@@ -27,9 +27,9 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/google/go-containerregistry/pkg/registry"
+	"github.com/tektoncd/pipeline/internal/remote"
+	"github.com/tektoncd/pipeline/internal/remote/oci"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
-	"github.com/tektoncd/pipeline/pkg/remote"
-	"github.com/tektoncd/pipeline/pkg/remote/oci"
 	"github.com/tektoncd/pipeline/test"
 	"github.com/tektoncd/pipeline/test/diff"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

pkg/remote/resolution/request.go → internal/remote/resolution/request.go

@@ -14,7 +14,7 @@ limitations under the License.
 package resolution
 
 import (
-	resolution "github.com/tektoncd/pipeline/pkg/resolution/resource"
+	resolution "github.com/tektoncd/pipeline/internal/resolution/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"knative.dev/pkg/kmeta"
 )

pkg/remote/resolution/resolver.go → internal/remote/resolution/resolver.go

@@ -18,11 +18,11 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/tektoncd/pipeline/internal/remote"
+	remoteresource "github.com/tektoncd/pipeline/internal/resolution/resource"
 	v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	"github.com/tektoncd/pipeline/pkg/client/clientset/versioned/scheme"
-	"github.com/tektoncd/pipeline/pkg/remote"
-	resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common"
-	remoteresource "github.com/tektoncd/pipeline/pkg/resolution/resource"
+	"github.com/tektoncd/pipeline/pkg/resolution/resolver/common"
 	"k8s.io/apimachinery/pkg/runtime"
 	"knative.dev/pkg/kmeta"
 )
@@ -64,7 +64,7 @@ func (resolver *Resolver) Get(ctx context.Context, _, _ string) (runtime.Object,
 	}
 	resolved, err := resolver.requester.Submit(ctx, resolverName, req)
 	switch {
-	case errors.Is(err, resolutioncommon.ErrRequestInProgress):
+	case errors.Is(err, common.ErrRequestInProgress):
 		return nil, nil, remote.ErrRequestInProgress
 	case err != nil:
 		return nil, nil, fmt.Errorf("error requesting remote resource: %w", err)

pkg/remote/resolution/resolver_test.go → internal/remote/resolution/resolver_test.go

@@ -19,10 +19,10 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/tektoncd/pipeline/internal/remote"
+	remoteresource "github.com/tektoncd/pipeline/internal/resolution/resource"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
-	"github.com/tektoncd/pipeline/pkg/remote"
-	resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common"
-	remoteresource "github.com/tektoncd/pipeline/pkg/resolution/resource"
+	"github.com/tektoncd/pipeline/pkg/resolution/resolver/common"
 	"github.com/tektoncd/pipeline/test"
 	"github.com/tektoncd/pipeline/test/diff"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -90,7 +90,7 @@ func TestGet_Errors(t *testing.T) {
 		expectedGetErr   error
 		resolvedResource remoteresource.ResolvedResource
 	}{{
-		submitErr:        resolutioncommon.ErrRequestInProgress,
+		submitErr:        common.ErrRequestInProgress,
 		expectedGetErr:   remote.ErrRequestInProgress,
 		resolvedResource: nil,
 	}, {

pkg/resolution/resource/crd_resource.go → internal/resolution/resource/crd_resource.go

@@ -26,7 +26,7 @@ import (
 	"github.com/tektoncd/pipeline/pkg/apis/resolution/v1beta1"
 	rrclient "github.com/tektoncd/pipeline/pkg/client/resolution/clientset/versioned"
 	rrlisters "github.com/tektoncd/pipeline/pkg/client/resolution/listers/resolution/v1beta1"
-	resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common"
+	"github.com/tektoncd/pipeline/pkg/resolution/resolver/common"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"knative.dev/pkg/apis"
@@ -63,7 +63,7 @@ func (r *CRDRequester) Submit(ctx context.Context, resolver ResolverName, req Re
 			!apierrors.IsAlreadyExists(err) {
 			return nil, err
 		}
-		return nil, resolutioncommon.ErrRequestInProgress
+		return nil, common.ErrRequestInProgress
 	}
 
 	if rr.Status.GetCondition(apis.ConditionSucceeded).IsUnknown() {
@@ -72,15 +72,15 @@ func (r *CRDRequester) Submit(ctx context.Context, resolver ResolverName, req Re
 		// that it doesn't get deleted until the caller is done
 		// with it. Use appendOwnerReference and then submit
 		// update to ResolutionRequest.
-		return nil, resolutioncommon.ErrRequestInProgress
+		return nil, common.ErrRequestInProgress
 	}
 
 	if rr.Status.GetCondition(apis.ConditionSucceeded).IsTrue() {
 		return crdIntoResource(rr), nil
 	}
 
 	message := rr.Status.GetCondition(apis.ConditionSucceeded).GetMessage()
-	err := resolutioncommon.NewError(resolutioncommon.ReasonResolutionFailed, errors.New(message))
+	err := common.NewError(common.ReasonResolutionFailed, errors.New(message))
 	return nil, err
 }
 
@@ -94,7 +94,7 @@ func (r *CRDRequester) createResolutionRequest(ctx context.Context, resolver Res
 			Name:      req.Name(),
 			Namespace: req.Namespace(),
 			Labels: map[string]string{
-				resolutioncommon.LabelKeyResolverType: string(resolver),
+				common.LabelKeyResolverType: string(resolver),
 			},
 		},
 		Spec: v1beta1.ResolutionRequestSpec{

pkg/resolution/resource/crd_resource_test.go → internal/resolution/resource/crd_resource_test.go

@@ -23,10 +23,11 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	resolutioncommon "github.com/tektoncd/pipeline/internal/resolution/common"
+	"github.com/tektoncd/pipeline/internal/resolution/resource"
 	"github.com/tektoncd/pipeline/pkg/apis/resolution/v1beta1"
 	ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing"
-	resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common"
-	"github.com/tektoncd/pipeline/pkg/resolution/resource"
+	"github.com/tektoncd/pipeline/pkg/resolution/resolver/common"
 	"github.com/tektoncd/pipeline/test"
 	"github.com/tektoncd/pipeline/test/diff"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -156,15 +157,15 @@ conditions:
 			inputResolutionRequest:    nil,
 			expectedResolutionRequest: createdRR.DeepCopy(),
 			expectedResolvedResource:  nil,
-			expectedErr:               resolutioncommon.ErrRequestInProgress,
+			expectedErr:               common.ErrRequestInProgress,
 		},
 		{
 			name:                      "resolution request exist and status is unknown",
 			inputRequest:              request,
 			inputResolutionRequest:    unknownRR.DeepCopy(),
 			expectedResolutionRequest: nil,
 			expectedResolvedResource:  nil,
-			expectedErr:               resolutioncommon.ErrRequestInProgress,
+			expectedErr:               common.ErrRequestInProgress,
 		},
 		{
 			name:                      "resolution request exist and status is succeeded",
@@ -188,7 +189,7 @@ conditions:
 			inputResolutionRequest:    failedRR.DeepCopy(),
 			expectedResolutionRequest: nil,
 			expectedResolvedResource:  nil,
-			expectedErr:               resolutioncommon.NewError(resolutioncommon.ReasonResolutionFailed, errors.New("error message")),
+			expectedErr:               common.NewError(common.ReasonResolutionFailed, errors.New("error message")),
 		},
 	}
 

pkg/resolution/resource/name_test.go → internal/resolution/resource/name_test.go

@@ -19,8 +19,8 @@ package resource_test
 import (
 	"testing"
 
+	"github.com/tektoncd/pipeline/internal/resolution/resource"
 	v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
-	"github.com/tektoncd/pipeline/pkg/resolution/resource"
 )
 
 func TestGenerateDeterministicName(t *testing.T) {

pkg/resolution/resource/request_test.go → internal/resolution/resource/request_test.go

@@ -20,8 +20,8 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/tektoncd/pipeline/internal/resolution/resource"
 	v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
-	"github.com/tektoncd/pipeline/pkg/resolution/resource"
 	"github.com/tektoncd/pipeline/test/diff"
 )
 

pkg/resolution/resource/resource.go → internal/resolution/resource/resource.go

@@ -17,7 +17,7 @@ limitations under the License.
 package resource
 
 import (
-	"github.com/tektoncd/pipeline/pkg/resolution/common"
+	"github.com/tektoncd/pipeline/internal/resolution/common"
 )
 
 // This is an alias for avoiding cycle import

pkg/apis/resolution/v1alpha1/resolution_request_lifecycle.go

@@ -17,7 +17,7 @@ limitations under the License.
 package v1alpha1
 
 import (
-	resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common"
+	resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/resolver/common"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"knative.dev/pkg/apis"
 )

pkg/apis/resolution/v1alpha1/resolution_request_validation.go

@@ -19,7 +19,7 @@ package v1alpha1
 import (
 	"context"
 
-	"github.com/tektoncd/pipeline/pkg/resolution/common"
+	"github.com/tektoncd/pipeline/pkg/resolution/resolver/common"
 	"knative.dev/pkg/apis"
 )
 

pkg/apis/resolution/v1beta1/resolution_request_lifecycle.go

@@ -17,7 +17,7 @@ limitations under the License.
 package v1beta1
 
 import (
-	resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common"
+	resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/resolver/common"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"knative.dev/pkg/apis"
 )

pkg/apis/resolution/v1beta1/resolution_request_validation.go

@@ -19,7 +19,7 @@ package v1beta1
 import (
 	"context"
 
-	"github.com/tektoncd/pipeline/pkg/resolution/common"
+	"github.com/tektoncd/pipeline/pkg/resolution/resolver/common"
 	"knative.dev/pkg/apis"
 )
 

pkg/reconciler/pipelinerun/controller.go

@@ -19,6 +19,7 @@ package pipelinerun
 import (
 	"context"
 
+	resolution "github.com/tektoncd/pipeline/internal/resolution/resource"
 	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
 	v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
@@ -33,7 +34,6 @@ import (
 	"github.com/tektoncd/pipeline/pkg/pipelinerunmetrics"
 	cloudeventclient "github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent"
 	"github.com/tektoncd/pipeline/pkg/reconciler/volumeclaim"
-	resolution "github.com/tektoncd/pipeline/pkg/resolution/resource"
 	"github.com/tektoncd/pipeline/pkg/tracing"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/utils/clock"

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -28,6 +28,8 @@ import (
 	"time"
 
 	"github.com/hashicorp/go-multierror"
+	"github.com/tektoncd/pipeline/internal/remote"
+	resolution "github.com/tektoncd/pipeline/internal/resolution/resource"
 	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
 	pipelineErrors "github.com/tektoncd/pipeline/pkg/apis/pipeline/errors"
@@ -51,8 +53,6 @@ import (
 	"github.com/tektoncd/pipeline/pkg/reconciler/taskrun"
 	tresources "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources"
 	"github.com/tektoncd/pipeline/pkg/reconciler/volumeclaim"
-	"github.com/tektoncd/pipeline/pkg/remote"
-	resolution "github.com/tektoncd/pipeline/pkg/resolution/resource"
 	"github.com/tektoncd/pipeline/pkg/substitution"
 	"github.com/tektoncd/pipeline/pkg/trustedresources"
 	"github.com/tektoncd/pipeline/pkg/workspace"

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -31,6 +31,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/sigstore/sigstore/pkg/signature"
+	remoteresource "github.com/tektoncd/pipeline/internal/resolution/resource"
 	"github.com/tektoncd/pipeline/pkg/apis/config"
 	cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
@@ -48,8 +49,7 @@ import (
 	taskresources "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources"
 	ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing"
 	"github.com/tektoncd/pipeline/pkg/reconciler/volumeclaim"
-	resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common"
-	remoteresource "github.com/tektoncd/pipeline/pkg/resolution/resource"
+	"github.com/tektoncd/pipeline/pkg/resolution/resolver/common"
 	"github.com/tektoncd/pipeline/pkg/tracing"
 	"github.com/tektoncd/pipeline/pkg/trustedresources"
 	"github.com/tektoncd/pipeline/pkg/trustedresources/verifier"
@@ -8853,7 +8853,7 @@ spec:
 		t.Fatalf("expected resource request type %q but saw %q", resolutionRequestType, resolverName)
 	}
 
-	resreq.Status.MarkFailed(resolutioncommon.ReasonResolutionTimedOut, "resolution took longer than global timeout of 1 minute")
+	resreq.Status.MarkFailed(common.ReasonResolutionTimedOut, "resolution took longer than global timeout of 1 minute")
 	resreq, err = client.UpdateStatus(prt.TestAssets.Ctx, resreq, metav1.UpdateOptions{})
 	if err != nil {
 		t.Fatalf("unexpected error updating resource request with resolved pipeline data: %v", err)
@@ -8914,7 +8914,7 @@ spec:
 		t.Fatalf("expected resource request type %q but saw %q", resolutionRequestType, resolverName)
 	}
 
-	resreq.Status.MarkFailed(resolutioncommon.ReasonResolutionTimedOut, "resolution took longer than global timeout of 1 minute")
+	resreq.Status.MarkFailed(common.ReasonResolutionTimedOut, "resolution took longer than global timeout of 1 minute")
 	resreq, err = client.UpdateStatus(prt.TestAssets.Ctx, resreq, metav1.UpdateOptions{})
 	if err != nil {
 		t.Fatalf("unexpected error updating resource request with resolved pipeline data: %v", err)

pkg/reconciler/pipelinerun/resources/pipelineref.go

@@ -21,15 +21,15 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/tektoncd/pipeline/internal/remote"
+	"github.com/tektoncd/pipeline/internal/remote/resolution"
+	remoteresource "github.com/tektoncd/pipeline/internal/resolution/resource"
 	v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned"
 	"github.com/tektoncd/pipeline/pkg/reconciler/apiserver"
 	rprp "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/pipelinespec"
-	"github.com/tektoncd/pipeline/pkg/remote"
-	"github.com/tektoncd/pipeline/pkg/remote/resolution"
-	remoteresource "github.com/tektoncd/pipeline/pkg/resolution/resource"
 	"github.com/tektoncd/pipeline/pkg/trustedresources"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"

pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go

@@ -24,13 +24,13 @@ import (
 	"strings"
 
 	"github.com/google/cel-go/cel"
+	"github.com/tektoncd/pipeline/internal/remote"
 	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
 	pipelineErrors "github.com/tektoncd/pipeline/pkg/apis/pipeline/errors"
 	v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	"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/substitution"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"knative.dev/pkg/apis"

pkg/reconciler/resolutionrequest/resolutionrequest.go

@@ -23,7 +23,7 @@ import (
 
 	"github.com/tektoncd/pipeline/pkg/apis/resolution/v1beta1"
 	rrreconciler "github.com/tektoncd/pipeline/pkg/client/resolution/injection/reconciler/resolution/v1beta1/resolutionrequest"
-	resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common"
+	"github.com/tektoncd/pipeline/pkg/resolution/resolver/common"
 	"k8s.io/utils/clock"
 	"knative.dev/pkg/apis"
 	"knative.dev/pkg/controller"
@@ -61,9 +61,9 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, rr *v1beta1.ResolutionRe
 	case rr.Status.Data != "":
 		rr.Status.MarkSucceeded()
 	case requestDuration(rr) > defaultMaximumResolutionDuration:
-		rr.Status.MarkFailed(resolutioncommon.ReasonResolutionTimedOut, timeoutMessage())
+		rr.Status.MarkFailed(common.ReasonResolutionTimedOut, timeoutMessage())
 	default:
-		rr.Status.MarkInProgress(resolutioncommon.MessageWaitingForResolver)
+		rr.Status.MarkInProgress(common.MessageWaitingForResolver)
 		return controller.NewRequeueAfter(defaultMaximumResolutionDuration - requestDuration(rr))
 	}