ghoneycutt / puppet-module-ssh

Puppet module to manage SSH
Other
34 stars 184 forks source link

V4 #307

Closed ghoneycutt closed 1 year ago

ghoneycutt commented 5 years ago

TODO:

manifests

docs

testing

Phil-Friderici commented 2 years ago

@TheMeier your comment is valid and they should look like you mentioned, if empty. These files are just placeholders and will get filled with data in PR https://github.com/ghoneycutt/puppet-module-ssh/pull/372 .

Phil-Friderici commented 2 years ago

Hej @ghoneycutt,

I have found some parameters that have been in v3 of the module but haven't added to v4 yet.

ssh_config UseRoaming, MACs sshd_config GSSAPIKeyExchange, Match, PAMAuthenticationViaKBDInt, ServerKeyBits, UsePrivilegeSeparation

I checked man pages for them and was able to find most of them on here but they were missing there.

I think we should add them to the module. They only get into the config files if the parameters are really set. So they shouldn't harm.

What is your opinion on that ?

PS: default_sshd_gssapikeyexchange& PAMAuthenticationViaKBDInt seems to be Solaris specific.

ghoneycutt commented 2 years ago

I'm OK with adding them. If it gets to be too much such as options only present in sun's SSH implementation you might just add those through a custom param.

Phil-Friderici commented 2 years ago

@ghoneycutt we are coming to an end here :)

From my perspective the code looks very much cleaned up now. The unit test should be complete. Still missing are the documentations, would feel better if a native speaker could write them.

Regarding the default settings for different OSes: I have taken great care to take over the existing settings from the old v3 version. While adding support for Debian 11, I realised that the old settings where more like a compromise to get new OSes integrated. Now that we have everything OS related in hiera files (one for each flavour) we could change the defaults to become closer to the distribution defaults.

And one more thing: I have created a simple text file that show the ssh(d)_config directives and the parameter names for v3 and v4. I think this could be helpfull for upgrading to v4. Should I integrate it ?

Please review and comment.

ghoneycutt commented 2 years ago

@Phil-Friderici Awesome work as always!

The default settings never really get used in practice because the code is running on a supported platform that has the data in Hiera. Because of that and because there are no guidance examples from OpenSSH for the config files, suggest we just use the most popular distro, EL8.

If you want to give the docs a go or even an outline, I'm happy to edit/finish.

For the CONTRIBUTING, could you add a checklist for places in the code that need updated to add a platform and how to add a parameter as those should cover the bulk of future updates to this code.

Please do include any migration notes.

Could you please resolve any merge conflicts so this can be merged cleanly.

ghoneycutt commented 2 years ago

@Phil-Friderici Could you please check out my late notes - https://github.com/ghoneycutt/puppet-module-ssh/pull/373#issuecomment-1040678698 Think perhaps this can be simplified more by just removing the code around use_roaming.

Phil-Friderici commented 2 years ago

@Phil-Friderici Could you please check out my late notes - #373 (comment) Think perhaps this can be simplified more by just removing the code around use_roaming.

Removed the UseRoaming functionality in PR https://github.com/ghoneycutt/puppet-module-ssh/pull/374. Additionally I have added a bulletpoint to the TODO list above.

Phil-Friderici commented 2 years ago

Could you please resolve any merge conflicts so this can be merged cleanly.

From my perspective rebasing only adds a lot of extra work and brings no real benefit. v4 is a new module which is completely rewritten and not compatible with older versions anymore. I would recommend to rename the old master branch to v3 and make the v4 branch the new master.

Phil-Friderici commented 2 years ago

Hej @ghoneycutt, what is missing here and is there something I can do to speed up the process ? Thanks & phile grüße

Phil-Friderici commented 2 years ago

ping

Phil-Friderici commented 1 year ago

I am closing this PR because the v4 branch already made it into the main/master branch.