Skip to content

Commit

Permalink
TEP-0154: Enable concise resolver syntax
Browse files Browse the repository at this point in the history
This PR enables concise resolver syntax as per TEP 0154.
It allows users to pass a url like string to Taskref/name field. In turn, the Tekton controller passes the string to the Resolution Request under the name `resolutionName` which the resolvers can parse and resolve.
  • Loading branch information
chitrangpatel committed Apr 5, 2024
1 parent fbed78f commit 99b1c6f
Show file tree
Hide file tree
Showing 47 changed files with 442 additions and 131 deletions.
2 changes: 2 additions & 0 deletions config/config-feature-flags.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,5 @@ data:
enable-artifacts: "false"
# Setting this flag to "true" will enable the built-in param input validation via param enum.
enable-param-enum: "false"
# Setting this flag to "true" will enable the use of concise resolver syntax
enable-concise-resolver-syntax: "false"
30 changes: 28 additions & 2 deletions docs/how-to-write-a-resolver.md
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,32 @@ func (r *resolver) ValidateParams(ctx context.Context, params map[string]string)
You'll also need to add the `"errors"` package to your list of imports at
the top of the file.

## The `ValidateResolutionName` method

The `ValidateResolutionName` method checks that the `resolutionName` submitted as part of
a resolution request is valid and can be parsed by the resolver. Our example resolver expects
format for the `resolutionName` to be `demoscheme://<path>` so we'll validate this format.

```go
// ValidateResolutionName ensures resolutionName from a request is as expected.
func (r *resolver) ValidateResolutionName(ctx context.Context, resolutionName string) error {
u, err := url.ParseRequestURI(resolutionName)
if err != nil {
return err
}
if u.Scheme != "demoscheme" {
return errors.New("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` and `"errors"` package to your list of imports at
the top of the file.

## The `Resolve` method

We implement the `Resolve` method to do the heavy lifting of fetching
Expand All @@ -233,8 +259,8 @@ The method signature we're implementing here has a
is another type we have to implement but it has a small footprint:

```go
// Resolve uses the given params to resolve the requested file or resource.
func (r *resolver) Resolve(ctx context.Context, params map[string]string) (framework.ResolvedResource, error) {
// Resolve uses the given resolutionName or params to resolve the requested file or resource.
func (r *resolver) Resolve(ctx context.Context, resolutionName string, params map[string]string) (framework.ResolvedResource, error) {
return &myResolvedResource{}, nil
}

Expand Down
4 changes: 4 additions & 0 deletions docs/pipeline-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -3494,6 +3494,10 @@ Example: &ldquo;task/git-clone/0.8/git-clone.yaml&rdquo;</p>
</tr>
</tbody>
</table>
<h3 id="tekton.dev/v1.RemoteResolutionUrl">RemoteResolutionUrl
(<code>string</code> alias)</h3>
<div>
</div>
<h3 id="tekton.dev/v1.ResolverName">ResolverName
(<code>string</code> alias)</h3>
<p>
Expand Down
25 changes: 25 additions & 0 deletions go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 23 additions & 13 deletions pkg/apis/config/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,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"

disableAffinityAssistantKey = "disable-affinity-assistant"
disableCredsInitKey = "disable-creds-init"
Expand Down Expand Up @@ -160,6 +160,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
Expand All @@ -183,16 +190,17 @@ 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
VerificationNoMatchPolicy string
EnableProvenanceInStatus bool
ResultExtractionMethod string
MaxResultSize int
SetSecurityContext bool
Coschedule string
EnableCELInWhenExpression bool
EnableStepActions bool
EnableParamEnum bool
EnableArtifacts bool
EnableConciseResolverSyntax bool
}

// GetFeatureFlagsConfigName returns the name of the configmap containing all
Expand Down Expand Up @@ -287,10 +295,12 @@ 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 := 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.
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/config/feature_flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
EnableCELInWhenExpression: config.DefaultEnableCELInWhenExpression.Enabled,
EnableStepActions: config.DefaultEnableStepActions.Enabled,
EnableParamEnum: config.DefaultEnableParamEnum.Enabled,
EnableConciseResolverSyntax: config.DefaultEnableConciseResolverSyntax.Enabled,
},
fileName: config.GetFeatureFlagsConfigName(),
},
Expand All @@ -81,6 +82,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
EnableStepActions: true,
EnableArtifacts: true,
EnableParamEnum: true,
EnableConciseResolverSyntax: true,
},
fileName: "feature-flags-all-flags-set",
},
Expand Down Expand Up @@ -307,6 +309,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)
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/config/testdata/feature-flags-all-flags-set.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@ data:
enable-step-actions: "true"
enable-param-enum: "true"
enable-artifacts: "true"
enable-concise-resolver-syntax: "true"
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Copyright 2024 The Tekton Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

apiVersion: v1
kind: ConfigMap
metadata:
name: feature-flags
namespace: tekton-pipelines
data:
enable-concise-resolver-syntax: "invalid"
36 changes: 32 additions & 4 deletions pkg/apis/pipeline/v1/container_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"context"
"strings"

"net/url"

"github.com/tektoncd/pipeline/pkg/apis/config"
"k8s.io/apimachinery/pkg/util/validation"
"knative.dev/pkg/apis"
Expand All @@ -37,7 +39,14 @@ func (ref *Ref) Validate(ctx context.Context) (errs *apis.FieldError) {
if ref.Resolver != "" {
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "resolver", config.BetaAPIFields).ViaField("resolver"))
if ref.Name != "" {
errs = errs.Also(apis.ErrMultipleOneOf("name", "resolver"))
// make sure that the name is url-like.
if err := nameLikeUrl(ref.Name); err != nil {
errs = errs.Also(apis.ErrInvalidValue(err, "name"))
}
// cannot have both name and params
if ref.Params != nil {
errs = errs.Also(apis.ErrMultipleOneOf("name", "params"))
}
}
}
if ref.Params != nil {
Expand All @@ -51,12 +60,31 @@ func (ref *Ref) Validate(ctx context.Context) (errs *apis.FieldError) {
errs = errs.Also(ValidateParameters(ctx, ref.Params))
}
case ref.Name != "":
// ref name must be a valid k8s name
if errSlice := validation.IsQualifiedName(ref.Name); len(errSlice) != 0 {
errs = errs.Also(apis.ErrInvalidValue(strings.Join(errSlice, ","), "name"))
// ref name can be a Url-like format.
if err := nameLikeUrl(ref.Name); err == nil {
// cannot have both name and params
if ref.Params != nil {
errs = errs.Also(apis.ErrMultipleOneOf("name", "params"))
}
// In stage1 of concise remote resolvers syntax, this is a required field.
// TODO: remove this check when implementing stage 2 where this is optional.
if ref.Resolver == "" {
errs = errs.Also(apis.ErrMissingField("resolver"))
}
// Or, it must be a valid k8s name
} else {
// ref name must be a valid k8s name
if errSlice := validation.IsQualifiedName(ref.Name); len(errSlice) != 0 {
errs = errs.Also(apis.ErrInvalidValue(strings.Join(errSlice, ","), "name"))
}
}
default:
errs = errs.Also(apis.ErrMissingField("name"))
}
return errs
}

func nameLikeUrl(name string) error {
_, err := url.ParseRequestURI(name)
return err
}
28 changes: 16 additions & 12 deletions pkg/apis/pipeline/v1/container_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,29 +93,33 @@ func TestRef_Invalid(t *testing.T) {
},
wantErr: apis.ErrMissingField("resolver"),
}, {
name: "ref resolver disallowed in conjunction with ref name",
name: "ref with resolver and k8s style name",
ref: &v1.Ref{
Name: "foo",
ResolverRef: v1.ResolverRef{
Resolver: "git",
},
},
wantErr: apis.ErrMultipleOneOf("name", "resolver"),
wantErr: apis.ErrInvalidValue(`parse "foo": invalid URI for request`, "name"),
wc: cfgtesting.EnableBetaAPIFields,
}, {
name: "ref params disallowed in conjunction with ref name",
name: "ref with url-like name without resolver",
ref: &v1.Ref{
Name: "bar",
Name: "https://foo.com/bar",
},
wantErr: apis.ErrMissingField("resolver"),
wc: cfgtesting.EnableBetaAPIFields,
}, {
name: "ref params disallowed in conjucntion with pipelineref name",
ref: &v1.Ref{
Name: "https://foo/bar",
ResolverRef: v1.ResolverRef{
Params: v1.Params{{
Name: "foo",
Value: v1.ParamValue{
Type: v1.ParamTypeString,
StringVal: "bar",
},
}},
Resolver: "git",
Params: v1.Params{{Name: "foo", Value: v1.ParamValue{StringVal: "bar"}}},
},
},
wantErr: apis.ErrMultipleOneOf("name", "params").Also(apis.ErrMissingField("resolver")),
wantErr: apis.ErrMultipleOneOf("name", "params"),
wc: cfgtesting.EnableBetaAPIFields,
}, {
name: "invalid ref name",
ref: &v1.Ref{Name: "_foo"},
Expand Down
24 changes: 22 additions & 2 deletions pkg/apis/pipeline/v1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -612,14 +612,34 @@ func TestPipelineTask_ValidateRegularTask_Failure(t *testing.T) {
Paths: []string{"taskRef.name"},
},
}, {
name: "pipeline task - taskRef with resolver and name",
name: "pipeline task - taskRef with resolver and k8s style name",
task: PipelineTask{
Name: "foo",
TaskRef: &TaskRef{Name: "foo", ResolverRef: ResolverRef{Resolver: "git"}},
},
expectedError: apis.FieldError{
Message: `invalid value: parse "foo": invalid URI for request`,
Paths: []string{"taskRef.name"},
},
}, {
name: "pipeline task - taskRef with url-like name without resolver",
task: PipelineTask{
Name: "foo",
TaskRef: &TaskRef{Name: "https://foo.com/bar"},
},
expectedError: apis.FieldError{
Message: `missing field(s)`,
Paths: []string{"taskRef.resolver"},
},
}, {
name: "pipeline task - taskRef with name and params",
task: PipelineTask{
Name: "foo",
TaskRef: &TaskRef{Name: "https://foo/bar", ResolverRef: ResolverRef{Resolver: "git", Params: Params{{Name: "foo", Value: ParamValue{StringVal: "bar"}}}}},
},
expectedError: apis.FieldError{
Message: `expected exactly one, got both`,
Paths: []string{"taskRef.name", "taskRef.resolver"},
Paths: []string{"taskRef.name", "taskRef.params"},
},
}, {
name: "pipeline task - taskRef with resolver params but no resolver",
Expand Down
Loading

0 comments on commit 99b1c6f

Please sign in to comment.