coveralls
commented
3 months ago coverage: 97.803% (-0.01%) from 97.815% when pulling cede280529490e2b55bf3d30f015a50a9c85167a on luks2_enable into 600ef2372424706c40291e3168bab4e6cadc0c12 on master.
Closed schubi2 closed 2 months ago
coveralls
commented
3 months ago coverage: 97.803% (-0.01%) from 97.815% when pulling cede280529490e2b55bf3d30f015a50a9c85167a on luks2_enable into 600ef2372424706c40291e3168bab4e6cadc0c12 on master.
ancorgs
commented
2 months ago Sorry for taking so long to review this. I'm starting now.
At first sight I must say this is more intrusive than what I tried to suggest at https://github.com/yast/yast-installation/issues/1088#issuecomment-2044736341
What I suggested was making the encryption details (eg. use LUKS2) configurable:
But what I see here goes further. What I see with a first sight at the code (I may be wrong, so take things with a bit of grain).
Anyway, I will continue with the review now.
joseivanlopez
commented
2 months ago I also think this change should be much easier:
ancorgs
commented
2 months ago What I suggested was making the encryption details (eg. use LUKS2) configurable:
- by product
- in case the user decided to use encryption.
So basically I meant https://github.com/yast/yast-storage-ng/pull/1383 (it was easy to implement based on this pull request but keeping the changes minimal).
schubi2
commented
2 months ago Sorry for taking so long to review this. I'm starting now.
I am happy that you have time for it again. After a so long time I would like to come to an end even it could be a "minimal" solution. ;-)
* The installer makes an initial proposal without encryption but prints an error about that being wrong, asking the user to run the Guided Setup to fix the inconsistency. * That's kind of an anti-pattern for YaST. The initial proposals should be sensible, not intentionally wrong and needing user intervention to fix them. All users of such a product will be faced with a installer error.
Yes, you are right, but I have not found any other general solution for requesting the password if encryption (LUKS1/LUGS2) has been set by the product descriptions. I am really open for suggestions here.....
ancorgs
commented
2 months ago Yes, you are right, but I have not found any other general solution for requesting the password if encryption (LUKS1/LUGS2) has been set by the product descriptions. I am really open for suggestions here.....
First, let me double check I understand what's your intention. You want that anyone installing openSUSE Tumbleweed (or any other product configured in the same way, for that matter) always gets a screen asking for the encryption password at a relatively early stage of the installation process, so that password is then used for the initial storage proposal.
That possibility is not contemplated by the current installation workflow because none of the steps of the workflow make that possible. So you will need to change the installation workflow introducing a new YaST client (maybe replacing one of the current ones).
I'm pretty sure you (@schubi2) know what the installation workflow is. But let me go into the details for other readers landing here.
As we all know, everything about the installation process is configured at control.xml (stuff like the repositories to use, whether to use Btrfs or snapshots, etc.). And actually the most important aspect defined on that control.xml is the workflows for installing, for upgrading and so on. That's basically the sequence of steps (YaST clients) that are executed during installation. For Tumbleweed installation such a sequence can be found here. https://github.com/yast/skelcd-control-openSUSE/blob/master/control/control.xml#L738
That's the reason why the TW installation process looks like this (including the name of each YaST client):
| install_inf | complex_welcome | system_analysis | system_role |
|---|---|---|---|
| disk_proposal | timezone | user_first | inst_proposal |
You can create your own YaST client asking for the encryption password and introduce the client as a step before the disk_proposal one. That's actually what the common criteria role does at SLE, because the installation roles have the ability to define their own installation steps that get injected to the workflow (if the user selects the role) after the role selection screen.
If you prefer, instead of adding an installation step, you could even fully substitute the disk_proposal client by an alternative that works in a different way. In its current incarnation, that client is not designed for a product in which encrypting is the default option and, thus, entering the password is mandatory for installing.
I hope that answers your question. If not, I hope it's at least illustrative information for someone. :-)
One final remark, if you introduce a completely new YaST client as mandatory in the Tumbleweed installation workflow then get ready to get all bugzilla reports redirected at you. ;-) It's quite a fundamental change for the distribution.
schubi2
commented
2 months ago Yes Ancor, you have summarised my intentions correctly and you have convinced me. :-) So, I would suggest that we are agreeing on using LUKS2 as default or make it configurable in the product settings at least. The UI will not be changed. I am not sure what's you intention has been in: https://github.com/yast/yast-storage-ng/pull/1383/files#diff-778a5f8930595efbaf3a5dc6db8b7b31ad8510ad94c536909870d3000300b7beR218 It is your decision which way we are going there. At least we should be consistent regarding the default value in expert and guided setup. So, in order to come to an end, we should use your PR and simply reject my one: https://github.com/yast/yast-storage-ng/pull/1383
schubi2
commented
2 months ago We have come to the decision to use https://github.com/yast/yast-storage-ng/pull/1383 for this issue.
I have tried to fulfill the requirements of https://github.com/yast/yast-installation/issues/1088#issuecomment-2044736341
Docu: https://github.com/yast/yast-installation-control/pull/126 https://github.com/yast/yast-installation/pull/1115