diff --git a/pkg/reconciler/apiserver/apiserver.go b/pkg/reconciler/apiserver/apiserver.go index 8ed16c20b12..14714791c36 100644 --- a/pkg/reconciler/apiserver/apiserver.go +++ b/pkg/reconciler/apiserver/apiserver.go @@ -7,6 +7,7 @@ import ( "github.com/google/uuid" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -56,6 +57,13 @@ func DryRunValidate(ctx context.Context, namespace string, obj runtime.Object, t if _, err := tekton.TektonV1beta1().Tasks(namespace).Create(ctx, dryRunObj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err != nil { return handleDryRunCreateErr(err, obj.Name) } + case *v1alpha1.StepAction: + dryRunObj := obj.DeepCopy() + dryRunObj.Name = dryRunObjName + dryRunObj.Namespace = namespace // Make sure the namespace is the same as the TaskRun + if _, err := tekton.TektonV1alpha1().StepActions(namespace).Create(ctx, dryRunObj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err != nil { + return handleDryRunCreateErr(err, obj.Name) + } default: return fmt.Errorf("unsupported object GVK %s", obj.GetObjectKind().GroupVersionKind()) } diff --git a/pkg/reconciler/apiserver/apiserver_test.go b/pkg/reconciler/apiserver/apiserver_test.go index 4a115248ef3..a16d515c9ab 100644 --- a/pkg/reconciler/apiserver/apiserver_test.go +++ b/pkg/reconciler/apiserver/apiserver_test.go @@ -7,6 +7,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/client/clientset/versioned/fake" "github.com/tektoncd/pipeline/pkg/reconciler/apiserver" @@ -34,6 +35,9 @@ func TestDryRunCreate_Valid_DifferentGVKs(t *testing.T) { }, { name: "v1beta1 pipeline", obj: &v1beta1.Pipeline{}, + }, { + name: "v1alpha1 stepaction", + obj: &v1alpha1.StepAction{}, }, { name: "unsupported gvk", obj: &v1beta1.ClusterTask{}, diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go index 5539f2cf62a..18098f97028 100644 --- a/pkg/reconciler/taskrun/resources/taskref.go +++ b/pkg/reconciler/taskrun/resources/taskref.go @@ -169,6 +169,9 @@ func resolveStepAction(ctx context.Context, resolver remote.Resolver, name, name } switch obj := obj.(type) { case *v1alpha1.StepAction: + if err := apiserver.DryRunValidate(ctx, namespace, obj, tekton); err != nil { + return nil, nil, err + } return obj, refSource, nil } return nil, nil, errors.New("resource is not a step action") diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index b2813dd5a82..eb3634233cc 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -2063,6 +2063,95 @@ spec: } } +func TestReconcile_InvalidRemoteStepAction(t *testing.T) { + namespace := "foo" + trName := "test-task-run-success" + trs := []*v1.TaskRun{parse.MustParseV1TaskRun(t, ` +metadata: + name: test-task-run-success + namespace: foo +spec: + taskSpec: + steps: + - ref: + resolver: bar +`)} + + stepAction := parse.MustParseV1alpha1StepAction(t, ` +metadata: + name: stepAction + namespace: foo +spec: + image: myImage + command: ["ls"] +`) + + saBytes, err := yaml.Marshal(stepAction) + if err != nil { + t.Fatal("failed to marshal task", err) + } + taskReq := getResolvedResolutionRequest(t, "bar", saBytes, namespace, trName) + + tcs := []struct { + name string + webhookErr error + wantPermanentErr bool + wantFailed bool + }{{ + name: "webhook validation fails: invalid object", + webhookErr: apierrors.NewBadRequest("bad request"), + wantPermanentErr: true, + wantFailed: true, + }, { + name: "webhook validation fails with permanent error", + webhookErr: apierrors.NewInvalid(schema.GroupKind{Group: "tekton.dev/v1", Kind: "TaskRun"}, "taskrun", field.ErrorList{}), + wantPermanentErr: true, + wantFailed: true, + }, { + name: "webhook validation fails: retryable", + webhookErr: apierrors.NewTimeoutError("timeout", 5), + wantPermanentErr: false, + wantFailed: false, + }} + for _, tc := range tcs { + d := test.Data{ + TaskRuns: trs, + ConfigMaps: []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{ + "enable-step-actions": "true", + }, + }, + }, + ResolutionRequests: []*resolutionv1beta1.ResolutionRequest{&taskReq}, + } + testAssets, cancel := getTaskRunController(t, d) + defer cancel() + c := testAssets.Controller + clients := testAssets.Clients + // Create an error when the Pipeline client attempts to create StepActions + clients.Pipeline.PrependReactor("create", "stepactions", func(action ktesting.Action) (bool, runtime.Object, error) { + return true, nil, tc.webhookErr + }) + err = c.Reconciler.Reconcile(testAssets.Ctx, fmt.Sprintf("%s/%s", namespace, trName)) + if tc.wantPermanentErr != controller.IsPermanentError(err) { + t.Errorf("expected permanent error: %t but got %s", tc.wantPermanentErr, err) + } + reconciledRun, err := clients.Pipeline.TektonV1().TaskRuns(namespace).Get(testAssets.Ctx, trName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Somehow had error getting reconciled run out of fake client: %s", err) + } + + if tc.wantFailed && reconciledRun.Status.GetCondition(apis.ConditionSucceeded).Reason != podconvert.ReasonTaskFailedValidation { + t.Errorf("Expected TaskRun to have reason FailedValidation, but condition reason is %s", reconciledRun.Status.GetCondition(apis.ConditionSucceeded)) + } + if !tc.wantFailed && reconciledRun.Status.GetCondition(apis.ConditionSucceeded).IsFalse() { + t.Errorf("Expected TaskRun to not be failed but has condition status false") + } + } +} + func TestReconcileTaskRunWithPermanentError(t *testing.T) { noTaskRun := parse.MustParseV1TaskRun(t, ` metadata: diff --git a/test/clients.go b/test/clients.go index b3bcec3bd19..58d32469d2a 100644 --- a/test/clients.go +++ b/test/clients.go @@ -64,6 +64,7 @@ type clients struct { V1beta1CustomRunClient v1beta1.CustomRunInterface V1alpha1ResolutionRequestclient resolutionv1alpha1.ResolutionRequestInterface V1alpha1VerificationPolicyClient v1alpha1.VerificationPolicyInterface + v1alpha1StepActionClient v1alpha1.StepActionInterface V1PipelineClient v1.PipelineInterface V1TaskClient v1.TaskInterface V1TaskRunClient v1.TaskRunInterface @@ -105,6 +106,7 @@ func newClients(t *testing.T, configPath, clusterName, namespace string) *client c.V1beta1CustomRunClient = cs.TektonV1beta1().CustomRuns(namespace) c.V1alpha1ResolutionRequestclient = rrcs.ResolutionV1alpha1().ResolutionRequests(namespace) c.V1alpha1VerificationPolicyClient = cs.TektonV1alpha1().VerificationPolicies(namespace) + c.v1alpha1StepActionClient = cs.TektonV1alpha1().StepActions(namespace) c.V1PipelineClient = cs.TektonV1().Pipelines(namespace) c.V1TaskClient = cs.TektonV1().Tasks(namespace) c.V1TaskRunClient = cs.TektonV1().TaskRuns(namespace)