Skip to content

Commit 0be0711

Browse files
kasemAlemkasemAlem
committed
fix(STONEINTG-1309): fix integration plr can be nill
Signed-off-by: Kasem Alem <[email protected]>
1 parent 61b852e commit 0be0711

File tree

2 files changed

+108
-75
lines changed

2 files changed

+108
-75
lines changed

‎tekton/integration_pipeline.go‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -209,27 +209,23 @@ func setGenerateNameForPipelineRun(pipelineRun *tektonv1.PipelineRun, defaultPre
209209
// Updates git resolver values parameters with values of params specified in the input map
210210
// updates only exsitings parameters, doens't create new ones
211211
func (iplr *IntegrationPipelineRun) WithUpdatedTestsGitResolver(params map[string]string) *IntegrationPipelineRun {
212-
// Add nil checks to prevent panic
213-
if iplr.Spec.PipelineRef == nil {
214-
return iplr
215-
}
216-
217-
if iplr.Spec.PipelineRef.ResolverRef == nil {
212+
213+
if iplr == nil {
218214
return iplr
219215
}
220-
221-
//nolint:staticcheck // Ignore QF1008
222-
if iplr.Spec.PipelineRef.ResolverRef.Resolver != consts.TektonResolverGit {
223-
// if the resolver is not git-resolver, we cannot update the git ref
216+
217+
// Defensive nil checks for the entire path
218+
if iplr.Spec.PipelineRef == nil {
224219
return iplr
225220
}
226221

227-
// Add nil check for Params
228-
if iplr.Spec.PipelineRef.ResolverRef.Params == nil {
222+
// Params is a slice, so check if it's empty (len handles nil slices)
223+
//nolint:staticcheck // QF1008: We specifically want ResolverRef.Params, not PipelineRef.Params
224+
if len(iplr.Spec.PipelineRef.ResolverRef.Params) == 0 {
229225
return iplr
230226
}
231227

232-
//nolint:staticcheck // Ignore QF1008
228+
//nolint:staticcheck // QF1008: We specifically want ResolverRef.Params, not PipelineRef.Params
233229
for originalParamIndex, originalParam := range iplr.Spec.PipelineRef.ResolverRef.Params {
234230
if _, ok := params[originalParam.Name]; ok {
235231
// remeber to use the original index to update the value, we cannot update value given by range directly

‎tekton/integration_pipeline_test.go‎

Lines changed: 99 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ limitations under the License.
1717
package tekton_test
1818

1919
import (
20-
2120
"bytes"
2221
"fmt"
2322
"os"
@@ -589,12 +588,19 @@ var _ = Describe("Integration pipeline", Ordered, func() {
589588
})
590589
})
591590

592-
Context("When WithUpdatedTestsGitResolver is called with nil pipeline reference", func() {
593-
It("should not panic when PipelineRef is nil", func() {
591+
Context("When testing WithUpdatedTestsGitResolver with edge cases", func() {
592+
It("should handle pipeline run created from base64 with missing resolver ref", func() {
593+
// This simulates a pipeline run created via generateIntegrationPipelineRunFromBase64
594+
// where the structure might not be fully initialized
594595
pipelineRun := &tekton.IntegrationPipelineRun{
595596
tektonv1.PipelineRun{
596597
Spec: tektonv1.PipelineRunSpec{
597-
PipelineRef: nil, // This is the problematic case
598+
PipelineRef: &tektonv1.PipelineRef{
599+
Name: "some-pipeline", // Has name but no resolver
600+
ResolverRef: tektonv1.ResolverRef{
601+
// Empty resolver ref - this was causing the panic
602+
},
603+
},
598604
},
599605
},
600606
}
@@ -604,18 +610,21 @@ var _ = Describe("Integration pipeline", Ordered, func() {
604610
"revision": "main",
605611
}
606612

607-
// This should not panic
613+
// This should not panic - it should return early because resolver is empty
608614
result := pipelineRun.WithUpdatedTestsGitResolver(params)
609615
Expect(result).NotTo(BeNil())
610-
Expect(result.Spec.PipelineRef).To(BeNil())
616+
Expect(result).To(Equal(pipelineRun)) // Should return the same instance unchanged
611617
})
612618

613-
It("should not panic when ResolverRef is nil", func() {
619+
It("should handle pipeline run with git resolver but empty params", func() {
614620
pipelineRun := &tekton.IntegrationPipelineRun{
615621
tektonv1.PipelineRun{
616622
Spec: tektonv1.PipelineRunSpec{
617623
PipelineRef: &tektonv1.PipelineRef{
618-
ResolverRef: tektonv1.ResolverRef{}, // Empty, params will be nil
624+
ResolverRef: tektonv1.ResolverRef{
625+
Resolver: tektonv1.ResolverName(tektonconsts.TektonResolverGit),
626+
Params: nil, // This was causing the panic
627+
},
619628
},
620629
},
621630
},
@@ -626,19 +635,20 @@ var _ = Describe("Integration pipeline", Ordered, func() {
626635
"revision": "main",
627636
}
628637

629-
// This should not panic
638+
// This should not panic - should return early because params is nil
630639
result := pipelineRun.WithUpdatedTestsGitResolver(params)
631640
Expect(result).NotTo(BeNil())
641+
Expect(result.Spec.PipelineRef.ResolverRef.Params).To(BeNil())
632642
})
633643

634-
It("should not panic when Params is nil", func() {
644+
It("should handle pipeline run with git resolver but empty params slice", func() {
635645
pipelineRun := &tekton.IntegrationPipelineRun{
636646
tektonv1.PipelineRun{
637647
Spec: tektonv1.PipelineRunSpec{
638648
PipelineRef: &tektonv1.PipelineRef{
639649
ResolverRef: tektonv1.ResolverRef{
640650
Resolver: tektonv1.ResolverName(tektonconsts.TektonResolverGit),
641-
Params: nil, // This is the problematic case
651+
Params: []tektonv1.Param{}, // Empty slice
642652
},
643653
},
644654
},
@@ -650,20 +660,28 @@ var _ = Describe("Integration pipeline", Ordered, func() {
650660
"revision": "main",
651661
}
652662

653-
// This should not panic
663+
// This should not panic - should return early because params is empty
654664
result := pipelineRun.WithUpdatedTestsGitResolver(params)
655665
Expect(result).NotTo(BeNil())
656-
Expect(result.Spec.PipelineRef.ResolverRef.Params).To(BeNil())
666+
Expect(result.Spec.PipelineRef.ResolverRef.Params).To(BeEmpty())
657667
})
658668

659-
It("should not panic when Resolver is not git", func() {
669+
It("should handle pipeline run with non-git resolver (bundles)", func() {
660670
pipelineRun := &tekton.IntegrationPipelineRun{
661671
tektonv1.PipelineRun{
662672
Spec: tektonv1.PipelineRunSpec{
663673
PipelineRef: &tektonv1.PipelineRef{
664674
ResolverRef: tektonv1.ResolverRef{
665-
Resolver: tektonv1.ResolverName("bundles"), // Not git resolver
666-
Params: nil,
675+
Resolver: tektonv1.ResolverName("bundles"), // Not git
676+
Params: []tektonv1.Param{
677+
{
678+
Name: "bundle",
679+
Value: tektonv1.ParamValue{
680+
Type: tektonv1.ParamTypeString,
681+
StringVal: "quay.io/test/bundle:latest",
682+
},
683+
},
684+
},
667685
},
668686
},
669687
},
@@ -675,68 +693,87 @@ var _ = Describe("Integration pipeline", Ordered, func() {
675693
"revision": "main",
676694
}
677695

678-
// This should not panic and should return early
696+
// Should not modify non-git resolvers
679697
result := pipelineRun.WithUpdatedTestsGitResolver(params)
680698
Expect(result).NotTo(BeNil())
699+
Expect(result.Spec.PipelineRef.ResolverRef.Params[0].Value.StringVal).To(Equal("quay.io/test/bundle:latest"))
681700
})
682-
})
683701

684-
Context("When snapshot has run=all label with different resolver types", func() {
685-
var (
686-
hasSnapshot *applicationapiv1alpha1.Snapshot
687-
adapter *tekton.Adapter
688-
)
689-
690-
BeforeEach(func() {
691-
hasSnapshot = &applicationapiv1alpha1.Snapshot{
692-
ObjectMeta: metav1.ObjectMeta{
693-
Name: "test-snapshot",
694-
Namespace: "default",
695-
Labels: map[string]string{
696-
gitops.SnapshotIntegrationTestRun: "all", // The problematic label
697-
},
698-
},
699-
Spec: applicationapiv1alpha1.SnapshotSpec{
700-
Application: "test-app",
701-
Components: []applicationapiv1alpha1.SnapshotComponent{
702-
{
703-
Name: "test-component",
704-
ContainerImage: "test-image",
702+
It("should successfully update git resolver parameters", func() {
703+
pipelineRun := &tekton.IntegrationPipelineRun{
704+
tektonv1.PipelineRun{
705+
Spec: tektonv1.PipelineRunSpec{
706+
PipelineRef: &tektonv1.PipelineRef{
707+
ResolverRef: tektonv1.ResolverRef{
708+
Resolver: tektonv1.ResolverName(tektonconsts.TektonResolverGit),
709+
Params: []tektonv1.Param{
710+
{
711+
Name: "url",
712+
Value: tektonv1.ParamValue{
713+
Type: tektonv1.ParamTypeString,
714+
StringVal: "https://github.com/old/repo.git",
715+
},
716+
},
717+
{
718+
Name: "revision",
719+
Value: tektonv1.ParamValue{
720+
Type: tektonv1.ParamTypeString,
721+
StringVal: "old-branch",
722+
},
723+
},
724+
},
725+
},
705726
},
706727
},
707728
},
708729
}
730+
731+
params := map[string]string{
732+
"url": "https://github.com/new/repo.git",
733+
"revision": "main",
734+
}
735+
736+
result := pipelineRun.WithUpdatedTestsGitResolver(params)
737+
Expect(result).NotTo(BeNil())
738+
Expect(result.Spec.PipelineRef.ResolverRef.Params[0].Value.StringVal).To(Equal("https://github.com/new/repo.git"))
739+
Expect(result.Spec.PipelineRef.ResolverRef.Params[1].Value.StringVal).To(Equal("main"))
709740
})
710741

711-
It("should not crash when integration test scenario has bundle resolver", func() {
712-
bundleScenario := &v1beta2.IntegrationTestScenario{
713-
ObjectMeta: metav1.ObjectMeta{
714-
Name: "bundle-test",
715-
Namespace: "default",
716-
},
717-
Spec: v1beta2.IntegrationTestScenarioSpec{
718-
Application: "test-app",
719-
ResolverRef: v1beta2.ResolverRef{
720-
ResourceKind: tektonconsts.ResourceKindPipelineRun,
721-
Resolver: "bundles",
722-
Params: []v1beta2.ResolverParameter{
723-
{
724-
Name: "bundle",
725-
Value: "quay.io/test/bundle:latest",
742+
It("should handle nil params map gracefully", func() {
743+
// This simulates a pipeline run created via generateIntegrationPipelineRunFromBase64
744+
// where the structure might not be fully initialized
745+
pipelineRun := &tekton.IntegrationPipelineRun{
746+
tektonv1.PipelineRun{
747+
Spec: tektonv1.PipelineRunSpec{
748+
PipelineRef: &tektonv1.PipelineRef{
749+
Name: "some-pipeline", // Has name but no resolver
750+
ResolverRef: tektonv1.ResolverRef{
751+
Resolver: tektonv1.ResolverName(tektonconsts.TektonResolverGit),
752+
Params: []tektonv1.Param{
753+
{
754+
Name: "url",
755+
Value: tektonv1.ParamValue{
756+
Type: tektonv1.ParamTypeString,
757+
StringVal: "https://github.com/test/repo.git",
758+
},
759+
},
760+
{
761+
Name: "revision",
762+
Value: tektonv1.ParamValue{
763+
Type: tektonv1.ParamTypeString,
764+
StringVal: "main",
765+
},
766+
},
767+
},
726768
},
727769
},
728770
},
729771
},
730772
}
731773

732-
Expect(k8sClient.Create(ctx, bundleScenario)).Should(Succeed())
733-
734-
adapter = tekton.NewAdapter(ctx, hasSnapshot, hasApp, helpers.IntegrationLogger{}, mockLoader, k8sClient)
735-
736-
// This should not panic
737-
Expect(func() {
738-
_ = adapter.EnsureIntegrationPipelineRunsExist()
739-
}).NotTo(Panic())
774+
result := pipelineRun.WithUpdatedTestsGitResolver(nil)
775+
Expect(result).NotTo(BeNil())
776+
// Should return unchanged
740777
})
741778
})
742779
})

0 commit comments

Comments
 (0)