Skip to content

Add support for new OS flavours (ready to merge) #398

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

Closed
wants to merge 11 commits into from

Conversation

Phil-Friderici
Copy link
Collaborator

@Phil-Friderici Phil-Friderici commented Jan 4, 2023

This change add support for these new OSes:

  • AlmaLinux 8/9
  • Archlinux
  • CentOS 9
  • RedHat 9
  • Rocky 8/9
  • Ubuntu 22.04

To be able to support new OS flavours these changes have been made too:

  • Migrate from os.family to os.name
    Make sure that we can have specific configurations for each RedHat based OS.

  • Add possibility to manage configuration files in .d directories
    Modern versions of SSH allow to use configuration files via include directive. Several OS vendors already use this feature.

  • Move default value for $host into hiera data
    Allow host directive to be unset in ssh_config. This is needed for OS flavours that do not set this directive in ssh_config but in .d directories.

  • Add OracleLinux and Scientific Linux to metadata

Fixes:

@Phil-Friderici
Copy link
Collaborator Author

Comment from @anders-larsson:

Hi @Phil-Friderici,

Would you mind adding RedHat 9 as well? It is currently not supported.

Looking through your changes for EL9 distributions they seem to be missing configuration/a decision needs to be made how the .d directories should be handled. Right now they are included by the main configuration file but not maintained. Is this how we want it to work?

Until this is officially supported by the module should all configuration be moved into the main configuration file ignoring the contents of the .d directories?

@Phil-Friderici
Copy link
Collaborator Author

Would you mind adding RedHat 9 as well? It is currently not supported.

Will do so.

Looking through your changes for EL9 distributions they seem to be missing configuration/a decision needs to be made how the .d directories should be handled. Right now they are included by the main configuration file but not maintained. Is this how we want it to work?

I have started with adding just basic support for the new OS flavours. This should enable users to fully configure SSH in the old style (all configuration inside ssh_config/sshd_config).
You are rights, to support the new style (using include and .d directories) the module would need maintaining these as well.
If wanted I can start working on that later as well. Maybe @ghoneycutt or @treydock can drop there opinion here too ?

Until this is officially supported by the module should all configuration be moved into the main configuration file ignoring the contents of the .d directories?

For the default configuration I try to touch as less as possible and change nothing from the OS vendor indented settings.
As long as the module doesn't manage .d directory configuration files itself you have to do so by yourself or put all your setings into the old configuration files and remove the include directives.

@Phil-Friderici
Copy link
Collaborator Author

Comment from @anders-larsson:

Sounds good to me. It can always be replaced locally by the user. I'll update the Ubuntu 22.04 to support support v4. Our stance than is to maintain the defaults (not setting stuff explicitly)? In previous versions stuff seem to be set explicitly (RedHat 8 for example) instead of using the implicit defaults.

@Phil-Friderici
Copy link
Collaborator Author

Our stance than is to maintain the defaults (not setting stuff explicitly)? In previous versions stuff seem to be set explicitly (RedHat 8 for example) instead of using the implicit defaults.

Well, staying as close as possible to the vendor settings is my approach for modules ;)

@Phil-Friderici Phil-Friderici changed the title Add support for Rocky and AlmaLinux Add support for RedHat 9, Rocky 8/9 and AlmaLinux 8/9 Jan 4, 2023
@Phil-Friderici
Copy link
Collaborator Author

@anders-larsson: support for RedHat 9 just added

@Phil-Friderici Phil-Friderici changed the title Add support for RedHat 9, Rocky 8/9 and AlmaLinux 8/9 Add support for RedHat 9, Rocky 8/9 and AlmaLinux 8/9 (work in progress) Jan 6, 2023
@Phil-Friderici Phil-Friderici changed the title Add support for RedHat 9, Rocky 8/9 and AlmaLinux 8/9 (work in progress) Add support for RedHat 9, Rocky 8/9 and AlmaLinux 8/9 (ready to merge) Jan 16, 2023
Phil Friderici added 2 commits January 17, 2023 15:00
Modern versions of SSH allow to include configuration files via include
directive. Several OS vendors already use this feature.

This change allows the module to manage configuration files in .d directories
as well.
Allow host directive to be unset in ssh_config. This is needed for OS flavours
that do not set this directive in ssh_config but in .d directories.
@Phil-Friderici
Copy link
Collaborator Author

@ghoneycutt could you please review and hopefully merge.
Btw: v4 is running in production already :)

@canihavethisone
Copy link

@Phil-Friderici Would it be possible to also add support for CentOS 9 please? Should be straight forward.

@Phil-Friderici
Copy link
Collaborator Author

@canihavethisone: et voilà, added support for CentOS 9.

Tests do fail due to problems with PDK gems. Guess it needs an update by Puppetlabs.

@canihavethisone
Copy link

@Phil-Friderici wow, thanks for the fast work! Perhaps centOS9 should also be added to supported os in metadata?

@Phil-Friderici
Copy link
Collaborator Author

@canihavethisone added CentOS 9 to metadata as @canihavethisone (thanks!) mentioned

@Phil-Friderici
Copy link
Collaborator Author

great Puppetlabs fixed the PDK and now all checks are good :)

@Phil-Friderici Phil-Friderici changed the title Add support for RedHat 9, Rocky 8/9 and AlmaLinux 8/9 (ready to merge) Add support for CentOS 9, RedHat 9, Rocky 8/9, AlmaLinux 8/9 and Ubuntu 22.04 (ready to merge) Jan 30, 2023
@Phil-Friderici Phil-Friderici changed the title Add support for CentOS 9, RedHat 9, Rocky 8/9, AlmaLinux 8/9 and Ubuntu 22.04 (ready to merge) Add support for new OS flavours (ready to merge) Jan 31, 2023
@ghoneycutt
Copy link
Owner

@Phil-Friderici Why ssh::config_file_server and ssh::config_file_client as it seems they are the same. Perhaps we only need ssh::config ?

@Phil-Friderici
Copy link
Collaborator Author

@ghoneycutt We need one for the server directives and the other for the client directives.

@Phil-Friderici
Copy link
Collaborator Author

Acceptance tests seem to be broken again :(

@Phil-Friderici
Copy link
Collaborator Author

@ghoneycutt Anything missing before it could get merged ?

@treydock
Copy link
Contributor

I ran into those same acceptance errors on some of my modules, switching to Voxpupli testing gems (ie newer testing gems) solved the issue: https://github.com/treydock/puppet-module-keycloak/pull/276/files#diff-d09ea66f8227784ff4393d88a19836f321c915ae10031d16c93d67e6283ab55f

@Phil-Friderici
Copy link
Collaborator Author

@treydock Thank you for pointing to the Voxpupli gems. Too bad I have never understood the way Voxpupli is testing.
I was able to get the acceptance tests runing with your hint. But now the unit tests doesn't work anymore :(

@treydock
Copy link
Contributor

https://github.com/treydock/puppet-module-keycloak/blob/master/.github/workflows/ci.yaml#L48 . I think you need to update ci.yaml and release.yaml in this repo to bump their Ruby gem cache version number to flush the cache and force Github Actions to download new Ruby gems. The caching of gems can be a problem for CI pipelines when Gemfile.lock is not part of the git repo.

@Phil-Friderici
Copy link
Collaborator Author

Phil-Friderici commented Mar 8, 2023

To speak frankly, I think that my PR is complete and working well. It is the testing harness that doesn't work as intended. I had managed to get it working for either the unit tests or the acceptance tests but not both.
From my perspective the testing harness should be fixed in another PR and shouldn't block merging this PR.

@baron1405
Copy link

@ghoneycutt @Phil-Friderici is there an ETA for getting this committed and a new version released?

@treydock
Copy link
Contributor

treydock commented Apr 1, 2023

@baron1405 I will be taking this and trying to get it across the finish line. I hope to have things ready to merge in next week or two.

@baron1405
Copy link

@treydock Thanks for the update and your efforts are truly appreciated!

@ghoneycutt
Copy link
Owner

Thank you for your contributions and help! This is merged in #402 and released in 4.1.0.

@ghoneycutt ghoneycutt closed this Apr 6, 2023
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.

Add hiera support for Oracle Linux 8 Add support for Ubuntu 22.04 LTS Support for Debian 11
6 participants