From 8415b77b856467493c696988c468c86ca2ab8f41 Mon Sep 17 00:00:00 2001 From: Johan Fleury Date: Fri, 30 Sep 2022 11:11:58 -0400 Subject: [PATCH 1/3] Support setting per-role config with ALTER ROLE SET --- README.rst | 3 + docs/index.rst | 3 + pgbedrock/attributes.py | 112 ++++++++++++++++++++++++++++++++--- pgbedrock/context.py | 20 +++++++ pgbedrock/memberships.py | 6 +- pgbedrock/spec_inspector.py | 4 +- tests/test_attributes.py | 113 +++++++++++++++++++++++++++--------- tests/test_context.py | 2 +- tests/test_core_generate.py | 20 +++---- tests/test_memberships.py | 2 +- tests/test_privileges.py | 2 +- 11 files changed, 233 insertions(+), 54 deletions(-) diff --git a/README.rst b/README.rst index f9cdbc2..c5940ac 100644 --- a/README.rst +++ b/README.rst @@ -40,6 +40,8 @@ As an example, the definition for the ``jdoe`` role in the spec might look like is_superuser: no attributes: - PASSWORD "{{ env['JDOE_PASSWORD'] }}" + configs: + statement_timeout: 42s member_of: - analyst owns: @@ -75,6 +77,7 @@ When pgbedrock is run, it would make sure that: * ``jdoe`` is not a superuser * ``jdoe``'s password is the same as what is in the ``$JDOE_PASSWORD`` environment variable * All other role attributes for ``jdoe`` are the Postgres defaults (as defined by `pg_authid`_). + * ``jdoe``’s session config ``statement_timeout`` is set to ``42s`` * ``jdoe`` is a member of the ``analyst`` role * ``jdoe`` is a member of no other roles * ``jdoe`` owns the ``finance_reports`` schema diff --git a/docs/index.rst b/docs/index.rst index a2e4e0d..7ced7a2 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -25,6 +25,8 @@ As an example, the definition for the ``jdoe`` role in the spec might look like is_superuser: no attributes: - PASSWORD "{{ env['JDOE_PASSWORD'] }}" + configs: + statement_timeout: 42s member_of: - analyst owns: @@ -58,6 +60,7 @@ When pgbedrock is run, it would make sure that: * ``jdoe`` is not a superuser * ``jdoe``'s password is the same as what is in the ``$JDOE_PASSWORD`` environment variable * All other role attributes for ``jdoe`` are the Postgres defaults (as defined by `pg_authid`_). + * ``jdoe``’s session config ``statement_timeout`` is set to ``42s`` * ``jdoe`` is a member of the ``analyst`` role * ``jdoe`` is a member of no other roles * ``jdoe`` owns the ``finance_reports`` schema diff --git a/pgbedrock/attributes.py b/pgbedrock/attributes.py index 39ad178..cba1b48 100644 --- a/pgbedrock/attributes.py +++ b/pgbedrock/attributes.py @@ -4,6 +4,8 @@ import hmac import base64 import logging +import re +import math import click import psycopg2 @@ -20,7 +22,9 @@ Q_ALTER_CONN_LIMIT = 'ALTER ROLE "{}" WITH CONNECTION LIMIT {}; -- Previous value: {}' Q_ALTER_PASSWORD = "ALTER ROLE \"{}\" WITH ENCRYPTED PASSWORD '{}';" Q_REMOVE_PASSWORD = "ALTER ROLE \"{}\" WITH PASSWORD NULL;" -Q_ALTER_ROLE = 'ALTER ROLE "{}" WITH {};' +Q_ALTER_ROLE_WITH = 'ALTER ROLE "{}" WITH {};' +Q_ALTER_ROLE_SET = 'ALTER ROLE "{}" SET {}="{}"; -- Previous value: {}' +Q_ALTER_ROLE_RESET = 'ALTER ROLE "{}" RESET {}; -- Previous value: {}' Q_ALTER_VALID_UNTIL = "ALTER ROLE \"{}\" WITH VALID UNTIL '{}'; -- Previous value: {}" Q_CREATE_ROLE = 'CREATE ROLE "{}";' @@ -67,17 +71,19 @@ def analyze_attributes(spec, cursor, verbose): show_eta=False, item_show_func=common.item_show_func) as all_roles: all_sql_to_run = [] password_all_sql_to_run = [] - for rolename, spec_config in all_roles: + for rolename, spec_settings in all_roles: logger.debug('Starting to analyze role {}'.format(rolename)) - spec_config = spec_config or {} - spec_attributes = spec_config.get('attributes', []) + spec_settings = spec_settings or {} + spec_attributes = spec_settings.get('attributes', []) for keyword, attribute in (('can_login', 'LOGIN'), ('is_superuser', 'SUPERUSER')): - is_desired = spec_config.get(keyword, False) + is_desired = spec_settings.get(keyword, False) spec_attributes.append(attribute if is_desired else 'NO' + attribute) - roleconf = AttributeAnalyzer(rolename, spec_attributes, dbcontext) + spec_configs = spec_settings.get('configs', {}) + + roleconf = AttributeAnalyzer(rolename, spec_attributes, spec_configs, dbcontext) roleconf.analyze() all_sql_to_run += roleconf.sql_to_run password_all_sql_to_run += roleconf.password_sql_to_run @@ -112,13 +118,15 @@ class AttributeAnalyzer(object): make it match the provided spec attributes. Note that spec_attributes is a list whereas current_attributes is a dict. """ - def __init__(self, rolename, spec_attributes, dbcontext): + def __init__(self, rolename, spec_attributes, spec_configs, dbcontext): self.sql_to_run = [] self.rolename = common.check_name(rolename) logger.debug('self.rolename set to {}'.format(self.rolename)) self.spec_attributes = spec_attributes + self.spec_configs = spec_configs self.current_attributes = dbcontext.get_role_attributes(rolename) + self.current_configs = dbcontext.get_role_configs(rolename) # We keep track of password-related SQL separately as we don't want running this to # go into the main SQL stream since that could leak password @@ -129,7 +137,10 @@ def analyze(self): self.create_role() desired_attributes = self.coalesce_attributes() + self.set_all_attributes(desired_attributes) + self.set_all_configs(self.spec_configs) + return self.sql_to_run def create_role(self): @@ -203,6 +214,12 @@ def get_attribute_value(self, attribute): logger.debug('Returning attribute "{}": "{}"'.format(attribute, value)) return value + def get_config_value(self, config): + """ Take a config (e.g. `statement_timeout`) and look up that value in our dbcontext """ + value = self.current_configs.get(config, "") + logger.debug('Returning config "{}": "{}"'.format(config, value)) + return value + def is_same_password(self, value): """ Compare stored password hash to the new password. @@ -275,10 +292,22 @@ def set_attribute_value(self, attribute, desired_value, current_value): base_keyword = COLUMN_NAME_TO_KEYWORD[attribute] # prepend 'NO' if desired_value is False keyword = base_keyword if desired_value else 'NO' + base_keyword - query = Q_ALTER_ROLE.format(self.rolename, keyword) + query = Q_ALTER_ROLE_WITH.format(self.rolename, keyword) self.sql_to_run.append(query) + def set_all_configs(self, configs): + for config, desired_value in configs.items(): + current_value = self.get_config_value(config) + + if self.parse_config_value(current_value) != self.parse_config_value(desired_value): + self.sql_to_run.append(Q_ALTER_ROLE_SET.format(self.rolename, config, desired_value, current_value)) + + if self.current_configs: + for current_config, current_value in self.current_configs.items(): + if current_config not in configs.keys(): + self.sql_to_run.append(Q_ALTER_ROLE_RESET.format(self.rolename, current_config, current_value)) + def set_password(self, desired_value): if desired_value is None: actual_query = Q_REMOVE_PASSWORD.format(self.rolename) @@ -288,3 +317,70 @@ def set_password(self, desired_value): sanitized_query = Q_ALTER_PASSWORD.format(self.rolename, '******') self.sql_to_run.append('--' + sanitized_query) + + @staticmethod + def parse_config_value(value): + """ + Parse a config (as in postgresql.conf setting) value and return it’s normalized value. + + If the value matches a well-known unit it is rounded to a multiple of the + next smaller unit (if there is one) then it is converted to kB for memory + units and to ms for time units + + If the value doesn’t match a well known unit it is returned as an int or a + float if conversion is possible or as-is otherwise. + + See: https://www.postgresql.org/docs/current/config-setting.html + """ + + if not isinstance(value, str): + return value + + try: + return int(value) + except ValueError: + pass + + try: + return float(value) + except ValueError: + pass + + # Valid memory units are B (bytes), kB (kilobytes), MB (megabytes), GB (gigabytes), and TB (terabytes). + # The multiplier for memory units is 1024, not 1000. + m = re.search("(?P[\-\+]?[0-9]+(\.[0-9]+)?)\s*(?PB|kB|MB|GB|TB)", value) + if m: + quantity = float(m.group("quantity")) + unit = m.group("unit") + + if unit == "B": + return quantity / 1024 + if unit == "kB": + return math.floor(quantity * 1024) / 1024 + if unit == "MB": + return math.floor(quantity * 1024) + if unit == "GB": + return math.floor(quantity * 1024) * 1024 + if unit == "TB": + return math.floor(quantity * 1024) * 1024 ** 2 + + # Valid time units are us (microseconds), ms (milliseconds), s (seconds), min (minutes), h (hours), and d (days). + m = re.search("(?P[\-\+]?[0-9]+(\.[0-9]+)?)\s*(?Pus|ms|s|min|h|d)", value) + if m: + quantity = float(m.group("quantity")) + unit = m.group("unit") + + if unit == "us": + return quantity / 1000 + if unit == "ms": + return math.floor(quantity * 1000) / 1000 + if unit == "s": + return math.floor(quantity * 1000) + if unit == "min": + return math.floor(quantity * 60) * 1000 + if unit == "h": + return math.floor(quantity * 60) * 60 * 1000 + if unit == "d": + return math.floor(quantity * 24) * 60 * 60 * 1000 + + return value diff --git a/pgbedrock/context.py b/pgbedrock/context.py index 3fb89d9..60858dd 100644 --- a/pgbedrock/context.py +++ b/pgbedrock/context.py @@ -124,6 +124,8 @@ ; """ +Q_GET_ALL_ROLE_CONFIGS = "SELECT usename, useconfig FROM pg_shadow;" + Q_GET_ALL_MEMBERSHIPS = """ SELECT auth_member.rolname AS member, @@ -250,6 +252,7 @@ class DatabaseContext(object): 'get_all_current_nondefaults', 'get_all_object_attributes', 'get_all_role_attributes', + 'get_all_role_configs', 'get_all_memberships', 'get_all_nonschema_objects_and_owners', 'get_all_personal_schemas', @@ -428,6 +431,23 @@ def is_superuser(self, rolename): role_attributes = self.get_role_attributes(rolename) return role_attributes.get('rolsuper', False) + def get_all_role_configs(self): + """ Return a dict of {rolname: {config_name: value}} from pg_shadow""" + common.run_query(self.cursor, self.verbose, Q_GET_ALL_ROLE_CONFIGS) + role_configs = {} + for row in self.cursor.fetchall(): + role_configs[row['usename']] = {} + + for config in row['useconfig'] or []: + k, v = config.split('=') + role_configs[row['usename']][k] = v + + return role_configs + + def get_role_configs(self, rolename): + all_role_configs = self.get_all_role_configs() + return all_role_configs.get(rolename, dict()) + def get_all_raw_object_attributes(self): """ Fetch results for all object attributes. diff --git a/pgbedrock/memberships.py b/pgbedrock/memberships.py index 68bdfb4..756b244 100644 --- a/pgbedrock/memberships.py +++ b/pgbedrock/memberships.py @@ -24,9 +24,9 @@ def analyze_memberships(spec, cursor, verbose): with click.progressbar(spec.items(), label='Analyzing memberships:', bar_template=bar_template, show_eta=False, item_show_func=common.item_show_func) as all_roles: all_sql_to_run = [] - for rolename, spec_config in all_roles: - spec_config = spec_config or {} - spec_memberships = set(spec_config.get('member_of', [])) + for rolename, spec_settings in all_roles: + spec_settings = spec_settings or {} + spec_memberships = set(spec_settings.get('member_of', [])) sql_to_run = MembershipAnalyzer(rolename, spec_memberships, dbcontext).analyze() all_sql_to_run += sql_to_run diff --git a/pgbedrock/spec_inspector.py b/pgbedrock/spec_inspector.py index 156c021..47ee96a 100644 --- a/pgbedrock/spec_inspector.py +++ b/pgbedrock/spec_inspector.py @@ -54,6 +54,8 @@ - NOLOGIN - SUPERUSER - NOSUPERUSER + configs: + type: dict member_of: type: list schema: @@ -490,4 +492,4 @@ def ensure_no_except_on_schema(spec): if config and config.get('privileges'): if config['privileges'].get('schemas') and config['privileges']['schemas'].get('except') and config['privileges']['schemas']['excepted']: error_messages.append(EXCEPTED_SCHEMAS_MSG.format(rolename)) - return error_messages \ No newline at end of file + return error_messages diff --git a/tests/test_attributes.py b/tests/test_attributes.py index 921b46d..3a230d2 100644 --- a/tests/test_attributes.py +++ b/tests/test_attributes.py @@ -34,7 +34,7 @@ def roleconf(request, mockdbcontext): role_attributes.update(nondefault_attributes) mockdbcontext.get_role_attributes = lambda x: role_attributes - roleconf = attr.AttributeAnalyzer(rolename=ROLE1, spec_attributes={}, dbcontext=mockdbcontext) + roleconf = attr.AttributeAnalyzer(rolename=ROLE1, spec_attributes={}, spec_configs={}, dbcontext=mockdbcontext) return roleconf @@ -70,7 +70,7 @@ def test_analyze_attributes_modifying_objects(capsys, cursor): keyword = base_keyword if v is True else ('NO' + base_keyword) # prefix = 'NO' if v is False else '' # desired = prefix + k - stmt = attr.Q_ALTER_ROLE.format(role, keyword) + stmt = attr.Q_ALTER_ROLE_WITH.format(role, keyword) elif k == 'rolconnlimit': stmt = attr.Q_ALTER_CONN_LIMIT.format(role, v, attr.DEFAULT_ATTRIBUTES[k]) elif k == 'rolvaliduntil': @@ -79,7 +79,7 @@ def test_analyze_attributes_modifying_objects(capsys, cursor): expected.add(stmt) expected.add(attr.Q_CREATE_ROLE.format(ROLE2)) - expected.add(attr.Q_ALTER_ROLE.format(ROLE2, 'SUPERUSER')) + expected.add(attr.Q_ALTER_ROLE_WITH.format(ROLE2, 'SUPERUSER')) expected.add(attr.Q_CREATE_ROLE.format(ROLE3)) actual, password_changed = attr.analyze_attributes(spec, cursor, verbose=False) @@ -92,7 +92,7 @@ def test_analyze_attributes_modifying_objects(capsys, cursor): def test_analyze_nonexistent_role_with_default_attributes(mockdbcontext): mockdbcontext.get_role_attributes = lambda x: dict() # Analyze the role with default attributes - roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes={}, dbcontext=mockdbcontext) + roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes={}, spec_configs={}, dbcontext=mockdbcontext) roleconf.analyze() assert roleconf.sql_to_run == [ @@ -108,16 +108,16 @@ def test_analyze_nonexistent_role_with_non_default_attributes(mockdbcontext): # Analyze the role with non-default attributes roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes=spec_attributes, - dbcontext=mockdbcontext) + spec_configs={}, dbcontext=mockdbcontext) roleconf.analyze() expected = set([ attr.Q_CREATE_ROLE.format(ROLE1), - attr.Q_ALTER_ROLE.format(ROLE1, 'LOGIN'), - attr.Q_ALTER_ROLE.format(ROLE1, 'SUPERUSER'), - attr.Q_ALTER_ROLE.format(ROLE1, 'CREATEDB'), - attr.Q_ALTER_ROLE.format(ROLE1, 'CREATEROLE'), - attr.Q_ALTER_ROLE.format(ROLE1, 'REPLICATION'), + attr.Q_ALTER_ROLE_WITH.format(ROLE1, 'LOGIN'), + attr.Q_ALTER_ROLE_WITH.format(ROLE1, 'SUPERUSER'), + attr.Q_ALTER_ROLE_WITH.format(ROLE1, 'CREATEDB'), + attr.Q_ALTER_ROLE_WITH.format(ROLE1, 'CREATEROLE'), + attr.Q_ALTER_ROLE_WITH.format(ROLE1, 'REPLICATION'), attr.Q_ALTER_CONN_LIMIT.format(ROLE1, '30', '-1'), attr.Q_ALTER_VALID_UNTIL.format(ROLE1, '2016-08-04', 'None'), ]) @@ -140,14 +140,14 @@ def test_analyze_existing_role_non_default_attributes(mockdbcontext): spec_attributes.extend(['LOGIN', 'SUPERUSER']) roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes=spec_attributes, - dbcontext=mockdbcontext) + spec_configs={}, dbcontext=mockdbcontext) roleconf.analyze() expected = set([ - attr.Q_ALTER_ROLE.format(ROLE1, 'SUPERUSER'), - attr.Q_ALTER_ROLE.format(ROLE1, 'CREATEDB'), - attr.Q_ALTER_ROLE.format(ROLE1, 'CREATEROLE'), + attr.Q_ALTER_ROLE_WITH.format(ROLE1, 'SUPERUSER'), + attr.Q_ALTER_ROLE_WITH.format(ROLE1, 'CREATEDB'), + attr.Q_ALTER_ROLE_WITH.format(ROLE1, 'CREATEROLE'), attr.Q_ALTER_CONN_LIMIT.format(ROLE1, '30', '27'), attr.Q_ALTER_VALID_UNTIL.format(ROLE1, '2016-08-04', 'None'), ]) @@ -166,7 +166,7 @@ def test_role_exists_true(roleconf): def test_role_exists_false(mockdbcontext, roleconf): mockdbcontext.get_role_attributes = lambda x: dict() - roleconf = attr.AttributeAnalyzer(DUMMY, spec_attributes=DUMMY, dbcontext=mockdbcontext) + roleconf = attr.AttributeAnalyzer(DUMMY, spec_attributes=DUMMY, spec_configs={}, dbcontext=mockdbcontext) assert not roleconf.role_exists() @@ -184,7 +184,7 @@ def test_converted_attributes_defaults(roleconf): @pytest.mark.parametrize('bogus_attribute', [('INVALID'), ('NOINVALID')]) def test_converted_attributes_invalid_attribute(capsys, mockdbcontext, bogus_attribute): roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes=[bogus_attribute], - dbcontext=mockdbcontext) + spec_configs={}, dbcontext=mockdbcontext) with pytest.raises(SystemExit): roleconf.converted_attributes() @@ -195,14 +195,14 @@ def test_converted_attributes_connection_limit(mockdbcontext): """ Make sure converted_attributes parses a connection limit attribute successfully, i.e. that it splits the string and converts the second part to an int """ roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes=['CONNECTION LIMIT 11'], - dbcontext=mockdbcontext) + spec_configs={}, dbcontext=mockdbcontext) attributes = roleconf.converted_attributes() assert attributes['rolconnlimit'] == 11 def test_converted_attributes_valid_until(mockdbcontext): roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes=["VALID UNTIL '2018-08-08'"], - dbcontext=mockdbcontext) + spec_configs={}, dbcontext=mockdbcontext) attributes = roleconf.converted_attributes() assert attributes['rolvaliduntil'] == "'2018-08-08'" @@ -210,7 +210,7 @@ def test_converted_attributes_valid_until(mockdbcontext): def test_converted_attributes_password(mockdbcontext): password_val = 'supeRSecret' roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes=["PASSWORD '{}'".format(password_val)], - dbcontext=mockdbcontext) + spec_configs={}, dbcontext=mockdbcontext) attributes = roleconf.converted_attributes() assert attributes['rolpassword'] == password_val @@ -219,7 +219,7 @@ def test_converted_attributes_password(mockdbcontext): def test_converted_attributes_password_error_on_quotes(capsys, mockdbcontext, password): with pytest.raises(SystemExit): roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes=["PASSWORD {}".format(password)], - dbcontext=mockdbcontext) + spec_configs={}, dbcontext=mockdbcontext) roleconf.converted_attributes() expected_err_msg = attr.UNSUPPORTED_CHAR_MSG.format(ROLE1) + '\n' @@ -229,7 +229,7 @@ def test_converted_attributes_password_error_on_quotes(capsys, mockdbcontext, pa def test_converted_attributes_boolean_attribute(mockdbcontext): set_attributes = ['LOGIN', 'NOINHERIT', 'CREATEROLE', 'BYPASSRLS'] roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes=set_attributes, - dbcontext=mockdbcontext) + spec_configs={}, dbcontext=mockdbcontext) converted_attributes = roleconf.converted_attributes() for opt in set_attributes: @@ -245,7 +245,7 @@ def test_coalesce_attributes(mockdbcontext): mockdbcontext.get_role_attributes = lambda x: copy.deepcopy(attr.DEFAULT_ATTRIBUTES) set_attributes = ['BYPASSRLS', 'CREATEDB', 'NOINHERIT', 'REPLICATION'] roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes=set_attributes, - dbcontext=mockdbcontext) + spec_configs={}, dbcontext=mockdbcontext) actual = roleconf.coalesce_attributes() expected = copy.deepcopy(attr.DEFAULT_ATTRIBUTES) @@ -270,12 +270,12 @@ def test_set_all_attributes(mockdbcontext): mockdbcontext.get_role_attributes = lambda x: copy.deepcopy(attr.DEFAULT_ATTRIBUTES) set_attributes = ['BYPASSRLS', 'CREATEDB', 'CREATEROLE', 'REPLICATION'] roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes=set_attributes, - dbcontext=mockdbcontext) + spec_configs={}, dbcontext=mockdbcontext) attributes = roleconf.coalesce_attributes() roleconf.set_all_attributes(attributes) actual = set(roleconf.sql_to_run) - expected = {attr.Q_ALTER_ROLE.format(ROLE1, opt) for opt in set_attributes} + expected = {attr.Q_ALTER_ROLE_WITH.format(ROLE1, opt) for opt in set_attributes} assert actual == expected @@ -290,7 +290,7 @@ def test_set_all_attributes_change_skips_same_password(mockdbcontext, password): mockdbcontext.get_role_attributes = lambda x: role_attributes attributes = ['PASSWORD {}'.format(password)] - roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes=attributes, dbcontext=mockdbcontext) + roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes=attributes, spec_configs={}, dbcontext=mockdbcontext) attributes = roleconf.coalesce_attributes() roleconf.set_all_attributes(attributes) assert roleconf.sql_to_run == [] @@ -317,7 +317,7 @@ def test_get_attribute_value(mockdbcontext, optname, optval): ) mockdbcontext.get_role_attributes = lambda x: role_attributes - roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes=[], dbcontext=mockdbcontext) + roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes=[], spec_configs={}, dbcontext=mockdbcontext) assert roleconf.get_attribute_value(optname) == optval @@ -351,7 +351,7 @@ def test_set_attribute_value(roleconf, optname, optval): base_keyword = attr.COLUMN_NAME_TO_KEYWORD[optname] # prepend 'NO' if desired_value is False keyword = base_keyword if optval is True else ('NO' + base_keyword) - expected = [attr.Q_ALTER_ROLE.format(ROLE1, keyword)] + expected = [attr.Q_ALTER_ROLE_WITH.format(ROLE1, keyword)] actual = roleconf.sql_to_run assert actual == expected @@ -361,8 +361,8 @@ def test_set_attribute_value_sql_to_run(roleconf): assert len(roleconf.sql_to_run) == 0 roleconf.set_attribute_value(attribute='rolcanlogin', desired_value=True, current_value='_') roleconf.set_attribute_value(attribute='rolsuper', desired_value=True, current_value='_') - assert roleconf.sql_to_run == [attr.Q_ALTER_ROLE.format(ROLE1, 'LOGIN'), - attr.Q_ALTER_ROLE.format(ROLE1, 'SUPERUSER')] + assert roleconf.sql_to_run == [attr.Q_ALTER_ROLE_WITH.format(ROLE1, 'LOGIN'), + attr.Q_ALTER_ROLE_WITH.format(ROLE1, 'SUPERUSER')] def test_set_attribute_value_valid_until(roleconf): @@ -448,3 +448,58 @@ def test_set_password_log_message_is_masked(capsys, cursor): _, password_all_sql_to_run = attr.analyze_attributes(spec, cursor, verbose=False) assert password_all_sql_to_run == [attr.Q_ALTER_PASSWORD.format(ROLE1, new_password)] + + +def test_get_config_value(mockdbcontext): + role_configs = { + 'statement_timeout': 42 + } + mockdbcontext.get_role_configs = lambda x: role_configs + + roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes=[], spec_configs={}, dbcontext=mockdbcontext) + for k, v in role_configs.items(): + assert roleconf.get_config_value(k) == v + + +def test_set_all_configs(mockdbcontext): + mockdbcontext.get_role_configs = lambda x: { + 'existing_extra': 'test', + } + + spec_configs = { + 'statement_timeout': 42, + 'idle_in_transaction_session_timeout': 180, + } + + roleconf = attr.AttributeAnalyzer(ROLE1, spec_attributes=[], spec_configs=spec_configs, dbcontext=mockdbcontext) + roleconf.set_all_configs(spec_configs) + + actual = set(roleconf.sql_to_run) + expected = set(attr.Q_ALTER_ROLE_SET.format(ROLE1, k, v, '') for k, v in spec_configs.items()) + expected.add(attr.Q_ALTER_ROLE_RESET.format(ROLE1, 'existing_extra', 'test')) + assert actual == expected + +@pytest.mark.parametrize('value, expected', [ + # No units + (None, None), + ("foo", "foo") + ('1', 1), + ('1.1', 1.1), + # Memory units + ('1B', 0.0009765625), + ('1kB', 1.0), + ('1 kB', 1.0), + ('30.1GB', 31_561_728), + ('1TB', 1_073_741_824), + # Time units + ('1us', 0.001), + ('1ms', 1), + ('1s', 1_000), + ('1 s', 1_000), + ('1min', 60_000), + ('1h', 3_600_000), + ('1d', 86_400_000), + ] +) +def test_parse_config_value(value, expected): + assert attr.AttributeAnalyzer.parse_config_value(value) == expected diff --git a/tests/test_context.py b/tests/test_context.py index 47e07c9..e6f94fc 100644 --- a/tests/test_context.py +++ b/tests/test_context.py @@ -298,7 +298,7 @@ def test_get_all_object_attributes(cursor): [attributes.Q_CREATE_ROLE.format(r) for r in ROLES[:3]] + # Grant login permission to 2 of the roles - [attributes.Q_ALTER_ROLE.format(r, 'LOGIN') for r in ROLES[1:3]] + + [attributes.Q_ALTER_ROLE_WITH.format(r, 'LOGIN') for r in ROLES[1:3]] + # Create personal schemas (i.e. schemas named identically to their owner) [ownerships.Q_CREATE_SCHEMA.format(r, r) for r in ROLES[:3]] diff --git a/tests/test_core_generate.py b/tests/test_core_generate.py index 1326248..15b3f65 100644 --- a/tests/test_core_generate.py +++ b/tests/test_core_generate.py @@ -274,18 +274,18 @@ def test_add_ownerships(mockdbcontext): # role1's personal schema has an object in it attr.Q_CREATE_ROLE.format('role1'), - attr.Q_ALTER_ROLE.format('role1', 'LOGIN'), + attr.Q_ALTER_ROLE_WITH.format('role1', 'LOGIN'), own.Q_CREATE_SCHEMA.format('role1', 'role1'), Q_CREATE_TABLE.format('role1', 'role1', 'table0'), # role2's personal schema has no objects attr.Q_CREATE_ROLE.format('role2'), - attr.Q_ALTER_ROLE.format('role2', 'LOGIN'), + attr.Q_ALTER_ROLE_WITH.format('role2', 'LOGIN'), own.Q_CREATE_SCHEMA.format('role2', 'role2'), # role3's personal schema has several objects in it attr.Q_CREATE_ROLE.format('role3'), - attr.Q_ALTER_ROLE.format('role3', 'LOGIN'), + attr.Q_ALTER_ROLE_WITH.format('role3', 'LOGIN'), own.Q_CREATE_SCHEMA.format('role3', 'role3'), Q_CREATE_TABLE.format('role3', 'role3', 'table1'), Q_CREATE_TABLE.format('role3', 'role3', 'table2'), @@ -312,7 +312,7 @@ def test_collapse_personal_schemas(cursor, objects, expected): # role1's personal schema has an object in it attr.Q_CREATE_ROLE.format('role1'), - attr.Q_ALTER_ROLE.format('role1', 'LOGIN'), + attr.Q_ALTER_ROLE_WITH.format('role1', 'LOGIN'), own.Q_CREATE_SCHEMA.format('role1', 'role1'), Q_CREATE_TABLE.format('role1', 'role1', 'table0'), @@ -323,7 +323,7 @@ def test_collapse_personal_schemas(cursor, objects, expected): # role3's personal schema has several objects in it attr.Q_CREATE_ROLE.format('role3'), - attr.Q_ALTER_ROLE.format('role3', 'LOGIN'), + attr.Q_ALTER_ROLE_WITH.format('role3', 'LOGIN'), own.Q_CREATE_SCHEMA.format('role3', 'role3'), Q_CREATE_TABLE.format('role3', 'role3', 'table1'), Q_CREATE_TABLE.format('role3', 'role3', 'table2'), @@ -358,14 +358,14 @@ def test_collapse_personal_schemas_no_personal_schemas_exist(cursor): # role1's personal schema has an object in it attr.Q_CREATE_ROLE.format('role1'), - attr.Q_ALTER_ROLE.format('role1', 'LOGIN'), + attr.Q_ALTER_ROLE_WITH.format('role1', 'LOGIN'), own.Q_CREATE_SCHEMA.format('role1', 'role1'), Q_CREATE_TABLE.format('role1', 'role1', 'table0'), # role2's personal schema has no objects. We could grant a default privilege here but all # that matters is that our input to the collapse_personal_schemas() function includes 'role2.*' attr.Q_CREATE_ROLE.format('role2'), - attr.Q_ALTER_ROLE.format('role2', 'LOGIN'), + attr.Q_ALTER_ROLE_WITH.format('role2', 'LOGIN'), own.Q_CREATE_SCHEMA.format('role2', 'role2'), ]) def test_collapse_personal_schemas_empty_schema_with_default_priv(cursor): @@ -485,9 +485,9 @@ def test_determine_schema_privileges_only_read_exists(cursor): attr.Q_CREATE_ROLE.format('role0'), attr.Q_CREATE_ROLE.format('role1'), attr.Q_CREATE_ROLE.format('role2'), - attr.Q_ALTER_ROLE.format('role0', 'LOGIN'), - attr.Q_ALTER_ROLE.format('role1', 'LOGIN'), - attr.Q_ALTER_ROLE.format('role2', 'LOGIN'), + attr.Q_ALTER_ROLE_WITH.format('role0', 'LOGIN'), + attr.Q_ALTER_ROLE_WITH.format('role1', 'LOGIN'), + attr.Q_ALTER_ROLE_WITH.format('role2', 'LOGIN'), # Create three personal schemas own.Q_CREATE_SCHEMA.format('role0', 'role0'), diff --git a/tests/test_memberships.py b/tests/test_memberships.py index 37f7fb1..3029a76 100644 --- a/tests/test_memberships.py +++ b/tests/test_memberships.py @@ -22,7 +22,7 @@ attributes.Q_CREATE_ROLE.format(CURRENT_GROUP1), attributes.Q_CREATE_ROLE.format(DESIRED_GROUP1), attributes.Q_CREATE_ROLE.format(DESIRED_GROUP2), - attributes.Q_ALTER_ROLE.format(ROLE1, 'SUPERUSER'), + attributes.Q_ALTER_ROLE_WITH.format(ROLE1, 'SUPERUSER'), memb.Q_GRANT_MEMBERSHIP.format(CURRENT_GROUP1, ROLE3), ]) def test_analyze_memberships(cursor): diff --git a/tests/test_privileges.py b/tests/test_privileges.py index 6d0a644..934bb03 100644 --- a/tests/test_privileges.py +++ b/tests/test_privileges.py @@ -232,7 +232,7 @@ def test_analyze_privileges(cursor): @run_setup_sql([ attributes.Q_CREATE_ROLE.format(ROLES[0]), - attributes.Q_ALTER_ROLE.format(ROLES[0], 'SUPERUSER') + attributes.Q_ALTER_ROLE_WITH.format(ROLES[0], 'SUPERUSER') ]) def test_analyze_privileges_skips_superuser(cursor): desired_spec = yaml.safe_load(""" From 63355d9e18805db7d6b6d38d49f90fdd370e7bfb Mon Sep 17 00:00:00 2001 From: Dennis Zhang Date: Wed, 5 Oct 2022 09:35:10 -0700 Subject: [PATCH 2/3] fix Q_GET_ALL_CURRENT_NONDEFAULTS missing partition tables --- pgbedrock/context.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pgbedrock/context.py b/pgbedrock/context.py index 60858dd..30ba468 100644 --- a/pgbedrock/context.py +++ b/pgbedrock/context.py @@ -54,6 +54,7 @@ ('v', 'tables'), ('m', 'tables'), ('f', 'tables'), + ('p', 'tables'), ('S', 'sequences') ), tables_and_sequences AS ( SELECT From 2531cda1820ca544f2e3f8759941d144ba61f9f7 Mon Sep 17 00:00:00 2001 From: Johan Fleury Date: Fri, 30 Sep 2022 11:18:54 -0400 Subject: [PATCH 3/3] Bump to version 0.5.0 --- CHANGELOG.md | 9 ++++++++- pgbedrock/__init__.py | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba5b031..30ef3b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,10 +7,17 @@ and this project tries to adhere to [Semantic Versioning](http://semver.org/spec ## [Unreleased] _this space intentionally left blank_ +## [0.5.0] - 2022-09-30 + +### In Code + +- BREAKING CHANGE: Add support for setting per-role config with ALTER ROLE SET (@johanfleury) +pgbedrock will now remove any existing config that’s not defined in its configuration. + ## [0.4.4] - 2022-09-06 ### In Code -- Remove Python 2 support +- Remove Python 2 support (@johanfleury) - Add support for SCRAM-SHA-256 passwords (passwords can now be passed plain text or hashed) (@johanfleury) ## [0.4.3] - 2022-09-01 diff --git a/pgbedrock/__init__.py b/pgbedrock/__init__.py index b64a672..ae11dda 100644 --- a/pgbedrock/__init__.py +++ b/pgbedrock/__init__.py @@ -1,2 +1,2 @@ -__version__ = '0.4.3' +__version__ = '0.5.0' LOG_FORMAT = '%(levelname)s:%(filename)s:%(funcName)s:%(lineno)s - %(message)s'