Merge pull request #539 from SaschaSchwarze0/sascha-520-propagate-str…

…ategy-annotations

Propagate annotations from BuildStrategy to TaskRun

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/<container_name>` 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/).

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:
 

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

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

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

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

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

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+"/")
+}

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 {

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 = `

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:

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
+`

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())
+		})
+	})
+})