From ecfa77738141537ea9b7d90640ac9b610fcea9a0 Mon Sep 17 00:00:00 2001 From: Chitrang Patel Date: Wed, 27 Mar 2024 16:01:44 -0400 Subject: [PATCH] TEP-0154: Enable concise resolver syntax 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. --- config/config-feature-flags.yaml | 2 ++ docs/how-to-write-a-resolver.md | 30 ++++++++++++++-- docs/pipeline-api.md | 4 +++ go.mod | 4 ++- go.sum | 20 +++++++++++ pkg/apis/config/feature_flags.go | 36 ++++++++++++------- pkg/apis/config/feature_flags_test.go | 5 +++ .../testdata/feature-flags-all-flags-set.yaml | 1 + ...nvalid-enable-concise-resolver-syntax.yaml | 21 +++++++++++ pkg/apis/pipeline/v1/container_validation.go | 36 ++++++++++++++++--- .../pipeline/v1/container_validation_test.go | 28 ++++++++------- pkg/apis/pipeline/v1/pipeline_types_test.go | 24 +++++++++++-- .../pipeline/v1/pipelineref_validation.go | 34 ++++++++++++++++-- .../v1/pipelineref_validation_test.go | 28 ++++++++------- pkg/apis/pipeline/v1/taskref_validation.go | 28 ++++++++++++--- .../pipeline/v1/taskref_validation_test.go | 26 +++++++------- .../v1beta1/resolution_request_types.go | 3 +- .../pipelinerun/pipelinerun_test.go | 2 +- .../pipelinerun/resources/pipelineref.go | 2 +- .../pipelinerun/resources/pipelineref_test.go | 2 ++ pkg/reconciler/taskrun/resources/taskref.go | 4 +-- .../taskrun/resources/taskref_test.go | 2 ++ pkg/reconciler/taskrun/taskrun_test.go | 2 +- pkg/remote/resolution/resolver.go | 12 ++++--- pkg/remote/resolution/resolver_test.go | 8 ++--- pkg/resolution/common/interface.go | 1 + pkg/resolution/resolver/bundle/resolver.go | 7 +++- .../resolver/bundle/resolver_test.go | 2 +- pkg/resolution/resolver/cluster/resolver.go | 7 +++- .../resolver/framework/fakeresolver.go | 12 ++++++- .../resolver/framework/interface.go | 8 +++-- .../resolver/framework/reconciler.go | 27 ++++++++++---- pkg/resolution/resolver/git/resolver.go | 7 +++- pkg/resolution/resolver/git/resolver_test.go | 2 +- pkg/resolution/resolver/http/resolver.go | 7 +++- pkg/resolution/resolver/http/resolver_test.go | 4 +-- pkg/resolution/resolver/hub/resolver.go | 7 +++- pkg/resolution/resolver/hub/resolver_test.go | 6 ++-- pkg/resolution/resource/crd_resource.go | 3 +- pkg/resolution/resource/crd_resource_test.go | 2 ++ pkg/resolution/resource/name.go | 5 ++- pkg/resolution/resource/name_test.go | 19 +++++++--- pkg/resolution/resource/request.go | 15 +++++--- pkg/resolution/resource/request_test.go | 10 +++--- test/e2e-tests-kind-prow-alpha.env | 1 + test/e2e-tests.sh | 14 ++++++++ test/featureflags.go | 15 ++++---- test/resolution.go | 27 ++++++++++---- 48 files changed, 440 insertions(+), 132 deletions(-) create mode 100644 pkg/apis/config/testdata/feature-flags-invalid-enable-concise-resolver-syntax.yaml diff --git a/config/config-feature-flags.yaml b/config/config-feature-flags.yaml index 6cb5073da37..8d8358551ae 100644 --- a/config/config-feature-flags.yaml +++ b/config/config-feature-flags.yaml @@ -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" diff --git a/docs/how-to-write-a-resolver.md b/docs/how-to-write-a-resolver.md index 229f5361603..60b45ca5d03 100644 --- a/docs/how-to-write-a-resolver.md +++ b/docs/how-to-write-a-resolver.md @@ -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://` 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 @@ -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 } diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index 4bee675fe6d..a3f3f95b266 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -3494,6 +3494,10 @@ Example: “task/git-clone/0.8/git-clone.yaml”

+

RemoteResolutionUrl +(string alias)

+
+

ResolverName (string alias)

diff --git a/go.mod b/go.mod index 525ad928f79..f7248e176db 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,8 @@ module github.com/tektoncd/pipeline -go 1.19 +go 1.21 + +toolchain go1.21.6 require ( github.com/Microsoft/go-winio v0.6.1 // indirect diff --git a/go.sum b/go.sum index 84e25039bda..c88bbfc2d06 100644 --- a/go.sum +++ b/go.sum @@ -21,6 +21,7 @@ cloud.google.com/go v0.79.0/go.mod h1:3bzgcEeQlzbuEAYu4mrWhKqWjmpprinYgKJLgKHnbb cloud.google.com/go v0.81.0/go.mod h1:mk/AM35KwGk/Nm2YSeZbxXdrNK3KZOYHmLkOqC2V6E0= cloud.google.com/go v0.83.0/go.mod h1:Z7MJUsANfY0pYPdw0lbnivPx4/vhy/e2FEkSkF7vAVY= cloud.google.com/go v0.112.0 h1:tpFCD7hpHFlQ8yPwT3x+QeXqc2T6+n6T+hmABHfDUSM= +cloud.google.com/go v0.112.0/go.mod h1:3jEEVwZ/MHU4djK5t5RHuKOA/GbLddgTdVubX1qnPD4= cloud.google.com/go/bigquery v1.0.1/go.mod h1:i/xbL2UlR5RvWAURpBYZTtm/cXjCha9lbfbpx4poX+o= cloud.google.com/go/bigquery v1.3.0/go.mod h1:PjpwJnslEMmckchkHFfq+HTD2DmtT67aNFKH1/VBDHE= cloud.google.com/go/bigquery v1.4.0/go.mod h1:S8dzgnTigyfTmLBfrtrhyYhwRxG72rYxvftPBK2Dvzc= @@ -140,6 +141,7 @@ github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk5 github.com/alexflint/go-filemutex v0.0.0-20171022225611-72bdc8eae2ae/go.mod h1:CgnQgUtFrFz9mxFNtED3jI5tLDjKlOM+oUF/sTk6ps0= github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239/go.mod h1:2FmKhYUyUczH0OGQWaF5ceTx0UBShxjsH6f8oGKYe2c= github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be h1:9AeTilPcZAjCFIImctFaOjnTIavg87rW78vTPkQqLI8= +github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be/go.mod h1:ySMOLuWl6zY27l47sB3qLNK6tF2fkHG55UZxx8oIVo4= github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI= github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g= @@ -152,6 +154,7 @@ github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkY github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY= github.com/aws/aws-sdk-go v1.15.11/go.mod h1:mFuSZ37Z9YOHbQEwBWztmVzqXrEkub65tZoCYDt7FT0= github.com/aws/aws-sdk-go v1.50.25 h1:vhiHtLYybv1Nhx3Kv18BBC6L0aPJHaG9aeEsr92W99c= +github.com/aws/aws-sdk-go v1.50.25/go.mod h1:LF8svs817+Nz+DmiMQKTO3ubZ/6IaTpq3TjupRn3Eqk= github.com/aws/aws-sdk-go-v2 v1.18.0/go.mod h1:uzbQtefpm44goOPmdKyAlXSNcwlRgF3ePWVW6EtJvvw= github.com/aws/aws-sdk-go-v2 v1.25.2 h1:/uiG1avJRgLGiQM9X3qJM8+Qa6KRGK5rRPuXE0HUM+w= github.com/aws/aws-sdk-go-v2 v1.25.2/go.mod h1:Evoc5AsmtveRt1komDwIsjHFyrP5tDuF1D1U+6z6pNo= @@ -258,6 +261,7 @@ github.com/cncf/xds/go v0.0.0-20210922020428-25de7278fc84/go.mod h1:eXthEFrGJvWH github.com/cncf/xds/go v0.0.0-20211001041855-01bcc9b48dfe/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= github.com/cncf/xds/go v0.0.0-20231128003011-0fa0005c9caa h1:jQCWAUqqlij9Pgj2i/PB79y4KOPYVyFYdROxgaCwdTQ= +github.com/cncf/xds/go v0.0.0-20231128003011-0fa0005c9caa/go.mod h1:x/1Gn8zydmfq8dk6e9PdstVsDgu9RuyIIJqAaF//0IM= github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa/go.mod h1:zn76sxSg3SzpJ0PPJaLDCu+Bu0Lg3sKTORVIj19EIF8= github.com/containerd/aufs v0.0.0-20200908144142-dab0cbea06f4/go.mod h1:nukgQABAEopAHvB6j7cnP5zJ+/3aVcE7hCYqvIwAHyE= github.com/containerd/aufs v0.0.0-20201003224125-76a6863f2989/go.mod h1:AkGGQs9NM2vtYHaUen+NljV0/baGCAPELGm2q9ZXpWU= @@ -390,6 +394,7 @@ github.com/dimchansky/utfbom v1.1.1 h1:vV6w1AhK4VMnhBno/TPVCoK9U/LP0PkLCS9tbxHdi github.com/dimchansky/utfbom v1.1.1/go.mod h1:SxdoEBH5qIqFocHMyGOXVAybYJdr71b1Q/j0mACtrfE= github.com/dnaeon/go-vcr v1.0.1/go.mod h1:aBB1+wY4s93YsC3HHjMBMrwTj2R9FHDzUr9KyGc8n1E= github.com/dnaeon/go-vcr v1.2.0 h1:zHCHvJYTMh1N7xnV7zf1m1GPBF9Ad0Jk/whtQ1663qI= +github.com/dnaeon/go-vcr v1.2.0/go.mod h1:R4UdLID7HZT3taECzJs4YgbbH6PIGXB6W/sc5OLb6RQ= github.com/docker/cli v20.10.7+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= github.com/docker/cli v24.0.7+incompatible h1:wa/nIwYFW7BVTGa7SWPVyyXU9lgORqUb1xfI36MSkFg= github.com/docker/cli v24.0.7+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= @@ -419,6 +424,7 @@ github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21/go.mod h1 github.com/eapache/queue v1.1.0/go.mod h1:6eCeP0CKFpHLu8blIFXhExK/dRa7WDZfr6jVFPTqq+I= github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153/go.mod h1:/Zj4wYkgs4iZTTu3o/KG3Itv/qCCa8VVMlb3i9OVuzc= github.com/elazarl/goproxy v0.0.0-20230808193330-2592e75ae04a h1:mATvB/9r/3gvcejNsXKSkQ6lcIaNec2nyfOdlTBR2lU= +github.com/elazarl/goproxy v0.0.0-20230808193330-2592e75ae04a/go.mod h1:Ro8st/ElPeALwNFlcTpWmkr6IoMFfkjXAvTHpevnDsM= github.com/emicklei/go-restful v0.0.0-20170410110728-ff4f55a20633/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs= github.com/emicklei/go-restful v2.9.5+incompatible/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs= github.com/emicklei/go-restful/v3 v3.8.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= @@ -439,6 +445,7 @@ github.com/envoyproxy/go-control-plane v0.9.10-0.20210907150352-cf90f659a021/go. github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/envoyproxy/protoc-gen-validate v1.0.4 h1:gVPz/FMfvh57HdSJQyvBtF00j8JU4zdyUgIUNhlgg0A= +github.com/envoyproxy/protoc-gen-validate v1.0.4/go.mod h1:qys6tmnRsYrQqIhm2bvKZH4Blx/1gTIZ2UKVY1M+Yew= github.com/evanphx/json-patch v4.9.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/evanphx/json-patch v4.12.0+incompatible h1:4onqiflcdA9EOZ4RxV643DvftH5pOlLGNtQ5lPWQu84= github.com/evanphx/json-patch v4.12.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= @@ -461,6 +468,7 @@ github.com/ghodss/yaml v0.0.0-20150909031657-73d445a93680/go.mod h1:4dBDuWmgqj2H github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= github.com/gliderlabs/ssh v0.2.2/go.mod h1:U7qILu1NlMHj9FlMhZLlkCdDnU1DBEAqr0aevW3Awn0= github.com/gliderlabs/ssh v0.3.5 h1:OcaySEmAQJgyYcArR+gGGTHCyE7nvhEMTlYY+Dp8CpY= +github.com/gliderlabs/ssh v0.3.5/go.mod h1:8XB4KraRrX39qHhT6yxPsHedjA08I/uBVwj4xC+/+z4= github.com/go-fed/httpsig v1.1.0 h1:9M+hb0jkEICD8/cAiNqEB66R87tTINszBRTjwjQzWcI= github.com/go-fed/httpsig v1.1.0/go.mod h1:RCMrTZvN1bJYtofsG4rd5NaO5obxQ5xBkdiS7xsT7bM= github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 h1:+zs/tPmkDkHx3U66DAb0lQFJrpS6731Oaa12ikc+DiI= @@ -468,6 +476,7 @@ github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376/go.mod h1:an3vInlBmS github.com/go-git/go-billy/v5 v5.5.0 h1:yEY4yhzCDuMGSv83oGxiBotRzhwhNr8VZyphhiu+mTU= github.com/go-git/go-billy/v5 v5.5.0/go.mod h1:hmexnoNsr2SJU1Ju67OaNz5ASJY3+sHgFRpCtpDCKow= github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399 h1:eMje31YglSBqCdIqdhKBW8lokaMrL3uTkpGYlE2OOT4= +github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399/go.mod h1:1OCfN199q1Jm3HZlxleg+Dw/mwps2Wbk9frAWm+4FII= github.com/go-git/go-git/v5 v5.11.0 h1:XIZc1p+8YzypNr34itUfSvYJcv+eYdTnTvOZ2vD3cA4= github.com/go-git/go-git/v5 v5.11.0/go.mod h1:6GFcX2P3NM7FPBfpePbpLd21XxsgdAt+lKqXmCUiUCY= github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= @@ -514,6 +523,7 @@ github.com/go-openapi/swag v0.22.3/go.mod h1:UzaqsxGiab7freDnrUUra0MwWfN/q7tE4j+ github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0/go.mod h1:fyg7847qk6SyHyPtNmDHnmrv/HOrqktSC+C9fM+CJOE= github.com/go-test/deep v1.1.0 h1:WOcxcdHcvdgThNXjw0t76K42FXTU7HpNQWHpA2HHNlg= +github.com/go-test/deep v1.1.0/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE= github.com/gobuffalo/flect v0.2.4 h1:BSYA8+T60cdyq+vynaSUjqSVI9mDEg9ZfQUXKmfjo4I= github.com/gobuffalo/flect v0.2.4/go.mod h1:1ZyCLIbg0YD7sDkzvFdPoOydPtD8y9JQnrOROolUcM8= github.com/goccy/kpoward v0.1.0 h1:UcrLMG9rq7NwrMiUc0h+qUyIlvqPzqLiPb+zQEqH8cE= @@ -667,6 +677,7 @@ github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFb github.com/grpc-ecosystem/grpc-gateway/v2 v2.19.0 h1:Wqo399gCIufwto+VfwCSvsnfGpF/w5E9CNxSwbpD6No= github.com/grpc-ecosystem/grpc-gateway/v2 v2.19.0/go.mod h1:qmOFXW2epJhM0qSnUUYpldc7gVz2KMQwJ/QYCDIa7XU= github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 h1:2VTzZjLZBgl62/EtslCrtky5vbi9dd7HrQPQIx6wqiw= +github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542/go.mod h1:Ow0tF8D4Kplbc8s8sSb3V2oUCygFHVp8gC3Dn6U4MNI= github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q= github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8= github.com/hashicorp/errwrap v0.0.0-20141028054710-7554cd9344ce/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= @@ -678,6 +689,7 @@ github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9n github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48= github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ= github.com/hashicorp/go-hclog v0.16.2 h1:K4ev2ib4LdQETX5cSZBG0DVLk1jwGqSPXBjdah3veNs= +github.com/hashicorp/go-hclog v0.16.2/go.mod h1:whpDNt7SSdeAju8AWKIWsul05p54N/39EeqMAyrmvFQ= github.com/hashicorp/go-immutable-radix v1.0.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60= github.com/hashicorp/go-msgpack v0.5.3/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iPBM1vqhUKIvfM= github.com/hashicorp/go-multierror v0.0.0-20161216184304-ed905158d874/go.mod h1:JMRHfdO9jKNzS/+BTlxCjKNQHg/jZAft8U7LloJvN7I= @@ -747,6 +759,7 @@ github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHW github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8= github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U= github.com/jmhodges/clock v1.2.0 h1:eq4kys+NI0PLngzaHEe7AmPT90XMGIEySD1JfV1PDIs= +github.com/jmhodges/clock v1.2.0/go.mod h1:qKjhA7x7u/lQpPB1XAqX1b1lCI/w3/fNuYpI/ZjLynI= github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= @@ -787,6 +800,7 @@ github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORN github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/pty v1.1.5/go.mod h1:9r2w37qlBe7rQ6e1fg1S/9xpWHSnaqNdHD3WcMdbPDA= github.com/kr/pty v1.1.8/go.mod h1:O1sed60cT9XZ5uDucP5qwvh+TE3NnUj51EiZO/lmSfw= @@ -898,6 +912,7 @@ github.com/onsi/gomega v1.21.1/go.mod h1:iYAIXgPSaDHak0LCMA+AWBpIKBr8WZicMxnE8lu github.com/onsi/gomega v1.22.1/go.mod h1:x6n7VNe4hw0vkyYUM4mjIXx3JbLiPaBPNgB7PRQ1tuM= github.com/onsi/gomega v1.23.0/go.mod h1:Z/NWtiqwBrwUt4/2loMmHL63EDLnYHmVbuBpDr2vQAg= github.com/onsi/gomega v1.27.10 h1:naR28SdDFlqrG6kScpT8VWpu1xWY5nJRCF3XaYyBjhI= +github.com/onsi/gomega v1.27.10/go.mod h1:RsS8tutOdbdgzbPtzzATp12yT7kM5I5aElG3evPbQ0M= github.com/opencontainers/go-digest v0.0.0-20170106003457-a6d0ee40d420/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s= github.com/opencontainers/go-digest v0.0.0-20180430190053-c9281466c8b2/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s= github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s= @@ -997,6 +1012,7 @@ github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6So github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M= +github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA= github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g= github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= @@ -1117,6 +1133,7 @@ github.com/urfave/cli v1.22.2/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtX github.com/urfave/cli v1.22.12/go.mod h1:sSBEIC79qR6OvcmsD4U3KABeOTxDqQtdDnaFuUN30b8= github.com/urfave/cli/v2 v2.3.0/go.mod h1:LJmUH05zAU44vOAcrfzZQKsZbVcdbOG8rtL3/XcUArI= github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= +github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= github.com/vbatts/tar-split v0.11.3 h1:hLFqsOLQ1SsppQNTMpkpPXClLDfC2A3Zgy9OUU+RVck= github.com/vbatts/tar-split v0.11.3/go.mod h1:9QlHN18E+fEH7RdG+QAJJcuya3rqT7eXSTY7wGrAokY= github.com/vishvananda/netlink v0.0.0-20181108222139-023a6dafdcdf/go.mod h1:+SR5DhBJrl6ZM7CoCKvpw5BKroDKQ+PJqOg65H/2ktk= @@ -1195,6 +1212,7 @@ go.uber.org/atomic v1.10.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0 go.uber.org/automaxprocs v1.4.0 h1:CpDZl6aOlLhReez+8S3eEotD7Jx0Os++lemPlMULQP0= go.uber.org/automaxprocs v1.4.0/go.mod h1:/mTEdr7LvHhs0v7mjdxDreTz1OG5zdZGqgOnhWiR/+Q= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= +go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= go.uber.org/multierr v1.10.0 h1:S0h4aNzvfcFsC3dRF1jLoaov7oRaKqRGC/pUEJ2yvPQ= @@ -1748,6 +1766,7 @@ gopkg.in/gemnasium/logrus-airbrake-hook.v2 v2.1.2/go.mod h1:Xk6kEKp8OKb+X14hQBKW gopkg.in/go-jose/go-jose.v2 v2.6.1 h1:qEzJlIDmG9q5VO0M/o8tGS65QMHMS1w01TQJB1VPJ4U= gopkg.in/go-jose/go-jose.v2 v2.6.1/go.mod h1:zzZDPkNNw/c9IE7Z9jr11mBZQhKQTMzoEEIoEdZlFBI= gopkg.in/h2non/gock.v1 v1.1.2 h1:jBbHXgGBK/AoPVfJh5x4r/WxIrElvbLel8TCZkkZJoY= +gopkg.in/h2non/gock.v1 v1.1.2/go.mod h1:n7UGz/ckNChHiK05rDoiC4MYSunEC/lyaUm2WWaDva0= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= gopkg.in/ini.v1 v1.62.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= @@ -1785,6 +1804,7 @@ gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81 gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk= gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8= gotest.tools/v3 v3.1.0 h1:rVV8Tcg/8jHUkPUorwjaMTtemIMVXfIPKiOqnhEhakk= +gotest.tools/v3 v3.1.0/go.mod h1:fHy7eyTmJFO5bQbUsEGQ1v4m2J3Jz9eWL54TP2/ZuYQ= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/pkg/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go index d4be3c024ee..a77dd4dc5a7 100644 --- a/pkg/apis/config/feature_flags.go +++ b/pkg/apis/config/feature_flags.go @@ -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" @@ -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 @@ -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 @@ -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. diff --git a/pkg/apis/config/feature_flags_test.go b/pkg/apis/config/feature_flags_test.go index 5cf208ba594..9b0309a32ed 100644 --- a/pkg/apis/config/feature_flags_test.go +++ b/pkg/apis/config/feature_flags_test.go @@ -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(), }, @@ -81,6 +82,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableStepActions: true, EnableArtifacts: true, EnableParamEnum: true, + EnableConciseResolverSyntax: true, }, fileName: "feature-flags-all-flags-set", }, @@ -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) diff --git a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml index 51f312c52b0..4c208639953 100644 --- a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml +++ b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml @@ -36,3 +36,4 @@ data: enable-step-actions: "true" enable-param-enum: "true" enable-artifacts: "true" + enable-concise-resolver-syntax: "true" diff --git a/pkg/apis/config/testdata/feature-flags-invalid-enable-concise-resolver-syntax.yaml b/pkg/apis/config/testdata/feature-flags-invalid-enable-concise-resolver-syntax.yaml new file mode 100644 index 00000000000..4945e2f6f76 --- /dev/null +++ b/pkg/apis/config/testdata/feature-flags-invalid-enable-concise-resolver-syntax.yaml @@ -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" diff --git a/pkg/apis/pipeline/v1/container_validation.go b/pkg/apis/pipeline/v1/container_validation.go index 56108b63db8..b277f8a73cd 100644 --- a/pkg/apis/pipeline/v1/container_validation.go +++ b/pkg/apis/pipeline/v1/container_validation.go @@ -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" @@ -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 { @@ -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 +} diff --git a/pkg/apis/pipeline/v1/container_validation_test.go b/pkg/apis/pipeline/v1/container_validation_test.go index 60c95f88120..50348fff7d0 100644 --- a/pkg/apis/pipeline/v1/container_validation_test.go +++ b/pkg/apis/pipeline/v1/container_validation_test.go @@ -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"}, diff --git a/pkg/apis/pipeline/v1/pipeline_types_test.go b/pkg/apis/pipeline/v1/pipeline_types_test.go index 61771b90f2a..821e20ad340 100644 --- a/pkg/apis/pipeline/v1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1/pipeline_types_test.go @@ -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", diff --git a/pkg/apis/pipeline/v1/pipelineref_validation.go b/pkg/apis/pipeline/v1/pipelineref_validation.go index 9fa7c9894d0..fbeab741d14 100644 --- a/pkg/apis/pipeline/v1/pipelineref_validation.go +++ b/pkg/apis/pipeline/v1/pipelineref_validation.go @@ -18,8 +18,10 @@ package v1 import ( "context" + "strings" "github.com/tektoncd/pipeline/pkg/apis/config" + "k8s.io/apimachinery/pkg/util/validation" "knative.dev/pkg/apis" ) @@ -30,11 +32,19 @@ func (ref *PipelineRef) Validate(ctx context.Context) (errs *apis.FieldError) { return } - if ref.Resolver != "" || ref.Params != nil { + switch { + case ref.Resolver != "" || ref.Params != nil: 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 { @@ -47,7 +57,25 @@ func (ref *PipelineRef) Validate(ctx context.Context) (errs *apis.FieldError) { } errs = errs.Also(ValidateParameters(ctx, ref.Params)) } - } else if ref.Name == "" { + case ref.Name != "": + // Pipelineref 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")) + } + } else { + // Or, it 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 diff --git a/pkg/apis/pipeline/v1/pipelineref_validation_test.go b/pkg/apis/pipeline/v1/pipelineref_validation_test.go index 114eb6a0f10..6733b86fef0 100644 --- a/pkg/apis/pipeline/v1/pipelineref_validation_test.go +++ b/pkg/apis/pipeline/v1/pipelineref_validation_test.go @@ -65,30 +65,32 @@ func TestPipelineRef_Invalid(t *testing.T) { wantErr: apis.ErrMissingField("resolver"), withContext: cfgtesting.EnableBetaAPIFields, }, { - name: "pipelineref resolver disallowed in conjunction with pipelineref name", + name: "pipelineRef with resolver and k8s style name", ref: &v1.PipelineRef{ Name: "foo", ResolverRef: v1.ResolverRef{ - Resolver: "bar", + Resolver: "git", }, }, - wantErr: apis.ErrMultipleOneOf("name", "resolver"), + wantErr: apis.ErrInvalidValue(`parse "foo": invalid URI for request`, "name"), withContext: cfgtesting.EnableBetaAPIFields, }, { - name: "pipelineref params disallowed in conjunction with pipelineref name", + name: "pipelineRef with url-like name without resolver", ref: &v1.PipelineRef{ - Name: "bar", + Name: "https://foo.com/bar", + }, + wantErr: apis.ErrMissingField("resolver"), + withContext: cfgtesting.EnableBetaAPIFields, + }, { + name: "pipelineRef params disallowed in conjucntion with pipelineref name", + ref: &v1.PipelineRef{ + 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"), withContext: cfgtesting.EnableBetaAPIFields, }} diff --git a/pkg/apis/pipeline/v1/taskref_validation.go b/pkg/apis/pipeline/v1/taskref_validation.go index 1a743e6dac6..2d1c5c000c0 100644 --- a/pkg/apis/pipeline/v1/taskref_validation.go +++ b/pkg/apis/pipeline/v1/taskref_validation.go @@ -37,7 +37,14 @@ func (ref *TaskRef) 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 { @@ -51,9 +58,22 @@ func (ref *TaskRef) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(ValidateParameters(ctx, ref.Params)) } case ref.Name != "": - // TaskRef 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")) + // TaskRef 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")) + } + } else { + // Or, it 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")) diff --git a/pkg/apis/pipeline/v1/taskref_validation_test.go b/pkg/apis/pipeline/v1/taskref_validation_test.go index d2c721bc6a1..860dcc93378 100644 --- a/pkg/apis/pipeline/v1/taskref_validation_test.go +++ b/pkg/apis/pipeline/v1/taskref_validation_test.go @@ -120,30 +120,32 @@ func TestTaskRef_Invalid(t *testing.T) { wantErr: apis.ErrMissingField("resolver"), wc: cfgtesting.EnableBetaAPIFields, }, { - name: "taskref resolver disallowed in conjunction with taskref name", + name: "taskRef with resolver and k8s style name", taskRef: &v1.TaskRef{ 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: "taskref params disallowed in conjunction with taskref name", + name: "taskRef with url-like name without resolver", taskRef: &v1.TaskRef{ - Name: "bar", + Name: "https://foo.com/bar", + }, + wantErr: apis.ErrMissingField("resolver"), + wc: cfgtesting.EnableBetaAPIFields, + }, { + name: "taskRef params disallowed in conjucntion with pipelineref name", + taskRef: &v1.TaskRef{ + 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, }} for _, ts := range tests { diff --git a/pkg/apis/resolution/v1beta1/resolution_request_types.go b/pkg/apis/resolution/v1beta1/resolution_request_types.go index 60b51fa0498..619fff927fc 100644 --- a/pkg/apis/resolution/v1beta1/resolution_request_types.go +++ b/pkg/apis/resolution/v1beta1/resolution_request_types.go @@ -63,7 +63,8 @@ type ResolutionRequestSpec struct { // path to file, the kind of authentication to leverage, etc. // +optional // +listType=atomic - Params []pipelinev1.Param `json:"params,omitempty"` + Params []pipelinev1.Param `json:"params,omitempty"` + ResolutionName string `json:"resolutionName,omitempty"` } // ResolutionRequestStatus are all the fields in a ResolutionRequest's diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 71d1e776f1c..38631133c60 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -16588,7 +16588,7 @@ spec: // the ResolutionRequest's name is generated by resolverName, namespace and runName. func getResolvedResolutionRequest(t *testing.T, resolverName string, resourceBytes []byte, namespace string, runName string) resolutionv1beta1.ResolutionRequest { t.Helper() - name, err := remoteresource.GenerateDeterministicName(resolverName, namespace+"/"+runName, nil) + name, err := remoteresource.GenerateDeterministicName(resolverName, namespace+"/"+runName, nil, "") if err != nil { t.Errorf("error generating name for %s/%s/%s: %v", resolverName, namespace, runName, err) } diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref.go b/pkg/reconciler/pipelinerun/resources/pipelineref.go index a6674f15483..97f2dad0159 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref.go @@ -70,7 +70,7 @@ func GetPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clien } replacedParams := pr.Params.ReplaceVariables(stringReplacements, arrayReplacements, objectReplacements) - resolver := resolution.NewResolver(requester, pipelineRun, string(pr.Resolver), "", "", replacedParams) + resolver := resolution.NewResolver(requester, pipelineRun, string(pr.Resolver), "", "", pr.Name, replacedParams) return resolvePipeline(ctx, resolver, name, namespace, k8s, tekton, verificationPolicies) } default: diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref_test.go b/pkg/reconciler/pipelinerun/resources/pipelineref_test.go index 564a5160996..db6a9693ac0 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref_test.go @@ -434,6 +434,7 @@ func TestGetPipelineFunc_RemoteResolution_ReplacedParams(t *testing.T) { ctx = config.ToContext(ctx, cfg) pipeline := parse.MustParseV1Pipeline(t, pipelineYAMLString) pipelineRef := &v1.PipelineRef{ + Name: "https://foo/bar", ResolverRef: v1.ResolverRef{ Resolver: "git", Params: []v1.Param{{ @@ -461,6 +462,7 @@ func TestGetPipelineFunc_RemoteResolution_ReplacedParams(t *testing.T) { Name: "bar", Value: *v1.NewStructuredValues("test-pipeline"), }}, + ResolutionName: "https://foo/bar", } fn := resources.GetPipelineFunc(ctx, nil, clients, requester, &v1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go index 14317583240..6e0ccf56b76 100644 --- a/pkg/reconciler/taskrun/resources/taskref.go +++ b/pkg/reconciler/taskrun/resources/taskref.go @@ -108,7 +108,7 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset } else { replacedParams = append(replacedParams, tr.Params...) } - resolver := resolution.NewResolver(requester, owner, string(tr.Resolver), trName, namespace, replacedParams) + resolver := resolution.NewResolver(requester, owner, string(tr.Resolver), trName, namespace, tr.Name, replacedParams) return resolveTask(ctx, resolver, name, namespace, kind, k8s, tekton, verificationPolicies) } @@ -135,7 +135,7 @@ func GetStepActionFunc(tekton clientset.Interface, k8s kubernetes.Interface, req // casting it to a StepAction. return func(ctx context.Context, name string) (*v1alpha1.StepAction, *v1.RefSource, error) { // TODO(#7259): support params replacements for resolver params - resolver := resolution.NewResolver(requester, tr, string(step.Ref.Resolver), trName, namespace, step.Ref.Params) + resolver := resolution.NewResolver(requester, tr, string(step.Ref.Resolver), trName, namespace, step.Ref.Name, step.Ref.Params) return resolveStepAction(ctx, resolver, name, namespace, k8s, tekton) } } diff --git a/pkg/reconciler/taskrun/resources/taskref_test.go b/pkg/reconciler/taskrun/resources/taskref_test.go index 27379a2f94b..39f5c78f2dc 100644 --- a/pkg/reconciler/taskrun/resources/taskref_test.go +++ b/pkg/reconciler/taskrun/resources/taskref_test.go @@ -936,6 +936,7 @@ func TestGetTaskFunc_RemoteResolution_ReplacedParams(t *testing.T) { ctx = config.ToContext(ctx, cfg) task := parse.MustParseV1Task(t, taskYAMLString) taskRef := &v1.TaskRef{ + Name: "https://foo/bar", ResolverRef: v1.ResolverRef{ Resolver: "git", Params: []v1.Param{{ @@ -963,6 +964,7 @@ func TestGetTaskFunc_RemoteResolution_ReplacedParams(t *testing.T) { Name: "bar", Value: *v1.NewStructuredValues("test-task"), }}, + ResolutionName: "https://foo/bar", } tr := &v1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 0f265e5cf9f..b834426d715 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -6940,7 +6940,7 @@ func TestIsConcurrentModificationError(t *testing.T) { // the ResolutionRequest's name is generated by resolverName, namespace and runName. func getResolvedResolutionRequest(t *testing.T, resolverName string, resourceBytes []byte, namespace string, runName string) resolutionv1beta1.ResolutionRequest { t.Helper() - name, err := remoteresource.GenerateDeterministicName(resolverName, namespace+"/"+runName, nil) + name, err := remoteresource.GenerateDeterministicName(resolverName, namespace+"/"+runName, nil, "") if err != nil { t.Errorf("error generating name for %s/%s/%s: %v", resolverName, namespace, runName, err) } diff --git a/pkg/remote/resolution/resolver.go b/pkg/remote/resolution/resolver.go index 772b39e416a..2bc4932d34b 100644 --- a/pkg/remote/resolution/resolver.go +++ b/pkg/remote/resolution/resolver.go @@ -36,6 +36,7 @@ type Resolver struct { owner kmeta.OwnerRefable resolverName string params v1.Params + resolutionName string targetName string targetNamespace string } @@ -44,11 +45,12 @@ var _ remote.Resolver = &Resolver{} // NewResolver returns an implementation of remote.Resolver capable // of performing asynchronous remote resolution. -func NewResolver(requester remoteresource.Requester, owner kmeta.OwnerRefable, resolverName string, targetName string, targetNamespace string, params v1.Params) remote.Resolver { +func NewResolver(requester remoteresource.Requester, owner kmeta.OwnerRefable, resolverName string, targetName string, targetNamespace string, resolutionName string, params v1.Params) remote.Resolver { return &Resolver{ requester: requester, owner: owner, resolverName: resolverName, + resolutionName: resolutionName, params: params, targetName: targetName, targetNamespace: targetNamespace, @@ -58,7 +60,7 @@ func NewResolver(requester remoteresource.Requester, owner kmeta.OwnerRefable, r // Get implements remote.Resolver. func (resolver *Resolver) Get(ctx context.Context, _, _ string) (runtime.Object, *v1.RefSource, error) { resolverName := remoteresource.ResolverName(resolver.resolverName) - req, err := buildRequest(resolver.resolverName, resolver.owner, resolver.targetName, resolver.targetNamespace, resolver.params) + req, err := buildRequest(resolver.resolverName, resolver.owner, resolver.targetName, resolver.targetNamespace, resolver.resolutionName, resolver.params) if err != nil { return nil, nil, fmt.Errorf("error building request for remote resource: %w", err) } @@ -88,7 +90,7 @@ func (resolver *Resolver) List(_ context.Context) ([]remote.ResolvedObject, erro return nil, nil } -func buildRequest(resolverName string, owner kmeta.OwnerRefable, name string, namespace string, params v1.Params) (*resolutionRequest, error) { +func buildRequest(resolverName string, owner kmeta.OwnerRefable, name string, namespace string, resolutionName string, params v1.Params) (*resolutionRequest, error) { if name == "" { name = owner.GetObjectMeta().GetName() namespace = owner.GetObjectMeta().GetNamespace() @@ -100,12 +102,12 @@ func buildRequest(resolverName string, owner kmeta.OwnerRefable, name string, na // prevents multiple requests being issued for the same // pipelinerun's pipelineRef or taskrun's taskRef. remoteResourceBaseName := namespace + "/" + name - name, err := remoteresource.GenerateDeterministicName(resolverName, remoteResourceBaseName, params) + name, err := remoteresource.GenerateDeterministicName(resolverName, remoteResourceBaseName, params, resolutionName) if err != nil { return nil, fmt.Errorf("error generating name for taskrun %s/%s: %w", namespace, name, err) } req := &resolutionRequest{ - Request: remoteresource.NewRequest(name, namespace, params), + Request: remoteresource.NewRequest(name, namespace, params, resolutionName), owner: owner, } return req, nil diff --git a/pkg/remote/resolution/resolver_test.go b/pkg/remote/resolution/resolver_test.go index 8e900ba50ea..84adab9f6e6 100644 --- a/pkg/remote/resolution/resolver_test.go +++ b/pkg/remote/resolution/resolver_test.go @@ -68,7 +68,7 @@ func TestGet_Successful(t *testing.T) { SubmitErr: nil, ResolvedResource: resolved, } - resolver := NewResolver(requester, owner, "git", "", "", nil) + resolver := NewResolver(requester, owner, "git", "", "", "", nil) if _, _, err := resolver.Get(ctx, "foo", "bar"); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -121,7 +121,7 @@ func TestGet_Errors(t *testing.T) { SubmitErr: tc.submitErr, ResolvedResource: tc.resolvedResource, } - resolver := NewResolver(requester, owner, "git", "", "", nil) + resolver := NewResolver(requester, owner, "git", "", "", "", nil) obj, refSource, err := resolver.Get(ctx, "foo", "bar") if obj != nil { t.Errorf("received unexpected resolved resource") @@ -155,7 +155,7 @@ func TestBuildRequest(t *testing.T) { }, } - req, err := buildRequest("git", owner, tc.targetName, tc.targetNamespace, nil) + req, err := buildRequest("git", owner, tc.targetName, tc.targetNamespace, "", nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -166,7 +166,7 @@ func TestBuildRequest(t *testing.T) { if tc.targetName != "" { reqNameBase = tc.targetNamespace + "/" + tc.targetName } - expectedReqName, err := remoteresource.GenerateDeterministicName("git", reqNameBase, nil) + expectedReqName, err := remoteresource.GenerateDeterministicName("git", reqNameBase, nil, "") if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/resolution/common/interface.go b/pkg/resolution/common/interface.go index 3d2941c8aed..c6f8ac5ac5a 100644 --- a/pkg/resolution/common/interface.go +++ b/pkg/resolution/common/interface.go @@ -45,6 +45,7 @@ type Request interface { Name() string Namespace() string Params() pipelinev1.Params + ResolutionName() string } // OwnedRequest is implemented by any type implementing Request that also needs diff --git a/pkg/resolution/resolver/bundle/resolver.go b/pkg/resolution/resolver/bundle/resolver.go index a5cd07ac5f3..292ba9c0777 100644 --- a/pkg/resolution/resolver/bundle/resolver.go +++ b/pkg/resolution/resolver/bundle/resolver.go @@ -74,6 +74,11 @@ func (r *Resolver) GetSelector(context.Context) map[string]string { } } +// ValidateResolutionName ensures that the resolutionName from the request can be parsed. +func (r *Resolver) ValidateResolutionName(ctx context.Context, resolutionName string) error { + return nil +} + // ValidateParams ensures parameters from a request are as expected. func (r *Resolver) ValidateParams(ctx context.Context, params []pipelinev1.Param) error { if r.isDisabled(ctx) { @@ -86,7 +91,7 @@ func (r *Resolver) ValidateParams(ctx context.Context, params []pipelinev1.Param } // Resolve uses the given params to resolve the requested file or resource. -func (r *Resolver) Resolve(ctx context.Context, params []pipelinev1.Param) (framework.ResolvedResource, error) { +func (r *Resolver) Resolve(ctx context.Context, resolutionName string, params []pipelinev1.Param) (framework.ResolvedResource, error) { if r.isDisabled(ctx) { return nil, errors.New(disabledError) } diff --git a/pkg/resolution/resolver/bundle/resolver_test.go b/pkg/resolution/resolver/bundle/resolver_test.go index 4f575d00b8f..598dadd3ba5 100644 --- a/pkg/resolution/resolver/bundle/resolver_test.go +++ b/pkg/resolution/resolver/bundle/resolver_test.go @@ -184,7 +184,7 @@ func TestResolveDisabled(t *testing.T) { Name: bundle.ParamImagePullSecret, Value: *pipelinev1.NewStructuredValues("baz"), }} - _, err = resolver.Resolve(resolverDisabledContext(), params) + _, err = resolver.Resolve(resolverDisabledContext(), "", params) if err == nil { t.Fatalf("expected disabled err") } diff --git a/pkg/resolution/resolver/cluster/resolver.go b/pkg/resolution/resolver/cluster/resolver.go index 44c5c800b9e..eb42b0b43e5 100644 --- a/pkg/resolution/resolver/cluster/resolver.go +++ b/pkg/resolution/resolver/cluster/resolver.go @@ -86,9 +86,14 @@ func (r *Resolver) ValidateParams(ctx context.Context, params []pipelinev1.Param return err } +// ValidateResolutionName ensures that the resolutionName from the request can be parsed. +func (r *Resolver) ValidateResolutionName(ctx context.Context, resolutionName string) error { + return nil +} + // Resolve performs the work of fetching a resource from a namespace with the given // parameters. -func (r *Resolver) Resolve(ctx context.Context, origParams []pipelinev1.Param) (framework.ResolvedResource, error) { +func (r *Resolver) Resolve(ctx context.Context, resolutionName string, origParams []pipelinev1.Param) (framework.ResolvedResource, error) { if r.isDisabled(ctx) { return nil, errors.New(disabledError) } diff --git a/pkg/resolution/resolver/framework/fakeresolver.go b/pkg/resolution/resolver/framework/fakeresolver.go index 0943199601b..fc03b06b9a7 100644 --- a/pkg/resolution/resolver/framework/fakeresolver.go +++ b/pkg/resolution/resolver/framework/fakeresolver.go @@ -38,6 +38,8 @@ const ( // FakeParamName is the name used for the fake resolver's single parameter. FakeParamName string = "fake-key" + + FakeResolutionName string = "fake://url" ) var _ Resolver = &FakeResolver{} @@ -92,6 +94,10 @@ func (r *FakeResolver) GetName(_ context.Context) string { return FakeResolverName } +func (r *FakeResolver) GetResolutionName(_ context.Context) string { + return FakeResolutionName +} + // GetSelector returns the labels that resource requests are required to have for // the fake resolver to process them. func (r *FakeResolver) GetSelector(_ context.Context) map[string]string { @@ -100,6 +106,10 @@ func (r *FakeResolver) GetSelector(_ context.Context) map[string]string { } } +func (r *FakeResolver) ValidateResolutionName(_ context.Context, resolutionName string) error { + return nil +} + // ValidateParams returns an error if the given parameter map is not // valid for a resource request targeting the fake resolver. func (r *FakeResolver) ValidateParams(_ context.Context, params []pipelinev1.Param) error { @@ -131,7 +141,7 @@ func (r *FakeResolver) ValidateParams(_ context.Context, params []pipelinev1.Par // Resolve performs the work of fetching a file from the fake resolver given a map of // parameters. -func (r *FakeResolver) Resolve(_ context.Context, params []pipelinev1.Param) (ResolvedResource, error) { +func (r *FakeResolver) Resolve(_ context.Context, resolutionName string, params []pipelinev1.Param) (ResolvedResource, error) { paramsMap := make(map[string]pipelinev1.ParamValue) for _, p := range params { paramsMap[p.Name] = p.Value diff --git a/pkg/resolution/resolver/framework/interface.go b/pkg/resolution/resolver/framework/interface.go index 0a0118c28da..7288f779209 100644 --- a/pkg/resolution/resolver/framework/interface.go +++ b/pkg/resolution/resolver/framework/interface.go @@ -43,13 +43,17 @@ type Resolver interface { // request and should return an error if any are missing or invalid. ValidateParams(context.Context, []pipelinev1.Param) error - // Resolve receives the parameters passed via a resource request + // ValidateResolutionName is given the resolutionName from a resource + // request and should return an error if the resolver cannot resolve it. + ValidateResolutionName(context.Context, string) error + + // Resolve receives the url and parameters passed via a resource request // and returns the resolved data along with any annotations // to include in the response. If resolution fails then an error // should be returned instead. If a resolution.Error // is returned then its Reason and Message are used as part of the // response to the request. - Resolve(context.Context, []pipelinev1.Param) (ResolvedResource, error) + Resolve(context.Context, string, []pipelinev1.Param) (ResolvedResource, error) } // ConfigWatcher is the interface to implement if your resolver accepts diff --git a/pkg/resolution/resolver/framework/reconciler.go b/pkg/resolution/resolver/framework/reconciler.go index b981ea6e02a..c9ff32106f2 100644 --- a/pkg/resolution/resolver/framework/reconciler.go +++ b/pkg/resolution/resolver/framework/reconciler.go @@ -119,15 +119,28 @@ func (r *Reconciler) resolve(ctx context.Context, key string, rr *v1beta1.Resolu defer cancelFn() go func() { - validationError := r.resolver.ValidateParams(resolutionCtx, rr.Spec.Params) - if validationError != nil { - errChan <- &resolutioncommon.InvalidRequestError{ - ResolutionRequestKey: key, - Message: validationError.Error(), + // if params are provided, use them instead of the resolutionName to resolve the request. + // The webhook will thwor a validation error if both are provided anyway. + if len(rr.Spec.Params) > 0 { + validationError := r.resolver.ValidateParams(resolutionCtx, rr.Spec.Params) + if validationError != nil { + errChan <- &resolutioncommon.InvalidRequestError{ + ResolutionRequestKey: key, + Message: validationError.Error(), + } + return + } + } else { + validationError := r.resolver.ValidateResolutionName(resolutionCtx, rr.Spec.ResolutionName) + if validationError != nil { + errChan <- &resolutioncommon.InvalidRequestError{ + ResolutionRequestKey: key, + Message: validationError.Error(), + } + return } - return } - resource, resolveErr := r.resolver.Resolve(resolutionCtx, rr.Spec.Params) + resource, resolveErr := r.resolver.Resolve(resolutionCtx, rr.Spec.ResolutionName, rr.Spec.Params) if resolveErr != nil { errChan <- &resolutioncommon.GetResourceError{ ResolverName: r.resolver.GetName(resolutionCtx), diff --git a/pkg/resolution/resolver/git/resolver.go b/pkg/resolution/resolver/git/resolver.go index 3405eff6a78..e69665c409c 100644 --- a/pkg/resolution/resolver/git/resolver.go +++ b/pkg/resolution/resolver/git/resolver.go @@ -124,9 +124,14 @@ func (r *Resolver) ValidateParams(ctx context.Context, params []pipelinev1.Param return nil } +// ValidateResolutionName ensures that the resolutionName from the request can be parsed. +func (r *Resolver) ValidateResolutionName(ctx context.Context, resolutionName string) error { + return nil +} + // Resolve performs the work of fetching a file from git given a map of // parameters. -func (r *Resolver) Resolve(ctx context.Context, origParams []pipelinev1.Param) (framework.ResolvedResource, error) { +func (r *Resolver) Resolve(ctx context.Context, resolutionName string, origParams []pipelinev1.Param) (framework.ResolvedResource, error) { if r.isDisabled(ctx) { return nil, errors.New(disabledError) } diff --git a/pkg/resolution/resolver/git/resolver_test.go b/pkg/resolution/resolver/git/resolver_test.go index a35ec7646a2..28ff38f40c8 100644 --- a/pkg/resolution/resolver/git/resolver_test.go +++ b/pkg/resolution/resolver/git/resolver_test.go @@ -245,7 +245,7 @@ func TestResolveNotEnabled(t *testing.T) { pathParam: "bar", revisionParam: "baz", } - _, err = resolver.Resolve(resolverDisabledContext(), toParams(someParams)) + _, err = resolver.Resolve(resolverDisabledContext(), "", toParams(someParams)) if err == nil { t.Fatalf("expected disabled err") } diff --git a/pkg/resolution/resolver/http/resolver.go b/pkg/resolution/resolver/http/resolver.go index 9e920f360f8..00bb1e2f605 100644 --- a/pkg/resolution/resolver/http/resolver.go +++ b/pkg/resolution/resolver/http/resolver.go @@ -87,6 +87,11 @@ func (r *Resolver) GetSelector(context.Context) map[string]string { } } +// ValidateResolutionName ensures that the resolutionName from the request can be parsed. +func (r *Resolver) ValidateResolutionName(ctx context.Context, resolutionName string) error { + return nil +} + // ValidateParams ensures parameters from a request are as expected. func (r *Resolver) ValidateParams(ctx context.Context, params []pipelinev1.Param) error { if r.isDisabled(ctx) { @@ -100,7 +105,7 @@ func (r *Resolver) ValidateParams(ctx context.Context, params []pipelinev1.Param } // Resolve uses the given params to resolve the requested file or resource. -func (r *Resolver) Resolve(ctx context.Context, oParams []pipelinev1.Param) (framework.ResolvedResource, error) { +func (r *Resolver) Resolve(ctx context.Context, resolutionName string, oParams []pipelinev1.Param) (framework.ResolvedResource, error) { if r.isDisabled(ctx) { return nil, errors.New(disabledError) } diff --git a/pkg/resolution/resolver/http/resolver_test.go b/pkg/resolution/resolver/http/resolver_test.go index e2e8e7758be..da2dd2cdf1e 100644 --- a/pkg/resolution/resolver/http/resolver_test.go +++ b/pkg/resolution/resolver/http/resolver_test.go @@ -191,7 +191,7 @@ func TestResolve(t *testing.T) { }) } resolver := Resolver{} - output, err := resolver.Resolve(contextWithConfig(defaultHttpTimeoutValue), params) + output, err := resolver.Resolve(contextWithConfig(defaultHttpTimeoutValue), "", params) if tc.expectedErr != "" { re := regexp.MustCompile(tc.expectedErr) if !re.MatchString(err.Error()) { @@ -226,7 +226,7 @@ func TestResolveNotEnabled(t *testing.T) { var err error resolver := Resolver{} someParams := map[string]string{} - _, err = resolver.Resolve(resolverDisabledContext(), toParams(someParams)) + _, err = resolver.Resolve(resolverDisabledContext(), "", toParams(someParams)) if err == nil { t.Fatalf("expected disabled err") } diff --git a/pkg/resolution/resolver/hub/resolver.go b/pkg/resolution/resolver/hub/resolver.go index abbc19c1985..20d313c8327 100644 --- a/pkg/resolution/resolver/hub/resolver.go +++ b/pkg/resolution/resolver/hub/resolver.go @@ -92,6 +92,11 @@ func (r *Resolver) ValidateParams(ctx context.Context, params []pipelinev1.Param return nil } +// ValidateResolutionName ensures that the resolutionName from the request can be parsed. +func (r *Resolver) ValidateResolutionName(ctx context.Context, resolutionName string) error { + return nil +} + type tektonHubDataResponse struct { YAML string `json:"yaml"` } @@ -109,7 +114,7 @@ type artifactHubResponse struct { } // Resolve uses the given params to resolve the requested file or resource. -func (r *Resolver) Resolve(ctx context.Context, params []pipelinev1.Param) (framework.ResolvedResource, error) { +func (r *Resolver) Resolve(ctx context.Context, resolutionName string, params []pipelinev1.Param) (framework.ResolvedResource, error) { if r.isDisabled(ctx) { return nil, errors.New(disabledError) } diff --git a/pkg/resolution/resolver/hub/resolver_test.go b/pkg/resolution/resolver/hub/resolver_test.go index e6ba327c969..d2ea4d716fe 100644 --- a/pkg/resolution/resolver/hub/resolver_test.go +++ b/pkg/resolution/resolver/hub/resolver_test.go @@ -359,7 +359,7 @@ func TestResolveConstraint(t *testing.T) { ParamCatalog: tt.catalog, ParamType: tt.hubType, } - output, err := resolver.Resolve(contextWithConfig(), toParams(params)) + output, err := resolver.Resolve(contextWithConfig(), "", toParams(params)) if tt.expectedErr != nil { checkExpectedErr(t, tt.expectedErr, err) } else { @@ -496,7 +496,7 @@ func TestResolveDisabled(t *testing.T) { ParamVersion: "bar", ParamCatalog: "baz", } - _, err = resolver.Resolve(resolverDisabledContext(), toParams(params)) + _, err = resolver.Resolve(resolverDisabledContext(), "", toParams(params)) if err == nil { t.Fatalf("expected missing name err") } @@ -597,7 +597,7 @@ func TestResolve(t *testing.T) { ParamType: tc.hubType, } - output, err := resolver.Resolve(contextWithConfig(), toParams(params)) + output, err := resolver.Resolve(contextWithConfig(), "", toParams(params)) if tc.expectedErr != nil { checkExpectedErr(t, tc.expectedErr, err) } else { diff --git a/pkg/resolution/resource/crd_resource.go b/pkg/resolution/resource/crd_resource.go index 90fd7653303..b640b9018e5 100644 --- a/pkg/resolution/resource/crd_resource.go +++ b/pkg/resolution/resource/crd_resource.go @@ -98,7 +98,8 @@ func (r *CRDRequester) createResolutionRequest(ctx context.Context, resolver Res }, }, Spec: v1beta1.ResolutionRequestSpec{ - Params: req.Params(), + Params: req.Params(), + ResolutionName: req.ResolutionName(), }, } appendOwnerReference(rr, req) diff --git a/pkg/resolution/resource/crd_resource_test.go b/pkg/resolution/resource/crd_resource_test.go index da5a06fac38..68716bc94ea 100644 --- a/pkg/resolution/resource/crd_resource_test.go +++ b/pkg/resolution/resource/crd_resource_test.go @@ -75,6 +75,7 @@ params: value: main - name: pathInRepo value: task/git-clone/0.6/git-clone.yaml +resolutionName: "https://foo/bar" `) baseRR := mustParseResolutionRequest(t, ` kind: "ResolutionRequest" @@ -99,6 +100,7 @@ spec: value: "main" - name: "pathInRepo" value: "task/git-clone/0.6/git-clone.yaml" + resolutionName: "https://foo/bar" `) createdRR := baseRR.DeepCopy() // diff --git a/pkg/resolution/resource/name.go b/pkg/resolution/resource/name.go index 051eabc89d0..4f2d23720ea 100644 --- a/pkg/resolution/resource/name.go +++ b/pkg/resolution/resource/name.go @@ -35,7 +35,7 @@ func nameHasher() hash.Hash { // will have the format {prefix}-{hash} where {prefix} is // given and {hash} is nameHasher(base) + nameHasher(param1) + // nameHasher(param2) + ... -func GenerateDeterministicName(prefix, base string, params v1.Params) (string, error) { +func GenerateDeterministicName(prefix, base string, params v1.Params, resolutionName string) (string, error) { hasher := nameHasher() if _, err := hasher.Write([]byte(base)); err != nil { return "", err @@ -67,5 +67,8 @@ func GenerateDeterministicName(prefix, base string, params v1.Params) (string, e } } } + if _, err := hasher.Write([]byte(resolutionName)); err != nil { + return "", err + } return fmt.Sprintf("%s-%x", prefix, hasher.Sum(nil)), nil } diff --git a/pkg/resolution/resource/name_test.go b/pkg/resolution/resource/name_test.go index 1a90a0aaf18..e24711d298d 100644 --- a/pkg/resolution/resource/name_test.go +++ b/pkg/resolution/resource/name_test.go @@ -25,9 +25,10 @@ import ( func TestGenerateDeterministicName(t *testing.T) { type args struct { - prefix string - base string - params []v1.Param + prefix string + base string + params []v1.Param + resolutionName string } golden := args{ prefix: "prefix", @@ -55,6 +56,7 @@ func TestGenerateDeterministicName(t *testing.T) { }, }, }, + resolutionName: "https://foo/bar", } tests := []struct { name string @@ -76,6 +78,13 @@ func TestGenerateDeterministicName(t *testing.T) { }, want: "-6989337ae0757277b806e97e86444ef0", }, + { + name: "only contains resolutionName", + args: args{ + resolutionName: golden.resolutionName, + }, + want: "-dcfaf53735f4a84a3e319e17352940b4", + }, { name: "only contains params", args: args{ @@ -97,12 +106,12 @@ func TestGenerateDeterministicName(t *testing.T) { { name: "contain all fields", args: golden, - want: "prefix-ba2f256f318de7f4154da577c283cb9e", + want: "prefix-ff25bd24688ab610bdc530a5ab3aabbd", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := resource.GenerateDeterministicName(tt.args.prefix, tt.args.base, tt.args.params) + got, err := resource.GenerateDeterministicName(tt.args.prefix, tt.args.base, tt.args.params, tt.args.resolutionName) if (err != nil) != tt.wantErr { t.Errorf("GenerateDeterministicName() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/pkg/resolution/resource/request.go b/pkg/resolution/resource/request.go index 9e0f3e194e7..4ba73a282ab 100644 --- a/pkg/resolution/resource/request.go +++ b/pkg/resolution/resource/request.go @@ -22,15 +22,16 @@ var _ Request = &BasicRequest{} // BasicRequest holds the fields needed to submit a new resource request. type BasicRequest struct { - name string - namespace string - params v1.Params + name string + namespace string + params v1.Params + resolutionName string } // NewRequest returns an instance of a BasicRequest with the given name, // namespace and params. -func NewRequest(name, namespace string, params v1.Params) Request { - return &BasicRequest{name, namespace, params} +func NewRequest(name, namespace string, params v1.Params, resolutionName string) Request { + return &BasicRequest{name, namespace, params, resolutionName} } var _ Request = &BasicRequest{} @@ -49,3 +50,7 @@ func (req *BasicRequest) Namespace() string { func (req *BasicRequest) Params() v1.Params { return req.params } + +func (req *BasicRequest) ResolutionName() string { + return req.resolutionName +} diff --git a/pkg/resolution/resource/request_test.go b/pkg/resolution/resource/request_test.go index e91b1c799b5..704cb2e9b2e 100644 --- a/pkg/resolution/resource/request_test.go +++ b/pkg/resolution/resource/request_test.go @@ -27,9 +27,10 @@ import ( func TestNewRequest(t *testing.T) { type args struct { - name string - namespace string - params v1.Params + name string + namespace string + params v1.Params + resolutionName string } type want = args golden := args{ @@ -39,6 +40,7 @@ func TestNewRequest(t *testing.T) { {Name: "param1", Value: v1.ParamValue{Type: v1.ParamTypeString, StringVal: "value1"}}, {Name: "param2", Value: v1.ParamValue{Type: v1.ParamTypeString, StringVal: "value2"}}, }, + resolutionName: "https://foo/bar", } tests := []struct { name string @@ -58,7 +60,7 @@ func TestNewRequest(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - request := resource.NewRequest(tt.args.name, tt.args.namespace, tt.args.params) + request := resource.NewRequest(tt.args.name, tt.args.namespace, tt.args.params, tt.args.resolutionName) if request == nil { t.Errorf("NewRequest() return nil") } diff --git a/test/e2e-tests-kind-prow-alpha.env b/test/e2e-tests-kind-prow-alpha.env index 255bdf63579..dfcc2e76444 100644 --- a/test/e2e-tests-kind-prow-alpha.env +++ b/test/e2e-tests-kind-prow-alpha.env @@ -8,3 +8,4 @@ ENABLE_STEP_ACTIONS=true ENABLE_CEL_IN_WHENEXPRESSION=true ENABLE_PARAM_ENUM=true ENABLE_ARTIFACTS=true +ENABLE_CONCISE_RESOLVER_SYNTAX=true diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index a291f27e4ba..8a8b63a48eb 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -32,6 +32,7 @@ ENABLE_STEP_ACTIONS=${ENABLE_STEP_ACTIONS:="false"} ENABLE_CEL_IN_WHENEXPRESSION=${ENABLE_CEL_IN_WHENEXPRESSION:="false"} ENABLE_PARAM_ENUM=${ENABLE_PARAM_ENUM:="false"} ENABLE_ARTIFACTS=${ENABLE_ARTIFACTS:="false"} +ENABLE_CONCISE_RESOLVER_SYNTAX=${ENABLE_CONCISE_RESOLVER_SYNTAX:="false"} failed=0 # Script entry point. @@ -130,6 +131,18 @@ function set_enable_artifacts() { kubectl patch configmap feature-flags -n tekton-pipelines -p "$jsonpatch" } +function set_enable_concise_resolver_syntax() { + local method="$1" + if [ "$method" != "false" ] && [ "$method" != "true" ]; then + printf "Invalid value for enable-concise-resolver-syntax %s\n" ${method} + exit 255 + fi + printf "Setting enable-concise-resolver-syntax to %s\n", ${method} + jsonpatch=$(printf "{\"data\": {\"enable-concise-resolver-syntax\": \"%s\"}}" $1) + echo "feature-flags ConfigMap patch: ${jsonpatch}" + kubectl patch configmap feature-flags -n tekton-pipelines -p "$jsonpatch" +} + function run_e2e() { # Run the integration tests header "Running Go e2e tests" @@ -157,6 +170,7 @@ set_enable_step_actions "$ENABLE_STEP_ACTIONS" set_cel_in_whenexpression "$ENABLE_CEL_IN_WHENEXPRESSION" set_enable_param_enum "$ENABLE_PARAM_ENUM" set_enable_artifacts "$ENABLE_ARTIFACTS" +set_enable_concise_resolver_syntax "$ENABLE_CONCISE_RESOLVER_SYNTAX" run_e2e (( failed )) && fail_test diff --git a/test/featureflags.go b/test/featureflags.go index 673d82cb75f..a8551be5401 100644 --- a/test/featureflags.go +++ b/test/featureflags.go @@ -111,13 +111,14 @@ func requireAllGates(gates map[string]string) func(context.Context, *testing.T, func getFeatureFlagsBaseOnAPIFlag(t *testing.T) *config.FeatureFlags { t.Helper() alphaFeatureFlags, err := config.NewFeatureFlagsFromMap(map[string]string{ - "enable-api-fields": "alpha", - "results-from": "sidecar-logs", - "enable-tekton-oci-bundles": "true", - "enable-step-actions": "true", - "enable-cel-in-whenexpression": "true", - "enable-param-enum": "true", - "enable-artifacts": "true", + "enable-api-fields": "alpha", + "results-from": "sidecar-logs", + "enable-tekton-oci-bundles": "true", + "enable-step-actions": "true", + "enable-cel-in-whenexpression": "true", + "enable-param-enum": "true", + "enable-artifacts": "true", + "enable-concise-resolver-syntax": "true", }) if err != nil { t.Fatalf("error creating alpha feature flags configmap: %v", err) diff --git a/test/resolution.go b/test/resolution.go index 514988427f2..9c5c31c67ff 100644 --- a/test/resolution.go +++ b/test/resolution.go @@ -61,7 +61,8 @@ type Requester struct { // An error to return when a request is submitted. SubmitErr error // Params that should match those on the request in order to return the resolved resource - Params []pipelinev1.Param + Params []pipelinev1.Param + ResolutionName string } // Submit implements resolution.Requester, accepting the name of a @@ -71,6 +72,10 @@ func (r *Requester) Submit(ctx context.Context, resolverName resolution.Resolver if len(r.Params) == 0 { return r.ResolvedResource, r.SubmitErr } + if r.ResolutionName == "" { + return r.ResolvedResource, r.SubmitErr + } + fmt.Println("PARAMS: ", req.Params()) reqParams := make(map[string]pipelinev1.ParamValue) for _, p := range req.Params() { reqParams[p.Name] = p.Value @@ -87,6 +92,10 @@ func (r *Requester) Submit(ctx context.Context, resolverName resolution.Resolver if len(wrongParams) > 0 { return nil, errors.New(strings.Join(wrongParams, "; ")) } + fmt.Println("RESOLUTION NAME CHECK: ", r.ResolutionName, req.ResolutionName()) + if r.ResolutionName != req.ResolutionName() { + return nil, errors.New(fmt.Sprintf("Resolution name did not match. Got %s; Want %s", req.ResolutionName(), r.ResolutionName)) + } return r.ResolvedResource, r.SubmitErr } @@ -130,7 +139,8 @@ type RawRequest struct { // the request namespace Namespace string // the params for the request - Params []pipelinev1.Param + Params []pipelinev1.Param + ResolutionName string } // Request returns a Request interface based on the RawRequest. @@ -152,12 +162,13 @@ type Request struct { var _ resolution.Request = &Request{} // NewRequest creates a mock request that is populated with the given name namespace and params -func NewRequest(name, namespace string, params []pipelinev1.Param) *Request { +func NewRequest(name, namespace string, resolutionName string, params []pipelinev1.Param) *Request { return &Request{ RawRequest: RawRequest{ - Name: name, - Namespace: namespace, - Params: params, + Name: name, + Namespace: namespace, + Params: params, + ResolutionName: resolutionName, }, } } @@ -176,3 +187,7 @@ func (r *Request) Namespace() string { func (r *Request) Params() pipelinev1.Params { return r.RawRequest.Params } + +func (r *Request) ResolutionName() string { + return r.RawRequest.ResolutionName +}