From 809925323587d44a43bc3e2502702247cbd5a6d7 Mon Sep 17 00:00:00 2001
From: Chmouel Boudjnah <chmouel@redhat.com>
Date: Wed, 26 Jun 2024 11:01:48 +0200
Subject: [PATCH] [PATCH] Cleanup resolved object before validating through
 dry-run

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).
---
 .../pipelinerun/resources/pipelineref.go         |  6 ++++++
 pkg/reconciler/taskrun/resources/taskref.go      | 16 +++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref.go b/pkg/reconciler/pipelinerun/resources/pipelineref.go
index 71fd76669..c6aab4a51 100644
--- a/pkg/reconciler/pipelinerun/resources/pipelineref.go
+++ b/pkg/reconciler/pipelinerun/resources/pipelineref.go
@@ -137,6 +137,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
@@ -155,6 +158,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)
diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go
index 7bddf0b05..ccde6a41e 100644
--- a/pkg/reconciler/taskrun/resources/taskref.go
+++ b/pkg/reconciler/taskrun/resources/taskref.go
@@ -84,7 +84,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
@@ -213,6 +214,10 @@ func resolveStepAction(ctx context.Context, resolver remote.Resolver, name, name
 	}
 	switch obj := obj.(type) { //nolint:gocritic
 	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
 		}
@@ -234,6 +239,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
@@ -253,6 +261,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
@@ -264,6 +275,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
-- 
2.45.2

