Skip to content

Make local backup of pg_hba.conf file before modification #138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 6, 2017

Conversation

vutny
Copy link
Contributor

@vutny vutny commented Jan 13, 2017

This PR enables local backup creation of modified pg_hba.conf file by default.
The config_backup Pillar setting has been introduced to trigger backup for all modified PostgreSQL server configuration files (postgresconf and pg_hba.conf).

It's very basic and useful feature, I think. @BABILEN or @javierbertoli , could you please review this? Thank you.

@EvaSDK
Copy link
Contributor

EvaSDK commented Jan 19, 2017

Any specific reason to choose the local copy of pg_hba to .bak instead of using the file.managed backup system like the postgresql-conf state ?

@vutny
Copy link
Contributor Author

vutny commented Jan 19, 2017

Yeah, there is a reason. Good question, thanks! 👍

The backup parameter works completely different in file.blockreplace and file.managed states.
The approach from file.blockreplace is naive: make the backup at the same directory and append provided backup extension to the file name.

The file.managed state processes backup argument in more complex way, see: https://docs.saltstack.com/en/latest/ref/states/backup_mode.html
This is more advanced, but not always convenient. Especially if someone else manages PostgreSQL with other tools than Salt. Also, you can configure that in a Minion config.

That's why I "emulated" the behavior of backup to be similar to postgresql-conf (file.blockreplace), but just for pg_hba.conf (file.managed) using separate file.copy state.

@EvaSDK
Copy link
Contributor

EvaSDK commented Jan 19, 2017

Ok, I thought that file.blockreplace and file.managed were behaving the same so 👍 for consistency.

pillar.example Outdated
{%- if 'status.time' in salt.keys() %}
postgresconf_backup: ".backup@{{ salt['status.time']('%y-%m-%d_%H:%M:%S') }}"
{%- endif %}
listen_addresses = '*' # listen on all interfaces
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like there was an indentation change with list_addresses?

Copy link

@myoung34 myoung34 Feb 5, 2017

Choose a reason for hiding this comment

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

Likely not on purpose, should be fixed before merge, but it's just for the example. Might cause confusion however.. It's also missing the -

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, I guess I switched context too often and started to mix things :)
Indeed, YAML (and SLS) literal style scalars should be indented exactly with two spaces. Fixed, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And since it is just a multi-line string which is directly used in the file.blockreplace context parameter in the same way (literal style), there should be no - list indicator.

@vutny
Copy link
Contributor Author

vutny commented Feb 6, 2017

@javierbertoli, could you please review this? Many thanks.

@javierbertoli
Copy link
Member

Looks good to me. Thanks @vutny and @EvaSDK!

@javierbertoli javierbertoli merged commit 3c2d65b into saltstack-formulas:master Feb 6, 2017
@vutny vutny deleted the backup-hba branch February 7, 2017 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants