puppetlabs / puppetlabs-postgresql

Puppet module for managing PostgreSQL
Apache License 2.0
228 stars 614 forks source link

define Enum on supported encryption types for postgresql_password #1611

Open figless opened 2 months ago

figless commented 2 months ago

Summary

This PR fixes 1575

Additional Context

Related Issues (if any)

Checklist

I was hitting 1575 when using a Deferred password. With this fix, puppet creates the database as before (v9.2.0)

postgresql::server::db { $db_name:
      user     => $db_username,
      password => Deferred('unwrap', [$db_password]),
}
CLAassistant commented 2 months ago

CLA assistant check
All committers have signed the CLA.

figless commented 3 weeks ago

We already have defined this as an enum:

https://github.com/puppetlabs/puppetlabs-postgresql/blob/05a78097377eebd34297e9a01054f4bd50507b79/types/pg_password_encryption.pp#L2

And it's used in multiple places now, so this would risk them going out of sync. I'd at least expect a comment in both places to remind people to keep them in sync.

I've added a comment to both files.

bastelfreak commented 3 weeks ago

Is there a chance you can add a test with Deferred(), to ensure this doesn't break in the future?

figless commented 3 weeks ago

Is there a chance you can add a test with Deferred(), to ensure this doesn't break in the future?

Sure - i've copied spec/acceptance/db_spec.rb to spec/acceptance/db_deferred_spec.rb and configured the test to use a Deferred password.

figless commented 2 weeks ago

The previous code in db_deferred_spec.rb failed the CI, even though it was essentially a copy from db_spec.rb.... I have adapted the code, which now passes locally.