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

Install redhat-lsb-core to provide additional facts #122

Closed
wants to merge 1 commit into from
Closed

Install redhat-lsb-core to provide additional facts #122

wants to merge 1 commit into from

Conversation

dhoppe
Copy link
Member

@dhoppe dhoppe commented Dec 6, 2019

Fixes #88
Fixes #102

@traylenator
Copy link
Contributor

Have you considered just using the standard $facts['os'] ?

@dhoppe
Copy link
Member Author

dhoppe commented Dec 6, 2019

@traylenator I did but they do not contain the codename of a release, which is used to point to the correct .epp template.

We could use a different directory structure like templates/$facts['os']['name']/$facts['os']['release']['major']/etc/fail2ban/jail.conf.epp but this would be a backwards incompatible change.

@traylenator
Copy link
Contributor

Why backwards incompatible?

Is it not just an internal rearrangement of file locations of the templates

I'll have a better look at the module before I comment any more.

@dhoppe
Copy link
Member Author

dhoppe commented Dec 6, 2019

@traylenator Yes, but it affects the use because he has to change his configuration. Otherwise the templates will not be found.

@bastelfreak
Copy link
Member

I'm not a big fan of the lsb facts. I think proper version numbers are a better approach, because those are always available and don't need additional packages. Note: The next release will already be a major one because it includes backwards-incompatible changes. But also we should not do too many of them if we can avoid it.

@gcoxmoz
Copy link

gcoxmoz commented Mar 10, 2020

Forcing RedHat servers to install lsb-core is just the wrong direction.

  • Facter gives us facts about the OS, there's no reason we need an extra package installed.
  • The answers that lsb-core gives are incomplete: CentOS is $facts['os']['family'] == 'RedHat' but lsb always calls it 'Core', so we can't distinguish between CentOS7 and CentOS8 by just the lsb name.

So, this PR adds an extra package to preserve a status quo that needs to go away.

@alexjfisher alexjfisher mentioned this pull request Apr 4, 2020
@dhoppe dhoppe closed this Apr 5, 2020
@dhoppe dhoppe deleted the add_dependency branch April 5, 2020 09:51
@neomilium neomilium mentioned this pull request Apr 6, 2020
@neomilium
Copy link
Contributor

@traylenator @bastelfreak I contributed some commits available in PR #135 that remove any lsb-related facts usage. Feel free to review and/or help me to test it on CentOS and RedHat. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants