Skip to content

Commit

Permalink
Allow less strict validation of the Resolver Name during Webhook.
Browse files Browse the repository at this point in the history
  • Loading branch information
Aleromerog authored and tekton-robot committed May 27, 2024
1 parent 13af266 commit 14b416b
Show file tree
Hide file tree
Showing 10 changed files with 30 additions and 16 deletions.
11 changes: 7 additions & 4 deletions pkg/apis/pipeline/v1/container_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ package v1

import (
"context"
"errors"
"fmt"
"regexp"
"strings"

"net/url"

"github.com/tektoncd/pipeline/pkg/apis/config"
"k8s.io/apimachinery/pkg/util/validation"
"knative.dev/pkg/apis"
Expand Down Expand Up @@ -91,6 +91,9 @@ func (ref *Ref) Validate(ctx context.Context) (errs *apis.FieldError) {

// RefNameLikeUrl checks if the name is url parsable and returns an error if it isn't.
func RefNameLikeUrl(name string) error {
_, err := url.ParseRequestURI(name)
return err
schemeRegex := regexp.MustCompile(`[\w-]+:\/\/*`)
if !schemeRegex.MatchString(name) {
return errors.New("invalid URI for request")
}
return nil
}
6 changes: 5 additions & 1 deletion pkg/apis/pipeline/v1/container_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ func TestRef_Valid(t *testing.T) {
}, {
name: "simple ref",
ref: &v1.Ref{Name: "refname"},
}, {
name: "ref name - consice syntax",
ref: &v1.Ref{Name: "foo://baz:ver", ResolverRef: v1.ResolverRef{Resolver: "git"}},
wc: enableConciseResolverSyntax,
}, {
name: "beta feature: valid resolver",
ref: &v1.Ref{ResolverRef: v1.ResolverRef{Resolver: "git"}},
Expand Down Expand Up @@ -110,7 +114,7 @@ func TestRef_Invalid(t *testing.T) {
Resolver: "git",
},
},
wantErr: apis.ErrInvalidValue(`parse "foo": invalid URI for request`, "name"),
wantErr: apis.ErrInvalidValue(`invalid URI for request`, "name"),
wc: enableConciseResolverSyntax,
}, {
name: "ref with url-like name without resolver",
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ func TestPipelineTask_ValidateRegularTask_Failure(t *testing.T) {
TaskRef: &TaskRef{Name: "foo", ResolverRef: ResolverRef{Resolver: "git"}},
},
expectedError: apis.FieldError{
Message: `invalid value: parse "foo": invalid URI for request`,
Message: `invalid value: invalid URI for request`,
Paths: []string{"taskRef.name"},
},
configMap: map[string]string{"enable-concise-resolver-syntax": "true"},
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1/pipelineref_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func TestPipelineRef_Invalid(t *testing.T) {
Resolver: "git",
},
},
wantErr: apis.ErrInvalidValue(`parse "foo": invalid URI for request`, "name"),
wantErr: apis.ErrInvalidValue(`invalid URI for request`, "name"),
withContext: enableConciseResolverSyntax,
}, {
name: "pipelineRef with url-like name without resolver",
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1/taskref_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func TestTaskRef_Invalid(t *testing.T) {
Resolver: "git",
},
},
wantErr: apis.ErrInvalidValue(`parse "foo": invalid URI for request`, "name"),
wantErr: apis.ErrInvalidValue(`invalid URI for request`, "name"),
wc: enableConciseResolverSyntax,
}, {
name: "taskRef with url-like name without resolver",
Expand Down
11 changes: 7 additions & 4 deletions pkg/apis/pipeline/v1beta1/container_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ package v1beta1

import (
"context"
"errors"
"fmt"
"regexp"
"strings"

"net/url"

"github.com/tektoncd/pipeline/pkg/apis/config"
"k8s.io/apimachinery/pkg/util/validation"
"knative.dev/pkg/apis"
Expand Down Expand Up @@ -91,6 +91,9 @@ func (ref *Ref) Validate(ctx context.Context) (errs *apis.FieldError) {

// RefNameLikeUrl checks if the name is url parsable and returns an error if it isn't.
func RefNameLikeUrl(name string) error {
_, err := url.ParseRequestURI(name)
return err
schemeRegex := regexp.MustCompile(`[\w-]+:\/\/*`)
if !schemeRegex.MatchString(name) {
return errors.New("invalid URI for request")
}
return nil
}
6 changes: 5 additions & 1 deletion pkg/apis/pipeline/v1beta1/container_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ func TestRef_Valid(t *testing.T) {
}, {
name: "simple ref",
ref: &v1beta1.Ref{Name: "refname"},
}, {
name: "ref name - consice syntax",
ref: &v1beta1.Ref{Name: "foo://baz:ver", ResolverRef: v1beta1.ResolverRef{Resolver: "git"}},
wc: enableConciseResolverSyntax,
}, {
name: "beta feature: valid resolver",
ref: &v1beta1.Ref{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}},
Expand Down Expand Up @@ -110,7 +114,7 @@ func TestRef_Invalid(t *testing.T) {
Resolver: "git",
},
},
wantErr: apis.ErrInvalidValue(`parse "foo": invalid URI for request`, "name"),
wantErr: apis.ErrInvalidValue(`invalid URI for request`, "name"),
wc: enableConciseResolverSyntax,
}, {
name: "ref with url-like name without resolver",
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1beta1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ func TestPipelineTask_ValidateRegularTask_Failure(t *testing.T) {
TaskRef: &TaskRef{Name: "foo", ResolverRef: ResolverRef{Resolver: "git"}},
},
expectedError: apis.FieldError{
Message: `invalid value: parse "foo": invalid URI for request`,
Message: `invalid value: invalid URI for request`,
Paths: []string{"taskRef.name"},
},
configMap: map[string]string{"enable-concise-resolver-syntax": "true"},
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func TestPipelineRef_Invalid(t *testing.T) {
Resolver: "git",
},
},
wantErr: apis.ErrInvalidValue(`parse "foo": invalid URI for request`, "name"),
wantErr: apis.ErrInvalidValue(`invalid URI for request`, "name"),
withContext: enableConciseResolverSyntax,
}, {
name: "pipelineRef with url-like name without resolver",
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1beta1/taskref_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func TestTaskRef_Invalid(t *testing.T) {
Resolver: "git",
},
},
wantErr: apis.ErrInvalidValue(`parse "foo": invalid URI for request`, "name"),
wantErr: apis.ErrInvalidValue(`invalid URI for request`, "name"),
wc: enableConciseResolverSyntax,
}, {
name: "taskRef with url-like name without resolver",
Expand Down

0 comments on commit 14b416b

Please sign in to comment.