Skip to content

Commit 445734d

Browse files
lbernicktekton-robot
lbernick
authored and
tekton-robot
committed
Add webhook validation for remote Tasks
A prior commit added validation for remote Pipelines by issuing dry-run create requests to the kubernetes API server, allowing validating admission webhooks to accept or reject remote pipelines without actually creating them. This commit adds the same logic for remote Tasks, and moves common logic into a shared package.
1 parent 7884311 commit 445734d

File tree

10 files changed

+422
-80
lines changed

10 files changed

+422
-80
lines changed

‎pkg/pod/status.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ const (
5151
// that taskrun failed runtime validation
5252
ReasonFailedValidation = "TaskRunValidationFailed"
5353

54+
// ReasonTaskFailedValidation indicated that the reason for failure status is
55+
// that task failed runtime validation
56+
ReasonTaskFailedValidation = "TaskValidationFailed"
57+
5458
// ReasonExceededResourceQuota indicates that the TaskRun failed to create a pod due to
5559
// a ResourceQuota in the namespace
5660
ReasonExceededResourceQuota = "ExceededResourceQuota"

‎pkg/reconciler/apiserver/apiserver.go‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
package apiserver
2+
3+
import (
4+
"context"
5+
"errors"
6+
"fmt"
7+
8+
"github.com/google/uuid"
9+
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
10+
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
11+
clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned"
12+
apierrors "k8s.io/apimachinery/pkg/api/errors"
13+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
14+
"k8s.io/apimachinery/pkg/runtime"
15+
)
16+
17+
var (
18+
ErrReferencedObjectValidationFailed = errors.New("validation failed for referenced object")
19+
ErrCouldntValidateObjectRetryable = errors.New("retryable error validating referenced object")
20+
ErrCouldntValidateObjectPermanent = errors.New("permanent error validating referenced object")
21+
)
22+
23+
// DryRunValidate validates the obj by issuing a dry-run create request for it in the given namespace.
24+
// This allows validating admission webhooks to process the object without actually creating it.
25+
// obj must be a v1/v1beta1 Task or Pipeline.
26+
func DryRunValidate(ctx context.Context, namespace string, obj runtime.Object, tekton clientset.Interface) error {
27+
dryRunObjName := uuid.NewString() // Use a randomized name for the Pipeline/Task in case there is already another Pipeline/Task of the same name
28+
29+
switch obj := obj.(type) {
30+
case *v1.Pipeline:
31+
dryRunObj := obj.DeepCopy()
32+
dryRunObj.Name = dryRunObjName
33+
dryRunObj.Namespace = namespace // Make sure the namespace is the same as the PipelineRun
34+
if _, err := tekton.TektonV1().Pipelines(namespace).Create(ctx, dryRunObj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err != nil {
35+
return handleDryRunCreateErr(err, obj.Name)
36+
}
37+
case *v1beta1.Pipeline:
38+
dryRunObj := obj.DeepCopy()
39+
dryRunObj.Name = dryRunObjName
40+
dryRunObj.Namespace = namespace // Make sure the namespace is the same as the PipelineRun
41+
if _, err := tekton.TektonV1beta1().Pipelines(namespace).Create(ctx, dryRunObj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err != nil {
42+
return handleDryRunCreateErr(err, obj.Name)
43+
}
44+
45+
case *v1.Task:
46+
dryRunObj := obj.DeepCopy()
47+
dryRunObj.Name = dryRunObjName
48+
dryRunObj.Namespace = namespace // Make sure the namespace is the same as the TaskRun
49+
if _, err := tekton.TektonV1().Tasks(namespace).Create(ctx, dryRunObj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err != nil {
50+
return handleDryRunCreateErr(err, obj.Name)
51+
}
52+
case *v1beta1.Task:
53+
dryRunObj := obj.DeepCopy()
54+
dryRunObj.Name = dryRunObjName
55+
dryRunObj.Namespace = namespace // Make sure the namespace is the same as the TaskRun
56+
if _, err := tekton.TektonV1beta1().Tasks(namespace).Create(ctx, dryRunObj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err != nil {
57+
return handleDryRunCreateErr(err, obj.Name)
58+
}
59+
default:
60+
return fmt.Errorf("unsupported object GVK %s", obj.GetObjectKind().GroupVersionKind())
61+
}
62+
return nil
63+
}
64+
65+
func handleDryRunCreateErr(err error, objectName string) error {
66+
var errType error
67+
switch {
68+
case apierrors.IsBadRequest(err): // Object rejected by validating webhook
69+
errType = ErrReferencedObjectValidationFailed
70+
case apierrors.IsInvalid(err), apierrors.IsMethodNotSupported(err):
71+
errType = ErrCouldntValidateObjectPermanent
72+
case apierrors.IsTimeout(err), apierrors.IsServerTimeout(err), apierrors.IsTooManyRequests(err):
73+
errType = ErrCouldntValidateObjectRetryable
74+
default:
75+
// Assume unknown errors are retryable
76+
// Additional errors can be added to the switch statements as needed
77+
errType = ErrCouldntValidateObjectRetryable
78+
}
79+
return fmt.Errorf("%w %s: %s", errType, objectName, err.Error())
80+
}

‎pkg/reconciler/apiserver/apiserver_test.go‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
package apiserver_test
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/google/go-cmp/cmp"
8+
"github.com/google/go-cmp/cmp/cmpopts"
9+
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
10+
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
11+
"github.com/tektoncd/pipeline/pkg/client/clientset/versioned/fake"
12+
"github.com/tektoncd/pipeline/pkg/reconciler/apiserver"
13+
apierrors "k8s.io/apimachinery/pkg/api/errors"
14+
"k8s.io/apimachinery/pkg/runtime"
15+
"k8s.io/apimachinery/pkg/runtime/schema"
16+
"k8s.io/apimachinery/pkg/util/validation/field"
17+
ktesting "k8s.io/client-go/testing"
18+
)
19+
20+
func TestDryRunCreate_Valid_DifferentGVKs(t *testing.T) {
21+
tcs := []struct {
22+
name string
23+
obj runtime.Object
24+
wantErr bool
25+
}{{
26+
name: "v1 task",
27+
obj: &v1.Task{},
28+
}, {
29+
name: "v1beta1 task",
30+
obj: &v1beta1.Task{},
31+
}, {
32+
name: "v1 pipeline",
33+
obj: &v1.Pipeline{},
34+
}, {
35+
name: "v1beta1 pipeline",
36+
obj: &v1beta1.Pipeline{},
37+
}, {
38+
name: "unsupported gvk",
39+
obj: &v1beta1.ClusterTask{},
40+
wantErr: true,
41+
}}
42+
for _, tc := range tcs {
43+
t.Run(tc.name, func(t *testing.T) {
44+
tektonclient := fake.NewSimpleClientset()
45+
err := apiserver.DryRunValidate(context.Background(), "default", tc.obj, tektonclient)
46+
if (err != nil) != tc.wantErr {
47+
t.Errorf("wantErr was %t but got err %v", tc.wantErr, err)
48+
}
49+
})
50+
}
51+
}
52+
53+
func TestDryRunCreate_Invalid_DifferentGVKs(t *testing.T) {
54+
tcs := []struct {
55+
name string
56+
obj runtime.Object
57+
wantErr error
58+
}{{
59+
name: "v1 task",
60+
obj: &v1.Task{},
61+
wantErr: apiserver.ErrReferencedObjectValidationFailed,
62+
}, {
63+
name: "v1beta1 task",
64+
obj: &v1beta1.Task{},
65+
wantErr: apiserver.ErrReferencedObjectValidationFailed,
66+
}, {
67+
name: "v1 pipeline",
68+
obj: &v1.Pipeline{},
69+
wantErr: apiserver.ErrReferencedObjectValidationFailed,
70+
}, {
71+
name: "v1beta1 pipeline",
72+
obj: &v1beta1.Pipeline{},
73+
wantErr: apiserver.ErrReferencedObjectValidationFailed,
74+
}, {
75+
name: "unsupported gvk",
76+
obj: &v1beta1.ClusterTask{},
77+
wantErr: cmpopts.AnyError,
78+
}}
79+
for _, tc := range tcs {
80+
t.Run(tc.name, func(t *testing.T) {
81+
tektonclient := fake.NewSimpleClientset()
82+
tektonclient.PrependReactor("create", "tasks", func(action ktesting.Action) (bool, runtime.Object, error) {
83+
return true, nil, apierrors.NewBadRequest("bad request")
84+
})
85+
tektonclient.PrependReactor("create", "pipelines", func(action ktesting.Action) (bool, runtime.Object, error) {
86+
return true, nil, apierrors.NewBadRequest("bad request")
87+
})
88+
err := apiserver.DryRunValidate(context.Background(), "default", tc.obj, tektonclient)
89+
if d := cmp.Diff(tc.wantErr, err, cmpopts.EquateErrors()); d != "" {
90+
t.Errorf("wrong error: %s", d)
91+
}
92+
})
93+
}
94+
}
95+
96+
func TestDryRunCreate_DifferentErrTypes(t *testing.T) {
97+
tcs := []struct {
98+
name string
99+
webhookErr error
100+
wantErr error
101+
}{{
102+
name: "no error",
103+
wantErr: nil,
104+
}, {
105+
name: "bad request",
106+
webhookErr: apierrors.NewBadRequest("bad request"),
107+
wantErr: apiserver.ErrReferencedObjectValidationFailed,
108+
}, {
109+
name: "invalid",
110+
webhookErr: apierrors.NewInvalid(schema.GroupKind{Group: "tekton.dev/v1", Kind: "Task"}, "task", field.ErrorList{}),
111+
wantErr: apiserver.ErrCouldntValidateObjectPermanent,
112+
}, {
113+
name: "not supported",
114+
webhookErr: apierrors.NewMethodNotSupported(schema.GroupResource{}, "create"),
115+
wantErr: apiserver.ErrCouldntValidateObjectPermanent,
116+
}, {
117+
name: "timeout",
118+
webhookErr: apierrors.NewTimeoutError("timeout", 5),
119+
wantErr: apiserver.ErrCouldntValidateObjectRetryable,
120+
}, {
121+
name: "server timeout",
122+
webhookErr: apierrors.NewServerTimeout(schema.GroupResource{}, "create", 5),
123+
wantErr: apiserver.ErrCouldntValidateObjectRetryable,
124+
}, {
125+
name: "too many requests",
126+
webhookErr: apierrors.NewTooManyRequests("foo", 5),
127+
wantErr: apiserver.ErrCouldntValidateObjectRetryable,
128+
}}
129+
for _, tc := range tcs {
130+
t.Run(tc.name, func(t *testing.T) {
131+
tektonclient := fake.NewSimpleClientset()
132+
tektonclient.PrependReactor("create", "tasks", func(action ktesting.Action) (bool, runtime.Object, error) {
133+
return true, nil, tc.webhookErr
134+
})
135+
err := apiserver.DryRunValidate(context.Background(), "default", &v1.Task{}, tektonclient)
136+
if d := cmp.Diff(tc.wantErr, err, cmpopts.EquateErrors()); d != "" {
137+
t.Errorf("wrong error: %s", d)
138+
}
139+
})
140+
}
141+
}

‎pkg/reconciler/pipelinerun/pipelinerun.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040
resolutionutil "github.com/tektoncd/pipeline/pkg/internal/resolution"
4141
"github.com/tektoncd/pipeline/pkg/pipelinerunmetrics"
4242
tknreconciler "github.com/tektoncd/pipeline/pkg/reconciler"
43+
"github.com/tektoncd/pipeline/pkg/reconciler/apiserver"
4344
"github.com/tektoncd/pipeline/pkg/reconciler/events"
4445
"github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent"
4546
"github.com/tektoncd/pipeline/pkg/reconciler/pipeline/dag"
@@ -409,10 +410,10 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel
409410
message := fmt.Sprintf("PipelineRun %s/%s awaiting remote resource", pr.Namespace, pr.Name)
410411
pr.Status.MarkRunning(ReasonResolvingPipelineRef, message)
411412
return nil
412-
case errors.Is(err, resources.ErrReferencedPipelineValidationFailed), errors.Is(err, resources.ErrCouldntValidatePipelinePermanent):
413+
case errors.Is(err, apiserver.ErrReferencedObjectValidationFailed), errors.Is(err, apiserver.ErrCouldntValidateObjectPermanent):
413414
pr.Status.MarkFailed(ReasonFailedValidation, err.Error())
414415
return controller.NewPermanentError(err)
415-
case errors.Is(err, resources.ErrCouldntValidatePipelineRetryable):
416+
case errors.Is(err, apiserver.ErrCouldntValidateObjectRetryable):
416417
return err
417418
case err != nil:
418419
logger.Errorf("Failed to determine Pipeline spec to use for pipelinerun %s: %v", pr.Name, err)

‎pkg/reconciler/pipelinerun/resources/pipelineref.go‎

Lines changed: 5 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -21,28 +21,21 @@ import (
2121
"errors"
2222
"fmt"
2323

24-
"github.com/google/uuid"
2524
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
2625
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
2726
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
2827
clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned"
28+
"github.com/tektoncd/pipeline/pkg/reconciler/apiserver"
2929
rprp "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/pipelinespec"
3030
"github.com/tektoncd/pipeline/pkg/remote"
3131
"github.com/tektoncd/pipeline/pkg/remote/resolution"
3232
remoteresource "github.com/tektoncd/pipeline/pkg/resolution/resource"
3333
"github.com/tektoncd/pipeline/pkg/trustedresources"
34-
apierrors "k8s.io/apimachinery/pkg/api/errors"
3534
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3635
"k8s.io/apimachinery/pkg/runtime"
3736
"k8s.io/client-go/kubernetes"
3837
)
3938

40-
var (
41-
ErrReferencedPipelineValidationFailed = errors.New("validation failed for referenced Pipeline")
42-
ErrCouldntValidatePipelineRetryable = errors.New("retryable error validating referenced Pipeline")
43-
ErrCouldntValidatePipelinePermanent = errors.New("permanent error validating referenced Pipeline")
44-
)
45-
4639
// GetPipelineFunc is a factory function that will use the given PipelineRef to return a valid GetPipeline function that
4740
// looks up the pipeline. It uses as context a k8s client, tekton client, namespace, and service account name to return
4841
// the pipeline. It knows whether it needs to look in the cluster or in a remote location to fetch the reference.
@@ -150,11 +143,8 @@ func readRuntimeObjectAsPipeline(ctx context.Context, namespace string, obj runt
150143
// Validation must happen before the v1beta1 Pipeline is converted into the storage version of the API,
151144
// since validation of beta features differs between v1 and v1beta1
152145
// TODO(#6592): Decouple API versioning from feature versioning
153-
dryRunObj := obj.DeepCopy()
154-
dryRunObj.Name = uuid.NewString() // Use a randomized name for the Pipeline in case there is already another Pipeline of the same name
155-
dryRunObj.Namespace = namespace // Make sure the namespace is the same as the PipelineRun
156-
if _, err := tekton.TektonV1beta1().Pipelines(namespace).Create(ctx, dryRunObj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err != nil {
157-
return nil, nil, handleDryRunCreateErr(err, obj.Name)
146+
if err := apiserver.DryRunValidate(ctx, namespace, obj, tekton); err != nil {
147+
return nil, nil, err
158148
}
159149
p := &v1.Pipeline{
160150
TypeMeta: metav1.TypeMeta{
@@ -170,30 +160,10 @@ func readRuntimeObjectAsPipeline(ctx context.Context, namespace string, obj runt
170160
vr := trustedresources.VerifyResource(ctx, obj, k8s, refSource, verificationPolicies)
171161
// Issue a dry-run request to create the remote Pipeline, so that it can undergo validation from validating admission webhooks
172162
// without actually creating the Pipeline on the cluster
173-
dryRunObj := obj.DeepCopy()
174-
dryRunObj.Name = uuid.NewString() // Use a randomized name for the Pipeline in case there is already another Pipeline of the same name
175-
dryRunObj.Namespace = namespace // Make sure the namespace is the same as the PipelineRun
176-
if _, err := tekton.TektonV1().Pipelines(namespace).Create(ctx, dryRunObj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err != nil {
177-
return nil, nil, handleDryRunCreateErr(err, obj.Name)
163+
if err := apiserver.DryRunValidate(ctx, namespace, obj, tekton); err != nil {
164+
return nil, nil, err
178165
}
179166
return obj, &vr, nil
180167
}
181168
return nil, nil, errors.New("resource is not a pipeline")
182169
}
183-
184-
func handleDryRunCreateErr(err error, objectName string) error {
185-
var errType error
186-
switch {
187-
case apierrors.IsBadRequest(err): // Pipeline rejected by validating webhook
188-
errType = ErrReferencedPipelineValidationFailed
189-
case apierrors.IsInvalid(err), apierrors.IsMethodNotSupported(err):
190-
errType = ErrCouldntValidatePipelinePermanent
191-
case apierrors.IsTimeout(err), apierrors.IsServerTimeout(err), apierrors.IsTooManyRequests(err):
192-
errType = ErrCouldntValidatePipelineRetryable
193-
default:
194-
// Assume unknown errors are retryable
195-
// Additional errors can be added to the switch statements as needed
196-
errType = ErrCouldntValidatePipelineRetryable
197-
}
198-
return fmt.Errorf("%w %s: %s", errType, objectName, err.Error())
199-
}

‎pkg/reconciler/pipelinerun/resources/pipelineref_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
3737
"github.com/tektoncd/pipeline/pkg/client/clientset/versioned/fake"
3838
clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned/fake"
39+
"github.com/tektoncd/pipeline/pkg/reconciler/apiserver"
3940
"github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/resources"
4041
ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing"
4142
"github.com/tektoncd/pipeline/pkg/trustedresources"
@@ -397,7 +398,7 @@ func TestGetPipelineFunc_RemoteResolution_ValidationFailure(t *testing.T) {
397398
})
398399

399400
resolvedPipeline, resolvedRefSource, _, err := fn(ctx, pipelineRef.Name)
400-
if !errors.Is(err, resources.ErrReferencedPipelineValidationFailed) {
401+
if !errors.Is(err, apiserver.ErrReferencedObjectValidationFailed) {
401402
t.Errorf("expected RemotePipelineValidationFailed error but got none")
402403
}
403404
if resolvedPipeline != nil {

0 commit comments

Comments
 (0)