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

Golang example on OpenShift 4.3 cannot produce deployments (finalizer RBAC error) #3477

Closed
a-roberts opened this issue Jul 21, 2020 · 12 comments · Fixed by #4162
Closed

Golang example on OpenShift 4.3 cannot produce deployments (finalizer RBAC error) #3477

a-roberts opened this issue Jul 21, 2020 · 12 comments · Fixed by #4162
Assignees
Labels
kind/documentation Categorizes issue or PR as related to documentation. triage/needs-information Indicates an issue needs more information in order to work on it.
Milestone

Comments

@a-roberts
Copy link

a-roberts commented Jul 21, 2020

Bug Report

What did you do?
Followed https://sdk.operatorframework.io/docs/golang/quickstart/

What did you expect to see?

Memcached pods appearing

What did you see instead? Under which circumstances?


2020-07-21T08:47:07.870Z        INFO    controllers.Memcached   Creating a new Deployment       {"memcached": "default/memcached-sample", "Deployment.Namespace": "default", "Deployment.Name": "memcached-sample"}
2020-07-21T08:47:07.877Z        ERROR   controllers.Memcached   Failed to create new Deployment {"memcached": "default/memcached-sample", "Deployment.Namespace": "default", "Deployment.Name": "memcached-sample", "error": "deployments.apps \"memcached-sample\" is forbidden: cannot set blockOwnerDeletion if an ownerReference refers to a resource you can't set finalizers on: , <nil>"}

No new pods. Above error comes from the logs of my operator.

Environment

  • operator-sdk version:
    0.19
  • go version:
    1.14.5
  • Kubernetes version information:

Client Version: version.Info{Major:"1", Minor:"17", GitVersion:"v1.17.0", GitCommit:"70132b0f130acc0bed193d9ba59dd186f0e634cf", GitTreeState:"clean", BuildDate:"2019-12-07T21:20:10Z", GoVersion:"go1.13.4", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"16+", GitVersion:"v1.16.2", GitCommit:"a02f27a", GitTreeState:"clean", BuildDate:"2020-04-13T12:04:13Z", GoVersion:"go1.12.12", Compiler:"gc", Platform:"linux/amd64"}
  • Kubernetes cluster kind:

OpenShift 4.3

  • Are you writing your operator in ansible, helm, or go?

Go

Possible Solution

Missing rbac in role.yaml surely, missing annotations somewhere to allow finalizer permissions?

Here's my generated role.yaml:


---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  creationTimestamp: null
  name: manager-role
rules:
- apiGroups:
  - apps
  resources:
  - deployments
  verbs:
  - create
  - delete
  - get
  - list
  - patch
  - update
  - watch
- apiGroups:
  - cache.example.com
  resources:
  - memcacheds
  verbs:
  - create
  - delete
  - get
  - list
  - patch
  - update
  - watch
- apiGroups:
  - cache.example.com
  resources:
  - memcacheds/status
  verbs:
  - get
  - patch
  - update
- apiGroups:
  - ""
  resources:
  - pods
  verbs:
  - get
  - list

My reconcile method as per the example:

// +kubebuilder:rbac:groups=cache.example.com,resources=memcacheds,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=cache.example.com,resources=memcacheds/status,verbs=get;update;patch
// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;

func (r *MemcachedReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
@estroz
Copy link
Member

estroz commented Jul 21, 2020

@a-roberts can you try adding the following marker and running make deploy again?

// +kubebuilder:rbac:groups=apps,resources=deployments/finalizers,verbs=get;create;update;patch;delete

@estroz estroz added the triage/needs-information Indicates an issue needs more information in order to work on it. label Jul 24, 2020
@estroz estroz added this to the Backlog milestone Jul 27, 2020
@camilamacedo86
Copy link
Contributor

camilamacedo86 commented Jul 29, 2020

HI @a-roberts,

It shows duplicated of #3590. See my comment in #3590 (comment).

I am closing this one and I'd like to ask for you follow up the #3590 if possible. However, if you do believes that it should be re-opened for any reason please feel free to ping and let us know.

@hasbro17
Copy link
Contributor

Reopening after I hit the same error while testing on Openshift 4.5.6:

ERROR   controllers.Memcached   Failed to create new Deployment {"memcached": "default/memcached-sample", "Deployment.Namespace": "default", "Deployment.Name": "memcached-sample", "error": "deployments.apps \"memcached-sample\" is forbidden: cannot set blockOwnerDeletion if an ownerReference refers to a resource you can't set finalizers on: , <nil>"}

The fix for me was to give the operator's ClusterRole permissions to update the memcached CR's finalizers, with the following marker in the controller:

// +kubebuilder:rbac:groups=cache.example.com,resources=memcacheds/finalizers,verbs=get;update;patch
- apiGroups:
  - cache.example.com
  resources:
  - memcacheds/finalizers
  verbs:
  - get
  - patch
  - update

@estroz @camilamacedo86 @joelanford Sorry if I missed it but did we discuss why we don't have the above in our docs and sample memcached controller to make the quickstart guide work by default on openshift (or any cluster with OwnerReferencesPermissionsEnforcement set).

https://sdk.operatorframework.io/docs/building-operators/golang/tutorial/#specify-permissions-and-generate-rbac-manifests

I know there's similar discussion for the same bug for Helm operators in #3767 (comment) which I think is for the same reason.

@hasbro17 hasbro17 reopened this Aug 27, 2020
@hasbro17 hasbro17 added the kind/documentation Categorizes issue or PR as related to documentation. label Aug 27, 2020
@hasbro17
Copy link
Contributor

hasbro17 commented Aug 27, 2020

Wouldn't call this a bug but definitely something to fix in the docs to make our quickstart example work on openshift by default.

@joelanford
Copy link
Member

This came up for the Helm operator as well in #3767

What is it about OpenShift that causes this error that doesn't happen in vanilla Kubernetes?

I'm curious if this is an OpenShift-specific API server customization, or if its an extra admission plugin that is not enabled by default in vanilla Kubernetes?

The reason I ask: Should we push to get this permission added to the default scaffold in Kubebuilder?

@joelanford
Copy link
Member

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

Given that this is built into upstream Kubernetes, I think we could make a case for including this in Kubebuilder's scaffolding. WDYT?

@hasbro17
Copy link
Contributor

Yeah that admission controller seems to be a reasonable default to have in any non-vanilla cluster.
So, not unreasonable to have the kubebuilder controller scaffold have the <kind>/finalizers marker comment as well.

But in the meanwhile we should probably update our docs and sample controller to include a note on this so that our example works on openshift or any non-vanilla cluster.

@joelanford
Copy link
Member

@fckbo
Copy link

fckbo commented Nov 2, 2020

Hello,

fyi, just ran into the same problem generating/installing the Go MemcachedStatus operator provided in https://github.com/operator-framework/operator-sdk/tree/master/testdata with.

operator-sdk version: "v1.1.0", commit: "9d27e224efac78fcc9354ece4e43a50eb30ea968", kubernetes version: "v1.18.2", go version: "go1.15 linux/amd64", GOOS: "linux", GOARCH: "amd64"

and using OCP:

oc version
Client Version: 4.5.17
Server Version: 4.5.17
Kubernetes Version: v1.18.3+45b9524

@camilamacedo86
Copy link
Contributor

Hi @fckbo,

The testdata/go/memcached operator has not the finalizer permission required for it works in OCP:

// +kubebuilder:rbac:groups=cache.example.com,resources=memcacheds/finalizers,verbs=get;update;patch

See:

https://github.com/operator-framework/operator-sdk/blob/master/testdata/go/memcached-operator/controllers/memcached_controller.go#L45-L48

We added this permission in upstream kubernetes-sigs/kubebuilder#1688 but for v3+ plugin which means that it will ONLY be available to SDK when this plugin version is supported here whcih is not the current case.

So, you can add the permission and run make manifests to gen the RBAC, build the project and test it in OCP. However, I think we can mock the permission in the example as well. See; https://github.com/operator-framework/operator-sdk/pull/4162/files.

@fckbo
Copy link

fckbo commented Nov 3, 2020

Hi @camilamacedo86,

thx for your answer, this is what I had done....it worked... and sorry, I actually did not realise that the fix would be included only in a future release. Thx for clarifying.

@camilamacedo86
Copy link
Contributor

It is fine and really thank you for your collaboration 🥇

camilamacedo86 added a commit that referenced this issue Nov 4, 2020
**Description of the change:**
- Add the RBAC finalizer permission to allow users to test the Go sample project on OCP
- See that for v3+ plugins this permission will be added by default 
- See that it was added as a NOTE to the quick start and by default in the tutorial in order to avoid the issue faced in #3477

**Motivation for the change:**
Closes : #3477

Co-authored-by: Joe Lanford <joe.lanford@gmail.com>
reinvantveer pushed a commit to reinvantveer/operator-sdk that referenced this issue Feb 5, 2021
…r-framework#4162)

**Description of the change:**
- Add the RBAC finalizer permission to allow users to test the Go sample project on OCP
- See that for v3+ plugins this permission will be added by default 
- See that it was added as a NOTE to the quick start and by default in the tutorial in order to avoid the issue faced in operator-framework#3477

**Motivation for the change:**
Closes : operator-framework#3477

Co-authored-by: Joe Lanford <joe.lanford@gmail.com>
Signed-off-by: reinvantveer <rein.van.t.veer@geodan.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/documentation Categorizes issue or PR as related to documentation. triage/needs-information Indicates an issue needs more information in order to work on it.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants