Improve Service Binding Cleanup job

internal/broker/client.go

@@ -29,6 +29,21 @@ const (
 	unbindTmpl         = "%s%s/%s/service_bindings/%s"
 )
 
+type UnexpectedStatusCodeError struct {
+	ExpectedStatusCode, UnexpectedStatusCode int
+}
+
+func NewUnexpectedStatusCodeError(expectedStatusCode, unexpectedStatusCode int) UnexpectedStatusCodeError {
+	return UnexpectedStatusCodeError{
+		ExpectedStatusCode:   expectedStatusCode,
+		UnexpectedStatusCode: unexpectedStatusCode,
+	}
+}
+
+func (e UnexpectedStatusCodeError) Error() string {
+	return fmt.Sprintf("unexpected status code: want %d, got: %d", e.ExpectedStatusCode, e.UnexpectedStatusCode)
+}
+
 type (
 	contextDTO struct {
 		GlobalAccountID string `json:"globalaccount_id"`
@@ -317,8 +332,7 @@ func (c *Client) executeRequest(method, url string, expectedStatus int, requestB
 
 	defer c.warnOnError(resp.Body.Close)
 	if resp.StatusCode != expectedStatus {
-		return fmt.Errorf("got unexpected status code while calling Kyma Environment Broker: want: %d, got: %d",
-			expectedStatus, resp.StatusCode)
+		return NewUnexpectedStatusCodeError(expectedStatus, resp.StatusCode)
 	}
 
 	err = json.NewDecoder(resp.Body).Decode(responseBody)

internal/servicebindingcleanup/service.go

@@ -5,9 +5,11 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"net/http"
 	"time"
 
 	"github.com/kyma-project/kyma-environment-broker/internal"
+	"github.com/kyma-project/kyma-environment-broker/internal/broker"
 	"github.com/kyma-project/kyma-environment-broker/internal/storage"
 )
 
@@ -39,17 +41,20 @@ func (s *Service) PerformCleanup() error {
 	slog.Info(fmt.Sprintf("Expired Service Bindings: %d", len(bindings)))
 	if s.dryRun {
 		return nil
-	} else {
-		slog.Info("Requesting Service Bindings removal...")
-		for _, binding := range bindings {
-			err := s.brokerClient.Unbind(binding)
-			if err != nil {
-				if errors.Is(err, context.DeadlineExceeded) {
-					continue
-				}
-				slog.Error(fmt.Sprintf("while sending unbind request for service binding ID %q: %s", binding.ID, err))
-				break
+	}
+	slog.Info("Requesting Service Bindings removal...")
+	for _, binding := range bindings {
+		if err := s.brokerClient.Unbind(binding); err != nil {
+			var unexpectedStatusCodeErr broker.UnexpectedStatusCodeError
+			if errors.Is(err, context.DeadlineExceeded) {
+				continue
+			}
+			if errors.As(err, &unexpectedStatusCodeErr) && unexpectedStatusCodeErr.UnexpectedStatusCode == http.StatusGone {
+				slog.Info(fmt.Sprintf("instance with ID: %q does not exist for service binding with ID %q", binding.InstanceID, binding.ID))
+				continue
 			}
+			slog.Error(fmt.Sprintf("while sending unbind request for service binding ID %q: %s", binding.ID, err))
+			break
 		}
 	}
 	return nil

internal/servicebindingcleanup/service_test.go

@@ -123,6 +123,83 @@ func TestServiceBindingCleanupJob(t *testing.T) {
 		require.NoError(t, bindingsStorage.Delete(expectedBinding.InstanceID, expectedBinding.ID))
 		handler.setHandlerFunc(handler.deleteServiceBindingFromStorage)
 	})
+
+	t.Run("should only unbind expired service binding", func(t *testing.T) {
+		// given
+		activeBinding := internal.Binding{
+			ID:         "active-binding-id",
+			InstanceID: "instance-id",
+			ExpiresAt:  time.Now().Add(time.Hour),
+		}
+		expiredBinding := internal.Binding{
+			ID:         "expired-binding-id",
+			InstanceID: "instance-id",
+			ExpiresAt:  time.Now().Add(-time.Hour),
+		}
+		require.NoError(t, bindingsStorage.Insert(&activeBinding))
+		require.NoError(t, bindingsStorage.Insert(&expiredBinding))
+
+		svc := servicebindingcleanup.NewService(false, brokerClient, bindingsStorage)
+
+		// when
+		err := svc.PerformCleanup()
+		require.NoError(t, err)
+
+		// then
+		actualBinding, err := bindingsStorage.Get(activeBinding.InstanceID, activeBinding.ID)
+		require.NoError(t, err)
+		assert.Equal(t, activeBinding.ID, actualBinding.ID)
+
+		_, err = bindingsStorage.Get(expiredBinding.InstanceID, expiredBinding.ID)
+		assert.True(t, dberr.IsNotFound(err))
+	})
+
+	t.Run("should continue cleanup when unbind endpoint returns 410 status code", func(t *testing.T) {
+		// given
+		binding := internal.Binding{
+			ID:         "binding-id-1",
+			InstanceID: "instance-id-1",
+			ExpiresAt:  time.Now().Add(-time.Hour),
+		}
+		require.NoError(t, bindingsStorage.Insert(&binding))
+
+		svc := servicebindingcleanup.NewService(false, brokerClient, bindingsStorage)
+
+		// when
+		handler.setHandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			bindingID := r.PathValue("binding_id")
+			instanceID := r.PathValue("instance_id")
+			if len(bindingID) == 0 || len(instanceID) == 0 {
+				w.WriteHeader(http.StatusBadRequest)
+				return
+			}
+			_, err := bindingsStorage.Get(instanceID, bindingID)
+			if err != nil {
+				w.WriteHeader(http.StatusNotFound)
+				return
+			}
+			if err = bindingsStorage.Delete(instanceID, bindingID); err != nil {
+				w.WriteHeader(http.StatusInternalServerError)
+				return
+			}
+			w.WriteHeader(http.StatusGone)
+			encoder := json.NewEncoder(w)
+			if err := encoder.Encode(apiresponses.EmptyResponse{}); err != nil {
+				w.WriteHeader(http.StatusInternalServerError)
+				return
+			}
+		})
+
+		err := svc.PerformCleanup()
+		require.NoError(t, err)
+
+		// then
+		_, err = bindingsStorage.Get(binding.InstanceID, binding.ID)
+		assert.True(t, dberr.IsNotFound(err))
+
+		// cleanup
+		handler.setHandlerFunc(handler.deleteServiceBindingFromStorage)
+	})
 }
 
 type server struct {

internal/storage/driver/postsql/binding.go

@@ -151,11 +151,8 @@ func (s *Binding) toBinding(dto dbmodel.BindingDTO) (internal.Binding, error) {
 
 func (s *Binding) toExpiredBinding(dto dbmodel.BindingDTO) (internal.Binding, error) {
 	return internal.Binding{
-		ID:                dto.ID,
-		InstanceID:        dto.InstanceID,
-		CreatedAt:         dto.CreatedAt,
-		ExpirationSeconds: dto.ExpirationSeconds,
-		CreatedBy:         dto.CreatedBy,
-		ExpiresAt:         dto.ExpiresAt,
+		ID:         dto.ID,
+		InstanceID: dto.InstanceID,
+		ExpiresAt:  dto.ExpiresAt,
 	}, nil
 }

internal/storage/postsql/read.go

@@ -59,10 +59,9 @@ func (r readSession) ListExpiredBindings() ([]dbmodel.BindingDTO, error) {
 	currentTime := time.Now().UTC()
 	var bindings []dbmodel.BindingDTO
 	_, err := r.session.
-		Select("*").
+		Select("id", "instance_id", "expires_at").
 		From(BindingsTableName).
 		Where(dbr.Lte("expires_at", currentTime)).
-		OrderBy("created_at").
 		Load(&bindings)
 
 	if err != nil {