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

Add tests against latest stable releases of Postgres 11, 12, & 13 #67

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

richardmcmillen
Copy link

@richardmcmillen richardmcmillen commented Mar 21, 2021

TLDR; Add tests against latest stable releases of Postgres 11, 12, & 13

Thanks to this PR #59 for giving me a head start. However, I didn't end up integrating some of the changes in their diff. Unless I am missing something I believe it was how the test failures manifested which led them down the path of having to do the except ownership but I don't think this is necessary.

Although it should be restated that the tool probably does work fine on versions 11, 12, 13 and this change is more about allowing the project to prove this officially by updating the expectations in tests.

Diff explanation

The changes in the tests were driven by two things:

  • New roles in newer versions of Postgres which need to be conditionally handled depending on Postgres version under test.
  • Since version 11, when creating the Postgres cluster using the official Docker image the standard postgres superuser is not created. There were some assumptions made in tests that reasonably expected this user to be there which need to be conditionally handled depending on the version under test.

Concretely the differences can be seen by generating a spec from freshly initialized Postgres clusters. This uses parts of the Makefile already in the repository.

make create_network

for version in 10.4 11.11 12.6 13.2
do
  export POSTGRES_VERSION="$version"
  make start_postgres
  make wait_for_postgres
  docker run -it \
    --net pgbedrock_network \
    quay.io/squarespace/pgbedrock generate \
    -h pgbedrock_postgres \
    -p 5432 \
    -d test_db \
    -U test_user \
    -w test_password | tee "/tmp/postgres-$version.yml"
done

make stop_postgres remove_network

I will put the contents of each version here but the main difference is between 10.4 and 11.11. It appears that since version 11 the postgres superuser isn't created when given an alternative user through the POSTGRES_USER environment variable. As a consequence of this the user provided owns all the items which the postgres superuser would have before.

In addition to these changes, you can see that version 11 also introduces some new roles not existing in older versions.

postgres-10.4.yml
# postgres-10.4.yml
pg_monitor:
    member_of:
        - pg_read_all_settings
        - pg_read_all_stats
        - pg_stat_scan_tables

pg_read_all_settings:

pg_read_all_stats:

pg_stat_scan_tables:

postgres:
    attributes:
        - BYPASSRLS
        - CREATEDB
        - CREATEROLE
        - REPLICATION
    can_login: true
    is_superuser: true
    owns:
        schemas:
            - information_schema
            - pg_catalog
            - public
        tables:
            - information_schema.*
            - pg_catalog.*
    privileges:
        schemas:
            write:
                - information_schema
                - pg_catalog
                - public

test_user:
    attributes:
        - PASSWORD "{{ env['TEST_USER_PASSWORD'] }}"
    can_login: true
    is_superuser: true
postgres-11.11.yml
# postgres-11.11.yml
pg_execute_server_program:

pg_monitor:
    member_of:
        - pg_read_all_settings
        - pg_read_all_stats
        - pg_stat_scan_tables

pg_read_all_settings:

pg_read_all_stats:

pg_read_server_files:

pg_stat_scan_tables:

pg_write_server_files:

test_user:
    attributes:
        - BYPASSRLS
        - CREATEDB
        - CREATEROLE
        - PASSWORD "{{ env['TEST_USER_PASSWORD'] }}"
        - REPLICATION
    can_login: true
    is_superuser: true
    owns:
        schemas:
            - information_schema
            - pg_catalog
            - public
        tables:
            - information_schema.*
            - pg_catalog.*
    privileges:
        schemas:
            write:
                - information_schema
                - pg_catalog
                - public
postgres-12.6.yml
# postgres-12.6.yml
pg_execute_server_program:

pg_monitor:
    member_of:
        - pg_read_all_settings
        - pg_read_all_stats
        - pg_stat_scan_tables

pg_read_all_settings:

pg_read_all_stats:

pg_read_server_files:

pg_stat_scan_tables:

pg_write_server_files:

test_user:
    attributes:
        - BYPASSRLS
        - CREATEDB
        - CREATEROLE
        - PASSWORD "{{ env['TEST_USER_PASSWORD'] }}"
        - REPLICATION
    can_login: true
    is_superuser: true
    owns:
        schemas:
            - information_schema
            - pg_catalog
            - public
        tables:
            - information_schema.*
            - pg_catalog.*
    privileges:
        schemas:
            write:
                - information_schema
                - pg_catalog
                - public
postgres-13.2.yml
# postgres-13.2.yml
pg_execute_server_program:

pg_monitor:
    member_of:
        - pg_read_all_settings
        - pg_read_all_stats
        - pg_stat_scan_tables

pg_read_all_settings:

pg_read_all_stats:

pg_read_server_files:

pg_stat_scan_tables:

pg_write_server_files:

test_user:
    attributes:
        - BYPASSRLS
        - CREATEDB
        - CREATEROLE
        - PASSWORD "{{ env['TEST_USER_PASSWORD'] }}"
        - REPLICATION
    can_login: true
    is_superuser: true
    owns:
        schemas:
            - information_schema
            - pg_catalog
            - public
        tables:
            - information_schema.*
            - pg_catalog.*
    privileges:
        schemas:
            write:
                - information_schema
                - pg_catalog
                - public

Summary

The changes I have made here a very brute force attempt at getting everything working here so maybe there is a more elegant way to address these changes. For now, all the commits are marked WIP and I intend to tidy them up, squash them, update the docs, and add more code comments if the maintainers of the repository are interested in having this submission. It's just at this juncture I am unsure about the maintainer status of the project and so I don't want to sink too much time in unless there is a chance of getting things merged.

A general sidenote just to say that I love the idea of this project and it would be great to use if I can address some of the other issues (#3, #49, #17) which I am fairly motivated to do. Once again, many thanks. :)

As of 21 March 2021 running `make build` on the default branch of this
repository breaks with the following error:

```
    ERROR: Command errored out with exit status 1:
     command: /usr/local/bin/python -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pip-install-bfkg7hzw/markupsafe_2ff6d76a552d4703960c9f49f15673c8/setup.py'"'"'; __file__='"'"'/tmp/pip-install-bfkg7hzw/markupsafe_2ff6d76a552d4703960c9f49f15673c8/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info --egg-base /tmp/pip-pip-egg-info-hn261ajv
         cwd: /tmp/pip-install-bfkg7hzw/markupsafe_2ff6d76a552d4703960c9f49f15673c8/
    Complete output (5 lines):
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-install-bfkg7hzw/markupsafe_2ff6d76a552d4703960c9f49f15673c8/setup.py", line 6, in <module>
        from setuptools import setup, Extension, Feature
    ImportError: cannot import name 'Feature'
    ----------------------------------------
WARNING: Discarding https://files.pythonhosted.org/packages/4d/de/32d741db316d8fdb7680822dd37001ef7a448255de9699ab4bfcbdf4172b/MarkupSafe-1.0.tar.gz#sha256=a6be69091dac236ea9c6bc7d012beab42010fa914c459791d627dad4910eb665 (from https://pypi.org/simple/markupsafe/). Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.
ERROR: Could not find a version that satisfies the requirement MarkupSafe==1.0
ERROR: No matching distribution found for MarkupSafe==1.0
```

There is a Closed issue logged in the MarkupSafe repository,
pallets/markupsafe#116 where a maintainer explained upgrading shouldn't
have any compatibility issues.

So the MarkupSafe requirement has been set to 1.1.1
@richardmcmillen richardmcmillen changed the title Add tests for postgresql 11 12 13 Add tests against latest stable releases of Postgres 11, 12, & 13 Mar 21, 2021
@coveralls
Copy link

coveralls commented Mar 21, 2021

Pull Request Test Coverage Report for Build 228

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.672%

Totals Coverage Status
Change from base Build 224: 0%
Covered Lines: 1278
Relevant Lines: 1322

💛 - Coveralls

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.

None yet

2 participants