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

Support setting default_privileges on all schemas #1298

Merged
merged 1 commit into from
Nov 1, 2021

Conversation

fish-face
Copy link
Contributor

The Postgres default is for the absent specification of a schema name
when altering default privileges to apply to all schemas.
Support that behaviour, but keep the current default behaviour for
an unset schema parameter.

This means that the behaviour doesn't match up with the postgres specification - a better match would be if leaving the schema parameter unset also left off the IN SCHEMA clause, but this would break backwards compatibility.

@fish-face fish-face requested a review from a team as a code owner September 15, 2021 14:55
@puppet-community-rangefinder
Copy link

postgresql::server::default_privileges is a type

that may have no external impact to Forge modules.

This module is declared in 70 of 578 indexed public Puppetfiles.


These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@fish-face fish-face force-pushed the allow_unset_schema branch 2 times, most recently from 6f17bc7 to 340dd70 Compare September 15, 2021 15:16
Comment on lines 138 to 139
'absent' => "SELECT 1 WHERE NOT EXISTS (SELECT * FROM pg_default_acl AS da LEFT JOIN pg_namespace AS n ON da.defaclnamespace = n.oid WHERE '%s=%s' = ANY (defaclacl)%s and defaclobjtype = '%s')",
default => "SELECT 1 WHERE EXISTS (SELECT * FROM pg_default_acl AS da LEFT JOIN pg_namespace AS n ON da.defaclnamespace = n.oid WHERE '%s=%s' = ANY (defaclacl)%s and defaclobjtype = '%s')"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think actually there is an issue here: if you leave schema unset and the default privilege exists for a specific schema, the subquery here will return a row, blocking a change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fixed with the most recent force-push.

@fish-face
Copy link
Contributor Author

The inline documentation reflected a previous version of the code; this has now been fixed. In particular this should now be backwards compatible. (I think an incompatible change would result in a better interface, but I don't know what requirements you'd have in this case. It could be a very significant difference in a small number of cases!)

@david22swan
Copy link
Member

@fish-face Could you rebase this please?
Got a few conflicts from merging your last pr

The Postgres default is for the absent specification of a schema name
when altering default privileges to apply to all schemas.
Support that behaviour, but keep the current default behaviour for
an unset schema parameter.
@fish-face
Copy link
Contributor Author

I think the only failure here was a timeout - is there anything you need from me?

@david22swan
Copy link
Member

@fish-face Sorry for leaving this so long, been a little full on here lately.
Yeh I'm happy to merge this, not sure when it will get released though.

Anyway thanks again for the hard work

@david22swan david22swan merged commit dc636d8 into puppetlabs:main Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants