Skip to content

Commit b0c9338

Browse files
committed
fix: rework dbt grants to sqlmesh grants converstion
- sqlmesh grants uses None it explicitly means unmanaged and {} to mean managed with no grants (revoke all existing grants) - empty {} dbt grants config is always considered as unmanaged dbt manifest always parses None grants as {}
1 parent cc82ddc commit b0c9338

File tree

10 files changed

+152
-40
lines changed

10 files changed

+152
-40
lines changed

sqlmesh/core/engine_adapter/base.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2444,7 +2444,7 @@ def _apply_grants_config_expr(
24442444
table: exp.Table,
24452445
grant_config: GrantsConfig,
24462446
table_type: DataObjectType = DataObjectType.TABLE,
2447-
) -> t.List[exp.Grant]:
2447+
) -> t.List[exp.Expression]:
24482448
"""Returns SQLGlot Grant expressions to apply grants to a table.
24492449
24502450
Args:
@@ -2453,7 +2453,7 @@ def _apply_grants_config_expr(
24532453
table_type: The type of database object (TABLE, VIEW, MATERIALIZED_VIEW).
24542454
24552455
Returns:
2456-
List of SQLGlot Grant expressions.
2456+
List of SQLGlot expressions for grant operations.
24572457
24582458
Raises:
24592459
NotImplementedError: If the engine does not support grants.

sqlmesh/core/engine_adapter/postgres.py

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ def _dcl_grants_config_expr(
145145
dcl_cmd: t.Type[DCL],
146146
relation: exp.Expression,
147147
grant_config: GrantsConfig,
148-
) -> t.Union[t.List[exp.Grant], t.List[exp.Revoke]]:
148+
) -> t.List[exp.Expression]:
149149
expressions = []
150150
for privilege, principals in grant_config.items():
151151
if not principals:
@@ -154,23 +154,20 @@ def _dcl_grants_config_expr(
154154
grant = dcl_cmd(
155155
privileges=[exp.GrantPrivilege(this=exp.Var(this=privilege))],
156156
securable=relation,
157-
principals=principals, # use original strings so user can to choose quote or not
157+
principals=principals, # use original strings; no quoting
158158
)
159159
expressions.append(grant)
160160

161-
return expressions
161+
return t.cast(t.List[exp.Expression], expressions)
162162

163163
def _apply_grants_config_expr(
164164
self,
165165
table: exp.Table,
166166
grant_config: GrantsConfig,
167167
table_type: DataObjectType = DataObjectType.TABLE,
168-
) -> t.List[exp.Grant]:
168+
) -> t.List[exp.Expression]:
169169
# https://www.postgresql.org/docs/current/sql-grant.html
170-
return t.cast(
171-
t.List[exp.Grant],
172-
self._dcl_grants_config_expr(exp.Grant, table, grant_config),
173-
)
170+
return self._dcl_grants_config_expr(exp.Grant, table, grant_config)
174171

175172
def _revoke_grants_config_expr(
176173
self,
@@ -179,10 +176,7 @@ def _revoke_grants_config_expr(
179176
table_type: DataObjectType = DataObjectType.TABLE,
180177
) -> t.List[exp.Expression]:
181178
# https://www.postgresql.org/docs/current/sql-revoke.html
182-
return t.cast(
183-
t.List[exp.Expression],
184-
self._dcl_grants_config_expr(exp.Revoke, table, grant_config),
185-
)
179+
return self._dcl_grants_config_expr(exp.Revoke, table, grant_config)
186180

187181
def _get_current_grants_config(self, table: exp.Table) -> GrantsConfig:
188182
"""Returns current grants for a Postgres table as a dictionary."""

sqlmesh/core/model/meta.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -545,8 +545,7 @@ def parse_exp_to_str(e: exp.Expression) -> str:
545545
if grantee: # skip empty strings
546546
grantee_list.append(grantee)
547547

548-
if grantee_list:
549-
grants_dict[permission_name.strip()] = grantee_list
548+
grants_dict[permission_name.strip()] = grantee_list
550549

551550
return grants_dict
552551

sqlmesh/dbt/basemodel.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ class BaseModelConfig(GeneralConfig):
126126
pre_hook: t.List[Hook] = Field([], alias="pre-hook")
127127
post_hook: t.List[Hook] = Field([], alias="post-hook")
128128
full_refresh: t.Optional[bool] = None
129-
grants: t.Optional[t.Dict[str, t.List[str]]] = None
129+
grants: t.Dict[str, t.List[str]] = {}
130130
columns: t.Dict[str, ColumnConfig] = {}
131131
quoting: t.Dict[str, t.Optional[bool]] = {}
132132

sqlmesh/dbt/model.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -573,7 +573,8 @@ def to_sqlmesh(
573573
if physical_properties:
574574
model_kwargs["physical_properties"] = physical_properties
575575

576-
if self.grants is not None:
576+
# A falsy grants config (None or {}) is considered as unmanaged per dbt semantics
577+
if self.grants:
577578
model_kwargs["grants"] = self.grants
578579

579580
kind = self.model_kind(context)

tests/core/engine_adapter/test_bigquery.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1173,3 +1173,89 @@ def test_drop_cascade(adapter: BigQueryEngineAdapter):
11731173
"DROP SCHEMA IF EXISTS `foo` CASCADE",
11741174
"DROP SCHEMA IF EXISTS `foo`",
11751175
]
1176+
1177+
1178+
def test_grants_expressions(adapter: BigQueryEngineAdapter, mocker: MockerFixture):
1179+
"""Test BigQuery grants expressions for tables and views using resource types."""
1180+
from sqlmesh.core.engine_adapter.shared import DataObjectType
1181+
1182+
relation = exp.to_table("test_project.test_dataset.test_table", dialect="bigquery")
1183+
grants_config = {
1184+
"roles/bigquery.dataViewer": ["user:test@example.com", "group:analysts@example.com"]
1185+
}
1186+
1187+
# Test GRANT expression for table
1188+
grant_exprs = adapter._apply_grants_config_expr(relation, grants_config, DataObjectType.TABLE)
1189+
expected_grants = [
1190+
"GRANT `roles/bigquery.dataViewer` ON TABLE test_project.test_dataset.test_table TO 'user:test@example.com'",
1191+
"GRANT `roles/bigquery.dataViewer` ON TABLE test_project.test_dataset.test_table TO 'group:analysts@example.com'",
1192+
]
1193+
grant_sqls = [expr.sql(dialect="bigquery") for expr in grant_exprs]
1194+
assert len(grant_sqls) == 2
1195+
for expected in expected_grants:
1196+
assert any(expected in sql for sql in grant_sqls)
1197+
1198+
# Test REVOKE expression for table
1199+
revoke_exprs = adapter._revoke_grants_config_expr(relation, grants_config, DataObjectType.TABLE)
1200+
expected_revokes = [
1201+
"REVOKE `roles/bigquery.dataViewer` ON TABLE test_project.test_dataset.test_table FROM user:test@example.com",
1202+
"REVOKE `roles/bigquery.dataViewer` ON TABLE test_project.test_dataset.test_table FROM group:analysts@example.com",
1203+
]
1204+
revoke_sqls = [expr.sql(dialect="bigquery") for expr in revoke_exprs]
1205+
assert len(revoke_sqls) == 2
1206+
for expected in expected_revokes:
1207+
assert any(expected in sql for sql in revoke_sqls)
1208+
1209+
# Test GRANT expression for view
1210+
grant_exprs = adapter._apply_grants_config_expr(relation, grants_config, DataObjectType.VIEW)
1211+
expected_grants_view = [
1212+
"GRANT `roles/bigquery.dataViewer` ON VIEW test_project.test_dataset.test_table TO 'user:test@example.com'",
1213+
"GRANT `roles/bigquery.dataViewer` ON VIEW test_project.test_dataset.test_table TO 'group:analysts@example.com'",
1214+
]
1215+
grant_sqls = [expr.sql(dialect="bigquery") for expr in grant_exprs]
1216+
assert len(grant_sqls) == 2
1217+
for expected in expected_grants_view:
1218+
assert any(expected in sql for sql in grant_sqls)
1219+
1220+
# Test REVOKE expression for view
1221+
revoke_exprs = adapter._revoke_grants_config_expr(relation, grants_config, DataObjectType.VIEW)
1222+
expected_revokes_view = [
1223+
"REVOKE `roles/bigquery.dataViewer` ON VIEW test_project.test_dataset.test_table FROM user:test@example.com",
1224+
"REVOKE `roles/bigquery.dataViewer` ON VIEW test_project.test_dataset.test_table FROM group:analysts@example.com",
1225+
]
1226+
revoke_sqls = [expr.sql(dialect="bigquery") for expr in revoke_exprs]
1227+
assert len(revoke_sqls) == 2
1228+
for expected in expected_revokes_view:
1229+
assert any(expected in sql for sql in revoke_sqls)
1230+
1231+
# Test MATERIALIZED_VIEW gets mapped to TABLE in BigQuery
1232+
grant_exprs = adapter._apply_grants_config_expr(
1233+
relation, grants_config, DataObjectType.MATERIALIZED_VIEW
1234+
)
1235+
expected_grants_materialized = [
1236+
"GRANT `roles/bigquery.dataViewer` ON TABLE test_project.test_dataset.test_table TO 'user:test@example.com'",
1237+
"GRANT `roles/bigquery.dataViewer` ON TABLE test_project.test_dataset.test_table TO 'group:analysts@example.com'",
1238+
]
1239+
grant_sqls = [expr.sql(dialect="bigquery") for expr in grant_exprs]
1240+
assert len(grant_sqls) == 2
1241+
for expected in expected_grants_materialized:
1242+
assert any(expected in sql for sql in grant_sqls)
1243+
1244+
1245+
def test_get_current_grants_expression(adapter: BigQueryEngineAdapter):
1246+
"""Test the SQL expression for getting current grants."""
1247+
relation = exp.to_table("test_project.test_dataset.test_table", dialect="bigquery")
1248+
1249+
grants_expr = adapter._get_current_grants_expr(relation)
1250+
expected_expr = """
1251+
SELECT
1252+
privilege_type AS privilege,
1253+
grantee
1254+
FROM `test_project.test_dataset`.`INFORMATION_SCHEMA`.`OBJECT_PRIVILEGES`
1255+
WHERE object_name = 'test_table'
1256+
AND object_schema = 'test_dataset'
1257+
"""
1258+
1259+
# Convert expression to SQL and clean whitespace for comparison
1260+
grants_sql = grants_expr.sql(dialect="bigquery")
1261+
assert " ".join(grants_sql.split()) == " ".join(expected_expr.split())

tests/core/engine_adapter/test_postgres.py

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,12 @@ def test_server_version(make_mocked_engine_adapter: t.Callable, mocker: MockerFi
183183
def test_apply_grants_config(make_mocked_engine_adapter: t.Callable):
184184
adapter = make_mocked_engine_adapter(PostgresEngineAdapter)
185185
relation = exp.to_table("test_table", dialect="postgres")
186-
grants_config = {"SELECT": ["user1", "user2"], "INSERT": ["admin_user"]}
186+
grants_config = {
187+
"SELECT": ["user1", "user2"],
188+
"INSERT": ["admin_user"],
189+
"UPDATE": [], # no-op
190+
"DELETE": [], # no-op
191+
}
187192

188193
adapter._apply_grants_config(relation, grants_config, DataObjectType.TABLE)
189194

@@ -197,7 +202,12 @@ def test_apply_grants_config(make_mocked_engine_adapter: t.Callable):
197202
def test_revoke_grants_config(make_mocked_engine_adapter: t.Callable):
198203
adapter = make_mocked_engine_adapter(PostgresEngineAdapter)
199204
relation = exp.to_table("test_table", dialect="postgres")
200-
grants_config = {"SELECT": ["old_user"], "INSERT": ["removed_role"]}
205+
grants_config = {
206+
"SELECT": ["old_user"],
207+
"INSERT": ["removed_role"],
208+
"UPDATE": [], # no-op
209+
"DELETE": [], # no-op
210+
}
201211

202212
adapter._revoke_grants_config(relation, grants_config, DataObjectType.TABLE)
203213

@@ -242,16 +252,22 @@ def test_sync_grants_config_with_overlaps(
242252
):
243253
adapter = make_mocked_engine_adapter(PostgresEngineAdapter)
244254
relation = exp.to_table("test_schema.test_table", dialect="postgres")
245-
new_grants_config = {"SELECT": ["user1", "user2", "user3"], "INSERT": ["user2", "user4"]}
246255

247256
current_grants = [
248257
("SELECT", "user1"),
249258
("SELECT", "user5"),
250259
("INSERT", "user2"),
251260
("UPDATE", "user3"),
261+
("DELETE", "user4"),
252262
]
253263
fetchall_mock = mocker.patch.object(adapter, "fetchall", return_value=current_grants)
254264

265+
new_grants_config = {
266+
"SELECT": ["user1", "user2", "user3"],
267+
"INSERT": ["user2", "user4"],
268+
# UPDATE will be revoked since it's missing
269+
"DELETE": [],
270+
}
255271
adapter._sync_grants_config(relation, new_grants_config)
256272

257273
fetchall_mock.assert_called_once()
@@ -265,26 +281,13 @@ def test_sync_grants_config_with_overlaps(
265281
)
266282

267283
sql_calls = to_sql_calls(adapter)
268-
assert len(sql_calls) == 4
284+
assert len(sql_calls) == 5
269285

270286
assert 'GRANT SELECT ON "test_schema"."test_table" TO user2, user3' in sql_calls
271287
assert 'GRANT INSERT ON "test_schema"."test_table" TO user4' in sql_calls
272288
assert 'REVOKE SELECT ON "test_schema"."test_table" FROM user5' in sql_calls
273289
assert 'REVOKE UPDATE ON "test_schema"."test_table" FROM user3' in sql_calls
274-
275-
276-
def test_diff_grants_configs(make_mocked_engine_adapter: t.Callable):
277-
new_grants = {"select": ["USER1", "USER2"], "insert": ["user3"]}
278-
old_grants = {"SELECT": ["user1", "user4"], "UPDATE": ["user5"]}
279-
280-
adapter = make_mocked_engine_adapter(PostgresEngineAdapter)
281-
additions, removals = adapter._diff_grants_configs(new_grants, old_grants)
282-
283-
assert additions["select"] == ["USER2"]
284-
assert additions["insert"] == ["user3"]
285-
286-
assert removals["SELECT"] == ["user4"]
287-
assert removals["UPDATE"] == ["user5"]
290+
assert 'REVOKE DELETE ON "test_schema"."test_table" FROM user4' in sql_calls
288291

289292

290293
def test_sync_grants_config_with_default_schema(
@@ -311,3 +314,20 @@ def test_sync_grants_config_with_default_schema(
311314
"WHERE table_schema = 'public' AND table_name = 'test_table' "
312315
"AND grantor = current_role AND grantee <> current_role"
313316
)
317+
318+
319+
def test_diff_grants_configs(make_mocked_engine_adapter: t.Callable):
320+
new_grants = {"select": ["USER1", "USER2"], "insert": ["user3"], "delete": []}
321+
old_grants = {"SELECT": ["user1", "user4"], "UPDATE": ["user5"], "DELETE": ["user6"]}
322+
323+
adapter = make_mocked_engine_adapter(PostgresEngineAdapter)
324+
additions, removals = adapter._diff_grants_configs(new_grants, old_grants)
325+
326+
assert len(additions) == 2
327+
assert additions["select"] == ["USER2"]
328+
assert additions["insert"] == ["user3"]
329+
330+
assert len(removals) == 3
331+
assert removals["SELECT"] == ["user4"]
332+
assert removals["UPDATE"] == ["user5"]
333+
assert removals["DELETE"] == ["user6"]

tests/core/test_model.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11647,6 +11647,13 @@ def test_grants_validation_no_grants():
1164711647
assert model.grants is None
1164811648

1164911649

11650+
def test_grants_validation_empty_grantees():
11651+
model = create_sql_model(
11652+
"db.table", parse_one("SELECT 1 AS id"), kind="FULL", grants={"select": []}
11653+
)
11654+
assert model.grants == {"select": []}
11655+
11656+
1165011657
def test_grants_table_type_view():
1165111658
model = create_sql_model("test_view", parse_one("SELECT 1 as id"), kind="VIEW")
1165211659
assert model.grants_table_type == DataObjectType.VIEW

tests/core/test_snapshot_evaluator.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4631,15 +4631,20 @@ def test_grants_unsupported_engine(make_mocked_engine_adapter, mocker):
46314631
sync_grants_mock.assert_not_called()
46324632

46334633

4634-
def test_grants_clears_grants(make_mocked_engine_adapter, mocker):
4634+
def test_grants_revokes_permissions(make_mocked_engine_adapter, mocker):
46354635
adapter = make_mocked_engine_adapter(EngineAdapter)
46364636
adapter.SUPPORTS_GRANTS = True
46374637
sync_grants_mock = mocker.patch.object(adapter, "_sync_grants_config")
46384638
strategy = ViewStrategy(adapter)
4639-
model = create_sql_model("test_model", parse_one("SELECT 1 as id"), grants={})
4639+
model = create_sql_model("test_model", parse_one("SELECT 1 as id"), grants={"select": []})
4640+
model2 = create_sql_model("test_model2", parse_one("SELECT 1 as id"), grants={})
46404641

46414642
strategy._apply_grants(model, "test_table", GrantsTargetLayer.PHYSICAL)
4643+
sync_grants_mock.assert_called_once()
4644+
4645+
sync_grants_mock.reset_mock()
46424646

4647+
strategy._apply_grants(model2, "test_table", GrantsTargetLayer.PHYSICAL)
46434648
sync_grants_mock.assert_called_once()
46444649

46454650

tests/dbt/test_model.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ def test_model_grants_empty_permissions() -> None:
213213
)
214214

215215
model_grants = sqlmesh_model.grants
216-
expected_grants = {"insert": ["admin_user"]}
216+
expected_grants = {"select": [], "insert": ["admin_user"]}
217217
assert model_grants == expected_grants
218218

219219

@@ -253,7 +253,7 @@ def test_model_empty_grants() -> None:
253253
)
254254

255255
grants_config = sqlmesh_model.grants
256-
assert grants_config == {}
256+
assert grants_config is None
257257

258258

259259
def test_model_grants_valid_special_characters() -> None:

0 commit comments

Comments
 (0)