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 functions to ownership list #30

Open
clrcrl opened this issue May 16, 2018 · 1 comment · May be fixed by #62
Open

Add functions to ownership list #30

clrcrl opened this issue May 16, 2018 · 1 comment · May be fixed by #62
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@clrcrl
Copy link

clrcrl commented May 16, 2018

Functions, like tables and schemas, have an owner, which should also be managed by pgbedrock.

For example

jdoe:
    ...
    owns:
        schemas:
            - finance_reports
        tables:
            - finance_reports.Q2_revenue
            - finance_reports.Q2_margin
        functions:
            - f_cast_string_as_number
@zcmarine
Copy link
Contributor

Definitely! I think this should be quite doable, though if we handle ownership for functions we should also handle permissions for them as well, though that is probably two separate PRs as those ideas don't have to be linked (although the privilege map mentioned below in Ownership point 1 is used on both parts and might force them to be more coupled).

Below is a brief overview on what would likely need to be done. If you have any interest in taking a crack at it let me know! Otherwise this will be on the roadmap but may be prioritized below a few other issues like #3 (Provide multi-database support).

Adding function ownership:
There are three places that we would need to modify:

  1. Context.py would need two changes: The query pulls in ownerships (here) would need to take functions in as well and the privilege map (here) would need to have functions in it.
  2. We'd want to assert via tests that the spec_inspector.py is properly checking a real function. The relevant checks in that module that should catch if someone is off in the spec are here.
  3. We'd want to assert via tests that ownership.py properly works with a real function. This should probably be done for analyze_ownerships() (and could just be an in-place modification of an existing test) and possibly a test for NonschemaAnalyzer specifically.

Adding function privileges:

  1. Modify the context.py queries that get default and non-default privileges.
  2. Add tests verifying that privileges.py operates correctly over default and non-default privileges for a real function. Again, these could be in-place modifications of tests or duplicates of tests.

It may sound like a lot, but other than getting the queries right and getting comfortable with how pgbedrock works, it shouldn't be too difficult as I think pgbedrock's machinery is generalized enough that once the queries are correct the rest of the code should just work (though emphasis on "should", of course).

@zcmarine zcmarine added the enhancement New feature or request label May 31, 2018
@zcmarine zcmarine added the good first issue Good for newcomers label Jun 29, 2018
@rdunklau rdunklau linked a pull request Apr 1, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants