postgresql_privs: Support FOREIGN DATA WRAPPER and FOREIGN SERVER #38803

lichensky · 2018-04-16T09:20:55Z

SUMMARY

This introduces support for FOREIGN DATA WRAPPER and FOREIGN SERVER as object types in postgresql_privs module.

Issues:
#38801

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

postgresql_privs

ANSIBLE VERSION

ansible 2.6.0
  config file = /etc/ansible/ansible.cfg
  python version = 2.7.13 (default, Nov 24 2017, 17:33:09) [GCC 6.3.0 20170516]

ansibot · 2018-04-16T09:30:18Z

cc @b6d @matburt
click here for bot help

Akasurde

Rest LGTM, need one more person to take a look.

lib/ansible/modules/database/postgresql/postgresql_privs.py

Akasurde · 2018-04-17T13:05:51Z

Thanks. We need one more person to look at this and then we can proceed.

Akasurde · 2018-04-17T13:09:15Z

cc @maveric-tellrv Would you like to review this PR ?

ansibot · 2019-02-09T03:17:39Z

cc @Andersson007 @amenonsen @andytom @dagwieers @jbscalia @kostiantyn-nemchenko @nerzhul
click here for bot help

ansibot · 2019-02-13T19:34:08Z

cc @Dorn- @kustodian
click here for bot help

lichensky · 2019-03-02T09:43:03Z

@Andersson007 Hi, conflicts resolved.

Andersson007

LGTM,
a couple of notices:

Add examples with new types to EXAMPLE block.
use the following example's form, (not as the existing examples):

- name: ....
  postgresql_privs:
    ...

To be sure that it won't brake down, would be better to add CI tests (for 'foreign_data_wrapper' type only is enough) to a new file like
test/integration/targets/postgresql/tasks/postgresql_privs.yml and include it to main.yml in the same dir.
You can use for example test/integration/targets/postgresql/tasks/postgresql_query.yml from the current dev branch.
Add the note about 2.8 as Akasurde said in April.

lichensky · 2019-03-06T20:40:47Z

Thanks for checking it out. I've added integration tests and mentioned changes.

Andersson007 · 2019-03-07T07:41:41Z

test/integration/targets/postgresql/tasks/postgresql_privs.yml

+    privs: ALL
+    objs: dummy
+    db: "{{ db_name }}"
+    login_user: "{{ pg_user }}"


I think we should check here at least that changed == true when we grant these privs

Suggested change

login_user: "{{ pg_user }}"

login_user: "{{ pg_user }}"

register: result

ignore_errors: yes

- assert:

that:

- result.changed == true

Andersson007 · 2019-03-07T07:46:46Z

test/integration/targets/postgresql/tasks/postgresql_privs.yml

+    privs: ALL
+    objs: dummy_server
+    db: "{{ db_name }}"
+    login_user: "{{ pg_user }}"


Suggested change

login_user: "{{ pg_user }}"

login_user: "{{ pg_user }}"

register: result

ignore_errors: yes

- assert:

that:

- result.changed == true

I think we should check here at least that changed == true when we grant these privs too because maybe this actually won't return changed == true. We can't be sure if we don't check

Also, to make your test complete:

try to grant both privs again and check that changed == false

try to drop both privs and check that changed == true

try to drop both privs again and check that changed == false

Andersson007 · 2019-03-07T07:59:38Z

Also, please, add a changelog fragment https://docs.ansible.com/ansible/devel/community/development_process.html#making-your-pr-merge-worthy

…odule

lichensky · 2019-03-08T08:50:35Z

Done

Andersson007

shipit

Andersson007 · 2019-03-08T09:05:44Z

@Akasurde , seems 2 shipits. Ready for merge

Andersson007 · 2019-03-08T09:40:49Z

@lichensky , @Akasurde , thank you, good job!
@lichensky , We have several tasks and new modules https://github.com/ansible/community/wiki/PostgreSQL. Would be better if you look at one, e.g. postgresql_tables. I think this module will be quite useful.
You can write yourself in the group as a member. Welcome!
Also you can find there some ideas to implement.

Andersson007 · 2019-03-08T10:38:06Z

If you have any questions, you may ask me directly by aaklychkov at mail dot ru

bcoca removed the needs_triage Needs a first human triage before being processed. label Apr 16, 2018

Akasurde requested changes Apr 17, 2018

View reviewed changes

lib/ansible/modules/database/postgresql/postgresql_privs.py Show resolved Hide resolved

ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Apr 17, 2018

Akasurde approved these changes Apr 17, 2018

View reviewed changes

ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Apr 17, 2018

ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Apr 25, 2018

Akasurde mentioned this pull request May 14, 2018

postgresql_privs: Support FOREIGN DATA WRAPPER and FOREIGN SERVER #38801

Closed

ansibot added the affects_2.6 This issue/PR affects Ansible v2.6 label May 19, 2018

ansibot added support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed support:community This issue/PR relates to code supported by the Ansible community. labels Sep 16, 2018

ansibot added support:community This issue/PR relates to code supported by the Ansible community. and removed support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Oct 2, 2018

dagwieers added the postgresql PostgreSQL community label Feb 1, 2019

dagwieers added the database Database category label Feb 13, 2019

lichensky force-pushed the feature/postgresql_privs_fdw_and_fs_obj_types branch from 30977cf to f8449cd Compare March 1, 2019 16:18

Andersson007 suggested changes Mar 4, 2019

View reviewed changes

ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Mar 4, 2019

lichensky force-pushed the feature/postgresql_privs_fdw_and_fs_obj_types branch from 3e42652 to f85b55a Compare March 6, 2019 20:35

ansibot added the test This PR relates to tests. label Mar 6, 2019

Andersson007 suggested changes Mar 7, 2019

View reviewed changes

lichensky force-pushed the feature/postgresql_privs_fdw_and_fs_obj_types branch from f85b55a to c826d78 Compare March 7, 2019 20:10

lichensky and others added 3 commits March 7, 2019 21:12

Support FOREIGN DATA WRAPPER and FOREIGN SERVER in postgresql_privs m…

575c165

…odule

Added available from note to fdw and fs object types

8da29c0

Integration tests, examples in documentation

6c692b1

lichensky force-pushed the feature/postgresql_privs_fdw_and_fs_obj_types branch 2 times, most recently from 043e8ed to 6842299 Compare March 8, 2019 07:37

Complete integration tests

6842299

Andersson007 approved these changes Mar 8, 2019

View reviewed changes

ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Mar 8, 2019

Akasurde merged commit 6e19848 into ansible:devel Mar 8, 2019

ansible locked and limited conversation to collaborators Jul 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

postgresql_privs: Support FOREIGN DATA WRAPPER and FOREIGN SERVER #38803

postgresql_privs: Support FOREIGN DATA WRAPPER and FOREIGN SERVER #38803

lichensky commented Apr 16, 2018

ansibot commented Apr 16, 2018

Akasurde left a comment

Akasurde commented Apr 17, 2018 •

edited

Akasurde commented Apr 17, 2018

ansibot commented Feb 9, 2019

ansibot commented Feb 13, 2019

lichensky commented Mar 2, 2019

Andersson007 left a comment •

edited

lichensky commented Mar 6, 2019

Andersson007 Mar 7, 2019

Andersson007 Mar 7, 2019

Andersson007 Mar 7, 2019

Andersson007 commented Mar 7, 2019 •

edited

lichensky commented Mar 8, 2019

Andersson007 left a comment

Andersson007 commented Mar 8, 2019

Andersson007 commented Mar 8, 2019 •

edited

Andersson007 commented Mar 8, 2019

-    login_user: "{{ pg_user }}"
+    login_user: "{{ pg_user }}"
+    register: result
+    ignore_errors: yes
+- assert:
+    that:
+    - result.changed == true

postgresql_privs: Support FOREIGN DATA WRAPPER and FOREIGN SERVER #38803

postgresql_privs: Support FOREIGN DATA WRAPPER and FOREIGN SERVER #38803

Conversation

lichensky commented Apr 16, 2018

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ansibot commented Apr 16, 2018

Akasurde left a comment

Choose a reason for hiding this comment

Akasurde commented Apr 17, 2018 • edited

Akasurde commented Apr 17, 2018

ansibot commented Feb 9, 2019

ansibot commented Feb 13, 2019

lichensky commented Mar 2, 2019

Andersson007 left a comment • edited

Choose a reason for hiding this comment

lichensky commented Mar 6, 2019

Andersson007 Mar 7, 2019

Choose a reason for hiding this comment

Andersson007 Mar 7, 2019

Choose a reason for hiding this comment

Andersson007 Mar 7, 2019

Choose a reason for hiding this comment

Andersson007 commented Mar 7, 2019 • edited

lichensky commented Mar 8, 2019

Andersson007 left a comment

Choose a reason for hiding this comment

Andersson007 commented Mar 8, 2019

Andersson007 commented Mar 8, 2019 • edited

Andersson007 commented Mar 8, 2019

Akasurde commented Apr 17, 2018 •

edited

Andersson007 left a comment •

edited

Andersson007 commented Mar 7, 2019 •

edited

Andersson007 commented Mar 8, 2019 •

edited