postgresql_user: fix bugs related to 'expires' option #23862

pilou- · 2017-04-21T13:44:46Z

SUMMARY

Add tests and fix various errors related to the expires option. The last 4 commits improve readability.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

postgresql_user

ANSIBLE VERSION

ansible-playbook 2.4.0 (devel 8ec4882ba0)

ADDITIONAL INFORMATION

Locally tested using:

ANSIBLE_REMOTE_USER=root ANSIBLE_ROLES_PATH=test/integration/targets ansible-playbook -i 'testhost,' -e ansible_host=10.0.1.129 --tags=test_postgresql,test_postgresql_db,test_postgresql_privs,test_postgresql_user,needs_privileged test/integration/destructive.yml

Related pull-requests:

PostgreSQL integration test suite: really generate locales on Debian #23613 PostgreSQL integration test suite: really generate locales on Debian
PostgreSQL Integration test suite: improvements #23631 PostgreSQL Integration test suite: improvements
postgresql_user: use standard module hashlib instead of passlib #23686 postgresql_user: use standard module hashlib instead of passlib

ansibot · 2017-04-21T13:52:55Z

cc @nerzhul
click here for bot help

mscherer

All look goods, just one question.

mscherer · 2017-04-21T14:06:30Z

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

-        for i in range(len(current_role_attrs)):
-            if current_role_attrs[i] != new_role_attrs[i]:
-                changed = True
+        changed = current_role_attrs != new_role_attrs


So , would it work for all psycopg2 version ?

psycopg2.extras.DictRow was already here in 1.99.12 (which was released in march 2005).

drrtuy · 2017-05-02T03:56:31Z

There is one more problem with "expires" code: there is no column rol_valid_until in a pg_authid relation. So this code always compare expires with a None. It should be changed.

pilou- · 2017-05-09T12:36:05Z

@drrtuy thanks for pointing that. I will update this pull request after the merge of #23686.

ansibot · 2017-05-31T02:15:01Z

cc @matburt
click here for bot help

ansibot · 2017-05-31T02:25:21Z

The test ansible-test sanity --test yamllint failed with the following error:

test/integration/targets/postgresql/tasks/test_no_password_change.yml:70:4: syntax error: expected <block end>, but found '<block sequence start>'

click here for bot help

pilou- · 2017-05-31T22:57:32Z

@matburt @drrtuy @mscherer : ready_for_review

abadger · 2017-06-06T19:25:50Z

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

 from hashlib import md5
 import itertools
 import re

-from distutils.version import StrictVersion


I'd move the hashlib import down here... imports are usually

import stdlib_foo from stdlib_foo import bar import thridparty_foo from thirdparty_foo import bar import project_foo from project_foo import bar

hashlib module belongs to the Python standard library (2.7, 3.6.1).

Based on @abadger's comment, due to it being from hashlib import md5 it should go below import re

oh right, I misunderstood! I fixed that and other PEP 8 related problems.

mscherer · 2017-06-10T07:29:50Z

shipit

abadger · 2017-06-11T21:52:58Z

Merged to devel. Doesn't cherrypick cleanly and I'm going on a one week vacation followed by ansiblefest London so I didn't want to backport it. If you think it should be backported, feel free to ask one of the other devs if they think it should be.

pilou- · 2017-06-12T21:54:15Z

This pull-request contains 4 one-liner fixes and many commits related to the tests. Without the bugfixes, expires parameter isn't usable.

Are there any recommendations/policies about backport of bugfixes ?

I have created a branch based on stable-2.3 with the 4 one-liners cherry picked. I have referenced the squashed commit, should I use commit id before the squash instead ?

… (#26539) * Change 'valid until' even it's the only updated field (cherry picked from commit 460d932) * value is changed when another value is provided (cherry picked from commit 460d932) * value isn't returned when unset (cherry picked from commit 460d932) * Fix comparison between user input and applied configuration (cherry picked from commit 460d932)

#23862) (#26539)" This reverts commit 5432414.

… (#26539) * Change 'valid until' even it's the only updated field (cherry picked from commit 460d932) * value is changed when another value is provided (cherry picked from commit 460d932) * value isn't returned when unset (cherry picked from commit 460d932) * Fix comparison between user input and applied configuration (cherry picked from commit 460d932) (cherry picked from commit 5432414)

ansibot added affects_2.4 This issue/PR affects Ansible v2.4 bugfix_pull_request community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. labels Apr 21, 2017

mscherer reviewed Apr 21, 2017

View reviewed changes

ansibot removed the needs_triage Needs a first human triage before being processed. label Apr 21, 2017

pilou- changed the title ~~postgresql_user: fix bugs related to 'expires' option~~ [WIP] postgresql_user: fix bugs related to 'expires' option Apr 21, 2017

ansibot added the WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. label Apr 21, 2017

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 May 2, 2017

pilou- force-pushed the postgresql_user_fix_valid_until branch from 4359c85 to fb1253c Compare May 31, 2017 02:08

pilou- changed the title ~~[WIP] postgresql_user: fix bugs related to 'expires' option~~ postgresql_user: fix bugs related to 'expires' option May 31, 2017

pilou- force-pushed the postgresql_user_fix_valid_until branch from fb1253c to 97dda60 Compare May 31, 2017 02:16

ansibot removed the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label May 31, 2017

pilou- force-pushed the postgresql_user_fix_valid_until branch 4 times, most recently from d3a7acd to 018236b Compare May 31, 2017 14:11

Refactor: deduplicate tasks

a7c6c9d

pilou- added 6 commits May 31, 2017 17:58

postgresql_user: define variable just before using it

d6b6f0b

Fix comparison between user input and applied configuration

7252877

new test: adding an invalid attribute

537b25e

Refactor, add cleaning task

41ec615

Check that using same attribute a 2nd time does nothing

f39107a

Always try to remove created user

c8000d0

pilou- force-pushed the postgresql_user_fix_valid_until branch from c8e9495 to c8000d0 Compare May 31, 2017 15:58

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 May 31, 2017

pilou- mentioned this pull request Jun 6, 2017

Public Core Meeting Meeting - April, May 2017 ansible/community#159

Closed

thaumos mentioned this pull request Jun 6, 2017

Public Core Meeting Agenda - June, July 2017 ansible/community#162

Closed

abadger reviewed Jun 6, 2017

View reviewed changes

postgresql_user: fix pep8

7c93c13

abadger merged commit 460d932 into ansible:devel Jun 11, 2017

pilou- mentioned this pull request Jul 7, 2017

postgresql_user: fix bugs related to 'expires' option (backport #23862) #26539

Merged

abadger added a commit that referenced this pull request Jul 7, 2017

Revert "postgresql_user: fix bugs related to 'expires' option (backport

eb76c13

#23862) (#26539)" This reverts commit 5432414.

abadger added a commit that referenced this pull request Jul 7, 2017

Revert "postgresql_user: fix bugs related to 'expires' option (backport

23020bf

#23862) (#26539)" This reverts commit 5432414.

ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 6, 2018

dagwieers added the postgresql PostgreSQL community label Feb 1, 2019

dagwieers added the database Database category label Feb 13, 2019

ansible locked and limited conversation to collaborators Apr 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

postgresql_user: fix bugs related to 'expires' option #23862

postgresql_user: fix bugs related to 'expires' option #23862

pilou- commented Apr 21, 2017 •

edited

ansibot commented Apr 21, 2017

mscherer left a comment

mscherer Apr 21, 2017

pilou- Apr 21, 2017

drrtuy commented May 2, 2017

pilou- commented May 9, 2017

ansibot commented May 31, 2017

ansibot commented May 31, 2017

pilou- commented May 31, 2017

abadger Jun 6, 2017

pilou- Jun 6, 2017

sivel Jun 6, 2017

pilou- Jun 6, 2017

mscherer commented Jun 10, 2017

abadger commented Jun 11, 2017

pilou- commented Jun 12, 2017 •

edited

postgresql_user: fix bugs related to 'expires' option #23862

postgresql_user: fix bugs related to 'expires' option #23862

Conversation

pilou- commented Apr 21, 2017 • edited

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ADDITIONAL INFORMATION

ansibot commented Apr 21, 2017

mscherer left a comment

Choose a reason for hiding this comment

mscherer Apr 21, 2017

Choose a reason for hiding this comment

pilou- Apr 21, 2017

Choose a reason for hiding this comment

drrtuy commented May 2, 2017

pilou- commented May 9, 2017

ansibot commented May 31, 2017

ansibot commented May 31, 2017

pilou- commented May 31, 2017

abadger Jun 6, 2017

Choose a reason for hiding this comment

pilou- Jun 6, 2017

Choose a reason for hiding this comment

sivel Jun 6, 2017

Choose a reason for hiding this comment

pilou- Jun 6, 2017

Choose a reason for hiding this comment

mscherer commented Jun 10, 2017

abadger commented Jun 11, 2017

pilou- commented Jun 12, 2017 • edited

pilou- commented Apr 21, 2017 •

edited

pilou- commented Jun 12, 2017 •

edited