fix: fix incorrect .Is() method on update skipped due to backoff error (

#4213)

internal/dataplane/kong_client.go

@@ -437,7 +437,7 @@ func (c *KongClient) maybeSendOutToKonnectClient(ctx context.Context, s *kongsta
 		// In case of an error, we only log it since we don't want the Konnect to affect the basic functionality
 		// of the controller.
 
-		if errors.Is(err, sendconfig.ErrUpdateSkippedDueToBackoffStrategy{}) {
+		if errors.As(err, &sendconfig.UpdateSkippedDueToBackoffStrategyError{}) {
 			c.logger.WithError(err).Warn("Skipped pushing configuration to Konnect")
 		} else {
 			c.logger.WithError(err).Warn("Failed pushing configuration to Konnect")

internal/dataplane/sendconfig/backoff_strategy.go

@@ -2,7 +2,6 @@ package sendconfig
 
 import (
 	"context"
-	"errors"
 	"fmt"
 
 	"github.com/sirupsen/logrus"
@@ -11,22 +10,18 @@ import (
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/metrics"
 )
 
-type ErrUpdateSkippedDueToBackoffStrategy struct {
+type UpdateSkippedDueToBackoffStrategyError struct {
 	explanation string
 }
 
-func NewErrUpdateSkippedDueToBackoffStrategy(explanation string) ErrUpdateSkippedDueToBackoffStrategy {
-	return ErrUpdateSkippedDueToBackoffStrategy{explanation: explanation}
+func NewUpdateSkippedDueToBackoffStrategyError(explanation string) UpdateSkippedDueToBackoffStrategyError {
+	return UpdateSkippedDueToBackoffStrategyError{explanation: explanation}
 }
 
-func (e ErrUpdateSkippedDueToBackoffStrategy) Error() string {
+func (e UpdateSkippedDueToBackoffStrategyError) Error() string {
 	return fmt.Sprintf("update skipped due to a backoff strategy not being satisfied: %s", e.explanation)
 }
 
-func (e ErrUpdateSkippedDueToBackoffStrategy) Is(err error) bool {
-	return errors.Is(err, ErrUpdateSkippedDueToBackoffStrategy{})
-}
-
 // UpdateStrategyWithBackoff decorates any UpdateStrategy to respect a passed adminapi.UpdateBackoffStrategy.
 type UpdateStrategyWithBackoff struct {
 	decorated       UpdateStrategy
@@ -57,7 +52,7 @@ func (s UpdateStrategyWithBackoff) Update(ctx context.Context, targetContent Con
 	resourceErrorsParseErr error,
 ) {
 	if canUpdate, whyNot := s.backoffStrategy.CanUpdate(targetContent.Hash); !canUpdate {
-		return NewErrUpdateSkippedDueToBackoffStrategy(whyNot), nil, nil
+		return NewUpdateSkippedDueToBackoffStrategyError(whyNot), nil, nil
 	}
 
 	err, resourceErrors, resourceErrorsParseErr = s.decorated.Update(ctx, targetContent)

internal/dataplane/sendconfig/backoff_strategy_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/deckerrors"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/sendconfig"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/metrics"
 )
@@ -107,7 +108,7 @@ func TestUpdateStrategyWithBackoff(t *testing.T) {
 			updateShouldBeAllowed: false,
 
 			expectUpdateCalled: false,
-			expectError:        sendconfig.NewErrUpdateSkippedDueToBackoffStrategy("some reason"),
+			expectError:        sendconfig.NewUpdateSkippedDueToBackoffStrategyError("some reason"),
 		},
 	}
 
@@ -130,3 +131,37 @@ func TestUpdateStrategyWithBackoff(t *testing.T) {
 		})
 	}
 }
+
+func TestUpdateSkippedDueToBackoffStrategyError(t *testing.T) {
+	skippedErr := sendconfig.NewUpdateSkippedDueToBackoffStrategyError("reason")
+
+	t.Run("errors.Is()", func(t *testing.T) {
+		assert.False(t,
+			errors.Is(skippedErr, deckerrors.ConfigConflictError{
+				Err: sendconfig.NewUpdateSkippedDueToBackoffStrategyError("different reason"),
+			}),
+			"shouldn't panic when using errors.Is() with NewUpdateSkippedDueToBackoffStrategyError",
+		)
+
+		assert.False(t, errors.Is(skippedErr, errors.New("")),
+			"empty error doesn't match",
+		)
+		assert.False(t, errors.Is(skippedErr, sendconfig.NewUpdateSkippedDueToBackoffStrategyError("different reason")),
+			"error with different reason shouldn't match",
+		)
+		assert.True(t, errors.Is(skippedErr, skippedErr),
+			"error with the same reason should match",
+		)
+	})
+
+	t.Run("errors.As()", func(t *testing.T) {
+		err := sendconfig.NewUpdateSkippedDueToBackoffStrategyError("reason")
+		assert.True(t, errors.As(skippedErr, &err),
+			"error with the same reason but wrapped should match",
+		)
+		err2 := sendconfig.NewUpdateSkippedDueToBackoffStrategyError("reason 2")
+		assert.True(t, errors.As(skippedErr, &err2),
+			"error with different reason should match",
+		)
+	})
+}

internal/dataplane/sendconfig/sendconfig.go

@@ -82,7 +82,7 @@ func PerformUpdate(
 	metricsProtocol := updateStrategy.MetricsProtocol()
 	if err != nil {
 		// Not pushing metrics in case it's an update skip due to a backoff.
-		if errors.Is(err, ErrUpdateSkippedDueToBackoffStrategy{}) {
+		if errors.As(err, &UpdateSkippedDueToBackoffStrategyError{}) {
 			return nil, []failures.ResourceFailure{}, err
 		}