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.

Sign up for GitHub

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

Jump to bottom

[ENHANCEMENT] Enable setting an encryption key to be used to encrypt/decrypt sensitive data #13

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

[ENHANCEMENT] Enable setting an encryption key to be used to encrypt/decrypt sensitive data #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6e6cb19
Support encryption_key and encryption_key_file values being set
Gabrielopesantos Jan 22, 2024
e5c43cb
Fix errors with names and file paths
Gabrielopesantos Feb 5, 2024
45de2e2
Remove providers from schema json
Gabrielopesantos Feb 18, 2024
18aaacb
Add missing checks to secret to be mount
Gabrielopesantos Feb 18, 2024
209a860
Fix typos in 'if' conditions and move set of encryption_key to its on…
Gabrielopesantos Feb 18, 2024
29efacc
Remove new line at end of values.yaml
Gabrielopesantos Feb 26, 2024
88edc9b
Add encryption key fields with an empty string to values.yaml
Gabrielopesantos Feb 26, 2024
84ccd5d
Enable using an existing secret with to enable encryption of sensitive
Gabrielopesantos Feb 26, 2024
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
2 changes: 1 addition & 1 deletion charts/perses/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ description: Perses helm chart
icon: https://avatars.githubusercontent.com/u/77209215?s=200&v=4
type: application
version: 0.3.0
appVersion: "0.42.1"
appVersion: "0.43.0"
sources:
- https://github.com/perses/perses
annotations:
Expand Down
43 changes: 43 additions & 0 deletions charts/perses/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,46 @@ Create the name of the service account to use
{{- define "perses.dns" -}}
http://{{ include "perses.fullname" . }}.{{ .Release.Namespace }}.svc.cluster.local:{{ .Values.service.targetPort }}
{{- end -}}

{{/*
TODO
*/}}
{{- define "perses.createEncryptionKeyFileSecret" -}}
{{- if (and .Values.config.security.encryptionKey .Values.config.security.encryptionKeyFile) }}
{{- printf "true" }}
{{- else }}
{{- printf "false" }}
{{- end }}
{{- end }}

{{/*
TODO
*/}}
{{- define "perses.mountEncryptionKeyFileSecret" -}}
{{- if or (eq (include "perses.createEncryptionKeyFileSecret" .) "true") .Values.overrideEncryptionKeySecret.secretName }}
{{- printf "true" }}
{{- else }}
{{- printf "false" }}
{{- end }}
{{- end }}

{{/*
TODO
*/}}
{{- define "perses.encryptionKeyVolume" -}}
- name: encryptionkey
secret:
secretName: {{ .Values.overrideEncryptionKeySecret.secretName | default (printf "%s-encryption-key" (include "perses.fullname" .)) | quote }}
items:
- key: {{ .Values.overrideEncryptionKeySecret.secretKey | default "key" | quote }}
path: "key"
{{- end }}

{{/*
TODO
*/}}
{{- define "perses.encryptionKeyVolumeMount" -}}
- name: encryptionkey
mountPath: {{ .Values.config.security.encryptionKeyFile | default "etc/perses/security/encryptionkey" | quote }}
readOnly: true
{{- end }}
10 changes: 8 additions & 2 deletions charts/perses/templates/config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ data:
config.yaml: |-
security:
readonly: {{ .Values.config.security.readOnly }}
{{- if and .Values.config.security.encryptionKeyFile .Values.config.security.encryptionKey }}
Copy link
Member

Choose a reason for hiding this comment

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

not possible as mutually exclusive

encryption_key_file: {{ printf "%s/key" (.Values.config.security.encryptionKeyFile | trimSuffix "/") }}
Copy link
Member

Choose a reason for hiding this comment

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

You're not taking into account your override* fields

{{- end }}
{{- if and (not .Values.config.security.encryptionKeyFile) .Values.config.security.encryptionKey }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I might be wrong, but from my understanding if user provides a encryption key file he must to provide a encryption key.

Is that right @Nexucis ?

Copy link
Author

Choose a reason for hiding this comment

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

That's my understanding as well. But if an encryption key file is provided, and the file is mount on the pod, is there any reason to also add the key in the configuration file, config.yaml?

Copy link
Member

Choose a reason for hiding this comment

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

yeah encryptionKey and encryptionKeyFile are mutually exclusive but they are not mandatory.
If you don't provide it, there is one hardcoded that will be used.

encryption_key: {{ .Values.config.security.encryptionKey }}
{{- end }}
enable_auth: {{ .Values.config.security.enableAuth }}

database:
Expand All @@ -24,12 +30,12 @@ data:
sql:
{{- tpl (toYaml .) $ | nindent 8 }}
{{ end -}}

{{- with .Values.config.important_dashboards }}
important_dashboards:
{{- toYaml . | nindent 6 }}
{{- end }}

{{- with .Values.config.schemas }}
schemas:
{{- toYaml . | nindent 6 }}
Expand Down
17 changes: 17 additions & 0 deletions charts/perses/templates/secrets.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{{- if eq (include "perses.createEncryptionKeyFileSecret" .) "true" }}
apiVersion: v1
kind: Secret
metadata:
name: {{ include "perses.fullname" . }}-encryption-key
labels:
{{- include "perses.labels" . | nindent 4 }}
{{- with .Values.config.annotations }}
annotations:
{{- toYaml . | nindent 4 }}
{{- end }}
type: Opaque
data:
key: {{ .Values.config.security.encryptionKey | b64enc }}
{{- end }}

---
8 changes: 7 additions & 1 deletion charts/perses/templates/statefulset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ spec:
- name: datasources
mountPath: /etc/perses/datasources
{{- end }}
{{- if eq (include "perses.mountEncryptionKeyFileSecret" .) "true" }}
{{- include "perses.encryptionKeyVolumeMount" . | nindent 10 }}
{{- end }}
ports:
- name: http
containerPort: {{ .Values.service.targetPort }}
Expand Down Expand Up @@ -97,4 +100,7 @@ spec:
- name: datasources
configMap:
name: {{ include "perses.fullname" . }}-datasources
{{- end }}
{{- end }}
{{- if eq (include "perses.mountEncryptionKeyFileSecret" .) "true" }}
{{- include "perses.encryptionKeyVolume" . | nindent 8 }}
{{- end }}
22 changes: 22 additions & 0 deletions charts/perses/values.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,14 @@
"type": "boolean",
"default": false
},
"encryptionKey": {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think makes sense add this values to the values.yaml as empty values, just to let users know it exists and is a possibility, as well as because the documentation is being created from the values file.

"type": "string",
"default": ""
},
"encryptionKeyFile": {
"type": "string",
"default": ""
},
"enableAuth": {
"type": "boolean",
"default": false
Expand Down Expand Up @@ -443,6 +451,20 @@
"volumeMounts": {
"type": "array"
},
"overrideEncryptionKeySecret": {
"type": "object",
"additionalProperties": false,
"properties": {
"secretName": {
"type": "string",
"default": ""
},
"secretKey": {
"type": "string",
"default": "key"
}
}
},
"readinessProbe": {
"type": "object",
"additionalProperties": false,
Expand Down
14 changes: 13 additions & 1 deletion charts/perses/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ volumes: []
# -- Additional VolumeMounts on the output StatefulSet definition.
volumeMounts: []


# -- Enable encryption with an existing secret.
# The key that holds that encryption key can also be provided with `secretKey`.
# If not set, `key` is assumed.
overrideEncryptionKeySecret:
# -- SecretName is name of the K8s secret where the encryption key to be used is stored
secretName: ""

# -- Resource limits & requests.
# Update according to your own use case as these values might be too low for a typical deployment.
# ref: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
Expand All @@ -96,6 +104,10 @@ config:
security:
# -- Configure Perses instance as readonly
readOnly: false
# -- Encryption key
encryptionKey: ""
# -- Encryption key file path
encryptionKeyFile: ""
# -- Enable Authentication
enableAuth: false

Expand Down Expand Up @@ -191,4 +203,4 @@ datasources:
# plugin:
# kind: PrometheusDatasource
# spec:
# directUrl: https://prometheus.demo.do.prometheus.io
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit) can we remove this change, just to keep the PR clean as possible?

Copy link
Author

Choose a reason for hiding this comment

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

Sure, my editor added it I don't mind rolling back.

Copy link
Member

Choose a reason for hiding this comment

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

I have also an editor adding newlines in all files. I know that it can be a pain to manage so to me this not that a problem if you don't ^^

# directUrl: https://prometheus.demo.do.prometheus.io
Loading