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

Fix password_encryption for DBVERSION in server::role #1515

Merged
merged 2 commits into from
Sep 15, 2023
Merged

Fix password_encryption for DBVERSION in server::role #1515

merged 2 commits into from
Sep 15, 2023

Conversation

cruelsmith
Copy link
Contributor

@cruelsmith cruelsmith commented Sep 11, 2023

Summary

After #1512 the postgresql::postgresql_password() must not default to 'md5' anymore which creates a edge case in postgresql::server::role. When DBVERSION in paramter connect_settings is set to a version before 14 and the global version is 14 or above, the used hash parameter of postgresql::postgresql_password() will be set to undef instead of 'md5'.

This PR fixes that edge case by setting the wanted hash type and do not relay anymore on the default hash type of postgresql::postgresql_password() itself.

Additional Context

Add any additional context about the problem here.

  • Root cause and the steps to reproduce. (If applicable)
  • Thought process behind the implementation.

Related Issues (if any)

Fixes #1512

Checklist

  • 🟢 Spec tests.
  • 🟢 Acceptance tests.
  • Manually verified. (For example puppet apply)

@bastelfreak
Copy link
Collaborator

@cruelsmith can you add a unit test to ensure the logic correct now?

@@ -142,7 +142,7 @@
$_hash = if $hash {
$hash
} elsif $connect_settings != undef and 'DBVERSION' in $connect_settings {
if (versioncmp($version, '14') >= 0) { 'scram-sha-256' } else { undef }
versioncmp($version, '14') ? { -1 => 'md5', default => 'scram-sha-256' }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

derp, I missed that earlier :(

@cruelsmith
Copy link
Contributor Author

Added acceptance unit test that now works as wanted. Needed some test since i was not sure how to do it.

The failing (RedHat-9, puppet7-nightly) run is unrelated and a general issue that i saw also on other runs. Some times the Install module phase is simply failing.

@bastelfreak bastelfreak merged commit 0588fe7 into puppetlabs:main Sep 15, 2023
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants