TEP0154 - Enable concise resolver syntax

This PR enables concise resolver syntax interface.

config/config-feature-flags.yaml

@@ -137,3 +137,5 @@ data:
   # "pipelinerun" for Pipelinerun and "taskrun" for Taskrun. Or a combination of
   # these.
   disable-inline-spec: ""
+  # Setting this flag to "true" will enable the use of concise resolver syntax
+  enable-concise-resolver-syntax: "false"

docs/how-to-write-a-resolver.md

@@ -255,6 +255,7 @@ import (
 The `Validate` method checks that the resolution-spec submitted as part of
 a resolution request are valid. Our example resolver doesn't expect
 any params in the spec so we'll simply ensure that the there are no params.
+Our example resolver also expects format for the `url` to be `demoscheme://<path>` so we'll validate this format.
 In the previous version, this was instead called `ValidateParams` method. See below 
 for the differences.
 
@@ -268,8 +269,24 @@ func (r *resolver) Validate(ctx context.Context, req *v1beta1.ResolutionRequestS
   if len(req.Params) > 0 {
     return errors.New("no params allowed")
   }
+  url := req.URL
+  u, err := neturl.ParseRequestURI(url)
+  if err != nil {
+    return err
+  }
+  if u.Scheme != "demoscheme" {
+    return fmt.Errorf("Invalid Scheme. Want %s, Got %s", "demoscheme", u.Scheme)
+  }
+  if u.Path == "" {
+    return errors.New("Empty path.")
+  }
   return nil
 }
+```
+
+You'll also need to add the `net/url` as `neturl` and `"errors"` package to your list of imports at
+the top of the file.
+
 ```
 {{% /tab %}}
 
@@ -295,8 +312,8 @@ the top of the file.
 ## The `Resolve` method
 
 We implement the `Resolve` method to do the heavy lifting of fetching
-the contents of a file and returning them. For this example we're just
-going to return a hard-coded string of YAML. Since Tekton Pipelines
+the contents of a file and returning them.  It takes in the resolution request spec as input.
+For this example we're just going to return a hard-coded string of YAML. Since Tekton Pipelines
 currently only supports fetching Pipeline resources via remote
 resolution that's what we'll return.
 
@@ -465,6 +482,7 @@ func (*myResolvedResource) RefSource() *pipelinev1.RefSource {
 }
 ```
 
+
 ## The deployment configuration
 
 Finally, our resolver needs some deployment configuration so that it can

docs/pipeline-api.md

@@ -304,6 +304,22 @@ resource being requested. For example: repo URL, commit SHA,
 path to file, the kind of authentication to leverage, etc.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>url</code><br/>
+<em>
+string
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>URL is the runtime url passed to the resolver
+to help it figure out how to resolver the resource being
+requested.
+This is currently at an ALPHA stability level and subject to
+alpha API compatibility policies.</p>
+</td>
+</tr>
 </table>
 </td>
 </tr>
@@ -358,6 +374,22 @@ resource being requested. For example: repo URL, commit SHA,
 path to file, the kind of authentication to leverage, etc.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>url</code><br/>
+<em>
+string
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>URL is the runtime url passed to the resolver
+to help it figure out how to resolver the resource being
+requested.
+This is currently at an ALPHA stability level and subject to
+alpha API compatibility policies.</p>
+</td>
+</tr>
 </tbody>
 </table>
 <h3 id="resolution.tekton.dev/v1beta1.ResolutionRequestStatus">ResolutionRequestStatus

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

@@ -16,6 +16,8 @@ package main
 import (
 	"context"
 	"errors"
+	"fmt"
+	neturl "net/url"
 
 	pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	"github.com/tektoncd/pipeline/pkg/apis/resolution/v1beta1"
@@ -57,6 +59,13 @@ func (r *resolver) Validate(ctx context.Context, req *v1beta1.ResolutionRequestS
 	if len(req.Params) > 0 {
 		return errors.New("no params allowed")
 	}
+	u, err := neturl.ParseRequestURI(req.URL)
+	if err != nil {
+		return err
+	}
+	if u.Scheme != "demoscheme" {
+		return fmt.Errorf("Invalid Scheme. Want %s, Got %s", "demoscheme", u.Scheme)
+	}
 	return nil
 }
 

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

@@ -18,15 +18,18 @@ package main
 
 import (
 	"encoding/base64"
+	"errors"
 	"testing"
 	"time"
 
+	pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	"github.com/tektoncd/pipeline/pkg/apis/resolution/v1beta1"
 	ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing"
 	frtesting "github.com/tektoncd/pipeline/pkg/remoteresolution/resolver/framework/testing"
 	resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common"
 	"github.com/tektoncd/pipeline/test"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	v1 "knative.dev/pkg/apis/duck/v1"
 	_ "knative.dev/pkg/system/testing"
 )
 
@@ -48,7 +51,9 @@ func TestResolver(t *testing.T) {
 				resolutioncommon.LabelKeyResolverType: "demo",
 			},
 		},
-		Spec: v1beta1.ResolutionRequestSpec{},
+		Spec: v1beta1.ResolutionRequestSpec{
+			URL: "demoscheme://foo/bar",
+		},
 	}
 	d := test.Data{
 		ResolutionRequests: []*v1beta1.ResolutionRequest{request},
@@ -65,3 +70,138 @@ func TestResolver(t *testing.T) {
 
 	frtesting.RunResolverReconcileTest(ctx, t, d, r, request, expectedStatus, expectedErr)
 }
+
+func TestResolver_Failure_Wrong_Scheme(t *testing.T) {
+	ctx, _ := ttesting.SetupFakeContext(t)
+
+	r := &resolver{}
+
+	request := &v1beta1.ResolutionRequest{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "resolution.tekton.dev/v1beta1",
+			Kind:       "ResolutionRequest",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:              "rr",
+			Namespace:         "foo",
+			CreationTimestamp: metav1.Time{Time: time.Now()},
+			Labels: map[string]string{
+				resolutioncommon.LabelKeyResolverType: "demo",
+			},
+		},
+		Spec: v1beta1.ResolutionRequestSpec{
+			URL: "wrongscheme://foo/bar",
+		},
+	}
+	d := test.Data{
+		ResolutionRequests: []*v1beta1.ResolutionRequest{request},
+	}
+
+	expectedStatus := &v1beta1.ResolutionRequestStatus{
+		Status: v1.Status{
+			Conditions: v1.Conditions{
+				{
+					Type:    "Succeeded",
+					Status:  "False",
+					Reason:  "ResolutionFailed",
+					Message: `invalid resource request "foo/rr": Invalid Scheme. Want demoscheme, Got wrongscheme`,
+				},
+			},
+		},
+	}
+
+	// If you want to test scenarios where an error should occur, pass a non-nil error to RunResolverReconcileTest
+	expectedErr := errors.New(`invalid resource request "foo/rr": Invalid Scheme. Want demoscheme, Got wrongscheme`)
+	frtesting.RunResolverReconcileTest(ctx, t, d, r, request, expectedStatus, expectedErr)
+}
+
+func TestResolver_Failure_InvalidUrl(t *testing.T) {
+	ctx, _ := ttesting.SetupFakeContext(t)
+
+	r := &resolver{}
+
+	request := &v1beta1.ResolutionRequest{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "resolution.tekton.dev/v1beta1",
+			Kind:       "ResolutionRequest",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:              "rr",
+			Namespace:         "foo",
+			CreationTimestamp: metav1.Time{Time: time.Now()},
+			Labels: map[string]string{
+				resolutioncommon.LabelKeyResolverType: "demo",
+			},
+		},
+		Spec: v1beta1.ResolutionRequestSpec{
+			URL: "foo/bar",
+		},
+	}
+	d := test.Data{
+		ResolutionRequests: []*v1beta1.ResolutionRequest{request},
+	}
+
+	expectedStatus := &v1beta1.ResolutionRequestStatus{
+		Status: v1.Status{
+			Conditions: v1.Conditions{
+				{
+					Type:    "Succeeded",
+					Status:  "False",
+					Reason:  "ResolutionFailed",
+					Message: `invalid resource request "foo/rr": parse "foo/bar": invalid URI for request`,
+				},
+			},
+		},
+	}
+
+	// If you want to test scenarios where an error should occur, pass a non-nil error to RunResolverReconcileTest
+	expectedErr := errors.New(`invalid resource request "foo/rr": parse "foo/bar": invalid URI for request`)
+	frtesting.RunResolverReconcileTest(ctx, t, d, r, request, expectedStatus, expectedErr)
+}
+
+func TestResolver_Failure_InvalidParams(t *testing.T) {
+	ctx, _ := ttesting.SetupFakeContext(t)
+
+	r := &resolver{}
+
+	request := &v1beta1.ResolutionRequest{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "resolution.tekton.dev/v1beta1",
+			Kind:       "ResolutionRequest",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:              "rr",
+			Namespace:         "foo",
+			CreationTimestamp: metav1.Time{Time: time.Now()},
+			Labels: map[string]string{
+				resolutioncommon.LabelKeyResolverType: "demo",
+			},
+		},
+		Spec: v1beta1.ResolutionRequestSpec{
+			Params: []pipelinev1.Param{{
+				Name:  "foo",
+				Value: *pipelinev1.NewStructuredValues("bar"),
+			}},
+		},
+	}
+	d := test.Data{
+		ResolutionRequests: []*v1beta1.ResolutionRequest{request},
+	}
+
+	expectedStatus := &v1beta1.ResolutionRequestStatus{
+		Status: v1.Status{
+			Conditions: v1.Conditions{
+				{
+					Type:    "Succeeded",
+					Status:  "False",
+					Reason:  "ResolutionFailed",
+					Message: `invalid resource request "foo/rr": no params allowed`,
+				},
+			},
+		},
+	}
+
+	// If you want to test scenarios where an error should occur, pass a non-nil error to RunResolverReconcileTest
+	expectedErr := errors.New(`invalid resource request "foo/rr": no params allowed`)
+	frtesting.RunResolverReconcileTest(ctx, t, d, r, request, expectedStatus, expectedErr)
+}

pkg/apis/config/feature_flags.go

@@ -102,12 +102,12 @@ const (
 	EnableCELInWhenExpression = "enable-cel-in-whenexpression"
 	// EnableStepActions is the flag to enable the use of StepActions in Steps
 	EnableStepActions = "enable-step-actions"
-
 	// EnableArtifacts is the flag to enable the use of Artifacts in Steps
 	EnableArtifacts = "enable-artifacts"
-
 	// EnableParamEnum is the flag to enabled enum in params
 	EnableParamEnum = "enable-param-enum"
+	// EnableConciseResolverSyntax is the flag to enable concise resolver syntax
+	EnableConciseResolverSyntax = "enable-concise-resolver-syntax"
 
 	// DisableInlineSpec is the flag to disable embedded spec
 	// in Taskrun or Pipelinerun
@@ -168,6 +168,13 @@ var (
 		Stability: AlphaAPIFields,
 		Enabled:   DefaultAlphaFeatureEnabled,
 	}
+
+	// DefaultEnableConciseResolverSyntax is the default PerFeatureFlag value for EnableConciseResolverSyntax
+	DefaultEnableConciseResolverSyntax = PerFeatureFlag{
+		Name:      EnableConciseResolverSyntax,
+		Stability: AlphaAPIFields,
+		Enabled:   DefaultAlphaFeatureEnabled,
+	}
 )
 
 // FeatureFlags holds the features configurations
@@ -189,17 +196,18 @@ type FeatureFlags struct {
 	// ignore: skip trusted resources verification when no matching verification policies found
 	// warn: skip trusted resources verification when no matching verification policies found and log a warning
 	// fail: fail the taskrun or pipelines run if no matching verification policies found
-	VerificationNoMatchPolicy string
-	EnableProvenanceInStatus  bool
-	ResultExtractionMethod    string
-	MaxResultSize             int
-	SetSecurityContext        bool
-	Coschedule                string
-	EnableCELInWhenExpression bool
-	EnableStepActions         bool
-	EnableParamEnum           bool
-	EnableArtifacts           bool
-	DisableInlineSpec         string
+	VerificationNoMatchPolicy   string
+	EnableProvenanceInStatus    bool
+	ResultExtractionMethod      string
+	MaxResultSize               int
+	SetSecurityContext          bool
+	Coschedule                  string
+	EnableCELInWhenExpression   bool
+	EnableStepActions           bool
+	EnableParamEnum             bool
+	EnableArtifacts             bool
+	DisableInlineSpec           string
+	EnableConciseResolverSyntax bool
 }
 
 // GetFeatureFlagsConfigName returns the name of the configmap containing all
@@ -294,14 +302,15 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) {
 	if err := setPerFeatureFlag(EnableParamEnum, DefaultEnableParamEnum, &tc.EnableParamEnum); err != nil {
 		return nil, err
 	}
-
 	if err := setPerFeatureFlag(EnableArtifacts, DefaultEnableArtifacts, &tc.EnableArtifacts); err != nil {
 		return nil, err
 	}
 	if err := setFeatureInlineSpec(cfgMap, DisableInlineSpec, DefaultDisableInlineSpec, &tc.DisableInlineSpec); err != nil {
 		return nil, err
 	}
-
+	if err := setPerFeatureFlag(EnableConciseResolverSyntax, DefaultEnableConciseResolverSyntax, &tc.EnableConciseResolverSyntax); err != nil {
+		return nil, err
+	}
 	// Given that they are alpha features, Tekton Bundles and Custom Tasks should be switched on if
 	// enable-api-fields is "alpha". If enable-api-fields is not "alpha" then fall back to the value of
 	// each feature's individual flag.

pkg/apis/config/feature_flags_test.go

@@ -58,6 +58,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
 				EnableStepActions:                config.DefaultEnableStepActions.Enabled,
 				EnableParamEnum:                  config.DefaultEnableParamEnum.Enabled,
 				DisableInlineSpec:                config.DefaultDisableInlineSpec,
+				EnableConciseResolverSyntax:      config.DefaultEnableConciseResolverSyntax.Enabled,
 			},
 			fileName: config.GetFeatureFlagsConfigName(),
 		},
@@ -83,6 +84,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
 				EnableArtifacts:                  true,
 				EnableParamEnum:                  true,
 				DisableInlineSpec:                "pipeline,pipelinerun,taskrun",
+				EnableConciseResolverSyntax:      true,
 			},
 			fileName: "feature-flags-all-flags-set",
 		},
@@ -316,6 +318,9 @@ func TestNewFeatureFlagsConfigMapErrors(t *testing.T) {
 	}, {
 		fileName: "feature-flags-invalid-enable-artifacts",
 		want:     `failed parsing feature flags config "invalid": strconv.ParseBool: parsing "invalid": invalid syntax for feature enable-artifacts`,
+	}, {
+		fileName: "feature-flags-invalid-enable-concise-resolver-syntax",
+		want:     `failed parsing feature flags config "invalid": strconv.ParseBool: parsing "invalid": invalid syntax for feature enable-concise-resolver-syntax`,
 	}} {
 		t.Run(tc.fileName, func(t *testing.T) {
 			cm := test.ConfigMapFromTestFile(t, tc.fileName)