service: --params flag retrieves service instance params

actor/v7action/service_instance_details.go

@@ -71,10 +71,6 @@ func (actor Actor) GetServiceInstanceDetails(serviceInstanceName string, spaceGU
 			serviceInstanceDetails, warnings, err = actor.getServiceInstanceDetails(serviceInstanceName, spaceGUID)
 			return
 		},
-		func() (warnings ccv3.Warnings, err error) {
-			serviceInstanceDetails.Parameters, warnings = actor.getServiceInstanceParameters(serviceInstanceDetails.GUID)
-			return
-		},
 		func() (warnings ccv3.Warnings, err error) {
 			serviceInstanceDetails.SharedStatus, warnings, err = actor.getServiceInstanceSharedStatus(serviceInstanceDetails, spaceGUID)
 			return
@@ -97,6 +93,26 @@ func (actor Actor) GetServiceInstanceDetails(serviceInstanceName string, spaceGU
 	return serviceInstanceDetails, Warnings(warnings), nil
 }
 
+func (actor Actor) GetServiceInstanceParameters(serviceInstanceName string, spaceGUID string) (ServiceInstanceDetails, Warnings, error) {
+	var serviceInstanceDetails ServiceInstanceDetails
+
+	warnings, err := railway.Sequentially(
+		func() (warnings ccv3.Warnings, err error) {
+			serviceInstanceDetails.ServiceInstance, _, warnings, err = actor.getServiceInstanceByNameAndSpace(serviceInstanceName, spaceGUID)
+			return
+		},
+		func() (warnings ccv3.Warnings, err error) {
+			serviceInstanceDetails.Parameters, warnings = actor.getServiceInstanceParameters(serviceInstanceDetails.ServiceInstance.GUID)
+			return
+		},
+	)
+	if err != nil {
+		return ServiceInstanceDetails{}, Warnings(warnings), err
+	}
+
+	return serviceInstanceDetails, Warnings(warnings), nil
+}
+
 func (actor Actor) getServiceInstanceDetails(serviceInstanceName string, spaceGUID string) (ServiceInstanceDetails, ccv3.Warnings, error) {
 	query := []ccv3.Query{
 		{
@@ -144,15 +160,19 @@ func (actor Actor) getServiceInstanceDetails(serviceInstanceName string, spaceGU
 
 func (actor Actor) getServiceInstanceParameters(serviceInstanceGUID string) (ServiceInstanceParameters, ccv3.Warnings) {
 	params, warnings, err := actor.CloudControllerClient.GetServiceInstanceParameters(serviceInstanceGUID)
-	if err != nil {
-		if e, ok := err.(ccerror.V3UnexpectedResponseError); ok && len(e.Errors) > 0 {
-			return ServiceInstanceParameters{MissingReason: e.Errors[0].Detail}, warnings
-		} else {
-			return ServiceInstanceParameters{MissingReason: err.Error()}, warnings
-		}
-	}
 
-	return ServiceInstanceParameters{Value: params}, warnings
+	switch err := err.(type) {
+	case nil:
+		return ServiceInstanceParameters{Value: params}, warnings
+	case ccerror.ResourceNotFoundError: // Old API versions did not produce a helpful error
+		return ServiceInstanceParameters{MissingReason: "This service does not support fetching service instance parameters."}, warnings
+	case ccerror.ServiceInstanceParametersFetchNotSupportedError:
+		return ServiceInstanceParameters{MissingReason: err.Message}, warnings
+	case ccerror.V3UnexpectedResponseError:
+		return ServiceInstanceParameters{MissingReason: err.Errors[0].Detail}, warnings
+	default:
+		return ServiceInstanceParameters{MissingReason: err.Error()}, warnings
+	}
 }
 
 func (actor Actor) getServiceInstanceSharedStatus(serviceInstanceDetails ServiceInstanceDetails, targetedSpace string) (SharedStatus, ccv3.Warnings, error) {

actor/v7action/service_instance_details_test.go

@@ -73,12 +73,6 @@ var _ = Describe("Service Instance Details Action", func() {
 				nil,
 			)
 
-			fakeCloudControllerClient.GetServiceInstanceParametersReturns(
-				types.JSONObject{"foo": "bar"},
-				ccv3.Warnings{"some-parameters-warning"},
-				nil,
-			)
-
 			fakeCloudControllerClient.GetFeatureFlagReturns(
 				resources.FeatureFlag{Name: "service_instance_sharing", Enabled: true},
 				ccv3.Warnings{},
@@ -129,7 +123,6 @@ var _ = Describe("Service Instance Details Action", func() {
 			Expect(executionError).NotTo(HaveOccurred())
 			Expect(warnings).To(ConsistOf(
 				"some-service-instance-warning",
-				"some-parameters-warning",
 				"some-bindings-warning",
 			))
 		})
@@ -155,9 +148,7 @@ var _ = Describe("Service Instance Details Action", func() {
 					ServicePlan:       resources.ServicePlan{Name: servicePlanName},
 					ServiceBrokerName: serviceBrokerName,
 					SharedStatus:      SharedStatus{},
-					Parameters: ServiceInstanceParameters{
-						Value: types.JSONObject{"foo": "bar"},
-					},
+					Parameters:        ServiceInstanceParameters{},
 					BoundApps: []resources.ServiceCredentialBinding{
 						{
 							Type:    resources.AppBinding,
@@ -263,53 +254,6 @@ var _ = Describe("Service Instance Details Action", func() {
 			})
 		})
 
-		Describe("getting the parameters", func() {
-			It("makes the correct call to get the parameters", func() {
-				Expect(fakeCloudControllerClient.GetServiceInstanceParametersCallCount()).To(Equal(1))
-				Expect(fakeCloudControllerClient.GetServiceInstanceParametersArgsForCall(0)).To(Equal(serviceInstanceGUID))
-			})
-
-			When("getting the parameters fails with an expected error", func() {
-				BeforeEach(func() {
-					fakeCloudControllerClient.GetServiceInstanceParametersReturns(
-						types.JSONObject{},
-						ccv3.Warnings{"some-parameters-warning"},
-						ccerror.V3UnexpectedResponseError{
-							V3ErrorResponse: ccerror.V3ErrorResponse{
-								Errors: []ccerror.V3Error{{
-									Code:   1234,
-									Detail: "cannot get parameters reason",
-									Title:  "CF-SomeRandomError",
-								}},
-							},
-						},
-					)
-				})
-
-				It("does not return an error, but returns warnings and the reason", func() {
-					Expect(executionError).NotTo(HaveOccurred())
-					Expect(warnings).To(ContainElement("some-parameters-warning"))
-					Expect(serviceInstance.Parameters.MissingReason).To(Equal("cannot get parameters reason"))
-				})
-			})
-
-			When("getting the parameters fails with an another error", func() {
-				BeforeEach(func() {
-					fakeCloudControllerClient.GetServiceInstanceParametersReturns(
-						types.JSONObject{},
-						ccv3.Warnings{"some-parameters-warning"},
-						errors.New("not expected"),
-					)
-				})
-
-				It("does not return an error, but returns warnings and the reason", func() {
-					Expect(executionError).NotTo(HaveOccurred())
-					Expect(warnings).To(ContainElement("some-parameters-warning"))
-					Expect(serviceInstance.Parameters.MissingReason).To(Equal("not expected"))
-				})
-			})
-		})
-
 		Describe("sharing", func() {
 			When("targeting originating service instance space", func() {
 				When("the service instance has shared spaces", func() {
@@ -467,7 +411,7 @@ var _ = Describe("Service Instance Details Action", func() {
 					It("returns an empty service instance, warnings, and the error", func() {
 						Expect(serviceInstance).To(Equal(ServiceInstanceDetails{}))
 						Expect(executionError).To(MatchError("error getting feature flag"))
-						Expect(warnings).To(ConsistOf("some-service-instance-warning", "some-parameters-warning", warningMessage))
+						Expect(warnings).To(ConsistOf("some-service-instance-warning", warningMessage))
 					})
 				})
 
@@ -485,7 +429,7 @@ var _ = Describe("Service Instance Details Action", func() {
 					It("returns an empty service instance, warnings, and the error", func() {
 						Expect(serviceInstance).To(Equal(ServiceInstanceDetails{}))
 						Expect(executionError).To(MatchError("error getting offering"))
-						Expect(warnings).To(ConsistOf("some-service-instance-warning", "some-parameters-warning", warningMessage))
+						Expect(warnings).To(ConsistOf("some-service-instance-warning", warningMessage))
 					})
 				})
 
@@ -518,7 +462,7 @@ var _ = Describe("Service Instance Details Action", func() {
 					It("returns an empty service instance, warnings, and the error", func() {
 						Expect(serviceInstance).To(Equal(ServiceInstanceDetails{}))
 						Expect(executionError).To(MatchError("no service instance"))
-						Expect(warnings).To(ConsistOf("some-service-instance-warning", "some-parameters-warning", warningMessage))
+						Expect(warnings).To(ConsistOf("some-service-instance-warning", warningMessage))
 					})
 				})
 
@@ -542,7 +486,7 @@ var _ = Describe("Service Instance Details Action", func() {
 					It("returns an empty service instance, warnings, and the error", func() {
 						Expect(serviceInstance).To(Equal(ServiceInstanceDetails{}))
 						Expect(executionError).To(MatchError("no service instance"))
-						Expect(warnings).To(ConsistOf("some-service-instance-warning", "some-parameters-warning", warningMessage))
+						Expect(warnings).To(ConsistOf("some-service-instance-warning", warningMessage))
 					})
 				})
 
@@ -764,4 +708,184 @@ var _ = Describe("Service Instance Details Action", func() {
 			})
 		})
 	})
+
+	Describe("GetServiceInstanceParameters", func() {
+		const (
+			serviceInstanceName = "some-service-instance-name"
+			serviceInstanceGUID = "some-service-instance-guid"
+			spaceGUID           = "some-source-space-guid"
+		)
+
+		var (
+			serviceInstance ServiceInstanceDetails
+			warnings        Warnings
+			executionError  error
+		)
+
+		BeforeEach(func() {
+			fakeCloudControllerClient.GetServiceInstanceByNameAndSpaceReturns(
+				resources.ServiceInstance{
+					Type:      resources.ManagedServiceInstance,
+					Name:      serviceInstanceName,
+					GUID:      serviceInstanceGUID,
+					SpaceGUID: spaceGUID,
+				},
+				ccv3.IncludedResources{},
+				ccv3.Warnings{"some-service-instance-warning"},
+				nil,
+			)
+
+			fakeCloudControllerClient.GetServiceInstanceParametersReturns(
+				types.JSONObject{"foo": "bar"},
+				ccv3.Warnings{"some-parameters-warning"},
+				nil,
+			)
+
+		})
+
+		JustBeforeEach(func() {
+			serviceInstance, warnings, executionError = actor.GetServiceInstanceParameters(serviceInstanceName, spaceGUID)
+		})
+
+		Describe("getting the service instance", func() {
+			It("makes the correct call to get the service instance", func() {
+				Expect(fakeCloudControllerClient.GetServiceInstanceByNameAndSpaceCallCount()).To(Equal(1))
+				actualName, actualSpaceGUID, actualQuery := fakeCloudControllerClient.GetServiceInstanceByNameAndSpaceArgsForCall(0)
+				Expect(actualName).To(Equal(serviceInstanceName))
+				Expect(actualSpaceGUID).To(Equal(spaceGUID))
+				Expect(actualQuery).To(BeNil())
+			})
+
+			When("the service instance cannot be found", func() {
+				BeforeEach(func() {
+					fakeCloudControllerClient.GetServiceInstanceByNameAndSpaceReturns(
+						resources.ServiceInstance{},
+						ccv3.IncludedResources{},
+						ccv3.Warnings{},
+						ccerror.ServiceInstanceNotFoundError{Name: serviceInstanceName},
+					)
+				})
+
+				It("returns an error and warnings", func() {
+					Expect(executionError).To(MatchError(actionerror.ServiceInstanceNotFoundError{Name: serviceInstanceName}))
+				})
+
+				It("does not attempt any other requests", func() {
+					Expect(fakeCloudControllerClient.GetServiceInstanceParametersCallCount()).To(Equal(0))
+					Expect(fakeCloudControllerClient.GetFeatureFlagCallCount()).To(Equal(0))
+					Expect(fakeCloudControllerClient.GetServiceOfferingByGUIDCallCount()).To(Equal(0))
+					Expect(fakeCloudControllerClient.GetServiceInstanceSharedSpacesCallCount()).To(Equal(0))
+					Expect(fakeCloudControllerClient.GetServicePlanByGUIDCallCount()).To(Equal(0))
+					Expect(fakeCloudControllerClient.GetServiceCredentialBindingsCallCount()).To(Equal(0))
+				})
+			})
+
+			When("getting the service instance returns an error", func() {
+				BeforeEach(func() {
+					fakeCloudControllerClient.GetServiceInstanceByNameAndSpaceReturns(
+						resources.ServiceInstance{},
+						ccv3.IncludedResources{},
+						ccv3.Warnings{"some-service-instance-warning"},
+						errors.New("no service instance"))
+				})
+
+				It("returns an error and warnings", func() {
+					Expect(executionError).To(MatchError("no service instance"))
+					Expect(warnings).To(ConsistOf("some-service-instance-warning"))
+				})
+
+				It("does not attempt any other requests", func() {
+					Expect(fakeCloudControllerClient.GetServiceInstanceParametersCallCount()).To(Equal(0))
+					Expect(fakeCloudControllerClient.GetFeatureFlagCallCount()).To(Equal(0))
+					Expect(fakeCloudControllerClient.GetServiceOfferingByGUIDCallCount()).To(Equal(0))
+					Expect(fakeCloudControllerClient.GetServiceInstanceSharedSpacesCallCount()).To(Equal(0))
+					Expect(fakeCloudControllerClient.GetServicePlanByGUIDCallCount()).To(Equal(0))
+					Expect(fakeCloudControllerClient.GetServiceCredentialBindingsCallCount()).To(Equal(0))
+				})
+			})
+		})
+
+		Describe("getting the parameters", func() {
+			It("makes the correct call to get the parameters", func() {
+				Expect(fakeCloudControllerClient.GetServiceInstanceParametersCallCount()).To(Equal(1))
+				Expect(fakeCloudControllerClient.GetServiceInstanceParametersArgsForCall(0)).To(Equal(serviceInstanceGUID))
+			})
+
+			When("getting the parameters fails with a V3UnexpectedResponseError", func() {
+				BeforeEach(func() {
+					fakeCloudControllerClient.GetServiceInstanceParametersReturns(
+						types.JSONObject{},
+						ccv3.Warnings{"some-parameters-warning"},
+						ccerror.V3UnexpectedResponseError{
+							V3ErrorResponse: ccerror.V3ErrorResponse{
+								Errors: []ccerror.V3Error{{
+									Code:   1234,
+									Detail: "cannot get parameters reason",
+									Title:  "CF-SomeRandomError",
+								}},
+							},
+						},
+					)
+				})
+
+				It("does not return an error, but returns warnings and the reason", func() {
+					Expect(executionError).NotTo(HaveOccurred())
+					Expect(warnings).To(ContainElement("some-parameters-warning"))
+					Expect(serviceInstance.Parameters.MissingReason).To(Equal("cannot get parameters reason"))
+				})
+			})
+
+			When("getting the parameters fails with a ServiceInstanceParametersFetchNotSupportedError", func() {
+				BeforeEach(func() {
+					fakeCloudControllerClient.GetServiceInstanceParametersReturns(
+						types.JSONObject{},
+						ccv3.Warnings{"some-parameters-warning"},
+						ccerror.ServiceInstanceParametersFetchNotSupportedError{
+							Message: "This service does not support fetching service instance parameters.",
+						},
+					)
+				})
+
+				It("does not return an error, but returns warnings and the reason", func() {
+					Expect(executionError).NotTo(HaveOccurred())
+					Expect(warnings).To(ContainElement("some-parameters-warning"))
+					Expect(serviceInstance.Parameters.MissingReason).To(Equal("This service does not support fetching service instance parameters."))
+				})
+			})
+
+			When("getting the parameters fails with a ResourceNotFoundError", func() {
+				BeforeEach(func() {
+					fakeCloudControllerClient.GetServiceInstanceParametersReturns(
+						types.JSONObject{},
+						ccv3.Warnings{"some-parameters-warning"},
+						ccerror.ResourceNotFoundError{
+							Message: "Service instance not found",
+						},
+					)
+				})
+
+				It("converts the error to fetching not supported", func() {
+					Expect(executionError).NotTo(HaveOccurred())
+					Expect(warnings).To(ContainElement("some-parameters-warning"))
+					Expect(serviceInstance.Parameters.MissingReason).To(Equal("This service does not support fetching service instance parameters."))
+				})
+			})
+
+			When("getting the parameters fails with an another error", func() {
+				BeforeEach(func() {
+					fakeCloudControllerClient.GetServiceInstanceParametersReturns(
+						types.JSONObject{},
+						ccv3.Warnings{"some-parameters-warning"},
+						errors.New("not expected"),
+					)
+				})
+
+				It("does not return an error, but returns warnings and the reason", func() {
+					Expect(executionError).NotTo(HaveOccurred())
+					Expect(warnings).To(ContainElement("some-parameters-warning"))
+					Expect(serviceInstance.Parameters.MissingReason).To(Equal("not expected"))
+				})
+			})
+		})
+	})
 })

api/cloudcontroller/ccerror/service_instance_parameters_fetch_not_supported_error.go

@@ -0,0 +1,12 @@
+package ccerror
+
+// ServiceInstanceParametersFetchNotSupportedError is returned when
+// the service instance is user-provided or
+// service instance fetching is not supported for managed instances
+type ServiceInstanceParametersFetchNotSupportedError struct {
+	Message string
+}
+
+func (e ServiceInstanceParametersFetchNotSupportedError) Error() string {
+	return e.Message
+}

api/cloudcontroller/ccv3/errors.go

@@ -115,9 +115,10 @@ func handleBadRequest(errorResponse ccerror.V3Error, _ *cloudcontroller.Request)
 	switch errorResponse.Detail {
 	case "Bad request: Cannot stage package whose state is not ready.":
 		return ccerror.InvalidStateError{}
+	case "This service does not support fetching service instance parameters.":
+		return ccerror.ServiceInstanceParametersFetchNotSupportedError{Message: errorResponse.Detail}
 	default:
 		return ccerror.BadRequestError{Message: errorResponse.Detail}
-
 	}
 }