Skip to content

fix(lifecycle): transfer ownership with the object type GRANT OWNERSHIP accepts - #84

Open
GClunies wants to merge 3 commits into
datacoves:mainfrom
GClunies:fix/transfer-ownership-object-type
Open

GClunies wants to merge 3 commits into
datacoves:mainfrom
GClunies:fix/transfer-ownership-object-type

Conversation

@GClunies

@GClunies GClunies commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

What

Snowcap transfers ownership with a GRANT OWNERSHIP statement. For four kinds of objects, that statement used an object type that the Snowflake reference does not tell you to use. This PR changes transfer__default to use the object type that the reference gives.

Resource type Before After
All integrations (security, storage, API, and more) GRANT OWNERSHIP ON SECURITY INTEGRATION X ... GRANT OWNERSHIP ON INTEGRATION X ...
Materialized view GRANT OWNERSHIP ON MATERIALIZED VIEW X ... GRANT OWNERSHIP ON VIEW X ...
Hybrid table GRANT OWNERSHIP ON HYBRID TABLE X ... GRANT OWNERSHIP ON TABLE X ...
External function GRANT OWNERSHIP ON EXTERNAL FUNCTION X ... GRANT OWNERSHIP ON FUNCTION X ...

Other resource types do not change.

Why

The GRANT OWNERSHIP reference gives these rules:

  • The object type list has INTEGRATION, but no integration subtypes.
  • The object type list has FUNCTION, but no EXTERNAL FUNCTION. External functions are granted as FUNCTION.
  • The usage notes say: "To grant ownership on a materialized view, use GRANT OWNERSHIP ON VIEW."
  • The usage notes say: "To grant ownership on a hybrid table, use GRANT OWNERSHIP ON TABLE."

We tested each form against a live account:

Statement Result
GRANT OWNERSHIP ON SECURITY INTEGRATION ... Syntax error 001003, so snowcap apply fails
GRANT OWNERSHIP ON INTEGRATION ... Accepted
GRANT OWNERSHIP ON MATERIALIZED VIEW ... and ON VIEW ... on a real materialized view Both accepted
GRANT OWNERSHIP ON HYBRID TABLE ... Not tested. Our account does not support hybrid tables.
GRANT OWNERSHIP ON EXTERNAL FUNCTION db.sch.fn(VARCHAR) ... Syntax error 001003
GRANT OWNERSHIP ON FUNCTION db.sch.fn(VARCHAR) ... on a name that does not exist Parsed. Error 002003, "does not exist"
GRANT OWNERSHIP ON DYNAMIC TABLE, ON ICEBERG TABLE, ON VIEW on a name that does not exist Parsed. Error 002003, so these types keep their own names

So the integration and external function changes fix real failures. The materialized view and hybrid table changes follow the usage notes. The materialized view change is safe, because Snowflake accepts both forms.

Example: our config declares ACCOUNTADMIN as the owner of a security integration, but a different role owns it in Snowflake. The plan contains a transfer, and the apply fails on this statement:

GRANT OWNERSHIP ON SECURITY INTEGRATION MCP_OAUTH_CLAUDE TO ROLE ACCOUNTADMIN COPY CURRENT GRANTS

With this fix, Snowcap sends the form that the reference lists:

GRANT OWNERSHIP ON INTEGRATION MCP_OAUTH_CLAUDE TO ROLE ACCOUNTADMIN COPY CURRENT GRANTS

The fix does not change privilege grants. GRANT <privilege> ON MATERIALIZED VIEW is valid, so create_grant keeps its object types. To find integrations, the fix uses the same test as create_grant: "INTEGRATION" in str(resource_type).

Known gap: external function names

ExternalFunction.fqn has no argument types. The UDF classes add them through udf_fqn. So for a real external function, the transfer is:

GRANT OWNERSHIP ON FUNCTION MY_DB.MY_SCHEMA.MY_FN TO ROLE NEW_OWNER COPY CURRENT GRANTS

Snowflake rejects this with error 090208: "Argument types of function must be specified". This PR fixes the object type only. To fix the name, ExternalFunction must identify itself by its signature, which changes how Snowcap matches it to live state. That belongs in a separate PR: #85.

Tests

  • The parametrized test TestTransferResource::test_transfer_uses_snowflake_ownership_object_type has 12 cases. The 9 mapped cases fail without the fix and pass with it. The 3 neighbor cases (DYNAMIC_TABLE, ICEBERG_TABLE, VIEW) check that the mapping does not grow by accident, for example to "anything that ends in TABLE becomes TABLE".
  • uv run pytest tests/test_lifecycle.py: 143 passed.
  • Full suite on the first commit: 2234 passed. test_connect_filters_none_values fails with and without this change, because it reads SNOWFLAKE_USER from the local environment.
  • A read-only snowcap plan against our account gives the same output with this branch and with 1.0.32. The fix changes only the SQL of a transfer, not the plan.

…IP accepts

GRANT OWNERSHIP rejects integration subtype names (SECURITY INTEGRATION, STORAGE INTEGRATION, ...), MATERIALIZED VIEW and HYBRID TABLE. Snowflake names them INTEGRATION, VIEW and TABLE. A plan that transferred ownership of any of these failed at apply with a SQL compilation error, for example: GRANT OWNERSHIP ON SECURITY INTEGRATION X TO ROLE ACCOUNTADMIN COPY CURRENT GRANTS.

https://docs.snowflake.com/en/sql-reference/sql/grant-ownership

noel commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Thanks for the fix. The mapping matches the GRANT OWNERSHIP docs: only INTEGRATION is listed, and the usage notes say to use ON VIEW for materialized views and ON TABLE for hybrid tables. tests/test_lifecycle.py passes locally (139 tests). A couple of things to address before merging:

  1. External functions have the same problem. ResourceType.EXTERNAL_FUNCTION still renders as GRANT OWNERSHIP ON EXTERNAL FUNCTION .... EXTERNAL FUNCTION isn't in Snowflake's GRANT OWNERSHIP object list, because external functions are granted as FUNCTION. ExternalFunction defaults its owner to SYSADMIN, so any transfer away from that fails at apply. Please add it to the mapping (snowcap/lifecycle.py:874):

    _OWNERSHIP_OBJECT_TYPES = {
        ResourceType.EXTERNAL_FUNCTION: "FUNCTION",
        ResourceType.HYBRID_TABLE: "TABLE",
        ResourceType.MATERIALIZED_VIEW: "VIEW",
    }

    Please also add a matching test case, e.g. (ResourceType.EXTERNAL_FUNCTION, "MY_SCHEMA", "FUNCTION").

  2. Test that similar types keep their own names (tests/test_lifecycle.py:1319). Nothing currently checks that types close to the mapped ones still render unchanged. Adding a few cases such as (ResourceType.DYNAMIC_TABLE, "MY_SCHEMA", "DYNAMIC TABLE"), (ResourceType.ICEBERG_TABLE, "MY_SCHEMA", "ICEBERG TABLE") and (ResourceType.VIEW, "MY_SCHEMA", "VIEW") would catch it if the mapping is ever widened by accident (for example, "anything ending in TABLE becomes TABLE").

Optional, not blocking: the "INTEGRATION" in str(...) check now appears four times (lifecycle.py:177, :196, :213 and :881). A shared helper would keep GRANT and GRANT OWNERSHIP from drifting apart, but that fits better in a follow-up than in this PR.


Generated by Claude Code

GRANT OWNERSHIP has no EXTERNAL FUNCTION object type, so a transfer away
from the default SYSADMIN owner failed with a syntax error. External
functions are granted as FUNCTION.
@GClunies

GClunies commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Both items are done.

  1. External functions: b0ddb01 maps EXTERNAL_FUNCTION to FUNCTION and adds the test case. A live probe agrees: GRANT OWNERSHIP ON EXTERNAL FUNCTION ... is syntax error 001003, and ON FUNCTION ... parses.
  2. Neighbor types: dd3c591 adds DYNAMIC_TABLE, ICEBERG_TABLE and VIEW cases. Snowflake accepts all three names as they are.

One gap remains for external functions. ExternalFunction.fqn has no argument types, unlike udf_fqn. So the transfer is GRANT OWNERSHIP ON FUNCTION MY_DB.MY_SCHEMA.MY_FN TO ROLE ..., and Snowflake rejects it with 090208, "Argument types of function must be specified". The fix changes how Snowcap identifies external functions, so I opened #85 for a separate PR.

I agree that the shared INTEGRATION helper fits a follow-up.

tests/test_lifecycle.py: 143 passed.

@noel

noel commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Solid, well-researched fix (the live-account testing against the GRANT OWNERSHIP reference is great). One reuse nit:

_ownership_object_type's "INTEGRATION" in str(resource_type) check duplicates logic that already exists twice in create_grant (lifecycle.py:177-178 and 196-197/213-214). That's now 4 copies of the same integration-detection substring check in this file. If a future edge case changes how integration subtypes are detected, it's easy to update one spot, pass tests, and leave the others silently inconsistent.

Worth extracting to one shared predicate, e.g.:

def _is_integration(resource_type) -> bool:
    return "INTEGRATION" in str(resource_type)

and using it in all four spots. Small, low-risk cleanup, not a blocker.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants