Refs #37678 - Update evr migration to use DSL

[sjha4]

What are the changes introduced in this pull request?

The evr migration was using SQL to drop the extension possibly causing privilege issues in EL8 upgrade pipeline in nightlies.

Considerations taken when implementing this change?

What are the testing steps for this pull request?

To test new installs, run bundle exec rails katello:reset
To test upgrades:
You could follow this on the dev box:
git reset --hard 8fed606
bundle exec rails katello:reset
Create and sync a repo to test data migration. You could also have some host registered to test data migration on installed_packages table
git checkout this branch
run db:migrate and make sure everything works normally.

[sjha4]

@ekohl : I have this PR here based on your reviews: https://github.com/Katello/katello/pull/11087/files#diff-cbabbde17488e6694825cf9687cc62368facc56fa11e554341e05c994b99ca1d .
@ianballou ^^

[ekohl]

If you want the CASCADE option you need to use force: true:

Suggested change

	disable_extension('evr')
	disable_extension('evr', force: true)

See https://github.com/rails/rails/blob/9ba208c16835f4a174ae9fd385ebc18972d758a4/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb#L476-L485

[sjha4]

My dev box is at this version: https://github.com/rails/rails/blob/v6.1.7.8/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb which does a cascade by default and doesn't accept the force parameter.

[ekohl]

Oh, so that's something we need to think about when we upgrade. I also now see it should have been force: :cascade.

[ianballou]

Is it more dangerous to switch to using disable_extension that could change into a non-cascading removal in the future than to keep using raw SQL?

[ianballou]

Worked well for me, tested on a 4.13 to nightly upgrade on Alma 8.

[ekohl]

Worked well for me, tested on a 4.13 to nightly upgrade on Alma 8.

Did you also upgrade from PostgreSQL 12 to 13? I wouldn't expect this PR to just solve the upgrade issue.

[ianballou]

Worked well for me, tested on a 4.13 to nightly upgrade on Alma 8.

Did you also upgrade from PostgreSQL 12 to 13? I wouldn't expect this PR to just solve the upgrade issue.

I ran through the upgrade steps but I can't remember if Postgres 13 was already on my 4.13 box. Do we need to test it on Postgres 12? At least from the docs, users should be on Postgres 13 when they upgrade to nightly / 4.14.

Edit: confirmed I was already on Postgresql 13. I'll try again on 12, but I'm still curious why it's necessary (is it only to unblock nightly?).

[sjha4]

@ekohl Do we have any way of running nightlies against PRs? Is that a thing?

[ekohl]

You need to combine https://theforeman.github.io/forklift/development/#packit-pr-builds and https://theforeman.github.io/forklift/testing/#pipeline-testing but I don't know how well it works since it only should be set up on the last install, not the n-2 and n-1 versions.

It's probably easier to start a pipeline, let it fail and then manually install the patched version and rerun foreman-rake db:migrate. Command I used to start the pipeline:

ansible-playbook pipelines/upgrade_pipeline.yml -e forklift_state=up -e pipeline_os=almalinux8 -e pipeline_type=katello -e pipeline_version=nightly

After that you can use vagrant ssh pipe-up-katello-nightly-almalinux8.

I've reproduced the issue by upgrading from Katello 4.12 instead of from Katello 4.13.

[ianballou]

After some investigation, it's looking like we may need to drop the evr extension from the database via the installer.

Using REASSIGN OWNED BY fails with ERROR: cannot reassign ownership of objects owned by role postgres because they are required by the database system. That appears to be the only way to transfer ownership of an extension.

[ianballou]

This thread might have a way to actually change the ownership (scroll down): https://postgrespro.com/list/thread-id/2333512

[ianballou]

Here's how I fixed it:

foreman=# \d pg_extension 
              Table "pg_catalog.pg_extension"
     Column     |  Type   | Collation | Nullable | Default 
----------------+---------+-----------+----------+---------
 oid            | oid     |           | not null | 
 extname        | name    |           | not null | 
 extowner       | oid     |           | not null | 
 extnamespace   | oid     |           | not null | 
 extrelocatable | boolean |           | not null | 
 extversion     | text    | C         | not null | 
 extconfig      | oid[]   |           |          | 
 extcondition   | text[]  | C         |          | 
Indexes:
    "pg_extension_name_index" UNIQUE, btree (extname)
    "pg_extension_oid_index" UNIQUE, btree (oid)

foreman=# select * from pg_extension ;
  oid  | extname | extowner | extnamespace | extrelocatable | extversion | extconfig | extcondition 
-------+---------+----------+--------------+----------------+------------+-----------+--------------
 13422 | plpgsql |       10 |           11 | f              | 1.0        |           | 
 16910 | evr     |       10 |         2200 | t              | 0.0.2      |           | 
(2 rows)

foreman=# SELECT oid, rolname
foreman-#      FROM pg_authid
foreman-#  WHERE rolname =  'foreman';
  oid  | rolname 
-------+---------
 18263 | foreman
(1 row)

foreman=# SELECT oid, rolname
     FROM pg_authid
 WHERE rolname =  'postgres';
 oid | rolname  
-----+----------
  10 | postgres
(1 row)

foreman=# update pg_extension set extowner = 18263 where extname = 'evr';
UPDATE 1
foreman=# select * from pg_extension ;                        
  oid  | extname | extowner | extnamespace | extrelocatable | extversion | extconfig | extcondition 
-------+---------+----------+--------------+----------------+------------+-----------+--------------
 13422 | plpgsql |       10 |           11 | f              | 1.0        |           | 
 16910 | evr     |    18263 |         2200 | t              | 0.0.2      |           | 
(2 rows)

[ekohl]

Here's how I fixed it:

https://postgrespro.com/list/id/CANu8FiwUHNbRkpVCGRkzqvQZVxTm1v9NsrRWkWU-KKD-W-xfqQ@mail.gmail.com says you also need to update pg_shdepend:

UPDATE pg_shdepend
   SET refobjid = {oid_of_new_owner}
 WHERE refobjid = {oid_of old_owner}
   AND deptype = 'o';

And I doubt the user foreman has permission to do this.

[ianballou]

The pg_shdepend change wasn't necessary for me, but perhaps it left some stale data in the table.

I think the above will blanket change things from postgres to foreman for the foreman DB. So we'd need to find a way to make it only work for the evr related items in the table.

[ianballou]

Some notes on using pg_shdepend:

It looks like you can use the evr extensions oid to find the entry in pg_shdepend and then update the refobjid to be foreman instead of postgres.

foreman=# select * from pg_extension where extname = 'evr';
  oid  | extname | extowner | extnamespace | extrelocatable | extversion | extconfig | extcondition 
-------+---------+----------+--------------+----------------+------------+-----------+--------------
 23270 | evr     |    16386 |         2200 | t              | 0.0.2      |           | 
(1 row)

foreman=# select * from pg_shdepend where objid = 23270;
 dbid  | classid | objid | objsubid | refclassid | refobjid | deptype 
-------+---------+-------+----------+------------+----------+---------
 16387 |    3079 | 23270 |        0 |       1260 |    16386 | o
(1 row)

[ianballou]

For some reference, here's an untested script that I had chatgpt produce just for display purposes:

-- Variables
\set extname 'evr'
\set new_owner 'foreman'

-- Find oid for the extension
WITH extension_info AS (
    SELECT oid AS ext_oid
    FROM pg_extension
    WHERE extname = :'extname'
),

-- Find database ID for the current database
database_info AS (
    SELECT oid AS dbid
    FROM pg_database
    WHERE datname = current_database()
),

-- Find oid for the new owner
owner_info AS (
    SELECT oid AS owner_oid
    FROM pg_authid
    WHERE rolname = :'new_owner'
)

-- Update extowner in pg_extension
UPDATE pg_extension
SET extowner = (SELECT owner_oid FROM owner_info)
WHERE extname = :'extname';

-- Update refobjid in pg_shdepend
UPDATE pg_shdepend
SET refobjid = (SELECT owner_oid FROM owner_info)
WHERE objid = (SELECT ext_oid FROM extension_info)
  AND dbid = (SELECT dbid FROM database_info);

[sjha4]

This seemed to be sufficient to drop the extension and upgrade:

UPDATE pg_extension
SET extowner = (SELECT oid FROM pg_authid WHERE rolname = 'foreman')
WHERE extname = 'evr';

[sjha4]

Thoughts on something like this: theforeman/foreman-installer#955

[ianballou]

Thoughts on something like this: theforeman/foreman-installer#955

I'm okay with this, although maybe we should verify that the pg_shdepend bits get dropped when the evr extension is dropped.
Edit: I verified this is true.

External DB users will need to run the same command via our docs at upgrade time.

[ianballou]

Now that we know using the active record method doesn't fix nightly, do we need this PR? I'm a little worried about the method in question turning into a non-cascading drop.

[sjha4]

Closing. This is not needed anymore. We rewrote the new migration with reviews from here:
#11159