From df6ebd161bd3a28a3f8db56d23cc56435646fd4f Mon Sep 17 00:00:00 2001 From: Korbinian Stoemmer Date: Thu, 14 Dec 2023 13:37:54 +0100 Subject: [PATCH] enable goerr113 --- .golangci.yaml | 1 - hack/e2e/common/eventing/publisher.go | 13 +++-- hack/e2e/common/eventing/utils.go | 2 +- .../testenvironment/test_environment.go | 25 +++++++--- internal/controller/errors/skip_unit_test.go | 18 ++++--- .../eventing/subscription/eventmesh/utils.go | 4 +- .../subscription/jetstream/matchers_test.go | 1 + .../operator/eventing/controller.go | 20 +++++--- .../operator/eventing/controller_test.go | 11 ++--- .../controller/operator/eventing/domain.go | 12 +++-- .../operator/eventing/domain_test.go | 11 +++-- .../controller/operator/eventing/eventmesh.go | 31 +++++++----- .../operator/eventing/eventmesh_test.go | 16 ++++-- internal/controller/operator/eventing/nats.go | 5 +- .../controller/operator/eventing/nats_test.go | 21 ++++---- pkg/backend/eventmesh/eventmesh.go | 10 ++-- pkg/backend/eventtype/parse.go | 9 +++- pkg/backend/jetstream/jetstream.go | 2 +- pkg/backend/metrics/collector.go | 10 ++-- pkg/ems/httpclient/error_unit_test.go | 1 + pkg/env/config.go | 5 +- pkg/errors/errors.go | 2 +- pkg/errors/errors_unit_test.go | 1 + pkg/eventing/manager.go | 20 +++++--- pkg/eventing/manager_test.go | 24 ++++----- pkg/k8s/client.go | 4 +- pkg/k8s/client_test.go | 5 +- pkg/signals/signals.go | 4 +- .../eventmesh/eventmesh.go | 49 +++++-------------- pkg/subscriptionmanager/manager/manager.go | 2 +- test/utils/utils.go | 9 ++-- testing/event/cehelper/converter.go | 2 +- testing/subscriber.go | 26 ++++++---- 33 files changed, 212 insertions(+), 164 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 0e438f2c..cc923862 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -30,7 +30,6 @@ linters: - goconst - gocritic - godox - - goerr113 - gomnd - gosec - inamedparam diff --git a/hack/e2e/common/eventing/publisher.go b/hack/e2e/common/eventing/publisher.go index b6ff589a..f279076e 100644 --- a/hack/e2e/common/eventing/publisher.go +++ b/hack/e2e/common/eventing/publisher.go @@ -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 @@ -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()) } } @@ -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): { @@ -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) } } } diff --git a/hack/e2e/common/eventing/utils.go b/hack/e2e/common/eventing/utils.go index d718698f..6486eb7e 100644 --- a/hack/e2e/common/eventing/utils.go +++ b/hack/e2e/common/eventing/utils.go @@ -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 } diff --git a/hack/e2e/common/testenvironment/test_environment.go b/hack/e2e/common/testenvironment/test_environment.go index 731d9360..6edf86c6 100644 --- a/hack/e2e/common/testenvironment/test_environment.go +++ b/hack/e2e/common/testenvironment/test_environment.go @@ -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 @@ -216,7 +225,7 @@ 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. @@ -224,7 +233,7 @@ func (te *TestEnvironment) WaitForSubscription(ctx context.Context, subsToTest e 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 }) @@ -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()) @@ -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 } @@ -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 } diff --git a/internal/controller/errors/skip_unit_test.go b/internal/controller/errors/skip_unit_test.go index 55b52805..c237f0c5 100644 --- a/internal/controller/errors/skip_unit_test.go +++ b/internal/controller/errors/skip_unit_test.go @@ -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 { @@ -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) } } @@ -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, }, } diff --git a/internal/controller/eventing/subscription/eventmesh/utils.go b/internal/controller/eventing/subscription/eventmesh/utils.go index 286ac1b1..a8265050 100644 --- a/internal/controller/eventing/subscription/eventmesh/utils.go +++ b/internal/controller/eventing/subscription/eventmesh/utils.go @@ -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() @@ -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 } diff --git a/internal/controller/eventing/subscription/jetstream/matchers_test.go b/internal/controller/eventing/subscription/jetstream/matchers_test.go index 0017b63b..c389d56f 100644 --- a/internal/controller/eventing/subscription/jetstream/matchers_test.go +++ b/internal/controller/eventing/subscription/jetstream/matchers_test.go @@ -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, diff --git a/internal/controller/operator/eventing/controller.go b/internal/controller/operator/eventing/controller.go index e9efdc71..3ce2971c 100644 --- a/internal/controller/operator/eventing/controller.go +++ b/internal/controller/operator/eventing/controller.go @@ -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 @@ -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...") @@ -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) } } @@ -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 @@ -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 } @@ -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 diff --git a/internal/controller/operator/eventing/controller_test.go b/internal/controller/operator/eventing/controller_test.go index 5ee10873..f85dba1c 100644 --- a/internal/controller/operator/eventing/controller_test.go +++ b/internal/controller/operator/eventing/controller_test.go @@ -2,7 +2,6 @@ package eventing import ( "context" - "errors" "fmt" "testing" @@ -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, @@ -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, @@ -321,7 +320,7 @@ func Test_startNatsCRWatch(t *testing.T) { { name: "NATS watcher error", watchStarted: false, - watchErr: errors.New("NATS watcher error"), + watchErr: ErrUseMeInMocks, }, } diff --git a/internal/controller/operator/eventing/domain.go b/internal/controller/operator/eventing/domain.go index 581e2e77..bbe4cb61 100644 --- a/internal/controller/operator/eventing/domain.go +++ b/internal/controller/operator/eventing/domain.go @@ -2,6 +2,7 @@ package eventing import ( "context" + "errors" "fmt" ) @@ -9,11 +10,13 @@ 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 { @@ -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, ) } diff --git a/internal/controller/operator/eventing/domain_test.go b/internal/controller/operator/eventing/domain_test.go index 25cd6c42..c6255ef3 100644 --- a/internal/controller/operator/eventing/domain_test.go +++ b/internal/controller/operator/eventing/domain_test.go @@ -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" @@ -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) @@ -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) } diff --git a/internal/controller/operator/eventing/eventmesh.go b/internal/controller/operator/eventing/eventmesh.go index cfc37cbc..2189ed1f 100644 --- a/internal/controller/operator/eventing/eventmesh.go +++ b/internal/controller/operator/eventing/eventmesh.go @@ -29,6 +29,11 @@ const ( secretKeyCertsURL = "certs_url" ) +var ( + ErrEMSecretMessagingMissing = errors.New("messaging is missing from EM secret") + ErrEMSecretNamespaceMissing = errors.New("namespace is missing from EM secret") +) + type oauth2Credentials struct { clientID []byte clientSecret []byte @@ -40,23 +45,23 @@ func (r *Reconciler) reconcileEventMeshSubManager(ctx context.Context, eventing // gets oauth2ClientID and secret and stops the EventMesh subscription manager if changed err := r.syncOauth2ClientIDAndSecret(ctx, eventing) if err != nil { - return errors.Errorf("failed to sync OAuth secret: %v", err) + return fmt.Errorf("failed to sync OAuth secret: %w", err) } // retrieve secret to authenticate with EventMesh eventMeshSecret, err := r.kubeClient.GetSecret(ctx, eventing.Spec.Backend.Config.EventMeshSecret) if err != nil { - return errors.Errorf("failed to get EventMesh secret: %v", err) + return fmt.Errorf("failed to get EventMesh secret: %w", err) } // CreateOrUpdate deployment for publisher proxy secret secretForPublisher, err := r.SyncPublisherProxySecret(ctx, eventMeshSecret) if err != nil { - return errors.Errorf("failed to sync Publisher Proxy secret: %v", err) + return fmt.Errorf("failed to sync Publisher Proxy secret: %w", err) } // Set environment with secrets for EventMesh subscription controller err = setUpEnvironmentForEventMesh(secretForPublisher, eventing) if err != nil { - return fmt.Errorf("failed to setup environment variables for EventMesh controller: %v", err) + return fmt.Errorf("failed to setup environment variables for EventMesh controller: %w", err) } // Read the cluster domain from the Eventing CR, or @@ -175,7 +180,7 @@ func (r *Reconciler) stopEventMeshSubManager(runCleanup bool, log *zap.SugaredLo func (r *Reconciler) SyncPublisherProxySecret(ctx context.Context, secret *kcorev1.Secret) (*kcorev1.Secret, error) { desiredSecret, err := getSecretForPublisher(secret) if err != nil { - return nil, fmt.Errorf("invalid secret for Event Publisher: %v", err) + return nil, fmt.Errorf("invalid secret for Event Publisher: %w", err) } err = r.kubeClient.PatchApply(ctx, desiredSecret) @@ -302,12 +307,12 @@ func getSecretForPublisher(eventMeshSecret *kcorev1.Secret) (*kcorev1.Secret, er } if _, ok := eventMeshSecret.Data["messaging"]; !ok { - return nil, errors.New("message is missing from BEB secret") + return nil, ErrEMSecretMessagingMissing } messagingBytes := eventMeshSecret.Data["messaging"] if _, ok := eventMeshSecret.Data["namespace"]; !ok { - return nil, errors.New("namespace is missing from BEB secret") + return nil, ErrEMSecretNamespaceMissing } namespaceBytes := eventMeshSecret.Data["namespace"] @@ -357,31 +362,31 @@ func getSecretStringData(clientID, clientSecret, tokenEndpoint, grantType, publi func setUpEnvironmentForEventMesh(secret *kcorev1.Secret, eventingCR *v1alpha1.Eventing) error { err := os.Setenv("BEB_API_URL", fmt.Sprintf("%s%s", string(secret.Data[PublisherSecretEMSHostKey]), EventMeshPublishEndpointForSubscriber)) if err != nil { - return fmt.Errorf("set BEB_API_URL env var failed: %v", err) + return fmt.Errorf("set BEB_API_URL env var failed: %w", err) } err = os.Setenv("CLIENT_ID", string(secret.Data[eventing.PublisherSecretClientIDKey])) if err != nil { - return fmt.Errorf("set CLIENT_ID env var failed: %v", err) + return fmt.Errorf("set CLIENT_ID env var failed: %w", err) } err = os.Setenv("CLIENT_SECRET", string(secret.Data[eventing.PublisherSecretClientSecretKey])) if err != nil { - return fmt.Errorf("set CLIENT_SECRET env var failed: %v", err) + return fmt.Errorf("set CLIENT_SECRET env var failed: %w", err) } err = os.Setenv("TOKEN_ENDPOINT", string(secret.Data[eventing.PublisherSecretTokenEndpointKey])) if err != nil { - return fmt.Errorf("set TOKEN_ENDPOINT env var failed: %v", err) + return fmt.Errorf("set TOKEN_ENDPOINT env var failed: %w", err) } err = os.Setenv("BEB_NAMESPACE", fmt.Sprintf("%s%s", NamespacePrefix, string(secret.Data[eventing.PublisherSecretBEBNamespaceKey]))) if err != nil { - return fmt.Errorf("set BEB_NAMESPACE env var failed: %v", err) + return fmt.Errorf("set BEB_NAMESPACE env var failed: %w", err) } if err := os.Setenv("EVENT_TYPE_PREFIX", eventingCR.Spec.Backend.Config.EventTypePrefix); err != nil { - return fmt.Errorf("set EVENT_TYPE_PREFIX env var failed: %v", err) + return fmt.Errorf("set EVENT_TYPE_PREFIX env var failed: %w", err) } return nil diff --git a/internal/controller/operator/eventing/eventmesh_test.go b/internal/controller/operator/eventing/eventmesh_test.go index db2a951f..836115d5 100644 --- a/internal/controller/operator/eventing/eventmesh_test.go +++ b/internal/controller/operator/eventing/eventmesh_test.go @@ -30,6 +30,12 @@ const ( defaultEventingWebhookAuthSecretNamespace = "kyma-system" ) +var ( + ErrFailedToStart = errors.New("failed to start") + ErrFailedToStop = errors.New("failed to stop") +) + +//nolint:goerr113 // all tests here need to be fixed, as they use require.ErrorAs and use it wrongly func Test_reconcileEventMeshSubManager(t *testing.T) { t.Parallel() @@ -482,11 +488,11 @@ func Test_stopEventMeshSubManager(t *testing.T) { name: "should return error when subscription manager fails to stop", givenEventMeshSubManagerMock: func() *submgrmanagermocks.Manager { managerMock := new(submgrmanagermocks.Manager) - managerMock.On("Stop", mock.Anything).Return(errors.New("failed to stop")).Once() + managerMock.On("Stop", mock.Anything).Return(ErrFailedToStop).Once() return managerMock }, givenIsEventMeshSubManagerStarted: true, - wantError: errors.New("failed to stop"), + wantError: ErrFailedToStop, wantAssertCheck: true, }, { @@ -642,7 +648,7 @@ func Test_GetSecretForPublisher(t *testing.T) { { name: "with empty message data", namespaceData: []byte("valid/namespace"), - expectedError: errors.New("message is missing from BEB secret"), + expectedError: ErrEMSecretMessagingMissing, }, { name: "with empty namespace data", @@ -693,7 +699,7 @@ func Test_GetSecretForPublisher(t *testing.T) { "uri": "https://rest-messaging" } ]`), - expectedError: errors.New("namespace is missing from BEB secret"), + expectedError: ErrEMSecretNamespaceMissing, }, } @@ -921,7 +927,7 @@ func Test_SyncPublisherProxySecret(t *testing.T) { givenSecret: utils.NewEventMeshSecret("valid", "test-namespace"), mockKubeClient: func() *k8smocks.Client { kubeClient := new(k8smocks.Client) - kubeClient.On("PatchApply", mock.Anything, mock.Anything).Return(errors.New("fake error")).Once() + kubeClient.On("PatchApply", mock.Anything, mock.Anything).Return(ErrUseMeInMocks).Once() return kubeClient }, wantErr: true, diff --git a/internal/controller/operator/eventing/nats.go b/internal/controller/operator/eventing/nats.go index 877c975b..fca7b348 100644 --- a/internal/controller/operator/eventing/nats.go +++ b/internal/controller/operator/eventing/nats.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/pkg/errors" "go.uber.org/zap" "github.com/kyma-project/eventing-manager/api/operator/v1alpha1" @@ -13,6 +14,8 @@ import ( "github.com/kyma-project/eventing-manager/pkg/subscriptionmanager/manager" ) +var ErrCannotBuildNATSURL = errors.New("NATS CR is not found to build NATS server URL") + func (r *Reconciler) reconcileNATSSubManager(ctx context.Context, eventing *v1alpha1.Eventing, log *zap.SugaredLogger) error { // get the subscription config defaultSubsConfig := r.getDefaultSubscriptionConfig() @@ -162,5 +165,5 @@ func (n *NatsConfigHandlerImpl) getNATSUrl(ctx context.Context, namespace string for _, nats := range natsList.Items { return fmt.Sprintf("nats://%s.%s.svc.cluster.local:%d", nats.Name, nats.Namespace, natsClientPort), nil } - return "", fmt.Errorf("NATS CR is not found to build NATS server URL") + return "", ErrCannotBuildNATSURL } diff --git a/internal/controller/operator/eventing/nats_test.go b/internal/controller/operator/eventing/nats_test.go index 976c6285..7dfc87c2 100644 --- a/internal/controller/operator/eventing/nats_test.go +++ b/internal/controller/operator/eventing/nats_test.go @@ -3,7 +3,6 @@ package eventing import ( "context" "errors" - "fmt" "testing" "time" @@ -23,6 +22,8 @@ import ( "github.com/kyma-project/eventing-manager/test/utils" ) +var ErrUseMeInMocks = errors.New("use me in mocks") + func Test_reconcileNATSSubManager(t *testing.T) { t.Parallel() @@ -135,7 +136,7 @@ func Test_reconcileNATSSubManager(t *testing.T) { givenNATSSubManagerMock: func() *submgrmanagermocks.Manager { jetStreamSubManagerMock := new(submgrmanagermocks.Manager) jetStreamSubManagerMock.On("Init", mock.Anything).Return(nil).Once() - jetStreamSubManagerMock.On("Start", mock.Anything, mock.Anything).Return(errors.New("failed to start")).Twice() + jetStreamSubManagerMock.On("Start", mock.Anything, mock.Anything).Return(ErrUseMeInMocks).Twice() return jetStreamSubManagerMock }, givenEventingManagerMock: func() *eventingmocks.Manager { @@ -155,7 +156,7 @@ func Test_reconcileNATSSubManager(t *testing.T) { }, wantAssertCheck: true, givenShouldRetry: true, - wantError: errors.New("failed to start"), + wantError: ErrUseMeInMocks, wantHashAfter: int64(-7550677537009891034), }, { @@ -308,11 +309,11 @@ func Test_stopNATSSubManager(t *testing.T) { name: "should return error when subscription manager fails to stop", givenNATSSubManagerMock: func() *submgrmanagermocks.Manager { managerMock := new(submgrmanagermocks.Manager) - managerMock.On("Stop", mock.Anything).Return(errors.New("failed to stop")).Once() + managerMock.On("Stop", mock.Anything).Return(ErrUseMeInMocks).Once() return managerMock }, givenIsNATSSubManagerStarted: true, - wantError: errors.New("failed to stop"), + wantError: ErrUseMeInMocks, wantAssertCheck: true, }, { @@ -419,7 +420,7 @@ func Test_GetNatsConfig(t *testing.T) { ), givenNatsResources: nil, expectedConfig: nil, - expectedError: fmt.Errorf("failed to get NATS URL"), + expectedError: ErrUseMeInMocks, }, } @@ -478,15 +479,15 @@ func Test_getNATSUrl(t *testing.T) { givenNamespace: "test-namespace", want: "", getNATSResourcesErr: nil, - wantErr: fmt.Errorf("NATS CR is not found to build NATS server URL"), + wantErr: ErrCannotBuildNATSURL, }, { name: "NATS resource does not exist", givenNatsResources: nil, givenNamespace: "test-namespace", want: "", - getNATSResourcesErr: fmt.Errorf("NATS CR is not found to build NATS server URL"), - wantErr: fmt.Errorf("NATS CR is not found to build NATS server URL"), + getNATSResourcesErr: ErrCannotBuildNATSURL, + wantErr: ErrCannotBuildNATSURL, }, } @@ -551,7 +552,7 @@ func Test_UpdateNatsConfig(t *testing.T) { utils.WithEventingStreamData("Memory", "700Mi", 2, 1000), ), givenNatsResources: nil, - expectedError: fmt.Errorf("failed to get NATS URL"), + expectedError: ErrCannotBuildNATSURL, }, } diff --git a/pkg/backend/eventmesh/eventmesh.go b/pkg/backend/eventmesh/eventmesh.go index 99f3fc88..361d7be6 100644 --- a/pkg/backend/eventmesh/eventmesh.go +++ b/pkg/backend/eventmesh/eventmesh.go @@ -31,6 +31,8 @@ const ( // Perform a compile time check. var _ Backend = &EventMesh{} +var ErrEMSubjectInvalid = errors.New("EventMesh subject invalid") + type Backend interface { // Initialize should initialize the communication layer with the messaging backend system Initialize(cfg env.Config) error @@ -216,7 +218,7 @@ func (em *EventMesh) getProcessedEventTypes(kymaSubscription *eventingv1alpha2.S if isEventTypeSegmentsOverLimit(eventMeshSubject) { return nil, fmt.Errorf("EventMesh subject exceeds the limit of segments, "+ - "max number of segements allowed: %d", eventTypeSegmentsLimit) + "max number of segements allowed: %d, %w", eventTypeSegmentsLimit, ErrEMSubjectInvalid) } result = append(result, backendutils.EventTypeInfo{ @@ -396,7 +398,7 @@ func (em *EventMesh) getSubscriptionIgnoreNotFound(name string) (*types.Subscrip func (em *EventMesh) getSubscription(name string) (*types.Subscription, error) { eventMeshSubscription, resp, err := em.client.Get(name) if err != nil { - return nil, fmt.Errorf("get subscription failed: %v", err) + return nil, fmt.Errorf("get subscription failed: %w", err) } if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("get subscription failed: %w; %v", @@ -410,7 +412,7 @@ func (em *EventMesh) deleteSubscription(name string) error { em.namedLogger().Debugf("Deleting EventMesh subscription: %s", name) resp, err := em.client.Delete(name) if err != nil { - return fmt.Errorf("delete subscription failed: %v", err) + return fmt.Errorf("delete subscription failed: %w", err) } if resp.StatusCode != http.StatusNoContent && resp.StatusCode != http.StatusNotFound { return fmt.Errorf("delete subscription failed: %w; %v", @@ -423,7 +425,7 @@ func (em *EventMesh) deleteSubscription(name string) error { func (em *EventMesh) createSubscription(subscription *types.Subscription) error { createResponse, err := em.client.Create(subscription) if err != nil { - return fmt.Errorf("create subscription failed: %v", err) + return fmt.Errorf("create subscription failed: %w", err) } if createResponse.StatusCode > http.StatusAccepted && createResponse.StatusCode != http.StatusConflict { return fmt.Errorf("create subscription failed: %w; %v", diff --git a/pkg/backend/eventtype/parse.go b/pkg/backend/eventtype/parse.go index 34fad73d..847cbf50 100644 --- a/pkg/backend/eventtype/parse.go +++ b/pkg/backend/eventtype/parse.go @@ -6,6 +6,11 @@ import ( "strings" ) +var ( + ErrPrefixNotFound = errors.New("prefix not found") + ErrInvalidFormat = errors.New("invalid format") +) + // parse splits the event-type using the given prefix and returns the application name, event and version // or an error if the event-type format is invalid. // A valid even-type format should be: prefix.application.event.version or application.event.version @@ -13,7 +18,7 @@ import ( // Constraint: the application segment in the input event-type should not contain ".". func parse(eventType, prefix string) (string, string, string, error) { if !strings.HasPrefix(eventType, prefix) { - return "", "", "", errors.New("prefix not found") + return "", "", "", ErrPrefixNotFound } // remove the prefix @@ -24,7 +29,7 @@ func parse(eventType, prefix string) (string, string, string, error) { // (e.g. application.businessObject.operation.version) parts := strings.Split(eventType, ".") if len(parts) < 4 { - return "", "", "", errors.New("invalid format") + return "", "", "", ErrInvalidFormat } // parse the event-type segments diff --git a/pkg/backend/jetstream/jetstream.go b/pkg/backend/jetstream/jetstream.go index f470d0e9..36f9b3c5 100644 --- a/pkg/backend/jetstream/jetstream.go +++ b/pkg/backend/jetstream/jetstream.go @@ -232,7 +232,7 @@ func (js *JetStream) validateConfig() error { return pkgerrors.New("Stream name cannot be empty") } if len(js.Config.JSStreamName) > jsMaxStreamNameLength { - return fmt.Errorf("stream name should be max %d characters long", jsMaxStreamNameLength) + return ErrStreamNameTooLong } if _, err := toJetStreamStorageType(js.Config.JSStreamStorageType); err != nil { return err diff --git a/pkg/backend/metrics/collector.go b/pkg/backend/metrics/collector.go index f83cf2a7..6417c665 100644 --- a/pkg/backend/metrics/collector.go +++ b/pkg/backend/metrics/collector.go @@ -1,7 +1,7 @@ package metrics import ( - "fmt" + "strconv" "time" "github.com/prometheus/client_golang/prometheus" @@ -132,8 +132,8 @@ func (c *Collector) RecordDeliveryPerSubscription(subscriptionName, subscription subscriptionName, subscriptionNamespace, eventType, - fmt.Sprintf("%v", sink), - fmt.Sprintf("%v", statusCode), + sink, + strconv.Itoa(statusCode), consumerName).Inc() } @@ -147,8 +147,8 @@ func (c *Collector) RecordLatencyPerSubscription( subscriptionName, subscriptionNamespace, eventType, - fmt.Sprintf("%v", sink), - fmt.Sprintf("%v", statusCode), + sink, + strconv.Itoa(statusCode), consumerName).Observe(duration.Seconds()) } diff --git a/pkg/ems/httpclient/error_unit_test.go b/pkg/ems/httpclient/error_unit_test.go index 70fafe80..5880dbb6 100644 --- a/pkg/ems/httpclient/error_unit_test.go +++ b/pkg/ems/httpclient/error_unit_test.go @@ -1,3 +1,4 @@ +//nolint:goerr113 // no need to wrap errors in the test for the error package package httpclient import ( diff --git a/pkg/env/config.go b/pkg/env/config.go index d4eabf1f..5604f5cb 100644 --- a/pkg/env/config.go +++ b/pkg/env/config.go @@ -8,6 +8,7 @@ import ( "time" "github.com/kelseyhightower/envconfig" + "github.com/pkg/errors" ) const ( @@ -17,6 +18,8 @@ const ( backendValueNats = "NATS" ) +var ErrInvalidBackend = errors.New("invalid backend") + // Backend returns the selected backend based on the environment variable // "BACKEND". "NATS" is the default value in case of an empty variable. func Backend() (string, error) { @@ -28,7 +31,7 @@ func Backend() (string, error) { case backendValueNats, "": return backendValueNats, nil default: - return "", fmt.Errorf("invalid BACKEND set: %v", backend) + return "", fmt.Errorf("%w: %v", ErrInvalidBackend, backend) } } diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index ea0580b0..51d74aa0 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -14,7 +14,7 @@ import ( // errors.Is(err, pkg.ErrPermission) instead of // err == pkg.ErrPermission { … }. func MakeError(actualError, underlyingError error) error { - return fmt.Errorf("%w: %v", actualError, underlyingError) + return fmt.Errorf("%w: %w", actualError, underlyingError) } // MakeSubscriptionError creates a new error and includes the underlyingError in the message diff --git a/pkg/errors/errors_unit_test.go b/pkg/errors/errors_unit_test.go index 09c33021..b420ef94 100644 --- a/pkg/errors/errors_unit_test.go +++ b/pkg/errors/errors_unit_test.go @@ -1,3 +1,4 @@ +//nolint:goerr113 // no need to wrap errors in the test for the error package package errors_test import ( diff --git a/pkg/eventing/manager.go b/pkg/eventing/manager.go index d9bd137e..65397e0a 100644 --- a/pkg/eventing/manager.go +++ b/pkg/eventing/manager.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/pkg/errors" kappsv1 "k8s.io/api/apps/v1" kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/record" @@ -29,6 +30,11 @@ var allowedAnnotations = map[string]string{ "kubectl.kubernetes.io/restartedAt": "", } +var ( + ErrUnknownBackendType = errors.New("unknown backend type") + ErrEPPDeployFailed = errors.New("failed to apply Publisher Proxy deployment") +) + //go:generate go run github.com/vektra/mockery/v2 --name=Manager --outpkg=mocks --case=underscore type Manager interface { IsNATSAvailable(ctx context.Context, namespace string) (bool, error) @@ -97,16 +103,16 @@ func (em *EventingManager) applyPublisherProxyDeployment( case v1alpha1.EventMeshBackendType: desiredPublisher = newEventMeshPublisherDeployment(eventing, em.backendConfig.PublisherConfig) default: - return nil, fmt.Errorf("unknown EventingBackend type %q", backendType) + return nil, fmt.Errorf("%w: %q", ErrUnknownBackendType, backendType) } if err := controllerutil.SetControllerReference(eventing, desiredPublisher, em.Scheme()); err != nil { - return nil, fmt.Errorf("failed to set controller reference: %v", err) + return nil, fmt.Errorf("failed to set controller reference: %w", err) } currentPublisher, err := em.kubeClient.GetDeployment(ctx, GetPublisherDeploymentName(*eventing), eventing.Namespace) if err != nil { - return nil, fmt.Errorf("failed to get Event Publisher deployment: %v", err) + return nil, fmt.Errorf("failed to get Event Publisher deployment: %w", err) } if currentPublisher != nil { @@ -120,7 +126,7 @@ func (em *EventingManager) applyPublisherProxyDeployment( // if a publisher deploy from eventing-controller exists, then update it. if err := em.migratePublisherDeploymentFromEC(ctx, eventing, *currentPublisher, *desiredPublisher); err != nil { - return nil, fmt.Errorf("failed to migrate publisher: %v", err) + return nil, fmt.Errorf("failed to migrate publisher: %w", err) } } @@ -133,7 +139,7 @@ func (em *EventingManager) applyPublisherProxyDeployment( // Update publisher proxy deployment if err := em.kubeClient.PatchApply(ctx, desiredPublisher); err != nil { - return nil, fmt.Errorf("failed to apply Publisher Proxy deployment: %v", err) + return nil, fmt.Errorf("%w: %w", ErrEPPDeployFailed, err) } return desiredPublisher, nil @@ -154,7 +160,7 @@ func (em *EventingManager) migratePublisherDeploymentFromEC( // change OwnerReference to Eventing CR. updatedPublisher.OwnerReferences = nil if err := controllerutil.SetControllerReference(eventing, updatedPublisher, em.Scheme()); err != nil { - return fmt.Errorf("failed to set controller reference: %v", err) + return fmt.Errorf("failed to set controller reference: %w", err) } // copy Spec from desired publisher // because some ENV variables conflicts with server-side patch apply. @@ -284,6 +290,6 @@ func convertECBackendType(backendType v1alpha1.BackendType) (eventingv1alpha1.Ba case v1alpha1.NatsBackendType: return eventingv1alpha1.NatsBackendType, nil default: - return "", fmt.Errorf("unknown backend type: %s", backendType) + return "", fmt.Errorf("%w: %s", ErrUnknownBackendType, backendType) } } diff --git a/pkg/eventing/manager_test.go b/pkg/eventing/manager_test.go index 548d4703..544de214 100644 --- a/pkg/eventing/manager_test.go +++ b/pkg/eventing/manager_test.go @@ -3,7 +3,6 @@ package eventing import ( "context" "errors" - "fmt" "testing" natsv1alpha1 "github.com/kyma-project/nats-manager/api/v1alpha1" @@ -26,6 +25,8 @@ import ( testutils "github.com/kyma-project/eventing-manager/test/utils" ) +var ErrUseMeWithMocks = errors.New("use me with mocks") + func Test_ApplyPublisherProxyDeployment(t *testing.T) { // given newScheme := runtime.NewScheme() @@ -77,7 +78,7 @@ func Test_ApplyPublisherProxyDeployment(t *testing.T) { testutils.WithEventingCRMinimal(), ), givenBackendType: "unknown-backend", - wantErr: fmt.Errorf("unknown EventingBackend type %q", "unknown-backend"), + wantErr: ErrUnknownBackendType, }, { name: "PatchApply failure", @@ -86,9 +87,8 @@ func Test_ApplyPublisherProxyDeployment(t *testing.T) { testutils.WithEventingEventTypePrefix("test-prefix"), ), givenBackendType: v1alpha1.NatsBackendType, - patchApplyErr: errors.New("patch apply error"), - wantErr: fmt.Errorf("failed to apply Publisher Proxy deployment: %v", - errors.New("patch apply error")), + patchApplyErr: ErrUseMeWithMocks, + wantErr: ErrUseMeWithMocks, }, } @@ -122,7 +122,7 @@ func Test_ApplyPublisherProxyDeployment(t *testing.T) { deployment, err := em.applyPublisherProxyDeployment(ctx, tc.givenEventing, &env.NATSConfig{}, tc.givenBackendType) // then - require.Equal(t, tc.wantErr, err) + require.ErrorIs(t, err, tc.wantErr) if tc.wantedDeployment != nil { require.NotNil(t, deployment) require.Equal(t, tc.wantedDeployment.Spec.Template.ObjectMeta.Annotations, @@ -289,7 +289,7 @@ func Test_IsNATSAvailable(t *testing.T) { givenNATSResources: nil, givenNamespace: "test-namespace", wantAvailable: false, - wantErr: errors.New("failed to get NATS resources"), + wantErr: ErrUseMeWithMocks, }, } @@ -339,7 +339,7 @@ func Test_ConvertECBackendType(t *testing.T) { name: "Unknown backend type", backendType: "unknown", expectedResult: "", - expectedError: fmt.Errorf("unknown backend type: unknown"), + expectedError: ErrUnknownBackendType, }, } @@ -349,7 +349,7 @@ func Test_ConvertECBackendType(t *testing.T) { // when result, err := convertECBackendType(tc.backendType) // then - require.Equal(t, tc.expectedError, err) + require.ErrorIs(t, err, tc.expectedError) require.Equal(t, tc.expectedResult, result) }) } @@ -407,7 +407,7 @@ func Test_DeployPublisherProxyResources(t *testing.T) { var createdObjects []client.Object // define mocks behaviours. if tc.wantError { - kubeClient.On("PatchApply", ctx, mock.Anything).Return(errors.New("failed")) + kubeClient.On("PatchApply", ctx, mock.Anything).Return(ErrUseMeWithMocks) } else { kubeClient.On("PatchApply", ctx, mock.Anything).Run(func(args mock.Arguments) { obj := args.Get(1).(client.Object) @@ -521,7 +521,7 @@ func Test_DeletePublisherProxyResources(t *testing.T) { // define mocks behaviours. if tc.wantError { - kubeClient.On("DeleteResource", ctx, mock.Anything).Return(errors.New("failed")) + kubeClient.On("DeleteResource", ctx, mock.Anything).Return(ErrUseMeWithMocks) } else { kubeClient.On("DeleteResource", ctx, mock.Anything).Return(nil).Times(tc.wantDeletedResourcesCount) } @@ -586,7 +586,7 @@ func Test_SubscriptionExists(t *testing.T) { { name: "error should have occurred", wantResult: false, - wantError: errors.New("client error"), + wantError: ErrUseMeWithMocks, }, } diff --git a/pkg/k8s/client.go b/pkg/k8s/client.go index 6d39e4f9..08fef405 100644 --- a/pkg/k8s/client.go +++ b/pkg/k8s/client.go @@ -30,6 +30,8 @@ var NatsGVK = schema.GroupVersionResource{ Resource: "nats", } +var ErrSecretRefInvalid = errors.New("invalid namespaced name. It must be in the format of 'namespace/name'") + //nolint:interfacebloat // FIXME //go:generate go run github.com/vektra/mockery/v2 --name=Client --outpkg=mocks --case=underscore type Client interface { @@ -194,7 +196,7 @@ func (c *KubeClient) PatchApply(ctx context.Context, object client.Object) error func (c *KubeClient) GetSecret(ctx context.Context, namespacedName string) (*kcorev1.Secret, error) { substrings := strings.Split(namespacedName, "/") if len(substrings) != 2 { - return nil, errors.New("invalid namespaced name. It must be in the format of 'namespace/name'") + return nil, ErrSecretRefInvalid } secret := &kcorev1.Secret{} err := c.client.Get(ctx, client.ObjectKey{ diff --git a/pkg/k8s/client_test.go b/pkg/k8s/client_test.go index 62d37e1c..6f999009 100644 --- a/pkg/k8s/client_test.go +++ b/pkg/k8s/client_test.go @@ -3,7 +3,6 @@ package k8s import ( "context" "crypto/rand" - "errors" "testing" "github.com/stretchr/testify/require" @@ -511,7 +510,7 @@ func Test_GetSecret(t *testing.T) { name: "namespaced name format error", givenNamespacedName: "my-secret", wantSecret: nil, - wantError: errors.New("invalid namespaced name. It must be in the format of 'namespace/name'"), + wantError: ErrSecretRefInvalid, }, } @@ -538,7 +537,7 @@ func Test_GetSecret(t *testing.T) { if tc.wantNotFoundError { require.True(t, kerrors.IsNotFound(err)) } else { - require.Equal(t, tc.wantError, err) + require.ErrorIs(t, err, tc.wantError) } require.Equal(t, tc.wantSecret, secret) }) diff --git a/pkg/signals/signals.go b/pkg/signals/signals.go index 3e2fcc8a..4f84f343 100644 --- a/pkg/signals/signals.go +++ b/pkg/signals/signals.go @@ -15,6 +15,8 @@ var ( // shutdownSignals array of system signals to cause shutdown. shutdownSignals = []os.Signal{syscall.SIGINT, syscall.SIGTERM} + + ErrTerminationRequested = errors.New("received a termination signal") ) // SetupSignalHandler registered for SIGTERM and SIGINT. A stop channel is returned @@ -72,7 +74,7 @@ func (scc *signalContext) Err() error { select { case _, ok := <-scc.Done(): if !ok { - return errors.New("received a termination signal") + return ErrTerminationRequested } default: } diff --git a/pkg/subscriptionmanager/eventmesh/eventmesh.go b/pkg/subscriptionmanager/eventmesh/eventmesh.go index 6d36a4be..f3e7bd20 100644 --- a/pkg/subscriptionmanager/eventmesh/eventmesh.go +++ b/pkg/subscriptionmanager/eventmesh/eventmesh.go @@ -2,7 +2,6 @@ package eventmesh import ( "context" - "fmt" "strings" "time" @@ -37,6 +36,11 @@ const ( subscriptionManagerName = "beb-subscription-manager" ) +var ( + ErrDecodingOauthCredentialFailed = errors.New("in") + ErrDomainEmpty = errors.New("domain must be a non-empty value") +) + // AddToScheme adds the own schemes to the runtime scheme. func AddToScheme(scheme *runtime.Scheme) error { if err := kkubernetesscheme.AddToScheme(scheme); err != nil { @@ -92,7 +96,7 @@ func NewSubscriptionManager(restCfg *rest.Config, metricsAddr string, resyncPeri // Init implements the subscriptionmanager.Manager interface. func (c *SubscriptionManager) Init(mgr manager.Manager) error { if len(c.domain) == 0 { - return fmt.Errorf("domain must be a non-empty value") + return ErrDomainEmpty } c.mgr = mgr return nil @@ -104,10 +108,7 @@ func (c *SubscriptionManager) Start(_ env.DefaultSubscriptionConfig, params subm ctx, cancel := context.WithCancel(context.Background()) c.cancel = cancel - oauth2credential, err := getOAuth2ClientCredentials(params) - if err != nil { - return errors.Wrap(err, "get oauth2client credentials failed") - } + oauth2credential := getOAuth2ClientCredentials(params) // Need to read env to read BEB related secrets c.envCfg = env.GetConfig() @@ -251,37 +252,13 @@ func cleanupEventMesh(backend backendeventmesh.Backend, dynamicClient dynamic.In return nil } -func getOAuth2ClientCredentials(params submgrmanager.Params) (*backendeventmesh.OAuth2ClientCredentials, error) { - val := params[submgrmanager.ParamNameClientID] - id, ok := val.([]byte) - if !ok { - return nil, fmt.Errorf("expected []byte value for %s", submgrmanager.ParamNameClientID) - } - - val = params[submgrmanager.ParamNameClientSecret] - secret, ok := val.([]byte) - if !ok { - return nil, fmt.Errorf("expected []byte value for %s", submgrmanager.ParamNameClientSecret) - } - - val = params[submgrmanager.ParamNameTokenURL] - tokenURL, ok := val.([]byte) - if !ok { - return nil, fmt.Errorf("expected []byte value for %s", submgrmanager.ParamNameTokenURL) - } - - val = params[submgrmanager.ParamNameCertsURL] - certsURL, ok := val.([]byte) - if !ok { - return nil, fmt.Errorf("expected []byte value for %s", submgrmanager.ParamNameCertsURL) - } - +func getOAuth2ClientCredentials(params submgrmanager.Params) *backendeventmesh.OAuth2ClientCredentials { return &backendeventmesh.OAuth2ClientCredentials{ - ClientID: string(id), - ClientSecret: string(secret), - TokenURL: string(tokenURL), - CertsURL: string(certsURL), - }, nil + ClientID: string(params[submgrmanager.ParamNameClientID]), + ClientSecret: string(params[submgrmanager.ParamNameClientSecret]), + TokenURL: string(params[submgrmanager.ParamNameTokenURL]), + CertsURL: string(params[submgrmanager.ParamNameCertsURL]), + } } func (c *SubscriptionManager) namedLogger() *zap.SugaredLogger { diff --git a/pkg/subscriptionmanager/manager/manager.go b/pkg/subscriptionmanager/manager/manager.go index 9f6052ab..5e7285b8 100644 --- a/pkg/subscriptionmanager/manager/manager.go +++ b/pkg/subscriptionmanager/manager/manager.go @@ -13,7 +13,7 @@ const ( ParamNameCertsURL = "certs_url" ) -type Params map[string]interface{} +type Params map[string][]byte // Manager defines the interface that subscription managers for different messaging backends should implement. // diff --git a/test/utils/utils.go b/test/utils/utils.go index 53e0411a..c2719740 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -32,7 +32,10 @@ const ( PublisherProxySuffix = "publisher-proxy" ) -var seededRand = rand.New(rand.NewSource(time.Now().UnixNano())) //nolint:gosec,gochecknoglobals // used in tests +var ( + seededRand = rand.New(rand.NewSource(time.Now().UnixNano())) //nolint:gosec,gochecknoglobals // used in tests + ErrNotFound = errors.New("not found") +) func GetRandString(length int) string { b := make([]byte, length) @@ -373,7 +376,7 @@ func FindObjectByKind(kind string, objects []client.Object) (client.Object, erro } } - return nil, errors.New("not found") + return nil, ErrNotFound } func FindServiceFromK8sObjects(name string, objects []client.Object) (client.Object, error) { @@ -384,5 +387,5 @@ func FindServiceFromK8sObjects(name string, objects []client.Object) (client.Obj } } - return nil, errors.New("not found") + return nil, ErrNotFound } diff --git a/testing/event/cehelper/converter.go b/testing/event/cehelper/converter.go index 7c398108..92f9123b 100644 --- a/testing/event/cehelper/converter.go +++ b/testing/event/cehelper/converter.go @@ -29,7 +29,7 @@ func RequestToEventString(r *http.Request) (string, error) { msg := cehttp.NewMessageFromHttpRequest(r) event, err := cebinding.ToEvent(context.Background(), msg) if err != nil { - return "", fmt.Errorf("failed to build a CloudEvent: %s", err.Error()) + return "", fmt.Errorf("failed to build a CloudEvent: %w", err) } return event.String(), nil } diff --git a/testing/subscriber.go b/testing/subscriber.go index d3593686..d6919646 100644 --- a/testing/subscriber.go +++ b/testing/subscriber.go @@ -26,6 +26,12 @@ const ( checkRetriesEndpoint = "/check_retries" ) +var ( + ErrEventNotReceived = errors.New("event not received") + ErrUnexpectedResponseCode = errors.New("unexpected response code received") + ErrWrongRetries = errors.New("wrong number of retries") +) + type Subscriber struct { server *httptest.Server SinkURL string @@ -207,22 +213,22 @@ func (s Subscriber) CheckEvent(expectedData string) error { // check if a response was received and that it's code is in 2xx-range resp, err := http.Get(s.checkURL) if err != nil { - return errors.Wrapf(err, "get HTTP request failed") + return fmt.Errorf("get HTTP request failed: %w", err) } if !is2XXStatusCode(resp.StatusCode) { - return fmt.Errorf("expected resonse code 2xx, actual response code: %d", resp.StatusCode) + return fmt.Errorf("%w: expected: 2xx, actual: %d", ErrUnexpectedResponseCode, resp.StatusCode) } // try to read the response body defer func() { _ = resp.Body.Close() }() body, err = io.ReadAll(resp.Body) if err != nil { - return errors.Wrapf(err, "read data failed") + return fmt.Errorf("read data failed: %w", err) } // compare response body with expectations if expectedData != string(body) { - return fmt.Errorf("event not received") + return ErrEventNotReceived } return nil }, @@ -232,7 +238,7 @@ func (s Subscriber) CheckEvent(expectedData string) error { retry.OnRetry(func(n uint, err error) { log.Printf("[%v] try failed: %s", n, err) }), ) if err != nil { - return errors.Wrapf(err, "check event after retries failed") + return fmt.Errorf("check event after retries failed: %w", err) } log.Print("event received") @@ -248,22 +254,22 @@ func (s Subscriber) CheckRetries(expectedNoOfRetries int, expectedData string) e func() error { resp, err := http.Get(s.checkRetriesURL) if err != nil { - return errors.Wrapf(err, "get HTTP request failed") + return fmt.Errorf("get HTTP request failed: %w", err) } if !is2XXStatusCode(resp.StatusCode) { - return fmt.Errorf("expected resonse code 2xx, actual response code: %d", resp.StatusCode) + return fmt.Errorf("%w: expected: 2xx, actual: %d", ErrUnexpectedResponseCode, resp.StatusCode) } defer func() { _ = resp.Body.Close() }() body, err = io.ReadAll(resp.Body) if err != nil { - return errors.Wrapf(err, "read data failed") + return fmt.Errorf("read data failed: %w", err) } actualRetires, err := strconv.Atoi(string(body)) if err != nil { - return errors.Wrapf(err, "read data failed") + return fmt.Errorf("read data failed: %w", err) } if actualRetires < expectedNoOfRetries { - return fmt.Errorf("number of retries do not match (actualRetires=%d, expectedRetries=%d)", actualRetires, expectedNoOfRetries) + return fmt.Errorf("%w: actualRetires:%d, expectedRetries:%d", ErrWrongRetries, actualRetires, expectedNoOfRetries) } return nil },