search

openshift / compliance-operator

Operator providing OpenShift cluster compliance checks
Apache License 2.0
110 stars 110 forks source link

Bug 1999374: Use ClusterRole/ClusterRoleBinding for monitoring permissions #694

Closed mrogers950 closed 3 years ago

mrogers950 commented 3 years ago

OLM doesn't support deploying Roles/RoleBindings that reference SAs outside of the operator namespace. To allow for out-of-the-box monitoring when deploying from OLM, this changes the permission objects to a ClusterRole and ClusterRoleBinding.

Also:

openshift-ci[bot] commented 3 years ago

@mrogers950: This pull request references Bugzilla bug 1999374, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug * bug is open, matching expected state (open) * bug target release (4.9.0) matches configured target release for branch (4.9.0) * bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla (pdhamdhe@redhat.com), skipping review request.

In response to [this](https://github.com/openshift/compliance-operator/pull/694): >Bug 1999374: Stage metrics rolebinding for CSV Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
openshift-ci[bot] commented 3 years ago

@mrogers950: This pull request references Bugzilla bug 1999374, which is invalid:

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to [this](https://github.com/openshift/compliance-operator/pull/694): >Bug 1999374: Stage prometheus rolebinding for OLM workaround Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
mrogers950 commented 3 years ago

@JAORMX @jhrozek Some updates after more CSV testing:

Placing the rolebinding directly into manifests/ allows OLM to deploy the rolebinding without stepping on the subject namespace. This is good. Unfortunately, it also means that OLM does not update the reference to the correct role (which is created dynamically by OLM).

$ oc get rolebinding/prometheus-k8s-monitoring -o yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
  creationTimestamp: "2021-09-07T18:33:22Z"
  name: prometheus-k8s-monitoring
  namespace: openshift-compliance
  resourceVersion: "70279"
  uid: a9e2de08-0ce5-4208-a1b5-f00d7c2c086b
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: Role
  name: prometheus-k8s
subjects:
- kind: ServiceAccount
  name: prometheus-k8s
  namespace: openshift-monitoring

$ oc get role/prometheus-k8s
Error from server (NotFound): roles.rbac.authorization.k8s.io "prometheus-k8s" not found

The OLM deployed role is instead:

$ oc get role
NAME                                                            CREATED AT
compliance-operator.v0.1.39                                     2021-09-07T18:33:21Z
compliance-operator.v0.1.39-api-resource-collector-5bfdbcc6d9   2021-09-07T18:33:23Z
compliance-operator.v0.1.39-compliance-operator-748c4bc79b      2021-09-07T18:33:25Z
compliance-operator.v0.1.39-profileparser-74c8c5d475            2021-09-07T18:33:24Z
compliance-operator.v0.1.39-prometheus-k8s-ff6bfd85c            2021-09-07T18:33:24Z  <----
compliance-operator.v0.1.39-remediation-aggregator-565566f444   2021-09-07T18:33:23Z
compliance-operator.v0.1.39-rerunner-6f85c6c95                  2021-09-07T18:33:24Z
compliance-operator.v0.1.39-resultscollector-5476bdc8bc         2021-09-07T18:33:25Z
compliance-operator.v0.1.39-resultserver-5f6bdc4687             2021-09-07T18:33:23Z

It's not possible to patch a rolebinding's roleRef, so the fix from this point is to manually create the Role under the expected name (I added a README blurb for this).

jhrozek commented 3 years ago

@JAORMX @jhrozek Some updates after more CSV testing:

[...]

It's not possible to patch a rolebinding's roleRef, so the fix from this point is to manually create the Role under the expected name (I added a README blurb for this).

I think this is fine at least for now, but I wonder if we're the only ones with this problem. Have you asked the operator-sdk/OLM developers if others have had the same problem as well?

openshift-ci[bot] commented 3 years ago

@mrogers950: This pull request references Bugzilla bug 1999374, which is invalid:

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to [this](https://github.com/openshift/compliance-operator/pull/694): >Bug 1999374: Use ClusterRole/ClusterRoleBinding for monitoring permissions Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
mrogers950 commented 3 years ago

Updated again. With using a ClusterRole/ClusterRoleBinding permission set, it works without requiring a manual step (this is the same approach cluster-logging uses until OLM can support RoleBindings in the way we need)

openshift-ci[bot] commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhrozek, mrogers950

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/openshift/compliance-operator/blob/master/OWNERS)~~ [jhrozek,mrogers950] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
mrogers950 commented 3 years ago

/bugzilla refresh

openshift-ci[bot] commented 3 years ago

@mrogers950: This pull request references Bugzilla bug 1999374, which is valid.

3 validation(s) were run on this bug * bug is open, matching expected state (open) * bug target release (4.10.0) matches configured target release for branch (4.10.0) * bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla (pdhamdhe@redhat.com), skipping review request.

In response to [this](https://github.com/openshift/compliance-operator/pull/694#issuecomment-916158993): >/bugzilla refresh Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
openshift-ci[bot] commented 3 years ago

@mrogers950: All pull requests linked via external trackers have merged:

Bugzilla bug 1999374 has been moved to the MODIFIED state.

In response to [this](https://github.com/openshift/compliance-operator/pull/694): >Bug 1999374: Use ClusterRole/ClusterRoleBinding for monitoring permissions Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.