From c776d8b324248635297b37d6c07ff951580bb143 Mon Sep 17 00:00:00 2001 From: encalada Date: Wed, 18 Oct 2023 15:48:52 +0200 Subject: [PATCH] Make the buildrun.spec.build a mandatory field Due to seeing panics reported in issues, when this field for v1beta1 CR's was not set by human error. --- pkg/apis/build/v1beta1/buildrun_conversion.go | 6 +++--- pkg/apis/build/v1beta1/buildrun_types.go | 8 ++++---- pkg/webhook/conversion/converter_test.go | 5 +++-- test/e2e/v1beta1/common_suite_test.go | 2 +- test/e2e/v1beta1/validators_test.go | 2 +- test/v1beta1_samples/catalog.go | 16 ++++++++-------- 6 files changed, 20 insertions(+), 19 deletions(-) diff --git a/pkg/apis/build/v1beta1/buildrun_conversion.go b/pkg/apis/build/v1beta1/buildrun_conversion.go index 54466d4c04..c731da9642 100644 --- a/pkg/apis/build/v1beta1/buildrun_conversion.go +++ b/pkg/apis/build/v1beta1/buildrun_conversion.go @@ -36,9 +36,9 @@ func (src *BuildRun) ConvertTo(ctx context.Context, obj *unstructured.Unstructur return err } alphaBuildRun.Spec.BuildSpec = &newBuildSpec - } else { + } else if src.Spec.Build.Name != nil { alphaBuildRun.Spec.BuildRef = &v1alpha1.BuildRef{ - Name: src.Spec.Build.Name, + Name: *src.Spec.Build.Name, } } @@ -200,7 +200,7 @@ func (dest *BuildRunSpec) ConvertFrom(orig *v1alpha1.BuildRunSpec) error { } } if orig.BuildRef != nil { - dest.Build.Name = orig.BuildRef.Name + dest.Build.Name = &orig.BuildRef.Name } // only interested on spec.sources as long as an item of the list diff --git a/pkg/apis/build/v1beta1/buildrun_types.go b/pkg/apis/build/v1beta1/buildrun_types.go index 087ae7e8d0..87eed134f3 100644 --- a/pkg/apis/build/v1beta1/buildrun_types.go +++ b/pkg/apis/build/v1beta1/buildrun_types.go @@ -29,15 +29,15 @@ type ReferencedBuild struct { // Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names // // +optional - Name string `json:"name,omitempty"` + Name *string `json:"name,omitempty"` } // BuildRunSpec defines the desired state of BuildRun type BuildRunSpec struct { // Build refers to an embedded build specification + // This field is mandatory // - // +optional - Build *ReferencedBuild `json:"build,omitempty"` + Build *ReferencedBuild `json:"build"` // Source refers to the location where the source code is, // this could only be a local source @@ -380,7 +380,7 @@ func (brs *BuildRunStatus) SetCondition(condition *Condition) { // build resource or an embedded build specification func (buildrunSpec *BuildRunSpec) BuildName() string { if buildrunSpec.Build != nil { - return buildrunSpec.Build.Name + return *buildrunSpec.Build.Name } // Only BuildRuns with a ReferencedBuild can actually return a proper Build name diff --git a/pkg/webhook/conversion/converter_test.go b/pkg/webhook/conversion/converter_test.go index 14fc9f47e4..580cbe554d 100644 --- a/pkg/webhook/conversion/converter_test.go +++ b/pkg/webhook/conversion/converter_test.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer/json" + "k8s.io/utils/pointer" "k8s.io/utils/ptr" ) @@ -1078,7 +1079,7 @@ request: }, Spec: v1beta1.BuildRunSpec{ Build: &v1beta1.ReferencedBuild{ - Name: "a_build", + Name: pointer.String("a_build"), }, Source: &v1beta1.BuildRunSource{ Type: v1beta1.LocalType, @@ -1162,7 +1163,7 @@ request: }, Spec: v1beta1.BuildRunSpec{ Build: &v1beta1.ReferencedBuild{ - Name: "a_build", + Name: pointer.String("a_build"), }, ServiceAccount: &sa, Timeout: &v1.Duration{ diff --git a/test/e2e/v1beta1/common_suite_test.go b/test/e2e/v1beta1/common_suite_test.go index 21b5fbc526..870a984ddb 100644 --- a/test/e2e/v1beta1/common_suite_test.go +++ b/test/e2e/v1beta1/common_suite_test.go @@ -283,7 +283,7 @@ func (b *buildRunPrototype) ForBuild(build *buildv1beta1.Build) *buildRunPrototy if b.buildRun.Spec.Build == nil { b.buildRun.Spec.Build = &buildv1beta1.ReferencedBuild{} } - b.buildRun.Spec.Build.Name = build.Name + b.buildRun.Spec.Build.Name = &build.Name b.buildRun.ObjectMeta.Namespace = build.Namespace return b } diff --git a/test/e2e/v1beta1/validators_test.go b/test/e2e/v1beta1/validators_test.go index 34cc8e99d4..46eb6f9a8e 100644 --- a/test/e2e/v1beta1/validators_test.go +++ b/test/e2e/v1beta1/validators_test.go @@ -331,7 +331,7 @@ func buildRunTestData(ns string, identifier string, filePath string) (*buildv1be buildRun.SetNamespace(ns) buildRun.SetName(identifier) - buildRun.Spec.Build.Name = identifier + buildRun.Spec.Build.Name = &identifier serviceAccountName := os.Getenv(EnvVarServiceAccountName) if serviceAccountName == "generated" { diff --git a/test/v1beta1_samples/catalog.go b/test/v1beta1_samples/catalog.go index 675ec595f6..b6c4cc1cb5 100644 --- a/test/v1beta1_samples/catalog.go +++ b/test/v1beta1_samples/catalog.go @@ -785,7 +785,7 @@ func (c *Catalog) DefaultBuildRun(buildRunName string, buildName string) *build. }, Spec: build.BuildRunSpec{ Build: &build.ReferencedBuild{ - Name: buildName, + Name: &buildName, }, }, Status: build.BuildRunStatus{ @@ -830,7 +830,7 @@ func (c *Catalog) BuildRunWithBuildSnapshot(buildRunName string, buildName strin }, Spec: build.BuildRunSpec{ Build: &build.ReferencedBuild{ - Name: buildName, + Name: &buildName, }, }, } @@ -855,7 +855,7 @@ func (c *Catalog) BuildRunWithExistingOwnerReferences(buildRunName string, build }, Spec: build.BuildRunSpec{ Build: &build.ReferencedBuild{ - Name: buildName, + Name: &buildName, }, }, } @@ -871,7 +871,7 @@ func (c *Catalog) BuildRunWithFakeNamespace(buildRunName string, buildName strin }, Spec: build.BuildRunSpec{ Build: &build.ReferencedBuild{ - Name: buildName, + Name: &buildName, }, }, } @@ -960,7 +960,7 @@ func (c *Catalog) BuildRunWithSA(buildRunName string, buildName string, saName s }, Spec: build.BuildRunSpec{ Build: &build.ReferencedBuild{ - Name: buildName, + Name: &buildName, }, ServiceAccount: &saName, }, @@ -976,7 +976,7 @@ func (c *Catalog) BuildRunWithoutSA(buildRunName string, buildName string) *buil }, Spec: build.BuildRunSpec{ Build: &build.ReferencedBuild{ - Name: buildName, + Name: &buildName, }, ServiceAccount: nil, }, @@ -992,7 +992,7 @@ func (c *Catalog) BuildRunWithSAGenerate(buildRunName string, buildName string) }, Spec: build.BuildRunSpec{ Build: &build.ReferencedBuild{ - Name: buildName, + Name: &buildName, }, ServiceAccount: ptr.To[string](".generate"), }, @@ -1054,7 +1054,7 @@ func (c *Catalog) LoadBRWithNameAndRef(name string, buildName string, d []byte) return nil, err } b.Name = name - b.Spec.Build.Name = buildName + b.Spec.Build.Name = &buildName return b, nil }