uyuni icon indicating copy to clipboard operation
uyuni copied to clipboard

Switch to using pg_roles instead of pg_authid

Open KeithMnemonic opened this issue 2 years ago • 3 comments

What does this PR change?

This PR switches two sql queries to use the pg_roles table instead of pg_authid. When using a Cloud Based "RDS" such as AWS, access to the pg_authid user is restricted as the cloud provider does not expose this.

GUI diff

No difference.

  • [ ] DONE

Documentation

  • No documentation needed: only internal and user invisible changes

  • [ ] DONE

Test coverage

  • No tests: already covered

  • [ ] DONE

Links

AWS limits access to pg_authid

  • [ ] DONE

Changelogs

Make sure the changelogs entries you are adding are compliant with https://github.com/uyuni-project/uyuni/wiki/Contributing#changelogs and https://github.com/uyuni-project/uyuni/wiki/Contributing#uyuni-projectuyuni-repository

If you don't need a changelog check, please mark this checkbox:

  • [] No changelog needed

If you uncheck the checkbox after the PR is created, you will need to re-run changelog_test (see below)

Re-run a test

If you need to re-run a test, please mark the related checkbox, it will be unchecked automatically once it has re-run:

  • [ X] Re-run test "changelog_test"
  • [ ] Re-run test "backend_unittests_pgsql"
  • [ ] Re-run test "java_pgsql_tests"
  • [ ] Re-run test "schema_migration_test_pgsql"
  • [ ] Re-run test "susemanager_unittests"
  • [ ] Re-run test "javascript_lint"
  • [ ] Re-run test "spacecmd_unittests"

KeithMnemonic avatar Jul 05 '22 15:07 KeithMnemonic

running CI tests: https://ci.suse.de/view/Manager/view/Uyuni-PRs/job/uyuni-prs-ci-tests/1400/

mbussolotto avatar Jul 06 '22 07:07 mbussolotto

@moio @mackdk anything further from Keith to address the previous review feedback?

jeremy-moffitt avatar Jul 25 '22 17:07 jeremy-moffitt

I will run the testsuite on this. Let's see what it say

mcalmer avatar Aug 03 '22 14:08 mcalmer

@mcalmer can we merge this now? Is there anything else needed ?

KeithMnemonic avatar Aug 12 '22 14:08 KeithMnemonic

@KeithMnemonic ~our testsuite had some problems in the past because of the AC problems and other issues with the code base. I try to run things again. I hope we can merge next week.~

mcalmer avatar Aug 12 '22 14:08 mcalmer

@KeithMnemonic forget my last comment. This was the small PR. This can be merged.

mcalmer avatar Aug 12 '22 14:08 mcalmer

@KeithMnemonic created a port for SUMA 4.3. I think we want to have this feature for 4.3

mcalmer avatar Aug 12 '22 14:08 mcalmer

Is it expected to not have any schema upgrade script for this change?

cbosdo avatar Aug 17 '22 12:08 cbosdo

@mcalmer ?

juliogonzalez avatar Aug 17 '22 13:08 juliogonzalez

pg_roles already exists: it is part of the PostgreSQL system catalog https://www.postgresql.org/docs/current/view-pg-roles.html

mackdk avatar Aug 17 '22 13:08 mackdk

Right. This is just a different way to query the same information.

mcalmer avatar Aug 17 '22 16:08 mcalmer