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

helm: support encryption config in ceph-csi-cephfs chart #4531

Open
wants to merge 3 commits into
base: devel
Choose a base branch
from
Open
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
31 changes: 31 additions & 0 deletions charts/ceph-csi-cephfs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,33 @@ version.
We recommend not to use `--reuse-values` in case there are new defaults AND
compare your currently used values with the new default values.

### Enabling encryption support

To enable FSCrypt support, you will need to include the KMS configuration in
`encryptionKMSConfig`.

Here is a `values.yaml` example using a Kubernetes secret (`kubernetes` KMS)

```yaml
encryptionKMSConfig:
encryptionKMSType: "metadata"
secretName: "cephfs-encryption-passphrase" # This secret needs to contain the passphrase as the key `encryptionPassphrase`
secretNamespace: "my-namespace"
storageClass:
encrypted: true
encryptionKMSID: kubernetes
```

#### Least privilege secret access

If you use the `metadata` and let RBAC created by the chart, permissions
will be given to access **only** the secret referenced in the
`encryptionKMSConfig`. This is something important to keep in mind, as a
manual change to the config to point to another secret or add further KMS
config will not be authorized. If you wish to give CephCSI a global secret
access to the cluster, you may set `rbac.leastPrivileges` to `false`, and
permissions will be granted globally via a *ClusterRole*.

#### Known Issues Upgrading

- When upgrading to version >=3.7.0, you might encounter an error that the
Expand Down Expand Up @@ -110,11 +137,13 @@ charts and their default values.
| Parameter | Description | Default |
| ---------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------- |
| `rbac.create` | Specifies whether RBAC resources should be created | `true` |
| `rbac.leastPrivileges` | Specifies whether RBAC resources should be created with a restricted scope when supported (only secrets supported currently) | `true` |
| `serviceAccounts.nodeplugin.create` | Specifies whether a nodeplugin ServiceAccount should be created | `true` |
| `serviceAccounts.nodeplugin.name` | The name of the nodeplugin ServiceAccount to use. If not set and create is true, a name is generated using the fullname | "" |
| `serviceAccounts.provisioner.create` | Specifies whether a provisioner ServiceAccount should be created | `true` |
| `serviceAccounts.provisioner.name` | The name of the provisioner ServiceAccount of provisioner to use. If not set and create is true, a name is generated using the fullname | "" |
| `csiConfig` | Configuration for the CSI to connect to the cluster | [] |
| `encryptionKMSConfig` | Configuration for the encryption KMS | `{}` |
| `commonLabels` | Labels to apply to all resources | `{}` |
| `logLevel` | Set logging level for csi containers. Supported values from 0 to 5. 0 for general useful logs, 5 for trace level verbosity. | `5` |
| `sidecarLogLevel` | Set logging level for csi sidecar containers. Supported values from 0 to 5. 0 for general useful logs, 5 for trace level verbosity. | `1` |
Expand Down Expand Up @@ -184,6 +213,8 @@ charts and their default values.
| `storageClass.name` | Specifies the cephFS StorageClass name | `csi-cephfs-sc` |
| `storageClass.annotations` | Specifies the annotations for the cephFS storageClass | `[]` |
| `storageClass.clusterID` | String representing a Ceph cluster to provision storage from | `<cluster-ID>` |
| `storageClass.encrypted` | Specifies whether volume should be encrypted. Set it to true if you want to enable encryption | `""` |
| `storageClass.encryptionKMSID` | Specifies the encryption kms id | `""` |
| `storageClass.fsName` | CephFS filesystem name into which the volume shall be created | `myfs` |
| `storageClass.pool` | Ceph pool into which volume data shall be stored | `""` |
| `storageClass.fuseMountOptions` | Comma separated string of Ceph-fuse mount options | `""` |
Expand Down
15 changes: 15 additions & 0 deletions charts/ceph-csi-cephfs/templates/encryptionkms-configmap.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ .Values.kmsConfigMapName | quote }}
namespace: {{ .Release.Namespace }}
labels:
app: {{ include "ceph-csi-cephfs.name" . }}
chart: {{ include "ceph-csi-cephfs.chart" . }}
component: {{ .Values.nodeplugin.name }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
{{- with .Values.commonLabels }}{{ toYaml . | trim | nindent 4 }}{{- end }}
data:
config.json: |-
{{ toJson .Values.encryptionKMSConfig | indent 4 -}}
4 changes: 4 additions & 0 deletions charts/ceph-csi-cephfs/templates/nodeplugin-clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,14 @@ rules:
- apiGroups: [""]
resources: ["nodes"]
verbs: ["get"]
# allow to read Vault Token and connection options from the Tenants namespace
- apiGroups: [""]
resources: ["configmaps"]
verbs: ["get"]
{{- if and .Values.encryptionKMSConfig .Values.encryptionKMSConfig.secretNamespace (not .Values.rbac.leastPrivileges) }}
# allow to read the encryption key used with the metadata KMS
- apiGroups: [""]
resources: ["secrets"]
verbs: ["get"]
{{- end -}}
{{- end -}}
22 changes: 22 additions & 0 deletions charts/ceph-csi-cephfs/templates/nodeplugin-role.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{{- if and .Values.rbac.create .Values.rbac.leastPrivileges -}}
{{- if and .Values.encryptionKMSConfig (eq .Values.encryptionKMSConfig.encryptionKMSType "metadata") .Values.encryptionKMSConfig.secretNamespace .Values.encryptionKMSConfig.secretName -}}
kind: Role
Copy link
Collaborator

Choose a reason for hiding this comment

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

we dont need Role and Role Binding right? we need to have access to secrets in different namespaces as well. i don't see Role https://github.com/ceph/ceph-csi/tree/devel/deploy/cephfs/kubernetes, am i missing anything?

Copy link
Author

Choose a reason for hiding this comment

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

The idea was to downgrade the ClusterRole to just a Role in case the configured secret in encryptionKMSConfig was local to the namespace. This was done to help with least privileged and limit the access to secret as much as possible. Happy to remove if you think this is overkill. Arguably, users who want least privileged roles can create manage the RBAC themselves

Copy link
Member

Choose a reason for hiding this comment

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

Ceph-CSI components which reads the Secrets are usually running in a different namespace than the namespace where Secrets for applications have their encryption key. Ceph-CSI needs to read the Secret from a different namespace than where it is running/mounting the PV.

Copy link
Author

@acolombier acolombier Apr 18, 2024

Choose a reason for hiding this comment

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

In this case, it will create the ClusterRole automatically. The logic at line 2 is if encryptionKMSConfig.secretNamespace == Release.Namespace, then create a Role instead of adding secret:read to the ClusterRole.

Copy link
Author

Choose a reason for hiding this comment

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

Happy to scratch that optimisation, and create RBAC myself! But just thought it would be a harmless optimisation to put in

Copy link
Author

Choose a reason for hiding this comment

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

I have slightly reworked the logic behind it to enforce determinism and added a some documentation in the REAMDE.md. I'd be nice if you could tell me how that looks, and if you like it, I can spin up a PR with the same feature for RBD.

Copy link
Author

Choose a reason for hiding this comment

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

@nixpanic it'd be great if you could let me know your though on this

Copy link
Collaborator

Choose a reason for hiding this comment

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

@acolombier @nixpanic is on PTO will be back 1st week of Jun, do you want me to block this PR until he is back to confirm on this?

Copy link
Author

Choose a reason for hiding this comment

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

Up to you! If it is ready to be merged, happy to do so too, and I'll wait and see if @nixpanic likes this approach for RBD and I can make the PR then

Copy link
Author

Choose a reason for hiding this comment

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

@nixpanic friendly ping in case you missed the notification :)

apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: {{ include "ceph-csi-cephfs.nodeplugin.fullname" . }}
namespace: {{ .Values.encryptionKMSConfig.secretNamespace }}
labels:
app: {{ include "ceph-csi-cephfs.name" . }}
chart: {{ include "ceph-csi-cephfs.chart" . }}
component: {{ .Values.nodeplugin.name }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
{{- with .Values.commonLabels }}{{ toYaml . | trim | nindent 4 }}{{- end }}
rules:
# allow to read the encryption key used with the metadata KMS
- apiGroups: [""]
resources: ["secrets"]
verbs: ["get"]
resourceNames: [{{ .Values.encryptionKMSConfig.secretName | quote }}]
{{- end -}}
{{- end -}}
24 changes: 24 additions & 0 deletions charts/ceph-csi-cephfs/templates/nodeplugin-rolebinding.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{{- if and .Values.rbac.create .Values.rbac.leastPrivileges -}}
{{- if and .Values.encryptionKMSConfig (eq .Values.encryptionKMSConfig.encryptionKMSType "metadata") .Values.encryptionKMSConfig.secretNamespace -}}
kind: RoleBinding
apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: {{ include "ceph-csi-cephfs.nodeplugin.fullname" . }}
namespace: {{ .Values.encryptionKMSConfig.secretNamespace }}
labels:
app: {{ include "ceph-csi-cephfs.name" . }}
chart: {{ include "ceph-csi-cephfs.chart" . }}
component: {{ .Values.nodeplugin.name }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
{{- with .Values.commonLabels }}{{ toYaml . | trim | nindent 4 }}{{- end }}
subjects:
- kind: ServiceAccount
name: {{ include "ceph-csi-cephfs.serviceAccountName.nodeplugin" . }}
namespace: {{ .Release.Namespace }}
roleRef:
kind: Role
name: {{ include "ceph-csi-cephfs.nodeplugin.fullname" . }}
apiGroup: rbac.authorization.k8s.io
{{- end -}}
{{- end -}}
6 changes: 6 additions & 0 deletions charts/ceph-csi-cephfs/templates/storageclass.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ parameters:
{{- if .Values.storageClass.pool }}
pool: {{ .Values.storageClass.pool }}
{{- end }}
{{- if .Values.storageClass.encrypted }}
encrypted: "{{ .Values.storageClass.encrypted }}"
{{- end }}
{{- if .Values.storageClass.encryptionKMSID }}
encryptionKMSID: {{ .Values.storageClass.encryptionKMSID }}
{{- end }}
{{- if .Values.storageClass.fuseMountOptions }}
fuseMountOptions: "{{ .Values.storageClass.fuseMountOptions }}"
{{- end }}
Expand Down
31 changes: 31 additions & 0 deletions charts/ceph-csi-cephfs/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
rbac:
# Specifies whether RBAC resources should be created
create: true
# When possible try and reduce the scope of permission to only give
# access to resources defined in the config. See the README for more info
leastPrivileges: true

serviceAccounts:
nodeplugin:
Expand Down Expand Up @@ -31,6 +34,20 @@ serviceAccounts:
# radosNamespace: "csi"
csiConfig: []

# Configuration for the encryption KMS
# yamllint disable-line rule:line-length
# Ref: https://github.com/ceph/ceph-csi/blob/devel/docs/deploy-cephfs.md#cephfs-volume-encryption
acolombier marked this conversation as resolved.
Show resolved Hide resolved
# Example:
# encryptionKMSConfig:
# encryptionKMSType: vault
# vaultAddress: https://vault.example.com
# vaultAuthPath: /v1/auth/kubernetes/login
# vaultRole: csi-kubernetes
# vaultPassphraseRoot: /v1/secret
# vaultPassphrasePath: ceph-csi/
# vaultCAVerify: "true"
encryptionKMSConfig: {}

# Labels to apply to all resources
commonLabels: {}

Expand Down Expand Up @@ -329,6 +346,18 @@ storageClass:
# If omitted, defaults to "csi-vol-".
# volumeNamePrefix: "foo-bar-"
volumeNamePrefix: ""

# (optional) Instruct the plugin it has to encrypt the volume
# By default it is disabled. Valid values are "true" or "false".
# A string is expected here, i.e. "true", not true.
# encrypted: "true"
encrypted: ""

# (optional) Use external key management system for encryption passphrases by
# specifying a unique ID matching KMS ConfigMap. The ID is only used for
# correlation to configmap entry.
encryptionKMSID: ""

# The secrets have to contain user and/or Ceph admin credentials.
provisionerSecret: csi-cephfs-secret
# If the Namespaces are not specified, the secrets are assumed to
Expand Down Expand Up @@ -400,6 +429,8 @@ configMapName: ceph-csi-config
externallyManagedConfigmap: false
# Name of the configmap used for ceph.conf
cephConfConfigMapName: ceph-config
# Name of the configmap used for encryption kms configuration
kmsConfigMapName: ceph-csi-encryption-kms-config
# CephFS RadosNamespace used to store CSI specific objects and keys.
# radosNamespaceCephFS: csi
# Unique ID distinguishing this instance of Ceph CSI among other instances,
Expand Down