Skip to content

Commit

Permalink
Cleanup resolved object before validating through dry-run
Browse files Browse the repository at this point in the history
This ensure that we are not going to fail during validation with
dry-run. An example of such a failure would be the following scenario.

- A task in a namespace has `ownerReferences` with
`blockOwnerDeletion: true`
- A user uses the `cluster` resolver to fetch that task
- That user doesn't have a lot of rights in that namespace (only
listing Tasks for example).

/kind bug

Signed-off-by: Vincent Demeester <[email protected]>
  • Loading branch information
vdemeester committed Jun 13, 2024
1 parent 351cb9a commit 69a6b8d
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
6 changes: 6 additions & 0 deletions pkg/reconciler/pipelinerun/resources/pipelineref.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,9 @@ func readRuntimeObjectAsPipeline(ctx context.Context, namespace string, obj runt
switch obj := obj.(type) {
case *v1beta1.Pipeline:
obj.SetDefaults(ctx)
// Cleanup object from things we don't care about
// FIXME: extract this in a function
obj.ObjectMeta.OwnerReferences = nil
// Verify the Pipeline once we fetch from the remote resolution, mutating, validation and conversion of the pipeline should happen after the verification, since signatures are based on the remote pipeline contents
vr := trustedresources.VerifyResource(ctx, obj, k8s, refSource, verificationPolicies)
// Issue a dry-run request to create the remote Pipeline, so that it can undergo validation from validating admission webhooks
Expand All @@ -169,6 +172,9 @@ func readRuntimeObjectAsPipeline(ctx context.Context, namespace string, obj runt
}
return p, &vr, nil
case *v1.Pipeline:
// Cleanup object from things we don't care about
// FIXME: extract this in a function
obj.ObjectMeta.OwnerReferences = nil
// This SetDefaults is currently not necessary, but for consistency, it is recommended to add it.
// Avoid forgetting to add it in the future when there is a v2 version, causing similar problems.
obj.SetDefaults(ctx)
Expand Down
18 changes: 17 additions & 1 deletion pkg/reconciler/taskrun/resources/taskref.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ func GetTaskFuncFromTaskRun(ctx context.Context, k8s kubernetes.Interface, tekto
// a remote image to fetch the reference. It will also return the "kind" of the task being referenced.
// OCI bundle and remote resolution tasks will be verified by trusted resources if the feature is enabled
func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset.Interface, requester remoteresource.Requester,
owner kmeta.OwnerRefable, tr *v1.TaskRef, trName string, namespace, saName string, verificationPolicies []*v1alpha1.VerificationPolicy) GetTask {
owner kmeta.OwnerRefable, tr *v1.TaskRef, trName string, namespace, saName string, verificationPolicies []*v1alpha1.VerificationPolicy,
) GetTask {
kind := v1.NamespacedTaskKind
if tr != nil && tr.Kind != "" {
kind = tr.Kind
Expand Down Expand Up @@ -231,12 +232,18 @@ func resolveStepAction(ctx context.Context, resolver remote.Resolver, name, name
}
switch obj := obj.(type) {
case *v1beta1.StepAction:
// Cleanup object from things we don't care about
// FIXME: extract this in a function
obj.ObjectMeta.OwnerReferences = nil
if err := apiserver.DryRunValidate(ctx, namespace, obj, tekton); err != nil {
return nil, nil, err
}
return obj, refSource, nil
case *v1alpha1.StepAction:
obj.SetDefaults(ctx)
// Cleanup object from things we don't care about
// FIXME: extract this in a function
obj.ObjectMeta.OwnerReferences = nil
if err := apiserver.DryRunValidate(ctx, namespace, obj, tekton); err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -268,6 +275,9 @@ func readRuntimeObjectAsTask(ctx context.Context, namespace string, obj runtime.
switch obj := obj.(type) {
case *v1beta1.Task:
obj.SetDefaults(ctx)
// Cleanup object from things we don't care about
// FIXME: extract this in a function
obj.ObjectMeta.OwnerReferences = nil
// Verify the Task once we fetch from the remote resolution, mutating, validation and conversion of the task should happen after the verification, since signatures are based on the remote task contents
vr := trustedresources.VerifyResource(ctx, obj, k8s, refSource, verificationPolicies)
// Issue a dry-run request to create the remote Task, so that it can undergo validation from validating admission webhooks
Expand All @@ -287,6 +297,9 @@ func readRuntimeObjectAsTask(ctx context.Context, namespace string, obj runtime.
return t, &vr, nil
case *v1beta1.ClusterTask:
obj.SetDefaults(ctx)
// Cleanup object from things we don't care about
// FIXME: extract this in a function
obj.ObjectMeta.OwnerReferences = nil
t, err := convertClusterTaskToTask(ctx, *obj)
// Issue a dry-run request to create the remote Task, so that it can undergo validation from validating admission webhooks
// without actually creating the Task on the cluster
Expand All @@ -298,6 +311,9 @@ func readRuntimeObjectAsTask(ctx context.Context, namespace string, obj runtime.
// This SetDefaults is currently not necessary, but for consistency, it is recommended to add it.
// Avoid forgetting to add it in the future when there is a v2 version, causing similar problems.
obj.SetDefaults(ctx)
// Cleanup object from things we don't care about
// FIXME: extract this in a function
obj.ObjectMeta.OwnerReferences = nil
vr := trustedresources.VerifyResource(ctx, obj, k8s, refSource, verificationPolicies)
// Issue a dry-run request to create the remote Task, so that it can undergo validation from validating admission webhooks
// without actually creating the Task on the cluster
Expand Down

0 comments on commit 69a6b8d

Please sign in to comment.