Skip to content

Conversation

ekohl
Copy link
Collaborator

@ekohl ekohl commented Oct 19, 2020

This adds a parameter (defaulting to true) to manage the postgresql_conf_path file resource. The actual content isn't managed because that's what postgresql::server::config_entry is for. However, managing the file ensures the correct permissions.

It also provides a workaround for PUP-10548 when using Red Hat SCL for packages when the correct SELinux file context isn't present. Without managing the file, an admin will need to either make sure the package is present before running Puppet, manage the file via another module or manually set the file context after Puppet ran. With this workaround, Puppet will converge on the second run and actually start PostgreSQL.

@ekohl ekohl requested a review from a team as a code owner October 19, 2020 15:20
@puppet-community-rangefinder
Copy link

postgresql::globals is a class

Breaking changes to this file WILL impact these 20 modules (exact match):
Breaking changes to this file MAY impact these 1 modules (near match):

postgresql::params is a class

Breaking changes to this file WILL impact these 1 modules (exact match):
Breaking changes to this file MAY impact these 3 modules (near match):

postgresql::server is a class

Breaking changes to this file WILL impact these 36 modules (exact match):
Breaking changes to this file MAY impact these 15 modules (near match):

postgresql::server::config is a class

that may have no external impact to Forge modules.

This module is declared in 71 of 575 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
Copy link
Collaborator Author

ekohl commented Oct 19, 2020

There are a ton of whitespace changes here, but the actual change itself is much smaller.

@sheenaajay sheenaajay self-assigned this Nov 2, 2020
@sheenaajay
Copy link
Contributor

sheenaajay commented Nov 2, 2020

Thanks alot @ekohl for your contribution. Acceptance tests are failing with the following error on ubuntu https://travis-ci.org/github/puppetlabs/puppetlabs-postgresql/jobs/737110592 Error: Puppet::Util::FileType::FileTypeFlat could not write /tmp/postgresql.conf: Permission denied @ rb_sysopen - /tmp/postgresql.conf
Please let us know if you need more information. Will run the changes on all OSes and let you know. Thank you.

@ekohl
Copy link
Collaborator Author

ekohl commented Nov 4, 2020

Looks like the default file mode on Debian is 644 so I'll need to introduce a parameter for the file permission.

@ekohl ekohl force-pushed the manage-postgresql-config branch from c357336 to fd08efd Compare November 4, 2020 18:47
@ekohl
Copy link
Collaborator Author

ekohl commented Nov 4, 2020

I updated it and added the parameter. I only know its value for Red Hat (taken from CentOS 7 & 8) and Debian (taken from 8, 9 & 10, assuming Ubuntu is the same).

@ekohl
Copy link
Collaborator Author

ekohl commented Nov 5, 2020

All builds failed with

.rake aborted!

NoMethodError: undefined method `trace' for #<Logging::Logger:0x000000000444b570>

Did you mean?  trap

/home/travis/build/puppetlabs/puppetlabs-postgresql/vendor/bundle/ruby/2.5.0/gems/bolt-2.24.1/lib/bolt/pal/logging.rb:15:in `handle'

This looks like a bolt bug rather than something I did.

@sheenaajay
Copy link
Contributor

@ekohl Thank you for the quick response. Looks like its using older version of bolt 2.24.1
https://travis-ci.org/github/puppetlabs/puppetlabs-postgresql/jobs/741465093#L498
Let me clear the cache and kick for a rerun. Thank you

@ekohl ekohl force-pushed the manage-postgresql-config branch from fd08efd to 113b27b Compare November 9, 2020 12:28
@ekohl
Copy link
Collaborator Author

ekohl commented Nov 9, 2020

Haven't been able to quite figure out why this failed. Rebased it to get the output cleanups in to make the output a bit easier to read.

@ekohl
Copy link
Collaborator Author

ekohl commented Nov 10, 2020

So the problem on Debian based systems is with /tmp:

# ls -ld /tmp /tmp/postgresql.conf 
drwxrwxrwt. 2 root     root     4096 Nov 10 14:09 /tmp
-rw-------. 1 postgres postgres    0 Nov 10 14:08 /tmp/postgresql.conf
# echo test >> /tmp/postgresql.conf 
bash: /tmp/postgresql.conf: Permission denied
# id
uid=0(root) gid=0(root) groups=0(root)

This is probably because it's mounted as sticky. Changing the path to /etc should help.

@ekohl ekohl force-pushed the manage-postgresql-config branch from 113b27b to a302bbb Compare November 10, 2020 14:16
@ekohl
Copy link
Collaborator Author

ekohl commented Nov 11, 2020

This is really hard to debug because litmus output is really not telling me a lot. I'd guess something can't read the config file, but I can't quite figure it out.

@sheenaajay
Copy link
Contributor

@ekohl Thanks alot for the quick changes. Was looking at the errors. Its failing with the folllowing error 1) postgresql::server::grant_role: grants a role to a user/superuser On hostlocalhost:2224'
Failure/Error: LitmusHelper.instance.run_shell("cd /tmp; su #{shellescape(user)} -c #{shellescape(psql)}", acceptable_exit_codes: exit_codes, &block)
RuntimeError:
shell failed
cd /tmp; su psql_grant_role_tester -c psql\ --command\=\"SELECT\ 1\ WHERE\ pg_has_role\(\'psql_grant_role_tester\',\ \'test_group\',\ \'MEMBER\'\)\ \=\ true\"\ grant_role_test
======
[{"target"=>"localhost:2224", "action"=>"command", "object"=>"cd /tmp; su psql_grant_role_tester -c psql\ --command\=\"SELECT\ 1\ WHERE\ pg_has_role\(\'psql_grant_role_tester\',\ \'test_group\',\ \'MEMBER\'\)\ \=\ true\"\ grant_role_test", "status"=>"failure", "value"=>{"stdout"=>"", "stderr"=>"Error: Invalid data directory\n", "exit_code"=>1, "_error"=>{"kind"=>"puppetlabs.tasks/command-error", "issue_code"=>"COMMAND_ERROR", "msg"=>"The command failed with exit code 1", "details"=>{"exit_code"=>1}}}}]

 # ./vendor/bundle/ruby/2.5.0/gems/puppet_litmus-0.18.4/lib/puppet_litmus/puppet_helpers.rb:173:in `block in run_shell'
 # ./vendor/bundle/ruby/2.5.0/gems/honeycomb-beeline-2.3.0/lib/honeycomb/client.rb:70:in `start_span'
 # ./vendor/bundle/ruby/2.5.0/gems/puppet_litmus-0.18.4/lib/puppet_litmus/puppet_helpers.rb:157:in `run_shell'
 # ./spec/spec_helper_acceptance_local.rb:47:in `psql'
 # ./spec/acceptance/server/grant_role_spec.rb:164:in `block (2 levels) in <top (required)>'

`
/spec/acceptance/server/grant_role_spec.rb:164

https://github.com/ekohl/puppetlabs-postgresql/blob/manage-postgresql-config/spec/acceptance/server/grant_role_spec.rb#L164

## Check that the role was granted to the user psql('--command="SELECT 1 WHERE pg_has_role(\'psql_grant_role_tester\', \'test_group\', \'MEMBER\') = true" grant_role_test', 'psql_grant_role_tester') do |r|

https://github.com/ekohl/puppetlabs-postgresql/blob/manage-postgresql-config/spec/spec_helper_acceptance_local.rb#L47

def psql(psql_cmd, user = 'postgres', exit_codes = [0, 1], &block) psql = "psql #{psql_cmd}" LitmusHelper.instance.run_shell("cd /tmp; su #{shellescape(user)} -c #{shellescape(psql)}", acceptable_exit_codes: exit_codes, &block) end

By default it executes psql command from /tmp directory. We can modify the psql function to accepts one parameter to make the config path configurable.

@ekohl
Copy link
Collaborator Author

ekohl commented Nov 16, 2020

There is a known issue that you often end up executing it from /root which can't be read. I can try to change it to / which should also not produce an error but I don't expect that to matter.

ensure => file,
owner => $user,
group => $group,
mode => '0600',
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be using postgresql_conf_mode?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think you may have found the thing I was struggling to find. Sometimes you're blind to your own code.

This adds a parameter (defaulting to true) to manage the
postgresql_conf_path file resource. The actual content isn't managed
because that's what postgresql::server::config_entry is for. However,
managing the file ensures the correct permissions.

It also provides a workaround for PUP-10548[1] when using Red Hat SCL
for packages when the correct SELinux file context isn't present.
Without managing the file, an admin will need to either make sure the
package is present before running Puppet, manage the file via another
module or manually set the file context after Puppet ran. With this
workaround, Puppet will converge on the second run and actually start
PostgreSQL.

The acceptance test path is changed to avoid problems on Debian(-based)
systems where the root user can't write to it. Root doens't own the file
and doesn't have write access due to the sticky bit.

[1]: https://tickets.puppetlabs.com/browse/PUP-10548
@ekohl ekohl force-pushed the manage-postgresql-config branch from 7a70d19 to 171a1be Compare November 16, 2020 14:24
@ekohl
Copy link
Collaborator Author

ekohl commented Nov 16, 2020

Thanks to @DavidS it's green now.

@sheenaajay
Copy link
Contributor

wow superb. Thank you @DavidS and @ekohl .

@sheenaajay
Copy link
Contributor

Thank you @ekohl for your contribution.

@sheenaajay sheenaajay merged commit dbb63aa into puppetlabs:main Nov 23, 2020
@ekohl ekohl deleted the manage-postgresql-config branch November 23, 2020 10:12
cegeka-jenkins pushed a commit to cegeka/puppet-postgresql that referenced this pull request Feb 3, 2022
Manage postgresql_conf_path file permissions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants