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

[Bug] Materializations fail when database roles are used #1151

Closed
2 tasks done
elventear opened this issue Jul 27, 2024 · 8 comments · Fixed by #1188
Closed
2 tasks done

[Bug] Materializations fail when database roles are used #1151

elventear opened this issue Jul 27, 2024 · 8 comments · Fixed by #1188
Assignees
Labels
bug Something isn't working

Comments

@elventear
Copy link

Is this a new bug in dbt-snowflake?

  • I believe this is a new bug in dbt-snowflake
  • 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

  1. create database role.
  2. create a database with grants on future tables to database role.
  3. 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

@elventear elventear added bug Something isn't working triage labels Jul 27, 2024
@amychen1776 amychen1776 removed the triage label Aug 1, 2024
@lucasmazza
Copy link

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
Copy link
Contributor

@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?

  1. 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;
  1. Create a table model in that database:
{{ config(
    materialized='table'
) }}

select 1 as id
  1. Turn on copy_grants:
models:
  +copy_grants: true
  1. 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
Copy link
Author

elventear commented Sep 26, 2024

My dbt_project.yml is a bit different:

models:
  +grants:
    select:
      - SOME_ROLE

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

@mikealfare
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Author

elventear commented Sep 30, 2024

@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
Copy link
Contributor

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
Copy link
Author

Thank for the explanation and fixing the issued.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants