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

[Feature] Support for database roles #1206

Open
3 tasks done
seediang opened this issue Oct 12, 2024 · 1 comment · May be fixed by #1207
Open
3 tasks done

[Feature] Support for database roles #1206

seediang opened this issue Oct 12, 2024 · 1 comment · May be fixed by #1207
Labels
enhancement New feature or request

Comments

@seediang
Copy link

Is this your first time submitting a feature request?

  • I have read the expectations for open source contributors
  • I have searched the existing issues, and I could not find an existing issue for this feature
  • I am requesting a straightforward extension of existing dbt-snowflake functionality, rather than a Big Idea better suited to a discussion

Describe the feature

This feature request is for dbt-snowflake to include support for database roles.

There is already a request against the dbt-core project dbt-labs/dbt-core#10587 but raising here for visibility. This request also shows there is synergy with other adapter plugins looking for better support around grants.

There is also a really nice looking pattern suggested by @dbeatty10 on feature request #dbt-core/10587 for how something like this might be implemented from a user point of view.

models:
    - name: MODEL_NAME_1
      config:
        grants:
            # New syntax option
            select:
                role: [ROLE_NAME_1, ROLE_NAME_2, ...]
                database_role: [DATABASE_ROLE_NAME_1, DATABASE_ROLE_NAME_2, ...]
            insert:
                role: [ROLE_NAME_1, ROLE_NAME_2, ...]
                database_role: [DATABASE_ROLE_NAME_1, DATABASE_ROLE_NAME_2, ...]
    - name: MODEL_NAME_2
      config:
        grants:
            # Also preserve existing syntax for full backwards compatibility
            select: [ROLE_NAME_1, ROLE_NAME_2, ...]

Describe alternatives you've considered

Today, I actually work around this issue by overriding macros get_grant_sql & get_revoke_sql to perform database role grants instead on account-level role grants. Unfortunately, #1188 will close this loop hole and break the technique. This is the real reason behind this request.

Who will this benefit?

I am strongly opinionated here, database roles can be created and managed with a lower level of privilege than account level roles. This will allow Analytics Engineers to better control and setup data access patterns, without having to coordinate with a central admin or platform team. The more dbt can be used to manage the desired end state the better the dbt experience. Who knows where this could go, perhaps in the future dbt could automatically create any missing database roles defined in the grants.

Are you interested in contributing this feature?

yes

Anything else?

No response

@kokorin
Copy link

kokorin commented Nov 25, 2024

Here is a workaround.

With grants defined as:

models:
    - name: MODEL_NAME_1
      config:
        grants:
            # Object type must be specified: ROLE or DATABASE ROLE
            select: ['ROLE ROLE_NAME', 'DATABASE ROLE DB_ROLE_NAME', ...]
            insert: ['ROLE ROLE_NAME', 'DATABASE ROLE DB_ROLE_NAME, ...]

The workaround itself

{% macro standardize_grants_dict(grants_table) %}
    {#- /*
     This macro is ALMOST copy-paste from Snowflake DBT adapter function standardize_grants_dict.
     It is adapted to be DBT macro
    */ -#}
    {% set grants_dict = {} %}

    {% for row in grants_table %}
        {% set grantee = row["grantee_name"] %}
        {% set granted_to = row["granted_to"] %}
        {% set privilege = row["privilege"] %}
        {% if privilege != "OWNERSHIP" and granted_to != "SHARE" %}
            {% set sf_grantee -%}
                {{ granted_to.replace('_', ' ') }} {{ grantee }}
            {%- endset %}
            {% if privilege in grants_dict.keys() %}
                {% do grants_dict[privilege].append(sf_grantee) %}
            {% else %}
                {% do grants_dict.update({privilege: [sf_grantee]}) %}
            {% endif %}
        {% endif %}
    {% endfor %}

    {{ return(grants_dict) }}
{% endmacro %}

{% macro snowflake__apply_grants(relation, grant_config, should_revoke=True) %}
    {#- /*
     This macro is ALMOST copy-paste from standard DBT macro with the same name.
     It aims at fixing grants in Snowflake as they are different from the way DBT applies grants:
     1. Snowflake syntax is different:
       Standard DBT: grant <privilege> on <object> to <role>
       Snowflake;s:  grant <privilege> on <object> to <role_type> <role>, where role type is either ROLE or DATABASE ROLE
     2, Snowflake's adapter function standardize_grants_dict(current_grants_table) returns no information if
          an object was granted to ROLE or DATABASE ROLE
    */ -#}
    {#-- If grant_config is {} or None, this is a no-op --#}
    {% if grant_config %}
        {% if should_revoke %}
            {#-- We think previous grants may have carried over --#}
            {#-- Show current grants and calculate diffs --#}
            {% set current_grants_table = run_query(get_show_grant_sql(relation)) %}
            {% set current_grants_dict = standardize_grants_dict(current_grants_table) %}
            {% set needs_granting = diff_of_two_dicts(grant_config, current_grants_dict) %}
            {% set needs_revoking = diff_of_two_dicts(current_grants_dict, grant_config) %}
            {% if not (needs_granting or needs_revoking) %}
                {{ log('On ' ~ relation ~': All grants are in place, no revocation or granting needed.')}}
            {% endif %}
        {% else %}
            {#-- We don't think there's any chance of previous grants having carried over. --#}
            {#-- Jump straight to granting what the user has configured. --#}
            {% set needs_revoking = {} %}
            {% set needs_granting = grant_config %}
        {% endif %}
        {% if needs_granting or needs_revoking %}
            {% set revoke_statement_list = get_dcl_statement_list(relation, needs_revoking, get_revoke_sql) %}
            {% set grant_statement_list = get_dcl_statement_list(relation, needs_granting, get_grant_sql) %}
            {% set dcl_statement_list = revoke_statement_list + grant_statement_list %}
            {% if dcl_statement_list %}
                {{ call_dcl_statements(dcl_statement_list) }}
            {% endif %}
        {% endif %}
    {% endif %}
{% endmacro %}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants