-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add support for RedHat 9 #51
Conversation
b365a31
to
335636b
Compare
The GitHub Action failures are false positives. I added some commits on top that fix a few genuine issues, but the rest is just GHA runtime environment issues. The first and possibly the last is this one: |
# IPA switched to mod_ssl as the crypto engine for Apache as of version 4.7.0 | ||
# see https://www.freeipa.org/page/Releases/4.7.0#Highlights_in_4.7.0 | ||
if versioncmp($facts['ipa_server_version'], '4.7.0') < 0 { | ||
exec { 'semanage-port-http_port_t': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this whole block become unnecessary with FreeIPA > 4.7.0? Most of this does not look specific to a crypto library change.
; | ||
'adjust uid max': | ||
line => "UID_MAX\t11999", | ||
match => '^UID_MAX\s*60000$', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps the match should be more flexible, e.g. ^UID_MAX\s\d$**. That way if the value is not exactly 60000 file_line would not fail or worse, add additional lines because no match is was found.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, does this apply to both FreeIPA clients and servers?
; | ||
'adjust gid max': | ||
line => "GID_MAX\t11999", | ||
match => '^GID_MAX\s*60000$', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment here as above.
manifests/install/server.pp
Outdated
# Note: SUB_* only affect user/group mapping in containers, so not of | ||
# concern here | ||
if $easy_ipa::adjust_login_defs { | ||
if $easy_ipa::idstart < Integer($facts['uid_max']) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would just return an Integer from Ruby. Easier, cleaner and more intuitive.
manifests/install/server.pp
Outdated
if $easy_ipa::idstart < Integer($facts['uid_max']) { | ||
$uid_max_value = $easy_ipa::idstart -1 | ||
} | ||
if $easy_ipa::idstart < Integer($facts['gid_max']) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
lib/facter/gid_max.rb
Outdated
Facter.add(:gid_max) do | ||
setcode do | ||
lines = File.readlines("/etc/login.defs") | ||
lines.find { |line| line.start_with?("GID_MAX") }.split("\s")[1].strip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if the line looks like this:
GID_MAX 50000
That is if there are multiple spaces between GID_MAX and the value? Will [1] resolve as " " (=a space)?
Also, what if there are multiple lines that start with GID_MAX? Normally that would not happen, but can't be ruled out. That would cause potentially hard to debug problems. Two options I can think of for handling that:
- Error out with a reasonable error message ("ERROR: multiple GID_MAX lines found in /etc/login.defs")
- Pick the first value (e.g. line.starts_with?("GID_MAX")[0]) and print an informal message ("WARNING: multiple GIT_MAX lines found in /etc/login.defs").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on testing this works as expected. The "find" method returns the first line that matches. The split("\s") considers multiple whitespace characters as one, so multiple spaces between GID_MAX and the value do not cause issues.
@mattock , can this be fixed regarding tests and merged? |
I think this is mergeable once the commits have been squashed. |
@mattock , could you please squash it? I don't have permissions for it:
To quash it you need to run: and then add squash to all commits except initial one. |
Add support for RHEL9 |
233cb0e
to
753389b
Compare
Signed-off-by: Kibahop <[email protected]>
Signed-off-by: Kibahop <[email protected]>
Signed-off-by: Kibahop <[email protected]>
Signed-off-by: Kibahop <[email protected]>
Signed-off-by: Kibahop <[email protected]>
The old workflow was from voxpupuli and was not really PDK-compatible. It could be made to work only by heavy modifications to the Gemfile, which made it practically incompatible with "pdk update" and .sync.yml. So, use a PDK-compatible workflow from puppetlabs-postgresql with trivial modifications. Signed-off-by: Samuli Seppänen <[email protected]>
@mattock - thanks for fixing tests!!! Highly appreciate it!!! |
This adds so far minimally tested support for RedHat 9. The most important change in IPA version 4.10, that is atm in RedHat 9, is that mod_nss is replaced with with mod_ssl for the WebUI. Installer will also clash if UID_MAX and GID_MAX in login.defs overlap with idstart. There is a new parameters to handle that 'adjust_login_defs'. By default this is false.