-
Notifications
You must be signed in to change notification settings - Fork 991
[v8] Command "unbind-service" can handle multiple bindings #3665
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: v8
Are you sure you want to change the base?
[v8] Command "unbind-service" can handle multiple bindings #3665
Conversation
* see https://github.com/cloudfoundry/community/blob/main/toc/rfc/rfc-0040-service-binding-rotation.md#cf-cli * add parameter "--guid" to delete only a specific binding
Dariquest
left a comment
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.
LGTM
anujc25
left a comment
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 one minor comment about updating the integration tests otherwise LGTM.
| case nil: | ||
| case actionerror.ServiceBindingNotFoundError: | ||
| cmd.UI.DisplayText("Binding between {{.ServiceInstanceName}} and {{.AppName}} did not exist", cmd.names()) | ||
| cmd.UI.DisplayText("Binding between {{.ServiceInstanceName}} and {{.AppName}} does not exist", cmd.names()) |
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.
This change requires changes in the integration test as the integration test is failing because of this change.
Test Details:
- Test Name: unbind-service command > when targeting a space > binding does not exist > succeeds saying the binding did not exist
- Test File:
integration/v7/isolated/unbind_service_command_test.go:252
What the CLI actually outputs:
Binding between INT-SI-int-9fdf09b3-2b4e-4824-4bf7-3a91cf9f2a60 and INTEGRATION-APP-int-3ac77377-8c7d-4433-75b6-2c4da19ad19d does not exist
OK
What the test expects to find:
Binding between INT-SI-int-9fdf09b3-2b4e-4824-4bf7-3a91cf9f2a60 and INTEGRATION-APP-int-3ac77377-8c7d-4433-75b6-2c4da19ad19d did not exist
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.
Fixed.
Description of the Change
The "unbind-service" command shall be able to handle multiple bindings per (app, service instance). This PR first retrieves the list of all bindings for a given app and a given service instance name. Then it loops over the list and calls
DELETE /v3/service_credential_bindings/:guidfor each item. A new parameter "--guid" allows to delete only a specific bindingWhy Is This PR Valuable?
Allows users to delete multiple service bindings with one CF CLI command.
Applicable Issues
https://github.com/cloudfoundry/community/blob/main/toc/rfc/rfc-0040-service-binding-rotation.md#cf-cli
How Urgent Is The Change?
Not super-urgent.
Other Relevant Parties