v8(services): remove detection of UPSI for keys (#2140)

We decided that the CLI should have no knowledge of the restriction that user-provided service instances cannot have service keys. While there is no plan to change that restriction, we should avoid duplicating checks in the CLI that are also performed in the cloud controller so that the checks do not get out of step in the future. [#176719946](https://www.pivotaltracker.com/story/show/176719946)
cloudfoundry · Feb 16, 2021 · ce24705 · ce24705
1 parent 93a421a
commit ce24705
Show file tree

Hide file tree

Showing 12 changed files with 8 additions and 252 deletions.
diff --git a/actor/v7action/service_instance.go b/actor/v7action/service_instance.go
@@ -287,21 +287,6 @@ func (actor Actor) getServiceInstanceByNameAndSpace(serviceInstanceName string,
 	}
 }
 
-func (actor Actor) getManagedServiceInstanceByNameAndSpace(serviceInstanceName string, spaceGUID string, query ...ccv3.Query) (serviceInstance resources.ServiceInstance, included ccv3.IncludedResources, warnings ccv3.Warnings, err error) {
-	warnings, err = railway.Sequentially(
-		func() (warnings ccv3.Warnings, err error) {
-			serviceInstance, included, warnings, err = actor.getServiceInstanceByNameAndSpace(serviceInstanceName, spaceGUID, query...)
-			return
-		},
-		func() (warnings ccv3.Warnings, err error) {
-			err = assertServiceInstanceType(resources.ManagedServiceInstance, serviceInstance)
-			return
-		},
-	)
-
-	return serviceInstance, included, warnings, err
-}
-
 func (actor Actor) getServiceInstanceForUpdate(serviceInstanceName string, spaceGUID string, includePlan bool) (resources.ServiceInstance, resources.ServiceOffering, resources.ServiceBroker, ccv3.Warnings, error) {
 	var query []ccv3.Query
 	if includePlan {

diff --git a/actor/v7action/service_key.go b/actor/v7action/service_key.go
@@ -55,7 +55,7 @@ func (actor Actor) GetServiceKeysByServiceInstance(serviceInstanceName, spaceGUI
 
 	warnings, err := railway.Sequentially(
 		func() (warnings ccv3.Warnings, err error) {
-			serviceInstance, _, warnings, err = actor.getManagedServiceInstanceByNameAndSpace(serviceInstanceName, spaceGUID)
+			serviceInstance, _, warnings, err = actor.getServiceInstanceByNameAndSpace(serviceInstanceName, spaceGUID)
 			return
 		},
 		func() (warnings ccv3.Warnings, err error) {
@@ -147,7 +147,7 @@ func (actor Actor) getServiceKeyByServiceInstanceAndName(serviceInstanceName, se
 
 	warnings, err := railway.Sequentially(
 		func() (warnings ccv3.Warnings, err error) {
-			serviceInstance, _, warnings, err = actor.getManagedServiceInstanceByNameAndSpace(serviceInstanceName, spaceGUID)
+			serviceInstance, _, warnings, err = actor.getServiceInstanceByNameAndSpace(serviceInstanceName, spaceGUID)
 			return
 		},
 		func() (warnings ccv3.Warnings, err error) {

diff --git a/actor/v7action/service_key_test.go b/actor/v7action/service_key_test.go
@@ -262,29 +262,6 @@ var _ = Describe("Service Key Action", func() {
 			}))
 		})
 
-		When("service instance is user-provided", func() {
-			BeforeEach(func() {
-				fakeCloudControllerClient.GetServiceInstanceByNameAndSpaceReturns(
-					resources.ServiceInstance{
-						Name: serviceInstanceName,
-						GUID: serviceInstanceGUID,
-						Type: resources.UserProvidedServiceInstance,
-					},
-					ccv3.IncludedResources{},
-					ccv3.Warnings{"get instance warning"},
-					nil,
-				)
-			})
-
-			It("returns an error and warning", func() {
-				Expect(warnings).To(ContainElement("get instance warning"))
-				Expect(executionError).To(MatchError(actionerror.ServiceInstanceTypeError{
-					Name:         serviceInstanceName,
-					RequiredType: resources.ManagedServiceInstance,
-				}))
-			})
-		})
-
 		When("service instance not found", func() {
 			BeforeEach(func() {
 				fakeCloudControllerClient.GetServiceInstanceByNameAndSpaceReturns(
@@ -400,29 +377,6 @@ var _ = Describe("Service Key Action", func() {
 			Expect(key.GUID).To(Equal(serviceKeyGUID))
 		})
 
-		When("service instance is user-provided", func() {
-			BeforeEach(func() {
-				fakeCloudControllerClient.GetServiceInstanceByNameAndSpaceReturns(
-					resources.ServiceInstance{
-						Name: serviceInstanceName,
-						GUID: serviceInstanceGUID,
-						Type: resources.UserProvidedServiceInstance,
-					},
-					ccv3.IncludedResources{},
-					ccv3.Warnings{"get instance warning"},
-					nil,
-				)
-			})
-
-			It("returns an error and warning", func() {
-				Expect(warnings).To(ContainElement("get instance warning"))
-				Expect(executionError).To(MatchError(actionerror.ServiceInstanceTypeError{
-					Name:         serviceInstanceName,
-					RequiredType: resources.ManagedServiceInstance,
-				}))
-			})
-		})
-
 		When("service instance not found", func() {
 			BeforeEach(func() {
 				fakeCloudControllerClient.GetServiceInstanceByNameAndSpaceReturns(
@@ -568,29 +522,6 @@ var _ = Describe("Service Key Action", func() {
 			Expect(details.Credentials).To(Equal(types.JSONObject{"foo": "bar"}))
 		})
 
-		When("service instance is user-provided", func() {
-			BeforeEach(func() {
-				fakeCloudControllerClient.GetServiceInstanceByNameAndSpaceReturns(
-					resources.ServiceInstance{
-						Name: serviceInstanceName,
-						GUID: serviceInstanceGUID,
-						Type: resources.UserProvidedServiceInstance,
-					},
-					ccv3.IncludedResources{},
-					ccv3.Warnings{"get instance warning"},
-					nil,
-				)
-			})
-
-			It("returns an error and warning", func() {
-				Expect(warnings).To(ContainElement("get instance warning"))
-				Expect(executionError).To(MatchError(actionerror.ServiceInstanceTypeError{
-					Name:         serviceInstanceName,
-					RequiredType: resources.ManagedServiceInstance,
-				}))
-			})
-		})
-
 		When("service instance not found", func() {
 			BeforeEach(func() {
 				fakeCloudControllerClient.GetServiceInstanceByNameAndSpaceReturns(

diff --git a/command/translatableerror/service_keys_unsupported_user_provided_error.go b/command/translatableerror/service_keys_unsupported_user_provided_error.go
diff --git a/command/v7/delete_service_key_command.go b/command/v7/delete_service_key_command.go
@@ -49,8 +49,6 @@ func (cmd DeleteServiceKeyCommand) Execute(args []string) error {
 		cmd.displayNotFound()
 	case actionerror.ServiceInstanceNotFoundError:
 		return translatableerror.ServiceInstanceNotFoundError{Name: cmd.RequiredArgs.ServiceInstance}
-	case actionerror.ServiceInstanceTypeError:
-		return translatableerror.ServiceKeysNotSupportedWithUserProvidedServiceInstances{}
 	default:
 		return err
 	}

diff --git a/command/v7/delete_service_key_command_test.go b/command/v7/delete_service_key_command_test.go
@@ -8,7 +8,6 @@ import (
 	"code.cloudfoundry.org/cli/command/translatableerror"
 	v7 "code.cloudfoundry.org/cli/command/v7"
 	"code.cloudfoundry.org/cli/command/v7/v7fakes"
-	"code.cloudfoundry.org/cli/resources"
 	"code.cloudfoundry.org/cli/util/configv3"
 	"code.cloudfoundry.org/cli/util/ui"
 	. "github.com/onsi/ginkgo"
@@ -175,25 +174,7 @@ var _ = Describe("delete-service-key Command", func() {
 		})
 	})
 
-	When("service instance is user-provided", func() {
-		BeforeEach(func() {
-			fakeActor.DeleteServiceKeyByServiceInstanceAndNameReturns(
-				nil,
-				v7action.Warnings{"upsi warning"},
-				actionerror.ServiceInstanceTypeError{
-					Name:         fakeServiceInstanceName,
-					RequiredType: resources.ManagedServiceInstance,
-				},
-			)
-		})
-
-		It("returns an appropriate error", func() {
-			Expect(testUI.Err).To(Say("upsi warning"))
-			Expect(executeErr).To(MatchError(translatableerror.ServiceKeysNotSupportedWithUserProvidedServiceInstances{}))
-		})
-	})
-
-	When("there is another error", func() {
+	When("there is an error", func() {
 		BeforeEach(func() {
 			fakeActor.DeleteServiceKeyByServiceInstanceAndNameReturns(
 				nil,

diff --git a/command/v7/service_key_command.go b/command/v7/service_key_command.go
@@ -3,9 +3,7 @@ package v7
 import (
 	"encoding/json"
 
-	"code.cloudfoundry.org/cli/actor/actionerror"
 	"code.cloudfoundry.org/cli/command/flag"
-	"code.cloudfoundry.org/cli/command/translatableerror"
 )
 
 type ServiceKeyCommand struct {
@@ -46,9 +44,6 @@ func (cmd ServiceKeyCommand) guid() error {
 	case nil:
 		cmd.UI.DisplayText(key.GUID)
 		return nil
-	case actionerror.ServiceInstanceTypeError:
-		cmd.UI.DisplayWarnings(warnings)
-		return translatableerror.ServiceKeysNotSupportedWithUserProvidedServiceInstances{}
 	default:
 		cmd.UI.DisplayWarnings(warnings)
 		return err
@@ -73,11 +68,7 @@ func (cmd ServiceKeyCommand) details() error {
 		cmd.Config.TargetedSpace().GUID,
 	)
 	cmd.UI.DisplayWarnings(warnings)
-	switch err.(type) {
-	case nil:
-	case actionerror.ServiceInstanceTypeError:
-		return translatableerror.ServiceKeysNotSupportedWithUserProvidedServiceInstances{}
-	default:
+	if err != nil {
 		return err
 	}
 

diff --git a/command/v7/service_key_command_test.go b/command/v7/service_key_command_test.go
@@ -5,10 +5,8 @@ import (
 
 	"code.cloudfoundry.org/cli/types"
 
-	"code.cloudfoundry.org/cli/actor/actionerror"
 	"code.cloudfoundry.org/cli/actor/v7action"
 	"code.cloudfoundry.org/cli/command/commandfakes"
-	"code.cloudfoundry.org/cli/command/translatableerror"
 	v7 "code.cloudfoundry.org/cli/command/v7"
 	"code.cloudfoundry.org/cli/command/v7/v7fakes"
 	"code.cloudfoundry.org/cli/resources"
@@ -113,24 +111,6 @@ var _ = Describe("service-key Command", func() {
 			})
 		})
 
-		When("not a managed service instance", func() {
-			BeforeEach(func() {
-				fakeActor.GetServiceKeyDetailsByServiceInstanceAndNameReturns(
-					resources.ServiceCredentialBindingDetails{},
-					v7action.Warnings{"a warning"},
-					actionerror.ServiceInstanceTypeError{
-						Name:         fakeServiceInstanceName,
-						RequiredType: resources.ManagedServiceInstance,
-					},
-				)
-			})
-
-			It("prints warnings and returns an error", func() {
-				Expect(testUI.Err).To(Say("a warning"))
-				Expect(executeErr).To(MatchError(translatableerror.ServiceKeysNotSupportedWithUserProvidedServiceInstances{}))
-			})
-		})
-
 		When("actor returns another error", func() {
 			BeforeEach(func() {
 				fakeActor.GetServiceKeyDetailsByServiceInstanceAndNameReturns(
@@ -173,25 +153,7 @@ var _ = Describe("service-key Command", func() {
 			Expect(testUI.Err).NotTo(Say("a warning"))
 		})
 
-		When("not a managed service instance", func() {
-			BeforeEach(func() {
-				fakeActor.GetServiceKeyByServiceInstanceAndNameReturns(
-					resources.ServiceCredentialBinding{},
-					v7action.Warnings{"a warning"},
-					actionerror.ServiceInstanceTypeError{
-						Name:         fakeServiceInstanceName,
-						RequiredType: resources.ManagedServiceInstance,
-					},
-				)
-			})
-
-			It("prints warnings and returns an error", func() {
-				Expect(testUI.Err).To(Say("a warning"))
-				Expect(executeErr).To(MatchError(translatableerror.ServiceKeysNotSupportedWithUserProvidedServiceInstances{}))
-			})
-		})
-
-		When("actor returns another error", func() {
+		When("actor returns an error", func() {
 			BeforeEach(func() {
 				fakeActor.GetServiceKeyByServiceInstanceAndNameReturns(
 					resources.ServiceCredentialBinding{},

diff --git a/command/v7/service_keys_command.go b/command/v7/service_keys_command.go
@@ -3,9 +3,7 @@ package v7
 import (
 	"fmt"
 
-	"code.cloudfoundry.org/cli/actor/actionerror"
 	"code.cloudfoundry.org/cli/command/flag"
-	"code.cloudfoundry.org/cli/command/translatableerror"
 	"code.cloudfoundry.org/cli/resources"
 	"code.cloudfoundry.org/cli/util/ui"
 )
@@ -31,11 +29,7 @@ func (cmd ServiceKeysCommand) Execute(args []string) error {
 		cmd.Config.TargetedSpace().GUID,
 	)
 	cmd.UI.DisplayWarnings(warnings)
-	switch err.(type) {
-	case nil:
-	case actionerror.ServiceInstanceTypeError:
-		return translatableerror.ServiceKeysNotSupportedWithUserProvidedServiceInstances{}
-	default:
+	if err != nil {
 		return err
 	}
 

diff --git a/command/v7/service_keys_command_test.go b/command/v7/service_keys_command_test.go
@@ -3,10 +3,8 @@ package v7_test
 import (
 	"errors"
 
-	"code.cloudfoundry.org/cli/actor/actionerror"
 	"code.cloudfoundry.org/cli/actor/v7action"
 	"code.cloudfoundry.org/cli/command/commandfakes"
-	"code.cloudfoundry.org/cli/command/translatableerror"
 	v7 "code.cloudfoundry.org/cli/command/v7"
 	"code.cloudfoundry.org/cli/command/v7/v7fakes"
 	"code.cloudfoundry.org/cli/resources"
@@ -120,21 +118,6 @@ var _ = Describe("service-keys Command", func() {
 		})
 	})
 
-	When("the service instance is user-provided", func() {
-		BeforeEach(func() {
-			fakeActor.GetServiceKeysByServiceInstanceReturns(
-				nil,
-				v7action.Warnings{"fake warning"},
-				actionerror.ServiceInstanceTypeError{},
-			)
-		})
-
-		It("returns a helpful error and prints warnings", func() {
-			Expect(testUI.Err).To(Say("fake warning"))
-			Expect(executeErr).To(MatchError(translatableerror.ServiceKeysNotSupportedWithUserProvidedServiceInstances{}))
-		})
-	})
-
 	When("checking the target returns an error", func() {
 		BeforeEach(func() {
 			fakeSharedActor.CheckTargetReturns(errors.New("explode"))

diff --git a/integration/v7/isolated/service_key_command_test.go b/integration/v7/isolated/service_key_command_test.go
@@ -103,28 +103,7 @@ var _ = Describe("service-key command", func() {
 			})
 		})
 
-		When("service instance is user-provided", func() {
-			var serviceInstance string
-
-			BeforeEach(func() {
-				serviceInstance = helpers.NewServiceInstanceName()
-				Eventually(helpers.CF("cups", serviceInstance)).Should(Exit(0))
-			})
-
-			It("reports a helpful error", func() {
-				session := helpers.CF(command, serviceInstance, "no-such-key")
-				Eventually(session).Should(Exit(1))
-
-				Expect(session.Out).To(SatisfyAll(
-					Say(`Getting key no-such-key for service instance %s as %s\.\.\.\n`, serviceInstance, username),
-					Say(`FAILED\n`),
-				))
-
-				Expect(session.Err).To(Say(`Service keys are not supported for user-provided service instances`))
-			})
-		})
-
-		When("service instance is managed", func() {
+		When("service instance exists", func() {
 			var (
 				broker          *servicebrokerstub.ServiceBrokerStub
 				serviceInstance string