rhobs / observability-operator

Create Monitoring Stacks you need on your Kubernetes clusters!
32 stars 56 forks source link

Add Clusterrole to allow Korrel8r to view all signal data #517

Closed shwetaap closed 4 months ago

shwetaap commented 4 months ago

This PR creates the required clusterroles and clusterrolebindingss, so that korrel8r can query all the required resources Adds additional clusterrolebinding and rolebinding so that korrel8r can connect to Prometheus endpoints and retrieve relevant data. All of the roles and rolebindings are created within the operator

openshift-ci[bot] commented 4 months ago

Skipping CI for Draft Pull Request. If you want CI signal for your change, please convert it to an actual PR. You can still manually trigger a test run with /test all

danielmellado commented 4 months ago

/assign @alanconway @jgbernalp @periklis

anpingli commented 4 months ago

Test using this PR, my korrel8r was deployed under openshift-operator,, but obo-korrel8r-view bind to operators/obo-korrel8r-sa. it should be openshift-operators/obo-korrel8r-sa. After I fix it manually. The korrel8r works.

operator-sdk run bundle quay.io/anli/observability-operator-bundle:0.0.1-dev \ --install-mode AllNamespaces --namespace openshift-operators --skip-tls


oc get ClusterRoleBinding obo-korrel8r-view -o json
{
    "apiVersion": "rbac.authorization.k8s.io/v1",
    "kind": "ClusterRoleBinding",
    "metadata": {
        "creationTimestamp": "2024-06-24T08:31:00Z",
        "labels": {
            "app.kubernetes.io/part-of": "observability-operator",
            "olm.managed": "true",
            "olm.owner": "observability-operator.v0.0.1-dev"
        },
        "name": "obo-korrel8r-view",
        "resourceVersion": "251006",
        "uid": "65b72a1a-408c-4584-8862-8ede4e4b7d04"
    },
    "roleRef": {
        "apiGroup": "rbac.authorization.k8s.io",
        "kind": "ClusterRole",
        "name": "obo-korrel8r-view"
    },
    "subjects": [
        {
            "kind": "ServiceAccount",
            "name": "obo-korrel8r-sa",
            "namespace": "operators"
        }
    ]
}
jan--f commented 4 months ago

I think we should consider the following points:

  1. If we deploy korrel8r resources that needs certain permissions we should consider adding that to the operator. It doesn't seem like a great user experience to tell a cluster admin to modify/create a resource so that the operator can function properly.
  2. Those permissions should be as minimal as possible while allowing all needed functionality.

So lets pare down the cluster role to what korrel8r needs. Then the role and rolebinding can be created by the controller, making the subject namespace dynamic. Additionally this adds the needed permissions to the operator. The service account can either stay in the bundle (OLM will add the correct namespace) or also move to be created by the controller.

shwetaap commented 4 months ago

@jan--f Made changes to the code as suggested. All (cluster)role and (cluster)rolebinding creation has moved to the operator. The controller now creates the clusterrole needed for korrel8r and creates the clusterrolebinding& rolebinding using existing monitoring clusterrrole and role respectively for allowing access to prometheus and alertmanager endpoints via korrle8r

shwetaap commented 4 months ago

/retest

openshift-ci[bot] commented 4 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: periklis, shwetaap

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/rhobs/observability-operator/blob/main/OWNERS)~~ [periklis] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
jgbernalp commented 4 months ago

/test ci-index-observability-bundle