From 89a7f53a7569ea906142a271188cf3ffb8e06f11 Mon Sep 17 00:00:00 2001 From: Shivam Mukhade Date: Mon, 15 Nov 2021 20:24:09 +0530 Subject: [PATCH] Adds support for specifying image labels and annotation in buildrun This adds support for specifying image labels and annotations in buildrun. While generating the taskrun spec, the labels annotation will merged with the ones specified in build object. Values in buildrun will task precedence over in build. Signed-off-by: Shivam Mukhade --- .../build/v1alpha1/zz_generated.deepcopy.go | 87 +++++++++++++++++++ pkg/reconciler/buildrun/resources/mutate.go | 21 ++++- pkg/reconciler/buildrun/resources/taskrun.go | 14 ++- .../buildrun/resources/taskrun_test.go | 84 ++++++++++++++++++ test/buildrun_samples.go | 22 +++++ 5 files changed, 222 insertions(+), 6 deletions(-) diff --git a/pkg/apis/build/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/build/v1alpha1/zz_generated.deepcopy.go index 3a3c0dcd5d..a66bc797f7 100644 --- a/pkg/apis/build/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/build/v1alpha1/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated // Copyright The Shipwright Contributors @@ -203,6 +204,18 @@ func (in *BuildRunSpec) DeepCopy() *BuildRunSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BuildRunStatus) DeepCopyInto(out *BuildRunStatus) { *out = *in + if in.Sources != nil { + in, out := &in.Sources, &out.Sources + *out = make([]SourceResult, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.Output != nil { + in, out := &in.Output, &out.Output + *out = new(Output) + **out = **in + } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make(Conditions, len(*in)) @@ -477,6 +490,22 @@ func (in *BundleContainer) DeepCopy() *BundleContainer { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BundleSourceResult) DeepCopyInto(out *BundleSourceResult) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BundleSourceResult. +func (in *BundleSourceResult) DeepCopy() *BundleSourceResult { + if in == nil { + return nil + } + out := new(BundleSourceResult) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterBuildStrategy) DeepCopyInto(out *ClusterBuildStrategy) { *out = *in @@ -593,6 +622,22 @@ func (in *FailedAt) DeepCopy() *FailedAt { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GitSourceResult) DeepCopyInto(out *GitSourceResult) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitSourceResult. +func (in *GitSourceResult) DeepCopy() *GitSourceResult { + if in == nil { + return nil + } + out := new(GitSourceResult) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Image) DeepCopyInto(out *Image) { *out = *in @@ -628,6 +673,22 @@ func (in *Image) DeepCopy() *Image { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Output) DeepCopyInto(out *Output) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Output. +func (in *Output) DeepCopy() *Output { + if in == nil { + return nil + } + out := new(Output) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ParamValue) DeepCopyInto(out *ParamValue) { *out = *in @@ -722,6 +783,32 @@ func (in *Source) DeepCopy() *Source { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SourceResult) DeepCopyInto(out *SourceResult) { + *out = *in + if in.Git != nil { + in, out := &in.Git, &out.Git + *out = new(GitSourceResult) + **out = **in + } + if in.Bundle != nil { + in, out := &in.Bundle, &out.Bundle + *out = new(BundleSourceResult) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SourceResult. +func (in *SourceResult) DeepCopy() *SourceResult { + if in == nil { + return nil + } + out := new(SourceResult) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Strategy) DeepCopyInto(out *Strategy) { *out = *in diff --git a/pkg/reconciler/buildrun/resources/mutate.go b/pkg/reconciler/buildrun/resources/mutate.go index 0d0128751a..558b598e0f 100644 --- a/pkg/reconciler/buildrun/resources/mutate.go +++ b/pkg/reconciler/buildrun/resources/mutate.go @@ -18,7 +18,7 @@ import ( func amendTaskSpecWithImageMutate( cfg *config.Config, taskSpec *tektonv1beta1.TaskSpec, - buildOutput buildv1alpha1.Image, + buildOutput, buildRunOutput buildv1alpha1.Image, ) { // initialize the step from the template mutateStep := tektonv1beta1.Step{ @@ -26,12 +26,29 @@ func amendTaskSpecWithImageMutate( } mutateStep.Container.Name = imageMutateContainerName - mutateStep.Container.Args = mutateArgs(buildOutput.Annotations, buildOutput.Labels) + + // if labels or annotations are specified in buildRun then merge them with build's + labels := mergeMaps(buildOutput.Labels, buildRunOutput.Labels) + annotations := mergeMaps(buildOutput.Annotations, buildRunOutput.Annotations) + + mutateStep.Container.Args = mutateArgs(annotations, labels) // append the mutate step taskSpec.Steps = append(taskSpec.Steps, mutateStep) } +// mergeMaps takes 2 maps as input and merge the second into the first +// values in second would takes precedence if both maps have same keys +func mergeMaps(first map[string]string, second map[string]string) map[string]string { + if len(first) == 0 { + first = map[string]string{} + } + for k, v := range second { + first[k] = v + } + return first +} + func mutateArgs(annotations, labels map[string]string) []string { args := []string{ "--image", diff --git a/pkg/reconciler/buildrun/resources/taskrun.go b/pkg/reconciler/buildrun/resources/taskrun.go index cdc081c7c7..6591ed6a91 100644 --- a/pkg/reconciler/buildrun/resources/taskrun.go +++ b/pkg/reconciler/buildrun/resources/taskrun.go @@ -11,7 +11,7 @@ import ( "strings" taskv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" - v1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -213,10 +213,16 @@ func GenerateTaskSpec( } } + buildRunOutput := buildRun.Spec.Output + if buildRunOutput == nil { + buildRunOutput = &buildv1alpha1.Image{} + } + // Amending task spec with image mutate step if annotations or labels are - // specified in build manifest - if len(build.Spec.Output.Annotations) > 0 || len(build.Spec.Output.Labels) > 0 { - amendTaskSpecWithImageMutate(cfg, &generatedTaskSpec, build.Spec.Output) + // specified in build manifest or buildRun manifest + if len(build.Spec.Output.Annotations) > 0 || len(build.Spec.Output.Labels) > 0 || + len(buildRunOutput.Annotations) > 0 || len(buildRunOutput.Labels) > 0 { + amendTaskSpecWithImageMutate(cfg, &generatedTaskSpec, build.Spec.Output, *buildRunOutput) } return &generatedTaskSpec, nil diff --git a/pkg/reconciler/buildrun/resources/taskrun_test.go b/pkg/reconciler/buildrun/resources/taskrun_test.go index 6c6ef73a85..45b1158c4f 100644 --- a/pkg/reconciler/buildrun/resources/taskrun_test.go +++ b/pkg/reconciler/buildrun/resources/taskrun_test.go @@ -280,6 +280,90 @@ var _ = Describe("GenerateTaskrun", func() { Expect(err.Error()).To(Equal("error(s) occurred merging environment variables into BuildStrategy \"buildah\" steps: [environment variable \"MY_VAR_1\" already exists, environment variable \"MY_VAR_2\" already exists]")) }) }) + + Context("when only BuildRun has output image labels and annotation defined ", func() { + BeforeEach(func() { + build, err = ctl.LoadBuildYAML([]byte(test.BuildahBuildWithOutput)) + Expect(err).To(BeNil()) + + buildRun, err = ctl.LoadBuildRunFromBytes([]byte(test.BuildahBuildRunWithOutputImageLabelsAndAnnotations)) + Expect(err).To(BeNil()) + + buildStrategy, err = ctl.LoadBuildStrategyFromBytes([]byte(test.MinimalBuildahBuildStrategy)) + Expect(err).To(BeNil()) + buildStrategy.Spec.BuildSteps[0].ImagePullPolicy = "Always" + + expectedCommandOrArg = []string{ + "bud", "--tag=$(params.shp-output-image)", fmt.Sprintf("--file=$(inputs.params.%s)", "DOCKERFILE"), "$(params.shp-source-context)", + } + + JustBeforeEach(func() { + got, err = resources.GenerateTaskSpec(config.NewDefaultConfig(), build, buildRun, buildStrategy.Spec.BuildSteps, []buildv1alpha1.Parameter{}) + Expect(err).To(BeNil()) + }) + + It("should contain a step to mutate the image with labels and annotations merged from build and buildrun", func() { + Expect(got.Steps[3].Name).To(Equal("mutate-image")) + Expect(got.Steps[3].Command[0]).To(Equal("/ko-app/mutate-image")) + Expect(got.Steps[3].Args).To(Equal([]string{ + "--image", + "$(params.shp-output-image)", + "--result-file-image-digest", + "$(results.shp-image-digest.path)", + "result-file-image-size", + "$(results.shp-image-size.path)", + "--annotation", + "org.opencontainers.owner=my-company", + "--label", + "maintainer=new-team@my-company.com", + "foo=bar", + })) + }) + }) + }) + + Context("when Build and BuildRun both have output image labels and annotation defined ", func() { + BeforeEach(func() { + build, err = ctl.LoadBuildYAML([]byte(test.BuildahBuildWithAnnotationAndLabel)) + Expect(err).To(BeNil()) + + buildRun, err = ctl.LoadBuildRunFromBytes([]byte(test.BuildahBuildRunWithOutputImageLabelsAndAnnotations)) + Expect(err).To(BeNil()) + + buildStrategy, err = ctl.LoadBuildStrategyFromBytes([]byte(test.MinimalBuildahBuildStrategy)) + Expect(err).To(BeNil()) + buildStrategy.Spec.BuildSteps[0].ImagePullPolicy = "Always" + + expectedCommandOrArg = []string{ + "bud", "--tag=$(params.shp-output-image)", fmt.Sprintf("--file=$(inputs.params.%s)", "DOCKERFILE"), "$(params.shp-source-context)", + } + + JustBeforeEach(func() { + got, err = resources.GenerateTaskSpec(config.NewDefaultConfig(), build, buildRun, buildStrategy.Spec.BuildSteps, []buildv1alpha1.Parameter{}) + Expect(err).To(BeNil()) + }) + + It("should contain a step to mutate the image with labels and annotations merged from build and buildrun", func() { + Expect(got.Steps[3].Name).To(Equal("mutate-image")) + Expect(got.Steps[3].Command[0]).To(Equal("/ko-app/mutate-image")) + Expect(got.Steps[3].Args).To(Equal([]string{ + "--image", + "$(params.shp-output-image)", + "--result-file-image-digest", + "$(results.shp-image-digest.path)", + "result-file-image-size", + "$(results.shp-image-size.path)", + "--annotation", + "org.opencontainers.owner=my-company", + "org.opencontainers.image.url=https://my-company.com/images", + "--label", + "maintainer=new-team@my-company.com", + "foo=bar", + })) + }) + }) + }) + }) Describe("Generate the TaskRun", func() { diff --git a/test/buildrun_samples.go b/test/buildrun_samples.go index 504da8e815..25eb1b15f6 100644 --- a/test/buildrun_samples.go +++ b/test/buildrun_samples.go @@ -23,6 +23,28 @@ spec: fieldPath: "my-fieldpath" ` +// BuildahBuildRunWithOutputImageLabelsAndAnnotations defines a BuildRun +// with a output image labels and annotation +const BuildahBuildRunWithOutputImageLabelsAndAnnotations = ` +apiVersion: shipwright.io/v1alpha1 +kind: BuildRun +metadata: + name: buildah-run + namespace: build-test +spec: + buildRef: + name: buildah + serviceAccount: + name: buildpacks-v3-serviceaccount + output: + image: image-registry.openshift-image-registry.svc:5000/example/buildpacks-app-v2 + labels: + "maintainer": "new-team@my-company.com" + "foo": "bar" + annotations: + "org.opencontainers.owner": "my-company" +` + // MinimalBuildahBuildRun defines a simple // BuildRun with a referenced Build const MinimalBuildahBuildRun = `