dbt-labs / dbt-bigquery

dbt-bigquery contains all of the code required to make dbt operate on a BigQuery database.
https://github.com/dbt-labs/dbt-bigquery
Apache License 2.0
210 stars 148 forks source link

[ADAP-345][Feature] Revoke/remove authorized view grant #576

Closed christopherekfeldt closed 11 months ago

christopherekfeldt commented 1 year ago

Is this your first time submitting a feature request?

Describe the feature

Today there is functionality to add grants for authorized views with dbt-bigquery. Although it only adds the authorization through grant_access_to it doesn't hold any functionality regarding revoking or removing the authorization once a model has been added.

Describe alternatives you've considered

There are two scenarios where the authorized view should be revoked/removed of that I can come up with right now:

  1. If the view that has been authorized has been removed
  2. If the grant_access_to config has been removed in the config of the model

Who will this benefit?

Everyone using authorized views

Are you interested in contributing this feature?

No

Anything else?

https://docs.getdbt.com/reference/resource-configs/bigquery-configs#authorized-views https://cloud.google.com/bigquery/docs/authorized-views https://cloud.google.com/blog/products/infrastructure/iam-policy-for-bigquery-dataset-authorized-views-terraform

Fleid commented 1 year ago

Hi @christopherekfeldt, thanks for opening this issue.

The first thing that's important to note is that dbt "doesn't have memory".

dbt is dumb in the sense that it takes a DAG of models (your project), and apply it against a database. The state that is taken into consideration while doing so is what already exists (in the database) versus what you want (the project). What you wanted before that (a previous version of the project) is not considered in this equation.

The typical example of this behavior is when you remove a model from a project, like a table, but still need to drop it from the database yourself if you want it gone.

Similarly, removing the view from the DAG is not an event for dbt. No operations are triggered by it.

Here the thing is that authorized view permissions are actually defined on the source schema of the data, not on the view itself. So if you remove the view, the permissions are still there. In my biased opinion, I don't understand why BigQuery doesn't drop these permissions when the view itself is dropped.

But what that means is that if we were to manage schemas as a first class object in dbt, the story could be different. Now permissions are properties of that schema and could be handled from there. But if a schema was removed from the dbt project, it would still need to be deleted manually at the database level.

I thought we had a discussion in dbt-core for that, managing schemas, but I can't find it right now.

Please let me know what you think about that.

christopherekfeldt commented 1 year ago

"I don't understand why BigQuery doesn't drop these permissions when the view itself is dropped."

Yeah this is super strange. We have created our own "vacuumer" which deleted models and datasets which are removed from our repo. We went with the approach of controlling permissions and grants through terraform instead. Atleast now it is bullet proof and we have more control then earlier.

Fleid commented 1 year ago

I'm not surprised. The current implementation in dbt covers the easy/simple use cases fairly well. But for more complicated workflows, finding an alternative approach makes total sense. You wouldn't have a write-up on yours somewhere? This conversation would be a good place to plug it ;)

github-actions[bot] commented 11 months ago

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please comment on the issue or else it will be closed in 7 days.

github-actions[bot] commented 11 months ago

Although we are closing this issue as stale, it's not gone forever. Issues can be reopened if there is renewed community interest. Just add a comment to notify the maintainers.