Skip to content

Commit

Permalink
enable goerr113
Browse files Browse the repository at this point in the history
  • Loading branch information
k15r committed Dec 14, 2023
1 parent 2d8a01a commit df6ebd1
Show file tree
Hide file tree
Showing 33 changed files with 212 additions and 164 deletions.
1 change: 0 additions & 1 deletion .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ linters:
- goconst
- gocritic
- godox
- goerr113
- gomnd
- gosec
- inamedparam
Expand Down
13 changes: 9 additions & 4 deletions hack/e2e/common/eventing/publisher.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ const (
CloudEventPublishEndpointFormat = "%s/publish"
)

var (
ErrFailedSendingCE = errors.New("failed to send cloudevent")
ErrEncodingNotSupported = errors.New("unsupported cloudevent encoding")
)

type Publisher struct {
clientCE client.Client
clientHTTP *http.Client
Expand Down Expand Up @@ -123,7 +128,7 @@ func (p *Publisher) SendCloudEvent(event *cloudevents.Event, encoding binding.En
}
default:
{
return fmt.Errorf("failed to use unsupported cloudevent encoding:[%s]", encoding.String())
return fmt.Errorf("%w:[%s]", ErrEncodingNotSupported, encoding.String())
}
}

Expand All @@ -140,11 +145,11 @@ func (p *Publisher) SendCloudEvent(event *cloudevents.Event, encoding binding.En
switch {
case cloudevents.IsUndelivered(result):
{
return fmt.Errorf("failed to send cloudevent-%s undelivered:[%s] response:[%s]", encoding.String(), ce.Type(), result)
return fmt.Errorf("%w: encoding:[%s] undelivered:[%s] response:[%s]", ErrFailedSendingCE, encoding.String(), ce.Type(), result)
}
case cloudevents.IsNACK(result):
{
return fmt.Errorf("failed to send cloudevent-%s nack:[%s] response:[%s]", encoding.String(), ce.Type(), result)
return fmt.Errorf("%w: encoding:[%s] nack:[%s] response:[%s]", ErrFailedSendingCE, encoding.String(), ce.Type(), result)
}
case cloudevents.IsACK(result):
{
Expand All @@ -153,7 +158,7 @@ func (p *Publisher) SendCloudEvent(event *cloudevents.Event, encoding binding.En
}
default:
{
return fmt.Errorf("failed to send cloudevent-%s unknown:[%s] response:[%s]", encoding.String(), ce.Type(), result)
return fmt.Errorf("%w: encoding:[%s] unknown:[%s] response:[%s]", ErrFailedSendingCE, encoding.String(), ce.Type(), result)
}
}
}
2 changes: 1 addition & 1 deletion hack/e2e/common/eventing/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func NewCloudEvent(eventSource, eventType string, encoding binding.Encoding) (*c
ce.SetType(eventType)
ce.SetSource(eventSource)
if err := ce.SetData(cloudevents.ApplicationJSON, data); err != nil {
return nil, fmt.Errorf("failed to set cloudevent-%s data with error:[%s]", encoding.String(), err)
return nil, fmt.Errorf("failed to set cloudevent-%s data with error:[%w]", encoding.String(), err)
}
return &ce, nil
}
25 changes: 17 additions & 8 deletions hack/e2e/common/testenvironment/test_environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,15 @@ const (
ThreeAttempts = 3
)

var (
ErrSubscriptionNotReconciled = errors.New("subscription not reconciled")
ErrSubscriptionNotReady = errors.New("subscription not READY")
ErrDeploymentNotReady = errors.New("deployment not READY")
ErrWrongActiveType = errors.New("specified backend not active")
ErrEventingNotReady = errors.New("eventing not READY")
ErrInvalidDeployment = errors.New("deployment.spec invalid")
)

// TestEnvironment provides mocked resources for integration tests.
type TestEnvironment struct {
Logger *zap.Logger
Expand Down Expand Up @@ -216,15 +225,15 @@ func (te *TestEnvironment) WaitForSubscription(ctx context.Context, subsToTest e
"in namespace: %s to get recocniled by backend: %s", subsToTest.Name, te.TestConfigs.TestNamespace,
te.TestConfigs.BackendType)
te.Logger.Debug(errMsg)
return errors.New(errMsg)
return fmt.Errorf("%s, %w", errMsg, ErrSubscriptionNotReconciled)
}

// check if subscription is ready.
if !gotSub.Status.Ready {
errMsg := fmt.Sprintf("waiting subscription: %s "+
"in namespace: %s to get ready", subsToTest.Name, te.TestConfigs.TestNamespace)
te.Logger.Debug(errMsg)
return errors.New(errMsg)
return fmt.Errorf("%s, %w", errMsg, ErrSubscriptionNotReady)
}
return nil
})
Expand Down Expand Up @@ -321,7 +330,7 @@ func (te *TestEnvironment) WaitForDeploymentReady(name, namespace, image string)

// if image is provided, then check if the deployment has correct image.
if image != "" && gotDeployment.Spec.Template.Spec.Containers[0].Image != image {
err = fmt.Errorf("expected deployment (%s) image to be: %s, but found: %s", name, image,
err = fmt.Errorf("%w: expected deployment (%s) image to be: %s, but found: %s", ErrInvalidDeployment, name, image,
gotDeployment.Spec.Template.Spec.Containers[0].Image,
)
te.Logger.Debug(err.Error())
Expand All @@ -332,7 +341,7 @@ func (te *TestEnvironment) WaitForDeploymentReady(name, namespace, image string)
if *gotDeployment.Spec.Replicas != gotDeployment.Status.UpdatedReplicas ||
*gotDeployment.Spec.Replicas != gotDeployment.Status.ReadyReplicas ||
*gotDeployment.Spec.Replicas != gotDeployment.Status.AvailableReplicas {
err = fmt.Errorf("waiting for deployment: %s to get ready", name)
err = fmt.Errorf("waiting for deployment: %s to get ready: %w", name, ErrDeploymentNotReady)
te.Logger.Debug(err.Error())
return err
}
Expand Down Expand Up @@ -549,13 +558,13 @@ func (te *TestEnvironment) WaitForEventingCRReady() error {
}

if gotEventingCR.Spec.Backend.Type != gotEventingCR.Status.ActiveBackend {
err := fmt.Errorf("waiting for Eventing CR to switch backend")
te.Logger.Debug(err.Error())
return err
msg := "waiting for Eventing CR to switch backend"
te.Logger.Debug(msg)
return fmt.Errorf("%s: %w", msg, ErrWrongActiveType)
}

if gotEventingCR.Status.State != operatorv1alpha1.StateReady {
err := fmt.Errorf("waiting for Eventing CR to get ready state")
err := fmt.Errorf("waiting for Eventing CR to get ready state: %w", ErrEventingNotReady)
te.Logger.Debug(err.Error())
return err
}
Expand Down
18 changes: 10 additions & 8 deletions internal/controller/errors/skip_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,21 @@ import (
"fmt"
"testing"

"github.com/stretchr/testify/require"

controllererrors "github.com/kyma-project/eventing-manager/internal/controller/errors"
)

var ErrGeneric = errors.New("some error")

func Test_NewSkippable(t *testing.T) {
testCases := []struct {
error error
}{
{error: controllererrors.NewSkippable(nil)},
{error: controllererrors.NewSkippable(controllererrors.NewSkippable(nil))},
{error: controllererrors.NewSkippable(fmt.Errorf("some error"))},
{error: controllererrors.NewSkippable(controllererrors.NewSkippable(fmt.Errorf("some error")))},
{error: controllererrors.NewSkippable(ErrGeneric)},
{error: controllererrors.NewSkippable(controllererrors.NewSkippable(ErrGeneric))},
}

for _, tc := range testCases {
Expand All @@ -24,9 +28,7 @@ func Test_NewSkippable(t *testing.T) {
t.Errorf("test NewSkippable retuned nil error")
continue
}
if err := errors.Unwrap(skippableErr); tc.error != err {
t.Errorf("test NewSkippable failed, want: %#v but got: %#v", tc.error, err)
}
require.ErrorIs(t, skippableErr, tc.error)
}
}

Expand All @@ -43,17 +45,17 @@ func Test_IsSkippable(t *testing.T) {
},
{
name: "skippable error, should be skipped",
givenError: controllererrors.NewSkippable(fmt.Errorf("some errore")),
givenError: controllererrors.NewSkippable(ErrGeneric),
wantSkippable: true,
},
{
name: "not-skippable error, should not be skipped",
givenError: fmt.Errorf("some error"),
givenError: ErrGeneric,
wantSkippable: false,
},
{
name: "not-skippable error which wraps a skippable error, should not be skipped",
givenError: fmt.Errorf("some error %w", controllererrors.NewSkippable(fmt.Errorf("some error"))),
givenError: fmt.Errorf("some error %w", controllererrors.NewSkippable(ErrGeneric)),
wantSkippable: false,
},
}
Expand Down
4 changes: 3 additions & 1 deletion internal/controller/eventing/subscription/eventmesh/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2"
)

var ErrInvalidSink = errors.New("invalid sink")

// isInDeletion checks if the Subscription shall be deleted.
func isInDeletion(subscription *eventingv1alpha2.Subscription) bool {
return !subscription.DeletionTimestamp.IsZero()
Expand Down Expand Up @@ -55,7 +57,7 @@ func removeFinalizer(sub *eventingv1alpha2.Subscription) {
func getSvcNsAndName(url string) (string, string, error) {
parts := strings.Split(url, ".")
if len(parts) < 2 {
return "", "", fmt.Errorf("invalid sinkURL for cluster local svc: %s", url)
return "", "", fmt.Errorf("%w url: %s", ErrInvalidSink, url)
}
return parts[1], parts[0], nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func BeJetStreamSubscriptionWithSubject(source, subject string,
}
result := info.Config.FilterSubject == js.GetJetStreamSubject(source, subject, typeMatching)
if !result {
//nolint: goerr113 // no production code, but test helper functionality
return false, fmt.Errorf(
"BeJetStreamSubscriptionWithSubject expected %v to be equal to %v",
info.Config.FilterSubject,
Expand Down
20 changes: 13 additions & 7 deletions internal/controller/operator/eventing/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@ const (
SubscriptionExistsErrMessage = "cannot delete the eventing module as subscription exists"
)

var (
ErrSubscriptionExists = errors.New(SubscriptionExistsErrMessage)
ErrUnsupportedBackedType = errors.New("backend type not supported")
ErrNatsModuleMissing = errors.New("NATS module has to be installed")
ErrNATSServerUnavailable = errors.New(NatsServerNotAvailableMsg)
ErrAPIGatewayModuleMissing = errors.New("API-Gateway module is needed for EventMesh backend. APIRules CRD is not installed")
)

// Reconciler reconciles an Eventing object
//
//go:generate go run github.com/vektra/mockery/v2 --name=Controller --dir=../../../../vendor/sigs.k8s.io/controller-runtime/pkg/controller --outpkg=mocks --case=underscore
Expand Down Expand Up @@ -387,8 +395,7 @@ func (r *Reconciler) handleEventingDeletion(ctx context.Context, eventing *opera
}
if exists {
eventing.Status.SetStateWarning()
return kctrl.Result{Requeue: true}, r.syncStatusWithDeletionErr(ctx, eventing,
errors.New(SubscriptionExistsErrMessage), log)
return kctrl.Result{Requeue: true}, r.syncStatusWithDeletionErr(ctx, eventing, ErrSubscriptionExists, log)
}

log.Info("handling Eventing deletion...")
Expand Down Expand Up @@ -474,7 +481,7 @@ func (r *Reconciler) handleEventingReconcile(ctx context.Context,
case operatorv1alpha1.EventMeshBackendType:
return r.reconcileEventMeshBackend(ctx, eventing, log)
default:
return kctrl.Result{Requeue: false}, fmt.Errorf("not supported backend type %s", eventing.Spec.Backend.Type)
return kctrl.Result{Requeue: false}, fmt.Errorf("%w: %s", ErrUnsupportedBackedType, eventing.Spec.Backend.Type)
}
}

Expand Down Expand Up @@ -515,7 +522,7 @@ func (r *Reconciler) reconcileNATSBackend(ctx context.Context, eventing *operato
log.Infof("NATS module not enabled, deleting publisher proxy resources")
delErr := r.eventingManager.DeletePublisherProxyResources(ctx, eventing)
// update the Eventing CR status.
notFoundErr := fmt.Errorf("NATS module has to be installed: %v", err)
notFoundErr := fmt.Errorf("%w: %v", ErrNatsModuleMissing, err)
return kctrl.Result{}, errors.Join(r.syncStatusWithNATSErr(ctx, eventing, notFoundErr, log), delErr)
}
return kctrl.Result{}, err
Expand Down Expand Up @@ -553,7 +560,7 @@ func (r *Reconciler) checkNATSAvailability(ctx context.Context, eventing *operat
return err
}
if !natsAvailable {
return fmt.Errorf(NatsServerNotAvailableMsg)
return ErrNATSServerUnavailable
}
return nil
}
Expand Down Expand Up @@ -592,8 +599,7 @@ func (r *Reconciler) reconcileEventMeshBackend(ctx context.Context, eventing *op
if err != nil {
return kctrl.Result{}, r.syncStatusWithSubscriptionManagerErr(ctx, eventing, err, log)
} else if !isAPIRuleCRDEnabled {
apiRuleMissingErr := errors.New("API-Gateway module is needed for EventMesh backend. APIRules CRD is not installed")
return kctrl.Result{}, r.syncStatusWithSubscriptionManagerErr(ctx, eventing, apiRuleMissingErr, log)
return kctrl.Result{}, r.syncStatusWithSubscriptionManagerErr(ctx, eventing, ErrAPIGatewayModuleMissing, log)
}

// Start the EventMesh subscription controller
Expand Down
11 changes: 5 additions & 6 deletions internal/controller/operator/eventing/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package eventing

import (
"context"
"errors"
"fmt"
"testing"

Expand Down Expand Up @@ -177,13 +176,13 @@ func Test_handleBackendSwitching(t *testing.T) {
),
givenNATSSubManagerMock: func() *submgrmanagermocks.Manager {
managerMock := new(submgrmanagermocks.Manager)
managerMock.On("Stop", true).Return(errors.New("failed to stop")).Once()
managerMock.On("Stop", true).Return(ErrFailedToStop).Once()
return managerMock
},
givenEventMeshSubManagerMock: func() *submgrmanagermocks.Manager {
return new(submgrmanagermocks.Manager)
},
wantError: errors.New("failed to stop"),
wantError: ErrFailedToStop,
wantEventingState: operatorv1alpha1.StateReady,
wantEventingConditionsLen: 1,
wantNATSStopped: false,
Expand Down Expand Up @@ -224,10 +223,10 @@ func Test_handleBackendSwitching(t *testing.T) {
},
givenEventMeshSubManagerMock: func() *submgrmanagermocks.Manager {
managerMock := new(submgrmanagermocks.Manager)
managerMock.On("Stop", true).Return(errors.New("failed to stop")).Once()
managerMock.On("Stop", true).Return(ErrFailedToStop).Once()
return managerMock
},
wantError: errors.New("failed to stop"),
wantError: ErrFailedToStop,
wantEventingState: operatorv1alpha1.StateReady,
wantEventingConditionsLen: 1,
wantNATSStopped: false,
Expand Down Expand Up @@ -321,7 +320,7 @@ func Test_startNatsCRWatch(t *testing.T) {
{
name: "NATS watcher error",
watchStarted: false,
watchErr: errors.New("NATS watcher error"),
watchErr: ErrUseMeInMocks,
},
}

Expand Down
12 changes: 7 additions & 5 deletions internal/controller/operator/eventing/domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,21 @@ package eventing

import (
"context"
"errors"
"fmt"
)

const (
shootInfoConfigMapName = "shoot-info"
shootInfoConfigMapNamespace = "kube-system"
shootInfoConfigMapKeyDomain = "domain"
domainMissingMessageFormat = `domain configuration is missing. domain must be configured in either the Eventing` +
domainMissingMessageFormat = `%w. domain must be configured in either the Eventing` +
` CustomResource under "Spec.Backend.Config.Domain" or in the ConfigMap "%s/%s" under "data.%s"`
domainMissingMessageFormatWithError = domainMissingMessageFormat + `: %v`
domainMissingMessageFormatWithError = domainMissingMessageFormat + `: %w`
)

var ErrDomainConfigMissing = errors.New("domain configuration missing")

func (r *Reconciler) readDomainFromConfigMap(ctx context.Context) (string, error) {
cm, err := r.kubeClient.GetConfigMap(ctx, shootInfoConfigMapName, shootInfoConfigMapNamespace)
if err != nil {
Expand All @@ -25,13 +28,12 @@ func (r *Reconciler) readDomainFromConfigMap(ctx context.Context) (string, error
func domainMissingError(err error) error {
if err != nil {
return fmt.Errorf(
domainMissingMessageFormatWithError,
domainMissingMessageFormatWithError, ErrDomainConfigMissing,
shootInfoConfigMapNamespace, shootInfoConfigMapName, shootInfoConfigMapKeyDomain, err,
)
}

return fmt.Errorf(
domainMissingMessageFormat,
domainMissingMessageFormat, ErrDomainConfigMissing,
shootInfoConfigMapNamespace, shootInfoConfigMapName, shootInfoConfigMapKeyDomain,
)
}
11 changes: 6 additions & 5 deletions internal/controller/operator/eventing/domain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@ package eventing

import (
"context"
"fmt"
"strings"
"testing"

"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
kcorev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -46,7 +45,7 @@ func Test_readDomainFromConfigMap(t *testing.T) {
func Test_domainMissingError(t *testing.T) {
// given
const errorMessage = "some error"
err := fmt.Errorf(errorMessage)
err := errors.New(errorMessage)

// when
err0 := domainMissingError(nil)
Expand All @@ -55,6 +54,8 @@ func Test_domainMissingError(t *testing.T) {
// then
require.Error(t, err0)
require.Error(t, err1)
require.False(t, strings.Contains(strings.ToLower(err0.Error()), "nil"))
require.True(t, strings.Contains(err1.Error(), errorMessage))
require.ErrorIs(t, err0, ErrDomainConfigMissing)
require.ErrorIs(t, err1, ErrDomainConfigMissing)
require.NotErrorIs(t, err0, err)
require.ErrorIs(t, err1, err)
}
Loading

0 comments on commit df6ebd1

Please sign in to comment.