Skip to content

Commit

Permalink
[BUGFIX] fix grants on multi word resource types (#141)
Browse files Browse the repository at this point in the history 
* fix grants on multi word resource types

* docs changes

---------

Co-authored-by: TJ Murphy <[email protected]>
  • Loading branch information
@teej
teej and teej authored Nov 6, 2024
1 parent 60df92e commit e0f6ca8
Show file tree
Hide file tree
Showing 4 changed files with 186 additions and 127 deletions.
225 changes: 118 additions & 107 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,12 +283,12 @@ jobs:

| Feature | Titan Core | Terraform | Schemachange | Permifrost |
|-----------------------------------------|------------|-----------|--------------| ------------|
| Plan and Execute Changes | ✅ | ✅ | ❌ | ✅ |
| Declarative Config | ✅ | ✅ | ❌ | ✅ |
| Python-Based Definitions | ✅ | w/ CDKTF | ❌ | ❌ |
| SQL Support | ✅ | ❌ | ✅ | ❌ |
| Dynamic Role Switching | ✅ | ❌ | N/A | ❌ |
| No State File Dependency | ✅ | ❌ | ✅ | ✅ |
| Plan and Execute Changes | ✅ | ✅ | ❌ | ✅ |
| Declarative Config | ✅ | ✅ | ❌ | ✅ |
| Python-Based Definitions | ✅ | w/ CDKTF | ❌ | ❌ |
| SQL Support | ✅ | ❌ | ✅ | ❌ |
| Dynamic Role Switching | ✅ | ❌ | N/A | ❌ |
| No State File Dependency | ✅ | ❌ | ✅ | ✅ |


### `titan core` vs Terraform
Expand Down Expand Up @@ -325,107 +325,118 @@ Permifrost can be very slow. Running simple Permifrost configs can take minutes


| Name | Supported |
|-------------------------------|-----------|
| **Account Resources** | |
| API Integration | ✅ |
| Catalog Integration | |
| ↳ Glue | ✅ |
| ↳ Object Store | ✅ |
| Compute Pool | ✅ |
| Connection | ❌ |
| Database | ✅ |
| External Access Integration | ✅ |
| External Volume | ❌ |
| Failover Group | 🚧 |
| Grant | |
| ↳ Future Grant | ✅ |
| ↳ Privilege Grant | ✅ |
| ↳ Role Grant | ✅ |
| Network Policy | ✅ |
| Notification Integration | |
| ↳ Email | 🚧 |
| ↳ AWS | 🚧 |
| ↳ Azure | 🚧 |
| ↳ GCP | 🚧 |
| Replication Group | 🚧 |
| Resource Monitor | ✅ |
| Role | ✅ |
| Role Grant | ✅ |
| Security Integration | |
| ↳ External API | ❌ |
| ↳ External OAuth | ❌ |
| ↳ Snowflake OAuth | 🚧 |
| ↳ SAML2 | ❌ |
| ↳ SCIM | ❌ |
| Share | ✅ |
| Storage Integration | |
| ↳ AWS | ✅ |
| ↳ Azure | ✅ |
| ↳ GCS | ✅ |
| User | ✅ |
| Warehouse | ✅ |
| | |
| **Database Resources** | |
| Database Role | ✅ |
| Schema | ✅ |
| | |
| **Schema Resources** | |
| Aggregation Policy | ✅ |
| Alert | ✅ |
| Dynamic Table | ✅ |
| Event Table | ✅ |
| External Function | 🚧 |
| External Table | ❌ |
| File Format | |
| ↳ CSV | ✅ |
| ↳ JSON | ✅ |
| ↳ AVRO | ❌ |
| ↳ ORC | ❌ |
| ↳ Parquet | ✅ |
| Hybrid Table | 🚧 |
| Iceberg Table | ❌ |
| Image Repository | ✅ |
| Masking Policy | ❌ |
| Materialized View | 🚧 |
| Model | ❌ |
| Network Rule | ✅ |
| Packages Policy | ✅ |
| Password Policy | ✅ |
| Pipe | ✅ |
| Projection Policy | ❌ |
| Row Access Policy | ❌ |
| Secret | |
| ↳ Generic | ✅ |
| ↳ OAuth | ✅ |
| ↳ Password | ✅ |
| Sequence | ✅ |
| Service | ✅ |
| Session Policy | ✅ |
| Stage | ✅ |
| ↳ External | ✅ |
| ↳ Internal | ✅ |
| Stored Procedure | |
| ↳ Java | ❌ |
| ↳ Javascript | ❌ |
| ↳ Python | 🚧 |
| ↳ Scala | ❌ |
| ↳ SQL | ❌ |
| Stream | |
| ↳ External Table | ❌ |
| ↳ Stage | ✅ |
| ↳ Table | ✅ |
| ↳ View | ✅ |
| Streamlit | ❌ |
| Table | 🚧 |
| Tag | ✅ |
| Task | ✅ |
| User-Defined Function | |
| ↳ Java | ❌ |
| ↳ Javascript | 🚧 |
| ↳ Python | ✅ |
| ↳ Scala | ❌ |
| ↳ SQL | ❌ |
| View | ✅ |
|-------------------------------|----|
| **Account Resources** | |
| Account Parameter | ✅ |
| API Integration | ✅ |
| Catalog Integration | |
| ↳ Glue | ✅ |
| ↳ Object Store | ✅ |
| Compute Pool | ✅ |
| Connection | ❌ |
| Database | ✅ |
| External Access Integration | ✅ |
| External Volume | ✅ |
| Failover Group | 🚧 |
| Grant | |
| ↳ Future Grant | ✅ |
| ↳ Privilege Grant | ✅ |
| ↳ Role Grant | ✅ |
| Network Policy | ✅ |
| Notification Integration | |
| ↳ Email | 🚧 |
| ↳ AWS | 🚧 |
| ↳ Azure | 🚧 |
| ↳ GCP | 🚧 |
| Replication Group | 🚧 |
| Resource Monitor | ✅ |
| Role | ✅ |
| Role Grant | ✅ |
| Scanner Package | ✅ |
| Security Integration | |
| ↳ External API | ❌ |
| ↳ External OAuth | ❌ |
| ↳ Snowflake OAuth | 🚧 |
| ↳ SAML2 | ❌ |
| ↳ SCIM | ❌ |
| Share | ✅ |
| Storage Integration | |
| ↳ AWS | ✅ |
| ↳ Azure | ✅ |
| ↳ GCS | ✅ |
| Tag Reference | ✅ |
| User | ✅ |
| Warehouse | ✅ |
| | |
| **Database Resources** | |
| Database Role | ✅ |
| Schema | ✅ |
| | |
| **Schema Resources** | |
| Aggregation Policy | ✅ |
| Alert | ✅ |
| Authentication Policy | ✅ |
| Dynamic Table | ✅ |
| Event Table | ✅ |
| External Function | 🚧 |
| External Table | ❌ |
| File Format | |
| ↳ CSV | ✅ |
| ↳ JSON | ✅ |
| ↳ AVRO | ❌ |
| ↳ ORC | ❌ |
| ↳ Parquet | ✅ |
| Hybrid Table | 🚧 |
| Iceberg Table | |
| ↳ Snowflake Catalog | ✅ |
| ↳ AWS Glue | ❌ |
| ↳ Iceberg files | ❌ |
| ↳ Delta files | ❌ |
| ↳ REST Catalog | ❌ |
| ↳ Open Catalog | ❌ |
| Image Repository | ✅ |
| Masking Policy | ❌ |
| Materialized View | 🚧 |
| Model | ❌ |
| Network Rule | ✅ |
| Notebook | ✅ |
| Packages Policy | ✅ |
| Password Policy | ✅ |
| Pipe | ✅ |
| Projection Policy | ❌ |
| Row Access Policy | ❌ |
| Secret | |
| ↳ Generic | ✅ |
| ↳ OAuth | ✅ |
| ↳ Password | ✅ |
| Sequence | ✅ |
| Service | ✅ |
| Session Policy | 🚧 |
| Stage | ✅ |
| ↳ External | ✅ |
| ↳ Internal | ✅ |
| Stored Procedure | |
| ↳ Java | ❌ |
| ↳ Javascript | ❌ |
| ↳ Python | 🚧 |
| ↳ Scala | ❌ |
| ↳ SQL | ❌ |
| Stream | |
| ↳ External Table | ❌ |
| ↳ Stage | ✅ |
| ↳ Table | ✅ |
| ↳ View | ✅ |
| Streamlit | ❌ |
| Table | 🚧 |
| Tag | ✅ |
| Task | ✅ |
| User-Defined Function | |
| ↳ Java | ❌ |
| ↳ Javascript | 🚧 |
| ↳ Python | ✅ |
| ↳ Scala | ❌ |
| ↳ SQL | ❌ |
| View | ✅ |


## Contributing
Expand Down
5 changes: 3 additions & 2 deletions tests/integration/test_lifecycle.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,12 @@ def test_create_drop_from_json(resource, cursor, suffix):

# Not easily testable without flakiness
if resource.__class__ in (
res.Service,
res.AccountParameter,
res.FutureGrant,
res.Grant,
res.RoleGrant,
res.FutureGrant,
res.ScannerPackage,
res.Service,
):
pytest.skip("Skipping")

Expand Down
57 changes: 57 additions & 0 deletions tests/test_grant.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,60 @@ def test_grant_on_accepts_resource_name():
grant = res.Grant(priv="usage", on_warehouse=wh.name, to="somerole")
assert grant.on == "SOMEWH"
assert grant.on_type == ResourceType.WAREHOUSE


def test_grant_on_dynamic_tables():
grant = res.Grant(
priv="SELECT",
on_dynamic_table="somedb.someschema.sometable",
to="somerole",
)
assert grant._data.on == "SOMEDB.SOMESCHEMA.SOMETABLE"
assert grant._data.on_type == ResourceType.DYNAMIC_TABLE

dynamic_table = ResourcePointer(name="sometable", resource_type=ResourceType.DYNAMIC_TABLE)
grant = res.Grant(
priv="SELECT",
on=dynamic_table,
to="somerole",
)
assert grant._data.on == "SOMETABLE"
assert grant._data.on_type == ResourceType.DYNAMIC_TABLE

grant_on_all = res.GrantOnAll(
priv="SELECT",
on_all_dynamic_tables_in_schema="somedb.someschema",
to="somerole",
)
assert grant_on_all._data.in_name == "SOMEDB.SOMESCHEMA"
assert grant_on_all._data.in_type == ResourceType.SCHEMA
assert grant_on_all._data.on_type == ResourceType.DYNAMIC_TABLE

schema = res.Schema(name="somedb.someschema")
grant_on_all = res.GrantOnAll(
priv="SELECT",
on_all_dynamic_tables_in=schema,
to="somerole",
)
assert grant_on_all._data.in_name == "SOMEDB.SOMESCHEMA"
assert grant_on_all._data.in_type == ResourceType.SCHEMA
assert grant_on_all._data.on_type == ResourceType.DYNAMIC_TABLE

future_grant = res.FutureGrant(
priv="CREATE TABLE",
on_future_dynamic_tables_in_schema="somedb.someschema",
to="somerole",
)
assert future_grant._data.in_name == "SOMEDB.SOMESCHEMA"
assert future_grant._data.in_type == ResourceType.SCHEMA
assert future_grant._data.on_type == ResourceType.DYNAMIC_TABLE

schema = res.Schema(name="somedb.someschema")
future_grant = res.FutureGrant(
priv="CREATE TABLE",
on_future_dynamic_tables_in=schema,
to="somerole",
)
assert future_grant._data.in_name == "SOMEDB.SOMESCHEMA"
assert future_grant._data.in_type == ResourceType.SCHEMA
assert future_grant._data.on_type == ResourceType.DYNAMIC_TABLE
26 changes: 8 additions & 18 deletions titan/resources/grant.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@

logger = logging.getLogger("titan")

# TODO: Should Grant objects verify grant types in advance?


@dataclass(unsafe_hash=True)
class _Grant(ResourceSpec):
Expand Down Expand Up @@ -218,8 +216,6 @@ def priv(self):

def grant_fqn(grant: _Grant):
on = f"{resource_label_for_type(grant.on_type)}/{grant.on}"
# if grant.on_type == ResourceType.ACCOUNT:
# on = "ACCOUNT"
return FQN(
name=grant.to.name,
params={
Expand Down Expand Up @@ -355,7 +351,7 @@ def __init__(
# At some point we need to support _in_sometype=SomeType(blah)

if isinstance(arg, Resource):
on_type = ResourceType(singularize(keyword[10:-3]))
on_type = resource_type_for_label(singularize(keyword[10:-3]))
in_type = arg.resource_type
in_name = str(arg.fqn)
granted_in_ref = arg
Expand Down Expand Up @@ -434,8 +430,6 @@ class _GrantOnAll(ResourceSpec):

def __post_init__(self):
super().__post_init__()
# if isinstance(self.priv, str):
# self.priv = self.priv.upper()
if self.in_type not in [ResourceType.DATABASE, ResourceType.SCHEMA]:
raise ValueError(f"in_type must be either DATABASE or SCHEMA, not {self.in_type}")

Expand Down Expand Up @@ -499,15 +493,19 @@ def __init__(
# Handle on_all_ kwargs
if on_kwargs:
for keyword, arg in on_kwargs.items():
on_keyword = keyword.split("_")[2]
on_type = ResourceType(singularize(on_keyword))
if isinstance(arg, Resource):
# In type inferred from Resource class
# on_all_schemas_in=Database(name="somedb")
in_type = arg.resource_type
in_name = str(arg.fqn)
on_type = resource_type_for_label(singularize(keyword[7:-3]))
else:
in_stmt = keyword.split("_in_")[1]
# In type named in kwarg
# on_all_schemas_in_database="somedb"
on_stmt, in_stmt = keyword.split("_in_")
in_type = ResourceType(in_stmt)
in_name = arg
on_type = resource_type_for_label(singularize(on_stmt[7:]))

super().__init__(**kwargs)
self._data: _GrantOnAll = _GrantOnAll(
Expand Down Expand Up @@ -542,14 +540,6 @@ def grant_on_all_fqn(data: _GrantOnAll):
"on": f"{in_type}/{collection}",
},
)
# return FQN(
# name=grant.to.name,
# params={
# "on_type": str(grant.on_type),
# "in_type": str(grant.in_type),
# "in_name": grant.in_name,
# },
# )


@dataclass(unsafe_hash=True)
Expand Down

0 comments on commit e0f6ca8

Please sign in to comment.