diff --git a/.github/workflows/issue-triage.yml b/.github/workflows/issue-triage.yml index 284ff7ae..0fc957fc 100644 --- a/.github/workflows/issue-triage.yml +++ b/.github/workflows/issue-triage.yml @@ -47,7 +47,7 @@ jobs: # silently. The `comment` job below keeps its own ISSUE/REPO — it needs them. GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Clear the agent→workflow handoff file run: rm -f "$RUNNER_TEMP/triage.md" @@ -163,7 +163,7 @@ jobs: - name: Upload triage note if: always() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v6 with: name: issue-triage-note path: ${{ runner.temp }}/triage.md @@ -186,6 +186,8 @@ jobs: REPO: ${{ github.repository }} steps: - name: Download triage note + # Pinned to v4 deliberately: actions/download-artifact has no Node 24 + # release yet, so a bump would not clear the deprecation warning. uses: actions/download-artifact@v4 continue-on-error: true with: diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml index 97d483d4..2d333e66 100644 --- a/.github/workflows/pr-review.yml +++ b/.github/workflows/pr-review.yml @@ -42,7 +42,7 @@ jobs: env: BASE_REF: ${{ github.event.pull_request.base.ref }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 with: fetch-depth: 0 # full history so the base...head diff is available @@ -107,7 +107,7 @@ jobs: - name: Upload findings if: always() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v6 with: name: pr-review-findings path: ${{ runner.temp }}/review.md @@ -134,6 +134,8 @@ jobs: REPO: ${{ github.repository }} steps: - name: Download findings + # Pinned to v4 deliberately: actions/download-artifact has no Node 24 + # release yet, so a bump would not clear the deprecation warning. uses: actions/download-artifact@v4 continue-on-error: true with: diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index e71e7d57..e966da0b 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -33,10 +33,12 @@ jobs: - name: Run checks (linter, code style, static type checks, tests) run: | source ./.venv/bin/activate - ruff check snowcap/ + make lint make typecheck python -m pytest --ignore=tests/integration --cov=snowcap --cov-report=xml - name: Upload coverage reports to Codecov - uses: codecov/codecov-action@v5 + # v6 is the first release that pins a Node 24 build of actions/github-script; + # v5 pins v7.0.1, which GitHub now reports as a deprecated Node 20 action. + uses: codecov/codecov-action@v6 env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/Makefile b/Makefile index deb90e68..57b7db77 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: install install-dev test integration style check clean build docs coverage provision-test-account drop-test-account +.PHONY: install install-dev test integration style lint check clean build docs coverage provision-test-account drop-test-account EDITION ?= standard or enterprise EMAIL ?= @@ -35,6 +35,12 @@ style: python -m black . codespell . +# Same checks as `style`, but read-only. This is what CI runs. +lint: + python -m black --check . + codespell . + ruff check snowcap/ + typecheck: mypy --exclude="snowcap/resources/.*" --exclude="snowcap/sql.py" --follow-imports=skip snowcap/ diff --git a/docs/resources/database_role_grant.md b/docs/resources/database_role_grant.md new file mode 100644 index 00000000..752bae88 --- /dev/null +++ b/docs/resources/database_role_grant.md @@ -0,0 +1,44 @@ +--- +description: >- + A database role grant in Snowflake. +--- + +# DatabaseRoleGrant + +[Snowflake Documentation](https://docs.snowflake.com/en/sql-reference/sql/grant-database-role) | Snowcap CLI label: `database_role_grant` + +Represents a grant of a database role to another role or database role in Snowflake. + + +## Examples + +### Python + +```python +# Grant to Database Role: +role_grant = DatabaseRoleGrant(database_role="somedb.somerole", to_database_role="somedb.someotherrole") +role_grant = DatabaseRoleGrant(database_role="somedb.somerole", to=DatabaseRole(database="somedb", name="someotherrole")) +# Grant to Role: +role_grant = DatabaseRoleGrant(database_role="somedb.somerole", to_role="somerole") +role_grant = DatabaseRoleGrant(database_role="somedb.somerole", to=Role(name="somerole")) +``` + + +### YAML + +```yaml +database_role_grants: + - database_role: somedb.somerole + to_database_role: somedb.someotherrole + - database_role: somedb.somerole + to_role: somerole +``` + + +## Fields + +* `database_role` (string or [Role](role.md), required) - The database role to be granted. +* `to_role` (string or [Role](role.md)) - The role to which the database role is granted. +* `to_database_role` (string or [User](user.md)) - The database role to which the database role is granted. + + diff --git a/docs/resources/grant.md b/docs/resources/grant.md index 24fe4915..bcdca581 100644 --- a/docs/resources/grant.md +++ b/docs/resources/grant.md @@ -7,7 +7,7 @@ description: >- [Snowflake Documentation](https://docs.snowflake.com/en/sql-reference/sql/grant-privilege) | Snowcap CLI label: `grant` -The `Grant` resource represents a privilege grant, a future grant, or a grant of privileges on all resources of a specified type to a role in Snowflake. +The `Grant` resource represents a privilege grant, a future grant, an inherited grant, or a grant of privileges on all resources of a specified type to a role in Snowflake. ## Examples @@ -85,7 +85,9 @@ grants: on: dbt project somedb.someschema.analytics_dbt to: analytics_observer - # AI: USAGE on an MCP Server so MCP clients can call its tools + # AI: USAGE on an MCP Server so MCP clients can call its tools. + # Snowflake reports grants on these with granted_on 'CORTEX_AGENT_SERVER', + # which Snowcap accepts as a synonym; the DDL grammar only takes MCP SERVER. - priv: USAGE on: mcp server somedb.someschema.someserver to: mcp_client_role @@ -156,6 +158,42 @@ grants: to: somerole ``` +#### Inherited Grants + +An inherited grant is a single grant on a container that covers every current **and +future** object of a type inside it, replacing an `all` + `future` pair. + +```yaml +grants: + - priv: SELECT + on: inherited tables in schema somedb.someschema + to: somerole + + # Multiple privileges expand to one statement each + - priv: + - SELECT + - INSERT + on: inherited tables in database somedb + to: somerole + + # The account can only be the container of an inherited grant + - priv: SELECT + on: inherited tables in account + to: somerole + + # Or turn a grant on all objects into an inherited one + - priv: SELECT + on: all tables in database somedb + inherited: true + to: somerole + + # Delegate to a role holding MANAGE GRANTS on the container + - priv: SELECT + on: inherited tables in database sales_db + to: analyst + owner: sales_db_admin +``` + ### Python #### Object Grants @@ -242,6 +280,32 @@ grant_on_all = Grant( ) ``` +#### Inherited Grants + +```python +inherited_grant = Grant( + priv="SELECT", + on="INHERITED TABLES IN SCHEMA somedb.someschema", + to="somerole", +) +inherited_grant = Grant( + priv="SELECT", + on=["INHERITED", "TABLES", Database(name="somedb")], + to="somerole", +) + +# The account can only be the container of an inherited grant +inherited_grant = Grant(priv="SELECT", on="INHERITED TABLES IN ACCOUNT", to="somerole") + +# Or turn a grant on all objects into an inherited one +inherited_grant = Grant( + priv="SELECT", + on="ALL TABLES IN DATABASE somedb", + inherited=True, + to="somerole", +) +``` + ## Fields - **`priv`** (`string` or `list`, required): @@ -260,6 +324,8 @@ grant_on_all = Grant( - `"service my_db.my_schema.my_service"` - for service privileges - `"future tables in schema my_schema"` - for future grants - `"all tables in database my_db"` - for grants on all existing objects + - `"inherited tables in database my_db"` - for inherited grants, covering existing and future objects + - `"inherited tables in account"` - inherited grants are the only kind that can be scoped to the account - **`to`** (`string` or [Role](role.md), required): The role to which the privileges are granted. @@ -268,10 +334,39 @@ grant_on_all = Grant( Specifies whether the grantee can grant the privileges to other roles. Defaults to `false`. - **`owner`** (`string` or [Role](role.md), optional): - The owner role of the grant. Defaults to `"SYSADMIN"`. + The owner role of the grant. Defaults to `"SYSADMIN"`. Grants are issued as + `SECURITYADMIN`; for inherited grants, an explicit owner names the role holding + `MANAGE GRANTS` on the container and is used to issue the grant instead. + +- **`inherited`** (`bool`, optional): + Turns a grant on all objects in a container into an inherited grant, which also covers + objects created later. Defaults to `false`. + +**Note:** Inherited grants are a Snowflake preview feature, opted into with an account +parameter. Snowcap manages it with an [AccountParameter](account_parameter.md), applied +before any inherited grant that depends on it: + +```yaml +account_parameters: + - name: FEATURE_RBAC_INHERITED_GRANTS + value: ENABLED +``` + +`snowcap plan` fails with a clear message if neither the account nor the config has opted +in. Snowflake does not allow inherited grants +to be combined with `WITH GRANT OPTION`, to carry `OWNERSHIP`, or to target shares and +integrations; `priv: ALL` is not supported either, so list privileges explicitly. See +[Managing access with inherited grants](https://docs.snowflake.com/en/user-guide/inherited-grants-intro). **Note:** `IMPORTED PRIVILEGES` is only valid on a [SharedDatabase](shared_database.md) (a database created `FROM SHARE`). It cannot be granted `WITH GRANT OPTION` and can only be granted to account roles, not database roles. Snowflake's `SHOW GRANTS` reports it as `USAGE` on shared databases — snowcap's fetch logic handles this quirk transparently. + +One `IMPORTED PRIVILEGES` grant also fans out in `SHOW GRANTS` into a row per object the +share exposes — every view, function, procedure, schema, database role, class, tag and +image repository in the database, which on the `SNOWFLAKE` database is several hundred +rows. Those rows are never in your config, so `--sync_resources grant` treats them as +covered by the declared grant rather than revoking them, the same way it treats the +per-object grants produced by an `ALL` or `INHERITED` grant. diff --git a/docs/role-based-access-control.md b/docs/role-based-access-control.md index 8eff8735..e050c5a0 100644 --- a/docs/role-based-access-control.md +++ b/docs/role-based-access-control.md @@ -221,6 +221,58 @@ grants: to: z_tables_views__select ``` +!!! warning "Database-level future grants can be silently ignored" + + When future grants exist on the **same object type** at both the database and the + schema level, Snowflake gives the schema-level grant precedence and + [ignores the database-level grant](https://docs.snowflake.com/en/sql-reference/sql/grant-privilege#future-grants-on-database-or-schema-objects) + for that schema. Objects created there never receive the privilege, and nothing + fails — access is simply missing. + + This is easy to trip over with managed access schemas, where privilege management is + centralized on the schema owner: the schema-level future grant that shadows the + database-level one is often added later, by a different config or a different team. + Managed access does not by itself disable database-level future grants (the one + exception is future grants of `OWNERSHIP`, which Snowcap does not support), but it is + where the conflict tends to appear. + + If your schemas use `managed_access: true`, declare the future grants at the schema + level, in the same template that creates the schemas: + + ```yaml + grants: + - for_each: var.schemas + priv: SELECT + on: + - "all tables in schema {{ each.value.name }}" + - "all views in schema {{ each.value.name }}" + - "future tables in schema {{ each.value.name }}" + - "future views in schema {{ each.value.name }}" + to: z_tables_views__select + ``` + + `snowcap plan` warns when it finds database-level future grants in a database that + contains managed access schemas, and when a schema-level future grant already shadows + a database-level one. + +!!! tip "Inherited grants avoid this problem entirely" + + Snowflake's [inherited grants](https://docs.snowflake.com/en/user-guide/inherited-grants-intro) + replace an `ALL` + `FUTURE` pair with a single container-level grant covering every + current and future object of a type. They are **not** subject to the precedence rule + above: a database-level and a schema-level inherited grant both apply, and managed + access schemas do not change that. + + ```yaml + grants: + - priv: SELECT + on: INHERITED TABLES IN DATABASE sales_db + to: analyst + ``` + + See [Inherited grants](#inherited-grants) below for the full syntax, the account + requirements, and how to migrate an existing `ALL` + `FUTURE` pair. + ### Functional Roles and Hierarchy (roles__functional.yml) Define functional roles and assemble the role hierarchy: @@ -330,6 +382,127 @@ grants: to: z_stage__raw__dbt_artifacts__artifacts__write ``` +## Inherited Grants + +An [inherited grant](https://docs.snowflake.com/en/user-guide/inherited-grants-intro) is a +single grant on a container — an account, database, or schema — that applies to every +current **and future** object of a type inside it. One inherited grant replaces the +`ALL` + `FUTURE` pair this pattern would otherwise need: + +```yaml +grants: + # Instead of "all tables in ..." plus "future tables in ..." + - priv: SELECT + on: INHERITED TABLES IN DATABASE sales_db + to: z_tables_views__r + + # Multiple privileges expand to one statement each + - priv: [SELECT, INSERT, UPDATE, DELETE] + on: INHERITED TABLES IN SCHEMA sales_db.us_west + to: z_tables__rw + + # The account can only be the container of an inherited grant + - priv: SELECT + on: INHERITED TABLES IN ACCOUNT + to: z_scanner + + # Or turn an existing grant on all objects into an inherited one + - priv: SELECT + on: "all tables in database sales_db" + inherited: true + to: z_tables_views__r +``` + +### Why it matters for this pattern + +| | `ALL` + `FUTURE` | `INHERITED` | +|---|---|---| +| Covers objects created later | Only via the `FUTURE` half | Yes | +| Shadowed by a schema-level grant | Yes, silently | No | +| Compared against Snowflake on each run | No — `ALL` grants are reapplied every time | Yes | +| Grant records created | One per object, plus one future grant | One | + +Because Snowflake reports an inherited grant back as a single durable record, `snowcap +plan` can compare it against your config. Grants on all objects cannot be compared, so +they are reapplied on every run. + +### Requirements + +Inherited grants are a Snowflake preview feature, opted into with an account parameter. +Snowcap manages that parameter like any other — declare it alongside the rest: + +```yaml +# account.yml +account_parameters: + - name: FEATURE_RBAC_INHERITED_GRANTS + value: ENABLED +``` + +Snowcap applies the parameter before any inherited grant that depends on it, so a single +`snowcap apply` can enable the preview and create the grants in one run. `ALTER ACCOUNT` +requires `ACCOUNTADMIN`, which is the role Snowcap already uses for account parameters. +The equivalent SQL, if you would rather set it outside of Snowcap: + +```sql +ALTER ACCOUNT SET FEATURE_RBAC_INHERITED_GRANTS = 'ENABLED'; +``` + +Either way, `snowcap plan` fails with a clear message if your config declares inherited +grants and neither the account nor the config has opted in. + +!!! note "If preview features are turned off account-wide" + + Preview access gates every preview feature at once and is + [enabled by default for most accounts](https://docs.snowflake.com/en/release-notes/preview-features), + so usually there is nothing to do. If it has been disabled, the parameter above will + not take effect until an account admin re-enables it: + + ```sql + SELECT SYSTEM$GET_PREVIEW_ACCESS_STATUS(); -- check + SELECT SYSTEM$ENABLE_PREVIEW_ACCESS(); -- enable + ``` + + These are system function calls rather than resources, so Snowcap cannot manage them + declaratively. It does detect the situation: if preview access is off, `snowcap plan` + says so and points at the function to call, rather than suggesting the parameter that + would not help. + +Creating one requires `MANAGE GRANTS` on the container, not just ownership of it. By +default Snowcap issues grants as `SECURITYADMIN`. To delegate to a database or schema +admin instead, name that role as the grant's owner: + +```yaml +grants: + - priv: SELECT + on: INHERITED TABLES IN DATABASE sales_db + to: analyst + owner: sales_db_admin # holds MANAGE GRANTS ON DATABASE sales_db +``` + +### Migrating from ALL + FUTURE + +Snowflake recommends adding the inherited grant first and revoking the originals only once +you have confirmed access is intact. A grant pair is safe to collapse when the privilege +and the grantee are the same on both halves, and no object in the container needs to be +excluded. If some objects need different access, keep the granular grants, or use +[masking policies](masking-policies.md) and [row access policies](row-access-policies.md) +for the exceptions. + +Snowcap will not revoke per-object grants that a declared inherited grant covers, so you +can add the inherited grant and remove the old declarations in either order without an +access gap. + +### Limitations + +Snowflake does not allow inherited grants to be combined with `WITH GRANT OPTION`, to +carry `OWNERSHIP`, to target shares or integrations, or to be granted on shared databases. +Snowcap rejects these at plan time. `priv: ALL` is also rejected — list the privileges +explicitly. + +Note that Snowflake's Information Schema does not currently account for inherited grants +when deciding whether an object is visible to a role, so an object a role can only reach +through one will not appear in `INFORMATION_SCHEMA` results. + ## Running Snowcap ### Environment Setup @@ -473,6 +646,49 @@ schemas: With `managed_access: true`, even if an analyst creates a view, they cannot grant SELECT on it—only the schema owner can. This ensures all access flows through your defined role hierarchy. +### Cloned Databases (QA, blue-green, PR environments) + +Cloning a database does two different things to grants: + +| What | Happens to grants | +|------|-------------------| +| The database itself | **Not** copied — the clone starts with no grants on it | +| Schemas, tables and other child objects | **Copied** — each keeps the grants its source had | + +So after `CREATE DATABASE BALBOA_QA CLONE BALBOA`, every `z_schema__` role +already holds USAGE on the clone's copy of its schema, without anyone writing +that down. Only the database-level grant is missing, which is why a clone is +normally followed by a re-grant of `USAGE ON DATABASE` to `z_db__`. + +That is the behaviour you want — a role named for a schema keeps its meaning in +every copy of that schema — but Snowcap does not know about it. With +`--sync_resources grant`, those copied grants are remote state that no config +declares, so a plan proposes dropping them. Applying that leaves roles with +usage on the clone's database and no access to anything inside it. + +Declare them with a filtered loop over the schema list you already keep: + +```yaml +grants: + - for_each: var.schemas + where: "each.value.name.split('.')[0] == 'BALBOA'" + priv: USAGE + on: "schema BALBOA_QA.{{ each.value.name.split('.')[1] }}" + to: "z_schema__{{ each.value.name.split('.')[1] }}" +``` + +The `where` keeps the block off schemas in databases that have no clone. Adding +a schema to the source layer covers its clone automatically, so the two cannot +drift apart. The clone's schemas themselves stay undeclared — the clone creates +them, and Snowcap only needs to describe the access. + +**Do not reach for `all schemas in database` here.** It looks like less +configuration, but it grants every role that holds it the entire clone. If +roles are scoped by layer — an analyst role seeing L1 through L3 and a reporter +role seeing only L3 — a database-wide grant silently flattens that distinction +in the clone while leaving it intact in the source, which is the kind of gap +that survives review precisely because the source still looks correct. + ## See Also - [Grant](resources/grant.md) diff --git a/docs/yaml-configuration.md b/docs/yaml-configuration.md index 6e23c88d..92950c7b 100644 --- a/docs/yaml-configuration.md +++ b/docs/yaml-configuration.md @@ -95,6 +95,27 @@ Inside a `for_each` block, you have access to: | `each.value.` | Access a field of the current item | | `each.index` | The index of the current item (0-based) | +### Filtering a Loop (where) + +Add `where` to run a block over only part of a list. The expression is bare +Jinja — no `{{ }}` — and items it evaluates falsy for are skipped: + +```yaml +grants: + # The schema list also covers databases other than BALBOA, so narrow it + - for_each: var.schemas + where: "each.value.name.split('.')[0] == 'BALBOA'" + priv: USAGE + on: "schema BALBOA_QA.{{ each.value.name.split('.')[1] }}" + to: "z_schema__{{ each.value.name.split('.')[1] }}" +``` + +This lets one list drive several blocks that each cover a subset of it, rather +than maintaining a second list per subset. It is the usual way to grant on a +clone of a database — a QA or blue-green copy — without restating every schema: +the same `z_schema__` role reaches the copy, so per-schema roles keep +their meaning and tiered roles stay tiered. + ### Creating Roles and Grants A common pattern is creating a role and grant for each resource: diff --git a/prompts/generate_new_resource_prompt.txt b/prompts/generate_new_resource_prompt.txt index 232e7cb9..dc8042a8 100644 --- a/prompts/generate_new_resource_prompt.txt +++ b/prompts/generate_new_resource_prompt.txt @@ -11,5 +11,5 @@ The file should be completed in this order: - Import statements - Enums, if necessary. When generating enums, always subclass the ParseableEnum class. -- The ResourceSpec class for the resource. This is always anotated with `@dataclass(unsafe_hash=True)`. The name is always the Resource class name with an underscore in front (eg _Warehouse for Warehouse). The ResourceSpec class should then specify each property of the class and its default, if necessary. The first two properties should always be `name: ResourceName` and `owner: Role`. +- The ResourceSpec class for the resource. This is always annotated with `@dataclass(unsafe_hash=True)`. The name is always the Resource class name with an underscore in front (eg _Warehouse for Warehouse). The ResourceSpec class should then specify each property of the class and its default, if necessary. The first two properties should always be `name: ResourceName` and `owner: Role`. - Then comes the Resource class itself. The resource class should always subclass `NamedResource, TaggableResource, Resource`. Do not write a docstring. Next should always be these class properties, in this order: resource_type, props, scope, spec. Scope should be AccountScope() if the resource can only be created at the account level, SchemaScope() if it can only be created within a schema. Use the docs to determine which. spec should always be set to the ResourceSpec class. Next you should write the __init__ method. The init method should take all the properties you used in the ResourceSpec class, plus additionally `tags: dict[str, str] = None` and `**kwargs`. The first line of __init__ should always be `super().__init__(name, **kwargs)`. Next you should set `self._data` to an instance of the ResourceSpec class. Finally in __init__ you should call `self.set_tags(tags)`. diff --git a/pyproject.toml b/pyproject.toml index 27e9a20b..24fc39df 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -2,10 +2,15 @@ line-length = 120 target-version = ['py310'] include = '\.pyi?$' +# Black's defaults already cover .venv; the alternates below are common local venv and +# build directory names that `make style` would otherwise walk into. extend-exclude = ''' -# A regex preceded with ^/ will apply only to files and directories -# in the root of the project. -^/foo.py # exclude a file named foo.py in the root of the project (in addition to the defaults) +/( + \.venv[^/]* + | build + | dist + | site +)/ ''' [tool.pyright] @@ -29,12 +34,8 @@ filterwarnings = [ ] [tool.codespell] -ignore-words-list = [ - "priv", - "sproc", - "snowpark", - "pathspec", -] -skip = [ - "./build/", -] \ No newline at end of file +# codespell reads these as comma-separated strings; a TOML array is silently ignored. +# "objec" and "selct" are deliberately malformed identifiers in parser and error-handling +# tests, and "unparseable" is an accepted variant spelling. +ignore-words-list = "priv,sproc,snowpark,pathspec,objec,selct,unparseable" +skip = "./build/,./dist/,./site/,./.git/,./.venv/,./.venv-test/,./*.egg-info/" \ No newline at end of file diff --git a/setup.py b/setup.py index 6022bd22..d0e5ae58 100644 --- a/setup.py +++ b/setup.py @@ -3,7 +3,7 @@ setup( name="snowcap", # Package version is managed by the string inside version.md. By default, - # setuptools doesnt copy this file into the build package. So we direct + # setuptools doesn't copy this file into the build package. So we direct # setuptools to include it using the `include_package_data=True` option # as well as the MANIFEST.in file which has the `include version.md` directive. version=open("version.md", encoding="utf-8").read().split(" ")[2], @@ -46,7 +46,9 @@ ], extras_require={ "dev": [ - "black", + # Pinned: black's formatting changes between releases, and an unpinned + # version means `make style` reformats files no one touched. + "black==26.5.1", "build", "codespell==2.2.6", "cryptography", diff --git a/snowcap/__init__.py b/snowcap/__init__.py index 7dccff58..ea8b3555 100644 --- a/snowcap/__init__.py +++ b/snowcap/__init__.py @@ -13,9 +13,9 @@ ] # Datacoves brand colors (24-bit true color ANSI codes) -WHITE = "\033[38;2;232;244;254m" # #E8F4FE (Light blue/white for snow) -BLUE = "\033[38;2;52;150;224m" # #3496E0 -YELLOW = "\033[38;2;255;209;1m" # #FFD101 +WHITE = "\033[38;2;232;244;254m" # #E8F4FE (Light blue/white for snow) +BLUE = "\033[38;2;52;150;224m" # #3496E0 +YELLOW = "\033[38;2;255;209;1m" # #FFD101 RESET = "\033[0m" LOGO = f""" @@ -25,6 +25,4 @@ {BLUE} |___/_| |_|\\___/ \\_/\\_/ \\___\\__,_| .__/ {BLUE} ▲▲▲▲▲{RESET} {BLUE} |_| {BLUE} ▲▲▲▲▲▲▲{RESET} {YELLOW} by Datacoves{RESET} -""".strip( - "\n" -) +""".strip("\n") diff --git a/snowcap/adapters/permifrost.py b/snowcap/adapters/permifrost.py index dcd641ca..cd662159 100644 --- a/snowcap/adapters/permifrost.py +++ b/snowcap/adapters/permifrost.py @@ -11,7 +11,6 @@ from snowcap.resources import Grant, RoleGrant from snowcap.resources.resource import ResourcePointer - DATABASE_READ_PRIVS = [DatabasePriv.USAGE] DATABASE_WRITE_PRIVS = [DatabasePriv.USAGE, DatabasePriv.MONITOR, DatabasePriv.CREATE_SCHEMA] diff --git a/snowcap/blueprint.py b/snowcap/blueprint.py index 311a7024..d550696d 100644 --- a/snowcap/blueprint.py +++ b/snowcap/blueprint.py @@ -17,6 +17,7 @@ ) import snowflake.connector +from inflection import pluralize from . import data_provider, lifecycle from .blueprint_config import BlueprintConfig @@ -30,6 +31,8 @@ ) from .data_provider import SessionContext from .enums import ( + INHERITED_GRANTS_FEATURE_FLAG, + OWNER_EXECUTED_RESOURCE_TYPES, AccountEdition, BlueprintScope, GrantType, @@ -51,12 +54,13 @@ NotADAGException, OrphanResourceException, ) -from .identifiers import URN, parse_identifier, parse_URN, resource_label_for_type +from .identifiers import URN, parse_identifier, parse_URN, resource_label_for_type, smart_split from .privs import AccountPriv, CREATE_PRIV_FOR_RESOURCE_TYPE, system_role_for_priv from .resource_name import ResourceName from .resource_tags import ResourceTags from .resources import Database, Grant, RoleGrant, Schema from .resources.database import public_schema_urn +from .resources.grant import INHERITED_GRANT_DOCS, _Grant, grant_on_clause from .resources.resource import ( RESOURCE_SCOPES, NamedResource, @@ -266,6 +270,327 @@ def manifest_future_grant_roles(manifest: "Manifest") -> set: return roles +FUTURE_GRANT_PRECEDENCE_DOCS = ( + "https://docs.snowflake.com/en/sql-reference/sql/grant-privilege#future-grants-on-database-or-schema-objects" +) + + +def _future_grant_scopes(entries) -> tuple[set[str], list[dict], set[tuple[str, str]]]: + """ + Bucket resources into the three things the managed-access check needs: + managed access schemas, database-level future grants, and schema-level future grants. + + `entries` is an iterable of (urn, resource_type, data) so the check can run over a + manifest, over remote state, or over a plan without caring which it was handed. + """ + managed_access_schemas: set[str] = set() + database_future_grants: list[dict] = [] + schema_future_grants: set[tuple[str, str]] = set() + + for urn, resource_type, data in entries: + if resource_type == ResourceType.SCHEMA: + if data.get("managed_access") and urn.fqn.database: + managed_access_schemas.add(f"{urn.fqn.database}.{urn.fqn.name}".upper()) + elif resource_type == ResourceType.GRANT: + if data.get("grant_type") != GrantType.FUTURE.value: + continue + items_type = str(data.get("items_type") or "").upper() + on_type = str(data.get("on_type") or "").upper() + on = str(data.get("on") or "").upper() + if not items_type or not on: + continue + if on_type == ResourceType.DATABASE.value: + database_future_grants.append( + { + "database": on, + "items_type": items_type, + "priv": str(data.get("priv") or ""), + # `to` is a bare role name or a labelled FQN (database_role/DB.ROLE). + "to": str(data.get("to") or "").split("/", 1)[-1], + } + ) + elif on_type == ResourceType.SCHEMA.value: + schema_future_grants.add((on, items_type)) + + return managed_access_schemas, database_future_grants, schema_future_grants + + +def _format_schema_list(schemas: Sequence[str]) -> str: + shown = list(schemas[:3]) + remainder = len(schemas) - len(shown) + if remainder > 0: + return f"{', '.join(shown)} (and {remainder} more)" + return ", ".join(shown) + + +def future_grant_precedence_warnings(entries) -> list[str]: + """ + Warn about database-level future grants that Snowflake will silently ignore. + + Snowflake gives schema-level future grants precedence over database-level future + grants on the same object type: when both exist, the database-level grant is ignored + for that schema, and objects created there never receive the privilege. + + Managed access schemas are where this bites hardest. They centralize privilege + management on the schema owner, so the schema-level future grants that shadow a + database-level grant are typically added by a different config (or a different team) + than the one that declared the database-level grant, and nothing surfaces the + conflict until someone reports missing access on a newly created table. + + Note that this precedence rule is not specific to managed access schemas, and that + managed access does not by itself disable database-level future grants -- Snowflake + documents that they apply to regular and managed access schemas alike. The one + managed-access-specific exception is future grants of OWNERSHIP, which Snowcap does + not support. + """ + managed_access_schemas, database_future_grants, schema_future_grants = _future_grant_scopes(entries) + + if not database_future_grants: + return [] + + warnings = [] + + # Case 1: a schema-level future grant on the same object type already shadows the + # database-level grant. This is a live misconfiguration, not just a risk. + shadowed: dict[tuple[str, str, str, str], list[str]] = defaultdict(list) + for grant in database_future_grants: + prefix = grant["database"] + "." + for schema_fqn, schema_items_type in schema_future_grants: + if schema_items_type == grant["items_type"] and schema_fqn.startswith(prefix): + key = (grant["database"], grant["items_type"], grant["priv"], grant["to"]) + shadowed[key].append(schema_fqn) + + for (database, items_type, priv, to), schemas in sorted(shadowed.items()): + schemas = sorted(set(schemas)) + managed = [schema for schema in schemas if schema in managed_access_schemas] + managed_note = ( + f" {_format_schema_list(managed)} {'is a' if len(managed) == 1 else 'are'} managed access " + f"{'schema' if len(managed) == 1 else 'schemas'}." + if managed + else "" + ) + warnings.append( + f"{priv} ON FUTURE {pluralize(items_type).upper()} IN DATABASE {database} to {to} is ignored for " + f"{_format_schema_list(schemas)}, which define their own future grants on {items_type}. " + f"Snowflake gives schema-level future grants precedence over database-level future grants on the " + f"same object type, so objects created in those schemas will not receive this privilege." + f"{managed_note} Declare the privilege as a schema-level future grant on those schemas. " + f"See {FUTURE_GRANT_PRECEDENCE_DOCS}" + ) + + # Case 2: managed access schemas covered only by database-level future grants. Not + # broken today, but a single schema-level future grant on the same object type -- + # added anywhere, by anyone -- silently switches the privilege off for that schema. + databases_with_managed_access: dict[str, list[str]] = defaultdict(list) + for schema_fqn in managed_access_schemas: + databases_with_managed_access[schema_fqn.split(".", 1)[0]].append(schema_fqn) + + at_risk: dict[str, set[str]] = defaultdict(set) + for grant in database_future_grants: + database = grant["database"] + if database not in databases_with_managed_access: + continue + if (database, grant["items_type"], grant["priv"], grant["to"]) in shadowed: + continue + at_risk[database].add(grant["items_type"]) + + for database, items_types in sorted(at_risk.items()): + schemas = sorted(databases_with_managed_access[database]) + types = ", ".join(pluralize(items_type).upper() for items_type in sorted(items_types)) + warnings.append( + f"Database {database} grants access to {types} with database-level future grants and contains " + f"managed access {'schema' if len(schemas) == 1 else 'schemas'} {_format_schema_list(schemas)}. " + f"Adding a schema-level future grant on the same object type to any of those schemas silently " + f"disables the database-level grant for that schema. Declaring future grants at the schema level " + f"alongside managed_access is the durable pattern. See {FUTURE_GRANT_PRECEDENCE_DOCS}" + ) + + return warnings + + +def _container_covers(container_type: str, container: str, object_name: str) -> bool: + """Does a container hold the named object, by identifier alone?""" + if container_type == ResourceType.ACCOUNT.value: + return True + # Quote-aware split: a quoted identifier can contain a literal dot (e.g. "a.b"), which a + # plain str.split would miscount and mis-classify. + parts = smart_split(object_name, ".") + if container_type == ResourceType.SCHEMA.value: + return len(parts) == 3 and ResourceName(".".join(parts[:2])) == ResourceName(container) + if container_type == ResourceType.DATABASE.value: + return len(parts) >= 2 and ResourceName(parts[0]) == ResourceName(container) + return False + + +def _covered_by_collection_grant(collection_grants: list["ManifestResource"], remote_res: dict) -> bool: + """ + Is a remote per-object grant already provided by a declared ALL or INHERITED grant? + + Snowflake materializes `GRANT ... ON ALL` into one grant per object, and those per-object + grants show up in remote state with nothing in the manifest to match them. Dropping them + would undo the collection grant on every apply. Inherited grants are matched the same + way so that migrating a config from per-object grants to an inherited grant does not + revoke access in the same run that establishes it. + """ + if remote_res.get("grant_type") != GrantType.OBJECT.value: + return False + for grant in collection_grants: + data = grant.data + if data["to"] != remote_res["to"]: + continue + # A declared `GRANT ALL` fans out into a concrete-privilege row per object (SELECT, + # INSERT, ...); matching the privilege exactly would miss those and drop them on every + # sync. ALL covers whatever privilege the row carries. Other collection grants still + # match their single privilege. + if data["priv"] != "ALL" and data["priv"] != remote_res["priv"]: + continue + if data["items_type"] != remote_res["on_type"]: + continue + if _container_covers(data["on_type"], data["on"], remote_res["on"]): + return True + return False + + +def _covered_by_imported_privileges(imported_privilege_grants: list["ManifestResource"], remote_res: dict) -> bool: + """ + Is a remote per-object grant already provided by a declared IMPORTED PRIVILEGES grant? + + `GRANT IMPORTED PRIVILEGES ON DATABASE TO ROLE ` is one statement, but + Snowflake fans it out in SHOW GRANTS into a row per object the share exposes -- every + view, function, procedure, schema, database role, class, tag and image repository in + the database, plus a USAGE row on the database itself. On the SNOWFLAKE shared database + that is several hundred rows. + + None of those rows can be in the manifest: config declares the single IMPORTED + PRIVILEGES grant, not the fan-out. Without this check they read as undeclared grants and + get revoked on every sync, undoing the access the declared grant just handed out. + + Unlike `_covered_by_collection_grant` this deliberately ignores the privilege. The + fan-out rows carry whatever privilege each object type takes -- SELECT on views, USAGE + on functions, READ on image repositories, APPLY on tags -- none of which is + "IMPORTED PRIVILEGES". Grantee and containment are what identify them. + + Matching on containment alone is safe because IMPORTED PRIVILEGES is only grantable on a + shared database, and objects in a shared database cannot be granted independently: the + share is the only source of privileges on them. + """ + if remote_res.get("grant_type") != GrantType.OBJECT.value: + return False + for grant in imported_privilege_grants: + data = grant.data + if data["to"] != remote_res["to"]: + continue + database = data["on"] + # The USAGE (and REFERENCE_USAGE) row Snowflake reports on the shared database itself + if remote_res["on_type"] == ResourceType.DATABASE.value and ResourceName(remote_res["on"]) == ResourceName( + database + ): + return True + if _container_covers(ResourceType.DATABASE.value, database, remote_res["on"]): + return True + return False + + +def manifest_inherited_grants(manifest: "Manifest") -> list["ManifestResource"]: + """Inherited grants declared in the manifest.""" + inherited = [] + for urn in manifest.urns: + if urn.resource_type != ResourceType.GRANT: + continue + item = manifest[urn] + if isinstance(item, ManifestResource) and item.data.get("grant_type") == GrantType.INHERITED.value: + inherited.append(item) + return inherited + + +def manifest_enables_inherited_grants(manifest: "Manifest") -> bool: + """Does the config itself turn the inherited grants preview on?""" + for urn in manifest.urns: + if urn.resource_type != ResourceType.ACCOUNT_PARAMETER: + continue + if ResourceName(urn.fqn.name) != ResourceName(INHERITED_GRANTS_FEATURE_FLAG): + continue + item = manifest[urn] + if isinstance(item, ManifestResource): + return str(item.data.get("value", "")).strip().upper() == "ENABLED" + return False + + +def raise_if_inherited_grants_unavailable(session, manifest: "Manifest") -> None: + """ + Fail before apply when config declares inherited grants an account cannot accept. + + Inherited grants are a preview feature. Without FEATURE_RBAC_INHERITED_GRANTS every + GRANT INHERITED statement fails as a syntax error partway through an apply, which is a + confusing way to learn the account is not opted in. + + A config that enables the parameter itself is left alone: the flag is off at plan time + by definition, and the apply turns it on before the grants run. + """ + declared = manifest_inherited_grants(manifest) + if not declared: + return + + if manifest_enables_inherited_grants(manifest): + return + + if data_provider.fetch_inherited_grants_enabled(session) is not False: + return + + example = grant_on_clause(_Grant(**declared[0].data)) + message = ( + f"This config declares {len(declared)} inherited grant(s), for example '{example}', but " + f"{INHERITED_GRANTS_FEATURE_FLAG} is not enabled on this account.\n" + ) + + # Preview access gates every preview feature at once and is normally on. When it is + # off, setting the parameter alone will not help, so say that rather than sending the + # operator down the wrong path. + if data_provider.fetch_preview_access_enabled(session) is False: + message += ( + " Preview features are disabled account-wide. Snowcap cannot change that -- it is a\n" + " system function, not a resource -- so an account admin needs to run:\n" + " SELECT SYSTEM$ENABLE_PREVIEW_ACCESS();\n" + " after which Snowcap can manage the parameter itself:\n" + ) + else: + message += " Let Snowcap manage it:\n" + + message += ( + " account_parameters:\n" + f" - name: {INHERITED_GRANTS_FEATURE_FLAG}\n" + " value: ENABLED\n" + " Or set it directly:\n" + f" ALTER ACCOUNT SET {INHERITED_GRANTS_FEATURE_FLAG} = 'ENABLED';\n" + f" See {INHERITED_GRANT_DOCS}" + ) + raise MissingPrivilegeException(message) + + +def manifest_state_entries(manifest: "Manifest", remote_state: Optional["State"] = None): + """Yield (urn, resource_type, data) for every concrete resource in the manifest, plus + anything remote state knows about that the manifest doesn't declare.""" + seen = set() + for urn, item in manifest.items(): + if isinstance(item, ManifestResource): + seen.add(urn) + yield urn, urn.resource_type, item.data + for urn, data in (remote_state or {}).items(): + if urn not in seen and isinstance(data, dict): + yield urn, urn.resource_type, data + + +def plan_entries(plan: "Plan"): + """Yield (urn, resource_type, data) for the resources a plan creates or updates. + + A plan only carries what is changing, so this sees less than the manifest does. It is + the fallback for `snowcap apply --plan plan.json`, where the manifest isn't rebuilt. + """ + for change in plan: + if isinstance(change, (CreateResource, UpdateResource)): + yield change.urn, change.urn.resource_type, change.after + + def manifest_future_grant_database_roles(manifest: "Manifest") -> set: """ Return the set of database role names that have future grants in the manifest. @@ -377,9 +702,12 @@ def to_dict(self) -> dict[str, str]: Plan = list[ResourceChange] -def plan_from_dict(plan_dict: dict) -> Plan: +def plan_from_dict(plan_dict) -> Plan: + # A plan file is either a bare list of changes (older format) or {"changes": [...], + # "levels": {...}} once dependency levels are persisted alongside it. + changes_data = plan_dict.get("changes", []) if isinstance(plan_dict, dict) else plan_dict changes: list[ResourceChange] = [] - for change in plan_dict: + for change in changes_data: action = change["action"] if action == "CREATE": container_descriptor: Optional[ContainerDescriptor] = None @@ -506,15 +834,34 @@ def resources(self): return list(self._resources.values()) -def dump_plan(plan: Plan, format: str = "json"): +def dump_plan(plan: Plan, format: str = "json", levels: Optional[dict[URN, int]] = None): if format == "json": - return json.dumps([change.to_dict() for change in plan], indent=2) + changes = [change.to_dict() for change in plan] + if levels is None: + return json.dumps(changes, indent=2) + # Persist each change's dependency level so `apply --plan` preserves ordering (ownership + # transfers before creates inside them, the inherited-grants feature flag before the + # grants that need it). Without it the apply-plan path has no levels and runs everything + # at level 0. Older plan files (a bare change list) fall back to that flat behaviour. + payload = { + "changes": changes, + "levels": {str(change.urn): levels.get(change.urn, 0) for change in plan}, + } + return json.dumps(payload, indent=2) elif format == "text": return _dump_plan_text(plan) else: raise Exception(f"Unsupported format {format}") +def levels_from_plan_dict(plan_dict) -> dict[URN, int]: + """Dependency levels persisted alongside a plan by dump_plan. Empty for older plan files + (a bare change list), so apply falls back to running everything at level 0.""" + if not isinstance(plan_dict, dict): + return {} + return {parse_URN(urn): level for urn, level in plan_dict.get("levels", {}).items()} + + def _render_value(value): """Render a value for display in plan output.""" if isinstance(value, str): @@ -676,20 +1023,15 @@ def _format_grant_name(urn: URN, change: "ResourceChange") -> str: # Handle FUTURE and ALL grants grant_type_str = str(grant_type).replace("GrantType.", "").upper() if grant_type else "OBJECT" - if grant_type_str == "FUTURE" and items_type: + if grant_type_str in ("FUTURE", "ALL", "INHERITED") and items_type: # Format: SELECT on FUTURE TABLES in DATABASE.MYDB → ROLE.X items_type_str = str(items_type).replace("ResourceType.", "").upper() # Pluralize the items type items_plural = items_type_str + "S" if not items_type_str.endswith("S") else items_type_str on_type_str = str(on_type).replace("ResourceType.", "").upper() if on_type else "" - return f"{priv} on FUTURE {items_plural} in {on_type_str}.{on} → {to_type_str}.{to}" - - elif grant_type_str == "ALL" and items_type: - # Format: SELECT on ALL TABLES in DATABASE.MYDB → ROLE.X - items_type_str = str(items_type).replace("ResourceType.", "").upper() - items_plural = items_type_str + "S" if not items_type_str.endswith("S") else items_type_str - on_type_str = str(on_type).replace("ResourceType.", "").upper() if on_type else "" - return f"{priv} on ALL {items_plural} in {on_type_str}.{on} → {to_type_str}.{to}" + # The account container has no name of its own + container = on_type_str if on_type_str == "ACCOUNT" else f"{on_type_str}.{on}" + return f"{priv} on {grant_type_str} {items_plural} in {container} → {to_type_str}.{to}" elif on_type: # Regular object grant: SELECT on TABLE.MYTABLE → ROLE.X @@ -873,6 +1215,40 @@ def print_plan(plan: Plan): print(dump_plan(plan, format="text")) +def print_surviving_drops(survivors: list["ResourceChange"]): + """ + Report drops Snowflake accepted without carrying out. + + Printed rather than logged: a logger warning scrolls past in the middle of an apply, + and the whole problem with these is that nothing tells you they happened. Formatted the + way the plan formats the same change, so the line reads as the one the user just saw + under DROP rather than as a raw URN. + """ + if not survivors: + return + + yellow = "\033[93m" + reset = "\033[0m" + print(f"\n{yellow}!{reset} {len(survivors)} drop(s) reported success but the resource is still there:\n") + for change in survivors: + print(f" {_format_resource_name(change.urn, change)}") + print( + "\n Snowflake accepted these statements without carrying them out. A REVOKE does that\n" + " when the executing role does not own the privilege or cannot resolve the grantee.\n" + " They will appear in the next plan as well. Check what granted the privilege\n" + " (SHOW GRANTS ... , granted_by) and whether that role is available to this session.\n" + ) + # Database-role grantees are the common cause: no held role could resolve one without + # USAGE on its database, so the revoke ran as SECURITYADMIN and silently did nothing. + # Name the role that would work. + db_role_databases = sorted({d for d in (_database_of_database_role_grantee(c) for c in survivors) if d}) + if db_role_databases: + print( + " Some are held by database roles, revocable only by a role that owns their\n" + " database. Grant your user the owner of: " + ", ".join(db_role_databases) + "\n" + ) + + def print_diffs(diffs): for action, target, deltas in diffs: print(f"[{action}]", target) @@ -1166,9 +1542,23 @@ def _raise_for_nonconforming_plan(self, session_ctx: SessionContext, plan: Plan) exception_block = "\n".join(exceptions) raise NonConformingPlanException("Non-conforming actions found in plan:\n" + exception_block) - def _warning_for_nonconforming_plan(self, session_ctx: SessionContext, plan: Plan): + def _warning_for_nonconforming_plan( + self, + session_ctx: SessionContext, + plan: Plan, + manifest: Optional[Manifest] = None, + remote_state: Optional[State] = None, + ): warnings = [] + # Future grant precedence is a property of the whole config, not of the changes in + # the plan, so use the manifest when we have it and fall back to the plan when we + # were handed a pre-built one. + if manifest is not None: + warnings.extend(future_grant_precedence_warnings(manifest_state_entries(manifest, remote_state))) + else: + warnings.extend(future_grant_precedence_warnings(plan_entries(plan))) + grant_to_system = False role_grant_to_system = False grant_on_all = False @@ -1230,18 +1620,22 @@ def fetch_remote_state(self, session, manifest: Manifest) -> State: if self._config.sync_resources: urns = [item for item in manifest.urns if item.resource_type not in self._config.sync_resources] - # Pre-compute whether manifest has future grants (for GRANT sync optimization) - has_future_grants = manifest_has_future_grants(manifest) - future_grant_roles = manifest_future_grant_roles(manifest) if has_future_grants else set() - future_grant_database_roles = manifest_future_grant_database_roles(manifest) if has_future_grants else set() for resource_type in self._config.sync_resources: - # Pass include_future_grants=False for grants if manifest has no future grants - # Also pass future_grant_roles to only query roles that have future grants + # Future grants are read in full whenever grants are synced, and neither + # the query nor the set of roles queried is narrowed to what the manifest + # declares. Narrowing would be sound for a plan that only creates, but + # syncing a resource type means removing what is not declared, and a future + # grant absent from config is precisely what has to be found. + # + # Skipping the query when the manifest declared no future grants kept the + # ones already in Snowflake out of remote state, so sync could not propose + # dropping them -- unseen rather than deliberately kept, with nothing in + # the plan to say so. Migrating a config from ALL plus FUTURE pairs to + # inherited grants removes the last future grant and hit exactly that: 26 + # orphaned future grants, zero drops, no warning. list_kwargs: dict[str, Any] = {} if resource_type == ResourceType.GRANT: - list_kwargs["include_future_grants"] = has_future_grants - list_kwargs["future_grant_roles"] = future_grant_roles - list_kwargs["future_grant_database_roles"] = future_grant_database_roles + list_kwargs["include_future_grants"] = True for fqn in data_provider.list_resource(session, resource_label_for_type(resource_type), **list_kwargs): if self._config.scope == BlueprintScope.DATABASE and fqn.database != self._config.database: continue @@ -1634,8 +2028,49 @@ def _finalize(self, session_ctx: SessionContext) -> None: self._create_ownership_refs(session_ctx) self._create_grandparent_refs() self._create_stage_privilege_refs() + # Must run after _build_resource_graph populates self._root and before + # _finalize_resources locks resources, like the other ref-creators above — + # otherwise it walks an empty graph and the grant->flag edge is never added. + self._link_inherited_grants_to_feature_flag() self._finalize_resources() + def _link_inherited_grants_to_feature_flag(self) -> None: + """ + Make inherited grants depend on the account parameter that enables them. + + A config can turn the preview on itself: + + account_parameters: + - name: FEATURE_RBAC_INHERITED_GRANTS + value: ENABLED + + Without a dependency between the two, both land at the same level of the plan and + run concurrently, so the grants can reach Snowflake before the parameter does. The + reference is only added when the parameter is declared, since a reference to a + resource that is not in the manifest is an error in its own right. + """ + resources = [r for r in _walk(self._root) if isinstance(r, Resource)] + feature_flag = next( + ( + r + for r in resources + if r.resource_type == ResourceType.ACCOUNT_PARAMETER + and isinstance(r, NamedResource) + and ResourceName(r.name) == ResourceName(INHERITED_GRANTS_FEATURE_FLAG) + ), + None, + ) + if feature_flag is None: + return + + for resource in resources: + if ( + resource.resource_type == ResourceType.GRANT + and getattr(resource, "grant_type", None) == GrantType.INHERITED + and not resource._finalized + ): + resource.requires(feature_flag) + def generate_manifest(self, session_ctx: SessionContext) -> Manifest: manifest = Manifest(account_locator=session_ctx["account_locator"]) self._finalize(session_ctx) @@ -1684,6 +2119,7 @@ def plan(self, session) -> Plan: logger.debug(f" {key}") session_ctx = data_provider.fetch_session(session) manifest = self.generate_manifest(session_ctx) + raise_if_inherited_grants_unavailable(session, manifest) remote_state = self.fetch_remote_state(session, manifest) try: finished_plan = diff(remote_state, manifest) @@ -1715,7 +2151,7 @@ def plan(self, session) -> Plan: logger.error(manifest) raise self._raise_for_nonconforming_plan(session_ctx, finished_plan) - self._warning_for_nonconforming_plan(session_ctx, finished_plan) + self._warning_for_nonconforming_plan(session_ctx, finished_plan, manifest, remote_state) return finished_plan def apply(self, session, plan: Optional[Plan] = None) -> None: @@ -1840,7 +2276,19 @@ def process_commands(commands, roles, available_roles): _raise_if_plan_would_drop_session_user(session_ctx, plan) - sql_commands_per_change, available_roles = compile_plan_to_sql(session_ctx, plan) + # Databases mounted from a share, resolved only when the plan actually revokes a + # grant: privileges on those cannot be revoked one at a time. See + # lifecycle.drop_shared_database_grant. + shared_databases: Optional[set[str]] = None + database_owners: Optional[dict[str, str]] = None + if any(c.urn.resource_type == ResourceType.GRANT for c in plan): + # Both read the same cached SHOW DATABASES response, so this is one query. + shared_databases = data_provider.list_shared_database_names(session) + database_owners = data_provider.list_database_owners(session) + + sql_commands_per_change, available_roles = compile_plan_to_sql( + session_ctx, plan, shared_databases, database_owners + ) roles_list: list[Any] = [] additive_commands = [] destructive_commands = [] @@ -1867,6 +2315,9 @@ def process_commands(commands, roles, available_roles): # Print completion summary print_apply_summary(plan, "end") + if not self._config.dry_run: + print_surviving_drops(surviving_drops(session, [c["change"] for c in destructive_commands])) + def _add(self, resource: Resource): if self._finalized: raise Exception("Cannot add resources to a finalized blueprint") @@ -1898,21 +2349,171 @@ def owner_for_change(change: ResourceChange) -> Optional[ResourceName]: return None +def _inherited_grant_execution_role(change: ResourceChange) -> Optional[str]: + """ + The role an inherited grant declares itself to be managed by, if any. + + Grants get a default owner (SYSADMIN, or ACCOUNTADMIN for integrations and Snowflake + schemas) when config does not name one, so those defaults are not treated as a + delegation and fall through to the usual SECURITYADMIN strategy. + """ + if change.urn.resource_type != ResourceType.GRANT: + return None + if isinstance(change, (CreateResource, UpdateResource)): + data = change.after + elif isinstance(change, DropResource): + data = change.before + else: + return None + if not isinstance(data, dict) or data.get("grant_type") != GrantType.INHERITED.value: + return None + owner = data.get("owner") + if not owner or owner in ("SYSADMIN", "ACCOUNTADMIN"): + return None + return str(owner) + + +def _shared_database_for_grant(change: ResourceChange, shared_databases: Optional[set[str]]) -> Optional[str]: + """ + The shared database a grant being dropped belongs to, if any. + + A grant on a shared database, or on anything inside one, is part of the fan-out of an + IMPORTED PRIVILEGES grant and cannot be revoked on its own. Returns the database so the + caller can revoke the share instead; None for ordinary grants. + """ + if not shared_databases or not isinstance(change, DropResource): + return None + if change.urn.resource_type != ResourceType.GRANT: + return None + on = change.before.get("on") + if not on: + return None + # A share fan-out only touches the database itself or objects that live inside one. An + # account-level object (warehouse, integration, role, ...) that merely shares a name with + # an imported database must revoke its own privilege, not the share. + on_type = change.before.get("on_type") + if on_type is not None: + try: + rt: Optional[ResourceType] = ResourceType(str(on_type)) + except ValueError: + rt = None + if rt is not None and rt != ResourceType.DATABASE and isinstance(RESOURCE_SCOPES.get(rt), AccountScope): + return None + # Quote-aware split (like _container_covers): a database quoted with a literal dot would + # otherwise mis-split and miss the shared-databases set. + database = smart_split(str(on), ".")[0].strip('"').upper() + return database if database in shared_databases else None + + +def surviving_drops(session, changes: list[ResourceChange]) -> list[ResourceChange]: + """ + Which of the resources an apply just dropped are still there. + + Snowflake does not always fail a statement it could not carry out. REVOKE is the + conspicuous case: it reports success when the executing role does not own the privilege + or cannot resolve the grantee, rather than raising. The apply sees no exception, counts + the drop as applied, and the grant survives -- so the same drop comes back in the next + plan, and the one after that, with nothing in any output saying why. + + Treating "no exception" as "applied" is what makes that invisible. Reading the dropped + resources back is the only thing that distinguishes a real drop from one Snowflake + quietly declined. + + Costs one existence check per dropped resource, and runs only when a plan dropped + something. A resource type that cannot be read back is skipped rather than reported: not + being able to confirm a drop is not evidence that it failed. + """ + dropped = [change for change in changes if isinstance(change, DropResource)] + if not dropped: + return [] + + # The apply just changed the very state these checks read. reset_cache() alone + # leaves the ACCOUNT_USAGE grant snapshot in place, and _show_grants_to_role serves + # it for account-role grants — so a revoked grant would re-appear as a false survivor + # on use_account_usage runs. Clear both. + reset_cache() + data_provider.reset_account_usage_caches() + + survivors: list[ResourceChange] = [] + for change in dropped: + try: + if data_provider.fetch_resource(session, change.urn, existence_only=True) is not None: + survivors.append(change) + except Exception: # pragma: no cover - defensive, see docstring + logger.debug(f"Could not verify drop of {change.urn}", exc_info=True) + return survivors + + +def _database_of_database_role_grantee(change: ResourceChange) -> Optional[str]: + """ + The database a grant's grantee belongs to, when that grantee is a database role. + + A database role is named . and lives inside its database. Managing a + grant held by one needs a role that can see that database: account-level MANAGE GRANTS + lets SECURITYADMIN administer grants, but without USAGE on the database it cannot + resolve the grantee, and REVOKE reports success rather than failing on a grantee it + cannot resolve. The grant survives, and the same drop comes back in every later plan. + + Returns None for grants to account roles, which account-level authority does reach. + """ + if change.urn.resource_type != ResourceType.GRANT: + return None + if isinstance(change, CreateResource): + data = change.after + elif isinstance(change, DropResource): + data = change.before + else: + return None + if str(data.get("to_type", "")).replace("_", " ").upper() != "DATABASE ROLE": + return None + grantee = str(data.get("to", "")) + if "." not in grantee: + return None + return grantee.split(".")[0].strip('"').upper() + + def execution_strategy_for_change( change: ResourceChange, available_roles: list[ResourceName], default_role: ResourceName, + transferred_owners: Optional[dict[URN, ResourceName]] = None, + database_owners: Optional[dict[str, str]] = None, ) -> tuple[ResourceName, bool]: change_owner = owner_for_change(change) if resource_type_is_grant(change.urn.resource_type): + # Inherited grants require MANAGE GRANTS on the container, which is how Snowflake + # lets a database or schema admin manage access without account-wide authority. An + # explicit `owner` on the grant names that delegated role, so it is used in + # preference to SECURITYADMIN when the session has it. + # https://docs.snowflake.com/en/user-guide/container-manage-grants-intro + inherited_grant_owner = _inherited_grant_execution_role(change) + if inherited_grant_owner and inherited_grant_owner in available_roles: + return ResourceName(inherited_grant_owner), False + # 2024-10-22: maybe the better thing to do is check role privs selectively - if isinstance(change, CreateResource) and change.urn.resource_type == ResourceType.GRANT: - execution_role = system_role_for_priv(change.after["priv"]) + # + # Revokes use the same role as grants. An account-level privilege belongs to the + # system role that owns it, and Snowflake will not take one back from a role that + # does not -- but it reports success rather than failing, so a revoke run as + # SECURITYADMIN silently leaves the privilege in place and the same drop reappears + # in every later plan. + if change.urn.resource_type == ResourceType.GRANT and isinstance(change, (CreateResource, DropResource)): + grant_data = change.after if isinstance(change, CreateResource) else change.before + execution_role = system_role_for_priv(grant_data["priv"]) if execution_role and execution_role in available_roles: return ResourceName(execution_role), False + # Grants held by a database role are managed from inside that database, by the role + # that owns it. SECURITYADMIN can hold MANAGE GRANTS and still be unable to resolve + # the grantee without USAGE on the database. + grantee_database = _database_of_database_role_grantee(change) + if grantee_database and database_owners: + database_owner = database_owners.get(grantee_database) + if database_owner and ResourceName(database_owner) in available_roles: + return ResourceName(database_owner), False + if "SECURITYADMIN" in available_roles: return ResourceName("SECURITYADMIN"), False @@ -1962,6 +2563,19 @@ def execution_strategy_for_change( ) elif isinstance(change, (UpdateResource, DropResource, TransferOwnership)): + if ( + isinstance(change, TransferOwnership) + and change.urn.resource_type in OWNER_EXECUTED_RESOURCE_TYPES + and "SECURITYADMIN" in available_roles + ): + # Owner-executed objects run their body or schedule with the privileges of + # their owner. Snowflake tightened authorization for transferring them: + # GRANT OWNERSHIP fails unless the receiving role is in the caller's active + # role hierarchy or the caller holds account-level MANAGE GRANTS. Running the + # transfer as the outgoing owner, which is what Snowcap does for every other + # resource, satisfies neither condition in the common case. + # https://docs.snowflake.com/en/user-guide/inherited-grants-intro + return ResourceName("SECURITYADMIN"), False if change_owner: return change_owner, False else: @@ -1994,7 +2608,15 @@ def execution_strategy_for_change( f" GRANT ROLE {system_role} TO USER your_user;" ) elif isinstance(change.resource_cls.scope, (DatabaseScope, SchemaScope)) and change.container: - container_owner = ResourceName(change.container[1]) + container_urn, container_owner = change.container + container_owner = ResourceName(container_owner) + # The container's owner is recorded when the plan is built. When the same plan + # also transfers that container, the transfer has already run by the time this + # CREATE executes -- a container sits at a lower dependency level than the + # resources inside it -- so the role recorded here no longer owns the container + # and cannot create anything in it. Use the owner the container ends up with. + if transferred_owners and container_urn in transferred_owners: + container_owner = transferred_owners[container_urn] transfer_ownership = container_owner != change_owner if transfer_ownership and change.urn.resource_type == ResourceType.NOTEBOOK: raise Exception("Notebook ownership cannot be transferred") @@ -2007,6 +2629,9 @@ def sql_commands_for_change( change: ResourceChange, available_roles: list[ResourceName], default_role: ResourceName, + transferred_owners: Optional[dict[URN, ResourceName]] = None, + shared_databases: Optional[set[str]] = None, + database_owners: Optional[dict[str, str]] = None, ) -> tuple[ResourceName, list[str]]: """ In Snowflake's RBAC model, a session has an active role, and zero or more secondary roles. @@ -2018,7 +2643,7 @@ def sql_commands_for_change( - Otherwise, the PUBLIC role is activated (PUBLIC cannot be revoked) - Any time the USE ROLE command is run, the active role is switched - A session may run any command thats allowed by the active role or any role downstream from it in the role hierarchy. + A session may run any command that's allowed by the active role or any role downstream from it in the role hierarchy. When secondary roles are active (by running the command USE SECONDARY ROLES ALL), then the session may also run any command that any secondary role or a role downstream from it is allowed to run. @@ -2037,6 +2662,8 @@ def sql_commands_for_change( change, available_roles, default_role, + transferred_owners, + database_owners, ) if isinstance(change, CreateResource): @@ -2081,11 +2708,15 @@ def sql_commands_for_change( copy_current_grants=True, ) ) - change_cmd = lifecycle.drop_resource( - change.urn, - change.before, - if_exists=True, - ) + shared_database = _shared_database_for_grant(change, shared_databases) + if shared_database: + change_cmd = lifecycle.drop_shared_database_grant(change.before, shared_database) + else: + change_cmd = lifecycle.drop_resource( + change.urn, + change.before, + if_exists=True, + ) elif isinstance(change, TransferOwnership): change_cmd = lifecycle.transfer_resource( change.urn, @@ -2099,7 +2730,10 @@ def sql_commands_for_change( def compile_plan_to_sql( - session_ctx: SessionContext, plan: Plan + session_ctx: SessionContext, + plan: Plan, + shared_databases: Optional[set[str]] = None, + database_owners: Optional[dict[str, str]] = None, ) -> tuple[list[dict], list[ResourceName]]: """Compile the plan into a list of SQL command lists, one per change. @@ -2111,6 +2745,12 @@ def compile_plan_to_sql( available_roles = session_ctx["available_roles"].copy() default_role = session_ctx["role"] current_user = ResourceName(session_ctx.get("user", "")) if session_ctx.get("user") else None + # Containers this plan hands to a new owner. Resources created inside one of them + # have to be created by the owner it ends up with, not the one it had when the plan + # was built, because the transfer runs first. + transferred_owners: dict[URN, ResourceName] = { + change.urn: ResourceName(change.to_owner) for change in plan if isinstance(change, TransferOwnership) + } for change in plan: if isinstance(change, CreateResource): if change.urn.resource_type == ResourceType.ROLE: @@ -2120,10 +2760,16 @@ def compile_plan_to_sql( if change.after.get("to_role") and change.after["to_role"] in available_roles: available_roles.append(ResourceName(change.after["role"])) # Handle role grants to the current user - elif current_user and change.after.get("to_user") and ResourceName(change.after["to_user"]) == current_user: + elif ( + current_user + and change.after.get("to_user") + and ResourceName(change.after["to_user"]) == current_user + ): available_roles.append(ResourceName(change.after["role"])) for change in plan: - role, commands = sql_commands_for_change(change, available_roles, default_role) + role, commands = sql_commands_for_change( + change, available_roles, default_role, transferred_owners, shared_databases, database_owners + ) sql_commands_per_change.append({"role": role, "commands": commands, "change": change}) return sql_commands_per_change, available_roles @@ -2290,31 +2936,35 @@ def _diff_resource_data(lhs: dict, rhs: dict) -> dict: logger.debug(f" resource_type match: {urn.resource_type == state_urn.resource_type}") logger.debug(f" account_locator match: {urn.account_locator == state_urn.account_locator}") - grant_on_all_resources = [ + collection_grants = [ r for r in manifest.resources if not isinstance(r, ResourcePointer) and r.resource_cls == Grant - and r.data["grant_type"] == GrantType.ALL.value + and r.data["grant_type"] in (GrantType.ALL.value, GrantType.INHERITED.value) + ] + + imported_privilege_grants = [ + r + for r in manifest.resources + if not isinstance(r, ResourcePointer) + and r.resource_cls == Grant + and r.data["priv"] == "IMPORTED PRIVILEGES" + and r.data["on_type"] == ResourceType.DATABASE.value ] # Resources in remote state but not in the manifest should be removed for urn in state_urns - manifest_urns: remote_res = remote_state[urn] - # If there are ALL grants and the current resource is included we should not drop it - if grant_on_all_resources and remote_res.get("grant_type") == GrantType.OBJECT.value: - matching_grants = [ - r - for r in grant_on_all_resources - if r.data["priv"] == remote_res["priv"] - and r.data["to"] == remote_res["to"] - and r.data["items_type"] == remote_res["on_type"] - and r.data["on"] == ".".join(remote_res["on"].split(".")[:-1]) - ] - if matching_grants: - continue - else: - changes.append(DropResource(urn, remote_state[urn])) + # A grant on a collection of objects covers the per-object grants it produced, which + # are in remote state but never in the manifest. Dropping those would revoke the + # access the collection grant just handed out. + if _covered_by_collection_grant(collection_grants, remote_res): + continue + # Same reasoning for the fan-out of an IMPORTED PRIVILEGES grant on a shared database. + if _covered_by_imported_privileges(imported_privilege_grants, remote_res): + continue + changes.append(DropResource(urn, remote_state[urn])) # Resources in the manifest but not in remote state should be added for urn in manifest_urns - state_urns: diff --git a/snowcap/blueprint_config.py b/snowcap/blueprint_config.py index 53f51777..e2448724 100644 --- a/snowcap/blueprint_config.py +++ b/snowcap/blueprint_config.py @@ -103,7 +103,7 @@ def set_vars_defaults(vars_spec: list[dict], vars: dict) -> dict: if "default" in spec: new_vars[spec["name"]] = spec["default"] else: - var_type = spec.get('type', 'unknown') + var_type = spec.get("type", "unknown") raise MissingVarException( f"Required var '{spec['name']}' ({var_type}) is missing and has no default value.\n" f" Provide it with: --vars '{{\"{ spec['name'] }\": ...}}'" diff --git a/snowcap/cli.py b/snowcap/cli.py index 72d618d2..a771cff7 100644 --- a/snowcap/cli.py +++ b/snowcap/cli.py @@ -240,14 +240,14 @@ def plan( cli_config["vars"] = merge_vars(cli_config.get("vars", {}), env_vars) try: - plan_obj = blueprint_plan(yaml_config, cli_config) + plan_obj, plan_levels = blueprint_plan(yaml_config, cli_config) if output_file: with open(output_file, "w") as f: - f.write(dump_plan(plan_obj, format="json")) + f.write(dump_plan(plan_obj, format="json", levels=plan_levels)) else: output = None if json_output: - output = dump_plan(plan_obj, format="json") + output = dump_plan(plan_obj, format="json", levels=plan_levels) else: output = dump_plan(plan_obj, format="text") print(output) diff --git a/snowcap/data_provider.py b/snowcap/data_provider.py index 91a7afd7..6bb9068b 100644 --- a/snowcap/data_provider.py +++ b/snowcap/data_provider.py @@ -3,7 +3,6 @@ import json import logging import sys -from functools import cache from typing import Any, Optional, TypedDict, Union import pytz @@ -21,17 +20,25 @@ from .client import ( ACCESS_CONTROL_ERR, DOES_NOT_EXIST_ERR, + INVALID_COLUMN_ERR, INVALID_IDENTIFIER, OBJECT_DOES_NOT_EXIST_ERR, UNSUPPORTED_FEATURE, execute, execute_in_parallel, ) -from .enums import AccountEdition, GrantType, ResourceType, WarehouseSize -from .identifiers import FQN, URN, parse_FQN, resource_type_for_label +from .enums import ( + INHERITED_GRANTS_FEATURE_FLAG, + AccountEdition, + GrantType, + ResourceType, + WarehouseSize, +) +from .identifiers import FQN, URN, parse_FQN, resource_label_for_type, resource_type_for_label from .parse import ( _parse_column, _parse_dynamic_table_text, + format_collection_string, parse_collection_string, parse_region, parse_view_ddl, @@ -182,6 +189,212 @@ def _fail_if_not_granted(result, *args): raise Exception(result[0]["status"], *args) +def _is_inherited_grant(row: dict[str, Any]) -> bool: + """ + True when a grant row was produced by an inherited grant (GRANT INHERITED ...). + + An inherited grant is a container-level grant that applies to every current and future + object of a type in an ACCOUNT, DATABASE, or SCHEMA. Snowflake reports these rows in + SHOW GRANTS and in ACCOUNT_USAGE.GRANTS_TO_ROLES with IS_INHERITED set, and with an + empty NAME, because the grant is defined on the container rather than on individual + securables. + + Accounts that have not enabled FEATURE_RBAC_INHERITED_GRANTS never produce these rows, + and older Snowflake versions do not return the column at all, so a missing column reads + as "not inherited". + """ + for key in ("is_inherited", "IS_INHERITED"): + if key in row: + value = row[key] + if isinstance(value, str): + return value.strip().lower() in ("true", "t", "yes", "y") + return bool(value) + return False + + +def _is_role_hierarchy_grant(row: dict[str, Any]) -> bool: + """ + True when a grant row describes one role being granted to another. + + Snowflake reports granting a role as a grant held by the grantee, so SHOW GRANTS and + ACCOUNT_USAGE return these alongside object grants. Snowcap models them separately, as + RoleGrant and DatabaseRoleGrant, listed by list_role_grants() and + list_database_role_grants(). Listing them as Grants as well would describe the same + Snowflake fact under two resource types, so the declared grant never matches the one + read back and sync proposes dropping it on every run. + + That drop is also unrunnable for a database role: a Grant revokes with + REVOKE ON ..., which for a database role reads REVOKE USAGE ON + DATABASE ROLE, and Snowflake rejects it as an unsupported feature. The revoke database + roles actually take is REVOKE DATABASE ROLE FROM ROLE , which + DatabaseRoleGrant already builds. + + ACCOUNT_USAGE spells the type DATABASE_ROLE and SHOW GRANTS spells it DATABASE ROLE, + so both are matched. + """ + return row["granted_on"].replace("_", " ").upper() in ("ROLE", "DATABASE ROLE") + + +def _is_intrinsic_database_role_usage(row: dict[str, Any], grantee: str) -> bool: + """ + True when a row is the USAGE on its own database that a database role is born with. + + Creating a database role gives it USAGE on the database it belongs to. Snowflake + reports that in SHOW GRANTS like any other grant, but with an empty granted_by and a + timestamp matching the CREATE, because no role granted it -- it is part of the role + existing, the way OWNERSHIP is. + + Nothing can revoke it. REVOKE reports success and leaves it in place, even run as the + database owner, so listing it as a grant puts a row in remote state that no config can + declare away and no apply can remove: sync proposes the same drop on every run, forever. + + An explicitly granted USAGE on the same database is a second, separate row with + granted_by populated. The two are indistinguishable once reduced to a grant URN, so + this skips both and a declared USAGE on a database role's own database simply re-grants + each apply -- harmless, since the role already has it. + """ + if row["privilege"] != "USAGE": + return False + if row["granted_on"].replace("_", " ").upper() != "DATABASE": + return False + if "." not in grantee: + return False + return str(row["name"]).upper() == grantee.split(".")[0].upper() + + +def _imported_privileges_priv(row: dict[str, Any], shared_databases: set[str]) -> Optional[str]: + """ + The privilege a grant on a share-backed database should be identified by. + + GRANT IMPORTED PRIVILEGES ON DATABASE is how access to a shared database is given, + and Snowflake reports the resulting grant on the database as plain USAGE. Identifying + it as USAGE means the declared grant never matches the one read back, so every plan + proposes creating it again -- forever, and invisibly, since re-granting changes nothing. + + fetch_grant already resolves this, but syncing a resource type builds remote state from + list_* alone and discards the manifest URNs, so that path never runs for a synced grant. + + Returns None for anything else, including USAGE on an ordinary database, where USAGE + means USAGE. + """ + if row["privilege"] != "USAGE": + return None + if row["granted_on"].replace("_", " ").upper() != "DATABASE": + return None + if str(row["name"]).upper() not in shared_databases: + return None + return "IMPORTED PRIVILEGES" + + +def _granted_on_label(granted_on: str) -> str: + """ + The object-type half of a grant URN's `on`, normalized the way the manifest builds it. + + Snowflake sometimes reports a grant against a different name than the one its DDL uses: + SHOW GRANTS says CORTEX_AGENT_SERVER for the object GRANT and CREATE call an MCP SERVER. + ResourceType maps those synonyms, and grant_fqn runs the manifest side through + resource_label_for_type, so going through the same function here is what makes the two + sides comparable. + + Taking the raw string instead leaves remote state identifying the grant as + cortex_agent_server/... while the manifest calls it mcp_server/..., so the declared grant + never matches the one read back. Every plan then both creates and drops it, and since + drops run after creates, applying takes the access away. + + ResourceType spells its members with spaces and Snowflake uses underscores, hence the + substitution. For every type that is not a synonym this returns exactly what + granted_on.lower() did, and anything ResourceType does not know falls back to it. + """ + try: + return resource_label_for_type(ResourceType(granted_on.replace("_", " "))) + except ValueError: + return granted_on.lower() + + +def _normalize_future_grant_name(name: str) -> str: + """Normalize the a SHOW FUTURE GRANTS name embeds (e.g. `DB.SCH.`) + through ResourceType, so a synonym type -- SHOW reports CORTEX_AGENT_SERVER for what the + manifest calls MCP_SERVER -- matches the declared grant instead of forcing a non-converging + DROP+CREATE that revokes the future grant. No-op for non-synonym and unknown types.""" + prefix, sep, bracketed = name.rpartition(".") + if not sep or not (bracketed.startswith("<") and bracketed.endswith(">")): + return name + try: + canonical = str(ResourceType(bracketed[1:-1].replace("_", " "))).replace(" ", "_") + except ValueError: + return name + return f"{prefix}.<{canonical}>" + + +def inherited_grant_fqn(grant: dict[str, Any], to_label: str, grantee: str) -> Optional[FQN]: + """ + Build the URN-level identity of an inherited grant from a SHOW GRANTS row. + + Returns None when the row does not name a container Snowcap understands, so an + unrecognized INHERITED_FROM value is skipped rather than turned into a grant that + cannot be revoked. + """ + container_type = str(grant.get("inherited_from") or "").upper() + database = str(grant.get("inherited_from_database") or "") + schema = str(grant.get("inherited_from_schema") or "") + + if container_type == "ACCOUNT": + container = "ACCOUNT" + elif container_type == "DATABASE": + container = database + elif container_type == "SCHEMA": + container = f"{database}.{schema}" + else: + logger.debug(f"Skipping inherited grant with unrecognized container {container_type!r}") + return None + + # Normalize the object type the way the manifest does — grant_fqn passes a ResourceType + # to format_collection_string — so synonym types (SHOW GRANTS reports CORTEX_AGENT_SERVER + # for what GRANT/CREATE call an MCP SERVER) match the declared type instead of producing a + # non-converging DROP+CREATE that revokes the inherited grant in sync mode. + try: + items_type: Any = ResourceType(grant["granted_on"].replace("_", " ")) + except ValueError: + items_type = grant["granted_on"].replace("_", " ") + collection = format_collection_string(container, items_type) + return FQN( + name=ResourceName("GRANT"), + params={ + "grant_type": GrantType.INHERITED.value, + "priv": grant["privilege"], + "on": f"{container_type.lower()}/{collection}", + "to": f"{to_label}/{grantee}", + }, + ) + + +def _drop_inherited_grants(rows: list[dict[str, Any]], context: str) -> list[dict[str, Any]]: + """ + Remove inherited grant rows from a grant listing. + + Snowcap has no representation for an inherited grant, and the rows cannot be treated as + object grants: they carry no object name, and revoking one requires + REVOKE INHERITED ON ALL IN FROM rather than a + per-object REVOKE. Left in remote state they would make grant sync mode emit invalid + REVOKEs and `snowcap export` write grants that cannot be applied. + + Filtering them out means Snowcap neither manages nor disturbs inherited grants. It also + means privileges a role holds only through inheritance are invisible to Snowcap's + preflight privilege checks, which can make those checks over-strict; failing loudly is + the safer direction until inherited grants are modeled. + """ + if not rows: + return rows + kept = [row for row in rows if not _is_inherited_grant(row)] + dropped = len(rows) - len(kept) + if dropped: + logger.debug( + f"Ignoring {dropped} inherited grant(s) in {context}. Snowcap does not manage grants created with " + "GRANT INHERITED." + ) + return kept + + def _fetch_grant_to_role( session: SnowflakeConnection, grant_type: GrantType, @@ -198,11 +411,16 @@ def _fetch_grant_to_role( grants = _show_future_grants_to_role(session, role, cacheable=True) else: grants = _show_grants_to_role(session, role, role_type=role_type, cacheable=True) + # Compare object types the way list_grants does. Snowflake sometimes reports a grant + # against a different name than the one its DDL uses -- CORTEX_AGENT_SERVER for what + # GRANT calls an MCP SERVER -- so a raw string comparison never matches the declared + # grant, and the plan proposes creating it on every run. + wanted_type = _granted_on_label(granted_on) for grant in grants: name = "ACCOUNT" if grant["granted_on"] == "ACCOUNT" else grant["name"] # Use ResourceName for comparison to handle quoted identifiers correctly name_matches = ResourceName(name) == ResourceName(on_name) if name != "ACCOUNT" else name == on_name - if grant["granted_on"] == granted_on and grant["privilege"] == privilege and name_matches: + if _granted_on_label(grant["granted_on"]) == wanted_type and grant["privilege"] == privilege and name_matches: return grant return None @@ -552,7 +770,7 @@ def _show_users(session) -> list[dict]: def _get_account_privilege_roles(session: SnowflakeConnection) -> dict[str, list[ResourceName]]: grant_map: dict[str, list[ResourceName]] = {} - grants = execute(session, "SHOW GRANTS ON ACCOUNT") + grants = _drop_inherited_grants(execute(session, "SHOW GRANTS ON ACCOUNT"), "SHOW GRANTS ON ACCOUNT") for grant in grants: # Skip system grants if grant["granted_by"] == "": @@ -593,6 +811,23 @@ def _show_grants_to_role( 'granted_by': 'ACCOUNTADMIN' } """ + grants = _show_all_grants_to_role(session, role, role_type=role_type, cacheable=cacheable) + return _drop_inherited_grants(grants, f"grants to {role_type} {role}") + + +def _show_all_grants_to_role( + session: SnowflakeConnection, + role: ResourceName, + role_type: ResourceType = ResourceType.ROLE, + cacheable: bool = False, +) -> list[dict[str, Any]]: + """ + Every grant to a role, object grants and inherited grants alike. + + Callers almost always want _show_grants_to_role() (object grants only) or + _show_inherited_grants_to_role(); this is the shared source both read from, so a role's + grants are fetched once regardless of which kinds the caller cares about. + """ # Automatically use ACCOUNT_USAGE cache for regular roles if it's been populated if role_type == ResourceType.ROLE: session_id = id(session) @@ -608,13 +843,29 @@ def _show_grants_to_role( return filtered_grants # Fall back to SHOW GRANTS - grants = execute( + return execute( session, f"SHOW GRANTS TO {role_type} {role}", cacheable=cacheable, empty_response_codes=[DOES_NOT_EXIST_ERR], ) - return grants + + +def _show_inherited_grants_to_role( + session: SnowflakeConnection, + role: ResourceName, + role_type: ResourceType = ResourceType.ROLE, + cacheable: bool = True, +) -> list[dict[str, Any]]: + """ + The inherited grants held by a role. + + Snowflake does not enumerate the individual securables an inherited grant covers, so + each row describes the container-level grant itself: NAME is empty, GRANTED_ON is the + object type the grant applies to, and INHERITED_FROM* identify the container. + """ + grants = _show_all_grants_to_role(session, role, role_type=role_type, cacheable=cacheable) + return [grant for grant in grants if _is_inherited_grant(grant)] def _show_future_grants_to_role( @@ -638,6 +889,7 @@ def _show_future_grants_to_role( empty_response_codes=[DOES_NOT_EXIST_ERR], ) for grant in grants: + grant["name"] = _normalize_future_grant_name(grant["name"]) grant["granted_on"] = "DATABASE" if len(grant["name"].split(".")) == 2 else "SCHEMA" return grants @@ -667,6 +919,7 @@ def _show_future_grants_to_database_role( # Infer granted_on from the name pattern # Database-level: "DB_NAME." (2 parts) # Schema-level: "DB_NAME.SCHEMA_NAME.
" (3 parts) + grant["name"] = _normalize_future_grant_name(grant["name"]) grant["granted_on"] = "DATABASE" if len(grant["name"].split(".")) == 2 else "SCHEMA" return grants @@ -755,7 +1008,66 @@ def fetch_region(session: SnowflakeConnection): return region -@cache +def fetch_inherited_grants_enabled(session: SnowflakeConnection) -> Optional[bool]: + """ + Is FEATURE_RBAC_INHERITED_GRANTS enabled for this account? + + Returns None when the answer cannot be determined -- the parameter does not exist on + Snowflake versions without the preview, and reading account parameters requires + privileges the session may not hold. Callers treat None as "assume enabled" so that an + unreadable parameter never blocks an apply that would otherwise succeed. + """ + session_id = id(session) + if session_id in _INHERITED_GRANTS_ENABLED_CACHE: + return _INHERITED_GRANTS_ENABLED_CACHE[session_id] + + enabled: Optional[bool] = None + try: + rows = execute( + session, + f"SHOW PARAMETERS LIKE '{INHERITED_GRANTS_FEATURE_FLAG}' IN ACCOUNT", + cacheable=True, + ) + for row in rows: + if str(row.get("key", "")).upper() == INHERITED_GRANTS_FEATURE_FLAG: + enabled = str(row.get("value", "")).strip().upper() == "ENABLED" + break + except Exception as err: + logger.debug(f"Could not read FEATURE_RBAC_INHERITED_GRANTS: {err}") + + _INHERITED_GRANTS_ENABLED_CACHE[session_id] = enabled + return enabled + + +def fetch_preview_access_enabled(session: SnowflakeConnection) -> Optional[bool]: + """ + Does this account have access to preview features at all? + + Preview access is on by default for most accounts, and it gates every preview feature + at once. It is toggled with the SYSTEM$ENABLE_PREVIEW_ACCESS and + SYSTEM$DISABLE_PREVIEW_ACCESS functions rather than with an account parameter, so it is + not something Snowcap can manage as a resource -- but knowing the answer turns + "GRANT INHERITED failed" into an actionable message. + + Returns None when the status cannot be read. + https://docs.snowflake.com/en/release-notes/preview-features + """ + try: + rows = execute(session, "SELECT SYSTEM$GET_PREVIEW_ACCESS_STATUS() AS status", cacheable=True) + except Exception as err: + logger.debug(f"Could not read preview access status: {err}") + return None + + if not rows: + return None + status = str(rows[0].get("STATUS") or rows[0].get("status") or "").upper() + if "ENABLED" in status: + return True + if "DISABLED" in status: + return False + return None + + def fetch_session(session: SnowflakeConnection) -> SessionContext: session_obj = execute( session, @@ -843,7 +1155,7 @@ def fetch_role_privileges( name=grant["name"], ) role_privileges[role_match].append(granted_priv) - # If snowcap isnt aware of the privilege, ignore it + # If snowcap isn't aware of the privilege, ignore it except ValueError: continue @@ -862,7 +1174,7 @@ def fetch_role_privileges( name=grant["name"], ) role_privileges[role].append(granted_priv) - # If snowcap isnt aware of the privilege, ignore it + # If snowcap isn't aware of the privilege, ignore it except ValueError: continue return role_privileges @@ -888,6 +1200,14 @@ def fetch_role_privileges( # Stores the normalized grant list from GRANTS_TO_USERS _ACCOUNT_USAGE_USER_GRANTS_CACHE: dict[int, list[dict[str, Any]]] = {} +# Tracks sessions whose GRANTS_TO_ROLES view has no IS_INHERITED column (keyed by session +# id). Absent means "assume the column exists"; the first query proves it either way. +_ACCOUNT_USAGE_INHERITED_COLUMN: dict[int, bool] = {} + +# Whether FEATURE_RBAC_INHERITED_GRANTS is enabled (keyed by session id). None means the +# parameter could not be read. +_INHERITED_GRANTS_ENABLED_CACHE: dict[int, Optional[bool]] = {} + def reset_account_usage_caches() -> None: """ @@ -898,11 +1218,14 @@ def reset_account_usage_caches() -> None: """ global _ACCOUNT_USAGE_ACCESS_CACHE, _ACCOUNT_USAGE_FALLBACK_CACHE global _ACCOUNT_USAGE_GRANTS_CACHE, _ACCOUNT_USAGE_USER_GRANTS_CACHE + global _ACCOUNT_USAGE_INHERITED_COLUMN, _INHERITED_GRANTS_ENABLED_CACHE _ACCOUNT_USAGE_ACCESS_CACHE.clear() _ACCOUNT_USAGE_FALLBACK_CACHE.clear() _ACCOUNT_USAGE_GRANTS_CACHE.clear() _ACCOUNT_USAGE_USER_GRANTS_CACHE.clear() + _ACCOUNT_USAGE_INHERITED_COLUMN.clear() + _INHERITED_GRANTS_ENABLED_CACHE.clear() def _mark_account_usage_fallback(session: SnowflakeConnection) -> None: @@ -1005,6 +1328,44 @@ def _has_account_usage_access(session: SnowflakeConnection) -> bool: # ------------------------------ +def _grants_to_roles_query(include_inherited: bool = True) -> str: + """ + Build the GRANTS_TO_ROLES query. + + The inherited-grant columns are selected so container-level grants can be told apart + from object grants, and so an inherited grant can be matched back to the container it + was created on. They are omitted for accounts whose view does not expose them, which + are accounts that cannot have inherited grants anyway. + """ + columns = [ + "CREATED_ON", + "PRIVILEGE", + "GRANTED_ON", + "NAME", + "TABLE_CATALOG", + "TABLE_SCHEMA", + "GRANTED_TO", + "GRANTEE_NAME", + "GRANT_OPTION", + "GRANTED_BY", + ] + if include_inherited: + columns.extend( + [ + "IS_INHERITED", + "INHERITED_FROM", + "INHERITED_FROM_DATABASE", + "INHERITED_FROM_SCHEMA", + ] + ) + return f""" + SELECT + {', '.join(columns)} + FROM SNOWFLAKE.ACCOUNT_USAGE.GRANTS_TO_ROLES + WHERE DELETED_ON IS NULL + """ + + def _fetch_grants_from_account_usage(session: SnowflakeConnection) -> list[dict[str, Any]] | None: """ Fetch all role grants from SNOWFLAKE.ACCOUNT_USAGE.GRANTS_TO_ROLES in a single query. @@ -1033,32 +1394,32 @@ def _fetch_grants_from_account_usage(session: SnowflakeConnection) -> list[dict[ if session_id in _ACCOUNT_USAGE_GRANTS_CACHE: return _ACCOUNT_USAGE_GRANTS_CACHE[session_id] - query = """ - SELECT - CREATED_ON, - PRIVILEGE, - GRANTED_ON, - NAME, - TABLE_CATALOG, - TABLE_SCHEMA, - GRANTED_TO, - GRANTEE_NAME, - GRANT_OPTION, - GRANTED_BY - FROM SNOWFLAKE.ACCOUNT_USAGE.GRANTS_TO_ROLES - WHERE DELETED_ON IS NULL - """ + has_inherited_column = _ACCOUNT_USAGE_INHERITED_COLUMN.get(session_id, True) try: - results = execute(session, query, cacheable=True) + results = execute(session, _grants_to_roles_query(include_inherited=has_inherited_column), cacheable=True) except ProgrammingError as err: - if err.errno == ACCESS_CONTROL_ERR: + if has_inherited_column and err.errno in (INVALID_COLUMN_ERR, INVALID_IDENTIFIER): + # Not every Snowflake version exposes the inherited-grant columns, so an account + # whose GRANTS_TO_ROLES view lacks them is queried without them. Such an account + # cannot have inherited grants in the first place. + logger.debug("GRANTS_TO_ROLES has no IS_INHERITED column, querying without it") + _ACCOUNT_USAGE_INHERITED_COLUMN[session_id] = False + try: + results = execute(session, _grants_to_roles_query(include_inherited=False), cacheable=True) + except Exception as retry_err: + logger.warning(f"ACCOUNT_USAGE query failed unexpectedly: {retry_err} - falling back to SHOW queries") + _mark_account_usage_fallback(session) + return None + elif err.errno == ACCESS_CONTROL_ERR: logger.warning("ACCOUNT_USAGE query failed: access denied - falling back to SHOW queries") + _mark_account_usage_fallback(session) + return None else: logger.warning( f"ACCOUNT_USAGE query failed with error {err.errno}: {err.msg} - falling back to SHOW queries" ) - _mark_account_usage_fallback(session) - return None + _mark_account_usage_fallback(session) + return None except Exception as err: logger.warning(f"ACCOUNT_USAGE query failed unexpectedly: {err} - falling back to SHOW queries") _mark_account_usage_fallback(session) @@ -1114,6 +1475,12 @@ def _fetch_grants_from_account_usage(session: SnowflakeConnection) -> list[dict[ "grantee_name": row["GRANTEE_NAME"], "grant_option": grant_option, "granted_by": row["GRANTED_BY"], + # Inherited grants describe a container, not a securable. NAME is empty for + # them, and the container comes from these columns instead. + "is_inherited": bool(row.get("IS_INHERITED")), + "inherited_from": row.get("INHERITED_FROM") or "", + "inherited_from_database": row.get("INHERITED_FROM_DATABASE") or "", + "inherited_from_schema": row.get("INHERITED_FROM_SCHEMA") or "", } ) @@ -1947,6 +2314,73 @@ def fetch_function(session: SnowflakeConnection, fqn: FQN): } +def _parse_grant_collection(on_type: str, on: str) -> dict[str, str]: + """ + Split a collection grant's encoded target into container and item type. + + The account container has no name, so it is encoded as "ACCOUNT.
" and cannot be + told apart from a database called ACCOUNT by looking at the string alone. The container + type travels alongside it in the URN, so it is passed in rather than inferred. + """ + if on_type == ResourceType.ACCOUNT.value: + _, _, items_type = on.partition(".") + return {"on": "ACCOUNT", "on_type": "account", "items_type": items_type.strip("<>")} + return parse_collection_string(on) + + +def _inherited_grant_matches(grant: dict[str, Any], items_type: str, container_type: str, container: str) -> bool: + """Does an inherited grant row describe a grant on this container and object type?""" + if grant["granted_on"].replace("_", " ").upper() != items_type.replace("_", " ").upper(): + return False + inherited_from = str(grant.get("inherited_from") or "").upper() + if inherited_from != container_type.upper(): + return False + if container_type == "ACCOUNT": + return True + database = str(grant.get("inherited_from_database") or "") + if container_type == "DATABASE": + return ResourceName(database) == ResourceName(container) + schema = str(grant.get("inherited_from_schema") or "") + return ResourceName(f"{database}.{schema}") == ResourceName(container) + + +def fetch_inherited_grant(session: SnowflakeConnection, fqn: FQN): + """ + Fetch a single inherited grant. + + Unlike ON ALL grants, an inherited grant is one durable record that Snowflake reports + back, so it can be compared against config instead of being reapplied on every run. + """ + priv = fqn.params["priv"] + on_type, on = fqn.params["on"].split("/", 1) + to_type, to = fqn.params["to"].split("/", 1) + to_type = resource_type_for_label(to_type) + + collection = _parse_grant_collection(on_type.upper(), on) + container_type = collection["on_type"].upper() + container = collection["on"] + items_type = collection["items_type"] + + for grant in _show_inherited_grants_to_role(session, to, role_type=to_type): + if grant["privilege"] != priv: + continue + if not _inherited_grant_matches(grant, items_type, container_type, container): + continue + return { + "priv": priv, + "on": container, + "on_type": container_type.replace("_", " "), + "to": to, + "to_type": resource_type_for_label(grant["granted_to"]), + "grant_option": False, + "owner": grant.get("granted_by") or "", + "_privs": [priv], + "items_type": items_type.replace("_", " "), + "grant_type": GrantType.INHERITED, + } + return None + + def fetch_grant(session: SnowflakeConnection, fqn: FQN): priv = fqn.params["priv"] on_type, on = fqn.params["on"].split("/", 1) @@ -1956,6 +2390,9 @@ def fetch_grant(session: SnowflakeConnection, fqn: FQN): # Default to OBJECT grant type if not specified grant_type = fqn.params.get("grant_type", GrantType.OBJECT) + if grant_type == GrantType.INHERITED: + return fetch_inherited_grant(session, fqn) + if priv == "ALL": filters = { "granted_on": on_type, @@ -1988,8 +2425,12 @@ def fetch_grant(session: SnowflakeConnection, fqn: FQN): # Gate on the database actually being shared: without this, a mistakenly-declared # IMPORTED PRIVILEGES grant on a regular database would false-match its (very common) # plain USAGE grant and mask the config error. + # Any kind but STANDARD is share-backed: IMPORTED DATABASE for a marketplace or + # direct share, APPLICATION for the SNOWFLAKE database, which behaves the same + # way and was missed by testing for IMPORTED DATABASE alone. A STANDARD database + # is the only case where a plain USAGE grant could be mistaken for this one. db_rows = _show_resources(session, "DATABASES", FQN(name=ResourceName(on))) - if db_rows and db_rows[0]["kind"] == "IMPORTED DATABASE": + if db_rows and db_rows[0]["kind"] != "STANDARD": data = _fetch_grant_to_role( session, grant_type=grant_type, @@ -2375,7 +2816,7 @@ def fetch_pipe(session: SnowflakeConnection, fqn: FQN): def fetch_procedure(session: SnowflakeConnection, fqn: FQN): # SHOW PROCEDURES IN SCHEMA {}.{} - # FIXME: This will fail if the database doesnt exist + # FIXME: This will fail if the database doesn't exist show_result = execute(session, f"SHOW PROCEDURES IN SCHEMA {fqn.database}.{fqn.schema}", cacheable=True) sprocs = _filter_result(show_result, name=fqn.name) if len(sprocs) == 0: @@ -3337,9 +3778,7 @@ def fetch_warehouse(session: SnowflakeConnection, fqn: FQN, include_params: bool if generation is not None: generation = str(generation) resource_constraint = _normalize_snowflake_optional(data.get("resource_constraint"), upper=True) - max_query_performance_level = _normalize_snowflake_optional( - data.get("max_query_performance_level"), upper=True - ) + max_query_performance_level = _normalize_snowflake_optional(data.get("max_query_performance_level"), upper=True) query_throughput_multiplier = _normalize_snowflake_optional(data.get("query_throughput_multiplier")) if warehouse_type == "STANDARD": @@ -3493,6 +3932,36 @@ def list_databases(session: SnowflakeConnection) -> list[FQN]: return [FQN(name=database) for database in databases] +def list_database_owners(session: SnowflakeConnection) -> dict[str, str]: + """ + Owner role of every database in the account, keyed by upper-cased name. + + Used to manage grants held by database roles, which live inside a database and cannot + be reached with account-level authority alone. Reads the same cached SHOW DATABASES + response _list_databases uses, so this costs no extra query. + """ + show_result = execute(session, "SHOW DATABASES", cacheable=True) + return {row["name"].upper(): row["owner"] for row in show_result if row.get("owner")} + + +def list_shared_database_names(session: SnowflakeConnection) -> set[str]: + """ + Names of databases whose privileges come from a share rather than from grants on the + database itself, upper-cased. + + Every kind but STANDARD qualifies. IMPORTED DATABASE is the marketplace or direct + share; APPLICATION covers the SNOWFLAKE database, which behaves the same way and would + be missed by testing for IMPORTED DATABASE alone. + + Two things depend on this. Privileges on them are granted with IMPORTED PRIVILEGES and + reported back as USAGE, and they cannot be revoked one at a time; see + lifecycle.drop_shared_database_grant. Reads the same cached SHOW DATABASES response + _list_databases uses, so this costs no extra query. + """ + show_result = execute(session, "SHOW DATABASES", cacheable=True) + return {row["name"].upper() for row in show_result if row["kind"] != "STANDARD"} + + def list_database_roles(session: SnowflakeConnection, database=None) -> list[FQN]: databases: list[ResourceName] if database: @@ -3683,6 +4152,10 @@ def list_grants( ) -> list[FQN]: grants: list[FQN] = [] + # Databases whose privileges come from a share. Grants on them are declared as + # IMPORTED PRIVILEGES and reported back as USAGE; see _imported_privileges_priv. + shared_databases = list_shared_database_names(session) + # Get all non-system role names for processing # Use "SHOW ROLES IN ACCOUNT" to match _show_resources for cache consistency roles_result = execute(session, "SHOW ROLES IN ACCOUNT", cacheable=True) @@ -3744,29 +4217,42 @@ def get_database_role_name_set() -> set[str]: # Skip other grantee types (e.g., USER) continue - # Skip role grants (hierarchy handled by list_role_grants) - if data["granted_on"] == "ROLE": + # Skip role and database role grants (hierarchy is handled by + # list_role_grants and list_database_role_grants) + if _is_role_hierarchy_grant(data): continue # Snowcap Grants don't support OWNERSHIP privilege if data["privilege"] == "OWNERSHIP": continue + # A database role is born holding usage on its own database + if to_prefix == "database_role" and _is_intrinsic_database_role_usage(data, grantee): + continue + # Skip undocumented privs if data["privilege"] in ["CANCEL QUERY"]: continue + # Inherited grants are container-level and carry no object name + if _is_inherited_grant(data): + inherited_fqn = inherited_grant_fqn(data, to_prefix, grantee) + if inherited_fqn: + grants.append(inherited_fqn) + continue + name = data["name"] if data["granted_on"] == "ACCOUNT": name = "ACCOUNT" - on = f"{data['granted_on'].lower()}/{name}" + on = f"{_granted_on_label(data['granted_on'])}/{name}" + priv = _imported_privileges_priv(data, shared_databases) or data["privilege"] to = f"{to_prefix}/{grantee}" grants.append( FQN( name=ResourceName("GRANT"), params={ "grant_type": "OBJECT", - "priv": data["privilege"], + "priv": priv, "on": on, "to": to, }, @@ -3782,7 +4268,9 @@ def get_database_role_name_set() -> set[str]: session, role_name, role_type=ResourceType.ROLE, cacheable=True, use_account_usage=False ) for data in grant_data: - if data["granted_on"] == "ROLE": + # Skip role and database role grants (hierarchy is handled by + # list_role_grants and list_database_role_grants) + if _is_role_hierarchy_grant(data): continue # Snowcap Grants don't support OWNERSHIP privilege @@ -3796,20 +4284,29 @@ def get_database_role_name_set() -> set[str]: name = data["name"] if data["granted_on"] == "ACCOUNT": name = "ACCOUNT" - on = f"{data['granted_on'].lower()}/{name}" + on = f"{_granted_on_label(data['granted_on'])}/{name}" + priv = _imported_privileges_priv(data, shared_databases) or data["privilege"] to = f"role/{role_name}" grants.append( FQN( name=ResourceName("GRANT"), params={ "grant_type": "OBJECT", - "priv": data["privilege"], + "priv": priv, "on": on, "to": to, }, ) ) + # Inherited grants come from the same (cached) SHOW GRANTS response, so listing them + # costs no extra queries. + for role_name in role_names: + for data in _show_inherited_grants_to_role(session, role_name, role_type=ResourceType.ROLE): + inherited_fqn = inherited_grant_fqn(data, "role", str(role_name)) + if inherited_fqn: + grants.append(inherited_fqn) + # Also fetch grants for database roles using SHOW GRANTS TO DATABASE ROLE for db_role_fqn in get_database_roles(): fq_db_role_name = f"{db_role_fqn.database}.{db_role_fqn.name}" @@ -3821,14 +4318,19 @@ def get_database_role_name_set() -> set[str]: use_account_usage=False, ) for data in grant_data: - # Skip database role grants (hierarchy handled by list_database_role_grants) - if data["granted_on"] == "DATABASE ROLE": + # Skip role and database role grants (hierarchy is handled by + # list_role_grants and list_database_role_grants) + if _is_role_hierarchy_grant(data): continue # Snowcap Grants don't support OWNERSHIP privilege if data["privilege"] == "OWNERSHIP": continue + # A database role is born holding usage on its own database + if _is_intrinsic_database_role_usage(data, fq_db_role_name): + continue + # Skip undocumented privs if data["privilege"] in ["CANCEL QUERY"]: continue @@ -3836,20 +4338,28 @@ def get_database_role_name_set() -> set[str]: name = data["name"] if data["granted_on"] == "ACCOUNT": name = "ACCOUNT" - on = f"{data['granted_on'].lower()}/{name}" + on = f"{_granted_on_label(data['granted_on'])}/{name}" + priv = _imported_privileges_priv(data, shared_databases) or data["privilege"] to = f"database_role/{fq_db_role_name}" grants.append( FQN( name=ResourceName("GRANT"), params={ "grant_type": "OBJECT", - "priv": data["privilege"], + "priv": priv, "on": on, "to": to, }, ) ) + for data in _show_inherited_grants_to_role( + session, ResourceName(fq_db_role_name), role_type=ResourceType.DATABASE_ROLE + ): + inherited_fqn = inherited_grant_fqn(data, "database_role", fq_db_role_name) + if inherited_fqn: + grants.append(inherited_fqn) + # Future grants always use SHOW commands (not available in ACCOUNT_USAGE) # Only fetch if include_future_grants is True (manifest has future grants) if include_future_grants: diff --git a/snowcap/enums.py b/snowcap/enums.py index 2e56cf10..3f0cb7b2 100644 --- a/snowcap/enums.py +++ b/snowcap/enums.py @@ -114,6 +114,14 @@ class ResourceType(ParseableEnum): VIEW = "VIEW" WAREHOUSE = "WAREHOUSE" + @classmethod + def synonyms(cls): + # Snowflake reports grants on an MCP server with granted_on + # 'CORTEX_AGENT_SERVER', but the DDL grammar only accepts MCP SERVER -- + # GRANT ... ON CORTEX AGENT SERVER is a syntax error. Same object, two + # names, so map the grant-side spelling onto the DDL one. + return {"CORTEX AGENT SERVER": "MCP_SERVER"} + class Scope(ParseableEnum): ORGANIZATION = "ORGANIZATION" @@ -388,6 +396,42 @@ def resource_type_is_grant(resource_type: ResourceType) -> bool: ) +# Object types whose body or schedule runs with the privileges of their owner rather than +# the caller. Snowflake requires stricter authorization to transfer ownership of these: +# the receiving role must be in the caller's active role hierarchy, or the caller must hold +# account-level MANAGE GRANTS. +# https://docs.snowflake.com/en/user-guide/inherited-grants-intro +OWNER_EXECUTED_RESOURCE_TYPES = frozenset( + { + # Queryable objects + ResourceType.VIEW, + ResourceType.MATERIALIZED_VIEW, + ResourceType.DYNAMIC_TABLE, + ResourceType.SEMANTIC_VIEW, + ResourceType.EXTERNAL_TABLE, + ResourceType.DIRECTORY_TABLE, + ResourceType.EVENT_TABLE, + # Procedures and functions + ResourceType.PROCEDURE, + ResourceType.FUNCTION, + # Schedulers and triggers + ResourceType.TASK, + ResourceType.ALERT, + # Ingest + ResourceType.PIPE, + # Function-based policies + ResourceType.MASKING_POLICY, + ResourceType.ROW_ACCESS_POLICY, + ResourceType.AGGREGATION_POLICY, + # File-based code + ResourceType.STREAMLIT, + # Container services and composites + ResourceType.SERVICE, + ResourceType.CORTEX_SEARCH_SERVICE, + } +) + + class EncryptionType(ParseableEnum): SNOWFLAKE_FULL = "SNOWFLAKE_FULL" SNOWFLAKE_SSE = "SNOWFLAKE_SSE" @@ -403,6 +447,33 @@ class GrantType(ParseableEnum): OBJECT = "OBJECT" FUTURE = "FUTURE" ALL = "ALL" + # A single container-level grant covering every current and future object of a type in + # an ACCOUNT, DATABASE, or SCHEMA. Replaces an ALL + FUTURE pair with one grant record. + # https://docs.snowflake.com/en/user-guide/inherited-grants-intro + INHERITED = "INHERITED" + + +# The account parameter that opts an account into the inherited grants preview. +INHERITED_GRANTS_FEATURE_FLAG = "FEATURE_RBAC_INHERITED_GRANTS" + + +# USAGE on a ROLE or USER cannot be the target of an inherited grant; Snowflake rejects it. +# https://docs.snowflake.com/en/user-guide/inherited-grants-intro +NON_INHERITABLE_USAGE_TARGETS = frozenset({ResourceType.ROLE, ResourceType.USER}) + +# Object types that cannot be the target of an inherited grant. +NON_INHERITABLE_RESOURCE_TYPES = frozenset( + { + ResourceType.SHARE, + ResourceType.INTEGRATION, + ResourceType.API_INTEGRATION, + ResourceType.CATALOG_INTEGRATION, + ResourceType.EXTERNAL_ACCESS_INTEGRATION, + ResourceType.NOTIFICATION_INTEGRATION, + ResourceType.SECURITY_INTEGRATION, + ResourceType.STORAGE_INTEGRATION, + } +) class TagPropagation(ParseableEnum): diff --git a/snowcap/error_formatting.py b/snowcap/error_formatting.py index 36a3d5ff..8b129809 100644 --- a/snowcap/error_formatting.py +++ b/snowcap/error_formatting.py @@ -238,7 +238,7 @@ def format_invalid_key_error( if resource_name: msg = f'Invalid keys {keys_str} in {resource_type} "{resource_name}".' else: - msg = f'Invalid keys {keys_str} in {resource_type}.' + msg = f"Invalid keys {keys_str} in {resource_type}." for key, suggestion in suggestions.items(): msg += f'\n "{key}" -> Did you mean: {suggestion}?' diff --git a/snowcap/gitops.py b/snowcap/gitops.py index c87c40ee..783ce362 100644 --- a/snowcap/gitops.py +++ b/snowcap/gitops.py @@ -14,7 +14,7 @@ from .identifiers import resource_label_for_type, resource_type_for_label from .resources import DatabaseRoleGrant, Resource, RoleGrant from .resources.resource import ResourcePointer -from .var import process_for_each, string_contains_var +from .var import evaluate_for_each_where, process_for_each, string_contains_var logger = logging.getLogger("snowcap") @@ -36,6 +36,30 @@ def construct_string_on_off(loader, node): VALID_ROLE_GRANT_KEYS = {"role", "roles", "to_user", "to_users", "to_role", "to_roles"} +# `roles` is the long-standing plural of `to_role` here, kept for compatibility; +# `database_roles` is the matching plural of `to_database_role`. +VALID_DATABASE_ROLE_GRANT_KEYS = {"database_role", "to_role", "roles", "to_database_role", "database_roles"} + + +def _as_list(config: dict, singular: str, plural: str) -> list: + """ + Values given under either the singular or plural spelling of a key. + + A key present but null counts as absent. `to_role:` with nothing after it is how YAML + spells "not specified", and serialized configs round-trip unset fields as explicit + nulls, so testing for the key rather than the value would read those as a request to + grant to nothing. + """ + values = [] + # Null counts as absent (see above); so does an empty string, which is what a bad template + # render leaves behind -- appending it would build a grant to an empty target name. Both + # callers pass role-name keys, where "" is never a legitimate value. The same applies to a + # plural-list element (a bad render of one item in the list). + if config.get(singular) not in (None, ""): + values.append(config[singular]) + values.extend(v for v in (config.get(plural) or []) if v not in (None, "")) + return values + def _validate_role_grant_structure(role_grant: dict) -> None: """Validate that role_grant has a valid key combination.""" @@ -68,9 +92,7 @@ def _validate_role_grant_structure(role_grant: dict) -> None: " - finance_team" ) if has_to_roles: - raise ValueError( - 'Cannot use "to_roles" with "roles". Use "to_role" (singular) instead.' - ) + raise ValueError('Cannot use "to_roles" with "roles". Use "to_role" (singular) instead.') def _resources_from_role_grants_config(role_grants_config: list) -> list: @@ -145,25 +167,41 @@ def _resources_from_role_grants_config(role_grants_config: list) -> list: def _resources_from_database_role_grants_config(database_role_grants_config: list) -> list: + """ + Build DatabaseRoleGrants from the `database_role_grants` block. + + A database role can be granted to an account role or to another database role; + DatabaseRoleGrant and the SQL either side of it have always handled both. Only this + loader did not, so nesting one database role inside another was expressible in Python + and not in config -- and an entry that asked for it produced no resource at all rather + than an error, so the grant simply never appeared in the plan. + """ if len(database_role_grants_config) == 0: return [] resources = [] for database_role_grant in database_role_grants_config: - if "to_role" in database_role_grant: - resources.append( - DatabaseRoleGrant( - database_role=database_role_grant["database_role"], - to_role=database_role_grant["to_role"], - ) + invalid_keys = set(database_role_grant.keys()) - VALID_DATABASE_ROLE_GRANT_KEYS + if invalid_keys: + raise ValueError(format_invalid_role_grant_keys(invalid_keys, VALID_DATABASE_ROLE_GRANT_KEYS)) + + if "database_role" not in database_role_grant: + raise ValueError('database_role_grant must specify "database_role"') + + granted = database_role_grant["database_role"] + targets = [(to_role, "to_role") for to_role in _as_list(database_role_grant, "to_role", "roles")] + targets += [ + (to_database_role, "to_database_role") + for to_database_role in _as_list(database_role_grant, "to_database_role", "database_roles") + ] + + if not targets: + raise ValueError( + f'database_role_grant for "{granted}" grants it to nothing. Specify one of ' + f"{', '.join(sorted(VALID_DATABASE_ROLE_GRANT_KEYS - {'database_role'}))}." ) - else: - for role in database_role_grant.get("roles", []): - resources.append( - DatabaseRoleGrant( - database_role=database_role_grant["database_role"], - to_role=role, - ) - ) + + for target, keyword in targets: + resources.append(DatabaseRoleGrant(database_role=granted, **{keyword: target})) return resources @@ -199,6 +237,7 @@ def _resources_for_config(config: dict, vars: dict): resource_cls = Resource.resolve_resource_cls(resource_type, resource_data) resource_instance = resource_data.copy() for_each = resource_instance.pop("for_each") + for_each_where = resource_instance.pop("where", None) if isinstance(for_each, str) and for_each.startswith("var."): var_name = for_each.split(".")[1] @@ -210,7 +249,16 @@ def _resources_for_config(config: dict, vars: dict): for each_value in for_each_input: try: + # Inside the try so a bad `where` (typo, unsupported var + # reference) is collected as this item's validation error + # instead of aborting the entire for_each block. + if for_each_where is not None and not evaluate_for_each_where( + for_each_where, each_value + ): + continue for key, value in resource_data.items(): + if key in ("for_each", "where"): + continue if isinstance(value, str) and string_contains_var(value): key_type = getattr(resource_cls.spec, key, None) resource_instance[key] = process_for_each(value, each_value) diff --git a/snowcap/lifecycle.py b/snowcap/lifecycle.py index 0aed2c3c..b4b8f531 100644 --- a/snowcap/lifecycle.py +++ b/snowcap/lifecycle.py @@ -150,12 +150,35 @@ def create_hybrid_table(urn: URN, data: dict, props: Props, if_not_exists: bool ) +def _grant_container_sql(data: dict) -> str: + """Render the `IN ` clause of a collection grant. + + The account container has no name of its own, so it renders as a bare `IN ACCOUNT`. + """ + if data["on_type"] == ResourceType.ACCOUNT: + return "IN ACCOUNT" + return f"IN {data['on_type']} {data['on']}" + + def create_grant(urn: URN, data: dict, props: Props, if_not_exists: bool): on_type = data["on_type"] if "INTEGRATION" in str(on_type): on_type = "INTEGRATION" elif on_type == "ACCOUNT": on_type = "" + if data["grant_type"] == GrantType.INHERITED: + # A single container-level grant covering current and future objects. Snowflake + # rejects WITH GRANT OPTION here, which the Grant resource validates up front. + return tidy_sql( + "GRANT INHERITED", + data["priv"], + "ON ALL", + pluralize(data["items_type"]).upper(), + _grant_container_sql(data), + "TO", + data["to_type"], + data["to"], + ) if data["grant_type"] == GrantType.FUTURE: items_type = data["items_type"] if "INTEGRATION" in items_type: @@ -581,6 +604,33 @@ def drop_database_role_grant(urn: URN, data: dict, **kwargs): ) +def drop_shared_database_grant(data: dict, database: str) -> str: + """ + Revoke a grant that a share handed out, given any one row of its fan-out. + + Privileges on a shared database are not independently revocable. Snowflake grants them + with one statement, GRANT IMPORTED PRIVILEGES ON DATABASE , then reports them in + SHOW GRANTS as a row per object the share exposes -- USAGE on the database, USAGE on + each schema, SELECT on each view, and so on. Revoking any of those rows on its own is + rejected: + + Revoking individual privileges on imported database is not allowed. + Use 'REVOKE IMPORTED PRIVILEGES' + + The share is the only source of privileges on those objects, so revoking IMPORTED + PRIVILEGES removes the whole fan-out for that grantee in one statement. Every row of the + fan-out therefore maps to the same revoke, which is idempotent: the first one takes the + access away and any repeat finds nothing left to revoke. + """ + return tidy_sql( + "REVOKE IMPORTED PRIVILEGES ON DATABASE", + ResourceName(database), + "FROM", + data["to_type"], + data["to"], + ) + + def drop_function(urn: URN, data: dict, if_exists: bool) -> str: return tidy_sql( "DROP", @@ -593,6 +643,17 @@ def drop_function(urn: URN, data: dict, if_exists: bool) -> str: def drop_grant(urn: URN, data: dict, **kwargs): if data["priv"] == "OWNERSHIP": raise NotImplementedError + if data["grant_type"] == GrantType.INHERITED: + return tidy_sql( + "REVOKE INHERITED", + data["priv"], + "ON ALL", + pluralize(data["items_type"]).upper(), + _grant_container_sql(data), + "FROM", + data["to_type"], + data["to"], + ) if data["grant_type"] == GrantType.FUTURE: return tidy_sql( "REVOKE", diff --git a/snowcap/operations/blueprint.py b/snowcap/operations/blueprint.py index eabe7058..dd6a3448 100644 --- a/snowcap/operations/blueprint.py +++ b/snowcap/operations/blueprint.py @@ -1,7 +1,7 @@ from typing import Any from snowcap.blueprint import Blueprint -from snowcap.blueprint import plan_from_dict +from snowcap.blueprint import plan_from_dict, levels_from_plan_dict from snowcap.blueprint_config import BlueprintConfig from snowcap.gitops import collect_blueprint_config @@ -13,7 +13,7 @@ def blueprint_plan(yaml_config: dict, cli_config: dict[str, Any]): blueprint = Blueprint.from_config(blueprint_config) session = connect() plan_obj = blueprint.plan(session) - return plan_obj + return plan_obj, blueprint._levels def blueprint_apply(yaml_config: dict, cli_config: dict): @@ -27,5 +27,8 @@ def blueprint_apply_plan(plan_dict: dict, cli_config: dict): blueprint_config = BlueprintConfig(**cli_config) blueprint = Blueprint.from_config(blueprint_config) plan = plan_from_dict(plan_dict) + # Restore the dependency levels the plan was saved with so ordering is preserved; without + # this the apply-plan path runs every change at level 0. + blueprint._levels = levels_from_plan_dict(plan_dict) session = connect() blueprint.apply(session, plan) diff --git a/snowcap/operations/export.py b/snowcap/operations/export.py index f54d936d..2af18e97 100644 --- a/snowcap/operations/export.py +++ b/snowcap/operations/export.py @@ -72,9 +72,7 @@ def _fetch_resource_safe(session, urn: URN): return None -def export_resource( - session, resource_type: ResourceType, threads: int = DEFAULT_EXPORT_THREADS -) -> dict[str, list]: +def export_resource(session, resource_type: ResourceType, threads: int = DEFAULT_EXPORT_THREADS) -> dict[str, list]: resource_label = resource_label_for_type(resource_type) resource_names = list_resource(session, resource_label) if len(resource_names) == 0: @@ -105,9 +103,7 @@ def export_resource( # Fetch resources in parallel resources = [] with ThreadPoolExecutor(max_workers=threads) as executor: - future_to_urn = { - executor.submit(_fetch_resource_safe, session, urn): urn for urn in urns - } + future_to_urn = {executor.submit(_fetch_resource_safe, session, urn): urn for urn in urns} for future in as_completed(future_to_urn): urn = future_to_urn[future] try: diff --git a/snowcap/parse.py b/snowcap/parse.py index 839426d3..57a6eb7b 100644 --- a/snowcap/parse.py +++ b/snowcap/parse.py @@ -215,17 +215,28 @@ def _parse_priv_grant(sql: str): results = grant.parse_string(sql, parse_all=True) results = results.as_dict() - privs = [priv.strip(" ") for priv in results["privs"].split(",")] + privs_text = results["privs"].strip() + # GRANT INHERITED ON ALL IN : the keyword sits between + # GRANT and the privilege, so it is stripped here and passed along as a flag. + inherited = False + if privs_text.upper().startswith("INHERITED "): + inherited = True + privs_text = privs_text[len("INHERITED ") :].strip() + + privs = [priv.strip(" ") for priv in privs_text.split(",")] if len(privs) > 1: raise NotImplementedError("Multi-priv grants are not supported") on_stmt = results.pop("on_stmt").strip() - return { + parsed = { "priv": privs[0].upper(), "on": on_stmt, "to": results["to"], } + if inherited: + parsed["inherited"] = True + return parsed except pp.ParseException as err: raise pp.ParseException("Failed to parse grant") from err diff --git a/snowcap/privs.py b/snowcap/privs.py index 0afe626d..3d35b8f2 100644 --- a/snowcap/privs.py +++ b/snowcap/privs.py @@ -55,6 +55,7 @@ class AccountPriv(Priv): CREATE_FAILOVER_GROUP = "CREATE FAILOVER GROUP" CREATE_INTEGRATION = "CREATE INTEGRATION" CREATE_NETWORK_POLICY = "CREATE NETWORK POLICY" + CREATE_OPENFLOW_DATA_PLANE_INTEGRATION = "CREATE OPENFLOW DATA PLANE INTEGRATION" CREATE_REPLICATION_GROUP = "CREATE REPLICATION GROUP" CREATE_ROLE = "CREATE ROLE" CREATE_SHARE = "CREATE SHARE" @@ -570,6 +571,7 @@ class WarehousePriv(Priv): AccountPriv.CREATE_FAILOVER_GROUP: "ACCOUNTADMIN", AccountPriv.CREATE_INTEGRATION: "ACCOUNTADMIN", AccountPriv.CREATE_NETWORK_POLICY: "SECURITYADMIN", + AccountPriv.CREATE_OPENFLOW_DATA_PLANE_INTEGRATION: "ACCOUNTADMIN", AccountPriv.CREATE_REPLICATION_GROUP: "ACCOUNTADMIN", AccountPriv.CREATE_ROLE: "USERADMIN", AccountPriv.CREATE_SHARE: "ACCOUNTADMIN", diff --git a/snowcap/resources/grant.py b/snowcap/resources/grant.py index 84175bbe..ef8bafc0 100644 --- a/snowcap/resources/grant.py +++ b/snowcap/resources/grant.py @@ -2,9 +2,15 @@ from dataclasses import dataclass, field from typing import Any, Union -from inflection import singularize - -from ..enums import GrantType, ParseableEnum, ResourceType +from inflection import pluralize, singularize + +from ..enums import ( + NON_INHERITABLE_RESOURCE_TYPES, + NON_INHERITABLE_USAGE_TARGETS, + GrantType, + ParseableEnum, + ResourceType, +) from ..identifiers import ( FQN, parse_FQN, @@ -24,6 +30,72 @@ logger = logging.getLogger("snowcap") +INHERITED_GRANT_DOCS = "https://docs.snowflake.com/en/user-guide/inherited-grants-intro" + +# Keywords that introduce a grant on a collection of objects in a container, as opposed to +# a grant on one named object. +COLLECTION_GRANT_KEYWORDS = (GrantType.FUTURE, GrantType.ALL, GrantType.INHERITED) + + +def _is_account_container(on_items: list) -> bool: + """ + True for ` IN ACCOUNT`, where the container is the account itself. + + A database or schema may legitimately be named ACCOUNT, so the trailing ACCOUNT only + means the account container when it is not preceded by a container type keyword: + ["INHERITED", "TABLES", "ACCOUNT"] is the account, while + ["ALL", "TABLES", "DATABASE", "ACCOUNT"] is a database that happens to be called that. + """ + last = on_items[-1] + if not isinstance(last, str) or last.upper() != "ACCOUNT": + return False + if len(on_items) < 3: + return False + preceding = on_items[-2] + return not ( + isinstance(preceding, str) and preceding.upper() in (ResourceType.DATABASE.value, ResourceType.SCHEMA.value) + ) + + +def _validate_inherited_grant(grant: "_Grant") -> None: + """ + Reject inherited grants that Snowflake will not accept. + + These are checked here rather than left to Snowflake so the failure surfaces at plan + time, against the line of config that caused it, instead of mid-apply. The list is not + exhaustive: Snowflake also rejects privileges whose only target is the account, and + inherited grants on shared databases, which Snowcap cannot always determine locally. + """ + if grant.items_type is None: + raise ValueError( + "Inherited grants target a collection of objects in a container, " + "e.g. on='INHERITED TABLES IN SCHEMA somedb.someschema'" + ) + if grant.grant_option: + raise ValueError(f"Inherited grants do not support WITH GRANT OPTION. See {INHERITED_GRANT_DOCS}") + if grant.priv == "ALL": + raise ValueError( + "Inherited grants require explicit privileges; priv='ALL' is not supported. " + "List the privileges you want, e.g. priv=['SELECT', 'INSERT']." + ) + # Inherited grants never transfer ownership, so OWNERSHIP is not a valid inherited priv. + if grant.priv == "OWNERSHIP": + raise ValueError(f"{grant.priv} cannot be granted as an inherited grant. See {INHERITED_GRANT_DOCS}") + if grant.items_type in NON_INHERITABLE_RESOURCE_TYPES: + raise ValueError( + f"{grant.items_type} objects cannot be the target of an inherited grant. See {INHERITED_GRANT_DOCS}" + ) + if grant.priv == "USAGE" and grant.items_type in NON_INHERITABLE_USAGE_TARGETS: + raise ValueError( + f"USAGE on {grant.items_type} cannot be granted as an inherited grant. See {INHERITED_GRANT_DOCS}" + ) + if grant.on_type not in (ResourceType.ACCOUNT, ResourceType.DATABASE, ResourceType.SCHEMA): + raise ValueError( + f"Inherited grants can only be created on ACCOUNT, DATABASE, or SCHEMA containers, " + f"got {grant.on_type}. See {INHERITED_GRANT_DOCS}" + ) + + @dataclass(unsafe_hash=True) class _Grant(ResourceSpec): priv: str @@ -60,6 +132,9 @@ def __post_init__(self): ) self.to_type = self.to.resource_type + if self.grant_type == GrantType.INHERITED: + _validate_inherited_grant(self) + class Grant(Resource): """ @@ -74,7 +149,8 @@ class Grant(Resource): on (string or Resource, required): The resource on which the privilege is granted. Can be a string like 'ACCOUNT' or a specific resource object. to (string or Role, required): The role to which the privileges are granted. grant_option (bool): Specifies whether the grantee can grant the privileges to other roles. Defaults to False. - owner (string or Role): The owner role of the grant. Defaults to 'SYSADMIN'. + owner (string or Role): The owner role of the grant. Defaults to 'SYSADMIN'. For inherited grants, names the role holding MANAGE GRANTS on the container. + inherited (bool): Turns a grant on all objects in a container into an inherited grant, which also covers objects created later. Defaults to False. Python: @@ -116,6 +192,27 @@ class Grant(Resource): to="somerole", ) + # Inherited Grants (covers current and future objects with one grant): + inherited_grant = Grant( + priv="SELECT", + on="INHERITED TABLES IN SCHEMA somedb.someschema", + to="somerole", + ) + inherited_grant = Grant( + priv="SELECT", + on=["INHERITED", "TABLES", Database(name="somedb")], + to="somerole", + ) + # The account can only be the container of an inherited grant + inherited_grant = Grant(priv="SELECT", on="INHERITED TABLES IN ACCOUNT", to="somerole") + # Or upgrade an existing grant on all objects in place + inherited_grant = Grant( + priv="SELECT", + on="ALL TABLES IN DATABASE somedb", + inherited=True, + to="somerole", + ) + ``` Yaml: @@ -138,6 +235,17 @@ class Grant(Resource): - priv: SELECT on: ALL IMAGE REPOSITORIES IN DATABASE somedb to: somerole + # One inherited grant replaces an ALL + FUTURE pair + - priv: SELECT + on: INHERITED TABLES IN SCHEMA somedb.someschema + to: somerole + - priv: SELECT + on: ALL TABLES IN DATABASE somedb + inherited: true + to: somerole + - priv: SELECT + on: INHERITED TABLES IN ACCOUNT + to: somerole ``` """ @@ -159,6 +267,7 @@ def __init__( to: Role = None, grant_option: bool = False, owner: str = None, + inherited: bool = False, **kwargs, ): self.kwargs = kwargs.copy() @@ -167,6 +276,7 @@ def __init__( self.kwargs["to"] = to self.kwargs["grant_option"] = grant_option self.kwargs["owner"] = owner + self.kwargs["inherited"] = inherited kwargs.pop("_privs", None) to_type = kwargs.pop("to_type", None) @@ -192,16 +302,13 @@ def __init__( # complete `on` spec (e.g. ["warehouse FOO", "warehouse BAR"] # or ["all schemas in database X", "future schemas in database X"]). # - # Heuristic: form (1) starts with the keyword FUTURE or ALL as its - # first element. Anything else is form (2). + # Heuristic: form (1) starts with the keyword FUTURE, ALL, or INHERITED + # as its first element. Anything else is form (2). first = on[0] - first_is_grant_type_keyword = ( - isinstance(first, str) - and first.upper() in (GrantType.FUTURE, GrantType.ALL) - ) + first_is_grant_type_keyword = isinstance(first, str) and first.upper() in COLLECTION_GRANT_KEYWORDS for item in on: if isinstance(item, list): - if item[0].upper() not in (GrantType.FUTURE, GrantType.ALL): + if item[0].upper() not in COLLECTION_GRANT_KEYWORDS: raise ValueError("You must specify a valid Grant Type when specifying a list of grants") has_many_ons = not first_is_grant_type_keyword if has_many_ons: @@ -251,7 +358,7 @@ def __init__( on_type = on.resource_type on = str(on.name) elif isinstance(on, NamedResource): - # It might make sense to explicitly fail if we cant fully resolve the resource + # It might make sense to explicitly fail if we can't fully resolve the resource on_type = on.resource_type on = str(on.fqn) elif isinstance(on, str) and on.upper() == "ACCOUNT": @@ -292,7 +399,10 @@ def __init__( mw_words = mw.split() if i + len(mw_words) <= len(parts): candidate = " ".join(parts[i : i + len(mw_words)]).upper() - if candidate == mw: + # Collection grants name the type in the plural ("CORTEX SEARCH + # SERVICES"); singularize the last word so a 3+-word type matches + # here as one item instead of splitting and tripping the count guard. + if candidate == mw or singularize(candidate) == mw: matched_multi = mw break if matched_multi is not None: @@ -300,12 +410,9 @@ def __init__( i += len(matched_multi.split()) continue # Single-word matches: ResourceType values or - # GrantType.FUTURE / GrantType.ALL keywords + # GrantType.FUTURE / ALL / INHERITED keywords part_normalized = part.upper().replace("_", " ") - if part_normalized in resource_type_values or part.upper() in [ - GrantType.FUTURE, - GrantType.ALL, - ]: + if part_normalized in resource_type_values or part.upper() in COLLECTION_GRANT_KEYWORDS: on_items.append(part_normalized) elif part: # This is likely the FQN - preserve case @@ -316,13 +423,28 @@ def __init__( elif on_items[0].upper() in [e.value for e in ResourceType]: on_type = resource_type_for_label(on_items[0]) on = on_items[1] - elif on_items[0].upper() in [GrantType.FUTURE, GrantType.ALL]: + elif on_items[0].upper() in COLLECTION_GRANT_KEYWORDS: grant_type = on_items[0] in_object = on_items[-1] - if isinstance(in_object, Resource): + if _is_account_container(on_items): + # "INHERITED TABLES IN ACCOUNT" -- the account is the container and + # has no name of its own. Only inherited grants can be scoped to + # the whole account; ALL and FUTURE grants cannot. Reject those here + # so it fails at plan time rather than rendering doubled-ACCOUNT SQL + # that only errors mid-apply. `inherited=True` upgrades an ALL grant to + # inherited later (also the from_sql round-trip path), so allow it. + if on_items[0].upper() != GrantType.INHERITED.value and not inherited: + raise ValueError( + f"{on_items[0]} grants cannot target the whole account; only inherited " + f"grants can be scoped to ACCOUNT. See {INHERITED_GRANT_DOCS}" + ) + items_type = resource_type_for_label(singularize(" ".join(on_items[1:-1]))) + on_type = ResourceType.ACCOUNT + on = "ACCOUNT" + elif isinstance(in_object, Resource): if len(on_items) > 4: - raise ValueError("You must specify only three paramters: [grant_type, items_type, object]") + raise ValueError("You must specify only three parameters: [grant_type, items_type, object]") items_type = resource_type_for_label(singularize(" ".join(on_items[1:-1]))) on_type = in_object.resource_type @@ -343,6 +465,17 @@ def __init__( else: raise ValueError(f"Grant type {on_items[0]} not recognized.") + if inherited: + # `inherited: true` upgrades an ALL grant in place, so an existing + # "all tables in database X" declaration becomes an inherited grant without + # having to be rewritten. + if GrantType(grant_type) not in (GrantType.ALL, GrantType.INHERITED): + raise ValueError( + "inherited=True applies to grants on all objects in a container, " + f"e.g. on='ALL TABLES IN DATABASE somedb'. Got {grant_type}." + ) + grant_type = GrantType.INHERITED + if owner is None: # Hacky fix if on_type == ResourceType.SCHEMA and on.upper().startswith("SNOWFLAKE"): @@ -450,6 +583,16 @@ def grant_fqn(grant: _Grant): ) +def grant_on_clause(grant: _Grant) -> str: + """Render a collection grant's target as the `on:` string that recreates it.""" + items = pluralize(str(grant.items_type)).upper() + if grant.on_type == ResourceType.ACCOUNT: + container = "ACCOUNT" + else: + container = f"{str(grant.on_type).upper()} {grant.on}" + return f"{grant.grant_type.value} {items} IN {container}" + + def grant_yaml(data: dict): grant = _Grant(**data) resource_label = resource_label_for_type(grant.on_type) @@ -460,7 +603,7 @@ def grant_yaml(data: dict): "grant_option": grant.grant_option, } if grant.items_type: - yml["on"] = f"{grant.items_type} IN {resource_label} {grant.on}" + yml["on"] = grant_on_clause(grant) else: yml[f"on_{resource_label}"] = grant.on return yml @@ -650,6 +793,15 @@ class DatabaseRoleGrant(Resource): to_database_role: somedb.someotherrole - database_role: somedb.somerole to_role: somerole + + # `roles` and `database_roles` grant the same database role to several + # targets, and both kinds may appear in one entry + - database_role: somedb.somerole + roles: + - analyst + - loader + database_roles: + - somedb.someotherrole ``` """ @@ -672,7 +824,7 @@ def __init__( to = kwargs.pop("to", None) if to: if to_role or to_database_role: - raise ValueError("You cant specify both to_role and to_database_role") + raise ValueError("You can't specify both to_role and to_database_role") if isinstance(to, Role): to_role = to elif isinstance(to, DatabaseRole): diff --git a/snowcap/resources/resource.py b/snowcap/resources/resource.py index b97bb94b..02446f3e 100644 --- a/snowcap/resources/resource.py +++ b/snowcap/resources/resource.py @@ -574,7 +574,7 @@ def _resolve_vars(self, vars: dict, resources: list): raise Exception(f"Cannot resolve vars for container {self.container} of {self}") # Only extract fields if container has a proper _data dataclass - if hasattr(container, '_data') and container._data is not None: + if hasattr(container, "_data") and container._data is not None: try: for f in fields(container._data): field_value = getattr(container._data, f.name) diff --git a/snowcap/resources/stream.py b/snowcap/resources/stream.py index d0a0a91c..6fe8cf98 100644 --- a/snowcap/resources/stream.py +++ b/snowcap/resources/stream.py @@ -398,7 +398,7 @@ def _resolver(data: dict): elif "on_view" in data: return ViewStream # using this as a workaround because there may not be enough properties during a small change to disambiguate - # really the different stream types should probably have seperate resource types. + # really the different stream types should probably have separate resource types. # Either that, or the resolver would need to look at the database to see what type of stream it is return TableStream diff --git a/snowcap/resources/view.py b/snowcap/resources/view.py index 8155ce6b..4322e3a0 100644 --- a/snowcap/resources/view.py +++ b/snowcap/resources/view.py @@ -38,15 +38,15 @@ def _extract_table_refs_from_sql(sql: str) -> list[str]: # Table names can be: simple (table), qualified (schema.table), or fully qualified (db.schema.table) # Also handles quoted identifiers like "TABLE_NAME" or "db"."schema"."table" identifier = r'(?:"[^"]+"|[A-Za-z_][A-Za-z0-9_$]*)' - qualified_name = rf'{identifier}(?:\.{identifier})*' + qualified_name = rf"{identifier}(?:\.{identifier})*" # Match FROM or any type of JOIN followed by a table name # Use word boundary and handle optional keywords like LATERAL, NATURAL, etc. - pattern = rf''' + pattern = rf""" (?:FROM|(?:CROSS|INNER|LEFT|RIGHT|FULL|NATURAL|LATERAL)\s+(?:OUTER\s+)?JOIN|JOIN) \s+ ({qualified_name}) - ''' + """ matches = re.findall(pattern, sql, re.IGNORECASE | re.VERBOSE) @@ -54,9 +54,7 @@ def _extract_table_refs_from_sql(sql: str) -> list[str]: result = [] for match in matches: # Remove surrounding quotes from each part if present - cleaned = ".".join( - part.strip('"') for part in match.split(".") - ) + cleaned = ".".join(part.strip('"') for part in match.split(".")) result.append(cleaned) return result diff --git a/snowcap/var.py b/snowcap/var.py index 3508383f..3d5522ff 100644 --- a/snowcap/var.py +++ b/snowcap/var.py @@ -1,4 +1,5 @@ import difflib +import re from typing import Any import jinja2.exceptions @@ -19,7 +20,7 @@ def _format_missing_key_error(key: str, available_keys: list[str], context: str msg = f'Key "{key}" not found.' if suggestions: - msg += f'\n Did you mean: {suggestions[0]}?' + msg += f"\n Did you mean: {suggestions[0]}?" if available_keys: msg += f'\n Available keys: {", ".join(sorted(available_keys))}' @@ -48,7 +49,9 @@ def to_string(self, vars: dict, parent: dict): raise MissingVarException( _format_missing_key_error(missing_key, available_keys, context="vars") + f"\n Template: {self.string}" - + "\n Provide vars with: --vars '{\"" + missing_key + "\": ...}'" + + "\n Provide vars with: --vars '{\"" + + missing_key + + "\": ...}'" ) from e raise MissingVarException(f"Missing var in template: {self.string}\n Error: {e}") @@ -104,3 +107,33 @@ def process_for_each(resource_value: str, each_value: Any) -> str: ) from e # Fallback to original error if we can't parse it raise MissingVarException(f"Error in for_each template '{resource_value}': {e}") from e + + +def evaluate_for_each_where(condition: str, each_value: Any) -> bool: + """Evaluate a for_each `where` expression against one item. + + The expression is bare Jinja (no braces), e.g. + + where: each.value.name.split('.')[0] == 'BALBOA' + + Items whose expression is falsy are skipped, which lets one var drive + several blocks that each cover a subset of it. + """ + # `where` only sees the current item as each.value. A var.* / parent.* reference resolves + # to a literal "{{ var.x }}" string via the stubs, so the whole expression is silently + # falsy for every item and the block declares nothing -- which, in sync mode, turns the + # grants it should own into DROPs. Reject it loudly rather than dropping access quietly. + # Strip string literals first so a var. inside a quoted value isn't mistaken for one. + without_literals = re.sub(r"'[^']*'|\"[^\"]*\"", "", condition) + if re.search(r"\b(?:var|parent)\.", without_literals): + raise MissingVarException( + f"for_each `where` expression '{condition}' may only reference `each.value`, not var/parent." + ) + try: + expression = GLOBAL_JINJA_ENV.compile_expression(condition) + except jinja2.exceptions.TemplateSyntaxError as e: + raise MissingVarException(f"Invalid for_each where expression '{condition}': {e}") from e + try: + return bool(expression(var=VarStub(), parent=ParentStub(), each={"value": each_value})) + except jinja2.exceptions.UndefinedError as e: + raise MissingVarException(f"Error in for_each where expression '{condition}': {e}") from e diff --git a/tests/integration/data_provider/test_fetch_resource.py b/tests/integration/data_provider/test_fetch_resource.py index 976d274c..74b840f4 100644 --- a/tests/integration/data_provider/test_fetch_resource.py +++ b/tests/integration/data_provider/test_fetch_resource.py @@ -726,5 +726,3 @@ def test_fetch_grant_of_database_role_multiple_grantees(cursor, suffix, marked_f assert clean_resource_data(res.DatabaseRoleGrant.spec, result_b) == clean_resource_data( res.DatabaseRoleGrant.spec, grant_b.to_dict() ) - - diff --git a/tests/integration/data_provider/test_list_resource.py b/tests/integration/data_provider/test_list_resource.py index 68aeb2d8..decbdf17 100644 --- a/tests/integration/data_provider/test_list_resource.py +++ b/tests/integration/data_provider/test_list_resource.py @@ -147,7 +147,9 @@ def test_list_resource(cursor, list_resources_database, resource, marked_for_cle list_kwargs = {} if resource.resource_type in GRANT_TYPES_DISABLE_ACCOUNT_USAGE: list_kwargs["use_account_usage"] = False - list_resources = data_provider.list_resource(cursor, resource_label_for_type(resource.resource_type), **list_kwargs) + list_resources = data_provider.list_resource( + cursor, resource_label_for_type(resource.resource_type), **list_kwargs + ) except snowflake.connector.errors.ProgrammingError as err: if err.errno == 2003: # Object does not exist - likely race condition with parallel tests diff --git a/tests/integration/test_account_usage.py b/tests/integration/test_account_usage.py index 69be78ca..45de4d5a 100644 --- a/tests/integration/test_account_usage.py +++ b/tests/integration/test_account_usage.py @@ -14,7 +14,6 @@ from snowcap import data_provider from snowcap.client import reset_cache - TEST_ROLE = os.environ.get("TEST_SNOWFLAKE_ROLE") pytestmark = pytest.mark.requires_snowflake @@ -99,8 +98,16 @@ def test_fetch_grants_from_account_usage_returns_list_or_none(self, cursor): grant = result[0] assert isinstance(grant, dict) # Check expected keys (normalized to lowercase) - expected_keys = {"created_on", "privilege", "granted_on", "name", - "granted_to", "grantee_name", "grant_option", "granted_by"} + expected_keys = { + "created_on", + "privilege", + "granted_on", + "name", + "granted_to", + "grantee_name", + "grant_option", + "granted_by", + } assert expected_keys.issubset(grant.keys()) else: # Without access, should return None (signaling fallback) @@ -264,7 +271,8 @@ def test_list_grants_returns_expected_grants_for_test_role( # Filter for our database grants (excluding OWNERSHIP and ROLE grants) test_grants = [ - g for g in grants + g + for g in grants if g["granted_on"] == "DATABASE" and g["name"].upper() == test_db.upper() and g["privilege"] in ("USAGE", "MONITOR") @@ -299,10 +307,7 @@ def test_list_role_grants_returns_role_hierarchy(self, cursor, suffix, marked_fo role_grants = execute(session, f"SHOW GRANTS OF ROLE {child_name}", cacheable=False) # Find our test grant (child granted to parent) - test_grants = [ - g for g in role_grants - if g["grantee_name"].upper() == parent_name.upper() - ] + test_grants = [g for g in role_grants if g["grantee_name"].upper() == parent_name.upper()] # Should have our role grant assert len(test_grants) >= 1, f"Expected role grant for {child_name}, got {test_grants}" diff --git a/tests/integration/test_grant_patterns.py b/tests/integration/test_grant_patterns.py index 18d757e5..78ba6661 100644 --- a/tests/integration/test_grant_patterns.py +++ b/tests/integration/test_grant_patterns.py @@ -250,9 +250,7 @@ def test_no_drift_after_multi_priv_grant_apply(self, cursor, suffix, test_db, ma class TestSPCSGrantsIntegration: """Test that grants on compute pools, image repositories, and services work correctly.""" - def test_grants_on_compute_pool_image_repository_and_service( - self, cursor, suffix, test_db, marked_for_cleanup - ): + def test_grants_on_compute_pool_image_repository_and_service(self, cursor, suffix, test_db, marked_for_cleanup): """Test: USAGE on compute pool + ALL on image repository + ALL on service in one apply. Compute pools are slow to provision, so this test creates one compute @@ -272,8 +270,7 @@ def test_grants_on_compute_pool_image_repository_and_service( pool_name = f"SPCS_POOL_{suffix}" cursor.execute( - f"CREATE COMPUTE POOL IF NOT EXISTS {pool_name} " - "MIN_NODES = 1 MAX_NODES = 1 INSTANCE_FAMILY = CPU_X64_XS" + f"CREATE COMPUTE POOL IF NOT EXISTS {pool_name} " "MIN_NODES = 1 MAX_NODES = 1 INSTANCE_FAMILY = CPU_X64_XS" ) repo_name = f"SPCS_REPO_{suffix}" @@ -282,8 +279,7 @@ def test_grants_on_compute_pool_image_repository_and_service( svc_name = f"SPCS_SVC_{suffix}" svc_fqn = f"{test_db}.{schema_name}.{svc_name}" - cursor.execute( - f""" + cursor.execute(f""" CREATE SERVICE IF NOT EXISTS {svc_fqn} IN COMPUTE POOL {pool_name} FROM SPECIFICATION $$ @@ -298,8 +294,7 @@ def test_grants_on_compute_pool_image_repository_and_service( $$ MIN_INSTANCES=1 MAX_INSTANCES=1 -""" - ) +""") # Cleanup order: service and compute pool are billable and account-scoped # (outside the DROP DATABASE safety net), so register them first - the # cleanup loop drops in this append order, and an aborted loop must @@ -374,9 +369,7 @@ def test_role_grant_single_role_to_role(self, cursor, suffix, marked_for_cleanup # Create role grant yaml_config = { - "role_grants": [ - {"role": source_role_name, "to_role": target_role_name} - ], + "role_grants": [{"role": source_role_name, "to_role": target_role_name}], } bc = collect_blueprint_config(yaml_config) diff --git a/tests/integration/test_lifecycle.py b/tests/integration/test_lifecycle.py index e3ccb875..6ae6c2e4 100644 --- a/tests/integration/test_lifecycle.py +++ b/tests/integration/test_lifecycle.py @@ -125,8 +125,8 @@ def test_create_drop_from_json(resource, cursor, suffix, lifecycle_db): database.add(resource) elif isinstance(resource.scope, SchemaScope): # Update resource to use the unique schema - if hasattr(resource, 'schema') and schema_name: - resource._data['schema'] = schema_name + if hasattr(resource, "schema") and schema_name: + resource._data["schema"] = schema_name database.public_schema.add(resource) blueprint = Blueprint() blueprint.add(resource) diff --git a/tests/test_blueprint.py b/tests/test_blueprint.py index 2db50482..acfd0ffc 100644 --- a/tests/test_blueprint.py +++ b/tests/test_blueprint.py @@ -48,17 +48,26 @@ def flatten_sql_commands(sql_commands_result) -> list[str]: from snowcap.blueprint import ( Blueprint, CreateResource, + DropResource, + TransferOwnership, UpdateResource, + compute_levels, _merge_pointers, compile_plan_to_sql, diff, dump_plan, + execution_strategy_for_change, + future_grant_precedence_warnings, + manifest_state_entries, + plan_entries, + raise_if_inherited_grants_unavailable, ) from snowcap.blueprint_config import BlueprintConfig from snowcap.data_provider import fetch_warehouse from snowcap.enums import AccountEdition, BlueprintScope, ResourceType from snowcap.exceptions import ( DuplicateResourceException, + MissingPrivilegeException, InvalidResourceException, MarkedForReplacementException, MissingVarException, @@ -664,6 +673,28 @@ def test_blueprint_dump_plan_drop(session_ctx): """ +def test_dump_plan_round_trips_dependency_levels(session_ctx, remote_state): + """apply --plan must preserve ordering, so dump_plan persists each change's level and the + loaders read it back. A bare-list plan (older format) restores to no levels.""" + from snowcap.blueprint import plan_from_dict, levels_from_plan_dict + + blueprint = Blueprint(resources=[res.Role("role1")]) + manifest = blueprint.generate_manifest(session_ctx) + plan = diff(remote_state, manifest) + urn = plan[0].urn + + dumped = json.loads(dump_plan(plan, format="json", levels={urn: 3})) + assert dumped["levels"] == {str(urn): 3} + assert [c.urn for c in plan_from_dict(dumped)] == [urn] + assert levels_from_plan_dict(dumped) == {urn: 3} + + # Backward compatibility: a bare-list plan still parses, with no levels restored. + bare = json.loads(dump_plan(plan, format="json")) + assert isinstance(bare, list) + assert [c.urn for c in plan_from_dict(bare)] == [urn] + assert levels_from_plan_dict(bare) == {} + + def test_blueprint_vars(session_ctx): blueprint = Blueprint( resources=[res.Role(name="role", comment=var.role_comment)], @@ -1345,7 +1376,10 @@ def test_apply_with_prebuilt_plan_warns_about_dropped_grants(self, monkeypatch, blueprint = Blueprint(resources=[]) monkeypatch.setattr("snowcap.blueprint.data_provider.fetch_session", lambda session: session_ctx) - monkeypatch.setattr("snowcap.blueprint.compile_plan_to_sql", lambda session_ctx, plan: ([], [])) + monkeypatch.setattr( + "snowcap.blueprint.compile_plan_to_sql", + lambda session_ctx, plan, shared_databases=None, database_owners=None: ([], []), + ) with caplog.at_level(logging.WARNING, logger="snowcap"): blueprint.apply(session=None, plan=[change]) @@ -1424,6 +1458,8 @@ def test_grant_already_in_plan_is_not_duplicated(self, session_ctx): ) grant_changes = [change for change in plan if change.urn == grant_urn] assert len(grant_changes) == 1 + + def test_blueprint_shared_database_create_default_owner(session_ctx, remote_state): shared_db = res.SharedDatabase(name="GONG", from_share="provider_account.share_name") blueprint = Blueprint(name="blueprint", resources=[shared_db]) @@ -1618,3 +1654,1142 @@ def test_schema_under_shared_database_raises_clear_error(session_ctx): with pytest.raises(OrphanResourceException, match="Cannot add SCHEMA '.*' to SharedDatabase 'GONG'"): blueprint.generate_manifest(session_ctx) + + +class TestFutureGrantPrecedenceWarnings: + """ + Tests for the database-level future grant warning surfaced by + Blueprint._warning_for_nonconforming_plan. + + Snowflake gives schema-level future grants precedence over database-level future + grants on the same object type, and silently ignores the database-level grant for + that schema. Managed access schemas make the conflict easy to introduce from a + separate config, so the check calls both situations out at plan time. + """ + + def _database_future_grants(self, database="MY_DB", to="READER", priv="SELECT"): + return [ + res.Grant(priv=priv, on=f"future tables in database {database}", to=to), + res.Grant(priv=priv, on=f"future views in database {database}", to=to), + ] + + def _warnings_for(self, session_ctx, resources): + blueprint = Blueprint(resources=resources) + manifest = blueprint.generate_manifest(session_ctx) + return future_grant_precedence_warnings(manifest_state_entries(manifest)) + + def test_managed_access_schema_with_database_future_grants_warns(self, session_ctx): + resources = [ + res.Database(name="MY_DB"), + res.Schema(name="MY_SCHEMA", database="MY_DB", managed_access=True), + res.Role(name="READER"), + *self._database_future_grants(), + ] + + warnings = self._warnings_for(session_ctx, resources) + + assert len(warnings) == 1 + assert "MY_DB.MY_SCHEMA" in warnings[0] + assert "managed access" in warnings[0] + assert "TABLES" in warnings[0] and "VIEWS" in warnings[0] + + def test_schema_without_managed_access_produces_no_warning(self, session_ctx): + resources = [ + res.Database(name="MY_DB"), + res.Schema(name="MY_SCHEMA", database="MY_DB"), + res.Role(name="READER"), + *self._database_future_grants(), + ] + + assert self._warnings_for(session_ctx, resources) == [] + + def test_shadowing_schema_future_grant_warns_that_database_grant_is_ignored(self, session_ctx): + resources = [ + res.Database(name="MY_DB"), + res.Schema(name="MY_SCHEMA", database="MY_DB", managed_access=True), + res.Role(name="READER"), + res.Role(name="WRITER"), + res.Grant(priv="SELECT", on="future tables in database MY_DB", to="READER"), + # A future grant on the same object type at the schema level, even to a + # different role, makes Snowflake ignore the database-level grant. + res.Grant(priv="INSERT", on="future tables in schema MY_DB.MY_SCHEMA", to="WRITER"), + ] + + warnings = self._warnings_for(session_ctx, resources) + + assert len(warnings) == 1 + assert "is ignored for MY_DB.MY_SCHEMA" in warnings[0] + assert "SELECT ON FUTURE TABLES IN DATABASE MY_DB to READER" in warnings[0] + assert "managed access" in warnings[0] + + def test_shadowing_is_scoped_to_the_same_object_type(self, session_ctx): + resources = [ + res.Database(name="MY_DB"), + res.Schema(name="MY_SCHEMA", database="MY_DB"), + res.Role(name="READER"), + res.Grant(priv="SELECT", on="future tables in database MY_DB", to="READER"), + res.Grant(priv="SELECT", on="future views in schema MY_DB.MY_SCHEMA", to="READER"), + ] + + assert self._warnings_for(session_ctx, resources) == [] + + def test_shadowing_is_scoped_to_the_same_database(self, session_ctx): + resources = [ + res.Database(name="MY_DB"), + res.Database(name="OTHER_DB"), + res.Schema(name="MY_SCHEMA", database="OTHER_DB"), + res.Role(name="READER"), + res.Grant(priv="SELECT", on="future tables in database MY_DB", to="READER"), + res.Grant(priv="SELECT", on="future tables in schema OTHER_DB.MY_SCHEMA", to="READER"), + ] + + assert self._warnings_for(session_ctx, resources) == [] + + def test_schema_level_future_grants_alone_produce_no_warning(self, session_ctx): + """The fix for the database-level trap: declare the grants at the schema level.""" + resources = [ + res.Database(name="MY_DB"), + res.Schema(name="MY_SCHEMA", database="MY_DB", managed_access=True), + res.Role(name="READER"), + res.Grant(priv="SELECT", on="future tables in schema MY_DB.MY_SCHEMA", to="READER"), + res.Grant(priv="SELECT", on="all tables in schema MY_DB.MY_SCHEMA", to="READER"), + ] + + assert self._warnings_for(session_ctx, resources) == [] + + def test_managed_access_from_remote_state_is_detected(self, session_ctx): + """The schema is already managed access in Snowflake and unchanged in this run, so + only remote state knows about it.""" + blueprint = Blueprint( + resources=[ + res.Database(name="MY_DB"), + res.Role(name="READER"), + *self._database_future_grants(), + ] + ) + manifest = blueprint.generate_manifest(session_ctx) + schema_urn = URN( + resource_type=ResourceType.SCHEMA, + fqn=FQN(ResourceName("REMOTE_SCHEMA"), database=ResourceName("MY_DB")), + account_locator=session_ctx["account_locator"], + ) + remote_state = {schema_urn: {"name": "REMOTE_SCHEMA", "managed_access": True}} + + warnings = future_grant_precedence_warnings(manifest_state_entries(manifest, remote_state)) + + assert len(warnings) == 1 + assert "MY_DB.REMOTE_SCHEMA" in warnings[0] + + def test_warning_is_surfaced_by_the_plan_warning_hook(self, session_ctx, caplog): + blueprint = Blueprint( + resources=[ + res.Database(name="MY_DB"), + res.Schema(name="MY_SCHEMA", database="MY_DB", managed_access=True), + res.Role(name="READER"), + *self._database_future_grants(), + ] + ) + manifest = blueprint.generate_manifest(session_ctx) + + with caplog.at_level(logging.WARNING, logger="snowcap"): + blueprint._warning_for_nonconforming_plan(session_ctx, [], manifest) + + assert "managed access" in caplog.text + assert "MY_DB.MY_SCHEMA" in caplog.text + + def test_prebuilt_plan_falls_back_to_plan_contents(self, session_ctx): + """`snowcap apply --plan plan.json` never rebuilds the manifest, so the check runs + over the changes in the plan instead.""" + blueprint = Blueprint( + resources=[ + res.Database(name="MY_DB"), + res.Schema(name="MY_SCHEMA", database="MY_DB", managed_access=True), + res.Role(name="READER"), + *self._database_future_grants(), + ] + ) + manifest = blueprint.generate_manifest(session_ctx) + plan = [ + CreateResource(urn, item.resource_cls, None, item.data) + for urn, item in manifest.items() + if hasattr(item, "data") + ] + + warnings = future_grant_precedence_warnings(plan_entries(plan)) + + assert len(warnings) == 1 + assert "MY_DB.MY_SCHEMA" in warnings[0] + + +class TestInheritedGrantPlanning: + """ + Tests for planning inherited grants: the account-level feature gate, how a container + grant covers the per-object grants it produced, and which role issues it. + """ + + def _object_grant_state(self, priv="SELECT", to="SOMEROLE", on="DB.SCH.TBL"): + urn = parse_URN(f"urn::ABCD123:grant/GRANT?grant_type=OBJECT&priv={priv}&on=table/{on}&to=role/{to}") + return urn, { + "priv": priv, + "on": on, + "on_type": "TABLE", + "to": to, + "items_type": None, + "to_type": "ROLE", + "grant_option": False, + "grant_type": "OBJECT", + "owner": "SYSADMIN", + "_privs": [priv], + } + + def _manifest(self, session_ctx, resources): + return Blueprint(resources=resources).generate_manifest(session_ctx) + + def test_plan_fails_when_the_account_has_not_enabled_the_feature(self, session_ctx): + manifest = self._manifest( + session_ctx, + [ + res.Database(name="MY_DB"), + res.Role(name="READER"), + res.Grant(priv="SELECT", on="INHERITED TABLES IN DATABASE MY_DB", to="READER"), + ], + ) + + with patch("snowcap.data_provider.fetch_inherited_grants_enabled", return_value=False): + with pytest.raises(MissingPrivilegeException, match="FEATURE_RBAC_INHERITED_GRANTS"): + raise_if_inherited_grants_unavailable(MagicMock(), manifest) + + def test_plan_proceeds_when_the_feature_flag_cannot_be_read(self, session_ctx): + """Reading account parameters needs privileges the session may not hold; that must + not block an apply that would otherwise succeed.""" + manifest = self._manifest( + session_ctx, + [ + res.Database(name="MY_DB"), + res.Role(name="READER"), + res.Grant(priv="SELECT", on="INHERITED TABLES IN DATABASE MY_DB", to="READER"), + ], + ) + + with patch("snowcap.data_provider.fetch_inherited_grants_enabled", return_value=None): + raise_if_inherited_grants_unavailable(MagicMock(), manifest) + + def test_the_feature_flag_is_not_probed_without_inherited_grants(self, session_ctx): + manifest = self._manifest(session_ctx, [res.Database(name="MY_DB")]) + + with patch("snowcap.data_provider.fetch_inherited_grants_enabled") as probe: + raise_if_inherited_grants_unavailable(MagicMock(), manifest) + + probe.assert_not_called() + + def test_inherited_grant_covers_remote_per_object_grants(self, session_ctx, remote_state): + """Migrating per-object grants to an inherited grant must not revoke the access the + inherited grant provides.""" + remote_state = remote_state.copy() + urn, data = self._object_grant_state() + remote_state[urn] = data + manifest = self._manifest( + session_ctx, + [ + res.Database(name="DB"), + res.Role(name="SOMEROLE"), + res.Grant(priv="SELECT", on="INHERITED TABLES IN DATABASE DB", to="SOMEROLE"), + ], + ) + + plan = diff(remote_state, manifest) + + assert not [change for change in plan if isinstance(change, DropResource) and change.urn == urn] + + def test_uncovered_object_grants_are_still_dropped(self, session_ctx, remote_state): + """Coverage is per privilege, grantee, object type, and container -- a collection + grant elsewhere in the config does not protect an unrelated grant.""" + remote_state = remote_state.copy() + urn, data = self._object_grant_state(priv="INSERT") + remote_state[urn] = data + manifest = self._manifest( + session_ctx, + [ + res.Database(name="DB"), + res.Role(name="SOMEROLE"), + res.Grant(priv="SELECT", on="INHERITED TABLES IN DATABASE DB", to="SOMEROLE"), + ], + ) + + plan = diff(remote_state, manifest) + + assert [change for change in plan if isinstance(change, DropResource) and change.urn == urn] + + def test_grant_on_all_covers_objects_in_its_database(self, session_ctx, remote_state): + remote_state = remote_state.copy() + urn, data = self._object_grant_state() + remote_state[urn] = data + manifest = self._manifest( + session_ctx, + [ + res.Database(name="DB"), + res.Role(name="SOMEROLE"), + res.Grant(priv="SELECT", on="ALL TABLES IN DATABASE DB", to="SOMEROLE"), + ], + ) + + plan = diff(remote_state, manifest) + + assert not [change for change in plan if isinstance(change, DropResource) and change.urn == urn] + + def test_grant_all_collection_covers_expanded_privilege_rows(self, session_ctx, remote_state): + """`GRANT ALL ON ALL TABLES` fans out into concrete-privilege rows (SELECT, INSERT, ...). + A declared ALL collection grant must cover them, or sync drops each one every run.""" + remote_state = remote_state.copy() + urn, data = self._object_grant_state(priv="SELECT") + remote_state[urn] = data + manifest = self._manifest( + session_ctx, + [ + res.Database(name="DB"), + res.Role(name="SOMEROLE"), + res.Grant(priv="ALL", on="ALL TABLES IN DATABASE DB", to="SOMEROLE"), + ], + ) + + plan = diff(remote_state, manifest) + + assert not [change for change in plan if isinstance(change, DropResource) and change.urn == urn] + + def test_container_covers_handles_quoted_identifiers_with_dots(self): + """A quoted identifier can contain a literal dot; a plain split miscounts the parts and + mis-classifies containment.""" + from snowcap.blueprint import _container_covers + from snowcap.enums import ResourceType + + assert _container_covers(ResourceType.SCHEMA.value, 'DB."a.b"', 'DB."a.b".TBL') + assert not _container_covers(ResourceType.SCHEMA.value, 'DB."a.b"', "DB.OTHER.TBL") + assert _container_covers(ResourceType.DATABASE.value, "DB", 'DB."a.b".TBL') + + def test_a_collection_grant_in_another_database_does_not_protect_the_grant(self, session_ctx, remote_state): + remote_state = remote_state.copy() + urn, data = self._object_grant_state() + remote_state[urn] = data + manifest = self._manifest( + session_ctx, + [ + res.Database(name="OTHER_DB"), + res.Role(name="SOMEROLE"), + res.Grant(priv="SELECT", on="ALL TABLES IN DATABASE OTHER_DB", to="SOMEROLE"), + ], + ) + + plan = diff(remote_state, manifest) + + assert [change for change in plan if isinstance(change, DropResource) and change.urn == urn] + + def test_inherited_grant_runs_as_its_declared_container_admin(self, session_ctx): + """Container-level MANAGE GRANTS is how a database admin manages access without + account-wide authority, so a declared owner is used in preference to SECURITYADMIN.""" + grant = res.Grant( + priv="SELECT", + on="INHERITED TABLES IN DATABASE SALES_DB", + to="ANALYST", + owner="SALES_DB_ADMIN", + ) + change = CreateResource( + urn=parse_URN( + "urn::ABCD123:grant/GRANT?grant_type=INHERITED&priv=SELECT&on=database/SALES_DB.
&to=role/ANALYST" + ), + resource_cls=res.Grant, + container=None, + after=grant.to_dict(), + ) + + role, _ = execution_strategy_for_change( + change, ["SYSADMIN", "SECURITYADMIN", "SALES_DB_ADMIN"], ResourceName("SYSADMIN") + ) + + assert role == ResourceName("SALES_DB_ADMIN") + + def test_inherited_grant_falls_back_to_securityadmin(self, session_ctx): + grant = res.Grant(priv="SELECT", on="INHERITED TABLES IN DATABASE SALES_DB", to="ANALYST") + change = CreateResource( + urn=parse_URN( + "urn::ABCD123:grant/GRANT?grant_type=INHERITED&priv=SELECT&on=database/SALES_DB.
&to=role/ANALYST" + ), + resource_cls=res.Grant, + container=None, + after=grant.to_dict(), + ) + + role, _ = execution_strategy_for_change(change, ["SYSADMIN", "SECURITYADMIN"], ResourceName("SYSADMIN")) + + assert role == ResourceName("SECURITYADMIN") + + def test_object_grants_are_unaffected_by_the_delegation_path(self, session_ctx): + """A declared owner on an ordinary grant keeps running as SECURITYADMIN, as before.""" + grant = res.Grant(priv="SELECT", on_table="DB.SCH.TBL", to="ANALYST", owner="SOME_ROLE") + change = CreateResource( + urn=parse_URN("urn::ABCD123:grant/GRANT?grant_type=OBJECT&priv=SELECT&on=table/DB.SCH.TBL&to=role/ANALYST"), + resource_cls=res.Grant, + container=None, + after=grant.to_dict(), + ) + + role, _ = execution_strategy_for_change( + change, ["SYSADMIN", "SECURITYADMIN", "SOME_ROLE"], ResourceName("SYSADMIN") + ) + + assert role == ResourceName("SECURITYADMIN") + + def test_an_existing_inherited_grant_produces_no_changes(self, session_ctx, remote_state): + """The point of inherited grants over ON ALL: Snowflake reports one durable record, + so the plan is empty on a second run instead of reapplying the grant every time.""" + from snowcap import data_provider + + grant = res.Grant(priv="SELECT", on="INHERITED TABLES IN DATABASE SALES_DB", to="ANALYST") + manifest = self._manifest(session_ctx, [res.Database(name="SALES_DB"), res.Role(name="ANALYST"), grant]) + urn = URN.from_resource(account_locator=session_ctx["account_locator"], resource=grant) + row = { + "privilege": "SELECT", + "granted_on": "TABLE", + "name": "", + "granted_to": "ROLE", + "grantee_name": "ANALYST", + "grant_option": "false", + "granted_by": "SECURITYADMIN", + "is_inherited": True, + "inherited_from": "DATABASE", + "inherited_from_database": "SALES_DB", + "inherited_from_schema": "", + } + + with patch("snowcap.data_provider.execute", return_value=[row]): + fetched = data_provider.fetch_inherited_grant(MagicMock(), urn.fqn) + + remote_state = remote_state.copy() + remote_state[urn] = fetched + + assert [change for change in diff(remote_state, manifest) if change.urn == urn] == [] + + def test_config_can_enable_the_feature_itself(self, session_ctx): + """Declaring the account parameter is the supported way to turn the preview on, so + the plan gate must not block the very config that enables it.""" + manifest = self._manifest( + session_ctx, + [ + res.AccountParameter(name="FEATURE_RBAC_INHERITED_GRANTS", value="ENABLED"), + res.Database(name="MY_DB"), + res.Role(name="READER"), + res.Grant(priv="SELECT", on="INHERITED TABLES IN DATABASE MY_DB", to="READER"), + ], + ) + + with patch("snowcap.data_provider.fetch_inherited_grants_enabled", return_value=False) as probe: + raise_if_inherited_grants_unavailable(MagicMock(), manifest) + + assert not probe.called + + def test_disabling_the_parameter_does_not_count_as_enabling_it(self, session_ctx): + manifest = self._manifest( + session_ctx, + [ + res.AccountParameter(name="FEATURE_RBAC_INHERITED_GRANTS", value="DISABLED"), + res.Database(name="MY_DB"), + res.Role(name="READER"), + res.Grant(priv="SELECT", on="INHERITED TABLES IN DATABASE MY_DB", to="READER"), + ], + ) + + with patch("snowcap.data_provider.fetch_inherited_grants_enabled", return_value=False): + with pytest.raises(MissingPrivilegeException): + raise_if_inherited_grants_unavailable(MagicMock(), manifest) + + def test_inherited_grants_are_applied_after_the_feature_flag(self, session_ctx): + """Both are account-scoped with nothing else linking them, so without an explicit + dependency they would land in the same level and run concurrently.""" + flag = res.AccountParameter(name="FEATURE_RBAC_INHERITED_GRANTS", value="ENABLED") + grant = res.Grant(priv="SELECT", on="INHERITED TABLES IN DATABASE MY_DB", to="READER") + manifest = self._manifest(session_ctx, [flag, res.Database(name="MY_DB"), res.Role(name="READER"), grant]) + + locator = session_ctx["account_locator"] + flag_urn = URN.from_resource(account_locator=locator, resource=flag) + grant_urn = URN.from_resource(account_locator=locator, resource=grant) + + # The ordering must come from a real dependency edge, not incidental level + # assignment: assert the grant->flag edge is present in the manifest. + assert (grant_urn, flag_urn) in set(manifest.refs) + + resource_set = set(manifest.urns) + for parent, ref in manifest.refs: + resource_set.add(parent) + resource_set.add(ref) + levels = compute_levels(resource_set, set(manifest.refs)) + assert levels[grant_urn] > levels[flag_urn] + + def test_grants_on_all_are_not_linked_to_the_feature_flag(self, session_ctx): + """Only inherited grants need the preview; an ON ALL grant must not be held back.""" + flag = res.AccountParameter(name="FEATURE_RBAC_INHERITED_GRANTS", value="ENABLED") + grant = res.Grant(priv="SELECT", on="ALL TABLES IN DATABASE MY_DB", to="READER") + manifest = self._manifest(session_ctx, [flag, res.Database(name="MY_DB"), res.Role(name="READER"), grant]) + + locator = session_ctx["account_locator"] + flag_urn = URN.from_resource(account_locator=locator, resource=flag) + grant_urn = URN.from_resource(account_locator=locator, resource=grant) + assert (grant_urn, flag_urn) not in manifest.refs + + def test_error_points_at_preview_access_when_it_is_disabled(self, session_ctx): + """Setting the parameter will not help while preview features are off account-wide, + and Snowcap cannot turn them on -- it is a system function, not a resource.""" + manifest = self._manifest( + session_ctx, + [ + res.Database(name="MY_DB"), + res.Role(name="READER"), + res.Grant(priv="SELECT", on="INHERITED TABLES IN DATABASE MY_DB", to="READER"), + ], + ) + + with patch("snowcap.data_provider.fetch_inherited_grants_enabled", return_value=False): + with patch("snowcap.data_provider.fetch_preview_access_enabled", return_value=False): + with pytest.raises(MissingPrivilegeException, match=r"SYSTEM\$ENABLE_PREVIEW_ACCESS"): + raise_if_inherited_grants_unavailable(MagicMock(), manifest) + + def test_error_suggests_the_parameter_when_preview_access_is_fine(self, session_ctx): + manifest = self._manifest( + session_ctx, + [ + res.Database(name="MY_DB"), + res.Role(name="READER"), + res.Grant(priv="SELECT", on="INHERITED TABLES IN DATABASE MY_DB", to="READER"), + ], + ) + + with patch("snowcap.data_provider.fetch_inherited_grants_enabled", return_value=False): + with patch("snowcap.data_provider.fetch_preview_access_enabled", return_value=True): + with pytest.raises(MissingPrivilegeException) as excinfo: + raise_if_inherited_grants_unavailable(MagicMock(), manifest) + + assert "account_parameters" in str(excinfo.value) + assert "SYSTEM$ENABLE_PREVIEW_ACCESS" not in str(excinfo.value) + + +class TestImportedPrivilegesPlanning: + """ + A single IMPORTED PRIVILEGES grant on a shared database fans out in SHOW GRANTS into a + row per object the share exposes. Those rows can never be in the manifest, so sync must + recognise them as covered rather than revoking the access the declared grant provides. + """ + + def _shared_object_grant_state(self, priv, on, on_type, to="Z_DB__SNOWFLAKE"): + urn = parse_URN( + f"urn::ABCD123:grant/GRANT?grant_type=OBJECT&priv={priv}&on={on_type.lower()}/{on}&to=role/{to}" + ) + return urn, { + "priv": priv, + "on": on, + "on_type": on_type, + "to": to, + "items_type": None, + "to_type": "ROLE", + "grant_option": False, + "grant_type": "OBJECT", + "owner": "SYSADMIN", + "_privs": [priv], + } + + def _manifest(self, session_ctx, resources): + return Blueprint(resources=resources).generate_manifest(session_ctx) + + def _snowflake_share_manifest(self, session_ctx, to="Z_DB__SNOWFLAKE"): + return self._manifest( + session_ctx, + [ + res.Role(name=to), + res.Grant(priv="IMPORTED PRIVILEGES", on="database SNOWFLAKE", to=to), + ], + ) + + @pytest.mark.parametrize( + "priv,on,on_type", + [ + # The fan-out carries whatever privilege each object type takes, never + # "IMPORTED PRIVILEGES" itself. + ("SELECT", "SNOWFLAKE.ACCOUNT_USAGE.QUERY_HISTORY", "VIEW"), + ("USAGE", "SNOWFLAKE.CORE.DUPLICATE_COUNT(TABLE(DATE)", "FUNCTION"), + ("USAGE", "SNOWFLAKE.CORTEX.CREATE_AI_FUNCTION(VARCHAR)", "PROCEDURE"), + ("USAGE", "SNOWFLAKE.ACCOUNT_USAGE", "SCHEMA"), + ("USAGE", "SNOWFLAKE.CORTEX_USER", "DATABASE_ROLE"), + ("READ", "SNOWFLAKE.IMAGES.SNOWFLAKE_IMAGES", "IMAGE_REPOSITORY"), + ("APPLY", "SNOWFLAKE.CORE.CERTIFICATION_STATUS", "TAG"), + # Snowflake also reports the database itself + ("USAGE", "SNOWFLAKE", "DATABASE"), + ("REFERENCE_USAGE", "SNOWFLAKE", "DATABASE"), + ], + ) + def test_fan_out_of_an_imported_privileges_grant_is_not_dropped(self, session_ctx, remote_state, priv, on, on_type): + remote_state = remote_state.copy() + urn, data = self._shared_object_grant_state(priv, on, on_type) + remote_state[urn] = data + + plan = diff(remote_state, self._snowflake_share_manifest(session_ctx)) + + assert not [change for change in plan if isinstance(change, DropResource) and change.urn == urn] + + def test_fan_out_to_a_different_grantee_is_still_dropped(self, session_ctx, remote_state): + """Coverage is scoped to the role named by the declared grant.""" + remote_state = remote_state.copy() + urn, data = self._shared_object_grant_state( + "SELECT", "SNOWFLAKE.ACCOUNT_USAGE.QUERY_HISTORY", "VIEW", to="SOME_OTHER_ROLE" + ) + remote_state[urn] = data + + plan = diff(remote_state, self._snowflake_share_manifest(session_ctx)) + + assert [change for change in plan if isinstance(change, DropResource) and change.urn == urn] + + def test_grants_outside_the_shared_database_are_still_dropped(self, session_ctx, remote_state): + """An IMPORTED PRIVILEGES grant protects only objects inside its own database.""" + remote_state = remote_state.copy() + urn, data = self._shared_object_grant_state("SELECT", "OTHER_DB.SCH.TBL", "TABLE") + remote_state[urn] = data + + plan = diff(remote_state, self._snowflake_share_manifest(session_ctx)) + + assert [change for change in plan if isinstance(change, DropResource) and change.urn == urn] + + def test_a_database_named_like_the_share_is_not_covered(self, session_ctx, remote_state): + """Containment is by identifier; a different database is a different container.""" + remote_state = remote_state.copy() + urn, data = self._shared_object_grant_state("USAGE", "SNOWFLAKE_OTHER", "DATABASE") + remote_state[urn] = data + + plan = diff(remote_state, self._snowflake_share_manifest(session_ctx)) + + assert [change for change in plan if isinstance(change, DropResource) and change.urn == urn] + + def test_object_grants_are_dropped_when_no_imported_privileges_are_declared(self, session_ctx, remote_state): + """Without a declared IMPORTED PRIVILEGES grant nothing changes: undeclared object + grants are still reaped by sync.""" + remote_state = remote_state.copy() + urn, data = self._shared_object_grant_state("SELECT", "SNOWFLAKE.ACCOUNT_USAGE.QUERY_HISTORY", "VIEW") + remote_state[urn] = data + manifest = self._manifest(session_ctx, [res.Role(name="Z_DB__SNOWFLAKE")]) + + plan = diff(remote_state, manifest) + + assert [change for change in plan if isinstance(change, DropResource) and change.urn == urn] + + +class TestCreateInsideTransferredContainer: + """A plan that adopts an existing database both transfers it and creates resources + inside it. Containers sit at a lower dependency level than their contents, so the + transfer runs first, and a CREATE planned against the container's old owner arrives + to find that role no longer owns anything.""" + + def _database_role_change(self, container_owner): + database_role = res.DatabaseRole(name="DR_READER_ROLE", database="GREAT_BAY_DEV", owner="USERADMIN") + return CreateResource( + urn=parse_URN("urn::ABCD123:database_role/GREAT_BAY_DEV.DR_READER_ROLE"), + resource_cls=res.DatabaseRole, + container=(parse_URN("urn::ABCD123:database/GREAT_BAY_DEV"), ResourceName(container_owner)), + after=database_role.to_dict(), + ) + + def test_create_runs_as_the_owner_the_container_ends_up_with(self): + change = self._database_role_change("ANALYST") + transferred = {parse_URN("urn::ABCD123:database/GREAT_BAY_DEV"): ResourceName("TRANSFORMER_DBT")} + + role, _ = execution_strategy_for_change( + change, + [ResourceName("ANALYST"), ResourceName("TRANSFORMER_DBT"), ResourceName("SECURITYADMIN")], + ResourceName("SECURITYADMIN"), + transferred, + ) + + assert role == ResourceName("TRANSFORMER_DBT") + + def test_create_runs_as_the_current_owner_when_the_container_is_not_transferred(self): + change = self._database_role_change("ANALYST") + + role, _ = execution_strategy_for_change( + change, + [ResourceName("ANALYST"), ResourceName("TRANSFORMER_DBT"), ResourceName("SECURITYADMIN")], + ResourceName("SECURITYADMIN"), + {}, + ) + + assert role == ResourceName("ANALYST") + + def test_a_transfer_of_a_different_container_is_ignored(self): + change = self._database_role_change("ANALYST") + transferred = {parse_URN("urn::ABCD123:database/BALBOA_DEV"): ResourceName("TRANSFORMER_DBT")} + + role, _ = execution_strategy_for_change( + change, + [ResourceName("ANALYST"), ResourceName("TRANSFORMER_DBT"), ResourceName("SECURITYADMIN")], + ResourceName("SECURITYADMIN"), + transferred, + ) + + assert role == ResourceName("ANALYST") + + def test_compile_plan_to_sql_picks_up_the_transfer_from_the_plan(self, session_ctx): + """The end-to-end path: compile_plan_to_sql derives the mapping from the plan + itself, so a caller does not have to know the transfer happened.""" + plan = [ + TransferOwnership( + urn=parse_URN("urn::ABCD123:database/GREAT_BAY_DEV"), + resource_cls=res.Database, + from_owner="ANALYST", + to_owner="TRANSFORMER_DBT", + ), + self._database_role_change("ANALYST"), + ] + + commands, _ = compile_plan_to_sql(session_ctx, plan) + + create = [c for c in commands if isinstance(c["change"], CreateResource)][0] + assert create["role"] == ResourceName("TRANSFORMER_DBT") + + +class TestDroppingGrantsOnSharedDatabases: + """Privileges on a shared database arrive as the fan-out of one IMPORTED PRIVILEGES + grant and cannot be revoked one at a time. Snowflake rejects the individual revoke with + "Revoking individual privileges on imported database is not allowed", and because that + is a SQL compilation error rather than a permissions one it aborts the apply.""" + + def _drop(self, priv, on, on_type, to="ANALYST"): + return DropResource( + urn=parse_URN( + f"urn::ABCD123:grant/GRANT?grant_type=OBJECT&priv={priv}&on={on_type.lower()}/{on}&to=role/{to}" + ), + before={ + "priv": priv, + "on": on, + "on_type": on_type, + "to": to, + "to_type": "ROLE", + "items_type": None, + "grant_option": False, + "grant_type": "OBJECT", + "owner": "SYSADMIN", + "_privs": [priv], + }, + ) + + @pytest.mark.parametrize( + "priv,on,on_type", + [ + ("USAGE", "WORLDWIDE_ADDRESS_DATA", "DATABASE"), + ("USAGE", "WORLDWIDE_ADDRESS_DATA.ADDRESS", "SCHEMA"), + ("SELECT", "WORLDWIDE_ADDRESS_DATA.ADDRESS.OPENADDRESS", "TABLE"), + ], + ) + def test_every_row_of_the_fan_out_revokes_the_share(self, session_ctx, priv, on, on_type): + """Database, schema and object rows all map to the same statement -- the share is + the only thing that can be given back.""" + commands, _ = compile_plan_to_sql(session_ctx, [self._drop(priv, on, on_type)], {"WORLDWIDE_ADDRESS_DATA"}) + + sql = " ".join(commands[0]["commands"]) + assert "REVOKE IMPORTED PRIVILEGES ON DATABASE WORLDWIDE_ADDRESS_DATA FROM ROLE ANALYST" in sql + assert "REVOKE USAGE ON DATABASE WORLDWIDE_ADDRESS_DATA" not in sql + + def test_ordinary_databases_still_revoke_the_individual_privilege(self, session_ctx): + """The share form must not leak onto normal databases, where it is invalid.""" + commands, _ = compile_plan_to_sql( + session_ctx, [self._drop("USAGE", "BALBOA", "DATABASE")], {"WORLDWIDE_ADDRESS_DATA"} + ) + + sql = " ".join(commands[0]["commands"]) + assert "REVOKE USAGE ON DATABASE BALBOA FROM ROLE ANALYST" in sql + assert "IMPORTED PRIVILEGES" not in sql + + def test_account_level_object_named_like_a_shared_db_is_not_the_share(self, session_ctx): + """A warehouse (or other account-level object) whose name collides with an imported + database must revoke its own privilege, not IMPORTED PRIVILEGES on the share.""" + commands, _ = compile_plan_to_sql( + session_ctx, [self._drop("USAGE", "WORLDWIDE_ADDRESS_DATA", "WAREHOUSE")], {"WORLDWIDE_ADDRESS_DATA"} + ) + + sql = " ".join(commands[0]["commands"]) + assert "REVOKE USAGE ON WAREHOUSE WORLDWIDE_ADDRESS_DATA FROM ROLE ANALYST" in sql + assert "IMPORTED PRIVILEGES" not in sql + + def test_shared_database_match_is_quote_aware(self): + """A database quoted with a literal dot must still match the shared-databases set; a + naive split would produce the wrong name and miss it.""" + from snowcap.blueprint import _shared_database_for_grant + + change = DropResource( + urn=parse_URN("urn::ABCD123:grant/GRANT?grant_type=OBJECT&priv=USAGE&on=database/X&to=role/R"), + before={"on": '"prod.mirror".ADDRESS', "on_type": "SCHEMA", "priv": "USAGE", "to": "R"}, + ) + + assert _shared_database_for_grant(change, {"PROD.MIRROR"}) == "PROD.MIRROR" + + def test_no_shared_databases_known_leaves_behaviour_unchanged(self, session_ctx): + commands, _ = compile_plan_to_sql(session_ctx, [self._drop("USAGE", "WORLDWIDE_ADDRESS_DATA", "DATABASE")]) + + sql = " ".join(commands[0]["commands"]) + assert "REVOKE USAGE ON DATABASE WORLDWIDE_ADDRESS_DATA" in sql + + +class TestRevokingAccountLevelPrivileges: + """An account-level privilege belongs to the system role that owns it. Snowflake will + not take one back from a role that does not own it -- and rather than failing, the + REVOKE reports success while leaving the privilege in place, so the same drop reappears + in every later plan and nothing in the output says why.""" + + def _account_grant_change(self, cls, priv="CREATE DATABASE", to="TRANSFORMER_DBT"): + data = { + "priv": priv, + "on": "ACCOUNT", + "on_type": "ACCOUNT", + "to": to, + "to_type": "ROLE", + "items_type": None, + "grant_option": False, + "grant_type": "OBJECT", + "owner": "SECURITYADMIN", + "_privs": [priv], + } + urn = parse_URN( + f"urn::ABCD123:grant/GRANT?grant_type=OBJECT&priv={priv.replace(' ', '%20')}" + f"&on=account/ACCOUNT&to=role/{to}" + ) + if cls is DropResource: + return DropResource(urn=urn, before=data) + return CreateResource(urn=urn, resource_cls=res.Grant, container=None, after=data) + + ROLES = [ResourceName("SYSADMIN"), ResourceName("SECURITYADMIN"), ResourceName("ACCOUNTADMIN")] + + def test_revoke_runs_as_the_system_role_that_owns_the_privilege(self): + change = self._account_grant_change(DropResource) + + role, _ = execution_strategy_for_change(change, self.ROLES, ResourceName("SECURITYADMIN")) + + assert role == ResourceName("SYSADMIN") + + def test_grant_and_revoke_agree_on_the_role(self): + """The asymmetry was the bug: grants already used the system role.""" + grant_role, _ = execution_strategy_for_change( + self._account_grant_change(CreateResource), self.ROLES, ResourceName("SECURITYADMIN") + ) + revoke_role, _ = execution_strategy_for_change( + self._account_grant_change(DropResource), self.ROLES, ResourceName("SECURITYADMIN") + ) + + assert grant_role == revoke_role == ResourceName("SYSADMIN") + + def test_openflow_data_plane_integration_is_a_known_account_privilege(self): + """Snowcap did not know this privilege, so it fell through to SECURITYADMIN and the + revoke silently did nothing.""" + from snowcap.privs import system_role_for_priv + + assert system_role_for_priv("CREATE OPENFLOW DATA PLANE INTEGRATION") == "ACCOUNTADMIN" + + change = self._account_grant_change(DropResource, priv="CREATE OPENFLOW DATA PLANE INTEGRATION", to="LOADER") + role, _ = execution_strategy_for_change(change, self.ROLES, ResourceName("SECURITYADMIN")) + + assert role == ResourceName("ACCOUNTADMIN") + + def test_revoke_falls_back_to_securityadmin_without_the_system_role(self): + change = self._account_grant_change(DropResource) + + role, _ = execution_strategy_for_change(change, [ResourceName("SECURITYADMIN")], ResourceName("SECURITYADMIN")) + + assert role == ResourceName("SECURITYADMIN") + + def test_object_grant_revokes_still_use_securityadmin(self): + """Only account-level privileges have an owning system role; ordinary object grants + must keep going through SECURITYADMIN.""" + data = { + "priv": "USAGE", + "on": "BALBOA", + "on_type": "DATABASE", + "to": "ANALYST", + "to_type": "ROLE", + "items_type": None, + "grant_option": False, + "grant_type": "OBJECT", + "owner": "SECURITYADMIN", + "_privs": ["USAGE"], + } + change = DropResource( + urn=parse_URN("urn::ABCD123:grant/GRANT?grant_type=OBJECT&priv=USAGE&on=database/BALBOA&to=role/ANALYST"), + before=data, + ) + + role, _ = execution_strategy_for_change(change, self.ROLES, ResourceName("SECURITYADMIN")) + + assert role == ResourceName("SECURITYADMIN") + + +class TestGrantsHeldByDatabaseRoles: + """A database role is named . and lives inside its database. Managing a + grant it holds needs a role that can see that database. SECURITYADMIN can hold + account-level MANAGE GRANTS and still lack USAGE on the database, and REVOKE reports + success rather than failing on a grantee it cannot resolve -- so the grant survives and + the same drop reappears in every later plan, with nothing in the output to say why.""" + + OWNERS = {"GREAT_BAY": "TRANSFORMER_DBT"} + + def _change(self, cls, to="GREAT_BAY.DR_CREATE_ROLE", to_type="DATABASE ROLE"): + data = { + "priv": "USAGE", + "on": "GREAT_BAY", + "on_type": "DATABASE", + "to": to, + "to_type": to_type, + "items_type": None, + "grant_option": False, + "grant_type": "OBJECT", + "owner": "SECURITYADMIN", + "_privs": ["USAGE"], + } + urn = parse_URN( + "urn::ABCD123:grant/GRANT?grant_type=OBJECT&priv=USAGE&on=database/GREAT_BAY" + f"&to={to_type.lower().replace(' ', '_')}/{to}" + ) + if cls is DropResource: + return DropResource(urn=urn, before=data) + return CreateResource(urn=urn, resource_cls=res.Grant, container=None, after=data) + + ROLES = [ResourceName("SECURITYADMIN"), ResourceName("TRANSFORMER_DBT")] + + def test_revoke_runs_as_the_database_owner(self): + role, _ = execution_strategy_for_change( + self._change(DropResource), self.ROLES, ResourceName("SECURITYADMIN"), None, self.OWNERS + ) + + assert role == ResourceName("TRANSFORMER_DBT") + + def test_grant_and_revoke_agree_on_the_role(self): + grant_role, _ = execution_strategy_for_change( + self._change(CreateResource), self.ROLES, ResourceName("SECURITYADMIN"), None, self.OWNERS + ) + revoke_role, _ = execution_strategy_for_change( + self._change(DropResource), self.ROLES, ResourceName("SECURITYADMIN"), None, self.OWNERS + ) + + assert grant_role == revoke_role == ResourceName("TRANSFORMER_DBT") + + def test_grants_to_account_roles_still_use_securityadmin(self): + """Account-level authority does reach an account role, so nothing changes there.""" + role, _ = execution_strategy_for_change( + self._change(DropResource, to="ANALYST", to_type="ROLE"), + self.ROLES, + ResourceName("SECURITYADMIN"), + None, + self.OWNERS, + ) + + assert role == ResourceName("SECURITYADMIN") + + def test_falls_back_when_the_database_owner_is_not_available(self): + role, _ = execution_strategy_for_change( + self._change(DropResource), + [ResourceName("SECURITYADMIN")], + ResourceName("SECURITYADMIN"), + None, + self.OWNERS, + ) + + assert role == ResourceName("SECURITYADMIN") + + def test_falls_back_when_the_database_is_unknown(self): + role, _ = execution_strategy_for_change( + self._change(DropResource), self.ROLES, ResourceName("SECURITYADMIN"), None, {"OTHER_DB": "SYSADMIN"} + ) + + assert role == ResourceName("SECURITYADMIN") + + def test_no_owner_map_leaves_behaviour_unchanged(self): + role, _ = execution_strategy_for_change(self._change(DropResource), self.ROLES, ResourceName("SECURITYADMIN")) + + assert role == ResourceName("SECURITYADMIN") + + +class TestSurvivingDropsAreReported: + """Snowflake does not always fail a statement it could not carry out -- REVOKE reports + success when the executing role does not own the privilege or cannot resolve the + grantee. The apply sees no exception and counts the drop as applied, so the grant + survives and the same drop returns in every later plan with nothing explaining why.""" + + def _drop(self, to="ANALYST"): + return DropResource( + urn=parse_URN(f"urn::ABCD123:grant/GRANT?grant_type=OBJECT&priv=USAGE&on=database/GREAT_BAY&to=role/{to}"), + before={ + "priv": "USAGE", + "on": "GREAT_BAY", + "on_type": "DATABASE", + "to": to, + "to_type": "ROLE", + "items_type": None, + "grant_option": False, + "grant_type": "OBJECT", + "owner": "SECURITYADMIN", + "_privs": ["USAGE"], + }, + ) + + @patch("snowcap.blueprint.reset_cache") + @patch("snowcap.blueprint.data_provider.fetch_resource") + def test_a_drop_whose_resource_is_still_there_is_reported(self, mock_fetch, _mock_reset): + from snowcap.blueprint import surviving_drops + + mock_fetch.return_value = {"priv": "USAGE"} # still present after the revoke + change = self._drop() + + assert surviving_drops(MagicMock(), [change]) == [change] + + @patch("snowcap.blueprint.reset_cache") + @patch("snowcap.blueprint.data_provider.fetch_resource") + def test_a_drop_that_took_effect_is_not_reported(self, mock_fetch, _mock_reset): + from snowcap.blueprint import surviving_drops + + mock_fetch.return_value = None + + assert surviving_drops(MagicMock(), [self._drop()]) == [] + + @patch("snowcap.blueprint.data_provider.reset_account_usage_caches") + @patch("snowcap.blueprint.reset_cache") + @patch("snowcap.blueprint.data_provider.fetch_resource") + def test_state_is_re_read_rather_than_served_from_the_apply_s_cache(self, mock_fetch, mock_reset, mock_au_reset): + """The apply just changed the state these checks read. Both the general cache and the + ACCOUNT_USAGE grant snapshot must be cleared, or revoked account-role grants re-appear + as false survivors on use_account_usage runs.""" + from snowcap.blueprint import surviving_drops + + mock_fetch.return_value = None + surviving_drops(MagicMock(), [self._drop()]) + + mock_reset.assert_called_once() + mock_au_reset.assert_called_once() + + @patch("snowcap.blueprint.reset_cache") + @patch("snowcap.blueprint.data_provider.fetch_resource") + def test_a_resource_that_cannot_be_read_back_is_not_reported_as_surviving(self, mock_fetch, _mock_reset): + """Not being able to confirm a drop is not evidence that it failed.""" + from snowcap.blueprint import surviving_drops + + mock_fetch.side_effect = Exception("no fetch function for this resource type") + + assert surviving_drops(MagicMock(), [self._drop()]) == [] + + @patch("snowcap.blueprint.reset_cache") + @patch("snowcap.blueprint.data_provider.fetch_resource") + def test_nothing_is_read_back_when_the_plan_dropped_nothing(self, mock_fetch, mock_reset): + """Applies that only create must not pay for this.""" + from snowcap.blueprint import surviving_drops + + change = CreateResource( + urn=parse_URN("urn::ABCD123:role/SOME_ROLE"), + resource_cls=res.Role, + container=None, + after={"name": "SOME_ROLE", "owner": "USERADMIN"}, + ) + + assert surviving_drops(MagicMock(), [change]) == [] + mock_fetch.assert_not_called() + mock_reset.assert_not_called() + + def test_report_names_each_survivor(self, capsys): + from snowcap.blueprint import print_surviving_drops + + print_surviving_drops([self._drop()]) + out = capsys.readouterr().out + + assert "1 drop(s) reported success" in out + assert "USAGE on DATABASE.GREAT_BAY \u2192 ROLE.ANALYST" in out + + def test_report_guides_on_database_role_grantees(self, capsys): + """A survivor held by a database role gets the specific remedy: grant the role that + owns its database, since SECURITYADMIN cannot resolve the grantee.""" + from snowcap.blueprint import print_surviving_drops + + survivor = DropResource( + urn=parse_URN( + "urn::ABCD123:grant/GRANT?grant_type=OBJECT&priv=USAGE" + "&on=database/GREAT_BAY&to=database_role/GREAT_BAY.DR" + ), + before={ + "priv": "USAGE", + "on": "GREAT_BAY", + "on_type": "DATABASE", + "to": "GREAT_BAY.DR", + "to_type": "DATABASE ROLE", + "items_type": None, + "grant_option": False, + "grant_type": "OBJECT", + "owner": "SECURITYADMIN", + "_privs": ["USAGE"], + }, + ) + + print_surviving_drops([survivor]) + out = capsys.readouterr().out + + assert "database roles" in out + assert "GREAT_BAY" in out + + def test_report_is_silent_when_every_drop_took_effect(self, capsys): + from snowcap.blueprint import print_surviving_drops + + print_surviving_drops([]) + + assert capsys.readouterr().out == "" + + +class TestSyncReadsFutureGrantsRegardless: + """Syncing a resource type means removing what config does not declare, so a future + grant absent from config is exactly what has to be found. Skipping the SHOW FUTURE + GRANTS query when the manifest declared none kept the ones already in Snowflake out of + remote state, so sync could not propose dropping them -- unseen rather than kept, with + nothing in the plan to say so. + + Migrating from ALL plus FUTURE pairs to inherited grants removes the last future grant + from config, which is precisely when this bites.""" + + def _grant_list_kwargs(self, resources): + """How fetch_remote_state asks for grants, for a config with no future grants. + + Only the listing call matters here, and it happens before the rest of + fetch_remote_state; the later failure is mock plumbing for reference resolution, + not the behaviour under test. + """ + from snowcap.blueprint_config import BlueprintConfig + + bp = Blueprint(resources=resources) + bp._config = BlueprintConfig(sync_resources={ResourceType.GRANT}) + + with ( + patch("snowcap.blueprint.data_provider.fetch_session") as mock_session, + patch("snowcap.blueprint.data_provider.use_secondary_roles"), + patch("snowcap.blueprint.data_provider.list_resource") as mock_list, + ): + mock_session.return_value = self.SESSION_CTX + mock_list.return_value = [] + manifest = bp.generate_manifest(self.SESSION_CTX) + try: + bp.fetch_remote_state(MagicMock(), manifest) + except Exception: + pass + grant_calls = [c for c in mock_list.call_args_list if c.args[1] == "grant"] + + assert grant_calls, "grants must be listed when grant is a sync_resource" + return grant_calls[0].kwargs + + @pytest.fixture(autouse=True) + def _ctx(self, session_ctx): + type(self).SESSION_CTX = session_ctx + + def test_future_grants_are_listed_when_config_declares_none(self): + kwargs = self._grant_list_kwargs([res.Role(name="SOME_ROLE")]) + + assert kwargs["include_future_grants"] is True + + def test_the_query_is_not_narrowed_to_roles_named_in_config(self): + """A role holding a future grant only in Snowflake was never queried, so its grant + could not be dropped either.""" + kwargs = self._grant_list_kwargs([res.Role(name="SOME_ROLE")]) + + assert "future_grant_roles" not in kwargs + assert "future_grant_database_roles" not in kwargs diff --git a/tests/test_blueprint_ownership.py b/tests/test_blueprint_ownership.py index eb4788a2..ac4a4841 100644 --- a/tests/test_blueprint_ownership.py +++ b/tests/test_blueprint_ownership.py @@ -9,6 +9,7 @@ UpdateResource, compile_plan_to_sql, diff, + execution_strategy_for_change, ) from snowcap.enums import AccountEdition from snowcap.identifiers import parse_URN @@ -349,3 +350,45 @@ def test_database_with_custom_owner_modifies_public_schema_owner(session_ctx, re assert any(cmd.startswith("CREATE DATABASE SOME_DATABASE") for cmd in sql_commands) assert "GRANT OWNERSHIP ON DATABASE SOME_DATABASE TO ROLE CUSTOM_ROLE COPY CURRENT GRANTS" in sql_commands assert "GRANT OWNERSHIP ON SCHEMA SOME_DATABASE.PUBLIC TO ROLE CUSTOM_ROLE COPY CURRENT GRANTS" in sql_commands + + +class TestOwnerExecutedOwnershipTransfer: + """ + Tests for which role Snowcap uses to transfer ownership of owner-executed objects. + + Owner-executed objects (views, tasks, procedures, Streamlit apps, and so on) run with + the privileges of their owner. Snowflake requires the caller to either hold + account-level MANAGE GRANTS or have the receiving role in their active role hierarchy + before it will transfer one. Snowcap's usual strategy -- run the transfer as the + outgoing owner -- satisfies neither condition in the common case. + """ + + def _transfer(self, urn_str, resource_cls): + return TransferOwnership( + urn=parse_URN(urn_str), + resource_cls=resource_cls, + from_owner="SYSADMIN", + to_owner="SOME_ROLE", + ) + + def test_owner_executed_transfer_uses_securityadmin(self, session_ctx): + change = self._transfer("urn::ABCD123:view/MY_DB.MY_SCHEMA.MY_VIEW", res.View) + + role, transfer = execution_strategy_for_change(change, session_ctx["available_roles"], ResourceName("SYSADMIN")) + + assert role == ResourceName("SECURITYADMIN") + assert transfer is False + + def test_regular_object_transfer_still_runs_as_the_outgoing_owner(self, session_ctx): + change = self._transfer("urn::ABCD123:table/MY_DB.MY_SCHEMA.MY_TABLE", res.Table) + + role, _ = execution_strategy_for_change(change, session_ctx["available_roles"], ResourceName("SYSADMIN")) + + assert role == ResourceName("SYSADMIN") + + def test_owner_executed_transfer_falls_back_when_securityadmin_is_unavailable(self, session_ctx): + change = self._transfer("urn::ABCD123:task/MY_DB.MY_SCHEMA.MY_TASK", res.Task) + + role, _ = execution_strategy_for_change(change, ["SYSADMIN", "PUBLIC"], ResourceName("SYSADMIN")) + + assert role == ResourceName("SYSADMIN") diff --git a/tests/test_client.py b/tests/test_client.py index 2f8782b7..d2d7fbff 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -324,11 +324,7 @@ def test_execute_empty_response_code_returns_empty_list(self): mock_connection.user = "testuser" mock_connection.role = "testrole" - result = execute( - mock_connection, - "SELECT * FROM missing", - empty_response_codes=[DOES_NOT_EXIST_ERR] - ) + result = execute(mock_connection, "SELECT * FROM missing", empty_response_codes=[DOES_NOT_EXIST_ERR]) assert result == [] @@ -344,12 +340,7 @@ def test_execute_empty_response_code_caches_empty_result(self): mock_connection.user = "testuser" mock_connection.role = "TESTROLE" - execute( - mock_connection, - "SELECT * FROM missing", - cacheable=True, - empty_response_codes=[DOES_NOT_EXIST_ERR] - ) + execute(mock_connection, "SELECT * FROM missing", cacheable=True, empty_response_codes=[DOES_NOT_EXIST_ERR]) assert snowcap.client._EXECUTION_CACHE["TESTROLE"]["SELECT * FROM missing"] == [] @@ -473,6 +464,7 @@ def test_execute_in_parallel_error_handler_called(self): mock_connection.role = "testrole" errors = [] + def error_handler(err, sql): errors.append((err, sql)) diff --git a/tests/test_connector.py b/tests/test_connector.py index 63687055..e4a0b128 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -31,7 +31,6 @@ UNLIMITED, ) - # ============================================================================= # Test Exception Classes # ============================================================================= @@ -462,9 +461,7 @@ def test_connect_with_session_and_master_token(self, mock_get_env, mock_connect) mock_conn = MagicMock() mock_connect.return_value = mock_conn - result = connect( - account="test", user="user", session_token="session123", master_token="master123" - ) + result = connect(account="test", user="user", session_token="session123", master_token="master123") call_kwargs = mock_connect.call_args[1] assert call_kwargs["server_session_keep_alive"] is True diff --git a/tests/test_data_provider.py b/tests/test_data_provider.py index 5876acef..6949a2d4 100644 --- a/tests/test_data_provider.py +++ b/tests/test_data_provider.py @@ -50,7 +50,7 @@ ) from snowcap.identifiers import FQN, URN from snowcap.resource_name import ResourceName -from snowcap.enums import ResourceType +from snowcap.enums import GrantType, ResourceType from snowcap import resources as res from snowcap.resources.warehouse import ADAPTIVE_UNSUPPORTED_FIELDS @@ -96,37 +96,23 @@ def test_simple_owner(self): def test_database_role_owner(self): # Lowercase names from Snowflake metadata get quoted - data = { - "owner": "my_role", - "owner_role_type": "DATABASE_ROLE", - "database_name": "my_db" - } + data = {"owner": "my_role", "owner_role_type": "DATABASE_ROLE", "database_name": "my_db"} result = _get_owner_identifier(data) assert result == '"my_db"."my_role"' def test_database_role_owner_uppercase(self): # Uppercase names pass through without quotes - data = { - "owner": "MY_ROLE", - "owner_role_type": "DATABASE_ROLE", - "database_name": "MY_DB" - } + data = {"owner": "MY_ROLE", "owner_role_type": "DATABASE_ROLE", "database_name": "MY_DB"} result = _get_owner_identifier(data) assert result == "MY_DB.MY_ROLE" def test_role_type_owner(self): - data = { - "owner": "SYSADMIN", - "owner_role_type": "ROLE" - } + data = {"owner": "SYSADMIN", "owner_role_type": "ROLE"} result = _get_owner_identifier(data) assert result == "SYSADMIN" def test_empty_owner_with_role_type(self): - data = { - "owner": "", - "owner_role_type": "ROLE" - } + data = {"owner": "", "owner_role_type": "ROLE"} result = _get_owner_identifier(data) assert result == "" @@ -141,11 +127,7 @@ def test_empty_owner_without_role_type(self): assert result == "" def test_unsupported_owner_role_type_raises(self): - data = { - "owner": "my_role", - "owner_role_type": "UNKNOWN_TYPE", - "database_name": "my_db" - } + data = {"owner": "my_role", "owner_role_type": "UNKNOWN_TYPE", "database_name": "my_db"} with pytest.raises(Exception, match="Unsupported owner role type"): _get_owner_identifier(data) @@ -178,65 +160,47 @@ class TestDescType2ResultToDict: """Tests for _desc_type2_result_to_dict helper function.""" def test_boolean_property(self): - desc_result = [ - {"property": "ENABLED", "property_value": "true", "property_type": "Boolean"} - ] + desc_result = [{"property": "ENABLED", "property_value": "true", "property_type": "Boolean"}] result = _desc_type2_result_to_dict(desc_result) assert result["ENABLED"] is True def test_boolean_false(self): - desc_result = [ - {"property": "ENABLED", "property_value": "false", "property_type": "Boolean"} - ] + desc_result = [{"property": "ENABLED", "property_value": "false", "property_type": "Boolean"}] result = _desc_type2_result_to_dict(desc_result) assert result["ENABLED"] is False def test_long_property(self): - desc_result = [ - {"property": "SIZE", "property_value": "1024", "property_type": "Long"} - ] + desc_result = [{"property": "SIZE", "property_value": "1024", "property_type": "Long"}] result = _desc_type2_result_to_dict(desc_result) assert result["SIZE"] == "1024" def test_long_empty_value(self): - desc_result = [ - {"property": "SIZE", "property_value": "", "property_type": "Long"} - ] + desc_result = [{"property": "SIZE", "property_value": "", "property_type": "Long"}] result = _desc_type2_result_to_dict(desc_result) assert result["SIZE"] is None def test_integer_property(self): - desc_result = [ - {"property": "COUNT", "property_value": "42", "property_type": "Integer"} - ] + desc_result = [{"property": "COUNT", "property_value": "42", "property_type": "Integer"}] result = _desc_type2_result_to_dict(desc_result) assert result["COUNT"] == 42 def test_string_property(self): - desc_result = [ - {"property": "NAME", "property_value": "my_name", "property_type": "String"} - ] + desc_result = [{"property": "NAME", "property_value": "my_name", "property_type": "String"}] result = _desc_type2_result_to_dict(desc_result) assert result["NAME"] == "my_name" def test_string_empty_value(self): - desc_result = [ - {"property": "NAME", "property_value": "", "property_type": "String"} - ] + desc_result = [{"property": "NAME", "property_value": "", "property_type": "String"}] result = _desc_type2_result_to_dict(desc_result) assert result["NAME"] is None def test_list_property(self): - desc_result = [ - {"property": "ROLES", "property_value": "[role1, role2]", "property_type": "List"} - ] + desc_result = [{"property": "ROLES", "property_value": "[role1, role2]", "property_type": "List"}] result = _desc_type2_result_to_dict(desc_result) assert result["ROLES"] == ["role1", "role2"] def test_object_property(self): - desc_result = [ - {"property": "CONFIG", "property_value": "[a, b, c]", "property_type": "Object"} - ] + desc_result = [{"property": "CONFIG", "property_value": "[a, b, c]", "property_type": "Object"}] result = _desc_type2_result_to_dict(desc_result) assert result["CONFIG"] == ["a", "b", "c"] @@ -245,9 +209,7 @@ class TestDescType3ResultToDict: """Tests for _desc_type3_result_to_dict helper function.""" def test_flat_property(self): - desc_result = [ - {"parent_property": "", "property": "NAME", "property_value": "test", "property_type": "String"} - ] + desc_result = [{"parent_property": "", "property": "NAME", "property_value": "test", "property_type": "String"}] result = _desc_type3_result_to_dict(desc_result) assert result["NAME"] == "test" @@ -752,11 +714,7 @@ def test_dispatches_to_correct_fetch_function(self, mock_fetch_database): mock_fetch_database.return_value = {"name": "MY_DB"} mock_session = MagicMock() - urn = URN( - resource_type=ResourceType.DATABASE, - account_locator="ABC123", - fqn=FQN(name=ResourceName("MY_DB")) - ) + urn = URN(resource_type=ResourceType.DATABASE, account_locator="ABC123", fqn=FQN(name=ResourceName("MY_DB"))) result = fetch_resource(mock_session, urn) @@ -771,7 +729,7 @@ def test_dispatches_to_schema_fetch(self, mock_fetch_schema): urn = URN( resource_type=ResourceType.SCHEMA, account_locator="ABC123", - fqn=FQN(database=ResourceName("MY_DB"), name=ResourceName("MY_SCHEMA")) + fqn=FQN(database=ResourceName("MY_DB"), name=ResourceName("MY_SCHEMA")), ) result = fetch_resource(mock_session, urn) @@ -782,14 +740,11 @@ def test_dispatches_to_schema_fetch(self, mock_fetch_schema): @patch("snowcap.data_provider.fetch_role") def test_returns_none_on_does_not_exist_error(self, mock_fetch_role): from snowflake.connector.errors import ProgrammingError + mock_fetch_role.side_effect = ProgrammingError(errno=2003) mock_session = MagicMock() - urn = URN( - resource_type=ResourceType.ROLE, - account_locator="ABC123", - fqn=FQN(name=ResourceName("MISSING_ROLE")) - ) + urn = URN(resource_type=ResourceType.ROLE, account_locator="ABC123", fqn=FQN(name=ResourceName("MISSING_ROLE"))) result = fetch_resource(mock_session, urn) assert result is None @@ -797,14 +752,11 @@ def test_returns_none_on_does_not_exist_error(self, mock_fetch_role): @patch("snowcap.data_provider.fetch_role") def test_raises_other_programming_errors(self, mock_fetch_role): from snowflake.connector.errors import ProgrammingError + mock_fetch_role.side_effect = ProgrammingError(errno=1234) mock_session = MagicMock() - urn = URN( - resource_type=ResourceType.ROLE, - account_locator="ABC123", - fqn=FQN(name=ResourceName("MY_ROLE")) - ) + urn = URN(resource_type=ResourceType.ROLE, account_locator="ABC123", fqn=FQN(name=ResourceName("MY_ROLE"))) with pytest.raises(ProgrammingError): fetch_resource(mock_session, urn) @@ -1487,6 +1439,7 @@ class TestHasAccountUsageAccess: def test_returns_true_when_access_granted(self, mock_execute): """When ACCOUNT_USAGE query succeeds, function returns True.""" from snowcap.data_provider import _has_account_usage_access + mock_execute.return_value = [{"1": 1}] # Query succeeds mock_session = MagicMock() @@ -1526,6 +1479,7 @@ def test_raises_on_other_programming_errors(self, mock_execute): def test_caches_result_per_session(self, mock_execute): """Result should be cached per session to avoid repeated queries.""" from snowcap.data_provider import _has_account_usage_access + mock_execute.return_value = [{"1": 1}] mock_session = MagicMock() @@ -1543,6 +1497,7 @@ def test_caches_result_per_session(self, mock_execute): def test_different_sessions_have_independent_cache(self, mock_execute): """Different sessions should have independent cache entries.""" from snowcap.data_provider import _has_account_usage_access + mock_execute.return_value = [{"1": 1}] session1 = MagicMock() session2 = MagicMock() @@ -1709,6 +1664,7 @@ class TestShouldUseAccountUsage: def test_returns_false_when_config_disabled(self, mock_has_access): """Returns False when use_account_usage config is False.""" from snowcap.data_provider import _should_use_account_usage + mock_session = MagicMock() result = _should_use_account_usage(mock_session, use_account_usage=False) @@ -1724,6 +1680,7 @@ def test_returns_false_when_fallback_cached(self, mock_has_access): _should_use_account_usage, _ACCOUNT_USAGE_FALLBACK_CACHE, ) + mock_session = MagicMock() _ACCOUNT_USAGE_FALLBACK_CACHE[id(mock_session)] = True @@ -1737,6 +1694,7 @@ def test_returns_false_when_fallback_cached(self, mock_has_access): def test_returns_access_check_result_when_enabled(self, mock_has_access): """Returns result of _has_account_usage_access when config is enabled.""" from snowcap.data_provider import _should_use_account_usage + mock_session = MagicMock() mock_has_access.return_value = True @@ -1749,6 +1707,7 @@ def test_returns_access_check_result_when_enabled(self, mock_has_access): def test_returns_false_when_no_access(self, mock_has_access): """Returns False when session doesn't have ACCOUNT_USAGE access.""" from snowcap.data_provider import _should_use_account_usage + mock_session = MagicMock() mock_has_access.return_value = False @@ -1766,6 +1725,7 @@ def test_marks_session_for_fallback(self): _mark_account_usage_fallback, _ACCOUNT_USAGE_FALLBACK_CACHE, ) + mock_session = MagicMock() _mark_account_usage_fallback(mock_session) @@ -1779,9 +1739,7 @@ class TestFetchRolePrivilegesAccountUsage: @patch("snowcap.data_provider._should_use_account_usage") @patch("snowcap.data_provider._fetch_grants_from_account_usage") @patch("snowcap.data_provider._show_grants_to_role") - def test_uses_account_usage_when_enabled_and_available( - self, mock_show_grants, mock_fetch_au, mock_should_use - ): + def test_uses_account_usage_when_enabled_and_available(self, mock_show_grants, mock_fetch_au, mock_should_use): """When ACCOUNT_USAGE is enabled and available, uses ACCOUNT_USAGE.""" from snowcap.data_provider import fetch_role_privileges from datetime import datetime @@ -1828,9 +1786,7 @@ def test_falls_back_to_show_when_disabled(self, mock_show_grants, mock_should_us @patch("snowcap.data_provider._should_use_account_usage") @patch("snowcap.data_provider._fetch_grants_from_account_usage") @patch("snowcap.data_provider._show_grants_to_role") - def test_falls_back_when_account_usage_returns_none( - self, mock_show_grants, mock_fetch_au, mock_should_use - ): + def test_falls_back_when_account_usage_returns_none(self, mock_show_grants, mock_fetch_au, mock_should_use): """When ACCOUNT_USAGE query fails (returns None), falls back to SHOW.""" from snowcap.data_provider import fetch_role_privileges @@ -1928,3 +1884,781 @@ def test_missing_streamlit_returns_none(self, mock_show_resources): mock_show_resources.return_value = [] assert fetch_streamlit(MagicMock(), FQN(name=ResourceName("MY_APP"))) is None + + +class TestInheritedGrants: + """ + Tests for how Snowcap reads inherited grants (GRANT INHERITED ...) from remote state. + + Inherited grants are container-level grants that apply to every current and future + object of a type in a container. Snowflake reports them in SHOW GRANTS and + ACCOUNT_USAGE.GRANTS_TO_ROLES with IS_INHERITED set, an empty NAME, and the container + in the INHERITED_FROM columns. They are kept apart from object grants throughout, since + an inherited grant is revoked with REVOKE INHERITED against its container rather than + with a per-object REVOKE. + """ + + def _inherited_row(self, **overrides): + row = { + "privilege": "SELECT", + "granted_on": "TABLE", + "name": "", + "granted_to": "ROLE", + "grantee_name": "MY_ROLE", + "grant_option": "false", + "granted_by": "SECURITYADMIN", + "is_inherited": "true", + "inherited_from": "DATABASE", + "inherited_from_database": "MY_DB", + "inherited_from_schema": "", + } + row.update(overrides) + return row + + def _regular_row(self, **overrides): + row = { + "privilege": "SELECT", + "granted_on": "TABLE", + "name": "MY_DB.MY_SCHEMA.MY_TABLE", + "granted_to": "ROLE", + "grantee_name": "MY_ROLE", + "grant_option": "false", + "granted_by": "SYSADMIN", + "is_inherited": "false", + } + row.update(overrides) + return row + + @pytest.mark.parametrize( + "row,expected", + [ + ({"is_inherited": "true"}, True), + ({"is_inherited": "TRUE"}, True), + ({"is_inherited": True}, True), + ({"IS_INHERITED": True}, True), + ({"IS_INHERITED": "true"}, True), + ({"is_inherited": "false"}, False), + ({"is_inherited": False}, False), + ({"is_inherited": None}, False), + ({"IS_INHERITED": None}, False), + # Accounts without the preview, and Snowflake versions without the column, + # simply do not return it. + ({}, False), + ], + ) + def test_is_inherited_grant_reads_both_casings_and_shapes(self, row, expected): + from snowcap.data_provider import _is_inherited_grant + + assert _is_inherited_grant(row) is expected + + def test_drop_inherited_grants_keeps_regular_grants(self): + from snowcap.data_provider import _drop_inherited_grants + + rows = [self._regular_row(), self._inherited_row(), self._regular_row(privilege="INSERT")] + + kept = _drop_inherited_grants(rows, "test") + + assert len(kept) == 2 + assert all(row["name"] for row in kept) + + def test_drop_inherited_grants_is_a_noop_without_the_column(self): + from snowcap.data_provider import _drop_inherited_grants + + rows = [{"privilege": "SELECT", "granted_on": "TABLE", "name": "MY_DB.MY_SCHEMA.MY_TABLE"}] + + assert _drop_inherited_grants(rows, "test") == rows + + @patch("snowcap.data_provider.execute") + def test_show_grants_to_role_filters_inherited_rows(self, mock_execute): + from snowcap.data_provider import _show_grants_to_role + + mock_execute.return_value = [self._regular_row(), self._inherited_row()] + + grants = _show_grants_to_role(MagicMock(), ResourceName("MY_ROLE")) + + assert len(grants) == 1 + assert grants[0]["name"] == "MY_DB.MY_SCHEMA.MY_TABLE" + + def _account_usage_rows(self): + from datetime import datetime + + return [ + { + "CREATED_ON": datetime(2024, 1, 1, 12, 0, 0), + "PRIVILEGE": "SELECT", + "GRANTED_ON": "TABLE", + "NAME": "MY_TABLE", + "TABLE_CATALOG": "MY_DB", + "TABLE_SCHEMA": "MY_SCHEMA", + "GRANTED_TO": "ACCOUNT ROLE", + "GRANTEE_NAME": "MY_ROLE", + "GRANT_OPTION": False, + "GRANTED_BY": "SYSADMIN", + "IS_INHERITED": False, + "INHERITED_FROM": None, + "INHERITED_FROM_DATABASE": None, + "INHERITED_FROM_SCHEMA": None, + }, + { + "CREATED_ON": datetime(2024, 1, 1, 12, 0, 0), + "PRIVILEGE": "SELECT", + "GRANTED_ON": "TABLE", + "NAME": None, + "TABLE_CATALOG": None, + "TABLE_SCHEMA": None, + "GRANTED_TO": "ACCOUNT ROLE", + "GRANTEE_NAME": "MY_ROLE", + "GRANT_OPTION": False, + "GRANTED_BY": "SECURITYADMIN", + "IS_INHERITED": True, + "INHERITED_FROM": "DATABASE", + "INHERITED_FROM_DATABASE": "MY_DB", + "INHERITED_FROM_SCHEMA": None, + }, + ] + + @patch("snowcap.data_provider.execute") + def test_account_usage_query_requests_the_container_columns(self, mock_execute): + from snowcap.data_provider import _fetch_grants_from_account_usage + + mock_execute.return_value = [] + + _fetch_grants_from_account_usage(MagicMock()) + + query = mock_execute.call_args[0][1] + for column in ("IS_INHERITED", "INHERITED_FROM", "INHERITED_FROM_DATABASE", "INHERITED_FROM_SCHEMA"): + assert column in query + + @patch("snowcap.data_provider.execute") + def test_account_usage_keeps_inherited_rows_tagged(self, mock_execute): + """Both kinds are cached together so a role's grants are fetched once; the split + happens at read time.""" + from snowcap.data_provider import _fetch_grants_from_account_usage + + mock_execute.return_value = self._account_usage_rows() + + result = _fetch_grants_from_account_usage(MagicMock()) + + assert result is not None + assert len(result) == 2 + regular, inherited = result + assert regular["name"] == "MY_DB.MY_SCHEMA.MY_TABLE" + assert regular["is_inherited"] is False + assert inherited["is_inherited"] is True + assert inherited["inherited_from"] == "DATABASE" + assert inherited["inherited_from_database"] == "MY_DB" + + @patch("snowcap.data_provider.execute") + def test_object_and_inherited_readers_split_the_same_rows(self, mock_execute): + from snowcap.data_provider import _show_grants_to_role, _show_inherited_grants_to_role + + mock_execute.return_value = [self._regular_row(), self._inherited_row()] + session = MagicMock() + + object_grants = _show_grants_to_role(session, ResourceName("MY_ROLE")) + inherited_grants = _show_inherited_grants_to_role(session, ResourceName("MY_ROLE")) + + assert [g["name"] for g in object_grants] == ["MY_DB.MY_SCHEMA.MY_TABLE"] + assert [g["inherited_from_database"] for g in inherited_grants] == ["MY_DB"] + + @patch("snowcap.data_provider.execute") + def test_account_usage_retries_without_the_column_when_unsupported(self, mock_execute): + """Snowflake versions without IS_INHERITED must not push the whole session onto the + slower per-role SHOW GRANTS path.""" + from snowflake.connector.errors import ProgrammingError + + from snowcap.client import INVALID_COLUMN_ERR + from snowcap.data_provider import _fetch_grants_from_account_usage + + mock_execute.side_effect = [ProgrammingError(errno=INVALID_COLUMN_ERR), []] + + result = _fetch_grants_from_account_usage(MagicMock()) + + assert result == [] + assert mock_execute.call_count == 2 + assert "IS_INHERITED" in mock_execute.call_args_list[0][0][1] + assert "IS_INHERITED" not in mock_execute.call_args_list[1][0][1] + + @patch("snowcap.data_provider.execute") + def test_account_usage_still_falls_back_on_access_denied(self, mock_execute): + from snowflake.connector.errors import ProgrammingError + + from snowcap.client import ACCESS_CONTROL_ERR + from snowcap.data_provider import _fetch_grants_from_account_usage + + mock_execute.side_effect = ProgrammingError(errno=ACCESS_CONTROL_ERR) + + assert _fetch_grants_from_account_usage(MagicMock()) is None + assert mock_execute.call_count == 1 + + @pytest.mark.parametrize( + "row_overrides,expected_on", + [ + ( + {"inherited_from": "ACCOUNT", "inherited_from_database": "", "inherited_from_schema": ""}, + "account/ACCOUNT.
", + ), + ( + {"inherited_from": "DATABASE", "inherited_from_database": "MY_DB", "inherited_from_schema": ""}, + "database/MY_DB.
", + ), + ( + {"inherited_from": "SCHEMA", "inherited_from_database": "MY_DB", "inherited_from_schema": "MY_SCHEMA"}, + "schema/MY_DB.MY_SCHEMA.
", + ), + ], + ) + def test_inherited_grant_fqn_encodes_each_container(self, row_overrides, expected_on): + from snowcap.data_provider import inherited_grant_fqn + + fqn = inherited_grant_fqn(self._inherited_row(**row_overrides), "role", "MY_ROLE") + + assert fqn is not None + assert fqn.params["grant_type"] == "INHERITED" + assert fqn.params["on"] == expected_on + assert fqn.params["to"] == "role/MY_ROLE" + + def test_inherited_grant_fqn_normalizes_synonym_object_types(self): + """SHOW GRANTS reports CORTEX_AGENT_SERVER for what GRANT/CREATE call an MCP SERVER. + The fetched URN must use the canonical MCP_SERVER type so it matches the declared + grant instead of forcing a non-converging DROP+CREATE that revokes inherited access.""" + from snowcap.data_provider import inherited_grant_fqn + + fqn = inherited_grant_fqn( + self._inherited_row(granted_on="CORTEX_AGENT_SERVER", inherited_from="SCHEMA", inherited_from_schema="SCH"), + "role", + "MY_ROLE", + ) + + assert fqn is not None + assert fqn.params["on"] == "schema/MY_DB.SCH." + + def test_normalize_future_grant_name_maps_synonym_object_types(self): + """A SHOW FUTURE GRANTS name embeds ; a synonym (CORTEX_AGENT_SERVER) + must map to the manifest's canonical MCP_SERVER so the future grant converges instead + of being dropped and re-created each sync. Non-synonym and unknown types are untouched.""" + from snowcap.data_provider import _normalize_future_grant_name + + assert _normalize_future_grant_name("RAW.") == "RAW." + assert _normalize_future_grant_name("DB.SCH.
") == "DB.SCH.
" + assert _normalize_future_grant_name("DB.SCH.") == "DB.SCH." + assert _normalize_future_grant_name("DB.SCH.") == "DB.SCH." + + def test_inherited_grant_fqn_skips_unrecognized_containers(self): + """An unknown container would produce a grant Snowcap could not revoke, so it is + left alone rather than guessed at.""" + from snowcap.data_provider import inherited_grant_fqn + + assert inherited_grant_fqn(self._inherited_row(inherited_from="SOMETHING_NEW"), "role", "MY_ROLE") is None + + @patch("snowcap.data_provider.list_database_roles") + @patch("snowcap.data_provider._should_use_account_usage") + @patch("snowcap.data_provider.execute") + def test_list_grants_reports_inherited_grants_as_inherited( + self, mock_execute, mock_should_use, mock_list_database_roles + ): + """A grant sync run must never treat an inherited grant as an object grant: there is + no object name to revoke it on, and it can only be removed with REVOKE INHERITED.""" + from snowcap.data_provider import list_grants + + mock_should_use.return_value = False + mock_list_database_roles.return_value = [] + + def execute_side_effect(session, query, **kwargs): + if "SHOW DATABASES" in query: + return [] + if "SHOW ROLES" in query: + return [{"name": "MY_ROLE"}] + return [self._regular_row(), self._inherited_row()] + + mock_execute.side_effect = execute_side_effect + + grants = list_grants(MagicMock(), include_future_grants=False) + + by_type = {fqn.params["grant_type"]: fqn for fqn in grants} + assert set(by_type) == {"OBJECT", "INHERITED"} + assert by_type["OBJECT"].params["on"] == "table/MY_DB.MY_SCHEMA.MY_TABLE" + assert by_type["INHERITED"].params["on"] == "database/MY_DB.
" + assert by_type["INHERITED"].params["priv"] == "SELECT" + + @patch("snowcap.data_provider.execute") + def test_fetch_inherited_grant_matches_its_container(self, mock_execute): + from snowcap.data_provider import fetch_inherited_grant + from snowcap.identifiers import FQN + + mock_execute.return_value = [self._regular_row(), self._inherited_row()] + fqn = FQN( + name=ResourceName("GRANT"), + params={ + "grant_type": "INHERITED", + "priv": "SELECT", + "on": "database/MY_DB.
", + "to": "role/MY_ROLE", + }, + ) + + data = fetch_inherited_grant(MagicMock(), fqn) + + assert data is not None + assert data["grant_type"] == GrantType.INHERITED + assert data["on"] == "MY_DB" + assert data["on_type"] == "DATABASE" + assert data["items_type"] == "TABLE" + assert data["grant_option"] is False + + @patch("snowcap.data_provider.execute") + def test_fetch_inherited_grant_does_not_match_another_container(self, mock_execute): + from snowcap.data_provider import fetch_inherited_grant + from snowcap.identifiers import FQN + + mock_execute.return_value = [self._inherited_row(inherited_from_database="OTHER_DB")] + fqn = FQN( + name=ResourceName("GRANT"), + params={ + "grant_type": "INHERITED", + "priv": "SELECT", + "on": "database/MY_DB.
", + "to": "role/MY_ROLE", + }, + ) + + assert fetch_inherited_grant(MagicMock(), fqn) is None + + @patch("snowcap.data_provider.execute") + def test_fetch_inherited_grant_reads_the_account_container(self, mock_execute): + from snowcap.data_provider import fetch_inherited_grant + from snowcap.identifiers import FQN + + mock_execute.return_value = [ + self._inherited_row(inherited_from="ACCOUNT", inherited_from_database="", inherited_from_schema="") + ] + fqn = FQN( + name=ResourceName("GRANT"), + params={ + "grant_type": "INHERITED", + "priv": "SELECT", + "on": "account/ACCOUNT.
", + "to": "role/MY_ROLE", + }, + ) + + data = fetch_inherited_grant(MagicMock(), fqn) + + assert data is not None + assert data["on"] == "ACCOUNT" + assert data["on_type"] == "ACCOUNT" + + @pytest.mark.parametrize( + "value,expected", + [("ENABLED", True), ("enabled", True), ("DISABLED", False), ("", False)], + ) + @patch("snowcap.data_provider.execute") + def test_feature_flag_probe_reads_the_parameter(self, mock_execute, value, expected): + from snowcap.data_provider import fetch_inherited_grants_enabled + + mock_execute.return_value = [{"key": "FEATURE_RBAC_INHERITED_GRANTS", "value": value}] + + assert fetch_inherited_grants_enabled(MagicMock()) is expected + + @patch("snowcap.data_provider.execute") + def test_feature_flag_probe_is_undetermined_when_unreadable(self, mock_execute): + """The parameter does not exist on older Snowflake versions, and reading account + parameters needs privileges the session may not have. Neither should block a run.""" + from snowcap.data_provider import fetch_inherited_grants_enabled + + mock_execute.side_effect = Exception("Insufficient privileges") + + assert fetch_inherited_grants_enabled(MagicMock()) is None + + @pytest.mark.parametrize( + "status,expected", + [ + ("Preview access is ENABLED for this account", True), + ("Preview access is DISABLED for this account", False), + ("something unexpected", None), + ], + ) + @patch("snowcap.data_provider.execute") + def test_preview_access_status_is_read_from_the_system_function(self, mock_execute, status, expected): + from snowcap.data_provider import fetch_preview_access_enabled + + mock_execute.return_value = [{"STATUS": status}] + + assert fetch_preview_access_enabled(MagicMock()) is expected + + @patch("snowcap.data_provider.execute") + def test_preview_access_status_is_undetermined_when_unreadable(self, mock_execute): + from snowcap.data_provider import fetch_preview_access_enabled + + mock_execute.side_effect = Exception("Insufficient privileges") + + assert fetch_preview_access_enabled(MagicMock()) is None + + +class TestDatabaseRoleGrantsAreNotListedAsGrants: + """Snowflake reports a database role granted to an account role as a USAGE grant held + by the grantee. Snowcap models that as a DatabaseRoleGrant, so listing it as a Grant + too describes one Snowflake fact under two resource types: the declared + DatabaseRoleGrant never matches, sync proposes dropping the stray Grant on every run, + and the revoke it builds -- REVOKE USAGE ON DATABASE ROLE -- is rejected by Snowflake + as an unsupported feature, which aborts the apply.""" + + def _row(self, **overrides): + row = { + "privilege": "USAGE", + "granted_on": "DATABASE_ROLE", + "name": "GREAT_BAY_DEV.DR_READER_ROLE", + "granted_to": "ROLE", + "grantee_name": "MY_ROLE", + "grant_option": "false", + "granted_by": "SECURITYADMIN", + "is_inherited": "false", + } + row.update(overrides) + return row + + @pytest.mark.parametrize( + "granted_on,expected", + [ + ("DATABASE_ROLE", True), + ("DATABASE ROLE", True), + ("ROLE", True), + ("TABLE", False), + ("DATABASE", False), + ("MCP_SERVER", False), + ], + ) + def test_role_hierarchy_rows_are_recognized_in_both_spellings(self, granted_on, expected): + """ACCOUNT_USAGE spells it DATABASE_ROLE, SHOW GRANTS spells it DATABASE ROLE, and + an underscore in an unrelated object type must not be mistaken for either.""" + from snowcap.data_provider import _is_role_hierarchy_grant + + assert _is_role_hierarchy_grant({"granted_on": granted_on}) is expected + + @patch("snowcap.data_provider.list_database_roles") + @patch("snowcap.data_provider._should_use_account_usage") + @patch("snowcap.data_provider.execute") + def test_list_grants_omits_database_role_grants(self, mock_execute, mock_should_use, mock_list_database_roles): + from snowcap.data_provider import list_grants + + mock_should_use.return_value = False + mock_list_database_roles.return_value = [] + + def execute_side_effect(session, query, **kwargs): + if "SHOW DATABASES" in query: + return [] + if "SHOW ROLES" in query: + return [{"name": "MY_ROLE"}] + return [ + self._row(), + self._row(granted_on="DATABASE ROLE", name="SNOWFLAKE.CORTEX_USER"), + { + "privilege": "SELECT", + "granted_on": "TABLE", + "name": "MY_DB.MY_SCHEMA.MY_TABLE", + "granted_to": "ROLE", + "grantee_name": "MY_ROLE", + "grant_option": "false", + "granted_by": "SYSADMIN", + "is_inherited": "false", + }, + ] + + mock_execute.side_effect = execute_side_effect + + grants = list_grants(MagicMock(), include_future_grants=False) + + on_values = [fqn.params["on"] for fqn in grants] + assert on_values == ["table/MY_DB.MY_SCHEMA.MY_TABLE"] + assert not [on for on in on_values if "database_role" in on] + + +class TestGrantsReportedUnderASynonym: + """Snowflake reports a grant on an MCP server as CORTEX_AGENT_SERVER, while GRANT and + CREATE call the object an MCP SERVER. Remote state and the manifest have to identify it + the same way, or every plan both creates and drops the grant -- and drops run after + creates, so applying takes the access away.""" + + def _row(self, granted_on, name, **overrides): + row = { + "privilege": "USAGE", + "granted_on": granted_on, + "name": name, + "granted_to": "ROLE", + "grantee_name": "Z_MCP__DATACOVES", + "grant_option": "false", + "granted_by": "ACCOUNTADMIN", + "is_inherited": "false", + } + row.update(overrides) + return row + + @pytest.mark.parametrize( + "granted_on,expected", + [ + ("CORTEX_AGENT_SERVER", "mcp_server"), + ("MCP_SERVER", "mcp_server"), + ("TABLE", "table"), + ("MATERIALIZED_VIEW", "materialized_view"), + ("IMAGE_REPOSITORY", "image_repository"), + # Unknown to ResourceType: behaves exactly as the raw lowercase did + ("SOMETHING_SNOWFLAKE_ADDED_LATER", "something_snowflake_added_later"), + ], + ) + def test_granted_on_label_matches_the_manifest_spelling(self, granted_on, expected): + from snowcap.data_provider import _granted_on_label + + assert _granted_on_label(granted_on) == expected + + def test_label_agrees_with_grant_fqn(self): + """The whole point is agreeing with the manifest, so assert against it directly + rather than against a hardcoded string.""" + from snowcap.data_provider import _granted_on_label + from snowcap.resources.grant import Grant, grant_fqn + + grant = Grant(priv="USAGE", on="mcp server ADMIN_DB.MCPS.DATACOVES", to="Z_MCP__DATACOVES") + manifest_on = grant_fqn(grant._data).params["on"] + + assert manifest_on.split("/")[0] == _granted_on_label("CORTEX_AGENT_SERVER") + + @patch("snowcap.data_provider.list_database_roles") + @patch("snowcap.data_provider._should_use_account_usage") + @patch("snowcap.data_provider.execute") + def test_list_grants_reports_a_cortex_agent_server_as_an_mcp_server( + self, mock_execute, mock_should_use, mock_list_database_roles + ): + from snowcap.data_provider import list_grants + + mock_should_use.return_value = False + mock_list_database_roles.return_value = [] + + def execute_side_effect(session, query, **kwargs): + if "SHOW DATABASES" in query: + return [] + if "SHOW ROLES" in query: + return [{"name": "Z_MCP__DATACOVES"}] + return [self._row("CORTEX_AGENT_SERVER", "ADMIN_DB.MCPS.DATACOVES")] + + mock_execute.side_effect = execute_side_effect + + grants = list_grants(MagicMock(), include_future_grants=False) + + assert [fqn.params["on"] for fqn in grants] == ["mcp_server/ADMIN_DB.MCPS.DATACOVES"] + + +class TestIntrinsicDatabaseRoleUsage: + """Creating a database role gives it USAGE on the database it belongs to. Snowflake + reports that like any other grant but with an empty granted_by, because no role granted + it -- it is part of the role existing, the way OWNERSHIP is. Nothing can revoke it: + REVOKE reports success and leaves it in place even when run as the database owner. So + listing it puts a row in remote state no config can declare away and no apply can + remove, and sync proposes the same drop on every run forever.""" + + def _row(self, **overrides): + row = { + "privilege": "USAGE", + "granted_on": "DATABASE", + "name": "GREAT_BAY", + "granted_to": "DATABASE_ROLE", + "grantee_name": "GREAT_BAY.DR_CREATE_ROLE", + "grant_option": "false", + "granted_by": "", + "is_inherited": "false", + } + row.update(overrides) + return row + + @pytest.mark.parametrize( + "row_overrides,grantee,expected,because", + [ + ({}, "GREAT_BAY.DR_CREATE_ROLE", True, "usage on its own database"), + ({"name": "OTHER_DB"}, "GREAT_BAY.DR_CREATE_ROLE", False, "usage on a different database is real"), + ( + {"granted_on": "SCHEMA", "name": "GREAT_BAY.PUBLIC"}, + "GREAT_BAY.DR_CREATE_ROLE", + False, + "schema usage is real", + ), + ({"privilege": "SELECT"}, "GREAT_BAY.DR_CREATE_ROLE", False, "only USAGE is intrinsic"), + ({}, "ANALYST", False, "an account role has no own database"), + ], + ) + def test_only_the_roles_own_database_usage_is_intrinsic(self, row_overrides, grantee, expected, because): + from snowcap.data_provider import _is_intrinsic_database_role_usage + + assert _is_intrinsic_database_role_usage(self._row(**row_overrides), grantee) is expected, because + + def test_an_explicit_grant_is_indistinguishable_and_also_skipped(self): + """Snowflake keeps a second row with granted_by populated when the same usage is + granted explicitly. Both reduce to one grant URN, so both are skipped and a declared + usage on a database role's own database simply re-grants -- the role already has it.""" + from snowcap.data_provider import _is_intrinsic_database_role_usage + + explicit = self._row(granted_by="TRANSFORMER_DBT") + + assert _is_intrinsic_database_role_usage(explicit, "GREAT_BAY.DR_CREATE_ROLE") is True + + @patch("snowcap.data_provider.list_database_roles") + @patch("snowcap.data_provider._should_use_account_usage") + @patch("snowcap.data_provider.execute") + def test_list_grants_omits_it_but_keeps_real_grants(self, mock_execute, mock_should_use, mock_list_database_roles): + from snowcap.data_provider import list_grants + from snowcap.identifiers import FQN + from snowcap.resource_name import ResourceName + + mock_should_use.return_value = False + mock_list_database_roles.return_value = [ + FQN(name=ResourceName("DR_CREATE_ROLE"), database=ResourceName("GREAT_BAY")) + ] + + def execute_side_effect(session, query, **kwargs): + if "SHOW DATABASES" in query: + return [] + if "SHOW ROLES" in query: + return [] + return [ + self._row(), # intrinsic, must not be listed + self._row(granted_on="SCHEMA", name="GREAT_BAY.COVE_MARKETING", granted_by="TRANSFORMER_DBT"), + ] + + mock_execute.side_effect = execute_side_effect + + grants = list_grants(MagicMock(), include_future_grants=False) + + assert [fqn.params["on"] for fqn in grants] == ["schema/GREAT_BAY.COVE_MARKETING"] + + +class TestShareBackedDatabaseGrants: + """GRANT IMPORTED PRIVILEGES ON DATABASE is how access to a shared database is + given, and Snowflake reports the resulting grant on the database as plain USAGE. + Identifying it as USAGE means the declared grant never matches the one read back, so + every plan proposes creating it again -- forever, and invisibly, since re-granting + changes nothing. fetch_grant already resolved this, but syncing a resource type builds + remote state from list_* alone and discards manifest URNs, so that path never ran.""" + + SHARED = {"SNOWFLAKE", "SNOWFLAKE_SAMPLE_DATA", "COVID19_EPIDEMIOLOGICAL_DATA"} + + def _row(self, **overrides): + row = { + "privilege": "USAGE", + "granted_on": "DATABASE", + "name": "SNOWFLAKE_SAMPLE_DATA", + "granted_to": "ROLE", + "grantee_name": "Z_DB__SNOWFLAKE_SAMPLE_DATA", + "grant_option": "false", + "granted_by": "ACCOUNTADMIN", + "is_inherited": "false", + } + row.update(overrides) + return row + + @pytest.mark.parametrize( + "overrides,expected,because", + [ + ({}, "IMPORTED PRIVILEGES", "usage on a shared database is imported privileges"), + ({"name": "SNOWFLAKE"}, "IMPORTED PRIVILEGES", "the SNOWFLAKE database is share-backed too"), + ({"name": "BALBOA"}, None, "on an ordinary database USAGE means USAGE"), + ({"privilege": "SELECT"}, None, "only USAGE is reported in place of imported privileges"), + ( + {"granted_on": "SCHEMA", "name": "SNOWFLAKE.ACCOUNT_USAGE"}, + None, + "the substitution is for the database grant, not objects inside it", + ), + ], + ) + def test_only_database_usage_on_a_shared_database_is_rewritten(self, overrides, expected, because): + from snowcap.data_provider import _imported_privileges_priv + + assert _imported_privileges_priv(self._row(**overrides), self.SHARED) == expected, because + + @patch("snowcap.data_provider.list_shared_database_names") + @patch("snowcap.data_provider.list_database_roles") + @patch("snowcap.data_provider._should_use_account_usage") + @patch("snowcap.data_provider.execute") + def test_list_grants_reports_it_the_way_config_declares_it( + self, mock_execute, mock_should_use, mock_list_database_roles, mock_shared + ): + from snowcap.data_provider import list_grants + + mock_should_use.return_value = False + mock_list_database_roles.return_value = [] + mock_shared.return_value = self.SHARED + + def execute_side_effect(session, query, **kwargs): + if "SHOW DATABASES" in query: + return [] + if "SHOW ROLES" in query: + return [{"name": "Z_DB__SNOWFLAKE_SAMPLE_DATA"}] + return [ + self._row(), + self._row(name="BALBOA"), # ordinary database, must stay USAGE + ] + + mock_execute.side_effect = execute_side_effect + + grants = list_grants(MagicMock(), include_future_grants=False) + by_on = {fqn.params["on"]: fqn.params["priv"] for fqn in grants} + + assert by_on["database/SNOWFLAKE_SAMPLE_DATA"] == "IMPORTED PRIVILEGES" + assert by_on["database/BALBOA"] == "USAGE" + + +class TestGrantFetchMatchesOnObjectType: + """fetch_grant compared granted_on as a raw string, so a grant Snowflake reports under + a different name than its DDL uses -- CORTEX_AGENT_SERVER for an MCP SERVER -- never + matched the declared grant, and the plan proposed creating it on every run.""" + + def _grants(self): + return [ + { + "privilege": "USAGE", + "granted_on": "CORTEX_AGENT_SERVER", + "name": "ADMIN_DB.MCPS.DATACOVES", + "granted_to": "ROLE", + "grantee_name": "Z_MCP__DATACOVES", + "grant_option": "false", + "granted_by": "ACCOUNTADMIN", + } + ] + + @patch("snowcap.data_provider._show_grants_to_role") + def test_a_grant_reported_under_a_synonym_is_found(self, mock_show): + from snowcap.data_provider import _fetch_grant_to_role + from snowcap.enums import GrantType, ResourceType + from snowcap.resource_name import ResourceName + + mock_show.return_value = self._grants() + + found = _fetch_grant_to_role( + MagicMock(), + grant_type=GrantType.OBJECT, + role=ResourceName("Z_MCP__DATACOVES"), + granted_on="MCP_SERVER", + on_name="ADMIN_DB.MCPS.DATACOVES", + privilege="USAGE", + role_type=ResourceType.ROLE, + ) + + assert found is not None + assert found["granted_on"] == "CORTEX_AGENT_SERVER" + + @patch("snowcap.data_provider._show_grants_to_role") + def test_an_unrelated_object_type_still_does_not_match(self, mock_show): + from snowcap.data_provider import _fetch_grant_to_role + from snowcap.enums import GrantType, ResourceType + from snowcap.resource_name import ResourceName + + mock_show.return_value = self._grants() + + assert ( + _fetch_grant_to_role( + MagicMock(), + grant_type=GrantType.OBJECT, + role=ResourceName("Z_MCP__DATACOVES"), + granted_on="TABLE", + on_name="ADMIN_DB.MCPS.DATACOVES", + privilege="USAGE", + role_type=ResourceType.ROLE, + ) + is None + ) diff --git a/tests/test_edge_cases.py b/tests/test_edge_cases.py index 8fa1b893..e9e9f55e 100644 --- a/tests/test_edge_cases.py +++ b/tests/test_edge_cases.py @@ -25,7 +25,6 @@ ) from snowcap.enums import ResourceType - # ============================================================================= # Empty Resource Name Tests # ============================================================================= diff --git a/tests/test_exception_handling.py b/tests/test_exception_handling.py index b48723d7..eab5a27b 100644 --- a/tests/test_exception_handling.py +++ b/tests/test_exception_handling.py @@ -49,7 +49,6 @@ from snowcap.blueprint import Blueprint from snowcap.identifiers import parse_URN - # ============================================================================= # Exception Class Tests # ============================================================================= @@ -490,13 +489,7 @@ def test_missing_required_field_in_yaml(self): from snowcap.gitops import collect_blueprint_config # Config with missing required 'name' for database - config = { - "databases": [ - { - "comment": "a database without a name" - } - ] - } + config = {"databases": [{"comment": "a database without a name"}]} with pytest.raises((ValueError, KeyError, TypeError)): collect_blueprint_config(config) @@ -505,11 +498,7 @@ def test_invalid_resource_type_in_yaml(self): from snowcap.gitops import collect_blueprint_config # 'invalid_resources' is not a valid key - config = { - "invalid_resources": [ - {"name": "test"} - ] - } + config = {"invalid_resources": [{"name": "test"}]} # Invalid keys are ignored, but if no valid resources are found, ValueError is raised with pytest.raises(ValueError) as exc_info: collect_blueprint_config(config) diff --git a/tests/test_gitops.py b/tests/test_gitops.py index d03484c6..db6949f5 100644 --- a/tests/test_gitops.py +++ b/tests/test_gitops.py @@ -50,7 +50,9 @@ def test_resource_config(resource_config): bp_config = collect_blueprint_config(resource_config) resource_types = set([resource.resource_type for resource in bp_config.resources]) # Exclude COLUMN types - they are pseudo-resources embedded in tables, not collected via config - expected_resource_types = set([resource_cls.resource_type for resource_cls, _ in JSON_FIXTURES if resource_cls.resource_type.name != "COLUMN"]) + expected_resource_types = set( + [resource_cls.resource_type for resource_cls, _ in JSON_FIXTURES if resource_cls.resource_type.name != "COLUMN"] + ) assert resource_types == expected_resource_types @@ -99,3 +101,140 @@ def test_for_each(): assert blueprint_config.resources is not None assert len(blueprint_config.resources) == 2 assert [resource.urn.fqn.name for resource in blueprint_config.resources] == ["role_bar", "role_baz"] + + +def test_for_each_where_filters_items(): + """`where` narrows a for_each to a subset without a second var. + + Lets one list drive several blocks that each cover part of it -- e.g. the + same schema list granting on a source database and on a clone of it. + """ + config = { + "vars": [{"name": "schemas", "default": ["SRC.ONE", "SRC.TWO", "OTHER.THREE"], "type": "list"}], + "roles": [ + { + "for_each": "var.schemas", + "where": "each.value.split('.')[0] == 'SRC'", + "name": "role_{{ each.value.split('.')[1] }}", + } + ], + } + blueprint_config = collect_blueprint_config(config) + assert blueprint_config.resources is not None + assert [resource.urn.fqn.name for resource in blueprint_config.resources] == ["role_ONE", "role_TWO"] + + +def test_for_each_without_where_is_unfiltered(): + config = { + "vars": [{"name": "schemas", "default": ["SRC.ONE", "OTHER.TWO"], "type": "list"}], + "roles": [{"for_each": "var.schemas", "name": "role_{{ each.value.split('.')[1] }}"}], + } + blueprint_config = collect_blueprint_config(config) + assert [resource.urn.fqn.name for resource in blueprint_config.resources] == ["role_ONE", "role_TWO"] + + +def test_for_each_where_rejects_var_reference(): + """var.* inside `where` used to resolve to a literal string and silently filter out every + item (an empty block, which becomes DROPs in sync mode). It must raise instead.""" + from snowcap.var import evaluate_for_each_where + from snowcap.exceptions import MissingVarException + + with pytest.raises(MissingVarException, match="each.value"): + evaluate_for_each_where("each.value == var.target_region", "SRC.ONE") + + +def test_for_each_where_allows_var_inside_a_string_literal(): + """A var. inside a quoted value is not a reference and must not trip the guard.""" + from snowcap.var import evaluate_for_each_where + + assert evaluate_for_each_where("each.value == 'see var.docs'", "see var.docs") is True + assert evaluate_for_each_where("each.value == 'see var.docs'", "other") is False + + +def test_for_each_where_var_reference_does_not_silently_empty_the_block(): + """End to end: a var.* reference in `where` surfaces an error rather than declaring zero + resources (which would drop the grants the block owns).""" + config = { + "vars": [{"name": "schemas", "default": ["SRC.ONE"], "type": "list"}], + "roles": [{"for_each": "var.schemas", "where": "each.value == var.target", "name": "r_{{ each.value }}"}], + } + with pytest.raises(Exception): + collect_blueprint_config(config) + + +class TestDatabaseRoleGrantsFromYaml: + """A database role can be granted to an account role or to another database role. + DatabaseRoleGrant and the SQL either side of it have always handled both; only this + loader did not, so nesting was expressible in Python and not in config -- and an entry + asking for it produced no resource rather than an error, so the grant never appeared in + the plan at all.""" + + def _build(self, config): + from snowcap.gitops import _resources_from_database_role_grants_config + + return [r.create_sql() for r in _resources_from_database_role_grants_config(config)] + + def test_grant_to_an_account_role(self): + assert self._build([{"database_role": "db.child", "to_role": "analyst"}]) == [ + "GRANT DATABASE ROLE DB.CHILD TO ROLE ANALYST" + ] + + def test_grant_to_several_account_roles(self): + """`roles` is the long-standing plural here and has to keep working.""" + assert self._build([{"database_role": "db.child", "roles": ["analyst", "loader"]}]) == [ + "GRANT DATABASE ROLE DB.CHILD TO ROLE ANALYST", + "GRANT DATABASE ROLE DB.CHILD TO ROLE LOADER", + ] + + def test_empty_string_list_element_is_ignored(self): + """A bad template render of one list item must not build a grant to an empty target.""" + assert self._build([{"database_role": "db.child", "roles": ["", "analyst"]}]) == [ + "GRANT DATABASE ROLE DB.CHILD TO ROLE ANALYST" + ] + + def test_grant_to_another_database_role(self): + assert self._build([{"database_role": "db.child", "to_database_role": "db.parent"}]) == [ + "GRANT DATABASE ROLE DB.CHILD TO DATABASE ROLE DB.PARENT" + ] + + def test_grant_to_several_database_roles(self): + assert self._build([{"database_role": "db.child", "database_roles": ["db.p1", "db.p2"]}]) == [ + "GRANT DATABASE ROLE DB.CHILD TO DATABASE ROLE DB.P1", + "GRANT DATABASE ROLE DB.CHILD TO DATABASE ROLE DB.P2", + ] + + def test_both_kinds_of_target_in_one_entry(self): + assert self._build([{"database_role": "db.child", "roles": ["analyst"], "database_roles": ["db.parent"]}]) == [ + "GRANT DATABASE ROLE DB.CHILD TO ROLE ANALYST", + "GRANT DATABASE ROLE DB.CHILD TO DATABASE ROLE DB.PARENT", + ] + + def test_an_entry_that_grants_to_nothing_is_an_error(self): + """This is what made the gap invisible: it used to yield no resource and no + complaint, so the grant was simply missing from the plan.""" + with pytest.raises(ValueError, match="grants it to nothing"): + self._build([{"database_role": "db.child"}]) + + def test_a_misspelled_key_is_an_error(self): + with pytest.raises(ValueError, match="to_rolez"): + self._build([{"database_role": "db.child", "to_rolez": "analyst"}]) + + def test_an_entry_without_a_database_role_is_an_error(self): + with pytest.raises(ValueError, match="must specify"): + self._build([{"to_role": "analyst"}]) + + def test_empty_config_builds_nothing(self): + assert self._build([]) == [] + + @pytest.mark.parametrize( + "config", + [ + {"database_role": "db.child", "to_role": "analyst", "to_database_role": None}, + {"database_role": "db.child", "to_role": "analyst", "database_roles": None}, + {"database_role": "db.child", "to_role": "analyst", "roles": None}, + ], + ) + def test_a_key_present_but_null_counts_as_absent(self, config): + """YAML spells "not specified" as a key with nothing after it, and serialized + configs round-trip unset fields as explicit nulls.""" + assert self._build([config]) == ["GRANT DATABASE ROLE DB.CHILD TO ROLE ANALYST"] diff --git a/tests/test_grant.py b/tests/test_grant.py index 2d63f2fe..ca6bfea9 100644 --- a/tests/test_grant.py +++ b/tests/test_grant.py @@ -5,6 +5,7 @@ from snowcap.privs import GrantedPrivilege, all_privs_for_resource_type from snowcap.identifiers import URN from snowcap.resource_name import ResourceName +from snowcap.resources.grant import _Grant from snowcap.resources.resource import ResourcePointer @@ -14,7 +15,8 @@ def test_grant_global_priv(): assert grant.on == "ACCOUNT" assert grant.to.name == "somerole" assert ( - str(URN.from_resource(grant)) == "urn:::grant/GRANT?grant_type=OBJECT&priv=CREATE WAREHOUSE&on=account/ACCOUNT&to=role/SOMEROLE" + str(URN.from_resource(grant)) + == "urn:::grant/GRANT?grant_type=OBJECT&priv=CREATE WAREHOUSE&on=account/ACCOUNT&to=role/SOMEROLE" ) assert grant.create_sql() == "GRANT CREATE WAREHOUSE ON ACCOUNT TO ROLE SOMEROLE" @@ -70,7 +72,10 @@ def test_grant_all(): assert grant.on_type == ResourceType.WAREHOUSE assert grant.to.name == "SOMEROLE" assert grant._data._privs == all_privs_for_resource_type(ResourceType.WAREHOUSE) - assert str(URN.from_resource(grant)) == "urn:::grant/GRANT?grant_type=OBJECT&priv=ALL&on=warehouse/SOMEWH&to=role/SOMEROLE" + assert ( + str(URN.from_resource(grant)) + == "urn:::grant/GRANT?grant_type=OBJECT&priv=ALL&on=warehouse/SOMEWH&to=role/SOMEROLE" + ) def test_role_grant_to_user(): @@ -174,6 +179,40 @@ def test_grant_on_cortex_search_service(): assert "MONITOR ON CORTEX SEARCH SERVICE" in monitor_grant.create_sql() +def test_grant_on_cortex_agent_server_resolves_to_mcp_server(): + """'CORTEX AGENT SERVER' is the grant-side spelling of an MCP server. + + Snowflake creates an MCP server when an account connects an MCP client. + SHOW MCP SERVERS lists it, but SHOW GRANTS reports privileges on it with + granted_on 'CORTEX_AGENT_SERVER', and the DDL grammar accepts only + MCP SERVER -- GRANT ... ON CORTEX AGENT SERVER is a syntax error. Reading + that remote state used to abort plan with "Expected Grant.on_type to be + one of (...)". + """ + assert ResourceType("CORTEX AGENT SERVER") is ResourceType.MCP_SERVER + + # fetch_remote_state builds the spec directly via resource_cls.spec(**data), + # bypassing Grant.__init__ and its OWNERSHIP rejection. Snowflake reports + # the server it creates for an MCP client as OWNERSHIP to ACCOUNTADMIN. + remote_spec = _Grant( + priv="OWNERSHIP", + on="somedb.someschema.someserver", + on_type="CORTEX_AGENT_SERVER".replace("_", " "), + to="somerole", + ) + assert remote_spec.on_type == ResourceType.MCP_SERVER + + # However it is spelled in config, it renders as the DDL Snowflake accepts + grant = res.Grant( + priv="USAGE", + on_cortex_agent_server="somedb.someschema.someserver", + to="somerole", + ) + assert grant._data.on == "SOMEDB.SOMESCHEMA.SOMESERVER" + assert grant._data.on_type == ResourceType.MCP_SERVER + assert "USAGE ON MCP SERVER" in grant.create_sql() + + def test_grant_on_dbt_project(): """USAGE/MONITOR on a DBT PROJECT parses and renders correctly. @@ -385,7 +424,7 @@ def test_grant_to_database_role_string(): grant = res.Grant( priv="SELECT", on_table="somedb.someschema.sometable", - to="somedb.mydbrole" # Database role inferred from dot notation + to="somedb.mydbrole", # Database role inferred from dot notation ) assert grant.to_type == ResourceType.DATABASE_ROLE assert grant.to.name == "MYDBROLE" @@ -395,11 +434,7 @@ def test_grant_to_database_role_string(): def test_grant_to_database_role_object(): """Test grant TO a database role using DatabaseRole object.""" db_role = res.DatabaseRole(name="mydbrole", database="somedb") - grant = res.Grant( - priv="SELECT", - on_table="somedb.someschema.sometable", - to=db_role - ) + grant = res.Grant(priv="SELECT", on_table="somedb.someschema.sometable", to=db_role) assert grant.to_type == ResourceType.DATABASE_ROLE assert grant.to.name == "MYDBROLE" assert "TO DATABASE ROLE SOMEDB.MYDBROLE" in grant.create_sql() @@ -407,11 +442,7 @@ def test_grant_to_database_role_object(): def test_future_grant_to_database_role(): """Test future grant TO a database role.""" - grant = res.Grant( - priv="SELECT", - on="FUTURE TABLES IN SCHEMA somedb.someschema", - to="somedb.mydbrole" - ) + grant = res.Grant(priv="SELECT", on="FUTURE TABLES IN SCHEMA somedb.someschema", to="somedb.mydbrole") assert grant.to_type == ResourceType.DATABASE_ROLE assert grant.grant_type == GrantType.FUTURE sql = grant.create_sql() @@ -421,11 +452,7 @@ def test_future_grant_to_database_role(): def test_future_grant_to_database_role_object(): """Test future grant TO a database role using DatabaseRole object.""" db_role = res.DatabaseRole(name="mydbrole", database="somedb") - grant = res.Grant( - priv="CREATE VIEW", - on=["FUTURE", "SCHEMAS", res.Database(name="somedb")], - to=db_role - ) + grant = res.Grant(priv="CREATE VIEW", on=["FUTURE", "SCHEMAS", res.Database(name="somedb")], to=db_role) assert grant.to_type == ResourceType.DATABASE_ROLE assert grant.grant_type == GrantType.FUTURE sql = grant.create_sql() @@ -581,11 +608,7 @@ def test_role_grants_single_role_to_single_role(self): """Test: role_grants: with role: X, to_role: Y creates one grant.""" from snowcap.gitops import collect_blueprint_config - config = { - "role_grants": [ - {"role": "ANALYST", "to_role": "SYSADMIN"} - ] - } + config = {"role_grants": [{"role": "ANALYST", "to_role": "SYSADMIN"}]} blueprint_config = collect_blueprint_config(config) assert len(blueprint_config.resources) == 1 grant = blueprint_config.resources[0] @@ -596,11 +619,7 @@ def test_role_grants_single_role_to_single_user(self): """Test: role_grants: with role: X, to_user: Y creates one grant.""" from snowcap.gitops import collect_blueprint_config - config = { - "role_grants": [ - {"role": "ANALYST", "to_user": "john_doe"} - ] - } + config = {"role_grants": [{"role": "ANALYST", "to_user": "john_doe"}]} blueprint_config = collect_blueprint_config(config) assert len(blueprint_config.resources) == 1 grant = blueprint_config.resources[0] @@ -611,14 +630,7 @@ def test_role_grants_roles_list_to_single_role(self): """Test: role_grants: with roles: [X, Y, Z] to_role: creates multiple grants.""" from snowcap.gitops import collect_blueprint_config - config = { - "role_grants": [ - { - "roles": ["ANALYST", "ENGINEER", "DATA_SCIENTIST"], - "to_role": "SYSADMIN" - } - ] - } + config = {"role_grants": [{"roles": ["ANALYST", "ENGINEER", "DATA_SCIENTIST"], "to_role": "SYSADMIN"}]} blueprint_config = collect_blueprint_config(config) assert len(blueprint_config.resources) == 3 @@ -635,14 +647,7 @@ def test_role_grants_roles_list_to_single_user(self): """Test: role_grants: with roles: [X, Y, Z] to_user: creates multiple grants.""" from snowcap.gitops import collect_blueprint_config - config = { - "role_grants": [ - { - "roles": ["ANALYST", "ENGINEER"], - "to_user": "jane_doe" - } - ] - } + config = {"role_grants": [{"roles": ["ANALYST", "ENGINEER"], "to_user": "jane_doe"}]} blueprint_config = collect_blueprint_config(config) assert len(blueprint_config.resources) == 2 @@ -658,14 +663,7 @@ def test_role_grants_single_role_to_roles_list(self): """Test: role_grants: with role: X, to_roles: [Y, Z] creates multiple grants.""" from snowcap.gitops import collect_blueprint_config - config = { - "role_grants": [ - { - "role": "ANALYST", - "to_roles": ["SYSADMIN", "ACCOUNTADMIN", "SECURITYADMIN"] - } - ] - } + config = {"role_grants": [{"role": "ANALYST", "to_roles": ["SYSADMIN", "ACCOUNTADMIN", "SECURITYADMIN"]}]} blueprint_config = collect_blueprint_config(config) assert len(blueprint_config.resources) == 3 @@ -683,14 +681,7 @@ def test_role_grants_single_role_to_users_list(self): """Test: role_grants: with role: X, to_users: [Y, Z] creates multiple grants.""" from snowcap.gitops import collect_blueprint_config - config = { - "role_grants": [ - { - "role": "ANALYST", - "to_users": ["john_doe", "jane_doe", "bob_smith"] - } - ] - } + config = {"role_grants": [{"role": "ANALYST", "to_users": ["john_doe", "jane_doe", "bob_smith"]}]} blueprint_config = collect_blueprint_config(config) assert len(blueprint_config.resources) == 3 @@ -714,10 +705,7 @@ def test_role_grants_nested_role_hierarchies(self): {"role": "DEV_ANALYST", "to_role": "DEV_ADMIN"}, {"role": "DEV_ADMIN", "to_role": "SYSADMIN"}, # Also grant DEV_ANALYST to users - { - "role": "DEV_ANALYST", - "to_users": ["developer1", "developer2"] - } + {"role": "DEV_ANALYST", "to_users": ["developer1", "developer2"]}, ] } blueprint_config = collect_blueprint_config(config) @@ -735,14 +723,7 @@ def test_role_grants_empty_roles_list_raises_error(self): """Test: Empty roles list raises error.""" from snowcap.gitops import collect_blueprint_config - config = { - "role_grants": [ - { - "roles": [], - "to_role": "SYSADMIN" - } - ] - } + config = {"role_grants": [{"roles": [], "to_role": "SYSADMIN"}]} with pytest.raises(ValueError, match="No role grants found"): collect_blueprint_config(config) @@ -871,3 +852,188 @@ def test_on_all_semantic_views_in_schema_kwarg_raises(self): """on_all_* kwargs are rejected by design; only on=[...]/on="..." forms are supported.""" with pytest.raises(ValueError): res.Grant(priv="SELECT", on_all_semantic_views_in_schema="somedb.someschema", to="somerole") + + +class TestInheritedGrants: + """ + Tests for inherited grants: one container-level grant covering every current and future + object of a type, in place of an ALL + FUTURE pair. + + https://docs.snowflake.com/en/user-guide/inherited-grants-intro + """ + + def test_string_form_on_a_schema(self): + grant = res.Grant(priv="SELECT", on="INHERITED TABLES IN SCHEMA somedb.someschema", to="somerole") + + assert grant.grant_type == GrantType.INHERITED + assert grant.items_type == ResourceType.TABLE + assert grant.on_type == ResourceType.SCHEMA + assert grant.on == "SOMEDB.SOMESCHEMA" + assert grant.create_sql() == ( + "GRANT INHERITED SELECT ON ALL TABLES IN SCHEMA SOMEDB.SOMESCHEMA TO ROLE SOMEROLE" + ) + assert grant.drop_sql() == ( + "REVOKE INHERITED SELECT ON ALL TABLES IN SCHEMA SOMEDB.SOMESCHEMA FROM ROLE SOMEROLE" + ) + + def test_multi_word_object_type_on_a_database(self): + grant = res.Grant(priv="SELECT", on="INHERITED DYNAMIC TABLES IN DATABASE somedb", to="somerole") + + assert grant.items_type == ResourceType.DYNAMIC_TABLE + assert grant.on_type == ResourceType.DATABASE + assert "ON ALL DYNAMIC TABLES IN DATABASE SOMEDB" in grant.create_sql() + + def test_list_form(self): + grant = res.Grant(priv="USAGE", on=["INHERITED", "SCHEMAS", "DATABASE", "somedb"], to="somerole") + + assert grant.grant_type == GrantType.INHERITED + assert grant.items_type == ResourceType.SCHEMA + assert "GRANT INHERITED USAGE ON ALL SCHEMAS IN DATABASE SOMEDB" in grant.create_sql() + + def test_resource_form(self): + grant = res.Grant(priv="SELECT", on=["INHERITED", "TABLES", res.Database(name="somedb")], to="somerole") + + assert grant.on_type == ResourceType.DATABASE + assert grant.on == "SOMEDB" + + def test_account_container(self): + """Only inherited grants can be scoped to the whole account, and the account has no + name of its own to render.""" + grant = res.Grant(priv="SELECT", on="INHERITED TABLES IN ACCOUNT", to="trust_center") + + assert grant.on_type == ResourceType.ACCOUNT + assert grant.on == "ACCOUNT" + assert grant.create_sql() == "GRANT INHERITED SELECT ON ALL TABLES IN ACCOUNT TO ROLE TRUST_CENTER" + assert grant.drop_sql() == "REVOKE INHERITED SELECT ON ALL TABLES IN ACCOUNT FROM ROLE TRUST_CENTER" + + def test_all_and_future_cannot_target_the_whole_account(self): + """Only inherited grants may be scoped to ACCOUNT. ALL/FUTURE + ACCOUNT must fail at + construction, not render doubled-ACCOUNT SQL that only errors mid-apply.""" + for keyword in ("ALL", "FUTURE"): + with pytest.raises(ValueError, match="cannot target the whole account"): + res.Grant(priv="SELECT", on=f"{keyword} TABLES IN ACCOUNT", to="somerole") + + def test_a_database_named_account_is_not_the_account_container(self): + grant = res.Grant(priv="SELECT", on="INHERITED TABLES IN DATABASE account", to="somerole") + + assert grant.on_type == ResourceType.DATABASE + assert grant.on == "ACCOUNT" + assert "IN DATABASE ACCOUNT" in grant.create_sql() + + def test_inherited_flag_upgrades_a_grant_on_all(self): + grant = res.Grant(priv="SELECT", on="ALL TABLES IN DATABASE somedb", inherited=True, to="somerole") + + assert grant.grant_type == GrantType.INHERITED + assert "GRANT INHERITED SELECT ON ALL TABLES IN DATABASE SOMEDB" in grant.create_sql() + + def test_inherited_flag_rejects_object_grants(self): + with pytest.raises(ValueError, match="inherited=True applies to grants on all objects"): + res.Grant(priv="SELECT", on_table="somedb.someschema.sometable", inherited=True, to="somerole") + + def test_inherited_flag_rejects_future_grants(self): + with pytest.raises(ValueError, match="inherited=True applies to grants on all objects"): + res.Grant(priv="SELECT", on="FUTURE TABLES IN DATABASE somedb", inherited=True, to="somerole") + + def test_database_role_grantee(self): + grant = res.Grant( + priv="SELECT", + on="INHERITED TABLES IN SCHEMA somedb.someschema", + to=res.DatabaseRole(name="somerole", database="somedb"), + ) + + assert "TO DATABASE ROLE SOMEDB.SOMEROLE" in grant.create_sql() + + def test_multiple_privs_expand_to_one_grant_each(self): + grant = res.Grant(priv=["SELECT", "INSERT"], on="INHERITED TABLES IN DATABASE somedb", to="somerole") + extra = grant.process_shortcuts() + + assert len(extra) == 1 + assert {grant.priv, extra[0].priv} == {"SELECT", "INSERT"} + assert extra[0].grant_type == GrantType.INHERITED + + def test_urn_is_distinct_from_the_equivalent_all_grant(self): + """An inherited grant and an ON ALL grant on the same target are different objects + in Snowflake and must not collide in the plan.""" + inherited = res.Grant(priv="SELECT", on="INHERITED TABLES IN DATABASE somedb", to="somerole") + on_all = res.Grant(priv="SELECT", on="ALL TABLES IN DATABASE somedb", to="somerole") + + assert inherited.fqn != on_all.fqn + assert inherited.fqn.params["grant_type"] == "INHERITED" + assert on_all.fqn.params["grant_type"] == "ALL" + + def test_grant_option_is_rejected(self): + with pytest.raises(ValueError, match="WITH GRANT OPTION"): + res.Grant( + priv="SELECT", + on="INHERITED TABLES IN DATABASE somedb", + to="somerole", + grant_option=True, + ) + + def test_priv_all_is_rejected(self): + with pytest.raises(ValueError, match="require explicit privileges"): + res.Grant(priv="ALL", on="INHERITED TABLES IN DATABASE somedb", to="somerole") + + def test_unsupported_object_types_are_rejected(self): + with pytest.raises(ValueError, match="cannot be the target of an inherited grant"): + res.Grant(priv="USAGE", on="INHERITED SHARES IN ACCOUNT", to="somerole") + + def test_usage_on_roles_is_rejected(self): + with pytest.raises(ValueError, match="USAGE on ROLE cannot be granted"): + res.Grant(priv="USAGE", on="INHERITED ROLES IN ACCOUNT", to="somerole") + + def test_export_round_trips(self): + from snowcap.resources.grant import grant_yaml + + for on in [ + "INHERITED TABLES IN SCHEMA somedb.someschema", + "INHERITED DYNAMIC TABLES IN DATABASE somedb", + "INHERITED TABLES IN ACCOUNT", + ]: + grant = res.Grant(priv="SELECT", on=on, to="somerole") + exported = grant_yaml(grant.to_dict()) + reimported = res.Grant(**exported) + + assert reimported.fqn == grant.fqn, on + assert reimported.create_sql() == grant.create_sql(), on + + def test_from_sql_round_trips_each_container(self): + for sql in [ + "GRANT INHERITED SELECT ON ALL TABLES IN SCHEMA somedb.someschema TO ROLE somerole", + "GRANT INHERITED USAGE ON ALL SCHEMAS IN DATABASE somedb TO ROLE somerole", + "GRANT INHERITED SELECT ON ALL TABLES IN ACCOUNT TO ROLE somerole", + ]: + grant = res.Grant.from_sql(sql) + + assert grant.grant_type == GrantType.INHERITED, sql + assert grant.create_sql() == sql.upper(), sql + + def test_string_form_accepts_a_three_word_collection_type(self): + """A 3-word object type in the plural (CORTEX SEARCH SERVICES) must parse in the string + form, not split into tokens and trip the item-count guard.""" + grant = res.Grant(priv="USAGE", on="INHERITED CORTEX SEARCH SERVICES IN SCHEMA db.s", to="r") + + assert grant.items_type == ResourceType.CORTEX_SEARCH_SERVICE + assert grant.on_type == ResourceType.SCHEMA + assert "ON ALL CORTEX SEARCH SERVICES IN SCHEMA DB.S" in grant.create_sql() + + def test_from_sql_leaves_grants_on_all_alone(self): + grant = res.Grant.from_sql("GRANT SELECT ON ALL TABLES IN SCHEMA somedb.someschema TO ROLE somerole") + + assert grant.grant_type == GrantType.ALL + + def test_yaml_config_accepts_the_inherited_key(self): + from snowcap.gitops import collect_blueprint_config + + config = { + "grants": [ + {"priv": "SELECT", "on": "INHERITED TABLES IN DATABASE somedb", "to": "somerole"}, + {"priv": "SELECT", "on": "ALL TABLES IN DATABASE otherdb", "inherited": True, "to": "somerole"}, + ] + } + + blueprint_config = collect_blueprint_config(config) + grants = [r for r in blueprint_config.resources if isinstance(r, res.Grant)] + + assert len(grants) == 2 + assert all(g.grant_type == GrantType.INHERITED for g in grants) diff --git a/tests/test_lifecycle.py b/tests/test_lifecycle.py index 484c495e..8248068a 100644 --- a/tests/test_lifecycle.py +++ b/tests/test_lifecycle.py @@ -60,7 +60,6 @@ from snowcap.resource_name import ResourceName from snowcap.props import Props - # ============================================================================ # Test fixtures and helpers # ============================================================================ @@ -423,6 +422,48 @@ def test_grant_all(self): result = create_grant(urn, data, props, if_not_exists=False) assert "ON ALL TABLES IN" in result + def test_grant_inherited(self): + """One container-level grant covering current and future objects.""" + urn = make_urn(ResourceType.GRANT, "GRANT") + data = { + "priv": "SELECT", + "on_type": "SCHEMA", + "on": "MY_DB.MY_SCHEMA", + "items_type": "TABLE", + "to_type": "ROLE", + "to": "MY_ROLE", + "grant_type": GrantType.INHERITED, + "grant_option": False, + } + props = MockProps("") + + assert create_grant(urn, data, props, if_not_exists=False) == ( + "GRANT INHERITED SELECT ON ALL TABLES IN SCHEMA MY_DB.MY_SCHEMA TO ROLE MY_ROLE" + ) + assert drop_grant(urn, data) == ( + "REVOKE INHERITED SELECT ON ALL TABLES IN SCHEMA MY_DB.MY_SCHEMA FROM ROLE MY_ROLE" + ) + + def test_grant_inherited_in_account(self): + """The account container has no name, so it renders as a bare IN ACCOUNT.""" + urn = make_urn(ResourceType.GRANT, "GRANT") + data = { + "priv": "SELECT", + "on_type": ResourceType.ACCOUNT, + "on": "ACCOUNT", + "items_type": "TABLE", + "to_type": "ROLE", + "to": "MY_ROLE", + "grant_type": GrantType.INHERITED, + "grant_option": False, + } + props = MockProps("") + + assert create_grant(urn, data, props, if_not_exists=False) == ( + "GRANT INHERITED SELECT ON ALL TABLES IN ACCOUNT TO ROLE MY_ROLE" + ) + assert drop_grant(urn, data) == "REVOKE INHERITED SELECT ON ALL TABLES IN ACCOUNT FROM ROLE MY_ROLE" + def test_grant_on_integration(self): """Test grant on integration (type normalization).""" urn = make_urn(ResourceType.GRANT, "GRANT") diff --git a/tests/test_parse.py b/tests/test_parse.py index 664c1f7e..227b80fd 100644 --- a/tests/test_parse.py +++ b/tests/test_parse.py @@ -28,7 +28,6 @@ from snowcap.enums import ResourceType, Scope from snowcap.scope import DatabaseScope, SchemaScope - # ============================================================================= # Test: parse_region (existing tests refactored) # ============================================================================= diff --git a/tests/test_polymorphic_resources.py b/tests/test_polymorphic_resources.py index be26248f..93e47354 100644 --- a/tests/test_polymorphic_resources.py +++ b/tests/test_polymorphic_resources.py @@ -6,7 +6,6 @@ from snowcap.enums import ResourceType from tests.helpers import get_json_fixture, camelcase_to_snakecase - logger = logging.getLogger("snowcap") diff --git a/tests/test_privs.py b/tests/test_privs.py index a86ff6e1..0aecf493 100644 --- a/tests/test_privs.py +++ b/tests/test_privs.py @@ -53,7 +53,6 @@ from snowcap.enums import ResourceType from snowcap.identifiers import resource_label_for_type, resource_type_for_label - # Pseudo-resources and meta-resources that don't have associated privileges # These are internal resources, grants, or container types that don't support GRANT statements PSEUDO_RESOURCE_TYPES = { diff --git a/tests/test_props.py b/tests/test_props.py index 79376f03..e31ca48d 100644 --- a/tests/test_props.py +++ b/tests/test_props.py @@ -140,8 +140,8 @@ def test_quote_value_none(self): assert result == "''" def test_quote_value_with_quotes(self): - result = quote_value("it's a \"test\"") - assert result == "$$it's a \"test\"$$" + result = quote_value('it\'s a "test"') + assert result == '$$it\'s a "test"$$' def test_quote_value_multiline(self): result = quote_value("line1\nline2") @@ -735,37 +735,29 @@ class TestSchemaPropExtended: def test_render_single_column(self): prop = SchemaProp() - result = prop.render([ - {"name": "col1", "data_type": DataType.VARCHAR, "not_null": False, "default": None} - ]) + result = prop.render([{"name": "col1", "data_type": DataType.VARCHAR, "not_null": False, "default": None}]) assert result == "(col1 VARCHAR)" def test_render_column_with_not_null(self): prop = SchemaProp() - result = prop.render([ - {"name": "col1", "data_type": DataType.NUMBER, "not_null": True, "default": None} - ]) + result = prop.render([{"name": "col1", "data_type": DataType.NUMBER, "not_null": True, "default": None}]) assert result == "(col1 NUMBER NOT NULL)" def test_render_column_with_string_default(self): prop = SchemaProp() - result = prop.render([ - {"name": "col1", "data_type": DataType.VARCHAR, "not_null": False, "default": "hello"} - ]) + result = prop.render([{"name": "col1", "data_type": DataType.VARCHAR, "not_null": False, "default": "hello"}]) assert result == "(col1 VARCHAR DEFAULT 'hello')" def test_render_column_with_numeric_default(self): prop = SchemaProp() - result = prop.render([ - {"name": "col1", "data_type": DataType.NUMBER, "not_null": False, "default": 42} - ]) + result = prop.render([{"name": "col1", "data_type": DataType.NUMBER, "not_null": False, "default": 42}]) assert result == "(col1 NUMBER DEFAULT 42)" def test_render_column_with_comment(self): prop = SchemaProp() - result = prop.render([ - {"name": "col1", "data_type": DataType.VARCHAR, "not_null": False, "default": None, "comment": "test"} - ]) + result = prop.render( + [{"name": "col1", "data_type": DataType.VARCHAR, "not_null": False, "default": None, "comment": "test"}] + ) assert result == "(col1 VARCHAR COMMENT 'test')" def test_render_empty(self): diff --git a/tests/test_resource_types.py b/tests/test_resource_types.py index ba316c5f..1bcf3052 100644 --- a/tests/test_resource_types.py +++ b/tests/test_resource_types.py @@ -139,7 +139,7 @@ def test_shared_database_resolver_picks_database(self): @pytest.mark.xfail( reason=( "ResourceName wraps from_share as a single opaque string, so a compound identifier " - "with one quoted component (e.g. SOME_ORG.\"My Share\") gets uppercased wholesale by " + 'with one quoted component (e.g. SOME_ORG."My Share") gets uppercased wholesale by ' "ResourceName.__str__ instead of preserving the quoted part. ResourceName has no " "concept of compound (dotted) identifiers with independently-quoted components -- " "this is a pre-existing limitation shared by every IdentifierProp field, not " @@ -153,9 +153,7 @@ def test_shared_database_from_share_quoted_component_round_trips(self): db = res.SharedDatabase(name="gong", from_share='SOME_ORG."My Share"') assert db.create_sql() == 'CREATE DATABASE GONG FROM SHARE SOME_ORG."My Share"' - round_tripped = res.SharedDatabase.from_sql( - 'CREATE DATABASE gong FROM SHARE SOME_ORG."My Share"' - ) + round_tripped = res.SharedDatabase.from_sql('CREATE DATABASE gong FROM SHARE SOME_ORG."My Share"') assert round_tripped._data.from_share == 'SOME_ORG."My Share"' @@ -1707,7 +1705,10 @@ class TestResourceCommon: (res.Role, {"name": "test"}), (res.Warehouse, {"name": "test"}), (res.User, {"name": "test"}), - (res.Table, {"name": "test", "database": "db", "schema": "sch", "columns": [{"name": "id", "data_type": "INT"}]}), + ( + res.Table, + {"name": "test", "database": "db", "schema": "sch", "columns": [{"name": "id", "data_type": "INT"}]}, + ), (res.View, {"name": "test", "database": "db", "schema": "sch", "as_": "SELECT 1"}), (res.NetworkPolicy, {"name": "test"}), ], diff --git a/tests/test_scope.py b/tests/test_scope.py index 705c43b9..88ec8e31 100644 --- a/tests/test_scope.py +++ b/tests/test_scope.py @@ -259,6 +259,7 @@ class TestResourceCanBeContainedIn: def test_account_scope_in_account_container(self): """Account-scoped resource can be contained in Account container.""" + class MockResource: scope = AccountScope() @@ -270,6 +271,7 @@ class Account: def test_database_scope_in_database_container(self): """Database-scoped resource can be contained in Database container.""" + class MockResource: scope = DatabaseScope() @@ -281,6 +283,7 @@ class Database: def test_schema_scope_in_schema_container(self): """Schema-scoped resource can be contained in Schema container.""" + class MockResource: scope = SchemaScope() @@ -292,6 +295,7 @@ class Schema: def test_account_scope_not_in_database_container(self): """Account-scoped resource cannot be contained in Database container.""" + class MockResource: scope = AccountScope() @@ -303,6 +307,7 @@ class Database: def test_database_scope_not_in_account_container(self): """Database-scoped resource cannot be contained in Account container.""" + class MockResource: scope = DatabaseScope() @@ -314,6 +319,7 @@ class Account: def test_schema_scope_not_in_database_container(self): """Schema-scoped resource cannot be contained in Database container.""" + class MockResource: scope = SchemaScope() @@ -325,6 +331,7 @@ class Database: def test_organization_scope_not_in_any_container(self): """Organization-scoped resource is not contained in standard containers.""" + class MockResource: scope = OrganizationScope() @@ -336,6 +343,7 @@ class Account: def test_anonymous_scope_not_in_any_container(self): """Anonymous-scoped resource is not contained in standard containers.""" + class MockResource: scope = AnonymousScope() diff --git a/tests/test_yaml_config.py b/tests/test_yaml_config.py index e6c85b3e..c0d3c914 100644 --- a/tests/test_yaml_config.py +++ b/tests/test_yaml_config.py @@ -554,9 +554,7 @@ class TestBalboaYamlFixtures: balboa patterns are valid and can be loaded by the config parser. """ - FIXTURES_DIR = os.path.join( - os.path.dirname(__file__), "fixtures", "yaml" - ) + FIXTURES_DIR = os.path.join(os.path.dirname(__file__), "fixtures", "yaml") def _load_fixture(self, filename): """Load a YAML fixture file and return the dict.""" diff --git a/tools/generate_resource.py b/tools/generate_resource.py index 1a6a501f..9a8e6e0f 100644 --- a/tools/generate_resource.py +++ b/tools/generate_resource.py @@ -1,7 +1,6 @@ import os import sys - # Set repo_root to the parent directory that this file lives in REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))