From 3da6b2d99e8e53d18787ba7b770719c80e9a44ab Mon Sep 17 00:00:00 2001 From: Sascha Schwarze Date: Thu, 21 Jan 2021 16:10:00 +0100 Subject: [PATCH] Propagate annotations from BuildStrategy to TaskRun --- docs/buildstrategies.md | 17 ++++ .../buildstrategy-annotation-propagation.md | 4 +- pkg/apis/build/v1alpha1/build_types.go | 13 +-- pkg/apis/build/v1alpha1/buildrun_types.go | 7 +- pkg/apis/build/v1alpha1/buildstrategy.go | 1 + .../build/v1alpha1/buildstrategy_types.go | 12 ++- .../v1alpha1/clusterbuildstrategy_types.go | 12 ++- pkg/controller/buildrun/generate_taskrun.go | 20 +++++ .../buildrun/generate_taskrun_test.go | 5 ++ test/build_samples.go | 13 +++ test/buildstrategy_samples.go | 4 + test/clusterbuildstrategy_samples.go | 50 +++++++++++ .../buildstrategy_to_taskruns_test.go | 85 +++++++++++++++++++ .../clusterbuildstrategy_to_taskruns_test.go | 85 +++++++++++++++++++ test/integration/utils/buildstrategies.go | 7 ++ 15 files changed, 322 insertions(+), 13 deletions(-) create mode 100644 test/integration/buildstrategy_to_taskruns_test.go create mode 100644 test/integration/clusterbuildstrategy_to_taskruns_test.go diff --git a/docs/buildstrategies.md b/docs/buildstrategies.md index 61f052137f..db1d5b34da 100644 --- a/docs/buildstrategies.md +++ b/docs/buildstrategies.md @@ -478,3 +478,20 @@ In the above scenario, we can see how the maximum numbers for resource requests **Scenario 3.** Namespace **with** a `LimitRange`. When a `LimitRange` exists on the namespace, `Tekton Pipeline` controller will do the same approach as stated in the above two scenarios. The difference is that for the containers that have lower values, instead of zero, they will get the `minimum values of the LimitRange`. + +## Annotations + +Annotations can be defined for a BuildStrategy/ClusterBuildStrategy as for any other Kubernetes object. Annotations are propagated to the TaskRun and from there, Tekton propagates them to the Pod. Use cases for this are for example: + +- The Kubernetes [Network Traffic Shaping](https://kubernetes.io/docs/concepts/extend-kubernetes/compute-storage-net/network-plugins/#support-traffic-shaping) feature looks for the `kubernetes.io/ingress-bandwidth` and `kubernetes.io/egress-bandwidth` annotations to limit the network bandwidth the `Pod` is allowed to use. +- The [AppArmor profile of a container](https://kubernetes.io/docs/tutorials/clusters/apparmor/) is defined using the `container.apparmor.security.beta.kubernetes.io/` annotation. + +The following annotations are not propagated: + +- `kubectl.kubernetes.io/last-applied-configuration` +- `clusterbuildstrategy.build.dev/*` +- `buildstrategy.build.dev/*` +- `build.build.dev/*` +- `buildrun.build.dev/*` + +A Kubernetes administrator can further restrict the usage of annotations by using policy engines like [Open Policy Agent](https://www.openpolicyagent.org/). diff --git a/docs/proposals/buildstrategy-annotation-propagation.md b/docs/proposals/buildstrategy-annotation-propagation.md index c8e5927810..f4e7ab3d1a 100644 --- a/docs/proposals/buildstrategy-annotation-propagation.md +++ b/docs/proposals/buildstrategy-annotation-propagation.md @@ -22,9 +22,9 @@ approvers: creation-date: 2020-12-16 -last-updated: 2020-12-16 +last-updated: 2020-01-20 -status: implementable +status: implemented see-also: diff --git a/pkg/apis/build/v1alpha1/build_types.go b/pkg/apis/build/v1alpha1/build_types.go index e955b5d9a0..7bf66fab8c 100644 --- a/pkg/apis/build/v1alpha1/build_types.go +++ b/pkg/apis/build/v1alpha1/build_types.go @@ -10,23 +10,26 @@ import ( ) const ( + // BuildDomain is the domain used for all labels and annotations for this resource + BuildDomain = "build.build.dev" + // LabelBuild is a label key for defining the build name - LabelBuild = "build.build.dev/name" + LabelBuild = BuildDomain + "/name" // LabelBuildGeneration is a label key for defining the build generation - LabelBuildGeneration = "build.build.dev/generation" + LabelBuildGeneration = BuildDomain + "/generation" // AnnotationBuildRunDeletion is a label key for enabling/disabling the BuildRun deletion - AnnotationBuildRunDeletion = "build.build.dev/build-run-deletion" + AnnotationBuildRunDeletion = BuildDomain + "/build-run-deletion" // AnnotationBuildRefSecret is an annotation that tells the Build Controller to reconcile on // events of the secret only if is referenced by a Build in the same namespace - AnnotationBuildRefSecret = "build.build.dev/referenced.secret" + AnnotationBuildRefSecret = BuildDomain + "/referenced.secret" // AnnotationBuildVerifyRepository tells the Build Controller to check a remote repository. If the annotation is not set // or has a value of 'true', the controller triggers the validation. A value of 'false' means the controller // will bypass checking the remote repository. - AnnotationBuildVerifyRepository = "build.build.dev/verify.repository" + AnnotationBuildVerifyRepository = BuildDomain + "/verify.repository" ) // BuildSpec defines the desired state of Build diff --git a/pkg/apis/build/v1alpha1/buildrun_types.go b/pkg/apis/build/v1alpha1/buildrun_types.go index b017f38231..642eda07ba 100644 --- a/pkg/apis/build/v1alpha1/buildrun_types.go +++ b/pkg/apis/build/v1alpha1/buildrun_types.go @@ -10,11 +10,14 @@ import ( ) const ( + // BuildRunDomain is the domain used for all labels and annotations for this resource + BuildRunDomain = "buildrun.build.dev" + // LabelBuildRun is a label key for BuildRuns to define the name of the BuildRun - LabelBuildRun = "buildrun.build.dev/name" + LabelBuildRun = BuildRunDomain + "/name" // LabelBuildRunGeneration is a label key for BuildRuns to define the generation - LabelBuildRunGeneration = "buildrun.build.dev/generation" + LabelBuildRunGeneration = BuildRunDomain + "/generation" ) // BuildRunSpec defines the desired state of BuildRun diff --git a/pkg/apis/build/v1alpha1/buildstrategy.go b/pkg/apis/build/v1alpha1/buildstrategy.go index 24d066d94e..13d31766a3 100644 --- a/pkg/apis/build/v1alpha1/buildstrategy.go +++ b/pkg/apis/build/v1alpha1/buildstrategy.go @@ -50,6 +50,7 @@ type StrategyRef struct { // BuilderStrategy defines the common elements of build strategies type BuilderStrategy interface { + GetAnnotations() map[string]string GetName() string GetGeneration() int64 GetResourceLabels() map[string]string diff --git a/pkg/apis/build/v1alpha1/buildstrategy_types.go b/pkg/apis/build/v1alpha1/buildstrategy_types.go index 2f26e16f8e..2a96bab73a 100644 --- a/pkg/apis/build/v1alpha1/buildstrategy_types.go +++ b/pkg/apis/build/v1alpha1/buildstrategy_types.go @@ -11,11 +11,14 @@ import ( ) const ( + // BuildStrategyDomain is the domain used for all labels and annotations for this resource + BuildStrategyDomain = "buildstrategy.build.dev" + // LabelBuildStrategyName is a label key for defining the build strategy name - LabelBuildStrategyName = "buildstrategy.build.dev/name" + LabelBuildStrategyName = BuildStrategyDomain + "/name" // LabelBuildStrategyGeneration is a label key for defining the build strategy generation - LabelBuildStrategyGeneration = "buildstrategy.build.dev/generation" + LabelBuildStrategyGeneration = BuildStrategyDomain + "/generation" ) // +genclient @@ -41,6 +44,11 @@ type BuildStrategyList struct { Items []BuildStrategy `json:"items"` } +// GetAnnotations returns the annotations of the build strategy +func (s BuildStrategy) GetAnnotations() map[string]string { + return s.Annotations +} + // GetName returns the name of the build strategy func (s BuildStrategy) GetName() string { return s.Name diff --git a/pkg/apis/build/v1alpha1/clusterbuildstrategy_types.go b/pkg/apis/build/v1alpha1/clusterbuildstrategy_types.go index 3111466481..3acf83c4cf 100644 --- a/pkg/apis/build/v1alpha1/clusterbuildstrategy_types.go +++ b/pkg/apis/build/v1alpha1/clusterbuildstrategy_types.go @@ -11,11 +11,14 @@ import ( ) const ( + // ClusterBuildStrategyDomain is the domain used for all labels and annotations for this resource + ClusterBuildStrategyDomain = "clusterbuildstrategy.build.dev" + // LabelClusterBuildStrategyName is a label key for defining the cluster build strategy name - LabelClusterBuildStrategyName = "clusterbuildstrategy.build.dev/name" + LabelClusterBuildStrategyName = ClusterBuildStrategyDomain + "/name" // LabelClusterBuildStrategyGeneration is a label key for defining the cluster build strategy generation - LabelClusterBuildStrategyGeneration = "clusterbuildstrategy.build.dev/generation" + LabelClusterBuildStrategyGeneration = ClusterBuildStrategyDomain + "/generation" ) // +genclient @@ -42,6 +45,11 @@ type ClusterBuildStrategyList struct { Items []ClusterBuildStrategy `json:"items"` } +// GetAnnotations returns the annotations of the build strategy +func (s ClusterBuildStrategy) GetAnnotations() map[string]string { + return s.Annotations +} + // GetName returns the name of the build strategy func (s ClusterBuildStrategy) GetName() string { return s.Name diff --git a/pkg/controller/buildrun/generate_taskrun.go b/pkg/controller/buildrun/generate_taskrun.go index 7c3c84bed4..d4e8b265c0 100644 --- a/pkg/controller/buildrun/generate_taskrun.go +++ b/pkg/controller/buildrun/generate_taskrun.go @@ -256,6 +256,18 @@ func GenerateTaskRun( }, } + // assign the annotations from the build strategy, filter out those that should not be propagated + for key, value := range strategy.GetAnnotations() { + taskRunAnnotations := make(map[string]string) + if isPropagatableAnnotation(key) { + taskRunAnnotations[key] = value + } + + if len(taskRunAnnotations) > 0 { + expectedTaskRun.Annotations = taskRunAnnotations + } + } + for label, value := range strategy.GetResourceLabels() { expectedTaskRun.Labels[label] = value } @@ -305,3 +317,11 @@ func effectiveTimeout(build *buildv1alpha1.Build, buildRun *buildv1alpha1.BuildR return nil } + +func isPropagatableAnnotation(key string) bool { + return key != "kubectl.kubernetes.io/last-applied-configuration" && + !strings.HasPrefix(key, buildv1alpha1.ClusterBuildStrategyDomain+"/") && + !strings.HasPrefix(key, buildv1alpha1.BuildStrategyDomain+"/") && + !strings.HasPrefix(key, buildv1alpha1.BuildDomain+"/") && + !strings.HasPrefix(key, buildv1alpha1.BuildRunDomain+"/") +} diff --git a/pkg/controller/buildrun/generate_taskrun_test.go b/pkg/controller/buildrun/generate_taskrun_test.go index 075d89e664..1165509f11 100644 --- a/pkg/controller/buildrun/generate_taskrun_test.go +++ b/pkg/controller/buildrun/generate_taskrun_test.go @@ -158,6 +158,11 @@ var _ = Describe("GenerateTaskrun", func() { Expect(got.Labels[buildv1alpha1.LabelBuildStrategyGeneration]).To(Equal("0")) }) + It("should filter out certain annotations when propagating them to the TaskRun", func() { + Expect(len(got.Annotations)).To(Equal(1)) + Expect(got.Annotations["kubernetes.io/ingress-bandwidth"]).To(Equal("1M")) + }) + It("should ensure generated TaskRun's input and output resources are correct", func() { inputResources := got.Spec.Resources.Inputs for _, inputResource := range inputResources { diff --git a/test/build_samples.go b/test/build_samples.go index bf58174acd..16fc603463 100644 --- a/test/build_samples.go +++ b/test/build_samples.go @@ -81,6 +81,19 @@ spec: timeout: 30s ` +// BuildBSMinimal defines a Build with a BuildStrategy +const BuildBSMinimal = ` +apiVersion: build.dev/v1alpha1 +kind: Build +spec: + source: + url: "https://github.com/sbose78/taxi" + strategy: + kind: BuildStrategy + output: + image: image-registry.openshift-image-registry.svc:5000/example/buildpacks-app +` + // BuildCBSMinimal defines a Build with a // ClusterBuildStrategy const BuildCBSMinimal = ` diff --git a/test/buildstrategy_samples.go b/test/buildstrategy_samples.go index cbb53172ff..10f6e6cb84 100644 --- a/test/buildstrategy_samples.go +++ b/test/buildstrategy_samples.go @@ -65,6 +65,10 @@ const BuildahBuildStrategySingleStep = ` apiVersion: build.dev/v1alpha1 kind: BuildStrategy metadata: + annotations: + kubernetes.io/ingress-bandwidth: 1M + clusterbuildstrategy.build.dev/dummy: aValue + kubectl.kubernetes.io/last-applied-configuration: anotherValue name: buildah spec: buildSteps: diff --git a/test/clusterbuildstrategy_samples.go b/test/clusterbuildstrategy_samples.go index 40e3d5ce51..0507698cb4 100644 --- a/test/clusterbuildstrategy_samples.go +++ b/test/clusterbuildstrategy_samples.go @@ -246,3 +246,53 @@ spec: cpu: 250m memory: 128Mi ` + +// ClusterBuildStrategyWithAnnotations is a cluster build strategy that contains annotations +const ClusterBuildStrategyWithAnnotations = ` +apiVersion: build.dev/v1alpha1 +kind: ClusterBuildStrategy +metadata: + annotations: + kubernetes.io/ingress-bandwidth: 1M + clusterbuildstrategy.build.dev/dummy: aValue + kubectl.kubernetes.io/last-applied-configuration: anotherValue + name: kaniko +spec: + buildSteps: + - name: step-build-and-push + image: gcr.io/kaniko-project/executor:v1.3.0 + workingDir: /workspace/source + securityContext: + runAsUser: 0 + capabilities: + add: + - CHOWN + - DAC_OVERRIDE + - FOWNER + - SETGID + - SETUID + - SETFCAP + env: + - name: DOCKER_CONFIG + value: /tekton/home/.docker + - name: AWS_ACCESS_KEY_ID + value: NOT_SET + - name: AWS_SECRET_KEY + value: NOT_SET + command: + - /kaniko/executor + args: + - --skip-tls-verify=true + - --dockerfile=$(build.dockerfile) + - --context=/workspace/source/$(build.source.contextDir) + - --destination=$(build.output.image) + - --oci-layout-path=/workspace/output/image + - --snapshotMode=redo + resources: + limits: + cpu: 500m + memory: 1Gi + requests: + cpu: 250m + memory: 65Mi +` diff --git a/test/integration/buildstrategy_to_taskruns_test.go b/test/integration/buildstrategy_to_taskruns_test.go new file mode 100644 index 0000000000..c7d0cd6ca1 --- /dev/null +++ b/test/integration/buildstrategy_to_taskruns_test.go @@ -0,0 +1,85 @@ +// Copyright The Shipwright Contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package integration_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" + "github.com/shipwright-io/build/test" +) + +var _ = Describe("Integration tests BuildStrategies and TaskRuns", func() { + var ( + bsObject *v1alpha1.BuildStrategy + buildObject *v1alpha1.Build + buildRunObject *v1alpha1.BuildRun + buildSample []byte + buildRunSample []byte + ) + + // Load the BuildStrategies before each test case + BeforeEach(func() { + bsObject, err = tb.Catalog.LoadBuildStrategyFromBytes([]byte(test.BuildahBuildStrategySingleStep)) + Expect(err).To(BeNil()) + + err = tb.CreateBuildStrategy(bsObject) + Expect(err).To(BeNil()) + }) + + // Delete the BuildStrategies after each test case + AfterEach(func() { + + _, err = tb.GetBuild(buildObject.Name) + if err == nil { + Expect(tb.DeleteBuild(buildObject.Name)).To(BeNil()) + } + + err := tb.DeleteBuildStrategy(bsObject.Name) + Expect(err).To(BeNil()) + }) + + // Override the Build and BuildRun CRD instances to use + // before an It() statement is executed + JustBeforeEach(func() { + if buildSample != nil { + buildObject, err = tb.Catalog.LoadBuildWithNameAndStrategy(BUILD+tb.Namespace, bsObject.Name, buildSample) + Expect(err).To(BeNil()) + } + + if buildRunSample != nil { + buildRunObject, err = tb.Catalog.LoadBRWithNameAndRef(BUILDRUN+tb.Namespace, BUILD+tb.Namespace, buildRunSample) + Expect(err).To(BeNil()) + } + }) + + Context("when a buildrun is created", func() { + + BeforeEach(func() { + buildSample = []byte(test.BuildBSMinimal) + buildRunSample = []byte(test.MinimalBuildRun) + }) + + It("should create a taskrun with the correct annotations", func() { + + Expect(tb.CreateBuild(buildObject)).To(BeNil()) + + Expect(tb.CreateBR(buildRunObject)).To(BeNil()) + + _, err = tb.GetBRTillStartTime(buildRunObject.Name) + Expect(err).To(BeNil()) + + taskRun, err := tb.GetTaskRunFromBuildRun(buildRunObject.Name) + Expect(err).To(BeNil()) + + Expect(taskRun.Annotations["kubernetes.io/ingress-bandwidth"]).To(Equal("1M")) + _, containsKey := taskRun.Annotations["clusterbuildstrategy.build.dev/dummy"] + Expect(containsKey).To(BeFalse()) + _, containsKey = taskRun.Annotations["kubectl.kubernetes.io/last-applied-configuration"] + Expect(containsKey).To(BeFalse()) + }) + }) +}) diff --git a/test/integration/clusterbuildstrategy_to_taskruns_test.go b/test/integration/clusterbuildstrategy_to_taskruns_test.go new file mode 100644 index 0000000000..78d57a7d89 --- /dev/null +++ b/test/integration/clusterbuildstrategy_to_taskruns_test.go @@ -0,0 +1,85 @@ +// Copyright The Shipwright Contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package integration_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" + "github.com/shipwright-io/build/test" +) + +var _ = Describe("Integration tests ClusterBuildStrategies and TaskRuns", func() { + var ( + cbsObject *v1alpha1.ClusterBuildStrategy + buildObject *v1alpha1.Build + buildRunObject *v1alpha1.BuildRun + buildSample []byte + buildRunSample []byte + ) + + // Load the BuildStrategies before each test case + BeforeEach(func() { + cbsObject, err = tb.Catalog.LoadCBSWithName(STRATEGY+tb.Namespace, []byte(test.ClusterBuildStrategyWithAnnotations)) + Expect(err).To(BeNil()) + + err = tb.CreateClusterBuildStrategy(cbsObject) + Expect(err).To(BeNil()) + }) + + // Delete the BuildStrategies after each test case + AfterEach(func() { + + _, err = tb.GetBuild(buildObject.Name) + if err == nil { + Expect(tb.DeleteBuild(buildObject.Name)).To(BeNil()) + } + + err := tb.DeleteClusterBuildStrategy(cbsObject.Name) + Expect(err).To(BeNil()) + }) + + // Override the Build and BuildRun CRD instances to use + // before an It() statement is executed + JustBeforeEach(func() { + if buildSample != nil { + buildObject, err = tb.Catalog.LoadBuildWithNameAndStrategy(BUILD+tb.Namespace, STRATEGY+tb.Namespace, buildSample) + Expect(err).To(BeNil()) + } + + if buildRunSample != nil { + buildRunObject, err = tb.Catalog.LoadBRWithNameAndRef(BUILDRUN+tb.Namespace, BUILD+tb.Namespace, buildRunSample) + Expect(err).To(BeNil()) + } + }) + + Context("when a buildrun is created", func() { + + BeforeEach(func() { + buildSample = []byte(test.BuildCBSMinimal) + buildRunSample = []byte(test.MinimalBuildRun) + }) + + It("should create a taskrun with the correct annotations", func() { + + Expect(tb.CreateBuild(buildObject)).To(BeNil()) + + Expect(tb.CreateBR(buildRunObject)).To(BeNil()) + + _, err = tb.GetBRTillStartTime(buildRunObject.Name) + Expect(err).To(BeNil()) + + taskRun, err := tb.GetTaskRunFromBuildRun(buildRunObject.Name) + Expect(err).To(BeNil()) + + Expect(taskRun.Annotations["kubernetes.io/ingress-bandwidth"]).To(Equal("1M")) + _, containsKey := taskRun.Annotations["clusterbuildstrategy.build.dev/dummy"] + Expect(containsKey).To(BeFalse()) + _, containsKey = taskRun.Annotations["kubectl.kubernetes.io/last-applied-configuration"] + Expect(containsKey).To(BeFalse()) + }) + }) +}) diff --git a/test/integration/utils/buildstrategies.go b/test/integration/utils/buildstrategies.go index dc433e7695..d2a287c195 100644 --- a/test/integration/utils/buildstrategies.go +++ b/test/integration/utils/buildstrategies.go @@ -24,3 +24,10 @@ func (t *TestBuild) CreateBuildStrategy(bs *v1alpha1.BuildStrategy) error { } return nil } + +// DeleteBuildStrategy deletes a BuildStrategy on the current test namespace +func (t *TestBuild) DeleteBuildStrategy(name string) error { + bsInterface := t.BuildClientSet.BuildV1alpha1().BuildStrategies(t.Namespace) + + return bsInterface.Delete(context.TODO(), name, metav1.DeleteOptions{}) +}