chore: report database error during LastOperation

We now differentiate between an instance not existing, and having an error getting the record from the database.
All the other endpoints did this, so this just gets LastOperation to follow the same pattern

brokerapi/broker/last_instance_operation.go

@@ -4,13 +4,13 @@ import (
 	"context"
 	"fmt"
 
-	"code.cloudfoundry.org/lager/v3"
-	"github.com/pivotal-cf/brokerapi/v11/domain"
 	"github.com/pivotal-cf/brokerapi/v11/domain/apiresponses"
 
+	"code.cloudfoundry.org/lager/v3"
 	"github.com/cloudfoundry/cloud-service-broker/v2/dbservice/models"
 	"github.com/cloudfoundry/cloud-service-broker/v2/pkg/broker"
 	"github.com/cloudfoundry/cloud-service-broker/v2/utils/correlation"
+	"github.com/pivotal-cf/brokerapi/v11/domain"
 )
 
 // LastOperation fetches last operation state for a service instance.
@@ -24,9 +24,21 @@ func (broker *ServiceBroker) LastOperation(ctx context.Context, instanceID strin
 		"operation_data": details.OperationData,
 	})
 
+	// make sure that instance actually exists
+	// Technically according to OSBAPI we should return HTTP 410 Gone only if the operation is a Deprovision,
+	// and for Provision or Update we should return an HTTP 404 Not Found, but we do not do this, and the
+	// pivotal-cf/brokerapi/apiresponses package does not provide the required error to do this.
+	exists, err := broker.store.ExistsServiceInstanceDetails(instanceID)
+	switch {
+	case err != nil:
+		return domain.LastOperation{}, fmt.Errorf("database error checking for existing instance: %s", err)
+	case !exists:
+		return domain.LastOperation{}, apiresponses.ErrInstanceDoesNotExist
+	}
+
 	instance, err := broker.store.GetServiceInstanceDetails(instanceID)
 	if err != nil {
-		return domain.LastOperation{}, apiresponses.ErrInstanceDoesNotExist
+		return domain.LastOperation{}, fmt.Errorf("error getting service instance details: %w", err)
 	}
 
 	_, serviceProvider, err := broker.getDefinitionAndProvider(instance.ServiceGUID)

brokerapi/broker/last_instance_operation_test.go

@@ -66,6 +66,7 @@ var _ = Describe("LastInstanceOperation", func() {
 		}
 
 		fakeStorage = &brokerfakes.FakeStorage{}
+		fakeStorage.ExistsServiceInstanceDetailsReturns(true, nil)
 		fakeStorage.GetServiceInstanceDetailsReturns(
 			storage.ServiceInstanceDetails{
 				GUID:             instanceID,
@@ -75,7 +76,9 @@ var _ = Describe("LastInstanceOperation", func() {
 				ServiceGUID:      offeringID,
 				SpaceGUID:        spaceID,
 				OrganizationGUID: orgID,
-			}, nil)
+			},
+			nil,
+		)
 
 		serviceBroker = must(broker.New(brokerConfig, fakeStorage, utils.NewLogger("brokers-test")))
 
@@ -228,14 +231,25 @@ var _ = Describe("LastInstanceOperation", func() {
 	})
 
 	Describe("storage errors", func() {
+		Context("storage error when checking whether SI exists", func() {
+			BeforeEach(func() {
+				fakeStorage.ExistsServiceInstanceDetailsReturns(false, errors.New("failed to check whether SI exists"))
+			})
+
+			It("should error", func() {
+				_, err := serviceBroker.LastOperation(context.TODO(), instanceID, pollDetails)
+				Expect(err).To(MatchError("database error checking for existing instance: failed to check whether SI exists"))
+			})
+		})
+
 		Context("storage errors when getting SI details", func() {
 			BeforeEach(func() {
 				fakeStorage.GetServiceInstanceDetailsReturns(storage.ServiceInstanceDetails{}, errors.New("failed to get SI details"))
 			})
 
 			It("should error", func() {
 				_, err := serviceBroker.LastOperation(context.TODO(), instanceID, pollDetails)
-				Expect(err).To(MatchError("instance does not exist"))
+				Expect(err).To(MatchError("error getting service instance details: failed to get SI details"))
 			})
 		})
 
@@ -291,7 +305,7 @@ var _ = Describe("LastInstanceOperation", func() {
 		})
 	})
 
-	Describe("Service provider error", func() {
+	Describe("service provider error", func() {
 		Context("service provider errors when deleting provider instance data", func() {
 			BeforeEach(func() {
 				fakeStorage.GetServiceInstanceDetailsReturns(
@@ -314,7 +328,7 @@ var _ = Describe("LastInstanceOperation", func() {
 
 	Describe("service instance does not exist", func() {
 		BeforeEach(func() {
-			fakeStorage.GetServiceInstanceDetailsReturns(storage.ServiceInstanceDetails{}, errors.New("does not exist"))
+			fakeStorage.ExistsServiceInstanceDetailsReturns(false, nil)
 		})
 
 		It("should return HTTP 410 as per OSBAPI spec", func() {