Skip to content
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

databricks update groups command is unnecessarily destructive #1942

Open
tim-sparq opened this issue Nov 28, 2024 · 6 comments
Open

databricks update groups command is unnecessarily destructive #1942

tim-sparq opened this issue Nov 28, 2024 · 6 comments
Assignees
Labels
CLI CLI related issues No Autoclose

Comments

@tim-sparq
Copy link

Describe the issue

Running the databricks groups update [group id] command without the --json flag deletes all of the specified group's configuration, including all members, potentially with destructive impact.

Recently this caused a production incident in our environment when all users were silently removed from our workspace's admins group, causing pipelines to break and locking us out of our workspace.

This behaviour - which is undocumented - is unnecessarily destructive. If no configuration is supplied, the command should arguably throw an error, instead of silently removing all users from the specified group. Compare to other major clis (such as az / gcloud) where such a destructive behavior is not possible.

Steps to reproduce the behavior

Please list the steps required to reproduce the issue, for example:

  1. Run databricks groups update [group id], omitting the --json flag

Expected Behavior

The command should return an error notifying the user that no configuration was supplied

Actual Behavior

The command silently deletes all of the specified group's configuration, including all members, potentially with destructive impact

OS and CLI version

macos, Databricks CLI v0.219.0,

Is this a regression?

No

Debug Logs

N/A

@tim-sparq tim-sparq added the CLI CLI related issues label Nov 28, 2024
@shreyas-goenka shreyas-goenka self-assigned this Dec 5, 2024
Copy link

github-actions bot commented Jan 8, 2025

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the Stale label Jan 8, 2025
@tim-sparq
Copy link
Author

@shreyas-goenka do you have an opinion on this?

@shreyas-goenka
Copy link
Contributor

@tim-sparq Thanks for reporting this discrepancy. Regarding documentation, It is documented in the help for the command itself:

➜  universe2 git:(master) ✗ databricks groups update -h
Replace a group.

  Updates the details of a group by replacing the entire group entity.

  Arguments:
    ID: Databricks group ID

I agree with you that the name update here is not great. I do think we should revisit the names for the commands in the CLI and standardize on the names being closer to what the API does. However, that would have to be a standardization done across all commands and not just limited to the groups API.

I'll keep this issue open for now, with the intention of picking up the standardization work sometime in the future.

@tim-sparq
Copy link
Author

@shreyas-goenka I don't have a problem with the use of the name update, or with the command completely replacing the contents of the specified group with the configuration provided by the user. That is all quite conventional.

I only have a problem with the command emptying a group of all its members when no configuration is supplied. It's an unnecessarily destructive behaviour. Of course, it should be possible for a user to remove all members of a group using this command. But I would argue that that command should be:

databricks groups update [group id] --json []

And that this command:

databricks groups update [group id]

Should simply return an error along the lines of update was requested but no group configuration was supplied

@shreyas-goenka
Copy link
Contributor

shreyas-goenka commented Jan 9, 2025

@tim-sparq Yeah your feedback makes sense to me. Any such change, though, should be consistent across all commands and thus would have to be looked into a bit.

Just FYI there's a different command available that does partial non-destructive updates, which seems more suitable for your use case.

databricks groups patch

Ideally, the databricks groups update command should have been named databricks groups replace, which would not have caused this confusion in the first place. This would match the corresponding API docs for this command as well: https://docs.databricks.com/api/workspace/groups/update.

@tim-sparq
Copy link
Author

Again, I would even argue that

databricks groups replace [group id]

Should (if it existed) throw an error, because the replacement group configuration has not been supplied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI CLI related issues No Autoclose
Projects
None yet
Development

No branches or pull requests

2 participants