From f18e823f47ae150ad57d7dd18584f711758efc19 Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Tue, 1 Nov 2022 21:19:23 +0000 Subject: [PATCH] tests --- .../pipelinerun/pipelinerun_test.go | 151 +++++++++---- .../pipelinerun/resources/pipelineref_test.go | 210 +++++++++++++----- .../taskrun/resources/taskref_test.go | 177 ++++++++++----- pkg/reconciler/taskrun/taskrun_test.go | 30 +-- 4 files changed, 390 insertions(+), 178 deletions(-) diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 11df10b9abf..72ceff7a2f4 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -10733,7 +10733,90 @@ spec: } } -func TestReconcile_verifyResolvedPipeline(t *testing.T) { +func TestReconcile_verifyResolvedPipeline_Success(t *testing.T) { + names.TestingSeed() + ctx := context.Background() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + + prs := parse.MustParsePipelineRun(t, ` +metadata: + name: test-pipelinerun + namespace: foo + selfLink: /pipeline/1234 +spec: + pipelineRef: + name: test-pipeline +`) + ps := parse.MustParsePipeline(t, ` +metadata: + name: test-pipeline + namespace: foo +spec: + tasks: + - name: test-1 + taskRef: + name: test-task +`) + ts := parse.MustParseTask(t, ` +metadata: + name: test-task + namespace: foo +spec: + steps: + - name: simple-step + image: foo + command: ["/mycmd"] + env: + - name: foo + value: bar +`) + + signer, secretpath, err := test.GetSignerFromFile(ctx, t) + if err != nil { + t.Fatal(err) + } + signedTask, err := test.GetSignedTask(ts, signer, "test-task") + if err != nil { + t.Fatal("fail to sign task", err) + } + signedPipeline, err := test.GetSignedPipeline(ps, signer, "test-pipeline") + if err != nil { + t.Fatal("fail to sign pipeline", err) + } + + cms := []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{ + "resource-verification-mode": "enforce", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetTrustedResourcesConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{ + config.PublicKeys: secretpath, + }, + }, + } + t.Logf("config maps: %s", cms) + + d := test.Data{ + PipelineRuns: []*v1beta1.PipelineRun{prs}, + Pipelines: []*v1beta1.Pipeline{signedPipeline}, + Tasks: []*v1beta1.Task{signedTask}, + ConfigMaps: cms, + } + prt := newPipelineRunTest(d, t) + defer prt.Cancel() + + reconciledRun, _ := prt.reconcileRun("foo", "test-pipelinerun", []string{}, false) + + checkPipelineRunConditionStatusAndReason(t, reconciledRun, corev1.ConditionUnknown, v1beta1.PipelineRunReasonRunning.String()) + +} + +func TestReconcile_verifyResolvedPipeline_Error(t *testing.T) { names.TestingSeed() ctx := context.Background() ctx, cancel := context.WithCancel(ctx) @@ -10814,57 +10897,33 @@ spec: t.Logf("config maps: %s", cms) testCases := []struct { - name string - pipelinerun []*v1beta1.PipelineRun - pipeline []*v1beta1.Pipeline - task []*v1beta1.Task - pernamentErr bool - conditionStatus corev1.ConditionStatus - conditionReason string + name string + pipelinerun []*v1beta1.PipelineRun + pipeline []*v1beta1.Pipeline + task []*v1beta1.Task }{ { - name: "unsigned pipeline fails verification", - pipelinerun: []*v1beta1.PipelineRun{prs}, - pipeline: []*v1beta1.Pipeline{ps}, - pernamentErr: true, - conditionStatus: corev1.ConditionFalse, - conditionReason: ReasonResourceVerificationFailed, - }, - { - name: "signed pipeline passes verification", - pipelinerun: []*v1beta1.PipelineRun{prs}, - pipeline: []*v1beta1.Pipeline{signedPipeline}, - task: []*v1beta1.Task{signedTask}, - pernamentErr: false, - conditionStatus: corev1.ConditionUnknown, - conditionReason: v1beta1.PipelineRunReasonRunning.String(), + name: "unsigned pipeline fails verification", + pipelinerun: []*v1beta1.PipelineRun{prs}, + pipeline: []*v1beta1.Pipeline{ps}, }, { - name: "signed pipeline with unsigned task fails verification", - pipelinerun: []*v1beta1.PipelineRun{prs}, - pipeline: []*v1beta1.Pipeline{signedPipeline}, - task: []*v1beta1.Task{ts}, - pernamentErr: true, - conditionStatus: corev1.ConditionFalse, - conditionReason: ReasonResourceVerificationFailed, + name: "signed pipeline with unsigned task fails verification", + pipelinerun: []*v1beta1.PipelineRun{prs}, + pipeline: []*v1beta1.Pipeline{signedPipeline}, + task: []*v1beta1.Task{ts}, }, { - name: "signed pipeline with modified task fails verification", - pipelinerun: []*v1beta1.PipelineRun{prs}, - pipeline: []*v1beta1.Pipeline{signedPipeline}, - task: []*v1beta1.Task{tamperedTask}, - pernamentErr: true, - conditionStatus: corev1.ConditionFalse, - conditionReason: ReasonResourceVerificationFailed, + name: "signed pipeline with modified task fails verification", + pipelinerun: []*v1beta1.PipelineRun{prs}, + pipeline: []*v1beta1.Pipeline{signedPipeline}, + task: []*v1beta1.Task{tamperedTask}, }, { - name: "modified pipeline with signed task fails verification", - pipelinerun: []*v1beta1.PipelineRun{prs}, - pipeline: []*v1beta1.Pipeline{tamperedPipeline}, - task: []*v1beta1.Task{signedTask}, - pernamentErr: true, - conditionStatus: corev1.ConditionFalse, - conditionReason: ReasonResourceVerificationFailed, + name: "modified pipeline with signed task fails verification", + pipelinerun: []*v1beta1.PipelineRun{prs}, + pipeline: []*v1beta1.Pipeline{tamperedPipeline}, + task: []*v1beta1.Task{signedTask}, }, } @@ -10879,9 +10938,9 @@ spec: prt := newPipelineRunTest(d, t) defer prt.Cancel() - reconciledRun, _ := prt.reconcileRun("foo", "test-pipelinerun", []string{}, tc.pernamentErr) + reconciledRun, _ := prt.reconcileRun("foo", "test-pipelinerun", []string{}, true) - checkPipelineRunConditionStatusAndReason(t, reconciledRun, tc.conditionStatus, tc.conditionReason) + checkPipelineRunConditionStatusAndReason(t, reconciledRun, corev1.ConditionFalse, ReasonResourceVerificationFailed) }) } } diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref_test.go b/pkg/reconciler/pipelinerun/resources/pipelineref_test.go index 6a238c3defa..fa37f35c96b 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref_test.go @@ -421,7 +421,7 @@ func TestGetPipelineFunc_RemoteResolutionInvalidData(t *testing.T) { } } -func TestLocalPipelineRef_TrustedResourceVerification(t *testing.T) { +func TestLocalPipelineRef_TrustedResourceVerification_Success(t *testing.T) { ctx := context.Background() ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -454,17 +454,7 @@ func TestLocalPipelineRef_TrustedResourceVerification(t *testing.T) { ref *v1beta1.PipelineRef resourceVerificationMode string expected runtime.Object - expectedErr error }{ - { - name: "local unsigned pipeline with enforce policy", - ref: &v1beta1.PipelineRef{ - Name: "test-pipeline", - }, - resourceVerificationMode: config.EnforceResourceVerificationMode, - expected: nil, - expectedErr: trustedresources.ErrorResourceVerificationFailed, - }, { name: "local signed pipeline with enforce policy", ref: &v1beta1.PipelineRef{ @@ -472,15 +462,6 @@ func TestLocalPipelineRef_TrustedResourceVerification(t *testing.T) { }, resourceVerificationMode: config.EnforceResourceVerificationMode, expected: signedPipeline, - expectedErr: nil, - }, { - name: "local tampered pipeline with enforce policy", - ref: &v1beta1.PipelineRef{ - Name: "test-signed2", - }, - resourceVerificationMode: config.EnforceResourceVerificationMode, - expected: nil, - expectedErr: trustedresources.ErrorResourceVerificationFailed, }, { name: "local unsigned pipeline with warn policy", ref: &v1beta1.PipelineRef{ @@ -488,7 +469,6 @@ func TestLocalPipelineRef_TrustedResourceVerification(t *testing.T) { }, resourceVerificationMode: config.WarnResourceVerificationMode, expected: unsignedPipeline, - expectedErr: nil, }, { name: "local signed pipeline with warn policy", @@ -497,7 +477,6 @@ func TestLocalPipelineRef_TrustedResourceVerification(t *testing.T) { }, resourceVerificationMode: config.WarnResourceVerificationMode, expected: signedPipeline, - expectedErr: nil, }, { name: "local tampered pipeline with warn policy", ref: &v1beta1.PipelineRef{ @@ -505,7 +484,6 @@ func TestLocalPipelineRef_TrustedResourceVerification(t *testing.T) { }, resourceVerificationMode: config.WarnResourceVerificationMode, expected: tamperedPipeline, - expectedErr: nil, }, { name: "local unsigned pipeline with skip policy", ref: &v1beta1.PipelineRef{ @@ -513,7 +491,6 @@ func TestLocalPipelineRef_TrustedResourceVerification(t *testing.T) { }, resourceVerificationMode: config.SkipResourceVerificationMode, expected: unsignedPipeline, - expectedErr: nil, }, { name: "local signed pipeline with skip policy", @@ -522,7 +499,6 @@ func TestLocalPipelineRef_TrustedResourceVerification(t *testing.T) { }, resourceVerificationMode: config.SkipResourceVerificationMode, expected: signedPipeline, - expectedErr: nil, }, { name: "local tampered pipeline with skip policy", ref: &v1beta1.PipelineRef{ @@ -530,7 +506,6 @@ func TestLocalPipelineRef_TrustedResourceVerification(t *testing.T) { }, resourceVerificationMode: config.SkipResourceVerificationMode, expected: tamperedPipeline, - expectedErr: nil, }, } @@ -543,9 +518,7 @@ func TestLocalPipelineRef_TrustedResourceVerification(t *testing.T) { } pipeline, err := lc.GetPipeline(ctx, tc.ref.Name) - if tc.expectedErr != nil && (err == nil || !errors.Is(err, tc.expectedErr)) { - t.Fatalf("Expected error %v but found %v instead", tc.expectedErr, err) - } else if tc.expectedErr == nil && err != nil { + if err != nil { t.Fatalf("Received unexpected error ( %#v )", err) } if d := cmp.Diff(pipeline, tc.expected); d != "" { @@ -555,7 +528,81 @@ func TestLocalPipelineRef_TrustedResourceVerification(t *testing.T) { } } -func TestGetPipelineFunc_RemoteResolution_TrustedResourceVerification(t *testing.T) { +func TestLocalPipelineRef_TrustedResourceVerification_Error(t *testing.T) { + ctx := context.Background() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + signer, secretpath, err := test.GetSignerFromFile(ctx, t) + if err != nil { + t.Fatal(err) + } + + unsignedPipeline := test.GetUnsignedPipeline("test-pipeline") + signedPipeline, err := test.GetSignedPipeline(unsignedPipeline, signer, "test-signed") + if err != nil { + t.Fatal("fail to sign pipeline", err) + } + + // attack another signed pipeline + signedPipeline2, err := test.GetSignedPipeline(test.GetUnsignedPipeline("test-pipeline2"), signer, "test-signed2") + if err != nil { + t.Fatal("fail to sign task", err) + } + tamperedPipeline := signedPipeline2.DeepCopy() + if tamperedPipeline.Annotations == nil { + tamperedPipeline.Annotations = make(map[string]string) + } + tamperedPipeline.Annotations["random"] = "attack" + + tektonclient := fake.NewSimpleClientset(signedPipeline, unsignedPipeline, tamperedPipeline) + + testcases := []struct { + name string + ref *v1beta1.PipelineRef + resourceVerificationMode string + expected runtime.Object + expectedErr error + }{ + { + name: "local unsigned pipeline with enforce policy", + ref: &v1beta1.PipelineRef{ + Name: "test-pipeline", + }, + resourceVerificationMode: config.EnforceResourceVerificationMode, + expected: nil, + expectedErr: trustedresources.ErrorResourceVerificationFailed, + }, + { + name: "local tampered pipeline with enforce policy", + ref: &v1beta1.PipelineRef{ + Name: "test-signed2", + }, + resourceVerificationMode: config.EnforceResourceVerificationMode, + expected: nil, + expectedErr: trustedresources.ErrorResourceVerificationFailed, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + ctx = test.SetupTrustedResourceConfig(ctx, secretpath, tc.resourceVerificationMode) + lc := &resources.LocalPipelineRefResolver{ + Namespace: "trusted-resources", + Tektonclient: tektonclient, + } + + pipeline, err := lc.GetPipeline(ctx, tc.ref.Name) + if err == nil || !errors.Is(err, tc.expectedErr) { + t.Fatalf("Expected error %v but found %v instead", tc.expectedErr, err) + } + if d := cmp.Diff(pipeline, tc.expected); d != "" { + t.Error(diff.PrintWantGot(d)) + } + }) + } +} + +func TestGetPipelineFunc_RemoteResolution_TrustedResourceVerification_Success(t *testing.T) { ctx := context.Background() signer, secretpath, err := test.GetSignerFromFile(ctx, t) if err != nil { @@ -599,62 +646,42 @@ func TestGetPipelineFunc_RemoteResolution_TrustedResourceVerification(t *testing requester *test.Requester resourceVerificationMode string expected runtime.Object - expectedErr error }{ { - name: "unsigned pipeline with enforce policy", - requester: requesterUnsigned, - resourceVerificationMode: config.EnforceResourceVerificationMode, - expected: nil, - expectedErr: trustedresources.ErrorResourceVerificationFailed, - }, { name: "signed pipeline with enforce policy", requester: requesterSigned, resourceVerificationMode: config.EnforceResourceVerificationMode, expected: signedPipeline, - expectedErr: nil, - }, { - name: "tampered pipeline with enforce policy", - requester: requesterTampered, - resourceVerificationMode: config.EnforceResourceVerificationMode, - expected: nil, - expectedErr: trustedresources.ErrorResourceVerificationFailed, }, { name: "unsigned pipeline with warn policy", requester: requesterUnsigned, resourceVerificationMode: config.WarnResourceVerificationMode, expected: unsignedPipeline, - expectedErr: nil, }, { name: "signed pipeline with warn policy", requester: requesterSigned, resourceVerificationMode: config.WarnResourceVerificationMode, expected: signedPipeline, - expectedErr: nil, }, { name: "tampered pipeline with warn policy", requester: requesterTampered, resourceVerificationMode: config.WarnResourceVerificationMode, expected: tamperedPipeline, - expectedErr: nil, }, { name: "unsigned pipeline with skip policy", requester: requesterUnsigned, resourceVerificationMode: config.SkipResourceVerificationMode, expected: unsignedPipeline, - expectedErr: nil, }, { name: "signed pipeline with skip policy", requester: requesterSigned, resourceVerificationMode: config.SkipResourceVerificationMode, expected: signedPipeline, - expectedErr: nil, }, { name: "tampered pipeline with skip policy", requester: requesterTampered, resourceVerificationMode: config.SkipResourceVerificationMode, expected: tamperedPipeline, - expectedErr: nil, }, } for _, tc := range testcases { @@ -673,13 +700,88 @@ func TestGetPipelineFunc_RemoteResolution_TrustedResourceVerification(t *testing } resolvedPipeline, err := fn(ctx, pipelineRef.Name) - - if tc.expectedErr != nil && (err == nil || !errors.Is(err, tc.expectedErr)) { - t.Fatalf("Expected error %v but found %v instead", tc.expectedErr, err) - } else if tc.expectedErr == nil && err != nil { + if err != nil { t.Fatalf("Received unexpected error ( %#v )", err) } + if d := cmp.Diff(tc.expected, resolvedPipeline); d != "" { + t.Error(d) + } + }) + } +} + +func TestGetPipelineFunc_RemoteResolution_TrustedResourceVerification_Error(t *testing.T) { + ctx := context.Background() + signer, secretpath, err := test.GetSignerFromFile(ctx, t) + if err != nil { + t.Fatal(err) + } + + unsignedPipeline := test.GetUnsignedPipeline("test-pipeline") + unsignedPipelineBytes, err := json.Marshal(unsignedPipeline) + if err != nil { + t.Fatal("fail to marshal pipeline", err) + } + + resolvedUnsigned := test.NewResolvedResource(unsignedPipelineBytes, nil, nil) + requesterUnsigned := test.NewRequester(resolvedUnsigned, nil) + + signedPipeline, err := test.GetSignedPipeline(unsignedPipeline, signer, "signed") + if err != nil { + t.Fatal("fail to sign pipeline", err) + } + + tamperedPipeline := signedPipeline.DeepCopy() + tamperedPipeline.Annotations["random"] = "attack" + tamperedPipelineBytes, err := json.Marshal(tamperedPipeline) + if err != nil { + t.Fatal("fail to marshal pipeline", err) + } + resolvedTampered := test.NewResolvedResource(tamperedPipelineBytes, nil, nil) + requesterTampered := test.NewRequester(resolvedTampered, nil) + + pipelineRef := &v1beta1.PipelineRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}} + + testcases := []struct { + name string + requester *test.Requester + resourceVerificationMode string + expected runtime.Object + expectedErr error + }{ + { + name: "unsigned pipeline with enforce policy", + requester: requesterUnsigned, + resourceVerificationMode: config.EnforceResourceVerificationMode, + expected: nil, + expectedErr: trustedresources.ErrorResourceVerificationFailed, + }, { + name: "tampered pipeline with enforce policy", + requester: requesterTampered, + resourceVerificationMode: config.EnforceResourceVerificationMode, + expected: nil, + expectedErr: trustedresources.ErrorResourceVerificationFailed, + }, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + ctx = test.SetupTrustedResourceConfig(ctx, secretpath, tc.resourceVerificationMode) + pr := &v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{Namespace: "trusted-resources"}, + Spec: v1beta1.PipelineRunSpec{ + PipelineRef: pipelineRef, + ServiceAccountName: "default", + }, + } + fn, err := resources.GetPipelineFunc(ctx, nil, nil, tc.requester, pr) + if err != nil { + t.Fatalf("failed to get pipeline fn: %s", err.Error()) + } + resolvedPipeline, err := fn(ctx, pipelineRef.Name) + if err == nil || !errors.Is(err, tc.expectedErr) { + t.Fatalf("Expected error %v but found %v instead", tc.expectedErr, err) + } if d := cmp.Diff(tc.expected, resolvedPipeline); d != "" { t.Error(d) } diff --git a/pkg/reconciler/taskrun/resources/taskref_test.go b/pkg/reconciler/taskrun/resources/taskref_test.go index 5239f6c2f9b..2d17dd13f79 100644 --- a/pkg/reconciler/taskrun/resources/taskref_test.go +++ b/pkg/reconciler/taskrun/resources/taskref_test.go @@ -846,7 +846,7 @@ func TestLocalTaskRef_TrustedResourceVerification_Error(t *testing.T) { } } -func TestGetTaskFunc_RemoteResolution_TrustedResourceVerification(t *testing.T) { +func TestGetTaskFunc_RemoteResolution_TrustedResourceVerification_Success(t *testing.T) { ctx := context.Background() signer, secretpath, err := test.GetSignerFromFile(ctx, t) if err != nil { @@ -885,68 +885,127 @@ func TestGetTaskFunc_RemoteResolution_TrustedResourceVerification(t *testing.T) taskRef := &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}} + testcases := []struct { + name string + requester *test.Requester + resourceVerificationMode string + expected runtime.Object + }{{ + name: "signed task with enforce policy", + requester: requesterSigned, + resourceVerificationMode: config.EnforceResourceVerificationMode, + expected: signedTask, + }, { + name: "unsigned task with warn policy", + requester: requesterUnsigned, + resourceVerificationMode: config.WarnResourceVerificationMode, + expected: unsignedTask, + }, { + name: "signed task with warn policy", + requester: requesterSigned, + resourceVerificationMode: config.WarnResourceVerificationMode, + expected: signedTask, + }, { + name: "tampered task with warn policy", + requester: requesterTampered, + resourceVerificationMode: config.SkipResourceVerificationMode, + expected: tamperedTask, + }, { + name: "unsigned task with skip policy", + requester: requesterUnsigned, + resourceVerificationMode: config.SkipResourceVerificationMode, + expected: unsignedTask, + }, { + name: "signed task with skip policy", + requester: requesterSigned, + resourceVerificationMode: config.SkipResourceVerificationMode, + expected: signedTask, + }, { + name: "tampered task with skip policy", + requester: requesterTampered, + resourceVerificationMode: config.WarnResourceVerificationMode, + expected: tamperedTask, + }, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + ctx = test.SetupTrustedResourceConfig(ctx, secretpath, tc.resourceVerificationMode) + tr := &v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{Namespace: "trusted-resources"}, + Spec: v1beta1.TaskRunSpec{ + TaskRef: taskRef, + ServiceAccountName: "default", + }, + } + fn, err := resources.GetTaskFunc(ctx, nil, nil, tc.requester, tr, tr.Spec.TaskRef, "", "default", "default") + if err != nil { + t.Fatalf("failed to get task fn: %s", err.Error()) + } + + resolvedTask, err := fn(ctx, taskRef.Name) + + if err != nil { + t.Fatalf("Received unexpected error ( %#v )", err) + } + + if d := cmp.Diff(tc.expected, resolvedTask); d != "" { + t.Error(d) + } + }) + } +} + +func TestGetTaskFunc_RemoteResolution_TrustedResourceVerification_Error(t *testing.T) { + ctx := context.Background() + signer, secretpath, err := test.GetSignerFromFile(ctx, t) + if err != nil { + t.Fatal(err) + } + + unsignedTask := test.GetUnsignedTask("test-task") + unsignedTaskBytes, err := json.Marshal(unsignedTask) + if err != nil { + t.Fatal("fail to marshal task", err) + } + + resolvedUnsigned := test.NewResolvedResource(unsignedTaskBytes, nil, nil) + requesterUnsigned := test.NewRequester(resolvedUnsigned, nil) + + signedTask, err := test.GetSignedTask(unsignedTask, signer, "signed") + if err != nil { + t.Fatal("fail to sign task", err) + } + + tamperedTask := signedTask.DeepCopy() + tamperedTask.Annotations["random"] = "attack" + tamperedTaskBytes, err := json.Marshal(tamperedTask) + if err != nil { + t.Fatal("fail to marshal task", err) + } + resolvedTampered := test.NewResolvedResource(tamperedTaskBytes, nil, nil) + requesterTampered := test.NewRequester(resolvedTampered, nil) + + taskRef := &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}} + testcases := []struct { name string requester *test.Requester resourceVerificationMode string expected runtime.Object expectedErr error - }{ - { - name: "unsigned task with enforce policy", - requester: requesterUnsigned, - resourceVerificationMode: config.EnforceResourceVerificationMode, - expected: nil, - expectedErr: trustedresources.ErrorResourceVerificationFailed, - }, { - name: "signed task with enforce policy", - requester: requesterSigned, - resourceVerificationMode: config.EnforceResourceVerificationMode, - expected: signedTask, - expectedErr: nil, - }, { - name: "tampered task with enforce policy", - requester: requesterTampered, - resourceVerificationMode: config.EnforceResourceVerificationMode, - expected: nil, - expectedErr: trustedresources.ErrorResourceVerificationFailed, - }, { - name: "unsigned task with warn policy", - requester: requesterUnsigned, - resourceVerificationMode: config.WarnResourceVerificationMode, - expected: unsignedTask, - expectedErr: nil, - }, { - name: "signed task with warn policy", - requester: requesterSigned, - resourceVerificationMode: config.WarnResourceVerificationMode, - expected: signedTask, - expectedErr: nil, - }, { - name: "tampered task with warn policy", - requester: requesterTampered, - resourceVerificationMode: config.SkipResourceVerificationMode, - expected: tamperedTask, - expectedErr: nil, - }, { - name: "unsigned task with skip policy", - requester: requesterUnsigned, - resourceVerificationMode: config.SkipResourceVerificationMode, - expected: unsignedTask, - expectedErr: nil, - }, { - name: "signed task with skip policy", - requester: requesterSigned, - resourceVerificationMode: config.SkipResourceVerificationMode, - expected: signedTask, - expectedErr: nil, - }, { - name: "tampered task with skip policy", - requester: requesterTampered, - resourceVerificationMode: config.WarnResourceVerificationMode, - expected: tamperedTask, - expectedErr: nil, - }, + }{{ + name: "unsigned task with enforce policy", + requester: requesterUnsigned, + resourceVerificationMode: config.EnforceResourceVerificationMode, + expected: nil, + expectedErr: trustedresources.ErrorResourceVerificationFailed, + }, { + name: "tampered task with enforce policy", + requester: requesterTampered, + resourceVerificationMode: config.EnforceResourceVerificationMode, + expected: nil, + expectedErr: trustedresources.ErrorResourceVerificationFailed, + }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { @@ -965,10 +1024,8 @@ func TestGetTaskFunc_RemoteResolution_TrustedResourceVerification(t *testing.T) resolvedTask, err := fn(ctx, taskRef.Name) - if tc.expectedErr != nil && (err == nil || !errors.Is(err, tc.expectedErr)) { + if err == nil || !errors.Is(err, tc.expectedErr) { t.Fatalf("Expected error %v but found %v instead", tc.expectedErr, err) - } else if tc.expectedErr == nil && err != nil { - t.Fatalf("Received unexpected error ( %#v )", err) } if d := cmp.Diff(tc.expected, resolvedTask); d != "" { diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index f80132950ac..eba6ab473fe 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -5120,26 +5120,20 @@ status: } testCases := []struct { - name string - taskrun []*v1beta1.TaskRun - task []*v1beta1.Task - expectedError error - conditionStatus corev1.ConditionStatus - conditionReason string + name string + taskrun []*v1beta1.TaskRun + task []*v1beta1.Task + expectedError error }{ { - name: "unsigned task fails verification", - task: []*v1beta1.Task{ts}, - expectedError: fmt.Errorf("1 error occurred:\n\t* error when listing tasks for taskRun test-taskrun: resource verification failed"), - conditionStatus: corev1.ConditionFalse, - conditionReason: podconvert.ReasonResourceVerificationFailed, + name: "unsigned task fails verification", + task: []*v1beta1.Task{ts}, + expectedError: fmt.Errorf("1 error occurred:\n\t* error when listing tasks for taskRun test-taskrun: resource verification failed"), }, { - name: "modified task fails verification", - task: []*v1beta1.Task{tamperedTask}, - expectedError: fmt.Errorf("1 error occurred:\n\t* error when listing tasks for taskRun test-taskrun: resource verification failed"), - conditionStatus: corev1.ConditionFalse, - conditionReason: podconvert.ReasonResourceVerificationFailed, + name: "modified task fails verification", + task: []*v1beta1.Task{tamperedTask}, + expectedError: fmt.Errorf("1 error occurred:\n\t* error when listing tasks for taskRun test-taskrun: resource verification failed"), }, } @@ -5164,8 +5158,8 @@ status: t.Fatalf("getting updated taskrun: %v", err) } condition := tr.Status.GetCondition(apis.ConditionSucceeded) - if condition.Type != apis.ConditionSucceeded || condition.Status != corev1.ConditionFalse || condition.Reason != tc.conditionReason { - t.Errorf("Expected TaskRun to fail with reason \"%s\" but it did not. Final conditions were:\n%#v", tc.conditionReason, tr.Status.Conditions) + if condition.Type != apis.ConditionSucceeded || condition.Status != corev1.ConditionFalse || condition.Reason != podconvert.ReasonResourceVerificationFailed { + t.Errorf("Expected TaskRun to fail with reason \"%s\" but it did not. Final conditions were:\n%#v", podconvert.ReasonResourceVerificationFailed, tr.Status.Conditions) } })