Business roles absent in business_role.yaml but specified in user.yaml are implicitly created and granted

[kokorin]

Describe the bug

#users.yaml
my_user:
  password: '123'
  business_roles:
    - reporter
    - analyst

#business_role.yaml
analyst: {}

No BRole reporter is configured, but it is created and granted to my_user

Expected behavior

I would expect validation error from SnowDDL. It would help in case of typos in user's business_roles property or in case of BRole rename.

Attach log

--- Suggested DDL ---

CREATE ROLE "ANALYST__B_ROLE";

GRANT ROLE "ANALYST__B_ROLE" TO ROLE "SNOWDDL_ADMIN";

CREATE ROLE "MY_USER__U_ROLE";

GRANT ROLE "MY_USER__U_ROLE" TO ROLE "SNOWDDL_ADMIN";

GRANT ROLE "REPORTER__B_ROLE" TO ROLE "MY_USER__U_ROLE";

GRANT ROLE "ANALYST__B_ROLE" TO ROLE "MY_USER__U_ROLE";

CREATE USER "MY_USER"
LOGIN_NAME = 'MY_USER'
DISPLAY_NAME = 'MY_USER'
DEFAULT_ROLE = "MY_USER__U_ROLE"
PASSWORD = '123' ;

GRANT ROLE "MY_USER__U_ROLE" TO USER "MY_USER";

Update: it appears that apply command fails in that scenario, but not plan:

2024-11-21 10:33:27.868 - WARNING - Resolved ROLE [<REDACTED>__U_ROLE]: ERROR
(
    message   =>  SQL compilation error:
Role '<REDACTED>__B_ROLE' does not exist or not authorized.
    errno     =>  2003
    sqlstate  =>  02000
    sfqid     =>  01b88479-0105-6df8-0001-<REDACTED>
    sql       =>  GRANT ROLE "<REDACTED>__B_ROLE" TO ROLE "<REDACTED>__U_ROLE"
)

[littleK0i]

It is an interesting question: "How many explicit validation do we want to have in the codebase?"

Each validation helps a bit, but it also increases the amount of code and makes it more difficult to re-shuffle things in future. Also, the only way to truly "validate" a config is to apply it to live Snowflake account.

In this specific case the workflow seems to be roughly the same:

1. Push changes with incorrect reference to business role;
2. Run apply, encounter error;
3. Fix the code, push and apply again;

That's why this specific validation is currently not there.

We actually have some other validations in place in parsers: https://github.com/search?q=repo%3AlittleK0i%2FSnowDDL%20ValueError&type=code

Mostly covering use-cases when lack of validation can silently break something. Like empty placeholders, incorrect pattern matchings, etc.

[kokorin]

I believe cross-references between blueprints can be validated, i.e. BRole granted Global Roles may have no validation, but references to WH, BRoles or TRoles should be validated.
I will try to find some time this weekend to take a look.

[littleK0i]

The question is how to organize it in the code to make such checks relatively "clean".

The most simple solution is to add a generic function to AbstractParser throwing error:

Object XXX of type YYY does not exist in config

... but it would be nice to know the context. For example:

User XXX references business role YYY which does not exist in config

That's great, but code would look like tons of IF statements. Maybe it is not too bad if moved into a dedicated "validate" function.

[littleK0i]

At this moment exceptions during parsing include path to file which caused an exception. Works very well for schema objects (tables, views, etc.). Does not work so well for account-level objects, such as business roles, since all roles are in the same file. And some configs may have hundres of such roles.