Skip to content
This repository was archived by the owner on May 16, 2023. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions kibana/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,16 @@ heritage: {{ .Release.Service }}
{{ toYaml .Values.labels }}
{{- end }}
{{- end -}}

{{/*
Use the fullname if the serviceAccount value is not set
*/}}
{{- define "kibana.serviceAccount" -}}
{{- if and .Values.rbac.serviceAccountName ( not ( eq .Values.podSecurityPolicy.name "" ) ) -}}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why are you checking podSecurityPolicy.name value if you're not using kibana.serviceAccount in kibana/templates/podsecuritypolicy.yaml?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would prefer removing kibana.serviceAccount template and keeping ClusterRole, ClusterRoleBinding and ServiceAccount name consistent with what we have in most other charts (you can check Logstash chart for that).

{{- .Values.rbac.serviceAccountName -}}
{{- else }}
{{- $name := default .Chart.Name .Values.nameOverride -}}
{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" -}}
{{- end -}}
{{- end -}}

21 changes: 21 additions & 0 deletions kibana/templates/clusterrole.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{{- if .Values.managedServiceAccount }}
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRole
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we need ClusterRole and ClusterRoleBinding for Kibana chart, using a Role and RoleBinding should be enough

metadata:
name: {{ template "kibana.serviceAccount" . }}-cluster-role
labels:
app: "{{ template "kibana.fullname" . }}"
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
heritage: {{ .Release.Service | quote }}
release: {{ .Release.Name | quote }}
rules:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

a specific rule for podsecuritypolicies resource is required similar to #496 (review)

- apiGroups:
- ""
resources:
- namespaces
- pods
verbs:
- get
- list
- watch
{{- end -}}
19 changes: 19 additions & 0 deletions kibana/templates/clusterrolebinding.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{{- if .Values.managedServiceAccount }}
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRoleBinding
metadata:
name: {{ template "kibana.serviceAccount" . }}-cluster-role-binding
labels:
app: "{{ template "kibana.fullname" . }}"
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
heritage: {{ .Release.Service | quote }}
release: {{ .Release.Name | quote }}
roleRef:
kind: ClusterRole
name: {{ template "kibana.serviceAccount" . }}-cluster-role
apiGroup: rbac.authorization.k8s.io
subjects:
- kind: ServiceAccount
name: {{ template "kibana.serviceAccount" . }}
namespace: {{ .Release.Namespace }}
{{- end -}}
4 changes: 2 additions & 2 deletions kibana/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ spec:
{{- end }}
securityContext:
{{ toYaml .Values.podSecurityContext | indent 8 }}
{{- if .Values.serviceAccount }}
serviceAccount: {{ .Values.serviceAccount }}
{{- if .Values.rbac.serviceAccountName }}
serviceAccount: {{ .Values.rbac.serviceAccountName }}
Comment on lines +36 to +37
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

a default ServiceAccount name should be used if serviceAccountName isn't defined. Here is an example from Logstash chart:

Suggested change
{{- if .Values.rbac.serviceAccountName }}
serviceAccount: {{ .Values.rbac.serviceAccountName }}
{{- if .Values.rbac.create }}
serviceAccountName: "{{ template "kibana.fullname" . }}"
{{- else if not (eq .Values.rbac.serviceAccountName "") }}
serviceAccountName: {{ .Values.rbac.serviceAccountName | quote }}

{{- end }}
volumes:
{{- range .Values.secretMounts }}
Expand Down
14 changes: 14 additions & 0 deletions kibana/templates/podsecuritypolicy.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{{- if .Values.podSecurityPolicy.create -}}
{{- $fullName := include "kibana.fullname" . -}}
apiVersion: policy/v1beta1
kind: PodSecurityPolicy
metadata:
name: {{ default $fullName .Values.podSecurityPolicy.name | quote }}
labels:
heritage: {{ .Release.Service | quote }}
release: {{ .Release.Name | quote }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
app: {{ $fullName | quote }}
spec:
{{ toYaml .Values.podSecurityPolicy.spec | indent 2 }}
{{- end -}}
11 changes: 11 additions & 0 deletions kibana/templates/serviceaccount.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{{- if .Values.managedServiceAccount }}
apiVersion: v1
kind: ServiceAccount
metadata:
name: {{ template "kibana.serviceAccount" . }}
labels:
app: "{{ template "kibana.fullname" . }}"
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
heritage: {{ .Release.Service | quote }}
release: {{ .Release.Name | quote }}
{{- end -}}
3 changes: 2 additions & 1 deletion kibana/tests/kibana_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,8 @@ def test_using_a_name_override():

def test_setting_a_custom_service_account():
config = """
serviceAccount: notdefault
rbac:
serviceAccountName: notdefault
"""
r = helm_template(config)
assert (
Expand Down
27 changes: 26 additions & 1 deletion kibana/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,32 @@ securityContext:
runAsNonRoot: true
runAsUser: 1000

serviceAccount: ""
# Whether this chart should self-manage its service account, role, and associated role binding.
managedServiceAccount: true

rbac:
create: false
serviceAccountName: ""

podSecurityPolicy:
create: false
name: ""
spec:
privileged: true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

privileged: true isn't required for kibana

fsGroup:
rule: RunAsAny
runAsUser:
rule: RunAsAny
seLinux:
rule: RunAsAny
supplementalGroups:
rule: RunAsAny
volumes:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's add downwardAPI and emptyDir like in #496 (comment)

- secret
- configMap
- persistentVolumeClaim
- projected
- emptyDir

# This is the PriorityClass settings as defined in
# https://kubernetes.io/docs/concepts/configuration/pod-priority-preemption/#priorityclass
Expand Down