[Bug] Materializations fail when database roles are used

elventear commented 4 months ago

Is this a new bug in dbt-snowflake?

[X] I believe this is a new bug in dbt-snowflake
[X] I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

DBT is failing to deploy a materialization during the grants revoke phase if the table has database roles applied to it. The failure stems from issuing the wrong syntax for database roles:

revoke <PRIVILEGE> on <TABLE> from <ROLE>;

where it should be:

revoke <PRIVILEGE> on <TABLE> from database role <ROLE>;

Reviewing the code the issue seems to stem from the macro default__apply_grants where it is getting both ROLE and DATABASE_ROLE entries in the results from SHOW GRANTS ON, but it is treating them all as account roles.

Expected Behavior

Deployment should not fail.

Steps To Reproduce

create database role.
create a database with grants on future tables to database role.
deploy materialization to database created in 1.

Relevant log output

Log from failed deployment:

Database Error in model <model> (<model path>)
  002003 (02000): SQL compilation error:
  Role '<ROLE>' does not exist or not authorized.
  compiled Code at <compiled path>


### Environment

```markdown
- OS: linux
- Python: 3.11
- dbt-core: 1.8.3
- dbt-snowflake: 1.8.3

Additional Context

No response

lucasmazza commented 3 months ago

We are seeing a similar issue with our builds on when the role needs quoting - dbt-snowflake produces revoke <PRIVILEGE> on <TABLE> from downcased_role_name instead of revoke <PRIVILEGE> on <TABLE> from "downcased_role_name"

mikealfare commented 2 months ago

@elventear I'm having issues reproducing this. I'll list what I did to reproduce your issue and maybe you can point out the difference?

Create a role with all privileges on futures tables:

CREATE DATABASE ROLE BLOCKING_DB_ROLE;
GRANT ALL PRIVILEGES ON FUTURE TABLES IN DATABASE DBT_TEST TO DATABASE ROLE BLOCKING_DB_ROLE;

Create a table model in that database:
```
{{ config(
materialized='table'
) }}
```

select 1 as id

3. Turn on `copy_grants`:
```yaml
models:
  +copy_grants: true

Run dbt multiple times:
```
dbt run
dbt run
dbt run --full-refresh
```

The above all ran fine for me. I'm using a user with all privileges on future schemas and future tables as well as create schema. I created a test case outlining the above: #1188. Hopefully I'm just missing something and you could point it out.

elventear commented 2 months ago

My dbt_project.yml is a bit different:

models:
  +grants:
    select:
      - SOME_ROLE

And I don't have +copy_grants: true at all.

mikealfare commented 1 month ago

My dbt_project.yml is a bit different:

models: +grants: select:

SOME_ROLE

And I don't have +copy_grants: true at all.

Aha! This worked (in the sense that it failed with the error you provided). Thanks! I'll take a look.

mikealfare commented 1 month ago

It looks like this was due to a combination of a few things:

incremental models persist objects, which require revoking permissions if permissions are updated; standard tables and views are simply dropped and recreated, which doesn't require revoking permissions
database roles are not managed at all by dbt-snowflake, so the DDL that would get produced is wrong, both for granting and revoking
show grants returns all permissions, including database roles; we filter out other scenarios, but missed this one
supplying a grants config triggers permissions syncing, causing an error even if the role being supplied in the grants config is valid (e.g. an existing account role)

It looks like removing database roles in the filter fixes this.

Thanks for reporting and for following up @elventear!

elventear commented 1 month ago

@mikealfare I think the problem goes beyond what you described last. SOME_ROLE in my example is not a database role. The problem is that DBT lists all the roles in the object and tries to revoke all the roles it doesn't know about. The command that dbt uses to list the roles lists both kinds of roles, but revoking the role has different syntax depending on the type and dbt is not aware of that.

A quick, short term, solution would be for dbt to filter out the database roles out when it's listing the roles with SHOW GRANTS ON.

mikealfare commented 1 month ago

I think the problem goes beyond what you described last. SOME_ROLE in my example is not a database role. The problem is that DBT lists all the roles in the object and tries to revoke all the roles it doesn't know about. The command that dbt uses to list the roles lists both kinds of roles, but revoking the role has different syntax depending on the type and dbt is not aware of that.

Agreed. Perhaps I could have stated it more clearly, but I was trying to include this scenario above.

A quick, short term, solution would be for dbt to filter out the database roles out when it's listing the roles with SHOW GRANTS ON.

This is exactly what I did. I did it in the python method that standardizes the grants config instead of the sql, but that was the approach.

Regarding the "short term" qualifier, I don't believe that dbt-snowflake currently supports managing database roles at all; it only supports account roles. This may change in the future, but it would need to be requested and prioritized. I reviewed the open issues and did not see an issue to add database role support. I would encourage you to submit a feature request if this is something you need, otherwise "short term" may not be that short. To add context, I'm simply setting expectations here.

elventear commented 1 month ago

Thank for the explanation and fixing the issued.

dbt-labs / dbt-snowflake