Merge pull request #807 from adambkaplan/fix-finalizer-rbac

Fix RBAC so Shipwright Build works with the OwnerReferencesPermissionEnforcement admission controller

Author: openshift-merge-robot

.github/workflows/ci.yml

@@ -104,6 +104,7 @@ jobs:
             version: v0.10.0
             node_image: kindest/node:${{ matrix.kubernetes }}
             cluster_name: kind
+            config: test/kind/config.yaml
             wait: 120s
         - name: Verify kind cluster
           run: |

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']

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

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() {

test/kind/config.yaml

@@ -0,0 +1,10 @@
+kind: Cluster
+apiVersion: kind.x-k8s.io/v1alpha4
+nodes:
+- role: control-plane
+  kubeadmConfigPatches:
+  - |
+    kind: ClusterConfiguration
+    apiServer:
+      extraArgs:
+        enable-admission-plugins: CertificateApproval,CertificateSigning,CertificateSubjectRestriction,DefaultIngressClass,DefaultStorageClass,DefaultTolerationSeconds,LimitRanger,MutatingAdmissionWebhook,NamespaceLifecycle,NodeRestriction,OwnerReferencesPermissionEnforcement,PersistentVolumeClaimResize,PersistentVolumeLabel,PodNodeSelector,PodTolerationRestriction,Priority,ResourceQuota,RuntimeClass,ServiceAccount,StorageObjectInUseProtection,TaintNodesByCondition,ValidatingAdmissionWebhook