-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[CT-506] Apply grants within materializations #5090
Labels
Comments
jtcohen6
added
enhancement
New feature or request
Team:Adapters
Issues designated for the adapter area of the code
labels
Apr 19, 2022
github-actions
bot
changed the title
Apply grants within materializations
[CT-506] Apply grants within materializations
Apr 19, 2022
This was referenced Apr 22, 2022
1 task
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
How this works today
During each materialization, in
run_hooks(post_hooks)
, dbt will run the arbitrary SQL that the user has provided.What's not great about this?
post_hook
is just a string. dbt doesn't know any structured information about grants, privileges, recipients. If we want metadata later on, "Who should have access to model X?", we have no idea—we'd need to run a database query (show grants
).{{ this }}
is a special thing, as ispost_hook
. If you don't get this syntax exactly right, or try granting on a different table instead, you can end up in some weird circumstances: Post hooks that call macros get parsed with execute = False #2370, get_relation returns none in hook context #2938, this.is_view and this.is_table not working in BigQuery inside a hook #3529, custom table schema path of {{ this }} parsed in correctly in post-hook macro #3985, Post-hook doesn't resolve custom schema #4023, [CT-80] [Bug] post-hook macro generates SQL with incorrect source table #4606. The proposal in this issue doesn't solve for that, but it may help newer users from needing to wade into deeper nested-curly waters.What we want
I can define grants as a resource config on each model/seed/snapshot. As with all resource configs, I can define reasonable defaults in
dbt_project.yml
, plus the ability to define within each model SQL file or yml file.When my dbt model runs, all grants are automatically applied:
We’re targeting the 95% use case here: The right people can select from your dbt models, as soon as those models are created. There may be super specific grants that users want to put together. For that, there are always hooks, as above.
Required changes
grants
as a supported node config. Grants should be merged/clobbered—likemeta
, nottags
. (Opt for less access, not more.)apply_grants
macro, very similar topersist_docs
{% macro get_grant_sql(relation, privilege, recipients) %}
, with a sanedefault__get_grant_sql
adapter__get_grant_sql
if the default doesn’t work for themConsiderations
grants
will support grants on the current model only. dbt grants access on model X, as soon as model X has finished running. It won’t be possible to grant permissions on model Y as soon as model X has finished running.On some databases, grants are automatically “inherited” when a table is recreated (e.g.
copy grants
on Snowflake). Should we strongly advise use of those configs, where available? Should we always revoke + rerun every grant, every time? Or should we first ask which grants are in place (show grants
), calculate diffs, and then decide which grants to use? Related: If users have configured column/row-level restrictive access policies, we need to ensure that those restrictions are applied first, before grants (which are permissive). Otherwise we risk a moment in which a user has more access than they should.The words here are different on different databases. ("Role" on BigQuery means "privilege," whereas on Snowflake it means "recipient group.") How should we factor this config, to avoid bad abstractions later on?
We’ll need to update, in our documentation, the places where we strongly recommend running grants inside of hooks:
The text was updated successfully, but these errors were encountered: