-
Notifications
You must be signed in to change notification settings - Fork 35
refactor(set): validate blocklist instead of allowlist #151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
refactor(set): validate blocklist instead of allowlist #151
Conversation
| query = args.query | ||
|
|
||
| utils_set.validate_expression(query, JMESPATH_UPDATE_DOMAINS) | ||
| # Validate against invalid queries - allow users to set any query parameter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove unnecessary comments
| "description", | ||
| "displayName", | ||
| "sparkSettings", | ||
| INVALID_QUERIES = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a thought - do we want to expose those lists to users via set --help?
| Raises: | ||
| FabricCLIError: If query is blocklisted or contains filters/wildcards. | ||
| """ | ||
| # Validate it's a simple path expression (no filters, wildcards, etc.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove comments
This pull request refactors the validation logic for the
setcommand across all Fabric resource types. Instead of using allowlists of supported properties, the code now uses a blocklist approach: any property can be set except those explicitly blocked (either globally or per resource). This makes the CLI more flexible and aligns it with the official REST API documentation, while still preventing updates to unsafe or unsupported properties. Documentation and error handling have also been updated to reflect this new approach.Core validation logic changes:
setcommand. Now, any property can be set unless it is in a global or resource-specific blocklist, or uses filters/wildcards. This is implemented via the newvalidate_query_not_in_blocklistutility. [1] [2]id,type,workspaceId,folderId) that cannot be set for any resource, and added resource-specific blocklists where needed. [1] [2] [3] [4] [5] [6]Resource command updates:
fab_fs_set_*command modules to use the new blocklist validation function instead of allowlist validation. Removed allowlist constants from these modules. [1] [2] [3] [4] [5] [6] [7] [8]Gateway-specific validation improvements:
Documentation and error messaging:
setcommand.These changes make the CLI more robust and easier to maintain, while giving users more flexibility and clearer guidance.