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

Add info alert for In-transit Encryption in external mode #1647

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

malayparida2000
Copy link
Contributor

@malayparida2000 malayparida2000 commented Oct 21, 2024

https://issues.redhat.com/browse/RHSTOR-6581

The configurations for In-Transit Encryption in external mode are all required to be done on the RHCS cluster by the ceph admin. Ticking the checkbox on ODF side only sets the kernel mount option for cephFS. So it might be misleading the user to think, the checkbox is enough to enable Eit in external mode. So added an info alert under the checkbox to clarify that settings need to be applied on the RHCS cluster. Also reworded the description of the checkbox to make it more clear to the user.

@malayparida2000
Copy link
Contributor Author

/cc @travisn

@openshift-ci openshift-ci bot requested a review from travisn October 21, 2024 06:58
@SanjalKatiyar
Copy link
Collaborator

SanjalKatiyar commented Oct 21, 2024

We need few more changes now if we are removing in-transit from external mode:

  1. Step is currently called Security and network, we need to talk to Eran/Chandan and rename this to maybe just Network now (if needed). Please make sure they are in loop.
  2. Also need to remove details from here: https://github.com/red-hat-storage/odf-console/blob/master/packages/odf/components/create-storage-system/create-storage-system-steps/review-and-create-step/review-and-create-step.tsx#L231-L239.

@parth-gr
Copy link
Member

@malayparida2000 i think we dont need to for internal mode also, as we made this by-default in both the modes downstream

@malayparida2000
Copy link
Contributor Author

@malayparida2000 i think we dont need to for internal mode also, as we made this by-default in both the modes downstream

No, Actually we made RequireMSGR2 default, Not Encryption. Encryption in transit is still an option to be choosen.

@SanjalKatiyar
Copy link
Collaborator

instead the instructions should be covered by docs >> @malayparida2000 can you please share the steps ?? this change could/might have an impact on another epic: https://issues.redhat.com/browse/RHSTOR-5286

@malayparida2000
Copy link
Contributor Author

malayparida2000 commented Oct 21, 2024

instead the instructions should be covered by docs >> @malayparida2000 can you please share the steps ?? this change could/might have an impact on another epic: https://issues.redhat.com/browse/RHSTOR-5286

For enabling Encryption for External Mode
On RHCS Cluster-

# ceph config set global ms_client_mode secure
# ceph config set global ms_cluster_mode secure
# ceph config set global ms_service_mode secure
# ceph config set global rbd_default_map_options ms_mode=secure

Restart all ceph daemons (mgr, mon, osd, mds etc)

On ODF Cluster-
storagecluster CR

network:
  connections:
      encryption:
        enabled: true
        
 oc patch storagecluster ocs-external-storagecluster -n openshift-storage --type json --patch  '[{ "op": "replace", "path": "/spec/network", "value": {"connections": {"encryption": {"enabled": true}}} }]'

@malayparida2000
Copy link
Contributor Author

We need few more changes now if we are removing in-transit from external mode:

  1. Step is currently called Security and network, we need to talk to Eran/Chandan and rename this to maybe just Network now (if needed). Please make sure they are in loop.
  2. Also need to remove details from here: https://github.com/red-hat-storage/odf-console/blob/master/packages/odf/components/create-storage-system/create-storage-system-steps/review-and-create-step/review-and-create-step.tsx#L231-L239.

Along with [1] removed another change I found.

@malayparida2000
Copy link
Contributor Author

malayparida2000 commented Oct 21, 2024

As I see we have 2 options,
1-Removing the UI checkbox from External Mode deployment flow, covering it via only doc to do via CLI only
2-Adding a tooltip or note near the checkbox which clarifies that other settings need to be applied on the RHCS cluster to achieve Encryption In Transit

@SanjalKatiyar
Copy link
Collaborator

instead the instructions should be covered by docs >> @malayparida2000 can you please share the steps ?? this change could/might have an impact on another epic: https://issues.redhat.com/browse/RHSTOR-5286

For enabling Encryption for External Mode On RHCS Cluster-

# ceph config set global ms_client_mode secure
# ceph config set global ms_cluster_mode secure
# ceph config set global ms_service_mode secure
# ceph config set global rbd_default_map_options ms_mode=secure

Restart all ceph daemons (mgr, mon, osd, mds etc)

On ODF Cluster- storagecluster CR

network:
  connections:
      encryption:
        enabled: true
        
 oc patch storagecluster ocs-storagecluster -n openshift-storage --type json --patch  '[{ "op": "replace", "path": "/spec/network", "value": {"connections": {"encryption": {"enabled": true}}} }]'

steps seems fairly straightforward to me, instead of removing checkbox from the external mode UI we can also consider just adding link to pre/post-requisite CLI steps (doc) and adding an info message on the UI. @malayparida2000 can we plz discuss this with Eran/Chandan ?? Or, is it already discussed and decided to move forward with removal of the checkbox ??

@SanjalKatiyar
Copy link
Collaborator

SanjalKatiyar commented Oct 21, 2024

instead the instructions should be covered by docs >> @malayparida2000 can you please share the steps ?? this change could/might have an impact on another epic: https://issues.redhat.com/browse/RHSTOR-5286

based on the steps shared here: #1647 (comment), looks like RHSTOR-5286 is not affected.

@malayparida2000
Copy link
Contributor Author

malayparida2000 commented Oct 21, 2024

instead the instructions should be covered by docs >> @malayparida2000 can you please share the steps ?? this change could/might have an impact on another epic: https://issues.redhat.com/browse/RHSTOR-5286

For enabling Encryption for External Mode On RHCS Cluster-

# ceph config set global ms_client_mode secure
# ceph config set global ms_cluster_mode secure
# ceph config set global ms_service_mode secure
# ceph config set global rbd_default_map_options ms_mode=secure

Restart all ceph daemons (mgr, mon, osd, mds etc)

On ODF Cluster- storagecluster CR

network:
  connections:
      encryption:
        enabled: true
        
 oc patch storagecluster ocs-storagecluster -n openshift-storage --type json --patch  '[{ "op": "replace", "path": "/spec/network", "value": {"connections": {"encryption": {"enabled": true}}} }]'

steps seems fairly straightforward to me, instead of removing checkbox from the external mode UI we can also consider just adding link to pre/post-requisite CLI steps (doc) and adding an info message on the UI. @malayparida2000 can we plz discuss this with Eran/Chandan ?? Or, is it already discussed and decided to move forward with removal of the checkbox ??

There has been no discussion, This PR is just a proposal from my side to drive that discussion forward, whichever option from the 2 I mentioned we find more preferable we can proceed with that.

@malayparida2000
Copy link
Contributor Author

There has been no discussion, This PR is just a proposal from my side to drive that discussion forward, whichever option from the 2 I mentioned we find more preferable we can proceed with that.

Created a slack thread for finalising the choice

https://ibm-systems-storage.slack.com/archives/C06SS96EECT/p1729503293391979

@malayparida2000
Copy link
Contributor Author

/hold until there is consensus on the step to be taken

@parth-gr
Copy link
Member

Encryption in transit is still an option to be choosen

then we need to introduce a new flag in the python script, before skipping it here.

@malayparida2000
Copy link
Contributor Author

Encryption in transit is still an option to be choosen

then we need to introduce a new flag in the python script, before skipping it here.

Flag for?

@parth-gr
Copy link
Member

Instead of having this, user need to pass the details using a flag, and it should update the encryption.
oc patch storagecluster ocs-storagecluster -n openshift-storage --type json --patch '[{ "op": "replace", "path": "/spec/network", "value": {"connections": {"encryption": {"enabled": true}}} }]'

@malayparida2000
Copy link
Contributor Author

Acc to the discussion with Eran we would keep the checkbox with additional details about the required configurations in RHCS cluster.
Updated the PR accordingly.

@malayparida2000
Copy link
Contributor Author

/retitle Change description of In-transit Encryption checkbox

@openshift-ci openshift-ci bot changed the title Hide In-transit Encryption checkbox in external Mode deployment Change description of In-transit Encryption checkbox Oct 23, 2024
@malayparida2000
Copy link
Contributor Author

/unhold

@SanjalKatiyar
Copy link
Collaborator

@malayparida2000 need to run yarn i18n command before commit else CI will fail.

Copy link
Contributor

openshift-ci bot commented Oct 23, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: malayparida2000
Once this PR has been reviewed and has the lgtm label, please assign bipuladh 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

@malayparida2000
Copy link
Contributor Author

@malayparida2000 need to run yarn i18n command before commit else CI will fail.

Thanks, Updated now.

@malayparida2000
Copy link
Contributor Author

/retitle Add info alert for In-transit Encryption in external mode

@openshift-ci openshift-ci bot changed the title Change description of In-transit Encryption checkbox Add info alert for In-transit Encryption in external mode Nov 5, 2024
@parth-gr
Copy link
Member

parth-gr commented Nov 5, 2024

@malayparida2000 so we are keeping the checkbox to update the storagecluster?

@malayparida2000
Copy link
Contributor Author

@malayparida2000 so we are keeping the checkbox to update the storagecluster?

Yes Parth according to the discussion we had with Eran & Chandan we would retain the checkbox with an added info alert.
Create-storagesystem (1)

@malayparida2000
Copy link
Contributor Author

/cc @SanjalKatiyar

@malayparida2000
Copy link
Contributor Author

malayparida2000 commented Nov 14, 2024

Internal Mode & Provider Mode
Screenshot 2024-11-14 at 2 28 07 PM
External Mode
Screenshot 2024-11-14 at 2 30 16 PM

The configurations for In-Transit Encryption in external mode are all
required to be done on the RHCS cluster by the ceph admin. Ticking the
checkbox on ODF side only sets the kernel mount option for cephFS.
So it might be misleading the user to think, the checkbox is enough to
enable Eit in external mode. So added an info alert under the checkbox
to clarify that settings need to be applied on the RHCS cluster. Also
reworded the description of the checkbox to make it more clear to the
user.

Signed-off-by: Malay Kumar Parida <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants