latchset / clevis

Automated Encryption Framework
GNU General Public License v3.0
832 stars 99 forks source link

Commit to separate Dracut #437

Closed gb-123-git closed 5 months ago

gb-123-git commented 10 months ago

This Commit is based on the work of @sergio-correia & @jpds. I have just rebased it with current tree. Should close #347 & #346.

Hope this gets merged quickly.

gb-123-git commented 10 months ago

@sergio-correia @jpds @Anonymous1157

I have re-based the same files over the current tree. Can you guys check and test it (for Alpine Linux too please) ?

gb-123-git commented 10 months ago

@sergio-correia Anyway this can be merged to a new branch so that there can be more changes by maintainers / other developers etc. and later when everything is ok, it can be merged with the main branch ? Anyway I can give you write access to my fork ?

gb-123-git commented 10 months ago

@sergio-correia Please let me know if anything else needs to be done. Made the changes you suggested. Thanks

sergio-correia commented 10 months ago

@sergio-correia Please let me know if anything else needs to be done. Made the changes you suggested. Thanks

Thanks, I will do a more thorough review and actual testing soon and will report back.

sarroutbi commented 10 months ago

Could you please review issues reported by shellcheck? I guess that, at least, next can be removed:

Error: SHELLCHECK_WARNING:
./src/dracut/clevis-pin-tpm2/module-setup.sh.in:39:26: error[SC2283]: Remove spaces around = to assign (or use [ ] to compare, or quote '=' if literal).
gb-123-git commented 10 months ago

@sarroutbi

Error: SHELLCHECK_WARNING:
./src/dracut/clevis-pin-tpm2/module-setup.sh.in:39:26: error[SC2283]: Remove spaces around = to assign (or use [ ] to compare, or quote '=' if literal).

The above has been resolved.

For the other issues i.e. 'variables assigned but not used', I would require help of @sergio-correia & @jpds who are the actual author of the code.

Update: Some of the variables are actually coming from command line so I think some of them are just false warnings. @jpds & @sergio-correia would be able to comment better.

gb-123-git commented 9 months ago

@sarroutbi @sergio-correia Any progress on merging this ? Anything I need to do ?

sarroutbi commented 9 months ago

@sarroutbi @sergio-correia Any progress on merging this ? Anything I need to do ?

Hello. Did you have a chance to check latest Code scanning/shellcheck issues?

hostonly appears unused. Verify use (or export if used externally)
...
instmods appears unused. Verify use (or export if used externally).

Do these values need to be exported?

gb-123-git commented 9 months ago

@sarroutbi @sergio-correia Any progress on merging this ? Anything I need to do ?

Hello. Did you have a chance to check latest Code scanning/shellcheck issues?

hostonly appears unused. Verify use (or export if used externally)
...
instmods appears unused. Verify use (or export if used externally).

Do these values need to be exported?

Some of the variables are actually coming from command line and passed on (eg 'hostonly' option) to dracut so I think they are just false warnings. @jpds & @sergio-correia would be able to comment better.

gb-123-git commented 9 months ago

@sergio-correia @jpds Can you guys please help fix this so it gets merged before it needs another re-base ?

gb-123-git commented 9 months ago

@sergio-correia & @jpds

Just another humble request to help me merge this ? Thanks!

sergio-correia commented 9 months ago

@sergio-correia & @jpds

Just another humble request to help me merge this ? Thanks!

Apologies for the delay. I intend to test it this week.

gb-123-git commented 7 months ago

@sergio-correia - Just checking if there are any updates ? Did you get the time to check ? Thanks

gb-123-git commented 7 months ago

@sergio-correia - Any update ?

sergio-correia commented 6 months ago

I got stuck with testing this, I will try to complete it ASAP.