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

controllers: kernel mount options for cephfs Encryption in transit #185

Closed
wants to merge 1 commit into from

Conversation

rohan47
Copy link
Contributor

@rohan47 rohan47 commented Jul 19, 2024

support encryption in transit for cephfs when kernel mount option is set to secure. This information is recieved from provider and processed during storage claim creation.

The PR on OCS operator side is: red-hat-storage/ocs-operator#2707

Copy link

openshift-ci bot commented Jul 19, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rohan47
Once this PR has been reviewed and has the lgtm label, please assign leelavg for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rohan47 rohan47 marked this pull request as draft July 19, 2024 10:20
@rohan47 rohan47 marked this pull request as ready for review July 20, 2024 23:32
@rohan47 rohan47 force-pushed the eit branch 2 times, most recently from 9c87d09 to a6d6a1b Compare July 22, 2024 14:06
@Madhu-1
Copy link
Member

Madhu-1 commented Jul 22, 2024

LGTM

support encryption in transit for cephfs when kernel mount option is
set to secure. This information is recieved from provider and processed
during storage claim creation.

Signed-off-by: Rohan Gupta <rohgupta@redhat.com>
@rohan47
Copy link
Contributor Author

rohan47 commented Jul 22, 2024

Tested the code changes

Csi config after the changes

oc get cm ceph-csi-configs -o yaml
apiVersion: v1
data:
  config.json: '[{"clusterID":"81fd58def37a35d1b4d4bee7ba35c3c3","storageClientID":"fb3e4f11-a38b-4859-ad18-1e8d579eb72b",
"monitors":["10.0.16.90:3300","10.0.61.203:3300","10.0.59.136:3300"],"rbd":{"radosNamespace":"cephradosnamespace-f6e94668817e4eefeb19b7a402fd947e"}},{"clusterID":"fe09c94a50c72d033da43478c21b8314",
"storageClientID":"fb3e4f11-a38b-4859-ad18-1e8d579eb72b","monitors":["10.0.16.90:3300","10.0.61.203:3300","10.0.59.136:3300"],
"cephFS":{"subvolumeGroup":"cephfilesystemsubvolumegroup-38beca789f5bc10e556b9832fd1070f6",
"kernelMountOptions":"ms_mode=secure"}}]'
kind: ConfigMap
metadata:
  creationTimestamp: "2024-07-22T19:24:10Z"
  name: ceph-csi-configs
  namespace: openshift-storage
  ownerReferences:
  - apiVersion: v1
    blockOwnerDeletion: true
    controller: true
    kind: ConfigMap
    name: ocs-client-operator-config
    uid: 664e804f-fc77-4aef-9387-a3c39dcb0b00
  resourceVersion: "3135818"
  uid: 7dee1733-974c-48f3-9cc7-7dbab17aee62

@@ -380,6 +382,9 @@ func (r *StorageClaimReconciler) reconcilePhases() (reconcile.Result, error) {
csiClusterConfigEntry.CephFS.SubvolumeGroup = data["subvolumegroupname"]
// delete groupname from data as its not required in storageclass
delete(data, "subvolumegroupname")
csiClusterConfigEntry.CephFS.KernelMountOptions = data[kernelMountOptionsKey]
// delete kernelmountoptions from data as its not required in storageclass
delete(data, kernelMountOptionsKey)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not piggyback information destination for one resource (csi configmap) as part of another resource (storage class) to send CSI config data. The two should not be coupled and the fact that you need to delete pieces of information from the incoming data proves you are using the wrong approach.

Please send all of the CSI config information as its own resource (a configmap with a config.json key) and handle it in a separate switch case! (similar to other resources we are sending from the provider). As a rule of thumb, each switch case need to be able to act independently!

@leelavg please make sure @rohan47's contribution here aligns with the proper way we set to send information from provider to client.

Copy link
Contributor

Choose a reason for hiding this comment

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

@nb-ohad the existing mechanism is also sending data as part of storageclass that corresponds to it and we pick the interested items to configmap and this is also following the same.

@rohan47
Copy link
Contributor Author

rohan47 commented Jul 24, 2024

The feature of this PR is to be delivered as part of the csi-operator changes and is no longer required.

@rohan47 rohan47 closed this Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants