Skip to content
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

postgresql_privs: Support FOREIGN DATA WRAPPER and FOREIGN SERVER #38803

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,3 @@
---
minor_changes:
- "postgresql_privs - introduces support for FOREIGN DATA WRAPPER and FOREIGN SERVER as object types in postgresql_privs module. (https://github.com/ansible/ansible/issues/38801)"
43 changes: 40 additions & 3 deletions lib/ansible/modules/database/postgresql/postgresql_privs.py
Expand Up @@ -41,10 +41,11 @@
description:
- Type of database object to set privileges on.
Akasurde marked this conversation as resolved.
Show resolved Hide resolved
- The `default_prives` choice is available starting at version 2.7.
- The 'foreign_data_wrapper' and 'foreign_server' object types are available from Ansible version '2.8'.
default: table
choices: [table, sequence, function, database,
schema, language, tablespace, group,
default_privs]
default_privs, foreign_data_wrapper, foreign_server]
objs:
description:
- Comma separated list of database objects to set privileges on.
Expand Down Expand Up @@ -272,6 +273,23 @@
type: default_privs
role: reader

# Available since version 2.8
# GRANT ALL PRIVILEGES ON FOREIGN DATA WRAPPER fdw TO reader
- postgresql_privs:
db: test
objs: fdw
privs: ALL
type: foreign_data_wrapper
role: reader

# GRANT ALL PRIVILEGES ON FOREIGN SERVER fdw_server TO reader
- postgresql_privs:
db: test
objs: fdw_server
privs: ALL
type: foreign_server
role: reader

"""

import traceback
Expand Down Expand Up @@ -484,6 +502,18 @@ def get_default_privs(self, schema, *args):
self.cursor.execute(query, (schema,))
return [t[0] for t in self.cursor.fetchall()]

def get_foreign_data_wrapper_acls(self, fdws):
query = """SELECT fdwacl FROM pg_catalog.pg_foreign_data_wrapper
WHERE fdwname = ANY (%s) ORDER BY fdwname"""
self.cursor.execute(query, (fdws,))
return [t[0] for t in self.cursor.fetchall()]

def get_foreign_server_acls(self, fs):
query = """SELECT srvacl FROM pg_catalog.pg_foreign_server
WHERE srvname = ANY (%s) ORDER BY srvname"""
self.cursor.execute(query, (fs,))
return [t[0] for t in self.cursor.fetchall()]

# Manipulating privileges

def manipulate_privs(self, obj_type, privs, objs, roles,
Expand Down Expand Up @@ -525,6 +555,10 @@ def manipulate_privs(self, obj_type, privs, objs, roles,
get_status = self.get_group_memberships
elif obj_type == 'default_privs':
get_status = partial(self.get_default_privs, schema_qualifier)
elif obj_type == 'foreign_data_wrapper':
get_status = self.get_foreign_data_wrapper_acls
elif obj_type == 'foreign_server':
get_status = self.get_foreign_server_acls
else:
raise Error('Unsupported database object type "%s".' % obj_type)

Expand Down Expand Up @@ -559,7 +593,8 @@ def manipulate_privs(self, obj_type, privs, objs, roles,
obj_ids = [pg_quote_identifier(i, 'table') for i in obj_ids]
# Note: obj_type has been checked against a set of string literals
# and privs was escaped when it was parsed
set_what = '%s ON %s %s' % (','.join(privs), obj_type,
# Note: Underscores are replaced with spaces to support multi-word obj_type
set_what = '%s ON %s %s' % (','.join(privs), obj_type.replace('_', ' '),
','.join(obj_ids))

# for_whom: SQL-fragment specifying for whom to set the above
Expand Down Expand Up @@ -706,7 +741,9 @@ def main():
'language',
'tablespace',
'group',
'default_privs']),
'default_privs',
'foreign_data_wrapper',
'foreign_server']),
objs=dict(required=False, aliases=['obj']),
schema=dict(required=False),
roles=dict(required=True, aliases=['role']),
Expand Down
3 changes: 3 additions & 0 deletions test/integration/targets/postgresql/tasks/main.yml
Expand Up @@ -777,6 +777,9 @@
# Test postgresql_tablespace module
- include: postgresql_tablespace.yml

# Test postgresql_privs
- include: postgresql_privs.yml

# dump/restore tests per format
# ============================================================
- include: state_dump_restore.yml test_fixture=user file=dbdata.sql
Expand Down
239 changes: 239 additions & 0 deletions test/integration/targets/postgresql/tasks/postgresql_privs.yml
@@ -0,0 +1,239 @@
---

######################################################
# Test foreign data wrapper and foreign server privs #
######################################################

- name: Create DB
become_user: "{{ pg_user }}"
become: True
postgresql_db:
state: present
name: "{{ db_name }}"
login_user: "{{ pg_user }}"
register: result

- name: Create test role
become: True
become_user: "{{ pg_user }}"
shell: echo "CREATE ROLE fdw_test" | psql -d "{{ db_name }}"

- name: Create fdw extension
become: True
become_user: "{{ pg_user }}"
shell: echo "CREATE EXTENSION postgres_fdw" | psql -d "{{ db_name }}"

- name: Create foreign data wrapper
become: True
become_user: "{{ pg_user }}"
shell: echo "CREATE FOREIGN DATA WRAPPER dummy" | psql -d "{{ db_name }}"

- name: Create foreign server
become: True
become_user: "{{ pg_user }}"
shell: echo "CREATE SERVER dummy_server FOREIGN DATA WRAPPER dummy" | psql -d "{{ db_name }}"

- name: Grant foreign data wrapper privileges
postgresql_privs:
state: present
type: foreign_data_wrapper
roles: fdw_test
privs: ALL
objs: dummy
db: "{{ db_name }}"
login_user: "{{ pg_user }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

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

register: result
ignore_errors: yes

- assert:
that:
- "result.changed == true"

- name: Get foreign data wrapper privileges
become: True
become_user: "{{ pg_user }}"
shell: echo "{{ fdw_query }}" | psql -d "{{ db_name }}"
vars:
fdw_query: >
SELECT fdwacl FROM pg_catalog.pg_foreign_data_wrapper
WHERE fdwname = ANY (ARRAY['dummy']) ORDER BY fdwname
register: fdw_result

- assert:
that:
- "fdw_result.stdout_lines[-1] == '(1 row)'"
- "'fdw_test' in fdw_result.stdout_lines[-2]"

- name: Grant foreign data wrapper privileges second time
postgresql_privs:
state: present
type: foreign_data_wrapper
roles: fdw_test
privs: ALL
objs: dummy
db: "{{ db_name }}"
login_user: "{{ pg_user }}"
register: result
ignore_errors: yes

- assert:
that:
- "result.changed == false"

- name: Revoke foreign data wrapper privileges
postgresql_privs:
state: absent
type: foreign_data_wrapper
roles: fdw_test
privs: ALL
objs: dummy
db: "{{ db_name }}"
login_user: "{{ pg_user }}"
register: result
ignore_errors: yes

- assert:
that:
- "result.changed == true"

- name: Get foreign data wrapper privileges
become: True
become_user: "{{ pg_user }}"
shell: echo "{{ fdw_query }}" | psql -d "{{ db_name }}"
vars:
fdw_query: >
SELECT fdwacl FROM pg_catalog.pg_foreign_data_wrapper
WHERE fdwname = ANY (ARRAY['dummy']) ORDER BY fdwname
register: fdw_result

- assert:
that:
- "fdw_result.stdout_lines[-1] == '(1 row)'"
- "'fdw_test' not in fdw_result.stdout_lines[-2]"

- name: Revoke foreign data wrapper privileges for second time
postgresql_privs:
state: absent
type: foreign_data_wrapper
roles: fdw_test
privs: ALL
objs: dummy
db: "{{ db_name }}"
login_user: "{{ pg_user }}"
register: result
ignore_errors: yes

- assert:
that:
- "result.changed == false"

- name: Grant foreign server privileges
postgresql_privs:
state: present
type: foreign_server
roles: fdw_test
privs: ALL
objs: dummy_server
db: "{{ db_name }}"
login_user: "{{ pg_user }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, to make your test complete:

  1. try to grant both privs again and check that changed == false
  2. try to drop both privs and check that changed == true
  3. try to drop both privs again and check that changed == false

register: result
ignore_errors: yes

- assert:
that:
- "result.changed == true"

- name: Get foreign server privileges
become: True
become_user: "{{ pg_user }}"
shell: echo "{{ fdw_query }}" | psql -d "{{ db_name }}"
vars:
fdw_query: >
SELECT srvacl FROM pg_catalog.pg_foreign_server
WHERE srvname = ANY (ARRAY['dummy_server']) ORDER BY srvname
register: fs_result

- assert:
that:
- "fs_result.stdout_lines[-1] == '(1 row)'"
- "'fdw_test' in fs_result.stdout_lines[-2]"

- name: Grant foreign server privileges for second time
postgresql_privs:
state: present
type: foreign_server
roles: fdw_test
privs: ALL
objs: dummy_server
db: "{{ db_name }}"
login_user: "{{ pg_user }}"
register: result
ignore_errors: yes

- assert:
that:
- "result.changed == false"

- name: Revoke foreign server privileges
postgresql_privs:
state: absent
type: foreign_server
roles: fdw_test
privs: ALL
objs: dummy_server
db: "{{ db_name }}"
login_user: "{{ pg_user }}"
register: result
ignore_errors: yes

- assert:
that:
- "result.changed == true"

- name: Get foreign server privileges
become: True
become_user: "{{ pg_user }}"
shell: echo "{{ fdw_query }}" | psql -d "{{ db_name }}"
vars:
fdw_query: >
SELECT srvacl FROM pg_catalog.pg_foreign_server
WHERE srvname = ANY (ARRAY['dummy_server']) ORDER BY srvname
register: fs_result

- assert:
that:
- "fs_result.stdout_lines[-1] == '(1 row)'"
- "'fdw_test' not in fs_result.stdout_lines[-2]"

- name: Revoke foreign server privileges for second time
postgresql_privs:
state: absent
type: foreign_server
roles: fdw_test
privs: ALL
objs: dummy_server
db: "{{ db_name }}"
login_user: "{{ pg_user }}"
register: result
ignore_errors: yes

- assert:
that:
- "result.changed == false"

- name: Cleanup
become: True
become_user: "{{ pg_user }}"
shell: echo "{{ item }}" | psql -d "{{ db_name }}"
with_items:
- DROP ROLE fdw_test
- DROP FOREIGN DATA WRAPPER dummy
- DROP SERVER dummy_server

- name: Destroy DB
become_user: "{{ pg_user }}"
become: True
postgresql_db:
state: absent
name: "{{ db_name }}"
login_user: "{{ pg_user }}"