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

[Action Platform PAR] Use actionsAllowlist to configure RBAC #1518

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

xornivore
Copy link
Contributor

@xornivore xornivore commented Sep 5, 2024

What this PR does / why we need it:

Simplified user configuration for Private Action Runner RBAC in ClusterRole that uses the following:

  • Uses explicitly set RBAC rules in kubernetesPermissions
  • Additively configures RBAC rules based on the contents of actionsAllowList.

How to test:

  • In values add or uncomment com.datadoghq.kubernetes.apps.updateDeployment in actionsAllowlist
  • Render the helm chart with helm template ./

Expected ClusterRole:

# Source: private-action-runner/templates/role.yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  namespace: process
  name:  "private-action-runner-default-role"
rules:
- apiGroups:
  - ""
  resources:
  - pods
  verbs:
  - list
  - get
- apiGroups:
  - apps
  resources:
  - deployments
  verbs:
  - list
  - get
- apiGroups:
  - apps
  resources:
  - deployment
  verbs:
  - update
---

Note that this is implementation is currently limited to the following actions from actionsAllowlist:

  • getDeployment
  • listDeployment
  • patchDeployment
  • restartDeployment

Special notes for your reviewer:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Chart Version bumped
  • Documentation has been updated with helm-docs (run: .github/helm-docs.sh)
  • CHANGELOG.md has been updated
  • Variables are documented in the README.md
  • For Datadog Operator chart or value changes update the test baselines (run: make update-test-baselines)

@xornivore xornivore changed the title WIP: PoC of using actionsAllowlist in ClusterRole [Action Platform PAR] Use actionsAllowlist to configure RBAC Sep 6, 2024
@xornivore xornivore marked this pull request as ready for review September 6, 2024 18:33
@xornivore xornivore requested a review from a team as a code owner September 6, 2024 18:33
Comment on lines +22 to +34
{{/*
Defines an RBAC "get" rule for provided apiGroup and resource type
*/}}
{{- define "rbacGetRule" }}
{{- include "rbacRule" (dict "apiGroup" .apiGroup "resource" .resource "verbs" (list "get"))}}
{{- end }}

{{/*
Defines an RBAC "list" rule for provided apiGroup and resource type
*/}}
{{- define "rbacListRule" }}
{{- include "rbacRule" (dict "apiGroup" .apiGroup "resource" .resource "verbs" (list "list"))}}
{{- end }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

It depends how much granularity you want, but maybe get and list can be define together. Same for patch and update

{{- include "rbacPatchRule" (dict "apiGroup" "apps" "resource" "deployments")}}
{{- end }}
{{- if has "com.datadoghq.kubernetes.apps.restartDeployment" $runner.config.actionsAllowlist }}
{{- include "rbacUpdateRule" (dict "apiGroup" "apps" "resource" "deployments")}}
Copy link
Contributor

Choose a reason for hiding this comment

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

restartDeployment is actually using patch under the hood

@@ -1,5 +1,9 @@
# Datadog changelog

### 0.9.1
Copy link
Contributor

@dd-gplassard dd-gplassard Sep 10, 2024

Choose a reason for hiding this comment

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

can you merge / rebase from main ? there was another release in the meantime

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants