From f2b1420db159d552d99c2483f4e33689a8cf38c9 Mon Sep 17 00:00:00 2001
From: Alejandro Romero <aleromero@google.com>
Date: Mon, 27 May 2024 17:44:25 +0000
Subject: [PATCH] Allow less strict validation of the Resolver Name during
Webhook.
---
pkg/apis/pipeline/v1/container_validation.go | 11 +++++++----
pkg/apis/pipeline/v1/container_validation_test.go | 6 +++++-
pkg/apis/pipeline/v1/pipeline_types_test.go | 2 +-
pkg/apis/pipeline/v1/pipelineref_validation_test.go | 2 +-
pkg/apis/pipeline/v1/taskref_validation_test.go | 2 +-
pkg/apis/pipeline/v1beta1/container_validation.go | 11 +++++++----
.../pipeline/v1beta1/container_validation_test.go | 6 +++++-
pkg/apis/pipeline/v1beta1/pipeline_types_test.go | 2 +-
.../pipeline/v1beta1/pipelineref_validation_test.go | 2 +-
pkg/apis/pipeline/v1beta1/taskref_validation_test.go | 2 +-
10 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/pkg/apis/pipeline/v1/container_validation.go b/pkg/apis/pipeline/v1/container_validation.go
index a145da01d9a..ec55189bc32 100644
--- a/pkg/apis/pipeline/v1/container_validation.go
+++ b/pkg/apis/pipeline/v1/container_validation.go
@@ -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"
@@ -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
}
diff --git a/pkg/apis/pipeline/v1/container_validation_test.go b/pkg/apis/pipeline/v1/container_validation_test.go
index 7bdd0e36bfa..74fbec19190 100644
--- a/pkg/apis/pipeline/v1/container_validation_test.go
+++ b/pkg/apis/pipeline/v1/container_validation_test.go
@@ -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"}},
@@ -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",
diff --git a/pkg/apis/pipeline/v1/pipeline_types_test.go b/pkg/apis/pipeline/v1/pipeline_types_test.go
index 12eb23477de..802034aafd2 100644
--- a/pkg/apis/pipeline/v1/pipeline_types_test.go
+++ b/pkg/apis/pipeline/v1/pipeline_types_test.go
@@ -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"},
diff --git a/pkg/apis/pipeline/v1/pipelineref_validation_test.go b/pkg/apis/pipeline/v1/pipelineref_validation_test.go
index 62baa5df3ff..06bceb31fc9 100644
--- a/pkg/apis/pipeline/v1/pipelineref_validation_test.go
+++ b/pkg/apis/pipeline/v1/pipelineref_validation_test.go
@@ -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",
diff --git a/pkg/apis/pipeline/v1/taskref_validation_test.go b/pkg/apis/pipeline/v1/taskref_validation_test.go
index 8640045e566..a98827470e5 100644
--- a/pkg/apis/pipeline/v1/taskref_validation_test.go
+++ b/pkg/apis/pipeline/v1/taskref_validation_test.go
@@ -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",
diff --git a/pkg/apis/pipeline/v1beta1/container_validation.go b/pkg/apis/pipeline/v1beta1/container_validation.go
index b9f66375d10..dc1b60d15ba 100644
--- a/pkg/apis/pipeline/v1beta1/container_validation.go
+++ b/pkg/apis/pipeline/v1beta1/container_validation.go
@@ -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"
@@ -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
}
diff --git a/pkg/apis/pipeline/v1beta1/container_validation_test.go b/pkg/apis/pipeline/v1beta1/container_validation_test.go
index 79b23fa6f25..cc423d3bd52 100644
--- a/pkg/apis/pipeline/v1beta1/container_validation_test.go
+++ b/pkg/apis/pipeline/v1beta1/container_validation_test.go
@@ -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"}},
@@ -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",
diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go
index 28fdcf2382d..507b90579fc 100644
--- a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go
+++ b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go
@@ -662,7 +662,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"},
diff --git a/pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go
index a5a00617dc1..988f4a4dbfb 100644
--- a/pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go
+++ b/pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go
@@ -105,7 +105,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",
diff --git a/pkg/apis/pipeline/v1beta1/taskref_validation_test.go b/pkg/apis/pipeline/v1beta1/taskref_validation_test.go
index 09136f812c3..b617fb224ae 100644
--- a/pkg/apis/pipeline/v1beta1/taskref_validation_test.go
+++ b/pkg/apis/pipeline/v1beta1/taskref_validation_test.go
@@ -120,7 +120,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",