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

Fix openshift custom host role #56

Merged
merged 4 commits into from Mar 26, 2020

Conversation

eguzki
Copy link
Member

@eguzki eguzki commented Mar 25, 2020

Add role rule routes.route.openshift.io/custom-host to support OCP. Does not affect k8s eventhough it is "openshift" related role.

Tested on:

  • k8s 1.14.10
  • OCP 4.2

An alternative option would be maintain operator metadata different for k8s and OCP "flavors".

The motivation for this change is that when creating an Ingress with an IngressRule that has the 'host' field set in an OpenShift environment the following error is shown:

{"level":"error","ts":1585080246.091675,"logger":"controller_apicast","msg":"Requeuing request...","error":"ingresses.extensions \"apicast-apicast-stage\" is invalid: spec.rules[0]: Forbidden: you do not have permission to set host fields in ingress rules

@miguelsorianod
Copy link
Contributor

miguelsorianod commented Mar 25, 2020

Looking at OpenShift code it seems it has some sort of custom code that includes an Admission Controller for Kubernetes Ingress objects:
https://github.com/openshift/origin/blob/v3.9.0/pkg/ingress/admission/ingress_admission.go#L106

There also seems to be this check for OpenShift Route objects too:
https://github.com/openshift/origin/blob/v3.9.0/pkg/route/registry/route/strategy.go#L104

We had a similar problem with Zync some time ago.

What are the implications of adding that element from a non-admin user point of view? Can we check them?
I seem to remember this permission is not added to the list of default permissions that is given to a non-admin user, which meant that a non-admin user could not deploy that role anymore.

Are we also sure that on k8s native the addition to that permission is not a problem? have this been tried with OLM or a non-admin user? I also seem to remember that permissions problems were not reproduced when deploying the role.yaml as an admin. For example, if you tried to add a non-existing verb for a given Kind/APIGroup and you deployed as admin I seem to remember K8s/OpenShift didn't complain but at the moment this was done as a non-admin the problem surfaced.

@miguelsorianod
Copy link
Contributor

I answer to myself a little bit of the comments I left regarding:

We had a similar problem with Zync some time ago.

What are the implications of adding that element from a non-admin user point of view? Can we check them?
I seem to remember this permission is not added to the list of default permissions that is given to a non-admin user, which meant that a non-admin user could not deploy that role anymore.

Are we also sure that on k8s native the addition to that permission is not a problem? have this been tried with OLM or a non-admin user? I also seem to remember that permissions problems were not reproduced when deploying the role.yaml as an admin. For example, if you tried to add a non-existing verb for a given Kind/APIGroup and you deployed as admin I seem to remember K8s/OpenShift didn't complain but at the moment this was done as a non-admin the problem surfaced.

The problem I was referring to was related to UPDATING the host name and not creating it.
The issue with allowing update (we are not giving that permission at the moment, not even in this PR) was that it allows you to "steal" routes from other elements. See this issue for more detail:
openshift/origin#18177 (comment). So, maybe the route/custom-host for create IS given to default non-admin users but not the update one. Can we check that?
Can we still check what happens when you deploy the role.yaml including the OpenShift reference (routes/custom-host create) as a non-admin user in a K8s native cluster?

@eguzki
Copy link
Member Author

eguzki commented Mar 25, 2020

Tested reconcilliation and added required role rules.

@miguelsorianod
Copy link
Contributor

miguelsorianod commented Mar 26, 2020

Are we also sure that on k8s native the addition to that permission is not a problem? have this been tried with OLM or a non-admin user? I also seem to remember that permissions problems were not reproduced when deploying the role.yaml as an admin. For example, if you tried to add a non-existing verb for a given Kind/APIGroup and you deployed as admin I seem to remember K8s/OpenShift didn't complain but at the moment this was done as a non-admin the problem surfaced.

I've tried the following example in an OpenShift cluster:
The following role can be deployed as a "admin" (but not cluster-admin) user:
Role content:

kind: Role
apiVersion: rbac.authorization.k8s.io/v1
metadata:
  name: simplerole
rules:
- apiGroups:
  - ""
  resources:
  - pods
  verbs:
  - create
  - delete
  - get
  - list
  - patch
  - update
  - watch

But, when adding a non existing APIGroup/Kind into the role an error is received when the user tries to create it:
Role content:

msoriano@localhost:~/go/src/github.com/3scale/testscaf2/deploy$ cat simplerole.yaml 
kind: Role
apiVersion: rbac.authorization.k8s.io/v1
metadata:
  name: simplerole
rules:
- apiGroups:
  - ""
  resources:
  - pods
  verbs:
  - create
  - delete
  - get
  - list
  - patch
  - update
  - watch
- apiGroups:
  - myapigroup.com
  resources:
  - mykind
  verbs:
  - create

Role creation attempt:

msoriano@localhost:~/go/src/github.com/3scale/testscaf2/deploy$ oc create -f simplerole.yaml 
Error from server (Forbidden): error when creating "simplerole.yaml": roles.rbac.authorization.k8s.io "simplerole" is forbidden: user "anonadminuser" (groups=["system:authenticated:oauth" "system:authenticated"]) is attempting to grant RBAC permissions not currently held:
{APIGroups:["myapigroup.com"], Resources:["mykind"], Verbs:["create"]}

However, if the user that tries to create the role is a cluster-admin, NO error is shown even when the APIGroup/Kind does not exist in the cluster, what confirms what I thought:
Role creation attempt:

msoriano@localhost:~/go/src/github.com/3scale/testscaf2/deploy$ oc create -f simplerole.yaml 
role.rbac.authorization.k8s.io/simplerole created

@eguzki
Copy link
Member Author

eguzki commented Mar 26, 2020

Some findings:
Operators can only be deployed by cluster admin users. https://access.redhat.com/documentation/en-us/openshift_container_platform/4.2/html/operators/olm-understanding-operatorhub

Namespace admin users should be able to deploy apicast instances

The new role definition in this PR can be deployed using OLM. Tested in OCP 4.2

@miguelsorianod
Copy link
Contributor

miguelsorianod commented Mar 26, 2020

As you commented, to deploy operator.yaml you have to be cluster-admin so I understand we can go forward with including the OpenShift permissions in a single file, even deploying in K8s where the API/Kind of those permissions does not exist, because it is ignored when you are cluster admin as I showed in OpenShift case.
I understand you tried the same in K8s and you verified the errors were ignored when referencing OpenShift APIGroups/Kinds

@miguelsorianod miguelsorianod merged commit 0fb6754 into master Mar 26, 2020
@eguzki eguzki deleted the fix-openshift-custom-host-role branch March 26, 2020 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants