Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"main" errors out with controller-runtime error #806

Closed
sbose78 opened this issue Jun 14, 2021 · 9 comments · Fixed by #807
Closed

"main" errors out with controller-runtime error #806

sbose78 opened this issue Jun 14, 2021 · 9 comments · Fixed by #807
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@sbose78
Copy link
Member

sbose78 commented Jun 14, 2021

"sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\tsigs.k8s.io/controller-runtime@v0.6.1/pkg/internal/controller/controller.go:209\n
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).worker\n\tsigs.k8s.io/controller-runtime@v0.6.1/pkg/internal/controller/controller.go:188\n
k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1\n\tk8s.io/apimachinery@v0.20.2/pkg/util/wait/wait.go:155\n
k8s.io/apimachinery/pkg/util/wait.BackoffUntil\n\tk8s.io/apimachinery@v0.20.2/pkg/util/wait/wait.go:156\n
k8s.io/apimachinery/pkg/util/wait.JitterUntil\n\tk8s.io/apimachinery@v0.20.2/pkg/util/wait/wait.go:133\n
k8s.io/apimachinery/pkg/util/wait.Until\n\tk8s.io/apimachinery@v0.20.2/pkg/util/wait/wait.go:90"

Initial discussion #768 (comment)

@sbose78 sbose78 added the kind/bug Categorizes issue or PR as related to a bug. label Jun 14, 2021
@gabemontero
Copy link
Member

So tekton calls this api machinery api: https://github.com/tektoncd/pipeline/blob/b7fa888082bd20e08edc3b89b6b4d52ed4f00651/pkg/pod/pod.go#L295

Which leads to the setting of BlockOwnerDeletion to true at https://github.com/tektoncd/pipeline/blob/c8dc797cf5a6f11f90cb742d014470a444fcdc60/vendor/k8s.io/apimachinery/pkg/apis/meta/v1/controller_ref.go#L55

But according to git blame neither of those precise spots has changed in 2 years

So, circling back to @imjasonh 's reduce RBAC permission PR he did remove permissions on deployment/finalizers at https://github.com/shipwright-io/build/pull/768/files#diff-4633b495133d1f73df291ee1bff05516c88e81f569e876994cf2239a4c5c0ba7L52-L55

but I don't think that setting that permission gets you pod/finalizers .....

Then, considering the doc around BlockOwnerDeletion from `metav1.ObjectReference

	// If true, AND if the owner has the "foregroundDeletion" finalizer, then
      
      
        	// the owner cannot be deleted from the key-value store until this
      
      
        	// reference is removed.
      
      
        	// Defaults to false.
      
      
        	// To set this field, a user needs "delete" permission of the owner,
      
      FinalizerDeleteDependents
        	// otherwise 422 (Unprocessable Entity) will be returned.
      
      
        	// +optional
      
      
        	<span class="pl-c1">BlockOwnerDeletion</span> <span class="pl-c1">*</span><span class="pl-smi">bool</span> <span class="pl-s">`json:"blockOwnerDeletion,omitempty" protobuf:"varint,7,opt,name=blockOwnerDeletion"`</span>

I looked for any possible recent changes for the `foregroundDeletion" finalizer, but I see no refs to that string or the metav1 constant defined for that.

For now, going to include readding the deployment finalizer perm into the list of permutations to try, and go from there.

@gabemontero
Copy link
Member

@adambkaplan confirmed that the same issue on the current main branch occurs at openshift 4.7 (which means a different version of k8s more aligned with the current shipwright k8s dependency)

@adambkaplan
Copy link
Member

Root error I'm seeing is forbidden: cannot set an onwerRef on a resource you can't delete. Seems to be happening It appears we are passing the the error up directly, so we aren't providing sufficient context around the error message.

{"level":"error","ts":1623681661.1684248,"logger":"controller-runtime.controller","msg":"Reconciler error","controller":"buildrun-controller","name":"kaniko-golang-build-1","namespace":"test-strategy","error":"buildruns.shipwright.io \"kaniko-golang-build-1\" is forbidden: cannot set an ownerRef on a resource you can't delete: , <nil>","stacktrace":"sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\tsigs.k8s.io/controller-runtime@v0.6.1/pkg/internal/controller/controller.go:209\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).worker\n\tsigs.k8s.io/controller-runtime@v0.6.1/pkg/internal/controller/controller.go:188\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1\n\tk8s.io/apimachinery@v0.20.2/pkg/util/wait/wait.go:155\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil\n\tk8s.io/apimachinery@v0.20.2/pkg/util/wait/wait.go:156\nk8s.io/apimachinery/pkg/util/wait.JitterUntil\n\tk8s.io/apimachinery@v0.20.2/pkg/util/wait/wait.go:133\nk8s.io/apimachinery/pkg/util/wait.Until\n\tk8s.io/apimachinery@v0.20.2/pkg/util/wait/wait.go:90"}

@adambkaplan
Copy link
Member

Note - after granting the controller permission to delete BuildRun objects, now hitting the "set finalizers" permission issue.

@gabemontero
Copy link
Member

the 4.7 vs. 4.8 variants are interesting

that said, for both permutations, revisting the rbac reduction PR, add/update/delete perms were removed on some of the object types in question

for me as well I'm systematically am adding removed perms for those types to at least get to a level playing field

then I'll see about adding specific finalizer perms

@adambkaplan
Copy link
Member

It appears on OpenShift that the OwnerRefererencesPermissionEnforcement admission controller is enabled by default. It is unclear if this admission controller is enabled or disabled by default on kind.

https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#ownerreferencespermissionenforcement

@gabemontero
Copy link
Member

To reiterate some detail I posted elsewhere here, when I replace deploying shipwright from main branch with

kubectl apply --filename https://github.com/shipwright-io/build/releases/download/nightly/nightly-2021-03-24-1616591545.yaml

on the same OpenShift and Tekton versions things work.

@gabemontero
Copy link
Member

OK I had to add finalizer permissions to get it to work again.

Now seeing if some of the perms that I readded that were removed via #768 have any bearing

@gabemontero
Copy link
Member

turns out, we only need permissions on buildruns/finalizers to get things to work

on 4.8 at least @adambkaplan and installing openshift pipelines to get tekton, I did NOT have to add delete perms on buildrun objs as you noted above

Now, circling back to @imjasonh 's rbac reduction PR, I believe https://github.com/shipwright-io/build/pull/768/files#diff-4633b495133d1f73df291ee1bff05516c88e81f569e876994cf2239a4c5c0ba7L72 and the use of * means that the buildruns/finalizers resource was included.

So I think we can at least say with certainty that that PR caused this issue, as we originally guessed/assumed.

Lastly, to potentially build upon @adambkaplan 's notes in #806 (comment), looking at the error message in what I got:

error":"taskruns.tekton.dev \"buildpack-nodejs-build-87cwh-49krk\" is forbidden: cannot set blockOwnerDeletion if an ownerReference refers to a resource you can't set finalizers on: , <nil>"

the reference to the taskrun initially threw me off and had me thinking about taskerror":"taskruns.tekton.dev "buildpack-nodejs-build-87cwh-49krk" is forbidden: cannot set blockOwnerDeletion if an ownerReference refers to a resource you can't set finalizers on: , "run permisssions, but now that I now what precise permission is needed, I guess the text could be construed as if the ownerRerference in question was the the buildrun's, and hence needing to be able to set finalizers on buildruns are needed.

Fine tuning the new permision to only include the required verbs, and then will have a PR up shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment