openstax / cnx-deploy

3 stars 2 forks source link

Migrations occasionally require superuser privileges #267

Closed mmulich closed 7 years ago

mmulich commented 7 years ago

Summary

We've recently changed the database user to a non-superuser after the initial provisioning has happened. As a result of this we have migrations that will not complete as coded. The following is example output of running the tea environment:

RUNNING HANDLER [repository_db : migrate repository database] ******************
fatal: [tea00.cnx.org]: FAILED! => {"changed": true, "cmd": ["/var/cnx/venvs/publishing/bin/dbmigrator", "--config", "/etc/cnx/archive/app.ini", "--context", "cnx-db", "migrate"], "delta": "0:00:01.547163", "end": "2017-04-07 16:41:24.461041", "failed": true, "rc": 1, "start": "2017-04-07 16:41:22.913878", "stderr": "Traceback (most recent call last):\n  File \"/var/cnx/venvs/publishing/bin/dbmigrator\", line 11, in <module>\n    sys.exit(main())\n  File \"/var/cnx/venvs/publishing/local/lib/python2.7/site-packages/dbmigrator/cli.py\", line 89, in main\n    return args['cmmd'](**args)\n  File \"/var/cnx/venvs/publishing/local/lib/python2.7/site-packages/dbmigrator/utils.py\", line 83, in wrapper\n    return func(cursor, *args, **kwargs)\n  File \"/var/cnx/venvs/publishing/local/lib/python2.7/site-packages/dbmigrator/commands/migrate.py\", line 30, in cli_command\n    migration)\n  File \"/var/cnx/venvs/publishing/local/lib/python2.7/site-packages/dbmigrator/utils.py\", line 150, in compare_schema\n    callback(*args, **kwargs)\n  File \"/var/cnx/venvs/publishing/local/lib/python2.7/site-packages/dbmigrator/utils.py\", line 161, in run_migration\n    migration.up(cursor)\n  File \"../../var/cnx/venvs/publishing/src/cnx-db/cnxdb/migrations/20170208213802_ident-hash-func.py\", line 20, in up\n    cursor.execute(f.read())\npsycopg2.ProgrammingError: permission denied for language plpythonu", "stdout": "Running migration 20170208213802 ident-hash-func", "stdout_lines": ["Running migration 20170208213802 ident-hash-func"], "warnings": []}

Environment

tea specifically, but generally all

Notes

The discussion so far has leaned toward making this a coding issue because there are times when creation will by default assign ownership to the creating role, which may conflict with the role actually using the database via our applications.

The proposal is to make minor modifications to db-migrator to enable the use of a superuser when necessary. ( @reedstrm will fill in the implementation details )

mmulich commented 7 years ago

To resolve this in the mean time, we will give the database role back their superuser privileges.

reedstrm commented 7 years ago

Another approach is to write migrations that attempt to escalate to superuser using the guaranteed to exist postgres user.

As to potential changes to dbmigrator - I can think of a couple options. One is up_super() and down_super() which are alternate methods to call, supplied with a cursor guaranteed to have superuser powers - then dbmigrator would need to take an additional --super-user <username> option, defaulting to postgres probably. Good migration scripts would need to know the expected non-superuser username, to use when creating objects in a up_super() script. Would need to define behavior for which order to run the scripts in, if both are present.

Hmm, how about a @with_super decorator instead, which then means the so decorated function will receive two cursors, one regular, one superuser? And if you need the username, extract it from the first one with cur.execute('SELECT CURRENT_USER')

mmulich commented 7 years ago

In the meantime we've introduced the xxx_archive_db_user_role_attr_flags variable. See also the specific line that handles this.

karenc commented 7 years ago

Super user context manager is in db-migrator v1.0.0: https://github.com/karenc/db-migrator/pull/43

karenc commented 7 years ago

We started using the super user context manager in db-migrator in our migrations and this worked in production when we deployed it.