Prevent nested sensitive wrap of password alter query

[laugmanuel]

Fixes #1417

[CLAassistant]

All committers have signed the CLA.

[puppet-community-rangefinder]

postgresql::server::role is a type

Breaking changes to this file WILL impact these 22 modules (exact match):

• nicopaez-dockerserver
• braiins-zabbix
• jjulien-trac
• landcareresearch-pidservice
• braiins-sentry
• nnutter-testdb
• deric-pgprobackup
• Aethylred-gitlab
• nicopaez-alfred
• nicopaez-triage
• lwo-dataverse
• hetzner-roundcube
• maestrodev-maestro
• it2ndq-barman
• deric-barman
• npwalker-pe_external_postgresql
• adullact-demarchessimplifiees
• puppetfinland-puppetmaster
• landcareresearch-ckan
• groupbuddies-gb
• blackknight36-bacula
• puppetlabs-puppetdb

Breaking changes to this file MAY impact these 6 modules (near match):

• Firebladee-moodle
• tedivm-hieratic
• Aethylred-puppetdashboard
• puppetfinland-patchwork
• echoes-wrappers
• soli-wrappers

This module is declared in 70 of 580 indexed public Puppetfiles.

---

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

[ekohl]

There are some related PRs. I think this is a better fix than #1404 but #1405 would improve the tests to catch this. Can you take a look and see if you can combine those tests into your PR?

[laugmanuel]

There are some related PRs. I think this is a better fix than #1404 but #1405 would improve the tests to catch this. Can you take a look and see if you can combine those tests into your PR?

Thanks for the heads-up. Somehow I did miss the existing Issue and PR - sorry for that.

I've cherry-picked the tests from #1404 to this PR.

[laugmanuel]

Broken tests for RHEL9 and SLES15 seem to be infrastructure related...

[cruelsmith]

Sry to be the party popper but make it even sense to call the postgresql::postgresql_password function with forcing it to be sensitive and instantly unwarping its result?

That is why in #1404 i just turned the sensitive return of the value off:

puppetlabs-postgresql/manifests/server/role.pp

Line 169 in e522f51

$password_hash =~ Sensitive[String],

This also means that the unwrap for the Deferred is not needed in this change because the function already returns a not sensitive value.

puppetlabs-postgresql/lib/puppet/functions/postgresql/postgresql_password.rb

Lines 39 to 43 in 83be16e

	if sensitive
	Puppet::Pops::Types::PSensitiveType::Sensitive.new(pass)
	else
	pass
	end

[laugmanuel]

I guess both ways are valid and it's mostly personal preference. However, I do prefer unwrapping in Puppet, as passing in a Sensitive also returns a Sensitive in this case - so the function itself does not change the datatype.

[ekohl]

I've merged #1404 because it was a quick fix, but I still wouldn't mind merging this. If you're still interested in this then please rebase.

[laugmanuel]

I've merged #1404 because it was a quick fix, but I still wouldn't mind merging this. If you're still interested in this then please rebase.

Rebased this MR against current main.

[smortex]

I am confused. Can you provide a non-working unit test that this change fix?

I feel like this got fixed by accident while fixing other misbehavior.

[smortex]

The 3rd arguments should prevent the return value from being a Sensitive, so this should be noop.

[smortex]

Same

[laugmanuel]

I am confused. Can you provide a non-working unit test that this change fix?

I feel like this got fixed by accident while fixing other misbehavior.

This PR fixes the same problem as #1404 does, but in a different way (as I wrote on Apr 28). Because of that, there are no broken tests at the moment.

I just followed the advice of @ekohl to rebase. However, I'm fine with closing this as well...

[smortex]

I see, it looks like the bugfix was also in main and the commit in this PR now only show the differences between the two bugfix attempts. Since the problem is now fixed, I will close this PR and the corresponding issue.

Thanks!