Provide new defined resources for managing pg_hba.conf

[kbarber]

This patch provides a more advanced way of managing pg_hba rules, by providing a
defined resource to manage a pg_hba file, and a defined resource for managing
rules within such a file (pg_hba_rule).

These new resources are wrappers around ripinaar-concat, and utilise file
assemblies instead of a template to compose the pg_hba.conf file.

I've provided a function that interprets the old ip4|6acl arrays and converts
them to this new format for backwards compatibility as well.

I slightly reformatted our documentation to allow for better documentation of
defined resources in 'Usage' as well, and provided examples of how to use this
new resource.

This hopefully should go a long way to solving the PR's related to lack of full
functionality for pg_hba.conf.

Signed-off-by: Ken Barber [email protected]

[cprice404]

Could we possibly have the system test create a postgres user account, add or deny privileges to that user using the new pg_hba_rule stuff, and then attempt to connect via psql -h? This test is good--it confirms that we're writing something to the correct file--but it doesn't actually validate the syntax of the data we're writing.

[kbarber]

Yes of course, we can give it a try.

[jhoblitt]

I haven't tested this myself yet but I really like the API. I'm a
little concerned about rule ordering becoming tedious but I can't think
of a better solution. I will try to test this patch out later this week.

The one improvement that occurred to me is validating the type &
auth_method parameter strings since there's no way of checking that the
pg_hba.conf syntax is correct without restarting the database.

-Josh

On 02/11/2013 12:12 PM, Ken Barber wrote:

This patch provides a more advanced way of managing pg_hba rules, by
providing a
defined resource to manage a pg_hba file, and a defined resource for
managing
rules within such a file (pg_hba_rule).

These new resources are wrappers around ripinaar-concat, and utilise file
assemblies instead of a template to compose the pg_hba.conf file.

I've provided a function that interprets the old ip4|6acl arrays and
converts
them to this new format for backwards compatibility as well.

I slightly reformatted our documentation to allow for better
documentation of
defined resources in 'Usage' as well, and provided examples of how to
use this
new resource.

This hopefully should go a long way to solving the PR's related to
lack of full
functionality for pg_hba.conf.

Signed-off-by: Ken Barber [email protected] mailto:[email protected]

---

    You can merge this Pull Request by running

git pull https://github.com/kbarber/puppet-postgresql ticket/master/pg_hba_template

Or view, comment on, or merge it at:

#120

    Commit Summary

• Provide new defined resources for managing pg_hba.conf

  File Changes
  

• M .fixtures.yml
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-0
  (1)

• M .gitignore
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-1
  (1)

• M Modulefile
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-2
  (1)

• M README.md
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-3
  (128)

• A
  lib/puppet/parser/functions/postgresql_acls_to_resources_hash.rb
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-4
  (76)

• M manifests/config/beforeservice.pp
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-5
  (66)

• A manifests/pg_hba.pp
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-6
  (18)

• A manifests/pg_hba_rule.pp
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-7
  (36)

• M spec/spec_helper.rb
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-8
  (16)

• M spec/support/shared_examples/system_default_postgres.rb
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-9
  (19)

• M spec/support/vagrant_common.rb
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-10
  (1)

• M spec/unit/classes/server_spec.rb
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-11
  (1)

• A spec/unit/defines/pg_hba_rule_spec.rb
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-12
  (68)

• A spec/unit/defines/pg_hba_spec.rb
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-13
  (20)

• A spec/unit/functions/postgresql_acls_to_resources_hash_spec.rb
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-14
  (137)

• D templates/pg_hba.conf.erb
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-15
  (94)

• A templates/pg_hba_rule.conf
  https://github.com/puppetlabs/puppet-postgresql/pull/120/files#diff-16
  (5)

  Patch Links:
  

• https://github.com/puppetlabs/puppet-postgresql/pull/120.patch

• https://github.com/puppetlabs/puppet-postgresql/pull/120.diff

[cprice404]

@kbarber this looks reasonable to me, though I must admin that you're using some patterns here that I'm not very experienced with so I have to defer to your judgment to some degree. It will be interesting to see if any of the authors of the other pull requests have any input!

My few minor comments:

1. As mentioned earlier in the pull request I think it would be nice to have an actual, functional system test rather than just grepping the file--if it's not too much work.
2. For myself, as a user, I think that adding an example or two of how to use this under the tests directory would be valuable. I know that you have mixed feelings about the existence of that directory, but I think we should either get rid of it entirely or include examples of this type of functionality in it... otherwise we're in kind of a weird half-cocked state.

[kbarber]

@jhoblitt: I haven't tested this myself yet but I really like the API. I'm a
little concerned about rule ordering becoming tedious but I can't think
of a better solution. I will try to test this patch out later this week.

I couldn't think of a better solution either. Having said that, the class based API could be changed after this PR to perhaps allow us to pass an array of hashes, pre-ordered as apposed to the current ipv4acl mechanism which just takes strings of rule lines. Then the order could be explicitly based on the array. I didn't want to do such a change to the class this time around, but we could explore it later on as an option.

@jhoblitt: The one improvement that occurred to me is validating the type &
auth_method parameter strings since there's no way of checking that the
pg_hba.conf syntax is correct without restarting the database.

Yes, this can be done.

@cprice-puppet : As mentioned earlier in the pull request I think it would be nice to have an actual, functional
system test rather than just grepping the file--if it's not too much work.

I'm on it.

@cprice-puppet: For myself, as a user, I think that adding an example or two of how to use this under the tests
directory would be valuable. I know that you have mixed feelings about the existence of that directory, but I think
we should either get rid of it entirely or include examples of this type of functionality in it... otherwise we're in kind
of a weird half-cocked state.

You are right of course, my feelings on the tests directory are secondary - its the status quo and we should continue to maintain it, or debate removing it in another ticket/PR and do it properly.

Thanks everyone for your comments ...

[kbarber]

@cprice-puppet @jhoblitt I think I've captured the things you've asked for, and I've pushed a new update to this branch. I also caught a bug with the 'service postgresql reload' call with the new system test you asked me to add @cprice-puppet so 👍. Let me know if there is anything else you need.