From d7a0850f85759f1155e3790317c1aa2f5f3012f4 Mon Sep 17 00:00:00 2001 From: Korbinian Stoemmer Date: Fri, 22 Dec 2023 17:16:11 +0100 Subject: [PATCH] enable more linters (part 4) (#354) * enable gochecknoglobals * enable gochecknoinits * enable noctx * enable godox * enable ginkgolinter * enable errorlint * enable stylecheck * enable gomnd * enable gocritic * enable thelper * enable godot * enable gocognit * enable staticcheck * enable goconst * enable unparam --- .golangci.yaml | 19 +-- api/eventing/v1alpha1/condition.go | 7 +- api/eventing/v1alpha1/fixtures_test.go | 10 +- api/eventing/v1alpha1/groupversion_info.go | 2 + .../v1alpha1/subscription_conversion.go | 148 ++++++++--------- .../subscription_conversion_unit_test.go | 3 + api/eventing/v1alpha1/subscription_types.go | 3 +- api/eventing/v1alpha1/subscription_webhook.go | 6 +- api/eventing/v1alpha2/condition.go | 4 +- api/eventing/v1alpha2/groupversion_info.go | 2 + api/eventing/v1alpha2/subscription_types.go | 1 + api/operator/v1alpha1/eventing_types.go | 1 + api/operator/v1alpha1/groupversion_info.go | 2 + api/operator/v1alpha1/status.go | 8 +- cmd/main.go | 45 +++--- hack/e2e/common/eventing/publisher.go | 5 +- hack/e2e/common/eventing/sinkclient.go | 20 +-- hack/e2e/common/eventing/utils.go | 4 +- hack/e2e/common/fixtures/fixtures.go | 1 + hack/e2e/common/http/http.go | 2 +- .../testenvironment/test_environment.go | 18 +-- internal/controller/cache/cache.go | 2 +- internal/controller/errors/skip.go | 2 +- .../subscription/eventmesh/reconciler.go | 6 +- .../reconciler_internal_integration_test.go | 2 + .../subscription/eventmesh/test/assertions.go | 2 +- .../subscription/eventmesh/test/utils.go | 69 +++++--- .../eventmesh/testwebhookauth/utils.go | 35 ++-- .../eventing/subscription/eventmesh/utils.go | 3 +- .../reconciler_internal_unit_test.go | 8 +- .../subscription/jetstream/test_utils_test.go | 7 +- .../operator/eventing/controller.go | 11 +- .../controller/operator/eventing/eventmesh.go | 35 ++-- .../operator/eventing/eventmesh_test.go | 12 +- .../controller/integration_test.go | 33 ++-- .../integration_test.go | 4 +- .../nats_disabled/integration_test.go | 4 +- .../validation/integration_test.go | 12 +- .../without_apirule_crd/integration_test.go | 2 +- internal/controller/operator/eventing/nats.go | 10 +- .../controller/operator/eventing/nats_test.go | 8 +- .../controller/operator/eventing/status.go | 8 +- .../controller/operator/eventing/unit_test.go | 1 + options/options.go | 23 ++- pkg/backend/eventmesh/eventmesh.go | 4 +- pkg/backend/eventtype/clean.go | 12 +- pkg/backend/eventtype/parse.go | 3 +- pkg/backend/jetstream/config.go | 3 +- .../jetstream/connection_integration_test.go | 2 + .../jetstream/jetstream_integration_test.go | 18 ++- pkg/backend/jetstream/stubs_test.go | 1 + pkg/backend/jetstream/test_helpers.go | 3 +- pkg/backend/jetstream/utils.go | 1 - pkg/backend/metrics/collector.go | 8 +- pkg/backend/utils/eventmesh_utils.go | 39 ++--- pkg/backend/utils/eventmesh_utils_test.go | 9 +- pkg/ems/auth/auth.go | 1 - pkg/ems/httpclient/client.go | 6 +- pkg/env/config_test.go | 2 +- pkg/errors/errors.go | 4 +- pkg/errors/errors_unit_test.go | 2 +- pkg/eventing/deployment.go | 36 +++-- pkg/eventing/deployment_test.go | 2 + pkg/eventing/manager.go | 8 +- pkg/eventing/utils.go | 35 ++-- pkg/informers/sync.go | 3 +- pkg/k8s/client.go | 15 +- pkg/object/equality.go | 2 + pkg/signals/signals.go | 14 +- .../eventmesh/eventmesh_test.go | 12 +- .../jetstream/jetstream_test.go | 8 +- pkg/tracing/tracing_test.go | 2 +- pkg/utils/utils.go | 8 +- pkg/watcher/watcher.go | 3 +- test/utils/integration/integration.go | 150 +++++++++++------- test/utils/utils.go | 1 + testing/eventmeshmock.go | 82 +++++----- testing/subscriber.go | 20 ++- 78 files changed, 639 insertions(+), 490 deletions(-) rename internal/controller/operator/eventing/integrationtests/{controller_switching => controllerswitching}/integration_test.go (99%) diff --git a/.golangci.yaml b/.golangci.yaml index cbecb050..795fa348 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -20,35 +20,19 @@ linters: - wsl # too strict and mostly code is not more readable ### disabled for now... will be enabled 1 by 1 - dupl - - errorlint - forcetypeassert - - funlen - - ginkgolinter - - gochecknoglobals - - gochecknoinits - - gocognit - - goconst - - gocritic - - godox - - gomnd - gosec - inamedparam - ireturn - maintidx - - noctx - nolintlint - paralleltest - prealloc - - stylecheck - testpackage - - thelper - tparallel - unconvert - - unparam - varnamelen - wrapcheck - - godot - - tagalign linters-settings: stylecheck: @@ -226,6 +210,9 @@ issues: - path: "_test\\.go" linters: - wrapcheck + - gochecknoglobals + - funlen # Table driven unit and integration tests exceed function length by design + - maintidx # Table driven unit and integration tests exceed maintainability index by design - linters: - importas text: has alias "" which is not part of config # Ignore false positives that emerged due to https://github.com/julz/importas/issues/15. diff --git a/api/eventing/v1alpha1/condition.go b/api/eventing/v1alpha1/condition.go index 74c23114..ea81a376 100644 --- a/api/eventing/v1alpha1/condition.go +++ b/api/eventing/v1alpha1/condition.go @@ -19,8 +19,6 @@ const ( ConditionControllerReady ConditionType = "Subscription Controller Ready" ) -var allSubscriptionConditions = MakeSubscriptionConditions() - type Condition struct { // Short description of the condition. Type ConditionType `json:"type,omitempty"` @@ -78,7 +76,7 @@ func (s *SubscriptionStatus) InitializeConditions() { } func (s SubscriptionStatus) IsReady() bool { - if !ContainSameConditionTypes(allSubscriptionConditions, s.Conditions) { + if !ContainSameConditionTypes(MakeSubscriptionConditions(), s.Conditions) { return false } @@ -166,8 +164,7 @@ func MakeCondition(conditionType ConditionType, reason ConditionReason, status k Status: status, LastTransitionTime: kmetav1.Now(), Reason: reason, - // TODO: https://github.com/kyma-project/kyma/issues/9770 - Message: message, + Message: message, } } diff --git a/api/eventing/v1alpha1/fixtures_test.go b/api/eventing/v1alpha1/fixtures_test.go index 0ddd7ddb..07fdc3b4 100644 --- a/api/eventing/v1alpha1/fixtures_test.go +++ b/api/eventing/v1alpha1/fixtures_test.go @@ -13,11 +13,11 @@ import ( const ( eventSource = "source" - orderCreatedEventType = "prefix." + "noapp." + "order.created.v1" - orderUpdatedEventType = "prefix." + "app." + "order.updated.v1" - orderDeletedEventType = "prefix." + "noapp." + "order.deleted.v1" - orderDeletedEventTypeNonClean = "prefix." + "noapp." + "order.deleted_&.v1" - orderProcessedEventType = "prefix." + "noapp." + "order.processed.v1" + orderCreatedEventType = "prefix.noapp.order.created.v1" + orderUpdatedEventType = "prefix.app.order.updated.v1" + orderDeletedEventType = "prefix.noapp.order.deleted.v1" + orderDeletedEventTypeNonClean = "prefix.noapp.order.deleted_&.v1" + orderProcessedEventType = "prefix.noapp.order.processed.v1" ) const ( diff --git a/api/eventing/v1alpha1/groupversion_info.go b/api/eventing/v1alpha1/groupversion_info.go index 8f4291fe..7b7f8d50 100644 --- a/api/eventing/v1alpha1/groupversion_info.go +++ b/api/eventing/v1alpha1/groupversion_info.go @@ -1,6 +1,8 @@ // Package v1alpha1 contains API Schema definitions for the eventing v1alpha1 API group // +kubebuilder:object:generate=true // +groupName=eventing.kyma-project.io +// +//nolint:gochecknoglobals // required for utilizing the API package v1alpha1 import ( diff --git a/api/eventing/v1alpha1/subscription_conversion.go b/api/eventing/v1alpha1/subscription_conversion.go index e0fea092..ebb170cc 100644 --- a/api/eventing/v1alpha1/subscription_conversion.go +++ b/api/eventing/v1alpha1/subscription_conversion.go @@ -25,12 +25,12 @@ func InitializeEventTypeCleaner(cleaner eventtype.Cleaner) { } // ConvertTo converts this Subscription in version v1 to the Hub version v2. -func (src *Subscription) ConvertTo(dstRaw conversion.Hub) error { +func (s *Subscription) ConvertTo(dstRaw conversion.Hub) error { dst, ok := dstRaw.(*v1alpha2.Subscription) if !ok { return errors.Errorf(ErrorHubVersionMsg) } - return V1ToV2(src, dst) + return V1ToV2(s, dst) } // V1ToV2 copies the v1alpha1-type field values into v1alpha2-type field values. @@ -61,12 +61,12 @@ func V1ToV2(src *Subscription, dst *v1alpha2.Subscription) error { } // ConvertFrom converts this Subscription from the Hub version (v2) to v1. -func (dst *Subscription) ConvertFrom(srcRaw conversion.Hub) error { +func (s *Subscription) ConvertFrom(srcRaw conversion.Hub) error { src, ok := srcRaw.(*v1alpha2.Subscription) if !ok { return errors.Errorf(ErrorHubVersionMsg) } - return V2ToV1(dst, src) + return V2ToV1(s, src) } // V2ToV1 copies the v1alpha2-type field values into v1alpha1-type field values. @@ -120,117 +120,117 @@ func V2ToV1(dst *Subscription, src *v1alpha2.Subscription) error { } // setV2TypeMatching sets the default typeMatching on the v1alpha2 Subscription version. -func (src *Subscription) setV2TypeMatching(dst *v1alpha2.Subscription) { +func (s *Subscription) setV2TypeMatching(dst *v1alpha2.Subscription) { dst.Spec.TypeMatching = v1alpha2.TypeMatchingExact } // setV2ProtocolFields converts the protocol-related fields from v1alpha1 to v1alpha2 Subscription version. -func (src *Subscription) setV2ProtocolFields(dst *v1alpha2.Subscription) { +func (s *Subscription) setV2ProtocolFields(dst *v1alpha2.Subscription) { dst.Spec.Config = map[string]string{} - if src.Spec.Protocol != "" { - dst.Spec.Config[v1alpha2.Protocol] = src.Spec.Protocol + if s.Spec.Protocol != "" { + dst.Spec.Config[v1alpha2.Protocol] = s.Spec.Protocol } // protocol settings - if src.Spec.ProtocolSettings != nil { - src.setProtocolSettings(dst) + if s.Spec.ProtocolSettings != nil { + s.setProtocolSettings(dst) } } -func (src *Subscription) setProtocolSettings(dst *v1alpha2.Subscription) { - if src.Spec.ProtocolSettings.ContentMode != nil { - dst.Spec.Config[v1alpha2.ProtocolSettingsContentMode] = *src.Spec.ProtocolSettings.ContentMode +func (s *Subscription) setProtocolSettings(dst *v1alpha2.Subscription) { + if s.Spec.ProtocolSettings.ContentMode != nil { + dst.Spec.Config[v1alpha2.ProtocolSettingsContentMode] = *s.Spec.ProtocolSettings.ContentMode } - if src.Spec.ProtocolSettings.ExemptHandshake != nil { - dst.Spec.Config[v1alpha2.ProtocolSettingsExemptHandshake] = strconv.FormatBool(*src.Spec.ProtocolSettings.ExemptHandshake) + if s.Spec.ProtocolSettings.ExemptHandshake != nil { + dst.Spec.Config[v1alpha2.ProtocolSettingsExemptHandshake] = strconv.FormatBool(*s.Spec.ProtocolSettings.ExemptHandshake) } - if src.Spec.ProtocolSettings.Qos != nil { - dst.Spec.Config[v1alpha2.ProtocolSettingsQos] = *src.Spec.ProtocolSettings.Qos + if s.Spec.ProtocolSettings.Qos != nil { + dst.Spec.Config[v1alpha2.ProtocolSettingsQos] = *s.Spec.ProtocolSettings.Qos } // webhookAuth fields - if src.Spec.ProtocolSettings.WebhookAuth != nil { - if src.Spec.ProtocolSettings.WebhookAuth.Type != "" { - dst.Spec.Config[v1alpha2.WebhookAuthType] = src.Spec.ProtocolSettings.WebhookAuth.Type + if s.Spec.ProtocolSettings.WebhookAuth != nil { + if s.Spec.ProtocolSettings.WebhookAuth.Type != "" { + dst.Spec.Config[v1alpha2.WebhookAuthType] = s.Spec.ProtocolSettings.WebhookAuth.Type } - dst.Spec.Config[v1alpha2.WebhookAuthGrantType] = src.Spec.ProtocolSettings.WebhookAuth.GrantType - dst.Spec.Config[v1alpha2.WebhookAuthClientID] = src.Spec.ProtocolSettings.WebhookAuth.ClientID - dst.Spec.Config[v1alpha2.WebhookAuthClientSecret] = src.Spec.ProtocolSettings.WebhookAuth.ClientSecret - dst.Spec.Config[v1alpha2.WebhookAuthTokenURL] = src.Spec.ProtocolSettings.WebhookAuth.TokenURL - if src.Spec.ProtocolSettings.WebhookAuth.Scope != nil { - dst.Spec.Config[v1alpha2.WebhookAuthScope] = strings.Join(src.Spec.ProtocolSettings.WebhookAuth.Scope, ",") + dst.Spec.Config[v1alpha2.WebhookAuthGrantType] = s.Spec.ProtocolSettings.WebhookAuth.GrantType + dst.Spec.Config[v1alpha2.WebhookAuthClientID] = s.Spec.ProtocolSettings.WebhookAuth.ClientID + dst.Spec.Config[v1alpha2.WebhookAuthClientSecret] = s.Spec.ProtocolSettings.WebhookAuth.ClientSecret + dst.Spec.Config[v1alpha2.WebhookAuthTokenURL] = s.Spec.ProtocolSettings.WebhookAuth.TokenURL + if s.Spec.ProtocolSettings.WebhookAuth.Scope != nil { + dst.Spec.Config[v1alpha2.WebhookAuthScope] = strings.Join(s.Spec.ProtocolSettings.WebhookAuth.Scope, ",") } } } -func (src *Subscription) initializeProtocolSettingsIfNil() { - if src.Spec.ProtocolSettings == nil { - src.Spec.ProtocolSettings = &ProtocolSettings{} +func (s *Subscription) initializeProtocolSettingsIfNil() { + if s.Spec.ProtocolSettings == nil { + s.Spec.ProtocolSettings = &ProtocolSettings{} } } -func (src *Subscription) initializeWebhookAuthIfNil() { - src.initializeProtocolSettingsIfNil() - if src.Spec.ProtocolSettings.WebhookAuth == nil { - src.Spec.ProtocolSettings.WebhookAuth = &WebhookAuth{} +func (s *Subscription) initializeWebhookAuthIfNil() { + s.initializeProtocolSettingsIfNil() + if s.Spec.ProtocolSettings.WebhookAuth == nil { + s.Spec.ProtocolSettings.WebhookAuth = &WebhookAuth{} } } // setV1ProtocolFields converts the protocol-related fields from v1alpha1 to v1alpha2 Subscription version. -func (src *Subscription) setV1ProtocolFields(dst *v1alpha2.Subscription) { +func (s *Subscription) setV1ProtocolFields(dst *v1alpha2.Subscription) { if protocol, ok := dst.Spec.Config[v1alpha2.Protocol]; ok { - src.Spec.Protocol = protocol + s.Spec.Protocol = protocol } if currentMode, ok := dst.Spec.Config[v1alpha2.ProtocolSettingsContentMode]; ok { - src.initializeProtocolSettingsIfNil() - src.Spec.ProtocolSettings.ContentMode = ¤tMode + s.initializeProtocolSettingsIfNil() + s.Spec.ProtocolSettings.ContentMode = ¤tMode } if qos, ok := dst.Spec.Config[v1alpha2.ProtocolSettingsQos]; ok { - src.initializeProtocolSettingsIfNil() - src.Spec.ProtocolSettings.Qos = &qos + s.initializeProtocolSettingsIfNil() + s.Spec.ProtocolSettings.Qos = &qos } if exemptHandshake, ok := dst.Spec.Config[v1alpha2.ProtocolSettingsExemptHandshake]; ok { handshake, err := strconv.ParseBool(exemptHandshake) if err != nil { handshake = true } - src.initializeProtocolSettingsIfNil() - src.Spec.ProtocolSettings.ExemptHandshake = &handshake + s.initializeProtocolSettingsIfNil() + s.Spec.ProtocolSettings.ExemptHandshake = &handshake } if authType, ok := dst.Spec.Config[v1alpha2.WebhookAuthType]; ok { - src.initializeWebhookAuthIfNil() - src.Spec.ProtocolSettings.WebhookAuth.Type = authType + s.initializeWebhookAuthIfNil() + s.Spec.ProtocolSettings.WebhookAuth.Type = authType } if grantType, ok := dst.Spec.Config[v1alpha2.WebhookAuthGrantType]; ok { - src.initializeWebhookAuthIfNil() - src.Spec.ProtocolSettings.WebhookAuth.GrantType = grantType + s.initializeWebhookAuthIfNil() + s.Spec.ProtocolSettings.WebhookAuth.GrantType = grantType } if clientID, ok := dst.Spec.Config[v1alpha2.WebhookAuthClientID]; ok { - src.initializeWebhookAuthIfNil() - src.Spec.ProtocolSettings.WebhookAuth.ClientID = clientID + s.initializeWebhookAuthIfNil() + s.Spec.ProtocolSettings.WebhookAuth.ClientID = clientID } if secret, ok := dst.Spec.Config[v1alpha2.WebhookAuthClientSecret]; ok { - src.initializeWebhookAuthIfNil() - src.Spec.ProtocolSettings.WebhookAuth.ClientSecret = secret + s.initializeWebhookAuthIfNil() + s.Spec.ProtocolSettings.WebhookAuth.ClientSecret = secret } if token, ok := dst.Spec.Config[v1alpha2.WebhookAuthTokenURL]; ok { - src.initializeWebhookAuthIfNil() - src.Spec.ProtocolSettings.WebhookAuth.TokenURL = token + s.initializeWebhookAuthIfNil() + s.Spec.ProtocolSettings.WebhookAuth.TokenURL = token } if scope, ok := dst.Spec.Config[v1alpha2.WebhookAuthScope]; ok { - src.initializeWebhookAuthIfNil() - src.Spec.ProtocolSettings.WebhookAuth.Scope = strings.Split(scope, ",") + s.initializeWebhookAuthIfNil() + s.Spec.ProtocolSettings.WebhookAuth.Scope = strings.Split(scope, ",") } } // setV2SpecTypes sets event types in the Subscription Spec in the v1alpha2 way. -func (src *Subscription) setV2SpecTypes(dst *v1alpha2.Subscription) error { +func (s *Subscription) setV2SpecTypes(dst *v1alpha2.Subscription) error { if v1alpha1TypeCleaner == nil { return errors.New("event type cleaner is not initialized") } - if src.Spec.Filter != nil { - for _, filter := range src.Spec.Filter.Filters { + if s.Spec.Filter != nil { + for _, filter := range s.Spec.Filter.Filters { if dst.Spec.Source == "" { dst.Spec.Source = filter.EventSource.Value } @@ -251,13 +251,13 @@ func (src *Subscription) setV2SpecTypes(dst *v1alpha2.Subscription) error { } // natsSpecConfigToV2 converts the v1alpha2 Spec config to v1alpha1. -func (src *Subscription) natsSpecConfigToV1(dst *v1alpha2.Subscription) error { +func (s *Subscription) natsSpecConfigToV1(dst *v1alpha2.Subscription) error { if maxInFlightMessages, ok := dst.Spec.Config[v1alpha2.MaxInFlightMessages]; ok { intVal, err := strconv.Atoi(maxInFlightMessages) if err != nil { return err } - src.Spec.Config = &SubscriptionConfig{ + s.Spec.Config = &SubscriptionConfig{ MaxInFlightMessages: intVal, } } @@ -265,24 +265,24 @@ func (src *Subscription) natsSpecConfigToV1(dst *v1alpha2.Subscription) error { } // natsSpecConfigToV2 converts the hardcoded v1alpha1 Spec config to v1alpha2 generic config version. -func (src *Subscription) natsSpecConfigToV2(dst *v1alpha2.Subscription) { - if src.Spec.Config != nil { +func (s *Subscription) natsSpecConfigToV2(dst *v1alpha2.Subscription) { + if s.Spec.Config != nil { if dst.Spec.Config == nil { dst.Spec.Config = map[string]string{} } - dst.Spec.Config[v1alpha2.MaxInFlightMessages] = strconv.Itoa(src.Spec.Config.MaxInFlightMessages) + dst.Spec.Config[v1alpha2.MaxInFlightMessages] = strconv.Itoa(s.Spec.Config.MaxInFlightMessages) } } // setBEBBackendStatus moves the BEB-related to Backend fields of the Status in the v1alpha2. -func (src *Subscription) bebBackendStatusToV1(dst *v1alpha2.Subscription) { - src.Status.Ev2hash = dst.Status.Backend.Ev2hash - src.Status.Emshash = dst.Status.Backend.EventMeshHash - src.Status.ExternalSink = dst.Status.Backend.ExternalSink - src.Status.FailedActivation = dst.Status.Backend.FailedActivation - src.Status.APIRuleName = dst.Status.Backend.APIRuleName +func (s *Subscription) bebBackendStatusToV1(dst *v1alpha2.Subscription) { + s.Status.Ev2hash = dst.Status.Backend.Ev2hash + s.Status.Emshash = dst.Status.Backend.EventMeshHash + s.Status.ExternalSink = dst.Status.Backend.ExternalSink + s.Status.FailedActivation = dst.Status.Backend.FailedActivation + s.Status.APIRuleName = dst.Status.Backend.APIRuleName if dst.Status.Backend.EventMeshSubscriptionStatus != nil { - src.Status.EmsSubscriptionStatus = &EmsSubscriptionStatus{ + s.Status.EmsSubscriptionStatus = &EmsSubscriptionStatus{ SubscriptionStatus: dst.Status.Backend.EventMeshSubscriptionStatus.Status, SubscriptionStatusReason: dst.Status.Backend.EventMeshSubscriptionStatus.StatusReason, LastSuccessfulDelivery: dst.Status.Backend.EventMeshSubscriptionStatus.LastSuccessfulDelivery, @@ -293,21 +293,21 @@ func (src *Subscription) bebBackendStatusToV1(dst *v1alpha2.Subscription) { } // natsBackendStatusToV1 moves the NATS-related to Backend fields of the Status in the v1alpha2. -func (src *Subscription) natsBackendStatusToV1(dst *v1alpha2.Subscription) { +func (s *Subscription) natsBackendStatusToV1(dst *v1alpha2.Subscription) { if maxInFlightMessages, ok := dst.Spec.Config[v1alpha2.MaxInFlightMessages]; ok { intVal, err := strconv.Atoi(maxInFlightMessages) if err == nil { - src.Status.Config = &SubscriptionConfig{} - src.Status.Config.MaxInFlightMessages = intVal + s.Status.Config = &SubscriptionConfig{} + s.Status.Config.MaxInFlightMessages = intVal } } } // setV1CleanEvenTypes sets the clean event types to v1alpha1 Subscription Status. -func (src *Subscription) setV1CleanEvenTypes(dst *v1alpha2.Subscription) { - src.Status.InitializeCleanEventTypes() +func (s *Subscription) setV1CleanEvenTypes(dst *v1alpha2.Subscription) { + s.Status.InitializeCleanEventTypes() for _, eventType := range dst.Status.Types { - src.Status.CleanEventTypes = append(src.Status.CleanEventTypes, eventType.CleanType) + s.Status.CleanEventTypes = append(s.Status.CleanEventTypes, eventType.CleanType) } } diff --git a/api/eventing/v1alpha1/subscription_conversion_unit_test.go b/api/eventing/v1alpha1/subscription_conversion_unit_test.go index d7134938..5c52d859 100644 --- a/api/eventing/v1alpha1/subscription_conversion_unit_test.go +++ b/api/eventing/v1alpha1/subscription_conversion_unit_test.go @@ -236,6 +236,7 @@ func Test_Conversion(t *testing.T) { // Test_CleanupInV1ToV2Conversion test the cleaning from non-alphanumeric characters // and also merging of segments in event types if they exceed the limit. +// nolint:goconst // the event types used here in tests do not get more readable by extracting them to constants func Test_CleanupInV1ToV2Conversion(t *testing.T) { type TestCase struct { name string @@ -365,6 +366,7 @@ func Test_CleanupInV1ToV2Conversion(t *testing.T) { } func v1ToV2Assertions(t *testing.T, wantSub, convertedSub *v1alpha2.Subscription) { + t.Helper() assert.Equal(t, wantSub.ObjectMeta, convertedSub.ObjectMeta) // Spec @@ -377,6 +379,7 @@ func v1ToV2Assertions(t *testing.T, wantSub, convertedSub *v1alpha2.Subscription } func v2ToV1Assertions(t *testing.T, wantSub, convertedSub *v1alpha1.Subscription) { + t.Helper() assert.Equal(t, wantSub.ObjectMeta, convertedSub.ObjectMeta) // Spec diff --git a/api/eventing/v1alpha1/subscription_types.go b/api/eventing/v1alpha1/subscription_types.go index ca52fbcd..15a68310 100644 --- a/api/eventing/v1alpha1/subscription_types.go +++ b/api/eventing/v1alpha1/subscription_types.go @@ -1,3 +1,4 @@ +//nolint:godox // this package will be removed soon package v1alpha1 import ( @@ -16,8 +17,6 @@ const ( NatsBackendType BackendType = "NATS" ) -var Finalizer = GroupVersion.Group - // WebhookAuth defines the Webhook called by an active subscription in BEB. // TODO: Remove it when depreciating code of v1alpha1. type WebhookAuth struct { diff --git a/api/eventing/v1alpha1/subscription_webhook.go b/api/eventing/v1alpha1/subscription_webhook.go index e64b8778..22e69e6f 100644 --- a/api/eventing/v1alpha1/subscription_webhook.go +++ b/api/eventing/v1alpha1/subscription_webhook.go @@ -4,10 +4,8 @@ import ( kctrl "sigs.k8s.io/controller-runtime" ) -func (r *Subscription) SetupWebhookWithManager(mgr kctrl.Manager) error { +func (s *Subscription) SetupWebhookWithManager(mgr kctrl.Manager) error { return kctrl.NewWebhookManagedBy(mgr). - For(r). + For(s). Complete() } - -// TODO(user): EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! diff --git a/api/eventing/v1alpha2/condition.go b/api/eventing/v1alpha2/condition.go index 447f5a78..8103d829 100644 --- a/api/eventing/v1alpha2/condition.go +++ b/api/eventing/v1alpha2/condition.go @@ -19,8 +19,6 @@ const ( ConditionControllerReady ConditionType = "Subscription Controller Ready" ) -var allSubscriptionConditions = MakeSubscriptionConditions() - type Condition struct { // Short description of the condition. Type ConditionType `json:"type,omitempty"` @@ -81,7 +79,7 @@ func (s *SubscriptionStatus) InitializeConditions() { } func (s SubscriptionStatus) IsReady() bool { - if !ContainSameConditionTypes(allSubscriptionConditions, s.Conditions) { + if !ContainSameConditionTypes(MakeSubscriptionConditions(), s.Conditions) { return false } diff --git a/api/eventing/v1alpha2/groupversion_info.go b/api/eventing/v1alpha2/groupversion_info.go index bb2cb894..883841d9 100644 --- a/api/eventing/v1alpha2/groupversion_info.go +++ b/api/eventing/v1alpha2/groupversion_info.go @@ -1,6 +1,8 @@ // Package v1alpha2 contains API Schema definitions for the eventing v1alpha2 API group. // +kubebuilder:object:generate=true // +groupName=eventing.kyma-project.io +// +//nolint:gochecknoglobals // required for utilizing the API package v1alpha2 import ( diff --git a/api/eventing/v1alpha2/subscription_types.go b/api/eventing/v1alpha2/subscription_types.go index a5ce05b7..6adff584 100644 --- a/api/eventing/v1alpha2/subscription_types.go +++ b/api/eventing/v1alpha2/subscription_types.go @@ -14,6 +14,7 @@ import ( type TypeMatching string +//nolint:gochecknoglobals // required for consistency var Finalizer = GroupVersion.Group // Defines the desired state of the Subscription. diff --git a/api/operator/v1alpha1/eventing_types.go b/api/operator/v1alpha1/eventing_types.go index a6111468..5ad0717d 100644 --- a/api/operator/v1alpha1/eventing_types.go +++ b/api/operator/v1alpha1/eventing_types.go @@ -216,6 +216,7 @@ type Logging struct { LogLevel string `json:"logLevel,omitempty"` } +//nolint:gochecknoinits // registers Eventing CRD at startup func init() { SchemeBuilder.Register(&Eventing{}, &EventingList{}) } diff --git a/api/operator/v1alpha1/groupversion_info.go b/api/operator/v1alpha1/groupversion_info.go index 34c9e3d4..f773d907 100644 --- a/api/operator/v1alpha1/groupversion_info.go +++ b/api/operator/v1alpha1/groupversion_info.go @@ -17,6 +17,8 @@ limitations under the License. // Package v1alpha1 contains API Schema definitions for the operator v1alpha1 API group // +kubebuilder:object:generate=true // +groupName=operator.kyma-project.io +// +//nolint:gochecknoglobals // required for utilizing the API package v1alpha1 import ( diff --git a/api/operator/v1alpha1/status.go b/api/operator/v1alpha1/status.go index 814d2836..a1d34190 100644 --- a/api/operator/v1alpha1/status.go +++ b/api/operator/v1alpha1/status.go @@ -48,7 +48,7 @@ func (es *EventingStatus) UpdateConditionWebhookReady(status kmetav1.ConditionSt meta.SetStatusCondition(&es.Conditions, condition) } -func (sm *EventingStatus) UpdateConditionSubscriptionManagerReady(status kmetav1.ConditionStatus, reason ConditionReason, +func (es *EventingStatus) UpdateConditionSubscriptionManagerReady(status kmetav1.ConditionStatus, reason ConditionReason, message string, ) { condition := kmetav1.Condition{ @@ -58,7 +58,7 @@ func (sm *EventingStatus) UpdateConditionSubscriptionManagerReady(status kmetav1 Reason: string(reason), Message: message, } - meta.SetStatusCondition(&sm.Conditions, condition) + meta.SetStatusCondition(&es.Conditions, condition) } func (es *EventingStatus) UpdateConditionDeletion(status kmetav1.ConditionStatus, reason ConditionReason, @@ -85,8 +85,8 @@ func (es *EventingStatus) SetStateReady() { es.UpdateConditionPublisherProxyReady(kmetav1.ConditionTrue, ConditionReasonDeployed, ConditionPublisherProxyReadyMessage) } -func (ns *EventingStatus) SetStateWarning() { - ns.State = StateWarning +func (es *EventingStatus) SetStateWarning() { + es.State = StateWarning } func (es *EventingStatus) SetNATSAvailableConditionToTrue() { diff --git a/cmd/main.go b/cmd/main.go index 925a7ce7..ddc91564 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -54,20 +54,11 @@ import ( "github.com/kyma-project/eventing-manager/pkg/subscriptionmanager/jetstream" ) -var ( - scheme = runtime.NewScheme() - setupLog = kctrl.Log.WithName("setup") -) - -func init() { +func registerSchemas(scheme *runtime.Scheme) { kutilruntime.Must(kkubernetesscheme.AddToScheme(scheme)) - kutilruntime.Must(operatorv1alpha1.AddToScheme(scheme)) - kutilruntime.Must(apigatewayv1beta1.AddToScheme(scheme)) - kutilruntime.Must(kapiextensionsv1.AddToScheme(scheme)) - kutilruntime.Must(jetstream.AddToScheme(scheme)) kutilruntime.Must(jetstream.AddV1Alpha2ToScheme(scheme)) kutilruntime.Must(eventingv1alpha1.AddToScheme(scheme)) @@ -75,12 +66,25 @@ func init() { //+kubebuilder:scaffold:scheme } -const defaultMetricsPort = 9443 +const ( + defaultMetricsPort = 9443 + webhookServerPort = 9443 +) + +func syncLogger(logger *logger.Logger) { + if err := logger.WithContext().Sync(); err != nil { + log.Printf("Failed to flush logger, error: %v", err) + } +} func main() { //nolint:funlen // main function needs to initialize many object + scheme := runtime.NewScheme() + registerSchemas(scheme) + var enableLeaderElection bool var leaderElectionID string var metricsPort int + setupLog := kctrl.Log.WithName("setup") flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. "+ "Enabling this will ensure there is only one active controller manager.") @@ -97,12 +101,6 @@ func main() { //nolint:funlen // main function needs to initialize many object if err != nil { log.Fatalf("Failed to initialize logger, error: %v", err) } - defer func() { - if err = ctrLogger.WithContext().Sync(); err != nil { - log.Printf("Failed to flush logger, error: %v", err) - } - }() - // Set controller core logger. kctrl.SetLogger(zapr.NewLogger(ctrLogger.WithContext().Desugar())) @@ -114,7 +112,7 @@ func main() { //nolint:funlen // main function needs to initialize many object HealthProbeBindAddress: opts.ProbeAddr, LeaderElection: enableLeaderElection, LeaderElectionID: leaderElectionID, - WebhookServer: webhook.NewServer(webhook.Options{Port: 9443}), + WebhookServer: webhook.NewServer(webhook.Options{Port: webhookServerPort}), Cache: cache.Options{SyncPeriod: &opts.ReconcilePeriod}, Metrics: server.Options{BindAddress: opts.MetricsAddr}, NewCache: controllercache.New, @@ -122,6 +120,7 @@ func main() { //nolint:funlen // main function needs to initialize many object }) if err != nil { setupLog.Error(err, "unable to start manager") + syncLogger(ctrLogger) os.Exit(1) } @@ -129,6 +128,7 @@ func main() { //nolint:funlen // main function needs to initialize many object k8sClient := mgr.GetClient() dynamicClient, err := dynamic.NewForConfig(k8sRestCfg) if err != nil { + syncLogger(ctrLogger) panic(err.Error()) } @@ -136,6 +136,8 @@ func main() { //nolint:funlen // main function needs to initialize many object apiClientSet, err := kapixclientset.NewForConfig(mgr.GetConfig()) if err != nil { setupLog.Error(err, "failed to create new k8s clientset") + syncLogger(ctrLogger) + os.Exit(1) } @@ -191,11 +193,13 @@ func main() { //nolint:funlen // main function needs to initialize many object // Setup webhooks. if err = (&eventingv1alpha1.Subscription{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "Failed to create webhook") + syncLogger(ctrLogger) os.Exit(1) } if err = (&eventingv1alpha2.Subscription{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "Failed to create webhook") + syncLogger(ctrLogger) os.Exit(1) } @@ -203,21 +207,26 @@ func main() { //nolint:funlen // main function needs to initialize many object err = peerauthentication.SyncPeerAuthentications(ctx, kubeClient, ctrLogger.WithContext().Named("main")) if err != nil { setupLog.Error(err, "unable to sync PeerAuthentication") + syncLogger(ctrLogger) os.Exit(1) } if err = mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { setupLog.Error(err, "unable to set up health check") + syncLogger(ctrLogger) os.Exit(1) } if err = mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { setupLog.Error(err, "unable to set up ready check") + syncLogger(ctrLogger) os.Exit(1) } setupLog.Info("starting manager") if err = mgr.Start(kctrl.SetupSignalHandler()); err != nil { setupLog.Error(err, "problem running manager") + syncLogger(ctrLogger) os.Exit(1) } + syncLogger(ctrLogger) } diff --git a/hack/e2e/common/eventing/publisher.go b/hack/e2e/common/eventing/publisher.go index f279076e..67c45c5b 100644 --- a/hack/e2e/common/eventing/publisher.go +++ b/hack/e2e/common/eventing/publisher.go @@ -64,9 +64,10 @@ func (p *Publisher) SendCloudEventWithRetries(event *cloudevents.Event, encoding } func (p *Publisher) SendLegacyEvent(source, eventType, payload string) error { + ctx := context.Background() url := p.LegacyPublishEndpoint(source) - req, err := http.NewRequest(http.MethodPost, url, bytes.NewBufferString(payload)) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewBufferString(payload)) if err != nil { err = errors.Wrap(err, "Failed to create HTTP request for sending legacy event") p.logger.Debug(err.Error()) @@ -142,6 +143,8 @@ func (p *Publisher) SendCloudEvent(event *cloudevents.Event, encoding binding.En p.PublishEndpoint(), encoding.String(), ce.ID(), ce.Source(), ce.Type(), ce.Data())) result := p.clientCE.Send(ctx, ce) + + //nolint:errorlint // this error is used in logs switch { case cloudevents.IsUndelivered(result): { diff --git a/hack/e2e/common/eventing/sinkclient.go b/hack/e2e/common/eventing/sinkclient.go index 78ed63d0..57a5088c 100644 --- a/hack/e2e/common/eventing/sinkclient.go +++ b/hack/e2e/common/eventing/sinkclient.go @@ -2,6 +2,7 @@ package eventing import ( "bytes" + "context" "encoding/json" "fmt" "io" @@ -40,25 +41,26 @@ func NewSinkClient(clientHTTP *http.Client, sinkURL string, logger *zap.Logger) } } -func (sc *SinkClient) EventsEndpoint(eventId string) string { - return fmt.Sprintf(EventsEndpointFormat, sc.sinkURL, eventId) +func (sc *SinkClient) EventsEndpoint(id string) string { + return fmt.Sprintf(EventsEndpointFormat, sc.sinkURL, id) } -func (sc *SinkClient) GetEventFromSinkWithRetries(eventId string, attempts int, interval time.Duration) (*SinkEvent, error) { +func (sc *SinkClient) GetEventFromSinkWithRetries(id string, attempts int, interval time.Duration) (*SinkEvent, error) { var gotEvent *SinkEvent err := common.Retry(attempts, interval, func() error { var err1 error - gotEvent, err1 = sc.GetEventFromSink(eventId) + gotEvent, err1 = sc.GetEventFromSink(id) return err1 }) return gotEvent, err } -func (sc *SinkClient) GetEventFromSink(eventId string) (*SinkEvent, error) { - url := sc.EventsEndpoint(eventId) - sc.logger.Debug(fmt.Sprintf("Fetching event with ID: %s from the sink URL: %s", eventId, url)) +func (sc *SinkClient) GetEventFromSink(id string) (*SinkEvent, error) { + url := sc.EventsEndpoint(id) + sc.logger.Debug(fmt.Sprintf("Fetching event with ID: %s from the sink URL: %s", id, url)) - req, err := http.NewRequest(http.MethodGet, url, bytes.NewBuffer([]byte{})) + ctx := context.Background() + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, bytes.NewBuffer([]byte{})) if err != nil { err = errors.Wrap(err, "Failed to create HTTP request for fetching event from sink") sc.logger.Debug(err.Error()) @@ -89,7 +91,7 @@ func (sc *SinkClient) GetEventFromSink(eventId string) (*SinkEvent, error) { // if not success, then return error. if !Is2XX(resp.StatusCode) { - err = errors.New(fmt.Sprintf("Failed to fetch eventID:[%s] response:[%d] body:[%s]", eventId, + err = errors.New(fmt.Sprintf("Failed to fetch eventID:[%s] response:[%d] body:[%s]", id, resp.StatusCode, string(respBody))) sc.logger.Debug(err.Error()) return nil, err diff --git a/hack/e2e/common/eventing/utils.go b/hack/e2e/common/eventing/utils.go index 6486eb7e..06b3e559 100644 --- a/hack/e2e/common/eventing/utils.go +++ b/hack/e2e/common/eventing/utils.go @@ -24,8 +24,8 @@ func LegacyEventData(source, eventType string) string { return `{\"` + keyApp + `\":\"` + source + `\",\"` + keyMode + `\":\"legacy\",\"` + keyType + `\":\"` + eventType + `\"}` } -func LegacyEventPayload(eventId, eventVersion, eventType, data string) string { - return `{"data":"` + data + `","event-id":"` + eventId + `","event-type":"` + eventType + `","event-time":"2020-04-02T21:37:00Z","event-type-version":"` + eventVersion + `"}` +func LegacyEventPayload(id, eventVersion, eventType, data string) string { + return `{"data":"` + data + `","event-id":"` + id + `","event-type":"` + eventType + `","event-time":"2020-04-02T21:37:00Z","event-type-version":"` + eventVersion + `"}` } func CloudEventMode(encoding binding.Encoding) string { diff --git a/hack/e2e/common/fixtures/fixtures.go b/hack/e2e/common/fixtures/fixtures.go index 90bd6484..371d4670 100644 --- a/hack/e2e/common/fixtures/fixtures.go +++ b/hack/e2e/common/fixtures/fixtures.go @@ -1,3 +1,4 @@ +// nolint: gomnd // used in tests package fixtures import ( diff --git a/hack/e2e/common/http/http.go b/hack/e2e/common/http/http.go index 30d61ad5..b6b80dc5 100644 --- a/hack/e2e/common/http/http.go +++ b/hack/e2e/common/http/http.go @@ -8,7 +8,7 @@ import ( ceclient "github.com/cloudevents/sdk-go/v2/client" ) -func NewHttpClient(transport *http.Transport) *http.Client { +func NewHTTPClient(transport *http.Transport) *http.Client { return &http.Client{Transport: transport} } diff --git a/hack/e2e/common/testenvironment/test_environment.go b/hack/e2e/common/testenvironment/test_environment.go index 6edf86c6..b6b9a6be 100644 --- a/hack/e2e/common/testenvironment/test_environment.go +++ b/hack/e2e/common/testenvironment/test_environment.go @@ -110,7 +110,7 @@ func (te *TestEnvironment) InitEventPublisherClient() error { maxIdleConnsPerHost := 10 idleConnTimeout := 1 * time.Minute t := http.NewTransport(maxIdleConns, maxConnsPerHost, maxIdleConnsPerHost, idleConnTimeout) - clientHTTP := http.NewHttpClient(t.Clone()) + clientHTTP := http.NewHTTPClient(t.Clone()) clientCE, err := http.NewCloudEventsClient(t.Clone()) if err != nil { return err @@ -125,7 +125,7 @@ func (te *TestEnvironment) InitSinkClient() { maxIdleConnsPerHost := 10 idleConnTimeout := 1 * time.Minute t := http.NewTransport(maxIdleConns, maxConnsPerHost, maxIdleConnsPerHost, idleConnTimeout) - clientHTTP := http.NewHttpClient(t.Clone()) + clientHTTP := http.NewHTTPClient(t.Clone()) te.SinkClient = eventing.NewSinkClient(clientHTTP, te.TestConfigs.SinkPortForwardedURL, te.Logger) } @@ -376,11 +376,11 @@ func (te *TestEnvironment) DeleteSubscriptionFromK8s(name, namespace string) err func (te *TestEnvironment) TestDeliveryOfLegacyEvent(eventSource, eventType string, subCRVersion fixtures.SubscriptionCRVersion) error { // define the event - var eventId, legacyEventSource, legacyEventType, payload string + var id, legacyEventSource, legacyEventType, payload string if subCRVersion == fixtures.V1Alpha1SubscriptionCRVersion { - eventId, legacyEventSource, legacyEventType, payload = eventing.NewLegacyEventForV1Alpha1(eventType, te.TestConfigs.EventTypePrefix) + id, legacyEventSource, legacyEventType, payload = eventing.NewLegacyEventForV1Alpha1(eventType, te.TestConfigs.EventTypePrefix) } else { - eventId, legacyEventSource, legacyEventType, payload = eventing.NewLegacyEvent(eventSource, eventType) + id, legacyEventSource, legacyEventType, payload = eventing.NewLegacyEvent(eventSource, eventType) } // publish the event @@ -390,8 +390,8 @@ func (te *TestEnvironment) TestDeliveryOfLegacyEvent(eventSource, eventType stri } // verify if the event was received by the sink. - te.Logger.Debug(fmt.Sprintf("Verifying if LegacyEvent (ID: %s) was received by the sink", eventId)) - return te.VerifyLegacyEventReceivedBySink(eventId, eventType, eventSource, payload) + te.Logger.Debug(fmt.Sprintf("Verifying if LegacyEvent (ID: %s) was received by the sink", id)) + return te.VerifyLegacyEventReceivedBySink(id, eventType, eventSource, payload) } func (te *TestEnvironment) TestDeliveryOfCloudEvent(eventSource, eventType string, encoding binding.Encoding) error { @@ -412,7 +412,7 @@ func (te *TestEnvironment) TestDeliveryOfCloudEvent(eventSource, eventType strin return te.VerifyCloudEventReceivedBySink(*ceEvent) } -func (te *TestEnvironment) VerifyLegacyEventReceivedBySink(eventId, eventType, eventSource, payload string) error { +func (te *TestEnvironment) VerifyLegacyEventReceivedBySink(id, eventType, eventSource, payload string) error { // publisher-proxy converts LegacyEvent to CloudEvent, so the sink should have received a CloudEvent. // extract data from payload of legacy event. result := make(map[string]interface{}) @@ -423,7 +423,7 @@ func (te *TestEnvironment) VerifyLegacyEventReceivedBySink(eventId, eventType, e // define the expected CloudEvent. expectedCEEvent := cloudevents.NewEvent() - expectedCEEvent.SetID(eventId) + expectedCEEvent.SetID(id) expectedCEEvent.SetType(eventType) expectedCEEvent.SetSource(eventSource) if err := expectedCEEvent.SetData(cloudevents.ApplicationJSON, data); err != nil { diff --git a/internal/controller/cache/cache.go b/internal/controller/cache/cache.go index 712db0d7..e201aa5e 100644 --- a/internal/controller/cache/cache.go +++ b/internal/controller/cache/cache.go @@ -19,7 +19,7 @@ func New(config *rest.Config, options cache.Options) (cache.Cache, error) { // applySelectors applies label selectors to runtime objects created by the EventingManager. func applySelectors(options cache.Options) cache.Options { - // TODO(marcobebway) filter by label "app.kubernetes.io/created-by=eventing-manager" when it is released + //nolint:godox // TODO(marcobebway) filter by label "app.kubernetes.io/created-by=eventing-manager" when it is released instanceEventing := fromLabelSelector(label.SelectorInstanceEventing()) options.ByObject = map[client.Object]cache.ByObject{ &kappsv1.Deployment{}: instanceEventing, diff --git a/internal/controller/errors/skip.go b/internal/controller/errors/skip.go index f24bba40..ff14b0a1 100644 --- a/internal/controller/errors/skip.go +++ b/internal/controller/errors/skip.go @@ -12,7 +12,7 @@ func IsSkippable(err error) bool { if err == nil { return true } - _, ok := err.(skippable) + _, ok := err.(skippable) //nolint:errorlint // here we do not want to check the chain return ok } diff --git a/internal/controller/eventing/subscription/eventmesh/reconciler.go b/internal/controller/eventing/subscription/eventmesh/reconciler.go index 2ad1d7cd..e6c29407 100644 --- a/internal/controller/eventing/subscription/eventmesh/reconciler.go +++ b/internal/controller/eventing/subscription/eventmesh/reconciler.go @@ -136,7 +136,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req kctrl.Request) (kctrl.Re // sync Finalizers, ensure the finalizer is set if err := r.syncFinalizer(sub, log); err != nil { if updateErr := r.updateSubscription(ctx, sub, log); updateErr != nil { - return kctrl.Result{}, xerrors.Errorf(updateErr.Error()+": %v", err) + return kctrl.Result{}, fmt.Errorf("%w: %w", updateErr, err) } return kctrl.Result{}, xerrors.Errorf("failed to sync finalizer: %v", err) } @@ -147,7 +147,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req kctrl.Request) (kctrl.Re sub.Status.SetConditionAPIRuleStatus(err) if !controllererrors.IsSkippable(err) { if updateErr := r.updateSubscription(ctx, sub, log); updateErr != nil { - return kctrl.Result{}, xerrors.Errorf(updateErr.Error()+": %v", err) + return kctrl.Result{}, fmt.Errorf("%w: %w", updateErr, err) } return kctrl.Result{}, err } @@ -156,7 +156,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req kctrl.Request) (kctrl.Re ready, err := r.syncEventMeshSubscription(sub, apiRule, log) if err != nil { if updateErr := r.updateSubscription(ctx, sub, log); updateErr != nil { - return kctrl.Result{}, xerrors.Errorf(updateErr.Error()+": %v", err) + return kctrl.Result{}, fmt.Errorf("%w: %w", updateErr, err) } return kctrl.Result{}, err } diff --git a/internal/controller/eventing/subscription/eventmesh/reconciler_internal_integration_test.go b/internal/controller/eventing/subscription/eventmesh/reconciler_internal_integration_test.go index 533e3285..f1ba33c2 100644 --- a/internal/controller/eventing/subscription/eventmesh/reconciler_internal_integration_test.go +++ b/internal/controller/eventing/subscription/eventmesh/reconciler_internal_integration_test.go @@ -1371,6 +1371,7 @@ type testEnvironment struct { // setupTestEnvironment is a testEnvironment constructor. func setupTestEnvironment(t *testing.T, objs ...client.Object) *testEnvironment { + t.Helper() mockedBackend := &mocks.Backend{} fakeClient := createFakeClientBuilder(t).WithObjects(objs...).WithStatusSubresource(objs...).Build() recorder := &record.FakeRecorder{} @@ -1396,6 +1397,7 @@ func setupTestEnvironment(t *testing.T, objs ...client.Object) *testEnvironment } func createFakeClientBuilder(t *testing.T) *fake.ClientBuilder { + t.Helper() err := eventingv1alpha2.AddToScheme(scheme.Scheme) require.NoError(t, err) err = apigatewayv1beta1.AddToScheme(scheme.Scheme) diff --git a/internal/controller/eventing/subscription/eventmesh/test/assertions.go b/internal/controller/eventing/subscription/eventmesh/test/assertions.go index e633c10b..09e48dfe 100644 --- a/internal/controller/eventing/subscription/eventmesh/test/assertions.go +++ b/internal/controller/eventing/subscription/eventmesh/test/assertions.go @@ -33,7 +33,7 @@ func getSubscriptionAssert(ctx context.Context, g *gomega.GomegaWithT, func getAPIRuleForASvcAssert(ctx context.Context, g *gomega.GomegaWithT, svc *kcorev1.Service) gomega.AsyncAssertion { return g.Eventually(func() apigatewayv1beta1.APIRule { apiRules, err := getAPIRulesList(ctx, svc) - g.Expect(err).Should(gomega.BeNil()) + g.Expect(err).ShouldNot(gomega.HaveOccurred()) return filterAPIRulesForASvc(apiRules, svc) }, smallTimeOut, smallPollingInterval) } diff --git a/internal/controller/eventing/subscription/eventmesh/test/utils.go b/internal/controller/eventing/subscription/eventmesh/test/utils.go index f5d829bf..4038b79d 100644 --- a/internal/controller/eventing/subscription/eventmesh/test/utils.go +++ b/internal/controller/eventing/subscription/eventmesh/test/utils.go @@ -115,20 +115,8 @@ func setupSuite() error { } // +kubebuilder:scaffold:scheme - // start eventMesh manager instance - syncPeriod := syncPeriodSeconds * time.Second - webhookInstallOptions := &emTestEnsemble.testEnv.WebhookInstallOptions - k8sManager, err := kctrl.NewManager(cfg, kctrl.Options{ - Cache: cache.Options{SyncPeriod: &syncPeriod}, - HealthProbeBindAddress: "0", // disable - Scheme: scheme.Scheme, - Metrics: server.Options{BindAddress: "0"}, // disable - WebhookServer: webhook.NewServer(webhook.Options{ - Port: webhookInstallOptions.LocalServingPort, - Host: webhookInstallOptions.LocalServingHost, - CertDir: webhookInstallOptions.LocalServingCertDir, - }), - }) + // setup eventMesh manager instance + k8sManager, webhookInstallOptions, err := setupManager(cfg) if err != nil { return err } @@ -140,21 +128,18 @@ func setupSuite() error { // setup eventMesh reconciler recorder := k8sManager.GetEventRecorderFor("eventing-controller") sinkValidator := sink.NewValidator(k8sManager.GetClient(), recorder) - credentials := &backendeventmesh.OAuth2ClientCredentials{ - ClientID: "foo-client-id", - ClientSecret: "foo-client-secret", - TokenURL: "foo-token-url", - CertsURL: certsURL, - } emTestEnsemble.envConfig = getEnvConfig() + + eventMesh, credentials := setupEventMesh(defaultLogger) + col := metrics.NewCollector() testReconciler := subscriptioncontrollereventmesh.NewReconciler( k8sManager.GetClient(), defaultLogger, recorder, - getEnvConfig(), + emTestEnsemble.envConfig, cleaner.NewEventMeshCleaner(defaultLogger), - backendeventmesh.NewEventMesh(credentials, emTestEnsemble.nameMapper, defaultLogger), + eventMesh, credentials, emTestEnsemble.nameMapper, sinkValidator, @@ -181,6 +166,38 @@ func setupSuite() error { return startAndWaitForWebhookServer(k8sManager, webhookInstallOptions) } +func setupManager(cfg *rest.Config) (manager.Manager, *envtest.WebhookInstallOptions, error) { + syncPeriod := syncPeriodSeconds * time.Second + webhookInstallOptions := &emTestEnsemble.testEnv.WebhookInstallOptions + opts := kctrl.Options{ + Cache: cache.Options{SyncPeriod: &syncPeriod}, + HealthProbeBindAddress: "0", // disable + Scheme: scheme.Scheme, + Metrics: server.Options{BindAddress: "0"}, // disable + WebhookServer: webhook.NewServer(webhook.Options{ + Port: webhookInstallOptions.LocalServingPort, + Host: webhookInstallOptions.LocalServingHost, + CertDir: webhookInstallOptions.LocalServingCertDir, + }), + } + k8sManager, err := kctrl.NewManager(cfg, opts) + if err != nil { + return nil, nil, err + } + return k8sManager, webhookInstallOptions, nil +} + +func setupEventMesh(defaultLogger *logger.Logger) (*backendeventmesh.EventMesh, *backendeventmesh.OAuth2ClientCredentials) { + credentials := &backendeventmesh.OAuth2ClientCredentials{ + ClientID: "foo-client-id", + ClientSecret: "foo-client-secret", + TokenURL: "foo-token-url", + CertsURL: certsURL, + } + eventMesh := backendeventmesh.NewEventMesh(credentials, emTestEnsemble.nameMapper, defaultLogger) + return eventMesh, credentials +} + func startAndWaitForWebhookServer(k8sManager manager.Manager, webhookInstallOpts *envtest.WebhookInstallOptions) error { if err := (&eventingv1alpha2.Subscription{}).SetupWebhookWithManager(k8sManager); err != nil { return err @@ -279,6 +296,7 @@ func getTestNamespace() string { } func ensureNamespaceCreated(ctx context.Context, t *testing.T, namespace string) { + t.Helper() if namespace == "default" { return } @@ -304,18 +322,22 @@ func fixtureNamespace(name string) *kcorev1.Namespace { } func ensureK8sResourceCreated(ctx context.Context, t *testing.T, obj client.Object) { + t.Helper() require.NoError(t, emTestEnsemble.k8sClient.Create(ctx, obj)) } func ensureK8sResourceNotCreated(ctx context.Context, t *testing.T, obj client.Object, err error) { + t.Helper() require.Equal(t, emTestEnsemble.k8sClient.Create(ctx, obj), err) } func ensureK8sResourceDeleted(ctx context.Context, t *testing.T, obj client.Object) { + t.Helper() require.NoError(t, emTestEnsemble.k8sClient.Delete(ctx, obj)) } func ensureK8sSubscriptionUpdated(ctx context.Context, t *testing.T, subscription *eventingv1alpha2.Subscription) { + t.Helper() require.Eventually(t, func() bool { latestSubscription := &eventingv1alpha2.Subscription{} lookupKey := types.NamespacedName{ @@ -333,6 +355,7 @@ func ensureK8sSubscriptionUpdated(ctx context.Context, t *testing.T, subscriptio // ensureAPIRuleStatusUpdatedWithStatusReady updates the status fof the APIRule (mocking APIGateway controller). func ensureAPIRuleStatusUpdatedWithStatusReady(ctx context.Context, t *testing.T, apiRule *apigatewayv1beta1.APIRule) { + t.Helper() require.Eventually(t, func() bool { fetchedAPIRule, err := getAPIRule(ctx, apiRule) if err != nil { @@ -351,6 +374,7 @@ func ensureAPIRuleStatusUpdatedWithStatusReady(ctx context.Context, t *testing.T // ensureAPIRuleNotFound ensures that a APIRule does not exists (or deleted). func ensureAPIRuleNotFound(ctx context.Context, t *testing.T, apiRule *apigatewayv1beta1.APIRule) { + t.Helper() require.Eventually(t, func() bool { apiRuleKey := client.ObjectKey{ Namespace: apiRule.Namespace, @@ -439,6 +463,7 @@ func getEventMeshKeyForMock(name string) string { // ensureK8sEventReceived checks if a certain event have triggered for the given namespace. func ensureK8sEventReceived(t *testing.T, event kcorev1.Event, namespace string) { + t.Helper() ctx := context.TODO() require.Eventually(t, func() bool { // get all events from k8s for namespace diff --git a/internal/controller/eventing/subscription/eventmesh/testwebhookauth/utils.go b/internal/controller/eventing/subscription/eventmesh/testwebhookauth/utils.go index cd637a19..ed78b1e2 100644 --- a/internal/controller/eventing/subscription/eventmesh/testwebhookauth/utils.go +++ b/internal/controller/eventing/subscription/eventmesh/testwebhookauth/utils.go @@ -117,19 +117,7 @@ func setupSuite() error { // +kubebuilder:scaffold:scheme // start eventMesh manager instance - syncPeriod := syncPeriodSeconds * time.Second - webhookInstallOptions := &emTestEnsemble.testEnv.WebhookInstallOptions - k8sManager, err := kctrl.NewManager(cfg, kctrl.Options{ - Scheme: scheme.Scheme, - Cache: cache.Options{SyncPeriod: &syncPeriod}, - Metrics: server.Options{BindAddress: "0"}, // disable - HealthProbeBindAddress: "0", // disable - WebhookServer: webhook.NewServer(webhook.Options{ - Host: webhookInstallOptions.LocalServingHost, - Port: webhookInstallOptions.LocalServingPort, - CertDir: webhookInstallOptions.LocalServingCertDir, - }), - }) + k8sManager, webhookInstallOptions, err := setupManager(cfg) if err != nil { return err } @@ -177,6 +165,23 @@ func setupSuite() error { return startAndWaitForWebhookServer(k8sManager, webhookInstallOptions) } +func setupManager(cfg *rest.Config) (manager.Manager, *envtest.WebhookInstallOptions, error) { + syncPeriod := syncPeriodSeconds * time.Second + webhookInstallOptions := &emTestEnsemble.testEnv.WebhookInstallOptions + k8sManager, err := kctrl.NewManager(cfg, kctrl.Options{ + Scheme: scheme.Scheme, + Cache: cache.Options{SyncPeriod: &syncPeriod}, + Metrics: server.Options{BindAddress: "0"}, // disable + HealthProbeBindAddress: "0", // disable + WebhookServer: webhook.NewServer(webhook.Options{ + Host: webhookInstallOptions.LocalServingHost, + Port: webhookInstallOptions.LocalServingPort, + CertDir: webhookInstallOptions.LocalServingCertDir, + }), + }) + return k8sManager, webhookInstallOptions, err +} + func startAndWaitForWebhookServer(manager manager.Manager, installOpts *envtest.WebhookInstallOptions) error { if err := (&eventingv1alpha2.Subscription{}).SetupWebhookWithManager(manager); err != nil { return err @@ -265,6 +270,7 @@ func getTestNamespace() string { } func ensureNamespaceCreated(ctx context.Context, t *testing.T, namespace string) { + t.Helper() if namespace == "default" { return } @@ -290,10 +296,12 @@ func fixtureNamespace(name string) *kcorev1.Namespace { } func ensureK8sResourceCreated(ctx context.Context, t *testing.T, obj client.Object) { + t.Helper() require.NoError(t, emTestEnsemble.k8sClient.Create(ctx, obj)) } func ensureK8sSubscriptionUpdated(ctx context.Context, t *testing.T, subscription *eventingv1alpha2.Subscription) { + t.Helper() require.Eventually(t, func() bool { latestSubscription := &eventingv1alpha2.Subscription{} lookupKey := types.NamespacedName{ @@ -311,6 +319,7 @@ func ensureK8sSubscriptionUpdated(ctx context.Context, t *testing.T, subscriptio // ensureAPIRuleStatusUpdatedWithStatusReady updates the status fof the APIRule (mocking APIGateway controller). func ensureAPIRuleStatusUpdatedWithStatusReady(ctx context.Context, t *testing.T, apiRule *apigatewayv1beta1.APIRule) { + t.Helper() require.Eventually(t, func() bool { fetchedAPIRule, err := getAPIRule(ctx, apiRule) if err != nil { diff --git a/internal/controller/eventing/subscription/eventmesh/utils.go b/internal/controller/eventing/subscription/eventmesh/utils.go index a8265050..479cdfe0 100644 --- a/internal/controller/eventing/subscription/eventmesh/utils.go +++ b/internal/controller/eventing/subscription/eventmesh/utils.go @@ -56,7 +56,8 @@ func removeFinalizer(sub *eventingv1alpha2.Subscription) { // getSvcNsAndName returns namespace and name of the svc from the URL. func getSvcNsAndName(url string) (string, string, error) { parts := strings.Split(url, ".") - if len(parts) < 2 { + const minSegments = 2 + if len(parts) < minSegments { return "", "", fmt.Errorf("%w url: %s", ErrInvalidSink, url) } return parts[1], parts[0], nil diff --git a/internal/controller/eventing/subscription/jetstream/reconciler_internal_unit_test.go b/internal/controller/eventing/subscription/jetstream/reconciler_internal_unit_test.go index bb1a2108..9d2e839b 100644 --- a/internal/controller/eventing/subscription/jetstream/reconciler_internal_unit_test.go +++ b/internal/controller/eventing/subscription/jetstream/reconciler_internal_unit_test.go @@ -731,6 +731,7 @@ type TestEnvironment struct { // setupTestEnvironment is a TestEnvironment constructor. func setupTestEnvironment(t *testing.T, objs ...client.Object) *TestEnvironment { + t.Helper() mockedBackend := &backendjetstreammocks.Backend{} fakeClient := createFakeClientBuilder(t).WithObjects(objs...).WithStatusSubresource(objs...).Build() recorder := &record.FakeRecorder{} @@ -763,12 +764,14 @@ func setupTestEnvironment(t *testing.T, objs ...client.Object) *TestEnvironment } func createFakeClientBuilder(t *testing.T) *fake.ClientBuilder { + t.Helper() err := eventingv1alpha2.AddToScheme(scheme.Scheme) require.NoError(t, err) return fake.NewClientBuilder().WithScheme(scheme.Scheme) } func ensureFinalizerMatch(t *testing.T, subscription *eventingv1alpha2.Subscription, wantFinalizers []string) { + t.Helper() if len(wantFinalizers) == 0 { require.Empty(t, subscription.ObjectMeta.Finalizers) } else { @@ -785,9 +788,8 @@ func fetchTestSubscription(ctx context.Context, r *Reconciler) (eventingv1alpha2 return fetchedSub, err } -func ensureSubscriptionMatchesConditionsAndStatus(t *testing.T, - subscription eventingv1alpha2.Subscription, wantConditions []eventingv1alpha2.Condition, wantStatus bool, -) { +func ensureSubscriptionMatchesConditionsAndStatus(t *testing.T, subscription eventingv1alpha2.Subscription, wantConditions []eventingv1alpha2.Condition, wantStatus bool) { + t.Helper() require.Equal(t, len(wantConditions), len(subscription.Status.Conditions)) comparisonResult := eventingv1alpha2.ConditionsEquals(wantConditions, subscription.Status.Conditions) require.True(t, comparisonResult) diff --git a/internal/controller/eventing/subscription/jetstream/test_utils_test.go b/internal/controller/eventing/subscription/jetstream/test_utils_test.go index 772628f2..64d20e62 100644 --- a/internal/controller/eventing/subscription/jetstream/test_utils_test.go +++ b/internal/controller/eventing/subscription/jetstream/test_utils_test.go @@ -331,9 +331,8 @@ func NewSubscription(ens *Ensemble, return subscription } -func CreateSubscription(t *testing.T, ens *Ensemble, - subscriptionOpts ...eventingtesting.SubscriptionOpt, -) *eventingv1alpha2.Subscription { +func CreateSubscription(t *testing.T, ens *Ensemble, subscriptionOpts ...eventingtesting.SubscriptionOpt) *eventingv1alpha2.Subscription { + t.Helper() subscription := NewSubscription(ens, subscriptionOpts...) EnsureNamespaceCreatedForSub(t, ens, subscription) require.NoError(t, ensureSubscriptionCreated(ens, subscription)) @@ -471,6 +470,7 @@ func createSubscriberSvcInK8s(ens *Ensemble) error { // EnsureNamespaceCreatedForSub creates the namespace for subscription if it does not exist. func EnsureNamespaceCreatedForSub(t *testing.T, ens *Ensemble, subscription *eventingv1alpha2.Subscription) { + t.Helper() // create subscription on cluster if subscription.Namespace != "default " { // create testing namespace @@ -493,6 +493,7 @@ func ensureSubscriptionCreated(ens *Ensemble, subscription *eventingv1alpha2.Sub // EnsureK8sResourceNotCreated ensures that the obj creation in K8s fails. func EnsureK8sResourceNotCreated(t *testing.T, ens *Ensemble, obj client.Object, err error) { + t.Helper() require.Equal(t, ens.K8sClient.Create(context.Background(), obj), err) } diff --git a/internal/controller/operator/eventing/controller.go b/internal/controller/operator/eventing/controller.go index 87773fdb..505a0916 100644 --- a/internal/controller/operator/eventing/controller.go +++ b/internal/controller/operator/eventing/controller.go @@ -337,7 +337,7 @@ func (r *Reconciler) startNATSCRWatch(eventing *operatorv1alpha1.Eventing) error } if !found { - natsWatcher = watcher.NewResourceWatcher(r.dynamicClient, k8s.NatsGVK, eventing.Namespace) + natsWatcher = watcher.NewResourceWatcher(r.dynamicClient, k8s.NatsGVK(), eventing.Namespace) r.natsWatchers[eventing.Namespace] = natsWatcher } @@ -456,7 +456,6 @@ func (r *Reconciler) handleEventingReconcile(ctx context.Context, r.InitStateProcessing(eventing) if eventing.Spec.Backend == nil { return kctrl.Result{Requeue: true}, r.syncStatusForEmptyBackend(ctx, - operatorv1alpha1.ConditionReasonBackendNotSpecified, operatorv1alpha1.ConditionBackendNotSpecifiedMessage, eventing, log) } @@ -534,7 +533,7 @@ func (r *Reconciler) handleBackendSwitching(ctx context.Context, func (r *Reconciler) reconcileNATSBackend(ctx context.Context, eventing *operatorv1alpha1.Eventing, log *zap.SugaredLogger) (kctrl.Result, error) { // retrieves the NATS CRD - _, err := r.kubeClient.GetCRD(ctx, k8s.NatsGVK.GroupResource().String()) + _, err := r.kubeClient.GetCRD(ctx, k8s.NatsGVK().GroupResource().String()) if err != nil { if kerrors.IsNotFound(err) { // delete the publisher proxy resources, because the publisher deployment will go @@ -545,7 +544,7 @@ func (r *Reconciler) reconcileNATSBackend(ctx context.Context, eventing *operato return kctrl.Result{}, delErr } // update the Eventing CR status. - notFoundErr := fmt.Errorf("%w: %v", ErrNatsModuleMissing, err) + notFoundErr := fmt.Errorf("%w: %w", ErrNatsModuleMissing, err) return kctrl.Result{}, r.syncStatusWithNATSState(ctx, operatorv1alpha1.StateWarning, eventing, notFoundErr, log) } @@ -571,7 +570,7 @@ func (r *Reconciler) reconcileNATSBackend(ctx context.Context, eventing *operato } // start NATS subscription manager - if err := r.reconcileNATSSubManager(ctx, eventing, log); err != nil { + if err := r.reconcileNATSSubManager(eventing, log); err != nil { return kctrl.Result{}, r.syncStatusWithNATSErr(ctx, eventing, err, log) } @@ -638,7 +637,7 @@ func (r *Reconciler) reconcileEventMeshBackend(ctx context.Context, eventing *op } // Start the EventMesh subscription controller - err = r.reconcileEventMeshSubManager(ctx, eventing, eventMeshSecret, log) + err = r.reconcileEventMeshSubManager(ctx, eventing, eventMeshSecret) if err != nil { return kctrl.Result{}, r.syncStatusWithSubscriptionManagerErr(ctx, eventing, err, log) } diff --git a/internal/controller/operator/eventing/eventmesh.go b/internal/controller/operator/eventing/eventmesh.go index c2fec08a..6bc38de5 100644 --- a/internal/controller/operator/eventing/eventmesh.go +++ b/internal/controller/operator/eventing/eventmesh.go @@ -44,9 +44,7 @@ type oauth2Credentials struct { certsURL []byte } -func (r *Reconciler) reconcileEventMeshSubManager(ctx context.Context, eventing *v1alpha1.Eventing, - eventMeshSecret *kcorev1.Secret, log *zap.SugaredLogger, -) error { +func (r *Reconciler) reconcileEventMeshSubManager(ctx context.Context, eventing *v1alpha1.Eventing, eventMeshSecret *kcorev1.Secret) error { // gets oauth2ClientID and secret and stops the EventMesh subscription manager if changed err := r.syncOauth2ClientIDAndSecret(ctx, eventing) if err != nil { @@ -67,18 +65,10 @@ func (r *Reconciler) reconcileEventMeshSubManager(ctx context.Context, eventing // Read the cluster domain from the Eventing CR, or // read it from the configmap managed by gardener - domain := eventing.Spec.Backend.Config.Domain - if utils.IsEmpty(domain) { - r.namedLogger().Infof( - `Domain is not configured in the Eventing CR, reading it from the ConfigMap %s/%s`, - shootInfoConfigMapNamespace, shootInfoConfigMapName, - ) - domain, err = r.readDomainFromConfigMap(ctx) - if err != nil || utils.IsEmpty(domain) { - return domainMissingError(err) - } + domain, err := r.checkDomain(ctx, eventing.Spec.Backend.Config.Domain) + if err != nil { + return err } - r.namedLogger().Infof(`Domain is %s`, domain) // get the subscription config defaultSubsConfig := r.getDefaultSubscriptionConfig() @@ -136,6 +126,23 @@ func (r *Reconciler) reconcileEventMeshSubManager(ctx context.Context, eventing return nil } +func (r *Reconciler) checkDomain(ctx context.Context, domain string) (string, error) { + ret := domain + if utils.IsEmpty(ret) { + r.namedLogger().Infof( + `Domain is not configured in the Eventing CR, reading it from the ConfigMap %s/%s`, + shootInfoConfigMapNamespace, shootInfoConfigMapName, + ) + cmDomain, err := r.readDomainFromConfigMap(ctx) + if err != nil || utils.IsEmpty(cmDomain) { + return "", domainMissingError(err) + } + ret = cmDomain + } + r.namedLogger().Infof(`Domain is %s`, ret) + return ret, nil +} + func (r *Reconciler) getEventMeshSubManagerParams() submgrmanager.Params { return submgrmanager.Params{ submgrmanager.ParamNameClientID: r.oauth2credentials.clientID, diff --git a/internal/controller/operator/eventing/eventmesh_test.go b/internal/controller/operator/eventing/eventmesh_test.go index a80d33d2..2b72cd24 100644 --- a/internal/controller/operator/eventing/eventmesh_test.go +++ b/internal/controller/operator/eventing/eventmesh_test.go @@ -263,8 +263,6 @@ func Test_reconcileEventMeshSubManager(t *testing.T) { testEnv := NewMockedUnitTestEnvironment(t, givenEventing, givenOauthSecret) testEnv.Reconciler.backendConfig = *givenBackendConfig - logger := testEnv.Reconciler.logger.WithContext().Named(ControllerName) - // get mocks from test-case. givenEventMeshSubManagerMock := tc.givenEventMeshSubManagerMock() givenManagerFactoryMock := tc.givenManagerFactoryMock(givenEventMeshSubManagerMock) @@ -290,15 +288,15 @@ func Test_reconcileEventMeshSubManager(t *testing.T) { eventMeshSecret := utils.NewEventMeshSecret("eventing-backend", givenEventing.Namespace) - err := testEnv.Reconciler.reconcileEventMeshSubManager(ctx, givenEventing, eventMeshSecret, logger) + err := testEnv.Reconciler.reconcileEventMeshSubManager(ctx, givenEventing, eventMeshSecret) if err != nil && tc.givenShouldRetry { // This is to test the scenario where initialization of eventMeshSubManager was successful but // starting the eventMeshSubManager failed. So on next try it should again try to start the eventMeshSubManager. - err = testEnv.Reconciler.reconcileEventMeshSubManager(ctx, givenEventing, eventMeshSecret, logger) + err = testEnv.Reconciler.reconcileEventMeshSubManager(ctx, givenEventing, eventMeshSecret) } if tc.givenUpdateTest { // Run reconcile again with newBackendConfig: - err = testEnv.Reconciler.reconcileEventMeshSubManager(ctx, givenEventing, eventMeshSecret, logger) + err = testEnv.Reconciler.reconcileEventMeshSubManager(ctx, givenEventing, eventMeshSecret) require.NoError(t, err) } @@ -425,8 +423,6 @@ func Test_reconcileEventMeshSubManager_ReadClusterDomain(t *testing.T) { testEnv := NewMockedUnitTestEnvironment(t, tc.givenEventing, givenOauthSecret) testEnv.Reconciler.backendConfig = *givenBackendConfig - logger := testEnv.Reconciler.logger.WithContext().Named(ControllerName) - givenEventMeshSubManagerMock := tc.givenEventMeshSubManagerMock() givenManagerFactoryMock := tc.givenManagerFactoryMock(givenEventMeshSubManagerMock) givenEventingManagerMock := tc.givenEventingManagerMock() @@ -443,7 +439,7 @@ func Test_reconcileEventMeshSubManager_ReadClusterDomain(t *testing.T) { // when eventMeshSecret := utils.NewEventMeshSecret("test-secret", tc.givenEventing.Namespace) - err := testEnv.Reconciler.reconcileEventMeshSubManager(ctx, tc.givenEventing, eventMeshSecret, logger) + err := testEnv.Reconciler.reconcileEventMeshSubManager(ctx, tc.givenEventing, eventMeshSecret) // then require.NoError(t, err) diff --git a/internal/controller/operator/eventing/integrationtests/controller/integration_test.go b/internal/controller/operator/eventing/integrationtests/controller/integration_test.go index 9420519f..292dedfa 100644 --- a/internal/controller/operator/eventing/integrationtests/controller/integration_test.go +++ b/internal/controller/operator/eventing/integrationtests/controller/integration_test.go @@ -224,7 +224,7 @@ func Test_CreateEventingCR_NATS(t *testing.T) { if tc.givenDeploymentReady && tc.givenEventing.Spec.Backend != nil { // check if EPP deployment, HPA resources created and values are reflected including owner reference. ensureEPPDeploymentAndHPAResources(t, tc.givenEventing, testEnvironment) - // TODO: ensure NATS Backend config is reflected. Done as subscription controller is implemented. + //nolint:godox // TODO: ensure NATS Backend config is reflected. Done as subscription controller is implemented. } if tc.wantEnsureK8sObjects && tc.givenEventing.Spec.Backend != nil { @@ -363,20 +363,20 @@ func Test_ReconcileSameEventingCR(t *testing.T) { resourceVersionBefore := eppDeployment.ObjectMeta.ResourceVersion for r := 0; r < runs; r++ { // when - runId := fmt.Sprintf("run-%d", r) + runID := fmt.Sprintf("run-%d", r) eventingCR, err = testEnvironment.GetEventingFromK8s(eventingCR.Name, namespace) require.NoError(t, err) require.NotNil(t, eventingCR) eventingCR = eventingCR.DeepCopy() - eventingCR.ObjectMeta.Labels = map[string]string{"reconcile": runId} // force new reconciliation + eventingCR.ObjectMeta.Labels = map[string]string{"reconcile": runID} // force new reconciliation testEnvironment.EnsureK8sResourceUpdated(t, eventingCR) eventingCR, err = testEnvironment.GetEventingFromK8s(eventingCR.Name, namespace) require.NoError(t, err) require.NotNil(t, eventingCR) - require.Equal(t, eventingCR.ObjectMeta.Labels["reconcile"], runId) + require.Equal(t, eventingCR.ObjectMeta.Labels["reconcile"], runID) // then testEnvironment.EnsureEventingSpecPublisherReflected(t, eventingCR) @@ -721,7 +721,7 @@ func Test_CreateEventingCR_EventMesh(t *testing.T) { if tc.givenDeploymentReady { // check if EPP deployment, HPA resources created and values are reflected including owner reference. ensureEPPDeploymentAndHPAResources(t, tc.givenEventing, testEnvironment) - // TODO: ensure NATS Backend config is reflected. Done as subscription controller is implemented. + //nolint:godox // TODO: ensure NATS Backend config is reflected. Done as subscription controller is implemented. } if tc.wantEnsureK8sObjects { @@ -988,16 +988,11 @@ func Test_WatcherNATSResource(t *testing.T) { testEnvironment.EnsureNamespaceCreation(t, givenNamespace) - // create NATS CR. + // create original NATS CR. originalNats := tc.givenOriginalNATS.DeepCopy() testEnvironment.EnsureK8sResourceCreated(t, originalNats) - // create original NATS CR. - if tc.givenOriginalNATS.Status.State == natsv1alpha1.StateReady { - testEnvironment.EnsureNATSResourceStateReady(t, originalNats) - } else if tc.givenOriginalNATS.Status.State == natsv1alpha1.StateError { - testEnvironment.EnsureNATSResourceStateError(t, originalNats) - } + testEnvironment.EnsureNATSResourceState(t, originalNats, tc.givenOriginalNATS.Status.State) // create Eventing CR. var eventingResource *operatorv1alpha1.Eventing @@ -1043,11 +1038,7 @@ func Test_WatcherNATSResource(t *testing.T) { // update target NATS CR to target NATS CR state if tc.givenOriginalNATS != nil { - if tc.givenOriginalNATS.Status.State == natsv1alpha1.StateReady { - testEnvironment.EnsureNATSResourceStateReady(t, tc.givenOriginalNATS) - } else if tc.givenOriginalNATS.Status.State == natsv1alpha1.StateError { - testEnvironment.EnsureNATSResourceStateError(t, tc.givenOriginalNATS) - } + testEnvironment.EnsureNATSResourceState(t, tc.givenOriginalNATS, tc.givenOriginalNATS.Status.State) } // check Eventing CR status. @@ -1055,11 +1046,7 @@ func Test_WatcherNATSResource(t *testing.T) { // update target NATS CR to target NATS CR state if tc.givenTargetNATS != nil { - if tc.givenTargetNATS.Status.State == natsv1alpha1.StateReady { - testEnvironment.EnsureNATSResourceStateReady(t, tc.givenTargetNATS) - } else if tc.givenTargetNATS.Status.State == natsv1alpha1.StateError { - testEnvironment.EnsureNATSResourceStateError(t, tc.givenTargetNATS) - } + testEnvironment.EnsureNATSResourceState(t, tc.givenTargetNATS, tc.givenTargetNATS.Status.State) } else { // delete NATS CR testEnvironment.EnsureK8sResourceDeleted(t, tc.givenOriginalNATS) @@ -1072,6 +1059,7 @@ func Test_WatcherNATSResource(t *testing.T) { } func ensureEPPDeploymentAndHPAResources(t *testing.T, givenEventing *operatorv1alpha1.Eventing, testEnvironment *testutilsintegration.TestEnvironment) { + t.Helper() testEnvironment.EnsureDeploymentExists(t, eventing.GetPublisherDeploymentName(*givenEventing), givenEventing.Namespace) testEnvironment.EnsureHPAExists(t, eventing.GetPublisherDeploymentName(*givenEventing), givenEventing.Namespace) testEnvironment.EnsureEventingSpecPublisherReflected(t, givenEventing) @@ -1081,6 +1069,7 @@ func ensureEPPDeploymentAndHPAResources(t *testing.T, givenEventing *operatorv1a } func ensureK8sResources(t *testing.T, givenEventing *operatorv1alpha1.Eventing, testEnvironment *testutilsintegration.TestEnvironment) { + t.Helper() testEnvironment.EnsureEPPK8sResourcesExists(t, *givenEventing) // check if the owner reference is set. diff --git a/internal/controller/operator/eventing/integrationtests/controller_switching/integration_test.go b/internal/controller/operator/eventing/integrationtests/controllerswitching/integration_test.go similarity index 99% rename from internal/controller/operator/eventing/integrationtests/controller_switching/integration_test.go rename to internal/controller/operator/eventing/integrationtests/controllerswitching/integration_test.go index 93f9a9e5..87e852b5 100644 --- a/internal/controller/operator/eventing/integrationtests/controller_switching/integration_test.go +++ b/internal/controller/operator/eventing/integrationtests/controllerswitching/integration_test.go @@ -1,4 +1,4 @@ -package controller_switching +package controllerswitching import ( "fmt" @@ -197,6 +197,7 @@ func Test_Switching(t *testing.T) { } func ensureEPPDeploymentAndHPAResources(t *testing.T, givenEventing *operatorv1alpha1.Eventing, testEnvironment *testutilsintegration.TestEnvironment) { + t.Helper() testEnvironment.EnsureDeploymentExists(t, eventing.GetPublisherDeploymentName(*givenEventing), givenEventing.Namespace) testEnvironment.EnsureHPAExists(t, eventing.GetPublisherDeploymentName(*givenEventing), givenEventing.Namespace) testEnvironment.EnsureEventingSpecPublisherReflected(t, givenEventing) @@ -205,6 +206,7 @@ func ensureEPPDeploymentAndHPAResources(t *testing.T, givenEventing *operatorv1a } func ensureK8sResources(t *testing.T, givenEventing *operatorv1alpha1.Eventing, testEnvironment *testutilsintegration.TestEnvironment) { + t.Helper() testEnvironment.EnsureEPPK8sResourcesExists(t, *givenEventing) // check if the owner reference is set. diff --git a/internal/controller/operator/eventing/integrationtests/nats_disabled/integration_test.go b/internal/controller/operator/eventing/integrationtests/nats_disabled/integration_test.go index 40c8f746..747a328a 100644 --- a/internal/controller/operator/eventing/integrationtests/nats_disabled/integration_test.go +++ b/internal/controller/operator/eventing/integrationtests/nats_disabled/integration_test.go @@ -98,7 +98,7 @@ func Test_DeletionOfPublisherResourcesWhenNATSNotEnabled(t *testing.T) { // check if EPP resources exists. ensureK8sResources(t, givenEventing, testEnvironment) - natsCRD, err := testEnvironment.KubeClient.GetCRD(context.TODO(), k8s.NatsGVK.GroupResource().String()) + natsCRD, err := testEnvironment.KubeClient.GetCRD(context.TODO(), k8s.NatsGVK().GroupResource().String()) require.NoError(t, err) // define cleanup. @@ -135,6 +135,7 @@ func Test_DeletionOfPublisherResourcesWhenNATSNotEnabled(t *testing.T) { } func ensureEPPDeploymentAndHPAResources(t *testing.T, givenEventing *operatorv1alpha1.Eventing, testEnvironment *testutilsintegration.TestEnvironment) { + t.Helper() testEnvironment.EnsureDeploymentExists(t, eventing.GetPublisherDeploymentName(*givenEventing), givenEventing.Namespace) testEnvironment.EnsureHPAExists(t, eventing.GetPublisherDeploymentName(*givenEventing), givenEventing.Namespace) testEnvironment.EnsureEventingSpecPublisherReflected(t, givenEventing) @@ -144,6 +145,7 @@ func ensureEPPDeploymentAndHPAResources(t *testing.T, givenEventing *operatorv1a } func ensureK8sResources(t *testing.T, givenEventing *operatorv1alpha1.Eventing, testEnvironment *testutilsintegration.TestEnvironment) { + t.Helper() testEnvironment.EnsureEPPK8sResourcesExists(t, *givenEventing) // check if the owner reference is set. diff --git a/internal/controller/operator/eventing/integrationtests/validation/integration_test.go b/internal/controller/operator/eventing/integrationtests/validation/integration_test.go index 6cbedbe0..ad78e2b8 100644 --- a/internal/controller/operator/eventing/integrationtests/validation/integration_test.go +++ b/internal/controller/operator/eventing/integrationtests/validation/integration_test.go @@ -61,9 +61,9 @@ const ( requests = "requests" cpu = "cpu" memory = "memory" - limitsCpuValue = "500m" + limitsCPUValue = "500m" limitsMemoryValue = "512Mi" - requestsCpuValue = "10m" + requestsCPUValue = "10m" requestsMemoryValue = "256Mi" logging = "logging" logLevel = "logLevel" @@ -885,11 +885,11 @@ func Test_Validate_Defaulting(t *testing.T) { }, resources: map[string]any{ limits: map[string]any{ - cpu: limitsCpuValue, + cpu: limitsCPUValue, memory: limitsMemoryValue, }, requests: map[string]any{ - cpu: requestsCpuValue, + cpu: requestsCPUValue, memory: requestsMemoryValue, }, }, @@ -965,11 +965,11 @@ func Test_Validate_Defaulting(t *testing.T) { }, resources: map[string]any{ limits: map[string]any{ - cpu: limitsCpuValue, + cpu: limitsCPUValue, memory: limitsMemoryValue, }, requests: map[string]any{ - cpu: requestsCpuValue, + cpu: requestsCPUValue, memory: requestsMemoryValue, }, }, diff --git a/internal/controller/operator/eventing/integrationtests/without_apirule_crd/integration_test.go b/internal/controller/operator/eventing/integrationtests/without_apirule_crd/integration_test.go index f6f93250..502ec12b 100644 --- a/internal/controller/operator/eventing/integrationtests/without_apirule_crd/integration_test.go +++ b/internal/controller/operator/eventing/integrationtests/without_apirule_crd/integration_test.go @@ -1,4 +1,4 @@ -package controller_switching +package controllerswitching import ( "os" diff --git a/internal/controller/operator/eventing/nats.go b/internal/controller/operator/eventing/nats.go index fca7b348..16e77f42 100644 --- a/internal/controller/operator/eventing/nats.go +++ b/internal/controller/operator/eventing/nats.go @@ -16,7 +16,7 @@ import ( 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 { +func (r *Reconciler) reconcileNATSSubManager(eventing *v1alpha1.Eventing, log *zap.SugaredLogger) error { // get the subscription config defaultSubsConfig := r.getDefaultSubscriptionConfig() // get the nats config @@ -136,7 +136,7 @@ func (n *NatsConfigHandlerImpl) GetNatsConfig(ctx context.Context, eventing v1al return nil, err } // identifies the NATs server url and sets to natsConfig - err = n.setUrlToNatsConfig(ctx, &eventing, &natsConfig) + err = n.setURLToNatsConfig(ctx, &eventing, &natsConfig) if err != nil { return nil, err } @@ -148,12 +148,12 @@ func (n *NatsConfigHandlerImpl) GetNatsConfig(ctx context.Context, eventing v1al return &natsConfig, nil } -func (n *NatsConfigHandlerImpl) setUrlToNatsConfig(ctx context.Context, eventing *v1alpha1.Eventing, natsConfig *env.NATSConfig) error { - natsUrl, err := n.getNATSUrl(ctx, eventing.Namespace) +func (n *NatsConfigHandlerImpl) setURLToNatsConfig(ctx context.Context, eventing *v1alpha1.Eventing, natsConfig *env.NATSConfig) error { + natsURL, err := n.getNATSUrl(ctx, eventing.Namespace) if err != nil { return err } - natsConfig.URL = natsUrl + natsConfig.URL = natsURL return nil } diff --git a/internal/controller/operator/eventing/nats_test.go b/internal/controller/operator/eventing/nats_test.go index 7dfc87c2..b2186c86 100644 --- a/internal/controller/operator/eventing/nats_test.go +++ b/internal/controller/operator/eventing/nats_test.go @@ -249,15 +249,15 @@ func Test_reconcileNATSSubManager(t *testing.T) { givenEventing.Status.BackendConfigHash = tc.givenHashBefore // when - err := testEnv.Reconciler.reconcileNATSSubManager(context.Background(), givenEventing, logger) + err := testEnv.Reconciler.reconcileNATSSubManager(givenEventing, logger) if err != nil && tc.givenShouldRetry { // This is to test the scenario where initialization of natsSubManager was successful but // starting the natsSubManager failed. So on next try it should again try to start the natsSubManager. - err = testEnv.Reconciler.reconcileNATSSubManager(context.Background(), givenEventing, logger) + err = testEnv.Reconciler.reconcileNATSSubManager(givenEventing, logger) } if tc.givenUpdateTest { // Run reconcile again with newBackendConfig: - err = testEnv.Reconciler.reconcileNATSSubManager(context.Background(), givenEventing, logger) + err = testEnv.Reconciler.reconcileNATSSubManager(givenEventing, logger) require.NoError(t, err) } @@ -574,7 +574,7 @@ func Test_UpdateNatsConfig(t *testing.T) { // when natsConfig := env.NATSConfig{} - err := natsConfigHandler.setUrlToNatsConfig(ctx, tc.eventing, &natsConfig) + err := natsConfigHandler.setURLToNatsConfig(ctx, tc.eventing, &natsConfig) // then require.Equal(t, tc.expectedError, err) diff --git a/internal/controller/operator/eventing/status.go b/internal/controller/operator/eventing/status.go index a554c6da..095487ad 100644 --- a/internal/controller/operator/eventing/status.go +++ b/internal/controller/operator/eventing/status.go @@ -18,7 +18,7 @@ import ( const RequeueTimeForStatusCheck = 10 // InitStateProcessing initializes the state of the EventingStatus if it is not set. -func (es *Reconciler) InitStateProcessing(eventing *operatorv1alpha1.Eventing) { +func (r *Reconciler) InitStateProcessing(eventing *operatorv1alpha1.Eventing) { if eventing.Status.State == "" { eventing.Status.SetStateProcessing() } @@ -44,7 +44,7 @@ func (r *Reconciler) syncStatusWithNATSState(ctx context.Context, state string, return errors.Join(err, r.syncEventingStatus(ctx, eventing, log)) } -func (r *Reconciler) syncStatusForEmptyBackend(ctx context.Context, reason operatorv1alpha1.ConditionReason, +func (r *Reconciler) syncStatusForEmptyBackend(ctx context.Context, message string, eventing *operatorv1alpha1.Eventing, log *zap.SugaredLogger, ) error { // Set error state in status @@ -218,7 +218,9 @@ func (r *Reconciler) handleEventingState(ctx context.Context, deployment *kappsv return kctrl.Result{}, r.syncEventingStatus(ctx, eventingCR, log) } -// to be able to mock this function in tests. +// IsDeploymentReady is a variable to able to mock this function in tests. +// +//nolint:gochecknoglobals //TODO: refactor the reconciler to support replacing this function without global variable var IsDeploymentReady = func(deployment *kappsv1.Deployment) bool { return deployment.Status.AvailableReplicas == *deployment.Spec.Replicas } diff --git a/internal/controller/operator/eventing/unit_test.go b/internal/controller/operator/eventing/unit_test.go index 4461528c..54200e88 100644 --- a/internal/controller/operator/eventing/unit_test.go +++ b/internal/controller/operator/eventing/unit_test.go @@ -37,6 +37,7 @@ type MockedUnitTestEnvironment struct { } func NewMockedUnitTestEnvironment(t *testing.T, objs ...client.Object) *MockedUnitTestEnvironment { + t.Helper() // setup logger ctrLogger, err := logger.New("json", "info") require.NoError(t, err) diff --git a/options/options.go b/options/options.go index a3758194..045abfae 100644 --- a/options/options.go +++ b/options/options.go @@ -21,6 +21,15 @@ const ( // All the available environment variables. envNameLogFormat = "APP_LOG_FORMAT" envNameLogLevel = "APP_LOG_LEVEL" + + // default values. + defaultMaxReconnects = 10 + defaultMetricsAddr = ":8080" + defaultReconnectWait = 3 * time.Second + defaultReconcilePeriod = time.Minute * 10 + defaultProbeAddr = ":8081" + defaultReadyEndpoint = "readyz" + defaultHealthEndpoint = "healthz" ) // Options represents the controller options. @@ -53,13 +62,13 @@ func New() *Options { // Parse parses the controller options. func (o *Options) Parse() error { - flag.IntVar(&o.MaxReconnects, argNameMaxReconnects, 10, "Maximum number of reconnect attempts (NATS).") - flag.StringVar(&o.MetricsAddr, argNameMetricsAddr, ":8080", "The address the metric endpoint binds to.") - flag.DurationVar(&o.ReconnectWait, argNameReconnectWait, 3*time.Second, "Wait time between reconnect attempts (NATS).") - flag.DurationVar(&o.ReconcilePeriod, argNameReconcilePeriod, time.Minute*10, "Period between triggering of reconciling calls (BEB).") - flag.StringVar(&o.ProbeAddr, argNameProbeAddr, ":8081", "The TCP address that the controller should bind to for serving health probes.") - flag.StringVar(&o.ReadyEndpoint, argNameReadyEndpoint, "readyz", "The endpoint of the readiness probe.") - flag.StringVar(&o.HealthEndpoint, argNameHealthEndpoint, "healthz", "The endpoint of the health probe.") + flag.IntVar(&o.MaxReconnects, argNameMaxReconnects, defaultMaxReconnects, "Maximum number of reconnect attempts (NATS).") + flag.StringVar(&o.MetricsAddr, argNameMetricsAddr, defaultMetricsAddr, "The address the metric endpoint binds to.") + flag.DurationVar(&o.ReconnectWait, argNameReconnectWait, defaultReconnectWait, "Wait time between reconnect attempts (NATS).") + flag.DurationVar(&o.ReconcilePeriod, argNameReconcilePeriod, defaultReconcilePeriod, "Period between triggering of reconciling calls (BEB).") + flag.StringVar(&o.ProbeAddr, argNameProbeAddr, defaultProbeAddr, "The TCP address that the controller should bind to for serving health probes.") + flag.StringVar(&o.ReadyEndpoint, argNameReadyEndpoint, defaultReadyEndpoint, "The endpoint of the readiness probe.") + flag.StringVar(&o.HealthEndpoint, argNameHealthEndpoint, defaultHealthEndpoint, "The endpoint of the health probe.") flag.Parse() if err := envconfig.Process("", &o.Env); err != nil { diff --git a/pkg/backend/eventmesh/eventmesh.go b/pkg/backend/eventmesh/eventmesh.go index 361d7be6..12e01508 100644 --- a/pkg/backend/eventmesh/eventmesh.go +++ b/pkg/backend/eventmesh/eventmesh.go @@ -100,9 +100,7 @@ func getWebHookAuth(credentials *OAuth2ClientCredentials) *types.WebhookAuth { // SyncSubscription synchronize the EV2 subscription with the EMS subscription. // It returns true, if the EV2 subscription status was changed. -func (em *EventMesh) SyncSubscription(subscription *eventingv1alpha2.Subscription, cleaner cleaner.Cleaner, - apiRule *apigatewayv1beta1.APIRule, -) (bool, error) { //nolint:funlen,gocognit +func (em *EventMesh) SyncSubscription(subscription *eventingv1alpha2.Subscription, cleaner cleaner.Cleaner, apiRule *apigatewayv1beta1.APIRule) (bool, error) { // Format logger log := backendutils.LoggerWithSubscription(em.namedLogger(), subscription) diff --git a/pkg/backend/eventtype/clean.go b/pkg/backend/eventtype/clean.go index 9cd060ee..de546e01 100644 --- a/pkg/backend/eventtype/clean.go +++ b/pkg/backend/eventtype/clean.go @@ -8,14 +8,12 @@ import ( "github.com/kyma-project/eventing-manager/pkg/logger" ) -var ( - // invalidEventTypeSegment used to match and replace none-alphanumeric characters in the event-type segments - // as per SAP Event spec https://github.tools.sap/CentralEngineering/sap-event-specification#type. - invalidEventTypeSegment = regexp.MustCompile("[^a-zA-Z0-9.]") +// invalidEventTypeSegment used to match and replace none-alphanumeric characters in the event-type segments +// as per SAP Event spec https://github.tools.sap/CentralEngineering/sap-event-specification#type. +var invalidEventTypeSegment = regexp.MustCompile("[^a-zA-Z0-9.]") - // cleanerName used as the logger name. - cleanerName = "event-type-cleaner" -) +// cleanerName used as the logger name. +const cleanerName = "event-type-cleaner" type Cleaner interface { Clean(eventType string) (string, error) diff --git a/pkg/backend/eventtype/parse.go b/pkg/backend/eventtype/parse.go index 847cbf50..ae35e754 100644 --- a/pkg/backend/eventtype/parse.go +++ b/pkg/backend/eventtype/parse.go @@ -27,8 +27,9 @@ func parse(eventType, prefix string) (string, string, string, error) { // make sure that the remaining string has at least 4 segments separated by "." // (e.g. application.businessObject.operation.version) + const minSAPEventTypeLength = 4 parts := strings.Split(eventType, ".") - if len(parts) < 4 { + if len(parts) < minSAPEventTypeLength { return "", "", "", ErrInvalidFormat } diff --git a/pkg/backend/jetstream/config.go b/pkg/backend/jetstream/config.go index f4c5d13a..84d8f1ca 100644 --- a/pkg/backend/jetstream/config.go +++ b/pkg/backend/jetstream/config.go @@ -3,7 +3,8 @@ package jetstream import "github.com/kyma-project/eventing-manager/pkg/env" // Validate ensures that the NatsConfig is valid and therefore can be used safely. -// TODO: as soon as backend/nats is gone, make this method a function of backendnats.Config. +// +//nolint:godox // TODO: as soon as backend/nats is gone, make this method a function of backendnats.Config. func Validate(natsConfig env.NATSConfig) error { if natsConfig.JSStreamName == "" { return ErrEmptyStreamName diff --git a/pkg/backend/jetstream/connection_integration_test.go b/pkg/backend/jetstream/connection_integration_test.go index 240afb81..21e8a6f1 100644 --- a/pkg/backend/jetstream/connection_integration_test.go +++ b/pkg/backend/jetstream/connection_integration_test.go @@ -96,6 +96,7 @@ func Test_ConnectionBuilder_IsConnected(t *testing.T) { // startManagedNATSServer starts a NATS server and shuts the server down as soon as the test is // completed (also when it failed!). func startManagedNATSServer(t *testing.T) *server.Server { + t.Helper() natsServer, _, err := jetstream.StartNATSServer(eventingtesting.WithJetStreamEnabled()) require.NoError(t, err) t.Cleanup(func() { @@ -106,6 +107,7 @@ func startManagedNATSServer(t *testing.T) *server.Server { // fixtureUnusedLocalhostUrl provides a localhost URL with an unused port. func fixtureUnusedLocalhostURL(t *testing.T) string { + t.Helper() port, err := eventingtesting.GetFreePort() require.NoError(t, err) url := fmt.Sprintf("http://localhost:%d", port) diff --git a/pkg/backend/jetstream/jetstream_integration_test.go b/pkg/backend/jetstream/jetstream_integration_test.go index 277c03de..8fde367b 100644 --- a/pkg/backend/jetstream/jetstream_integration_test.go +++ b/pkg/backend/jetstream/jetstream_integration_test.go @@ -289,6 +289,7 @@ func TestJetStreamSubAfterSync_DeleteOldFilterConsumerForTypeChangeWhileNatsDown // HELPER functions func prepareTestEnvironment(t *testing.T) *TestEnvironment { + t.Helper() testEnvironment := setupTestEnvironment(t) testEnvironment.jsBackend.Config.JSStreamStorageType = StorageTypeFile testEnvironment.jsBackend.Config.MaxReconnects = 0 @@ -297,10 +298,8 @@ func prepareTestEnvironment(t *testing.T) *TestEnvironment { return testEnvironment } -func createSubscriptionAndAssert(t *testing.T, - testEnv *TestEnvironment, - subscriber *eventingtesting.Subscriber, -) (SubscriptionSubjectIdentifier, *eventingv1alpha2.Subscription) { +func createSubscriptionAndAssert(t *testing.T, testEnv *TestEnvironment, subscriber *eventingtesting.Subscriber) (SubscriptionSubjectIdentifier, *eventingv1alpha2.Subscription) { + t.Helper() sub := eventingtesting.NewSubscription("sub", "foo", eventingtesting.WithCleanEventSourceAndType(), eventingtesting.WithNotCleanEventSourceAndType(), @@ -325,6 +324,7 @@ func createSubscriptionAndAssert(t *testing.T, } func shutdownJetStream(t *testing.T, testEnv *TestEnvironment) { + t.Helper() testEnv.natsServer.Shutdown() require.Eventually(t, func() bool { return !testEnv.jsBackend.Conn.IsConnected() @@ -337,6 +337,7 @@ func deleteSecondFilter(testEnv *TestEnvironment, sub *eventingv1alpha2.Subscrip } func startJetStream(t *testing.T, testEnv *TestEnvironment) { + t.Helper() _ = eventingtesting.RunNatsServerOnPort( eventingtesting.WithPort(testEnv.natsPort), eventingtesting.WithJetStreamEnabled()) @@ -347,10 +348,8 @@ func startJetStream(t *testing.T, testEnv *TestEnvironment) { }, 60*time.Second, 5*time.Second) } -func assertNewSubscriptionReturnItsKey(t *testing.T, - testEnv *TestEnvironment, - sub *eventingv1alpha2.Subscription, -) SubscriptionSubjectIdentifier { +func assertNewSubscriptionReturnItsKey(t *testing.T, testEnv *TestEnvironment, sub *eventingv1alpha2.Subscription) SubscriptionSubjectIdentifier { + t.Helper() firstSubject, err := testEnv.cleaner.CleanEventType(sub.Spec.Types[0]) require.NoError(t, err) require.NotEmpty(t, firstSubject) @@ -696,6 +695,8 @@ func defaultNATSConfig(url string, port int) env.NATSConfig { // getJetStreamClient creates a client with JetStream context, or fails the caller test. func getJetStreamClient(t *testing.T, serverURL string) *jetStreamClient { + t.Helper() + conn, err := nats.Connect(serverURL) if err != nil { t.Error(err.Error()) @@ -713,6 +714,7 @@ func getJetStreamClient(t *testing.T, serverURL string) *jetStreamClient { // setupTestEnvironment is a TestEnvironment constructor. func setupTestEnvironment(t *testing.T) *TestEnvironment { + t.Helper() natsServer, natsPort, err := StartNATSServer(eventingtesting.WithJetStreamEnabled()) require.NoError(t, err) natsConfig := defaultNATSConfig(natsServer.ClientURL(), natsPort) diff --git a/pkg/backend/jetstream/stubs_test.go b/pkg/backend/jetstream/stubs_test.go index 1acb6882..55409a70 100644 --- a/pkg/backend/jetstream/stubs_test.go +++ b/pkg/backend/jetstream/stubs_test.go @@ -1,3 +1,4 @@ +//nolint:godox // stubs for testing package jetstream import ( diff --git a/pkg/backend/jetstream/test_helpers.go b/pkg/backend/jetstream/test_helpers.go index 811f5200..d39b3a42 100644 --- a/pkg/backend/jetstream/test_helpers.go +++ b/pkg/backend/jetstream/test_helpers.go @@ -76,7 +76,8 @@ func SendCloudEventToJetStream(jetStreamClient *JetStream, subject, eventData, c } else { headers = eventingtesting.GetStructuredMessageHeaders() } - req, err := http.NewRequest(http.MethodPost, "dummy", bytes.NewBufferString(eventData)) + ctx := context.Background() + req, err := http.NewRequestWithContext(ctx, http.MethodPost, "dummy", bytes.NewBufferString(eventData)) if err != nil { return err } diff --git a/pkg/backend/jetstream/utils.go b/pkg/backend/jetstream/utils.go index 811fd02c..e487dcd5 100644 --- a/pkg/backend/jetstream/utils.go +++ b/pkg/backend/jetstream/utils.go @@ -203,7 +203,6 @@ func GetCleanEventTypesFromEventTypes(eventTypes []eventingv1alpha2.EventType) [ return cleantypes } -// TODO: to be moved to subscription types. func getUniqueEventTypes(eventTypes []string) []string { unique := make([]string, 0, len(eventTypes)) mapper := make(map[string]bool) diff --git a/pkg/backend/metrics/collector.go b/pkg/backend/metrics/collector.go index 6417c665..223ed782 100644 --- a/pkg/backend/metrics/collector.go +++ b/pkg/backend/metrics/collector.go @@ -56,6 +56,12 @@ type Collector struct { // NewCollector a new instance of Collector. func NewCollector() *Collector { + const ( + // for the latency we want 10 exponential Buckets starting at 0.002 + bucketMin = 0.002 + bucketFactor = 2 + bucketCount = 10 + ) return &Collector{ deliveryPerSubscription: prometheus.NewCounterVec( prometheus.CounterOpts{ @@ -68,7 +74,7 @@ func NewCollector() *Collector { prometheus.HistogramOpts{ Name: latencyMetricKey, Help: latencyMetricHelp, - Buckets: prometheus.ExponentialBuckets(0.002, 2, 10), + Buckets: prometheus.ExponentialBuckets(bucketMin, bucketFactor, bucketCount), }, []string{subscriptionNameLabel, subscriptionNamespaceLabel, eventTypeLabel, sinkLabel, responseCodeLabel, consumerNameLabel}, ), diff --git a/pkg/backend/utils/eventmesh_utils.go b/pkg/backend/utils/eventmesh_utils.go index 8c62dee2..3a402164 100644 --- a/pkg/backend/utils/eventmesh_utils.go +++ b/pkg/backend/utils/eventmesh_utils.go @@ -74,13 +74,13 @@ func hashSubscriptionFullName(domainName, namespace, name string) string { return hex.EncodeToString(hash[:]) } -func getDefaultSubscriptionV1Alpha2(protocolSettings *ProtocolSettings) (*types.Subscription, error) { +func getDefaultSubscriptionV1Alpha2(protocolSettings *ProtocolSettings) *types.Subscription { // @TODO: Rename this method to getDefaultSubscription once old BEB backend is depreciated emsSubscription := &types.Subscription{} emsSubscription.ContentMode = *protocolSettings.ContentMode emsSubscription.ExemptHandshake = *protocolSettings.ExemptHandshake emsSubscription.Qos = types.GetQos(*protocolSettings.Qos) - return emsSubscription, nil + return emsSubscription } func getEventMeshEvents(typeInfos []EventTypeInfo, typeMatching eventingv1alpha2.TypeMatching, @@ -105,23 +105,22 @@ func getEventMeshEvents(typeInfos []EventTypeInfo, typeMatching eventingv1alpha2 return events } -func ConvertKymaSubToEventMeshSub(subscription *eventingv1alpha2.Subscription, typeInfos []EventTypeInfo, - apiRule *apigatewayv1beta1.APIRule, defaultWebhookAuth *types.WebhookAuth, +func ConvertKymaSubToEventMeshSub( + subscription *eventingv1alpha2.Subscription, + typeInfos []EventTypeInfo, + apiRule *apigatewayv1beta1.APIRule, + defaultWebhookAuth *types.WebhookAuth, defaultProtocolSettings *ProtocolSettings, - defaultNamespace string, nameMapper NameMapper, -) (*types.Subscription, error) { //nolint:gocognit + defaultNamespace string, + nameMapper NameMapper, +) (*types.Subscription, error) { // get default EventMesh subscription object - eventMeshSubscription, err := getDefaultSubscriptionV1Alpha2(defaultProtocolSettings) - if err != nil { - return nil, errors.Wrap(err, "apply default protocol settings failed") - } + eventMeshSubscription := getDefaultSubscriptionV1Alpha2(defaultProtocolSettings) // set Name of EventMesh subscription eventMeshSubscription.Name = nameMapper.MapSubscriptionName(subscription.Name, subscription.Namespace) // Applying protocol settings if provided in subscription CR - if setErr := setEventMeshProtocolSettings(subscription, eventMeshSubscription); setErr != nil { - return nil, setErr - } + setEventMeshProtocolSettings(subscription, eventMeshSubscription) // Events // set the event types in EventMesh subscription instance @@ -137,15 +136,12 @@ func ConvertKymaSubToEventMeshSub(subscription *eventingv1alpha2.Subscription, t eventMeshSubscription.WebhookURL = urlTobeRegistered // set webhook auth - eventMeshSubscription.WebhookAuth, err = getEventMeshWebhookAuth(subscription, defaultWebhookAuth) - if err != nil { - return nil, err - } + eventMeshSubscription.WebhookAuth = getEventMeshWebhookAuth(subscription, defaultWebhookAuth) return eventMeshSubscription, nil } -func setEventMeshProtocolSettings(subscription *eventingv1alpha2.Subscription, eventMeshSub *types.Subscription) error { +func setEventMeshProtocolSettings(subscription *eventingv1alpha2.Subscription, eventMeshSub *types.Subscription) { // Applying protocol settings if provided in subscription CR // qos if qosStr, ok := subscription.Spec.Config[eventingv1alpha2.ProtocolSettingsQos]; ok { @@ -163,20 +159,19 @@ func setEventMeshProtocolSettings(subscription *eventingv1alpha2.Subscription, e } eventMeshSub.ExemptHandshake = handshake } - return nil } // getEventMeshWebhookAuth uses default webhook auth unless specified in Subscription CR. func getEventMeshWebhookAuth(subscription *eventingv1alpha2.Subscription, defaultWebhookAuth *types.WebhookAuth, -) (*types.WebhookAuth, error) { +) *types.WebhookAuth { auth := &types.WebhookAuth{} // extract auth info from subscription CR if any if authType, ok := subscription.Spec.Config[eventingv1alpha2.WebhookAuthType]; ok { auth.Type = types.GetAuthType(authType) } else { // if auth type was not provided then use default webhook auth - return defaultWebhookAuth, nil + return defaultWebhookAuth } if grantType, ok := subscription.Spec.Config[eventingv1alpha2.WebhookAuthGrantType]; ok { @@ -195,7 +190,7 @@ func getEventMeshWebhookAuth(subscription *eventingv1alpha2.Subscription, auth.ClientSecret = clientSecret } - return auth, nil + return auth } func GetCleanedEventMeshSubscription(subscription *types.Subscription) *types.Subscription { diff --git a/pkg/backend/utils/eventmesh_utils_test.go b/pkg/backend/utils/eventmesh_utils_test.go index 587c843b..2ed0e609 100644 --- a/pkg/backend/utils/eventmesh_utils_test.go +++ b/pkg/backend/utils/eventmesh_utils_test.go @@ -222,10 +222,9 @@ func Test_setEventMeshProtocolSettings(t *testing.T) { eventMeshSubscription := tc.givenEventMeshSubscription // when - err := setEventMeshProtocolSettings(tc.givenSubscription, eventMeshSubscription) + setEventMeshProtocolSettings(tc.givenSubscription, eventMeshSubscription) // then - require.NoError(t, err) require.Equal(t, tc.wantEventMeshSubscription, eventMeshSubscription) }) } @@ -283,10 +282,9 @@ func Test_getEventMeshWebhookAuth(t *testing.T) { // given // when - webhookAuth, err := getEventMeshWebhookAuth(tc.givenSubscription, defaultWebhookAuth) + webhookAuth := getEventMeshWebhookAuth(tc.givenSubscription, defaultWebhookAuth) // then - require.NoError(t, err) require.Equal(t, tc.wantWebhook, webhookAuth) }) } @@ -327,7 +325,6 @@ func TestGetCleanedEventMeshSubscription(t *testing.T) { Type: eventingtesting.OrderCreatedEventTypeNotClean, }, })) - g.Expect(eventMeshSubscription) } func TestEventMeshSubscriptionNameMapper(t *testing.T) { @@ -417,7 +414,7 @@ func TestEventMeshSubscriptionNameMapper(t *testing.T) { g.Expect(strings.HasSuffix(s, test.outputHash)).To(BeTrue()) // and have the first 10 char of the name prefixLen := minFunc(len(test.inputSub.Name), test.maxLen-hashLength) - g.Expect(strings.HasPrefix(s, test.inputSub.Name[:prefixLen])) + g.Expect(s).To(HavePrefix(test.inputSub.Name[:prefixLen])) } // Same domain and subscription name/namespace should map to the same name diff --git a/pkg/ems/auth/auth.go b/pkg/ems/auth/auth.go index e56ff3b4..805eb378 100644 --- a/pkg/ems/auth/auth.go +++ b/pkg/ems/auth/auth.go @@ -18,6 +18,5 @@ func NewAuthenticatedClient(cfg env.Config) *http.Client { base := http.DefaultTransport.(*http.Transport).Clone() client.Transport.(*oauth2.Transport).Base = base - // TODO: Support tracing in eventing-controller #9767: https://github.com/kyma-project/kyma/issues/9767 return client } diff --git a/pkg/ems/httpclient/client.go b/pkg/ems/httpclient/client.go index 824f6169..dbee9b77 100644 --- a/pkg/ems/httpclient/client.go +++ b/pkg/ems/httpclient/client.go @@ -2,6 +2,7 @@ package httpclient import ( "bytes" + "context" "encoding/json" "fmt" "io" @@ -37,7 +38,7 @@ func NewHTTPClient(baseURL string, client *http.Client) (*Client, error) { // add trailing '/' to the url path, so that we can combine the url with other paths according to standards if !strings.HasSuffix(u.Path, "/") { - u.Path = u.Path + "/" + u.Path += "/" } if err != nil { return nil, err @@ -66,7 +67,8 @@ func (c Client) NewRequest(method, path string, body interface{}) (*http.Request return nil, NewError(err) } u := resolveReferenceAsRelative(c.baseURL, pu) - req, err := http.NewRequest(method, u.String(), jsonBody) + ctx := context.Background() + req, err := http.NewRequestWithContext(ctx, method, u.String(), jsonBody) if err != nil { return nil, NewError(err) } diff --git a/pkg/env/config_test.go b/pkg/env/config_test.go index 93086f5d..bee63a77 100644 --- a/pkg/env/config_test.go +++ b/pkg/env/config_test.go @@ -37,5 +37,5 @@ func Test_GetConfig(t *testing.T) { webhookActivationTimeout, err := time.ParseDuration(envs["WEBHOOK_ACTIVATION_TIMEOUT"]) g.Expect(err).ShouldNot(HaveOccurred()) g.Expect(config.WebhookActivationTimeout).To(Equal(webhookActivationTimeout)) - g.Expect(config.NATSProvisioningEnabled).To(Equal(true)) + g.Expect(config.NATSProvisioningEnabled).To(BeTrue()) } diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index 51d74aa0..7925aec1 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -20,13 +20,13 @@ func MakeError(actualError, underlyingError error) error { // MakeSubscriptionError creates a new error and includes the underlyingError in the message // for subscription-related errors. func MakeSubscriptionError(actualError, underlyingError error, subscription any) error { - return fmt.Errorf("%w: %v, subscription: %v", actualError, underlyingError, subscription) + return fmt.Errorf("%w: %w, subscription: %v", actualError, underlyingError, subscription) } // MakeConsumerError creates a new error and includes the underlyingError in the message // for consumer-related errors. func MakeConsumerError(actualError, underlyingError error, consumer any) error { - return fmt.Errorf("%w: %v, consumer: %v", actualError, underlyingError, consumer) + return fmt.Errorf("%w: %w, consumer: %v", actualError, underlyingError, consumer) } // ArgumentError is a generic error which can be used to create errors that shall include any kind of argument diff --git a/pkg/errors/errors_unit_test.go b/pkg/errors/errors_unit_test.go index b420ef94..96ea31d8 100644 --- a/pkg/errors/errors_unit_test.go +++ b/pkg/errors/errors_unit_test.go @@ -49,7 +49,7 @@ func Test_ArgumentError_Is(t *testing.T) { name: "with argument and wrapped", givenError: func() error { e := errInvalidStorageType.WithArg(givenStorageType) - return fmt.Errorf("%v: %w", errors.New("new error"), e) + return fmt.Errorf("%w: %w", errors.New("new error"), e) }, wantIsTrue: true, }, diff --git a/pkg/eventing/deployment.go b/pkg/eventing/deployment.go index ef6a2ad3..7b89163b 100644 --- a/pkg/eventing/deployment.go +++ b/pkg/eventing/deployment.go @@ -35,11 +35,10 @@ const ( PublisherSecretEMSURLKey = "ems-publish-url" PublisherSecretBEBNamespaceKey = "beb-namespace" - PriorityClassName = "eventing-manager-priority-class" + PriorityClassName = "eventing-manager-priority-class" + TerminationGracePeriodSeconds = int64(30) ) -var TerminationGracePeriodSeconds = int64(30) - func newNATSPublisherDeployment( eventing *v1alpha1.Eventing, natsConfig env.NATSConfig, @@ -77,6 +76,7 @@ func newEventMeshPublisherDeployment( type DeployOpt func(deployment *kappsv1.Deployment) func newDeployment(eventing *v1alpha1.Eventing, publisherConfig env.PublisherConfig, opts ...DeployOpt) *kappsv1.Deployment { + trmGrcPrd := TerminationGracePeriodSeconds newDeployment := &kappsv1.Deployment{ TypeMeta: kmetav1.TypeMeta{ Kind: "Deployment", @@ -94,7 +94,7 @@ func newDeployment(eventing *v1alpha1.Eventing, publisherConfig env.PublisherCon Spec: kcorev1.PodSpec{ RestartPolicy: kcorev1.RestartPolicyAlways, ServiceAccountName: GetPublisherServiceAccountName(*eventing), - TerminationGracePeriodSeconds: &TerminationGracePeriodSeconds, + TerminationGracePeriodSeconds: &trmGrcPrd, PriorityClassName: publisherConfig.PriorityClassName, SecurityContext: getPodSecurityContext(), }, @@ -165,11 +165,12 @@ func WithPriorityClassName(name string) DeployOpt { func WithAffinity(publisherName string) DeployOpt { return func(d *kappsv1.Deployment) { + const affinityWeight = 100 d.Spec.Template.Spec.Affinity = &kcorev1.Affinity{ PodAntiAffinity: &kcorev1.PodAntiAffinity{ PreferredDuringSchedulingIgnoredDuringExecution: []kcorev1.WeightedPodAffinityTerm{ { - Weight: 100, + Weight: affinityWeight, PodAffinityTerm: kcorev1.PodAffinityTerm{ LabelSelector: &kmetav1.LabelSelector{ MatchLabels: map[string]string{label.KeyName: publisherName}, @@ -266,32 +267,43 @@ func getContainerSecurityContext() *kcorev1.SecurityContext { } func getReadinessProbe() *kcorev1.Probe { + const ( + readyPath = "/readyz" + readyPort = 8080 + maxFailure = 3 + ) return &kcorev1.Probe{ ProbeHandler: kcorev1.ProbeHandler{ HTTPGet: &kcorev1.HTTPGetAction{ - Path: "/readyz", - Port: intstr.FromInt32(8080), + Path: readyPath, + Port: intstr.FromInt32(readyPort), Scheme: kcorev1.URISchemeHTTP, }, }, - FailureThreshold: 3, + FailureThreshold: maxFailure, } } func getLivenessProbe() *kcorev1.Probe { + const ( + healthPath = "/healthz" + healthPort = 8080 + minSuccess = 1 + maxError = 3 + ) return &kcorev1.Probe{ ProbeHandler: kcorev1.ProbeHandler{ HTTPGet: &kcorev1.HTTPGetAction{ - Path: "/healthz", - Port: intstr.FromInt32(8080), + Path: healthPath, + Port: intstr.FromInt32(healthPort), Scheme: kcorev1.URISchemeHTTP, }, }, InitialDelaySeconds: livenessInitialDelaySecs, TimeoutSeconds: livenessTimeoutSecs, PeriodSeconds: livenessPeriodSecs, - SuccessThreshold: 1, - FailureThreshold: 3, + SuccessThreshold: minSuccess, + FailureThreshold: maxError, } } diff --git a/pkg/eventing/deployment_test.go b/pkg/eventing/deployment_test.go index a134237a..5ff31c50 100644 --- a/pkg/eventing/deployment_test.go +++ b/pkg/eventing/deployment_test.go @@ -289,6 +289,7 @@ func Test_GetEventMeshEnvVars(t *testing.T) { // natsBackendAssertions checks that the NATS-specific data was set in the NewNATSPublisherDeployment. func natsBackendAssertions(t *testing.T, publisherName string, deployment kappsv1.Deployment) { + t.Helper() container := findPublisherContainer(publisherName, deployment) assert.NotNil(t, container) @@ -308,6 +309,7 @@ func natsBackendAssertions(t *testing.T, publisherName string, deployment kappsv // eventMeshBackendAssertions checks that the eventmesh-specific data was set in the NewEventMeshPublisherDeployment. func eventMeshBackendAssertions(t *testing.T, publisherName string, deployment kappsv1.Deployment) { + t.Helper() container := findPublisherContainer(publisherName, deployment) assert.NotNil(t, container) diff --git a/pkg/eventing/manager.go b/pkg/eventing/manager.go index 65397e0a..948d483a 100644 --- a/pkg/eventing/manager.go +++ b/pkg/eventing/manager.go @@ -26,8 +26,10 @@ const ( // allowedAnnotations are the publisher proxy deployment spec template annotations // which should be preserved during reconciliation. -var allowedAnnotations = map[string]string{ - "kubectl.kubernetes.io/restartedAt": "", +func allowedAnnotations() map[string]string { + return map[string]string{ + "kubectl.kubernetes.io/restartedAt": "", + } } var ( @@ -119,7 +121,7 @@ func (em *EventingManager) applyPublisherProxyDeployment( // preserve only allowed annotations desiredPublisher.Spec.Template.ObjectMeta.Annotations = make(map[string]string) for k, v := range currentPublisher.Spec.Template.ObjectMeta.Annotations { - if _, ok := allowedAnnotations[k]; ok { + if _, ok := allowedAnnotations()[k]; ok { desiredPublisher.Spec.Template.ObjectMeta.Annotations[k] = v } } diff --git a/pkg/eventing/utils.go b/pkg/eventing/utils.go index 4d8a347f..2722c304 100644 --- a/pkg/eventing/utils.go +++ b/pkg/eventing/utils.go @@ -158,9 +158,11 @@ func newPublisherProxyClusterRoleBinding(name, namespace string, labels map[stri } } -func newPublisherProxyService(name, namespace string, labels map[string]string, - selectorLabels map[string]string, -) *kcorev1.Service { +func newPublisherProxyService(name, namespace string, labels map[string]string, selectorLabels map[string]string) *kcorev1.Service { + const ( + srcPort = 80 + targetPort = 8080 + ) // setting `TypeMeta` is important for patch apply to work. return &kcorev1.Service{ TypeMeta: kmetav1.TypeMeta{ @@ -178,17 +180,19 @@ func newPublisherProxyService(name, namespace string, labels map[string]string, { Name: "http-client", Protocol: "TCP", - Port: 80, - TargetPort: intstr.FromInt(8080), + Port: srcPort, + TargetPort: intstr.FromInt(targetPort), }, }, }, } } -func newPublisherProxyMetricsService(name, namespace string, labels map[string]string, - selectorLabels map[string]string, -) *kcorev1.Service { +func newPublisherProxyMetricsService(name, namespace string, labels map[string]string, selectorLabels map[string]string) *kcorev1.Service { + const ( + srcPort = 80 + trgtPort = 9090 + ) // setting `TypeMeta` is important for patch apply to work. return &kcorev1.Service{ TypeMeta: kmetav1.TypeMeta{ @@ -211,17 +215,18 @@ func newPublisherProxyMetricsService(name, namespace string, labels map[string]s { Name: "http-metrics", Protocol: "TCP", - Port: 80, - TargetPort: intstr.FromInt(9090), + Port: srcPort, + TargetPort: intstr.FromInt(trgtPort), }, }, }, } } -func newPublisherProxyHealthService(name, namespace string, labels map[string]string, - selectorLabels map[string]string, -) *kcorev1.Service { +func newPublisherProxyHealthService(name, namespace string, labels map[string]string, selectorLabels map[string]string) *kcorev1.Service { + const ( + port = 15020 + ) // setting `TypeMeta` is important for patch apply to work. return &kcorev1.Service{ TypeMeta: kmetav1.TypeMeta{ @@ -239,8 +244,8 @@ func newPublisherProxyHealthService(name, namespace string, labels map[string]st { Name: "http-status", Protocol: "TCP", - Port: 15020, - TargetPort: intstr.FromInt(15020), + Port: port, + TargetPort: intstr.FromInt32(port), }, }, }, diff --git a/pkg/informers/sync.go b/pkg/informers/sync.go index e59fc07d..ed8420fa 100644 --- a/pkg/informers/sync.go +++ b/pkg/informers/sync.go @@ -20,12 +20,13 @@ func WaitForCacheSyncOrDie(ctx context.Context, dc dynamicinformer.DynamicShared dc.Start(ctx.Done()) ctx, cancel := context.WithTimeout(context.Background(), DefaultResyncPeriod) - defer cancel() err := hasSynced(ctx, dc.WaitForCacheSync) if err != nil { log.Fatalf("Failed to sync informer caches: %v", err) } + // no need to defer cancel, as we either end the program or cancel here + cancel() } func hasSynced(ctx context.Context, fn waitForCacheSyncFunc) error { diff --git a/pkg/k8s/client.go b/pkg/k8s/client.go index 08fef405..813c4332 100644 --- a/pkg/k8s/client.go +++ b/pkg/k8s/client.go @@ -24,10 +24,12 @@ import ( eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" ) -var NatsGVK = schema.GroupVersionResource{ - Group: natsv1alpha1.GroupVersion.Group, - Version: natsv1alpha1.GroupVersion.Version, - Resource: "nats", +func NatsGVK() schema.GroupVersionResource { + return schema.GroupVersionResource{ + Group: natsv1alpha1.GroupVersion.Group, + Version: natsv1alpha1.GroupVersion.Version, + Resource: "nats", + } } var ErrSecretRefInvalid = errors.New("invalid namespaced name. It must be in the format of 'namespace/name'") @@ -158,7 +160,7 @@ func (c *KubeClient) DeleteResource(ctx context.Context, object client.Object) e } func (c *KubeClient) GetNATSResources(ctx context.Context, namespace string) (*natsv1alpha1.NATSList, error) { - unstructuredList, err := c.dynamicClient.Resource(NatsGVK).Namespace(namespace).List(ctx, kmetav1.ListOptions{}) + unstructuredList, err := c.dynamicClient.Resource(NatsGVK()).Namespace(namespace).List(ctx, kmetav1.ListOptions{}) if err != nil { return nil, err } @@ -194,8 +196,9 @@ func (c *KubeClient) PatchApply(ctx context.Context, object client.Object) error // GetSecret returns the secret with the given namespaced name. // namespacedName is in the format of "namespace/name". func (c *KubeClient) GetSecret(ctx context.Context, namespacedName string) (*kcorev1.Secret, error) { + const nameNamespace = 2 substrings := strings.Split(namespacedName, "/") - if len(substrings) != 2 { + if len(substrings) != nameNamespace { return nil, ErrSecretRefInvalid } secret := &kcorev1.Secret{} diff --git a/pkg/object/equality.go b/pkg/object/equality.go index 01fff25d..506934d5 100644 --- a/pkg/object/equality.go +++ b/pkg/object/equality.go @@ -16,6 +16,8 @@ import ( // Semantic can do semantic deep equality checks for API objects. Fields which // are not relevant for the reconciliation logic are intentionally omitted. +// +//nolint:gochecknoglobals // same pattern as in apimachinery var Semantic = conversion.EqualitiesOrDie( apiRuleEqual, publisherProxyDeploymentEqual, diff --git a/pkg/signals/signals.go b/pkg/signals/signals.go index 4f84f343..e53a9232 100644 --- a/pkg/signals/signals.go +++ b/pkg/signals/signals.go @@ -11,14 +11,17 @@ import ( var ( // onlyOneSignalHandler to make sure that only one signal handler is registered. + //nolint:gochecknoglobals // needs to be global so that the application is always closed on second signal onlyOneSignalHandler = make(chan struct{}) - // shutdownSignals array of system signals to cause shutdown. - shutdownSignals = []os.Signal{syscall.SIGINT, syscall.SIGTERM} - ErrTerminationRequested = errors.New("received a termination signal") ) +// shutdownSignals array of system signals to cause shutdown. +func shutdownSignals() []os.Signal { + return []os.Signal{syscall.SIGINT, syscall.SIGTERM} +} + // SetupSignalHandler registered for SIGTERM and SIGINT. A stop channel is returned // which is closed on one of these signals. If a second signal is caught, the program // is terminated with exit code 1. @@ -29,9 +32,10 @@ func SetupSignalHandler() <-chan struct{} { } func setupStopChannel() <-chan struct{} { + const chanSize = 2 // we want to be able to read twice stop := make(chan struct{}) - osSignal := make(chan os.Signal, 2) - signal.Notify(osSignal, shutdownSignals...) + osSignal := make(chan os.Signal, chanSize) + signal.Notify(osSignal, shutdownSignals()...) go func() { <-osSignal close(stop) diff --git a/pkg/subscriptionmanager/eventmesh/eventmesh_test.go b/pkg/subscriptionmanager/eventmesh/eventmesh_test.go index 0b9f2ceb..4452d579 100644 --- a/pkg/subscriptionmanager/eventmesh/eventmesh_test.go +++ b/pkg/subscriptionmanager/eventmesh/eventmesh_test.go @@ -111,7 +111,9 @@ func Test_cleanupEventMesh(t *testing.T) { getSubscriptionURL := fmt.Sprintf(client.GetURLFormat, nameMapper.MapSubscriptionName(subscription.Name, subscription.Namespace)) getSubscriptionURL = bebMock.MessagingURL + getSubscriptionURL - resp, err := http.Get(getSubscriptionURL) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, getSubscriptionURL, nil) + require.NoError(t, err) + resp, err := http.DefaultClient.Do(req) require.NoError(t, err) defer resp.Body.Close() require.Equal(t, http.StatusOK, resp.StatusCode) @@ -135,7 +137,10 @@ func Test_cleanupEventMesh(t *testing.T) { // then // the BEB subscription should be deleted from BEB Mock - resp, err = http.Get(getSubscriptionURL) + req, err = http.NewRequestWithContext(ctx, http.MethodGet, getSubscriptionURL, nil) + require.NoError(t, err) + resp, err = http.DefaultClient.Do(req) + require.NoError(t, err) require.NoError(t, err) defer resp.Body.Close() require.Equal(t, http.StatusNotFound, resp.StatusCode) @@ -216,9 +221,6 @@ func Test_markAllV1Alpha2SubscriptionsAsNotReady(t *testing.T) { } func startBEBMock() *eventingtesting.EventMeshMock { - // TODO(k15r): FIX THIS HACK - // this is a very evil hack for the time being, until we refactored the config properly - // it sets the URLs to relative paths, that can easily be used in the mux. b := eventingtesting.NewEventMeshMock() b.Start() return b diff --git a/pkg/subscriptionmanager/jetstream/jetstream_test.go b/pkg/subscriptionmanager/jetstream/jetstream_test.go index 825d8bb5..27b2880c 100644 --- a/pkg/subscriptionmanager/jetstream/jetstream_test.go +++ b/pkg/subscriptionmanager/jetstream/jetstream_test.go @@ -55,6 +55,7 @@ type TestEnvironment struct { } func getJetStreamClient(t *testing.T, natsURL string) nats.JetStreamContext { + t.Helper() conn, err := nats.Connect(natsURL) require.NoError(t, err) jsClient, err := conn.JetStream() @@ -77,9 +78,8 @@ func getNATSConf(natsURL string, natsPort int) env.NATSConfig { } } -func createAndSyncSubscription(t *testing.T, sinkURL string, - jsBackend *jetstream.JetStream, -) *eventingv1alpha2.Subscription { +func createAndSyncSubscription(t *testing.T, sinkURL string, jsBackend *jetstream.JetStream) *eventingv1alpha2.Subscription { + t.Helper() // create test subscription testSub := eventingtesting.NewSubscription( subscriptionName, subscriptionNamespace, @@ -101,6 +101,7 @@ func createAndSyncSubscription(t *testing.T, sinkURL string, } func (te *TestEnvironment) consumersEquals(t *testing.T, length int) { + t.Helper() // verify that the number of consumers is one info, err := te.jsCtx.StreamInfo(te.envConf.JSStreamName) require.NoError(t, err) @@ -108,6 +109,7 @@ func (te *TestEnvironment) consumersEquals(t *testing.T, length int) { } func setUpTestEnvironment(t *testing.T) *TestEnvironment { + t.Helper() // create a test subscriber and natsServer subscriber := eventingtesting.NewSubscriber() // create NATS Server with JetStream enabled diff --git a/pkg/tracing/tracing_test.go b/pkg/tracing/tracing_test.go index 69c1471b..7166c293 100644 --- a/pkg/tracing/tracing_test.go +++ b/pkg/tracing/tracing_test.go @@ -64,7 +64,7 @@ func TestAddTracingHeadersToContext(t *testing.T) { ctx := context.Background() gotContext := AddTracingHeadersToContext(ctx, tc.event) g.Expect(cehttp.HeaderFrom(gotContext)).To(Equal(tc.expectedHeaders)) - g.Expect(len(getTracingExtensions(tc.event))).To(Equal(0)) + g.Expect(getTracingExtensions(tc.event)).To(BeEmpty()) }) } } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 97bfd872..9d7c6c83 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -21,6 +21,10 @@ var ErrParseSink = errors.Errorf("failed to parse subscription sink URL") // GetPortNumberFromURL converts string port from url.URL to uint32 port. func GetPortNumberFromURL(u url.URL) (uint32, error) { + const ( + httpPort = 80 + httpsPort = 443 + ) port := uint32(0) sinkPort := u.Port() if sinkPort != "" { @@ -33,9 +37,9 @@ func GetPortNumberFromURL(u url.URL) (uint32, error) { if port == uint32(0) { switch strings.ToLower(u.Scheme) { case "https": - port = uint32(443) + port = uint32(httpsPort) default: - port = uint32(80) + port = uint32(httpPort) } } return port, nil diff --git a/pkg/watcher/watcher.go b/pkg/watcher/watcher.go index 65aa04a6..7e7feed5 100644 --- a/pkg/watcher/watcher.go +++ b/pkg/watcher/watcher.go @@ -14,8 +14,9 @@ import ( ) func NewResourceWatcher(client dynamic.Interface, gvk schema.GroupVersionResource, namespace string) *ResourceWatcher { + const resyncTime = 30 * time.Second dynamicInformerFactory := dynamicinformer.NewFilteredDynamicSharedInformerFactory( - client, 30*time.Second, namespace, nil) + client, resyncTime, namespace, nil) dynamicInformer := dynamicInformerFactory.ForResource(gvk).Informer() return &ResourceWatcher{ client: client, diff --git a/test/utils/integration/integration.go b/test/utils/integration/integration.go index 490bf840..6a9a3f31 100644 --- a/test/utils/integration/integration.go +++ b/test/utils/integration/integration.go @@ -7,6 +7,7 @@ import ( "fmt" "log" "path/filepath" + "strconv" "strings" "testing" "time" @@ -57,7 +58,7 @@ import ( const ( useExistingCluster = false - attachControlPlaneOutput = false + attachControlPlaneOutput = true testEnvStartDelay = time.Minute testEnvStartAttempts = 10 BigPollingInterval = 3 * time.Second @@ -108,7 +109,7 @@ func NewTestEnvironment(config TestEnvironmentConfig) (*TestEnvironment, error) // Set controller core logger. kctrl.SetLogger(zapr.NewLogger(ctrLogger.WithContext().Desugar())) - testEnv, envTestKubeCfg, err := StartEnvTest(config) + testEnv, envTestKubeCfg, err := SetupAndStartEnvTest(config) if err != nil { return nil, err } @@ -226,7 +227,8 @@ func NewTestEnvironment(config TestEnvironmentConfig) (*TestEnvironment, error) } // create webhook cert secret. - newCABundle := make([]byte, 40) + const caBundleSize = 40 + newCABundle := make([]byte, caBundleSize) if _, err := rand.Read(newCABundle); err != nil { return nil, err } @@ -249,24 +251,23 @@ func NewTestEnvironment(config TestEnvironmentConfig) (*TestEnvironment, error) }, nil } -func StartEnvTest(config TestEnvironmentConfig) (*envtest.Environment, *rest.Config, error) { - // Reference: https://book.kubebuilder.io/reference/envtest.html - useExistingCluster := useExistingCluster - - dummyCABundle := make([]byte, 20) +func SetupAndStartEnvTest(config TestEnvironmentConfig) (*envtest.Environment, *rest.Config, error) { + const caBundleSize = 20 + dummyCABundle := make([]byte, caBundleSize) if _, err := rand.Read(dummyCABundle); err != nil { return nil, nil, err } url := "https://eventing-controller.kyma-system.svc.cluster.local" sideEffectClassNone := kadmissionregistrationv1.SideEffectClassNone + webhookClientConfig := kadmissionregistrationv1.WebhookClientConfig{ + URL: &url, + CABundle: dummyCABundle, + } mwh := getMutatingWebhookConfig([]kadmissionregistrationv1.MutatingWebhook{ { - Name: "reconciler.eventing.test", - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{ - URL: &url, - CABundle: dummyCABundle, - }, + Name: "reconciler.eventing.test", + ClientConfig: webhookClientConfig, SideEffects: &sideEffectClassNone, AdmissionReviewVersions: []string{"v1beta1"}, }, @@ -276,11 +277,8 @@ func StartEnvTest(config TestEnvironmentConfig) (*envtest.Environment, *rest.Con // setup dummy validating webhook vwh := getValidatingWebhookConfig([]kadmissionregistrationv1.ValidatingWebhook{ { - Name: "reconciler2.eventing.test", - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{ - URL: &url, - CABundle: dummyCABundle, - }, + Name: "reconciler.eventing.test", + ClientConfig: webhookClientConfig, SideEffects: &sideEffectClassNone, AdmissionReviewVersions: []string{"v1beta1"}, }, @@ -304,11 +302,13 @@ func StartEnvTest(config TestEnvironmentConfig) (*envtest.Environment, *rest.Con filepath.Join(config.ProjectRootDir, "config", "crd", "for-tests", "operator.kyma-project.io_nats.yaml")) } + // Reference: https://book.kubebuilder.io/reference/envtest.html + uec := useExistingCluster testEnv := &envtest.Environment{ CRDDirectoryPaths: includedCRDs, ErrorIfCRDPathMissing: true, AttachControlPlaneOutput: attachControlPlaneOutput, - UseExistingCluster: &useExistingCluster, + UseExistingCluster: &uec, WebhookInstallOptions: envtest.WebhookInstallOptions{ MutatingWebhooks: []*kadmissionregistrationv1.MutatingWebhookConfiguration{mwh}, ValidatingWebhooks: []*kadmissionregistrationv1.ValidatingWebhookConfiguration{vwh}, @@ -316,12 +316,13 @@ func StartEnvTest(config TestEnvironmentConfig) (*envtest.Environment, *rest.Con } args := testEnv.ControlPlane.GetAPIServer().Configure() - if config.CELValidationEnabled { - args.Set("feature-gates", "CustomResourceValidationExpressions=true") - } else { - args.Set("feature-gates", "CustomResourceValidationExpressions=false") - } + args.Set("feature-gates", fmt.Sprintf("CustomResourceValidationExpressions=%s", strconv.FormatBool(config.CELValidationEnabled))) + + cfg, err := StartEnvTestWithRetry(testEnv) + return testEnv, cfg, err +} +func StartEnvTestWithRetry(testEnv *envtest.Environment) (*rest.Config, error) { var cfg *rest.Config err := retry.Do(func() error { defer func() { @@ -343,7 +344,7 @@ func StartEnvTest(config TestEnvironmentConfig) (*envtest.Environment, *rest.Con } }), ) - return testEnv, cfg, err + return cfg, err } func (env TestEnvironment) TearDown() error { @@ -384,6 +385,7 @@ func (env TestEnvironment) GetEventingAssert(g *gomega.GomegaWithT, } func (env TestEnvironment) EnsureNamespaceCreation(t *testing.T, namespace string) { + t.Helper() if namespace == "default" { return } @@ -397,10 +399,12 @@ func (env TestEnvironment) CreateK8sResource(obj client.Object) error { } func (env TestEnvironment) EnsureK8sResourceCreated(t *testing.T, obj client.Object) { + t.Helper() require.NoError(t, env.k8sClient.Create(context.Background(), obj)) } func (env TestEnvironment) EnsureEPPK8sResourcesExists(t *testing.T, eventingCR v1alpha1.Eventing) { + t.Helper() env.EnsureK8sServiceExists(t, eventing.GetPublisherPublishServiceName(eventingCR), eventingCR.Namespace) env.EnsureK8sServiceExists(t, @@ -416,6 +420,7 @@ func (env TestEnvironment) EnsureEPPK8sResourcesExists(t *testing.T, eventingCR } func (env TestEnvironment) EnsureEPPK8sResourcesHaveOwnerReference(t *testing.T, eventingCR v1alpha1.Eventing) { + t.Helper() env.EnsureEPPPublishServiceOwnerReferenceSet(t, eventingCR) env.EnsureEPPMetricsServiceOwnerReferenceSet(t, eventingCR) env.EnsureEPPHealthServiceOwnerReferenceSet(t, eventingCR) @@ -423,6 +428,7 @@ func (env TestEnvironment) EnsureEPPK8sResourcesHaveOwnerReference(t *testing.T, } func (env TestEnvironment) EnsureDeploymentExists(t *testing.T, name, namespace string) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetDeploymentFromK8s(name, namespace) return err == nil && result != nil @@ -430,6 +436,7 @@ func (env TestEnvironment) EnsureDeploymentExists(t *testing.T, name, namespace } func (env TestEnvironment) EnsureK8sServiceExists(t *testing.T, name, namespace string) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetServiceFromK8s(name, namespace) return err == nil && result != nil @@ -437,6 +444,7 @@ func (env TestEnvironment) EnsureK8sServiceExists(t *testing.T, name, namespace } func (env TestEnvironment) EnsureK8sServiceAccountExists(t *testing.T, name, namespace string) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetServiceAccountFromK8s(name, namespace) return err == nil && result != nil @@ -444,6 +452,7 @@ func (env TestEnvironment) EnsureK8sServiceAccountExists(t *testing.T, name, nam } func (env TestEnvironment) EnsureK8sClusterRoleExists(t *testing.T, name, namespace string) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetClusterRoleFromK8s(name, namespace) return err == nil && result != nil @@ -451,6 +460,7 @@ func (env TestEnvironment) EnsureK8sClusterRoleExists(t *testing.T, name, namesp } func (env TestEnvironment) EnsureK8sClusterRoleBindingExists(t *testing.T, name, namespace string) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetClusterRoleBindingFromK8s(name, namespace) return err == nil && result != nil @@ -458,6 +468,7 @@ func (env TestEnvironment) EnsureK8sClusterRoleBindingExists(t *testing.T, name, } func (env TestEnvironment) EnsureHPAExists(t *testing.T, name, namespace string) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetHPAFromK8s(name, namespace) return err == nil && result != nil @@ -465,6 +476,7 @@ func (env TestEnvironment) EnsureHPAExists(t *testing.T, name, namespace string) } func (env TestEnvironment) EnsureSubscriptionExists(t *testing.T, name, namespace string) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetSubscriptionFromK8s(name, namespace) return err == nil && result != nil @@ -472,21 +484,24 @@ func (env TestEnvironment) EnsureSubscriptionExists(t *testing.T, name, namespac } func (env TestEnvironment) EnsureK8sResourceUpdated(t *testing.T, obj client.Object) { + t.Helper() require.NoError(t, env.k8sClient.Update(context.Background(), obj)) } func (env TestEnvironment) EnsureK8sResourceDeleted(t *testing.T, obj client.Object) { + t.Helper() require.NoError(t, env.k8sClient.Delete(context.Background(), obj)) } func (env TestEnvironment) EnsureNATSCRDDeleted(t *testing.T) { + t.Helper() crdManifest := &kapiextensionsv1.CustomResourceDefinition{ TypeMeta: kmetav1.TypeMeta{ APIVersion: "apiextensions.k8s.io/v1", Kind: "CustomResourceDefinition", }, ObjectMeta: kmetav1.ObjectMeta{ - Name: k8s.NatsGVK.GroupResource().String(), + Name: k8s.NatsGVK().GroupResource().String(), }, } require.NoError(t, env.k8sClient.Delete(context.Background(), crdManifest)) @@ -498,11 +513,13 @@ func (env TestEnvironment) EnsureNATSCRDDeleted(t *testing.T) { } func (env TestEnvironment) EnsureCRDCreated(t *testing.T, crd *kapiextensionsv1.CustomResourceDefinition) { + t.Helper() crd.ResourceVersion = "" require.NoError(t, env.k8sClient.Create(context.Background(), crd)) } func (env TestEnvironment) EnsureNamespaceDeleted(t *testing.T, namespace string) { + t.Helper() require.NoError(t, env.k8sClient.Delete(context.Background(), &kcorev1.Namespace{ ObjectMeta: kmetav1.ObjectMeta{ Name: namespace, @@ -511,6 +528,7 @@ func (env TestEnvironment) EnsureNamespaceDeleted(t *testing.T, namespace string } func (env TestEnvironment) EnsureDeploymentDeletion(t *testing.T, name, namespace string) { + t.Helper() deployment := &kappsv1.Deployment{ ObjectMeta: kmetav1.ObjectMeta{ Name: name, @@ -525,6 +543,7 @@ func (env TestEnvironment) EnsureDeploymentDeletion(t *testing.T, name, namespac } func (env TestEnvironment) EnsureDeploymentNotFound(t *testing.T, name, namespace string) { + t.Helper() require.Eventually(t, func() bool { _, err := env.GetDeploymentFromK8s(name, namespace) return err != nil && errors.IsNotFound(err) @@ -532,6 +551,7 @@ func (env TestEnvironment) EnsureDeploymentNotFound(t *testing.T, name, namespac } func (env TestEnvironment) EnsureK8sServiceNotFound(t *testing.T, name, namespace string) { + t.Helper() require.Eventually(t, func() bool { _, err := env.GetServiceFromK8s(name, namespace) return err != nil && errors.IsNotFound(err) @@ -539,6 +559,7 @@ func (env TestEnvironment) EnsureK8sServiceNotFound(t *testing.T, name, namespac } func (env TestEnvironment) EnsureK8sServiceAccountNotFound(t *testing.T, name, namespace string) { + t.Helper() require.Eventually(t, func() bool { _, err := env.GetServiceAccountFromK8s(name, namespace) return err != nil && errors.IsNotFound(err) @@ -546,6 +567,7 @@ func (env TestEnvironment) EnsureK8sServiceAccountNotFound(t *testing.T, name, n } func (env TestEnvironment) EnsureK8sClusterRoleNotFound(t *testing.T, name, namespace string) { + t.Helper() require.Eventually(t, func() bool { _, err := env.GetClusterRoleFromK8s(name, namespace) return err != nil && errors.IsNotFound(err) @@ -553,6 +575,7 @@ func (env TestEnvironment) EnsureK8sClusterRoleNotFound(t *testing.T, name, name } func (env TestEnvironment) EnsureK8sClusterRoleBindingNotFound(t *testing.T, name, namespace string) { + t.Helper() require.Eventually(t, func() bool { _, err := env.GetClusterRoleBindingFromK8s(name, namespace) return err != nil && errors.IsNotFound(err) @@ -560,6 +583,7 @@ func (env TestEnvironment) EnsureK8sClusterRoleBindingNotFound(t *testing.T, nam } func (env TestEnvironment) EnsureHPADeletion(t *testing.T, name, namespace string) { + t.Helper() hpa := &kautoscalingv1.HorizontalPodAutoscaler{ ObjectMeta: kmetav1.ObjectMeta{ Name: name, @@ -574,6 +598,7 @@ func (env TestEnvironment) EnsureHPADeletion(t *testing.T, name, namespace strin } func (env TestEnvironment) EnsureHPANotFound(t *testing.T, name, namespace string) { + t.Helper() require.Eventually(t, func() bool { _, err := env.GetHPAFromK8s(name, namespace) return err != nil && errors.IsNotFound(err) @@ -581,6 +606,7 @@ func (env TestEnvironment) EnsureHPANotFound(t *testing.T, name, namespace strin } func (env TestEnvironment) EnsureEventingResourceDeletion(t *testing.T, name, namespace string) { + t.Helper() eventing := &v1alpha1.Eventing{ ObjectMeta: kmetav1.ObjectMeta{ Name: name, @@ -595,6 +621,7 @@ func (env TestEnvironment) EnsureEventingResourceDeletion(t *testing.T, name, na } func (env TestEnvironment) EnsureEventingResourceDeletionStateError(t *testing.T, name, namespace string) { + t.Helper() eventing := &v1alpha1.Eventing{ ObjectMeta: kmetav1.ObjectMeta{ Name: name, @@ -609,6 +636,7 @@ func (env TestEnvironment) EnsureEventingResourceDeletionStateError(t *testing.T } func (env TestEnvironment) EnsureSubscriptionResourceDeletion(t *testing.T, name, namespace string) { + t.Helper() subscription := &eventingv1alpha2.Subscription{ ObjectMeta: kmetav1.ObjectMeta{ Name: name, @@ -622,23 +650,27 @@ func (env TestEnvironment) EnsureSubscriptionResourceDeletion(t *testing.T, name }, BigTimeOut, BigPollingInterval, "failed to ensure deletion of Subscription") } -func (env TestEnvironment) EnsureNATSResourceStateReady(t *testing.T, nats *natsv1alpha1.NATS) { - env.makeNATSCrReady(t, nats) +func (env TestEnvironment) EnsureNATSResourceState(t *testing.T, nats *natsv1alpha1.NATS, status string) { + t.Helper() + env.setNATSCRStatus(t, nats, status) require.Eventually(t, func() bool { err := env.k8sClient.Get(context.Background(), types.NamespacedName{Name: nats.Name, Namespace: nats.Namespace}, nats) - return err == nil && nats.Status.State == natsv1alpha1.StateReady + return err == nil && nats.Status.State == string(status) }, BigTimeOut, BigPollingInterval, "failed to ensure NATS CR is stored") } +func (env TestEnvironment) EnsureNATSResourceStateReady(t *testing.T, nats *natsv1alpha1.NATS) { + t.Helper() + env.EnsureNATSResourceState(t, nats, natsv1alpha1.StateReady) +} + func (env TestEnvironment) EnsureNATSResourceStateError(t *testing.T, nats *natsv1alpha1.NATS) { - env.makeNatsCrError(t, nats) - require.Eventually(t, func() bool { - err := env.k8sClient.Get(context.Background(), types.NamespacedName{Name: nats.Name, Namespace: nats.Namespace}, nats) - return err == nil && nats.Status.State == natsv1alpha1.StateError - }, BigTimeOut, BigPollingInterval, "failed to ensure NATS CR is stored") + t.Helper() + env.EnsureNATSResourceState(t, nats, natsv1alpha1.StateError) } func (env TestEnvironment) EnsureEventingSpecPublisherReflected(t *testing.T, eventingCR *v1alpha1.Eventing) { + t.Helper() require.Eventually(t, func() bool { deployment, err := env.GetDeploymentFromK8s(eventing.GetPublisherDeploymentName(*eventingCR), eventingCR.Namespace) if err != nil { @@ -657,6 +689,7 @@ func (env TestEnvironment) EnsureEventingSpecPublisherReflected(t *testing.T, ev } func (env TestEnvironment) EnsureEventingReplicasReflected(t *testing.T, eventingCR *v1alpha1.Eventing) { + t.Helper() require.Eventually(t, func() bool { hpa, err := env.GetHPAFromK8s(eventing.GetPublisherDeploymentName(*eventingCR), eventingCR.Namespace) if err != nil { @@ -668,6 +701,7 @@ func (env TestEnvironment) EnsureEventingReplicasReflected(t *testing.T, eventin } func (env TestEnvironment) EnsurePublisherDeploymentENVSet(t *testing.T, eventingCR *v1alpha1.Eventing) { + t.Helper() require.Eventually(t, func() bool { deployment, err := env.GetDeploymentFromK8s(eventing.GetPublisherDeploymentName(*eventingCR), eventingCR.Namespace) if err != nil { @@ -680,6 +714,7 @@ func (env TestEnvironment) EnsurePublisherDeploymentENVSet(t *testing.T, eventin } func (env TestEnvironment) EnsureDeploymentOwnerReferenceSet(t *testing.T, eventingCR *v1alpha1.Eventing) { + t.Helper() require.Eventually(t, func() bool { deployment, err := env.GetDeploymentFromK8s(eventing.GetPublisherDeploymentName(*eventingCR), eventingCR.Namespace) if err != nil { @@ -691,6 +726,7 @@ func (env TestEnvironment) EnsureDeploymentOwnerReferenceSet(t *testing.T, event } func (env TestEnvironment) EnsureEPPPublishServiceOwnerReferenceSet(t *testing.T, eventingCR v1alpha1.Eventing) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetServiceFromK8s(eventing.GetPublisherPublishServiceName(eventingCR), eventingCR.Namespace) if err != nil { @@ -702,6 +738,7 @@ func (env TestEnvironment) EnsureEPPPublishServiceOwnerReferenceSet(t *testing.T } func (env TestEnvironment) EnsureEPPMetricsServiceOwnerReferenceSet(t *testing.T, eventingCR v1alpha1.Eventing) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetServiceFromK8s(eventing.GetPublisherMetricsServiceName(eventingCR), eventingCR.Namespace) if err != nil { @@ -713,6 +750,7 @@ func (env TestEnvironment) EnsureEPPMetricsServiceOwnerReferenceSet(t *testing.T } func (env TestEnvironment) EnsureEPPHealthServiceOwnerReferenceSet(t *testing.T, eventingCR v1alpha1.Eventing) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetServiceFromK8s(eventing.GetPublisherHealthServiceName(eventingCR), eventingCR.Namespace) if err != nil { @@ -724,6 +762,7 @@ func (env TestEnvironment) EnsureEPPHealthServiceOwnerReferenceSet(t *testing.T, } func (env TestEnvironment) EnsureEPPServiceAccountOwnerReferenceSet(t *testing.T, eventingCR v1alpha1.Eventing) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetServiceAccountFromK8s(eventing.GetPublisherServiceAccountName(eventingCR), eventingCR.Namespace) if err != nil { @@ -735,6 +774,7 @@ func (env TestEnvironment) EnsureEPPServiceAccountOwnerReferenceSet(t *testing.T } func (env TestEnvironment) EnsureEPPClusterRoleOwnerReferenceSet(t *testing.T, eventingCR v1alpha1.Eventing) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetClusterRoleFromK8s(eventing.GetPublisherClusterRoleName(eventingCR), eventingCR.Namespace) if err != nil { @@ -746,6 +786,7 @@ func (env TestEnvironment) EnsureEPPClusterRoleOwnerReferenceSet(t *testing.T, e } func (env TestEnvironment) EnsureEPPClusterRoleBindingOwnerReferenceSet(t *testing.T, eventingCR v1alpha1.Eventing) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetClusterRoleBindingFromK8s(eventing.GetPublisherClusterRoleBindingName(eventingCR), eventingCR.Namespace) @@ -757,9 +798,8 @@ func (env TestEnvironment) EnsureEPPClusterRoleBindingOwnerReferenceSet(t *testi }, SmallTimeOut, SmallPollingInterval, "failed to ensure ClusterRoleBinding owner reference is set") } -func (env TestEnvironment) EnsureEPPPublishServiceCorrect(t *testing.T, eppDeployment *kappsv1.Deployment, - eventingCR v1alpha1.Eventing, -) { +func (env TestEnvironment) EnsureEPPPublishServiceCorrect(t *testing.T, eppDeployment *kappsv1.Deployment, eventingCR v1alpha1.Eventing) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetServiceFromK8s(eventing.GetPublisherPublishServiceName(eventingCR), eventingCR.Namespace) if err != nil { @@ -770,9 +810,8 @@ func (env TestEnvironment) EnsureEPPPublishServiceCorrect(t *testing.T, eppDeplo }, SmallTimeOut, SmallPollingInterval, "failed to ensure PublishService correctness.") } -func (env TestEnvironment) EnsureEPPMetricsServiceCorrect(t *testing.T, eppDeployment *kappsv1.Deployment, - eventingCR v1alpha1.Eventing, -) { +func (env TestEnvironment) EnsureEPPMetricsServiceCorrect(t *testing.T, eppDeployment *kappsv1.Deployment, eventingCR v1alpha1.Eventing) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetServiceFromK8s(eventing.GetPublisherMetricsServiceName(eventingCR), eventingCR.Namespace) if err != nil { @@ -783,9 +822,8 @@ func (env TestEnvironment) EnsureEPPMetricsServiceCorrect(t *testing.T, eppDeplo }, SmallTimeOut, SmallPollingInterval, "failed to ensure MetricsService correctness.") } -func (env TestEnvironment) EnsureEPPHealthServiceCorrect(t *testing.T, eppDeployment *kappsv1.Deployment, - eventingCR v1alpha1.Eventing, -) { +func (env TestEnvironment) EnsureEPPHealthServiceCorrect(t *testing.T, eppDeployment *kappsv1.Deployment, eventingCR v1alpha1.Eventing) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetServiceFromK8s(eventing.GetPublisherHealthServiceName(eventingCR), eventingCR.Namespace) if err != nil { @@ -797,6 +835,7 @@ func (env TestEnvironment) EnsureEPPHealthServiceCorrect(t *testing.T, eppDeploy } func (env TestEnvironment) EnsureEPPClusterRoleCorrect(t *testing.T, eventingCR v1alpha1.Eventing) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetClusterRoleFromK8s(eventing.GetPublisherClusterRoleName(eventingCR), eventingCR.Namespace) if err != nil { @@ -808,6 +847,7 @@ func (env TestEnvironment) EnsureEPPClusterRoleCorrect(t *testing.T, eventingCR } func (env TestEnvironment) EnsureEPPClusterRoleBindingCorrect(t *testing.T, eventingCR v1alpha1.Eventing) { + t.Helper() require.Eventually(t, func() bool { result, err := env.GetClusterRoleBindingFromK8s(eventing.GetPublisherClusterRoleBindingName(eventingCR), eventingCR.Namespace) @@ -820,6 +860,7 @@ func (env TestEnvironment) EnsureEPPClusterRoleBindingCorrect(t *testing.T, even } func (env TestEnvironment) EnsureCABundleInjectedIntoWebhooks(t *testing.T) { + t.Helper() require.Eventually(t, func() bool { // get cert secret from k8s. certSecret, err := env.GetSecretFromK8s(getTestBackendConfig().WebhookSecretName, @@ -863,23 +904,27 @@ func (env TestEnvironment) EnsureCABundleInjectedIntoWebhooks(t *testing.T) { } func (env TestEnvironment) EnsureEventMeshSecretCreated(t *testing.T, eventing *v1alpha1.Eventing) { + t.Helper() subarr := strings.Split(eventing.Spec.Backend.Config.EventMeshSecret, "/") secret := testutils.NewEventMeshSecret(subarr[1], subarr[0]) env.EnsureK8sResourceCreated(t, secret) } func (env TestEnvironment) EnsureEventMeshSecretDeleted(t *testing.T, eventing *v1alpha1.Eventing) { + t.Helper() subarr := strings.Split(eventing.Spec.Backend.Config.EventMeshSecret, "/") secret := testutils.NewEventMeshSecret(subarr[1], subarr[0]) env.EnsureK8sResourceDeleted(t, secret) } func (env TestEnvironment) EnsureOAuthSecretCreated(t *testing.T, eventing *v1alpha1.Eventing) { + t.Helper() secret := testutils.NewOAuthSecret("eventing-webhook-auth", eventing.Namespace) env.EnsureK8sResourceCreated(t, secret) } func (env TestEnvironment) EnsurePublishServiceInEventingStatus(t *testing.T, name, namespace string) { + t.Helper() eventingCR, err := env.GetEventingFromK8s(name, namespace) require.NoError(t, err) require.NotNil(t, eventingCR) @@ -961,22 +1006,10 @@ func (env TestEnvironment) UpdateNATSStatus(nats *natsv1alpha1.NATS) error { return env.k8sClient.Status().Update(context.Background(), baseNats) } -func (env TestEnvironment) makeNATSCrReady(t *testing.T, nats *natsv1alpha1.NATS) { - require.Eventually(t, func() bool { - nats.Status.State = natsv1alpha1.StateReady - - err := env.UpdateNATSStatus(nats) - if err != nil { - env.Logger.WithContext().Errorw("failed to update NATS CR status", "error", err) - return false - } - return true - }, BigTimeOut, BigPollingInterval, "failed to update status of NATS CR") -} - -func (env TestEnvironment) makeNatsCrError(t *testing.T, nats *natsv1alpha1.NATS) { +func (env TestEnvironment) setNATSCRStatus(t *testing.T, nats *natsv1alpha1.NATS, status string) { + t.Helper() require.Eventually(t, func() bool { - nats.Status.State = natsv1alpha1.StateError + nats.Status.State = status err := env.UpdateNATSStatus(nats) if err != nil { @@ -1168,5 +1201,6 @@ func (env TestEnvironment) UpdateUnstructuredK8sResource(obj *unstructured.Unstr } func (env TestEnvironment) EnsureK8sUnStructResourceCreated(t *testing.T, obj *unstructured.Unstructured) { + t.Helper() require.NoError(t, env.k8sClient.Create(context.Background(), obj)) } diff --git a/test/utils/utils.go b/test/utils/utils.go index 93f58ee9..d67b34b7 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -1,3 +1,4 @@ +//nolint:gomnd // magic numbers here are used only in context of the function package utils import ( diff --git a/testing/eventmeshmock.go b/testing/eventmeshmock.go index 99146735..2302fa34 100644 --- a/testing/eventmeshmock.go +++ b/testing/eventmeshmock.go @@ -111,20 +111,58 @@ func (m *EventMeshMock) Start() string { mux := http.NewServeMux() // oauth2 request - mux.HandleFunc(TokenURLPath, func(w http.ResponseWriter, r *http.Request) { - // TODO(k15r): method not allowed/implementd handling + mux.HandleFunc(TokenURLPath, m.handleToken()) + mux.HandleFunc(client.ListURL, m.handleList()) + mux.HandleFunc(MessagingURLPath+"/", m.handleMessaging()) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + m.log.V(1).Info(r.RequestURI) + }) + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + defer GinkgoRecover() + + // store request + m.Requests.StoreRequest(r) + + description := "" + reqBytes, err := httputil.DumpRequest(r, true) + if err == nil { + description = string(reqBytes) + } + m.log.V(1).Info("received request", + "uri", r.RequestURI, + "method", r.Method, + "description", description, + ) + + w.Header().Set("Content-Type", "application/json") + mux.ServeHTTP(w, r) + })) + uri := ts.URL + m.server = ts + m.MessagingURL = m.server.URL + MessagingURLPath + m.TokenURL = m.server.URL + TokenURLPath + return uri +} + +func (m *EventMeshMock) handleToken() func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { if r.Method == http.MethodPost { m.AuthResponse(w) } - }) + } +} - mux.HandleFunc(client.ListURL, func(w http.ResponseWriter, r *http.Request) { +func (m *EventMeshMock) handleList() func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { if r.Method == http.MethodGet { m.ListResponse(w) } - }) + } +} - mux.HandleFunc(MessagingURLPath+"/", func(w http.ResponseWriter, r *http.Request) { +func (m *EventMeshMock) handleMessaging() func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { switch r.Method { case http.MethodDelete: key := r.URL.Path @@ -179,37 +217,7 @@ func (m *EventMeshMock) Start() string { default: w.WriteHeader(http.StatusNotImplemented) } - }) - - mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - m.log.V(1).Info(r.RequestURI) - }) - - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - defer GinkgoRecover() - - // store request - m.Requests.StoreRequest(r) - - description := "" - reqBytes, err := httputil.DumpRequest(r, true) - if err == nil { - description = string(reqBytes) - } - m.log.V(1).Info("received request", - "uri", r.RequestURI, - "method", r.Method, - "description", description, - ) - - w.Header().Set("Content-Type", "application/json") - mux.ServeHTTP(w, r) - })) - uri := ts.URL - m.server = ts - m.MessagingURL = m.server.URL + MessagingURLPath - m.TokenURL = m.server.URL + TokenURLPath - return uri + } } func (m *EventMeshMock) Stop() { diff --git a/testing/subscriber.go b/testing/subscriber.go index d6919646..69e2cb73 100644 --- a/testing/subscriber.go +++ b/testing/subscriber.go @@ -1,6 +1,7 @@ package testing import ( + "context" "fmt" "io" "log" @@ -24,6 +25,7 @@ const ( checkEndpoint = "/check" internalErrorEndpoint = "/return500" checkRetriesEndpoint = "/check_retries" + notFoundTimeout = 500 * time.Millisecond ) var ( @@ -104,7 +106,7 @@ func getCloudEventServeMux() *http.ServeMux { select { case m := <-store: msg = m - case <-time.After(500 * time.Millisecond): + case <-time.After(notFoundTimeout): msg = "" } _, err := w.Write([]byte(msg)) @@ -160,7 +162,7 @@ func getDataServeMux() *http.ServeMux { select { case m := <-store: msg = m - case <-time.After(500 * time.Millisecond): + case <-time.After(notFoundTimeout): msg = "" } _, err := w.Write([]byte(msg)) @@ -211,7 +213,12 @@ func (s Subscriber) CheckEvent(expectedData string) error { err := retry.Do( func() error { // check if a response was received and that it's code is in 2xx-range - resp, err := http.Get(s.checkURL) + ctx := context.Background() + req, err := http.NewRequestWithContext(ctx, http.MethodGet, s.checkURL, nil) + if err != nil { + return err + } + resp, err := http.DefaultClient.Do(req) if err != nil { return fmt.Errorf("get HTTP request failed: %w", err) } @@ -252,7 +259,12 @@ func (s Subscriber) CheckRetries(expectedNoOfRetries int, expectedData string) e delay := time.Second err := retry.Do( func() error { - resp, err := http.Get(s.checkRetriesURL) + ctx := context.Background() + req, err := http.NewRequestWithContext(ctx, http.MethodGet, s.checkRetriesURL, nil) + if err != nil { + return err + } + resp, err := http.DefaultClient.Do(req) if err != nil { return fmt.Errorf("get HTTP request failed: %w", err) }