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.

Sign up for GitHub

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

Jump to bottom

Feature: policy_is_permissive to check permissive/restrictive #343

Open
iamed2 opened this issue Nov 6, 2024 · 5 comments · May be fixed by #344
Open

Feature: policy_is_permissive to check permissive/restrictive #343

iamed2 opened this issue Nov 6, 2024 · 5 comments · May be fixed by #344

Comments

@iamed2
Copy link

iamed2 commented Nov 6, 2024

This would sit parallel to policy_cmd_is, checking pg_catalog.pg_policy.polpermissive (boolean not null) (introduced in PostgreSQL 10).

Implementation of the base case:

-- policy_is_permissive( schema, table, policy, description )
CREATE OR REPLACE FUNCTION policy_is_permissive( NAME, NAME, NAME, text )
RETURNS TEXT AS $$
DECLARE
    permissive boolean;
BEGIN
    SELECT pp.polpermissive
      FROM pg_catalog.pg_policy AS pp
      JOIN pg_catalog.pg_class AS pc ON pc.oid = pp.polrelid
      JOIN pg_catalog.pg_namespace AS pn ON pn.oid = pc.relnamespace
     WHERE pn.nspname = $1
       AND pc.relname = $2
       AND pp.polname = $3
      INTO permissive;

    RETURN ok( permissive, $4 );
END;
$$ LANGUAGE plpgsql;

and then either policy_is_restrictive or policy_isnt_permissive.
@theory
Copy link
Owner

theory commented Nov 17, 2024

Should there also be policy_is_restrictive()? Or one function to cover both?

@iamed2
Copy link
Author

iamed2 commented Nov 19, 2024

I think it makes sense to have that, yes. Having two functions goes along with other e.g. is/isnt conditions, and I now think policy_is_restrictive is more intuitive than policy_isnt_permissive (which matches how PostgreSQL stores the flag, but not how people write policies).

@theory
Copy link
Owner

theory commented Nov 23, 2024 •
edited
Loading

Agreed that policy_is_restrictive is a better name (it's generally better to use positives in function names rather than negatives).

Would you like to make a pull request, ideally with tests?

@iamed2
Copy link
Author

iamed2 commented Nov 25, 2024

I'll set some time for it on Thursday and see what I can accomplish.

I think I understand everything I need to do except how the compat stuff works (this is a 10+ feature). Is it enough to create one patch file like this one for 9.6 for my migration SQL, or is there more? Would my tests need to indicate somehow that they're 10+?

@theory
Copy link
Owner

theory commented Nov 25, 2024

There are two approaches:

If the code is compatible with 9.6 and earlier, but just doesn't function properly, then we structure the test so that it doesn't execute the tests on 9.6 and earlier, usually by checking the version in a test function and just emitting expected output on earlier versions.

If the code is not compatible, and won't even compile, then a patch is necessary on 9.6 and earlier to remove the code. I think we'd just append the relevant patch hunks to this patch.

Let me know if you run into difficulty, I can probably help get it over the finish line.

@iamed2 iamed2 linked a pull request Nov 29, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

WIP policy_is_permissive, policy_is_restrictive iamed2/pgtap
2 participants
@theory @iamed2