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

Support ChannelName RBAC mapping for executors #1023

Merged
merged 14 commits into from
Mar 28, 2023

Conversation

josefkarasek
Copy link

@josefkarasek josefkarasek commented Mar 23, 2023

Description

Changes proposed in this pull request:

  • Support of ChannelName RBAC for executors

Testing

Use executors:

  • helm
  • kubectl

Sources:

  • kubernetes
  • cm-watcher
sources:
  'cm':
    displayName: "Events based on plugin"
    botkube/cm-watcher:
      enabled: true
      config:
        configMap:
          name: cm-watcher-trigger
          namespace: botkube
          event: ADDED
      context: *default-plugin-context

Self-host plugins

make gen-plugins-index
PLUGIN_SERVER_HOST=http://host.k3d.internal go run test/helpers/plugin_server.go
plugins:
  repositories:
    botkube:
      url: http://host.k3d.internal:3000/botkube.yaml

Install botkube:

  • Add ClusterRole and ClusterRoleBinding
  • Add channelName RBAC config to your plugins
botkube/helm:
  context: &default-executor-plugin-context
    defaultNamespace: "default"
    rbac:
      group:
        type: ChannelName
        prefix: ""
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: {{ .Values.rbac.channelGroupName }}
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: {{ .Values.rbac.channelGroupName }}
subjects:
- kind: Group
  name: {{ .Values.rbac.channelGroupName }}
  apiGroup: rbac.authorization.k8s.io
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: {{ .Values.rbac.channelGroupName }}
rules:
- apiGroups:
  - '*'
  resources:
  - '*'
  verbs:
  - get
  - watch
  - list

.Values.rbac.channelGroupName is your slack channel name, in which you intend to test this PR.

# executor kubectl
kubectl get pod -n botkube
# source kubernets
kubectl run nginx --image=nginx
# source cm-watcher
kubectl create cm cm-watcher-trigger -n botkube
# executor helm
helm list
helm install --namespace botkube --repo https://charts.bitnami.com/bitnami psql postgresql

During testing add create/update/delete verbs to ClusterRole {{ .Values.rbac.channelGroupName }}.
Check that you don't have permissions when running botkube in a different channel.

Scenario: 2 channels with different RBAC

Create two socketSlack channels and invite Botkube

socketSlack:
  enabled: true
  channels:
    'ch-1':
      name: ch-1
      bindings:
        executors:
          - helm-1
    'ch-2':
      name: 'ch-2'
      bindings:
        executors:
          - helm-2

Define helm executors

executors:
  'helm-1':
    botkube/helm@v1:
      enabled: true
      context:
        defaultNamespace: "botkube"
        rbac:
          group:
            type: Static
            static:
              values: [*static-group-name]
  'helm-2':
    botkube/helm@v1:
      enabled: true
      context:
        defaultNamespace: "botkube"
        rbac:
          group:
            type: Static
            static:
              values: [helm-editor]

helm-1 uses default read only configuration.
helm-2 uses a custom rbac definition helm-editor:

---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: helm-editor
rules:
  - apiGroups: ["*"]
    resources: ["*"]
    verbs: ["get", "watch", "list", "update", "create", "delete"]
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: helm-editor
  labels:
    app.kubernetes.io/name: helm-editor
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: helm-editor
subjects:
- kind: Group
  name: helm-editor
  apiGroup: rbac.authorization.k8s.io

In channel ch-1 list helm targets @Botkube helm list - this should work, as helm-1 has read scope.
Try to apply a target @Botkube helm install --repo https://charts.bitnami.com/bitnami psql postgresql - this should not work.

Try the same in ch-2 - it should work, because helm2 has create scope.
With @Botkube helm list in ch-1 you should see the postgresql target.

Related issue(s)

#934

@github-advanced-security
Copy link

You have successfully added a new Trivy configuration .github/workflows/vulnerability-scan.yml:scan-repo. As part of the setup process, we have scanned this repository and found 2 existing alerts. Please check the repository Security tab to see all alerts.

defaultNamespace: "default"
# -- RBAC configuration for this plugin.
rbac:
# -- Static impersonation for a given username and groups.
group:
user:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved user before group. I find it more readable this way.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's comment out the user mapping completely from the default configuration.

So the default configuration will be only group mapping (botkube-plugins-read-only) and that's it. And we'll use the "internal" user for impersonation.

@josefkarasek josefkarasek marked this pull request as ready for review March 28, 2023 08:27
@josefkarasek josefkarasek requested a review from a team March 28, 2023 08:27
@josefkarasek josefkarasek requested a review from pkosiec March 28, 2023 08:27
Copy link
Collaborator

@pkosiec pkosiec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, just minor comments. Please clean up the Helm chart according to the comments.
Also, can you provide an instruction in this PR how to test it with multiple channels? So that means just a very quick instruction what (Cluster)Role+(Cluster)RoleBinding resources create for two channels, and see that one command is possible on one, and a different one on the other one.

This will be a good basegroud for working on the RBAC doc. Thanks a lot!

Comment on lines 50 to 51
# default channel in slack & discord is called "general"
channelGroupName: &channel-group-name "general"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just for testing purposes, right? We shouldn't expose such option as it is up to the user to create proper resources. Of course we should document how to create (Cluster)Role + (Cluster)RoleBinding manually as a part of docs.

Comment on lines 46 to 59
# ---
# apiVersion: rbac.authorization.k8s.io/v1
# kind: ClusterRole
# metadata:
# name: {{ .Values.rbac.channelGroupName }}
# labels:
# app.kubernetes.io/name: {{ include "botkube.name" . }}
# helm.sh/chart: {{ include "botkube.chart" . }}
# app.kubernetes.io/instance: {{ .Release.Name }}
# app.kubernetes.io/managed-by: {{ .Release.Service }}
# rules:
# {{- with .Values.rbac.rules }}
# {{- toYaml . | nindent 2 }}
# {{- end }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I wrote before, please remove it.

Comment on lines 37 to 54
# ---
# apiVersion: rbac.authorization.k8s.io/v1
# kind: ClusterRoleBinding
# metadata:
# name: {{ .Values.rbac.channelGroupName }}
# labels:
# app.kubernetes.io/name: {{ include "botkube.name" . }}
# helm.sh/chart: {{ include "botkube.chart" . }}
# app.kubernetes.io/instance: {{ .Release.Name }}
# app.kubernetes.io/managed-by: {{ .Release.Service }}
# roleRef:
# apiGroup: rbac.authorization.k8s.io
# kind: ClusterRole
# name: {{ .Values.rbac.channelGroupName }}
# subjects:
# - kind: Group
# name: {{ .Values.rbac.channelGroupName }}
# apiGroup: rbac.authorization.k8s.io
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I wrote before, please remove it.

defaultNamespace: "default"
# -- RBAC configuration for this plugin.
rbac:
# -- Static impersonation for a given username and groups.
group:
user:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's comment out the user mapping completely from the default configuration.

So the default configuration will be only group mapping (botkube-plugins-read-only) and that's it. And we'll use the "internal" user for impersonation.

@pkosiec pkosiec self-assigned this Mar 28, 2023
@pkosiec pkosiec added the enhancement New feature or request label Mar 28, 2023
# -- Name of user.rbac.authorization.k8s.io the plugin will be bound to.
value: *static-group-name
# -- Name of group.rbac.authorization.k8s.io the plugin will be bound to.
values: [*static-group-name] # "botkube-plugins-read-only" is the default
Copy link
Collaborator

@pkosiec pkosiec Mar 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the comment:

Suggested change
values: [*static-group-name] # "botkube-plugins-read-only" is the default
values: [*static-group-name]

Apply it everywhere 👍

Copy link
Collaborator

@pkosiec pkosiec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! Code LGTM, once I'll finalize the testing I'll give ✅ 🤞

Copy link
Collaborator

@pkosiec pkosiec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works flawlessly 👌 Great job!

@@ -116,25 +116,25 @@ sources:
# -- Describes Kubernetes source configuration.
# @default -- See the `values.yaml` file for full object.
botkube/kubernetes:
context: &defaultPluginContext
context: &default-plugin-context
defaultNamespace: "default"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI, @mszostok will be removing that as this is a part of Helm + Kubectl plugin config, which is already handled. So this field is redundant. You don't need to do anything, but just FYI 🙂

Co-authored-by: Pawel Kosiec <[email protected]>
@pkosiec
Copy link
Collaborator

pkosiec commented Mar 28, 2023

FYI I used the following config for channel-name based mapping:

communications:
  "default-group":
    socketSlack:
      enabled: true
      appToken: "xapp-"
      botToken: "xoxb-"
      notification:
        type: "short"
      channels:
        "default":
          name: botkube-demo
          bindings:
            executors:
              - helm-3
            sources:
              - k8s-all-events
              - k8s-recommendation-events
        "secondary":
          name: priv-channel
          bindings:
            executors:
              - helm-3
            sources:
              - k8s-recommendation-events
              - k8s-err-events

executors:
  'helm-3':
    botkube/helm@v1:
      enabled: true
      context:
        defaultNamespace: "default"
        rbac:
          group:
            type: ChannelName
    botkube/kubectl:
      enabled: true
      # -- Custom kubectl configuration.
      # @default -- See the `values.yaml` file for full object including optional properties related to interactive builder.
      context:
        defaultNamespace: "default"
        rbac:
          group:
            type: ChannelName


plugins:
  repositories:
    botkube:
      url: https://storage.googleapis.com/botkube-plugins-latest/plugins-index.yaml
helm install botkube -n botkube --create-namespace \
--set image.repository=kubeshop/pr/botkube \
--set image.tag=1023-PR \
-f ~/rbac-testing.values.yaml ./helm/botkube

Resources:

---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: priv-channel
rules:
  - apiGroups: ["*"]
    resources: ["*"]
    verbs: ["get", "watch", "list", "update", "create", "delete"]
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: priv-channel
  labels:
    app.kubernetes.io/name: helm-editor
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: priv-channel
subjects:
- kind: Group
  name: priv-channel
  apiGroup: rbac.authorization.k8s.io
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: reader
rules:
  - apiGroups: ["*"]
    resources: ["*"]
    verbs: ["get", "watch", "list"]
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: reader
  labels:
    app.kubernetes.io/name: helm-editor
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: reader
subjects:
- kind: Group
  name: botkube-demo
  apiGroup: rbac.authorization.k8s.io

CC @huseyinbabal

@pkosiec pkosiec changed the title ChannelName RBAC for executors Support ChannelName RBAC mapping for executors Mar 28, 2023
@josefkarasek josefkarasek merged commit 5c209b9 into kubeshop:main Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants