From 9579ced0f93845da861e740988a727fad19e9e0d Mon Sep 17 00:00:00 2001
From: Adam Kaplan <adam.kaplan@redhat.com>
Date: Mon, 14 Jun 2021 12:44:40 -0400
Subject: [PATCH] Add RBAC for Owner Refs Permission Enforcement

Add necessary permissions so that the shipwright build controller can
add owner references if a cluster has the
OwnerReferencesPermissionEnforcement admission controller enabled.
With this admission controller enabled, service accounts need to have
explicit permission to delete objects that they set owner references on.
When `blockOwnerDeletion` is set on an owner ref, the controller must
also have explicit permission to update the finalizer subresource of the
parent object.

- Added delete permissions for objects we set owner refs on
- Added update permissions for the finalizer subresource on owner ref
  parents
- Fix ordering of permissions to use View (read) -> Edit (create,
  update) -> Admin (delete) ordering.
---
 deploy/200-role.yaml                          | 24 ++++++++++--
 .../build_buildpacks-v3_golang_delete_cr.yaml | 16 ++++++++
 test/e2e/e2e_test.go                          | 37 +++++++++++++++++++
 3 files changed, 73 insertions(+), 4 deletions(-)
 create mode 100644 test/data/build_buildpacks-v3_golang_delete_cr.yaml

diff --git a/deploy/200-role.yaml b/deploy/200-role.yaml
index 0c9a5379b0..a583ebea91 100644
--- a/deploy/200-role.yaml
+++ b/deploy/200-role.yaml
@@ -6,7 +6,7 @@ metadata:
 rules:
 - apiGroups: ['']
   resources: ['configmaps']
-  verbs:     ['create', 'get', 'update']
+  verbs:     ['get', 'create', 'update']
 
 - apiGroups: ['']
   resources: ['events']
@@ -21,7 +21,15 @@ metadata:
 rules:
 - apiGroups: ['shipwright.io']
   resources: ['buildruns']
-  verbs:     ['get', 'list', 'update', 'watch']
+  # The build-run-deletion annotation sets an owner ref on BuildRun objects.
+  # With the OwnerReferencesPermissionEnforcement admission controller enabled, controllers need the "delete" permission on objects that they set owner references on.
+  verbs:     ['get', 'list', 'watch', 'update', 'delete']
+
+- apiGroups: ['shipwright.io']
+  # BuildRuns are set as the owners of Tekton TaskRuns.
+  # With the OwnerReferencesPermissionEnforcement admission controller enabled, controllers need the "update" permission on the finalizer of the parent object in the owner reference.
+  resources: ['buildruns/finalizers']
+  verbs:     ['update']
 
 - apiGroups: ['shipwright.io']
   resources: ['buildruns/status']
@@ -31,6 +39,12 @@ rules:
   resources: ['builds']
   verbs:     ['get', 'list', 'watch']
 
+- apiGroups: ['shipwright.io']
+  # The build-run-deletion annotation makes Builds an owner of BuildRun objects.
+  # With the OwnerReferencesPermissionEnforcement admission controller enabled, controllers need the "update" permission on the finalizer of the parent object in the owner reference.
+  resources: ['builds/finalizers']
+  verbs:     ['update']
+
 - apiGroups: ['shipwright.io']
   resources: ['builds/status']
   verbs:     ['update']
@@ -45,7 +59,9 @@ rules:
 
 - apiGroups: ['tekton.dev']
   resources: ['taskruns']
-  verbs:     ['get', 'create', 'list', 'watch']
+  # BuildRuns are set as the owners of Tekton TaskRuns.
+  # With the OwnerReferencesPermissionEnforcement admission controller enabled, controllers need the "delete" permission on objects that they set owner references on.
+  verbs:     ['get', 'list', 'watch', 'create', 'delete']
 
 - apiGroups: ['']
   resources: ['pods']
@@ -57,4 +73,4 @@ rules:
 
 - apiGroups: ['']
   resources: ['serviceaccounts']
-  verbs:     ['create', 'delete', 'get', 'list', 'update', 'watch']
+  verbs:     ['get', 'list', 'watch', 'create', 'update', 'delete']
diff --git a/test/data/build_buildpacks-v3_golang_delete_cr.yaml b/test/data/build_buildpacks-v3_golang_delete_cr.yaml
new file mode 100644
index 0000000000..891df8bb3a
--- /dev/null
+++ b/test/data/build_buildpacks-v3_golang_delete_cr.yaml
@@ -0,0 +1,16 @@
+---
+apiVersion: shipwright.io/v1alpha1
+kind: Build
+metadata:
+  name: buildpack-golang-build
+  annotations:
+    build.shipwright.io/build-run-deletion: "true"
+spec:
+  source:
+    url: https://github.com/shipwright-io/sample-go
+    contextDir: source-build
+  strategy:
+    name: buildpacks-v3
+    kind: ClusterBuildStrategy
+  output:
+    image: image-registry.openshift-image-registry.svc:5000/build-examples/taxi-app
diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go
index af702d31fd..21db12c562 100644
--- a/test/e2e/e2e_test.go
+++ b/test/e2e/e2e_test.go
@@ -9,6 +9,7 @@ import (
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	"k8s.io/apimachinery/pkg/api/errors"
 
 	buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1"
 )
@@ -254,6 +255,42 @@ var _ = Describe("For a Kubernetes cluster with Tekton and build installed", fun
 		})
 	})
 
+	Context("when a build uses the build-run-deletion annotation", func() {
+
+		BeforeEach(func() {
+			testID = generateTestID("buildpacks-v3-golang")
+
+			// create the build definition
+			build = createBuild(
+				testBuild,
+				testID,
+				"test/data/build_buildpacks-v3_golang_delete_cr.yaml",
+			)
+		})
+
+		It("successfully deletes the BuildRun after the Build is deleted", func() {
+			By("running a build and expecting it to succeed")
+			buildRun, err = buildRunTestData(testBuild.Namespace, testID, "test/data/buildrun_buildpacks-v3_golang_cr.yaml")
+			Expect(err).ToNot(HaveOccurred(), "Error retrieving buildrun test data")
+
+			validateBuildRunToSucceed(testBuild, buildRun)
+
+			By("deleting the parent Build object")
+			err = testBuild.DeleteBuild(build.Name)
+			Expect(err).NotTo(HaveOccurred(), "error deleting the parent Build")
+			Eventually(func() bool {
+				_, err = testBuild.GetBR(buildRun.Name)
+				if err == nil {
+					return false
+				}
+				if !errors.IsNotFound(err) {
+					return false
+				}
+				return true
+			}).Should(BeTrue())
+		})
+	})
+
 	Context("when a Buildpacks v3 build is defined for a java runtime", func() {
 
 		BeforeEach(func() {