fixes #20650 - validates postgres has write access to backup directory

[theforeman-bot]

Issues: #20650

[johnpmitsch]

@cfouant Something about restricting backups to /tmp and /var/tmp doesn't sit well with me, I have a few suggestions about alternatives, curious to hear your thoughts: 1) Can we just chown the backup directory to postgres? We do that in satellite-clone 2) Could we check if postgres has access to the folder and fail with an error message if not? It would look something like this (would have to rubify this)

[vagrant@centos7-katello-3-4 ~]$ sudo -u postgres test -r /backup/
[vagrant@centos7-katello-3-4 ~]$ echo $?
0

any non-zero exit code would mean postgres user doesn't have read access to the folder.

[cfouant]

@johnpmitsch actually we can chown the backup directory to postgres, which we do. But if the base directory doesn't have write access for postgres, then Postgres still cannot write to it. And chowning all the way down to root would be a bad thing.

It also states in the docs that we can only write to /var/tmp or /tmp - this simply makes a failure easier to pinpoint why.

I was looking at your version (checking if postgres has write access) but wasn't really finding any neat ways to do it with ruby. I'll keep looking and see what I can find, but as it stands, we still have the docs saying 'only /tmp or /var/tmp', so this matches up with the docs.

[stbenjam]

Why not backup only postgres to /var/tmp and then move it to the target directory?

[johnpmitsch]

@cfouant Let me share an example of what I am thinking:

This is recreating the steps we take here for two different directories, one in / and the other in /root/

irb(main):001:0> has_access = "/has-access"
=> "/has-access"
irb(main):002:0> no_access = "/root/no-access/"
=> "/root/no-access/"
irb(main):004:0> require "fileutils"
=> true
irb(main):005:0> FileUtils.mkdir_p has_access
=> ["/has-access"]
irb(main):006:0> FileUtils.chown_R nil, 'postgres', has_access
=> ["/has-access"]
irb(main):007:0> FileUtils.chmod_R 0770, has_access
=> ["/has-access"]
irb(main):008:0> FileUtils.mkdir_p no_access
=> ["/root/no-access/"]
irb(main):009:0> FileUtils.chown_R nil, 'postgres', no_access
=> ["/root/no-access/"]
irb(main):010:0> FileUtils.chmod_R 0770, no_access
=> ["/root/no-access/"]

Here is the part I think we should add, its testing if postgres has write access to that directory

irb(main):013:0>  `sudo -u postgres test -w  #{has_access}`
=> ""
irb(main):014:0> $?.success?
=> true
irb(main):015:0>  `sudo -u postgres test -w  #{no_access}`
=> ""
irb(main):016:0> $?.success?
=> false

The directory in /root didn't have write access because its parent directory (/root) is owned by root, and we were able to see that using $?.success? of the test command. We can do the same in the script and print out an error message to the user if the test command fails.

I'm in favor of doing a check like this and leaving it up to the user to ensure the backup directory is in the correct location, this will ensure the psql dumps will run without permission errors and allow the users to place the backup directory wherever it will work.

[ehelms]

@cfouant This looks to have stagnated, what can we do to get this finished up?

[cfouant]

@ehelms - it'll be in before tomorrow! Just tweaking a few details.

[theforeman-bot]

There were the following issues with the commit message:

• length of the first commit message line for 6b321a83be7046e38b3e956af833cea6936cbb74 exceeds 65 characters

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.

---

This message was auto-generated by Foreman's prprocessor

[theforeman-bot]

There were the following issues with the commit message:

• length of the first commit message line for a24c83ea2b5a8ea96e2a3a940192448c0e847042 exceeds 65 characters

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.

---

This message was auto-generated by Foreman's prprocessor

[BenGig]

Checking the backup directory for postgres access makes sense with online backup mode only. Offline backup should skip the test because the postgres user is not involved.