From f71e3c441d0fe114931248aac335d179a7952673 Mon Sep 17 00:00:00 2001 From: Marco Bebway Date: Thu, 18 Apr 2024 16:46:45 +0200 Subject: [PATCH] feat: remove the mutating and validating webhooks for v1alpha2 Subscription (#549) * feat: remove the mutating and validating webhooks for v1alpha2 Subscription. * Fix lint issues * Revert image update * Fix tests * Refactor subscription validation * Fix lint issues * Refactor subscription validation * Refactoring * Add subscription validation tests * Test subscription validation in the reconciler using mocks * Fix lint issues * Refactoring * Refactor type matching validation * Refactoring validation * Undo doc changes * Update validation tests * Add defaulting tests * Remove not needed setup * Update defaulting expr * Fix lint issues * Add tests for webhook resources cleanup * Add tests for subscription spec valid conditions * Restrict delete job and cronjobs only to the target resources * Fix lint issues * Preserve the old subscription status in case of the EventMesh backend * Fix generated config * Remove webhook cleanup logic --- .golangci.yaml | 4 + Makefile | 4 +- api/eventing/v1alpha2/condition.go | 136 +++++-- api/eventing/v1alpha2/condition_test.go | 62 +++ api/eventing/v1alpha2/condition_unit_test.go | 146 ++++++- api/eventing/v1alpha2/errors.go | 41 -- api/eventing/v1alpha2/subscription_types.go | 5 +- api/eventing/v1alpha2/subscription_webhook.go | 197 ---------- api/operator/v1alpha1/eventing_types.go | 5 - api/operator/v1alpha1/eventing_types_test.go | 1 - api/operator/v1alpha1/status.go | 18 - api/operator/v1alpha1/status_test.go | 12 - .../v1alpha1/zz_generated.deepcopy.go | 2 +- cmd/main.go | 10 - ...venting.kyma-project.io_subscriptions.yaml | 3 + config/crd/kustomization.yaml | 12 - config/crd/kustomizeconfig.yaml | 19 - config/crd/patches/webhook_in_eventings.yaml | 16 - .../crd/patches/webhook_in_subscriptions.yaml | 16 - config/default/kustomization.yaml | 120 ------ config/manager/manager.yaml | 15 - config/rbac/kustomization.yaml | 3 - config/rbac/webhook_role.yaml | 11 - config/rbac/webhook_role_binding.yaml | 17 - config/rbac/webhook_service_account.yaml | 10 - config/webhook/cronjob.yaml | 33 -- config/webhook/job.yaml | 29 -- config/webhook/kustomization.yaml | 13 - config/webhook/manifests.yaml | 52 --- config/webhook/secret.yaml | 11 - config/webhook/service.yaml | 20 - config/webhook/webhook_configs.yaml | 63 --- hack/ci/render-sec-scanners-config.sh | 9 - hack/e2e/common/eventing/publisher.go | 3 +- hack/e2e/common/fixtures/fixtures.go | 20 +- hack/e2e/setup/setup_test.go | 63 --- .../subscription/eventmesh/reconciler.go | 45 ++- .../reconciler_internal_integration_test.go | 88 ++++- .../test/reconciler_integration_test.go | 298 ++++++++------- .../subscription/eventmesh/test/utils.go | 64 +--- .../eventmesh/testwebhookauth/utils.go | 47 +-- .../subscription/jetstream/reconciler.go | 100 ++--- .../jetstream/reconciler_integration_test.go | 188 +++++---- .../reconciler_internal_unit_test.go | 75 +++- .../subscription/jetstream/test_utils_test.go | 60 +-- .../eventing/subscription/validator/errors.go | 43 +++ .../validator/mocks/SubscriptionValidator.go | 84 ++++ .../eventing/subscription/validator/sink.go | 63 +++ .../subscription/validator/sink_test.go | 83 ++++ .../eventing/subscription/validator/spec.go | 174 +++++++++ .../subscription/validator/spec_test.go | 360 +++++++++--------- .../subscription/validator/subscription.go | 49 +++ .../validator/subscription_test.go | 83 ++++ .../operator/eventing/controller.go | 7 - .../controller/integration_test.go | 4 - .../controller/operator/eventing/status.go | 11 - .../controller/operator/eventing/webhook.go | 80 ---- .../operator/eventing/webhook_test.go | 251 ------------ pkg/backend/sink/validator.go | 70 ---- pkg/backend/sink/validator_test.go | 95 ----- pkg/env/backend_config.go | 5 - pkg/k8s/client.go | 33 -- pkg/k8s/client_test.go | 144 ------- pkg/k8s/mocks/client.go | 124 +----- .../eventmesh/eventmesh.go | 7 +- .../jetstream/jetstream.go | 7 +- test/utils/integration/integration.go | 148 +------ testing/matchers.go | 6 + 68 files changed, 1635 insertions(+), 2462 deletions(-) create mode 100644 api/eventing/v1alpha2/condition_test.go delete mode 100644 api/eventing/v1alpha2/errors.go delete mode 100644 api/eventing/v1alpha2/subscription_webhook.go delete mode 100644 config/crd/kustomizeconfig.yaml delete mode 100644 config/crd/patches/webhook_in_eventings.yaml delete mode 100644 config/crd/patches/webhook_in_subscriptions.yaml delete mode 100644 config/rbac/webhook_role.yaml delete mode 100644 config/rbac/webhook_role_binding.yaml delete mode 100644 config/rbac/webhook_service_account.yaml delete mode 100644 config/webhook/cronjob.yaml delete mode 100644 config/webhook/job.yaml delete mode 100644 config/webhook/kustomization.yaml delete mode 100644 config/webhook/manifests.yaml delete mode 100644 config/webhook/secret.yaml delete mode 100644 config/webhook/service.yaml delete mode 100644 config/webhook/webhook_configs.yaml create mode 100644 internal/controller/eventing/subscription/validator/errors.go create mode 100644 internal/controller/eventing/subscription/validator/mocks/SubscriptionValidator.go create mode 100644 internal/controller/eventing/subscription/validator/sink.go create mode 100644 internal/controller/eventing/subscription/validator/sink_test.go create mode 100644 internal/controller/eventing/subscription/validator/spec.go rename api/eventing/v1alpha2/subscription_webhook_unit_test.go => internal/controller/eventing/subscription/validator/spec_test.go (56%) create mode 100644 internal/controller/eventing/subscription/validator/subscription.go create mode 100644 internal/controller/eventing/subscription/validator/subscription_test.go delete mode 100644 internal/controller/operator/eventing/webhook.go delete mode 100644 internal/controller/operator/eventing/webhook_test.go delete mode 100644 pkg/backend/sink/validator.go delete mode 100644 pkg/backend/sink/validator_test.go diff --git a/.golangci.yaml b/.golangci.yaml index 0c3770e2d..090b5c220 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -74,6 +74,8 @@ linters-settings: alias: kappsv1 - pkg: k8s.io/api/rbac/v1 alias: krbacv1 + - pkg: k8s.io/api/batch/v1 + alias: kbatchv1 - pkg: k8s.io/apimachinery/pkg/runtime/schema alias: kschema - pkg: k8s.io/apimachinery/pkg/labels @@ -194,6 +196,8 @@ linters-settings: alias: pkgerrors - pkg: github.com/kyma-project/eventing-manager/testing/eventmeshsub alias: eventmeshsubmatchers + - pkg: github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/validator/mocks + alias: subscriptionvalidatormocks ireturn: allow: diff --git a/Makefile b/Makefile index 001763e49..88d056ea8 100644 --- a/Makefile +++ b/Makefile @@ -85,8 +85,8 @@ help: ## Display this help. ##@ Development .PHONY: manifests -manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects. - $(CONTROLLER_GEN) rbac:roleName=eventing-manager crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases +manifests: controller-gen ## Generate ClusterRole and CustomResourceDefinition objects. + $(CONTROLLER_GEN) rbac:roleName=eventing-manager crd paths="./..." output:crd:artifacts:config=config/crd/bases $(MAKE) crd-docs-gen .PHONY: generate diff --git a/api/eventing/v1alpha2/condition.go b/api/eventing/v1alpha2/condition.go index 8103d829e..e7ceafb5f 100644 --- a/api/eventing/v1alpha2/condition.go +++ b/api/eventing/v1alpha2/condition.go @@ -10,10 +10,11 @@ import ( type ConditionType string const ( - ConditionSubscribed ConditionType = "Subscribed" - ConditionSubscriptionActive ConditionType = "Subscription active" - ConditionAPIRuleStatus ConditionType = "APIRule status" - ConditionWebhookCallStatus ConditionType = "Webhook call status" + ConditionSubscribed ConditionType = "Subscribed" + ConditionSubscriptionActive ConditionType = "Subscription active" + ConditionSubscriptionSpecValid ConditionType = "Subscription spec valid" + ConditionAPIRuleStatus ConditionType = "APIRule status" + ConditionWebhookCallStatus ConditionType = "Webhook call status" ConditionPublisherProxyReady ConditionType = "Publisher Proxy Ready" ConditionControllerReady ConditionType = "Subscription Controller Ready" @@ -37,6 +38,9 @@ type Condition struct { type ConditionReason string const ( + ConditionReasonSubscriptionSpecHasValidationErrors ConditionReason = "Subscription spec has validation errors" + ConditionReasonSubscriptionSpecHasNoValidationErrors ConditionReason = "Subscription spec has no validation errors" + // JetStream Conditions. ConditionReasonNATSSubscriptionActive ConditionReason = "NATS Subscription active" ConditionReasonNATSSubscriptionNotActive ConditionReason = "NATS Subscription not active" @@ -133,6 +137,11 @@ func MakeSubscriptionConditions() []Condition { LastTransitionTime: kmetav1.Now(), Status: kcorev1.ConditionUnknown, }, + { + Type: ConditionSubscriptionSpecValid, + LastTransitionTime: kmetav1.Now(), + Status: kcorev1.ConditionUnknown, + }, } return conditions } @@ -241,7 +250,7 @@ func ConditionsEquals(existing, expected []Condition) bool { return true } -// ConditionsEquals checks if two conditions are equal. +// ConditionEquals checks if two conditions are equal. func ConditionEquals(existing, expected Condition) bool { isTypeEqual := existing.Type == expected.Type isStatusEqual := existing.Status == expected.Status @@ -259,29 +268,112 @@ func CreateMessageForConditionReasonSubscriptionCreated(eventMeshName string) st return fmt.Sprintf("EventMesh subscription name is: %s", eventMeshName) } -// GetSubscriptionActiveCondition updates the ConditionSubscriptionActive condition based on the given error value. -func GetSubscriptionActiveCondition(sub *Subscription, err error) []Condition { - subscriptionActiveCondition := Condition{ +// makeSubscriptionActiveCondition returns a new active condition based on the given error. +func makeSubscriptionActiveCondition(err error) Condition { + var ( + status kcorev1.ConditionStatus + reason ConditionReason + message string + ) + if err != nil { + status = kcorev1.ConditionFalse + reason = ConditionReasonNATSSubscriptionNotActive + message = err.Error() + } else { + status = kcorev1.ConditionTrue + reason = ConditionReasonNATSSubscriptionActive + } + return Condition{ Type: ConditionSubscriptionActive, + Status: status, LastTransitionTime: kmetav1.Now(), + Reason: reason, + Message: message, } - if err == nil { - subscriptionActiveCondition.Status = kcorev1.ConditionTrue - subscriptionActiveCondition.Reason = ConditionReasonNATSSubscriptionActive +} + +// makeSubscriptionSpecValidCondition returns a new validation condition based on the given error. +func makeSubscriptionSpecValidCondition(err error) Condition { + var ( + status kcorev1.ConditionStatus + reason ConditionReason + message string + ) + if err != nil { + status = kcorev1.ConditionFalse + reason = ConditionReasonSubscriptionSpecHasValidationErrors + message = err.Error() } else { - subscriptionActiveCondition.Message = err.Error() - subscriptionActiveCondition.Reason = ConditionReasonNATSSubscriptionNotActive - subscriptionActiveCondition.Status = kcorev1.ConditionFalse + status = kcorev1.ConditionTrue + reason = ConditionReasonSubscriptionSpecHasNoValidationErrors + } + return Condition{ + Type: ConditionSubscriptionSpecValid, + Status: status, + LastTransitionTime: kmetav1.Now(), + Reason: reason, + Message: message, } - for _, activeCond := range sub.Status.Conditions { - if activeCond.Type == ConditionSubscriptionActive { - if subscriptionActiveCondition.Status == activeCond.Status && - subscriptionActiveCondition.Reason == activeCond.Reason && - subscriptionActiveCondition.Message == activeCond.Message { - return []Condition{activeCond} - } +} + +// setCondition sets the given condition in the Subscription status. +// If the condition is already present, it will be updated. +// If the condition is not present, it will be added. +func (s *SubscriptionStatus) setCondition(condition Condition) { + isFound, isSet := false, false + conditions := make([]Condition, 0, len(s.Conditions)) + for _, cond := range s.Conditions { + if cond.Type != condition.Type { + conditions = append(conditions, cond) + continue + } + isFound = true + if !ConditionEquals(cond, condition) { + isSet = true + conditions = append(conditions, condition) } } + if !isFound { + isSet = true + conditions = append(conditions, condition) + } + if isSet { + s.Conditions = conditions + } +} + +// SetSubscriptionActiveCondition sets a subscription active condition based on the given error. +// If the given error is nil, the status will have the Subscription active condition set to true, +// otherwise it will have the Subscription active condition set to false and the error as the message. +func SetSubscriptionActiveCondition(status *SubscriptionStatus, err error) { + condition := makeSubscriptionActiveCondition(err) + status.setCondition(condition) +} + +// SetSubscriptionSpecValidCondition sets a subscription spec valid condition based on the given error. +// If the given error is nil, the status will have the Subscription spec valid condition set to true, +// otherwise it will have the Subscription spec valid condition set to false and the error as the message. +func (s *SubscriptionStatus) SetSubscriptionSpecValidCondition(err error) { + condition := makeSubscriptionSpecValidCondition(err) + s.setCondition(condition) +} + +// SetNotReady sets the Subscription status to not ready. +func (s *SubscriptionStatus) SetNotReady() { + s.Ready = false +} + +// ClearConditions sets the Subscription conditions to an empty list. +func (s *SubscriptionStatus) ClearConditions() { + s.Conditions = []Condition{} +} + +// ClearBackend sets the Subscription Backend to an empty struct. +func (s *SubscriptionStatus) ClearBackend() { + s.Backend = Backend{} +} - return []Condition{subscriptionActiveCondition} +// ClearTypes sets the Subscription Types to an empty list. +func (s *SubscriptionStatus) ClearTypes() { + s.Types = []EventType{} } diff --git a/api/eventing/v1alpha2/condition_test.go b/api/eventing/v1alpha2/condition_test.go new file mode 100644 index 000000000..4ad40a767 --- /dev/null +++ b/api/eventing/v1alpha2/condition_test.go @@ -0,0 +1,62 @@ +package v1alpha2 + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/require" + kcorev1 "k8s.io/api/core/v1" + kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func Test_makeSubscriptionSpecValidCondition(t *testing.T) { + t.Parallel() + + var ( + // subscription spec valid condition + subscriptionSpecValidTrueCondition = Condition{ + Type: ConditionSubscriptionSpecValid, + Status: kcorev1.ConditionTrue, + LastTransitionTime: kmetav1.Now(), + Reason: ConditionReasonSubscriptionSpecHasNoValidationErrors, + Message: "", + } + + // subscription spec invalid condition + subscriptionSpecValidFalseCondition = Condition{ + Type: ConditionSubscriptionSpecValid, + Status: kcorev1.ConditionFalse, + LastTransitionTime: kmetav1.Now(), + Reason: ConditionReasonSubscriptionSpecHasValidationErrors, + Message: "some error", + } + ) + + tests := []struct { + name string + givenError error + wantSubscriptionSpecValidCondition Condition + }{ + { + name: "no error", + givenError: nil, + wantSubscriptionSpecValidCondition: subscriptionSpecValidTrueCondition, + }, + { + name: "error", + givenError: errors.New("some error"), //nolint: goerr113 // used for testing only + wantSubscriptionSpecValidCondition: subscriptionSpecValidFalseCondition, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + // when + gotCondition := makeSubscriptionSpecValidCondition(test.givenError) + + // then + require.True(t, ConditionEquals(gotCondition, test.wantSubscriptionSpecValidCondition)) + }) + } +} diff --git a/api/eventing/v1alpha2/condition_unit_test.go b/api/eventing/v1alpha2/condition_unit_test.go index e3f2b2ca8..3f55eac62 100644 --- a/api/eventing/v1alpha2/condition_unit_test.go +++ b/api/eventing/v1alpha2/condition_unit_test.go @@ -48,6 +48,7 @@ func Test_InitializeSubscriptionConditions(t *testing.T) { v1alpha2.ConditionSubscriptionActive, v1alpha2.ConditionAPIRuleStatus, v1alpha2.ConditionWebhookCallStatus, + v1alpha2.ConditionSubscriptionSpecValid, } // when @@ -130,6 +131,7 @@ func Test_IsReady(t *testing.T) { {Type: v1alpha2.ConditionSubscriptionActive, Status: kcorev1.ConditionTrue}, {Type: v1alpha2.ConditionAPIRuleStatus, Status: kcorev1.ConditionTrue}, {Type: v1alpha2.ConditionWebhookCallStatus, Status: kcorev1.ConditionTrue}, + {Type: v1alpha2.ConditionSubscriptionSpecValid, Status: kcorev1.ConditionTrue}, }, wantReadyStatus: true, }, @@ -208,6 +210,7 @@ func Test_ShouldUpdateReadyStatus(t *testing.T) { {Type: v1alpha2.ConditionSubscriptionActive, Status: kcorev1.ConditionTrue}, {Type: v1alpha2.ConditionAPIRuleStatus, Status: kcorev1.ConditionTrue}, {Type: v1alpha2.ConditionWebhookCallStatus, Status: kcorev1.ConditionTrue}, + {Type: v1alpha2.ConditionSubscriptionSpecValid, Status: kcorev1.ConditionTrue}, }, wantStatus: false, }, @@ -230,6 +233,7 @@ func Test_ShouldUpdateReadyStatus(t *testing.T) { {Type: v1alpha2.ConditionSubscriptionActive, Status: kcorev1.ConditionTrue}, {Type: v1alpha2.ConditionAPIRuleStatus, Status: kcorev1.ConditionTrue}, {Type: v1alpha2.ConditionWebhookCallStatus, Status: kcorev1.ConditionTrue}, + {Type: v1alpha2.ConditionSubscriptionSpecValid, Status: kcorev1.ConditionTrue}, }, wantStatus: true, }, @@ -540,13 +544,149 @@ func Test_SetConditionSubscriptionActive(t *testing.T) { sub.Status.Conditions = testcase.givenConditions // when - conditions := v1alpha2.GetSubscriptionActiveCondition(sub, testcase.givenError) + v1alpha2.SetSubscriptionActiveCondition(&sub.Status, testcase.givenError) // then - require.True(t, v1alpha2.ConditionsEquals(conditions, testcase.wantConditions)) + require.True(t, v1alpha2.ConditionsEquals(sub.Status.Conditions, testcase.wantConditions)) if testcase.wantLastTransitionTime != nil { - require.Equal(t, *testcase.wantLastTransitionTime, conditions[0].LastTransitionTime) + require.Equal(t, *testcase.wantLastTransitionTime, sub.Status.Conditions[0].LastTransitionTime) } }) } } + +func TestSetSubscriptionSpecValidCondition(t *testing.T) { + t.Parallel() + + var ( + // subscription spec valid conditions + subscriptionSpecValidTrueCondition = v1alpha2.Condition{ + Type: v1alpha2.ConditionSubscriptionSpecValid, + Status: kcorev1.ConditionTrue, + LastTransitionTime: kmetav1.Now(), + Reason: v1alpha2.ConditionReasonSubscriptionSpecHasNoValidationErrors, + Message: "", + } + subscriptionSpecValidFalseCondition = v1alpha2.Condition{ + Type: v1alpha2.ConditionSubscriptionSpecValid, + Status: kcorev1.ConditionFalse, + LastTransitionTime: kmetav1.Now(), + Reason: v1alpha2.ConditionReasonSubscriptionSpecHasValidationErrors, + Message: "some error", + } + + // test conditions + testCondition0 = v1alpha2.Condition{ + Type: v1alpha2.ConditionType("test-0"), + Status: kcorev1.ConditionTrue, + LastTransitionTime: kmetav1.Now(), + Reason: "test-0", + Message: "test-0", + } + testCondition1 = v1alpha2.Condition{ + Type: v1alpha2.ConditionType("test-1"), + Status: kcorev1.ConditionTrue, + LastTransitionTime: kmetav1.Now(), + Reason: "test-1", + Message: "test-1", + } + ) + + tests := []struct { + name string + givenSubscriptionStatus v1alpha2.SubscriptionStatus + givenError error + wantSubscriptionStatus v1alpha2.SubscriptionStatus + }{ + { + name: "add SubscriptionSpecValid condition to nil conditions", + givenSubscriptionStatus: v1alpha2.SubscriptionStatus{ + Conditions: nil, + }, + givenError: nil, + wantSubscriptionStatus: v1alpha2.SubscriptionStatus{ + Conditions: []v1alpha2.Condition{ + subscriptionSpecValidTrueCondition, + }, + }, + }, + { + name: "add SubscriptionSpecValid condition to empty conditions", + givenSubscriptionStatus: v1alpha2.SubscriptionStatus{ + Conditions: []v1alpha2.Condition{}, + }, + givenError: nil, + wantSubscriptionStatus: v1alpha2.SubscriptionStatus{ + Conditions: []v1alpha2.Condition{ + subscriptionSpecValidTrueCondition, + }, + }, + }, + { + name: "add SubscriptionSpecValid condition and preserve other conditions", + givenSubscriptionStatus: v1alpha2.SubscriptionStatus{ + Conditions: []v1alpha2.Condition{ + testCondition0, + testCondition1, + }, + }, + givenError: nil, + wantSubscriptionStatus: v1alpha2.SubscriptionStatus{ + Conditions: []v1alpha2.Condition{ + testCondition0, + testCondition1, + subscriptionSpecValidTrueCondition, + }, + }, + }, + { + name: "update existing SubscriptionSpecValid condition to true and preserve other conditions", + givenSubscriptionStatus: v1alpha2.SubscriptionStatus{ + Conditions: []v1alpha2.Condition{ + testCondition0, + testCondition1, + subscriptionSpecValidFalseCondition, + }, + }, + givenError: nil, + wantSubscriptionStatus: v1alpha2.SubscriptionStatus{ + Conditions: []v1alpha2.Condition{ + testCondition0, + testCondition1, + subscriptionSpecValidTrueCondition, + }, + }, + }, + { + name: "update existing SubscriptionSpecValid condition to false and preserve other conditions", + givenSubscriptionStatus: v1alpha2.SubscriptionStatus{ + Conditions: []v1alpha2.Condition{ + testCondition0, + testCondition1, + subscriptionSpecValidTrueCondition, + }, + }, + givenError: errors.New("some error"), + wantSubscriptionStatus: v1alpha2.SubscriptionStatus{ + Conditions: []v1alpha2.Condition{ + testCondition0, + testCondition1, + subscriptionSpecValidFalseCondition, + }, + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + // when + test.givenSubscriptionStatus.SetSubscriptionSpecValidCondition(test.givenError) + gotConditions := test.givenSubscriptionStatus.Conditions + wantConditions := test.wantSubscriptionStatus.Conditions + + // then + require.True(t, v1alpha2.ConditionsEquals(gotConditions, wantConditions)) + }) + } +} diff --git a/api/eventing/v1alpha2/errors.go b/api/eventing/v1alpha2/errors.go deleted file mode 100644 index 6afbe419d..000000000 --- a/api/eventing/v1alpha2/errors.go +++ /dev/null @@ -1,41 +0,0 @@ -package v1alpha2 - -import ( - "fmt" - "strconv" - - "k8s.io/apimachinery/pkg/util/validation/field" - - "github.com/kyma-project/eventing-manager/pkg/ems/api/events/types" -) - -//nolint:gochecknoglobals // these are required for testing -var ( - SourcePath = field.NewPath("spec").Child("source") - TypesPath = field.NewPath("spec").Child("types") - ConfigPath = field.NewPath("spec").Child("config") - SinkPath = field.NewPath("spec").Child("sink") - NSPath = field.NewPath("metadata").Child("namespace") - - EmptyErrDetail = "must not be empty" - InvalidURIErrDetail = "must be valid as per RFC 3986" - DuplicateTypesErrDetail = "must not have duplicate types" - LengthErrDetail = "must not be of length zero" - MinSegmentErrDetail = fmt.Sprintf("must have minimum %s segments", strconv.Itoa(minEventTypeSegments)) - InvalidPrefixErrDetail = fmt.Sprintf("must not have %s as type prefix", InvalidPrefix) - StringIntErrDetail = fmt.Sprintf("%s must be a stringified int value", MaxInFlightMessages) - - InvalidQosErrDetail = fmt.Sprintf("must be a valid QoS value %s or %s", - types.QosAtLeastOnce, types.QosAtMostOnce) - InvalidAuthTypeErrDetail = fmt.Sprintf("must be a valid Auth Type value %s", types.AuthTypeClientCredentials) - InvalidGrantTypeErrDetail = fmt.Sprintf("must be a valid Grant Type value %s", types.GrantTypeClientCredentials) - - MissingSchemeErrDetail = "must have URL scheme 'http' or 'https'" - SuffixMissingErrDetail = fmt.Sprintf("must have valid sink URL suffix %s", ClusterLocalURLSuffix) - SubDomainsErrDetail = fmt.Sprintf("must have sink URL with %d sub-domains: ", subdomainSegments) - NSMismatchErrDetail = "must have the same namespace as the subscriber: " -) - -func MakeInvalidFieldError(path *field.Path, subName, detail string) *field.Error { - return field.Invalid(path, subName, detail) -} diff --git a/api/eventing/v1alpha2/subscription_types.go b/api/eventing/v1alpha2/subscription_types.go index 6adff5845..db566ea83 100644 --- a/api/eventing/v1alpha2/subscription_types.go +++ b/api/eventing/v1alpha2/subscription_types.go @@ -30,6 +30,7 @@ type SubscriptionSpec struct { // Defines how types should be handled.
// - `standard`: backend-specific logic will be applied to the configured source and types.
// - `exact`: no further processing will be applied to the configured source and types. + // +kubebuilder:default:="standard" TypeMatching TypeMatching `json:"typeMatching,omitempty"` // Defines the origin of the event. @@ -40,6 +41,7 @@ type SubscriptionSpec struct { // Map of configuration options that will be applied on the backend. // +optional + // +kubebuilder:default:={"maxInFlightMessages":"10"} Config map[string]string `json:"config,omitempty"` } @@ -140,6 +142,3 @@ type SubscriptionList struct { func init() { //nolint:gochecknoinits SchemeBuilder.Register(&Subscription{}, &SubscriptionList{}) } - -// Hub marks this type as a conversion hub. -func (*Subscription) Hub() {} diff --git a/api/eventing/v1alpha2/subscription_webhook.go b/api/eventing/v1alpha2/subscription_webhook.go deleted file mode 100644 index 2b76fea61..000000000 --- a/api/eventing/v1alpha2/subscription_webhook.go +++ /dev/null @@ -1,197 +0,0 @@ -package v1alpha2 - -import ( - "strconv" - "strings" - - kerrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/validation/field" - kctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/webhook" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - "github.com/kyma-project/eventing-manager/pkg/ems/api/events/types" - "github.com/kyma-project/eventing-manager/pkg/utils" -) - -const ( - DefaultMaxInFlightMessages = "10" - minEventTypeSegments = 2 - subdomainSegments = 5 - InvalidPrefix = "sap.kyma.custom" - ClusterLocalURLSuffix = "svc.cluster.local" - ValidSource = "source" -) - -func (s *Subscription) SetupWebhookWithManager(mgr kctrl.Manager) error { - return kctrl.NewWebhookManagedBy(mgr). - For(s). - Complete() -} - -// +kubebuilder:webhook:path=/mutate-eventing-kyma-project-io-v1alpha2-subscription,mutating=true,failurePolicy=fail,sideEffects=None,groups=eventing.kyma-project.io,resources=subscriptions,verbs=create;update,versions=v1alpha2,name=msubscription.kb.io,admissionReviewVersions=v1beta1 - -var _ webhook.Defaulter = &Subscription{} - -// Default implements webhook.Defaulter so a webhook will be registered for the type. -func (s *Subscription) Default() { - if s.Spec.TypeMatching == "" { - s.Spec.TypeMatching = TypeMatchingStandard - } - if s.Spec.Config[MaxInFlightMessages] == "" { - if s.Spec.Config == nil { - s.Spec.Config = map[string]string{} - } - s.Spec.Config[MaxInFlightMessages] = DefaultMaxInFlightMessages - } -} - -// +kubebuilder:webhook:path=/validate-eventing-kyma-project-io-v1alpha2-subscription,mutating=false,failurePolicy=fail,sideEffects=None,groups=eventing.kyma-project.io,resources=subscriptions,verbs=create;update,versions=v1alpha2,name=vsubscription.kb.io,admissionReviewVersions=v1beta1 - -var _ webhook.Validator = &Subscription{} - -// ValidateCreate implements webhook.Validator so a webhook will be registered for the type. -func (s *Subscription) ValidateCreate() (admission.Warnings, error) { - return s.ValidateSubscription() -} - -// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. -func (s *Subscription) ValidateUpdate(_ runtime.Object) (admission.Warnings, error) { - return s.ValidateSubscription() -} - -// ValidateDelete implements webhook.Validator so a webhook will be registered for the type. -func (s *Subscription) ValidateDelete() (admission.Warnings, error) { - return nil, nil -} - -func (s *Subscription) ValidateSubscription() (admission.Warnings, error) { - var allErrs field.ErrorList - - if err := s.validateSubscriptionSource(); err != nil { - allErrs = append(allErrs, err) - } - if err := s.validateSubscriptionTypes(); err != nil { - allErrs = append(allErrs, err) - } - if err := s.validateSubscriptionConfig(); err != nil { - allErrs = append(allErrs, err...) - } - if err := s.validateSubscriptionSink(); err != nil { - allErrs = append(allErrs, err) - } - if len(allErrs) == 0 { - return nil, nil - } - - return nil, kerrors.NewInvalid(GroupKind, s.Name, allErrs) -} - -func (s *Subscription) validateSubscriptionSource() *field.Error { - if s.Spec.Source == "" && s.Spec.TypeMatching != TypeMatchingExact { - return MakeInvalidFieldError(SourcePath, s.Name, EmptyErrDetail) - } - // Check only if the source is valid for the cloud event, with a valid event type. - if IsInvalidCE(s.Spec.Source, "") { - return MakeInvalidFieldError(SourcePath, s.Name, InvalidURIErrDetail) - } - return nil -} - -func (s *Subscription) validateSubscriptionTypes() *field.Error { - if s.Spec.Types == nil || len(s.Spec.Types) == 0 { - return MakeInvalidFieldError(TypesPath, s.Name, EmptyErrDetail) - } - if len(s.GetUniqueTypes()) != len(s.Spec.Types) { - return MakeInvalidFieldError(TypesPath, s.Name, DuplicateTypesErrDetail) - } - for _, etype := range s.Spec.Types { - if len(etype) == 0 { - return MakeInvalidFieldError(TypesPath, s.Name, LengthErrDetail) - } - if segments := strings.Split(etype, "."); len(segments) < minEventTypeSegments { - return MakeInvalidFieldError(TypesPath, s.Name, MinSegmentErrDetail) - } - if s.Spec.TypeMatching != TypeMatchingExact && strings.HasPrefix(etype, InvalidPrefix) { - return MakeInvalidFieldError(TypesPath, s.Name, InvalidPrefixErrDetail) - } - // Check only is the event type is valid for the cloud event, with a valid source. - if IsInvalidCE(ValidSource, etype) { - return MakeInvalidFieldError(TypesPath, s.Name, InvalidURIErrDetail) - } - } - return nil -} - -func (s *Subscription) validateSubscriptionConfig() field.ErrorList { - var allErrs field.ErrorList - if isNotInt(s.Spec.Config[MaxInFlightMessages]) { - allErrs = append(allErrs, MakeInvalidFieldError(ConfigPath, s.Name, StringIntErrDetail)) - } - if s.ifKeyExistsInConfig(ProtocolSettingsQos) && types.IsInvalidQoS(s.Spec.Config[ProtocolSettingsQos]) { - allErrs = append(allErrs, MakeInvalidFieldError(ConfigPath, s.Name, InvalidQosErrDetail)) - } - if s.ifKeyExistsInConfig(WebhookAuthType) && types.IsInvalidAuthType(s.Spec.Config[WebhookAuthType]) { - allErrs = append(allErrs, MakeInvalidFieldError(ConfigPath, s.Name, InvalidAuthTypeErrDetail)) - } - if s.ifKeyExistsInConfig(WebhookAuthGrantType) && types.IsInvalidGrantType(s.Spec.Config[WebhookAuthGrantType]) { - allErrs = append(allErrs, MakeInvalidFieldError(ConfigPath, s.Name, InvalidGrantTypeErrDetail)) - } - return allErrs -} - -func (s *Subscription) validateSubscriptionSink() *field.Error { - if s.Spec.Sink == "" { - return MakeInvalidFieldError(SinkPath, s.Name, EmptyErrDetail) - } - - if !utils.IsValidScheme(s.Spec.Sink) { - return MakeInvalidFieldError(SinkPath, s.Name, MissingSchemeErrDetail) - } - - trimmedHost, subDomains, err := utils.GetSinkData(s.Spec.Sink) - if err != nil { - return MakeInvalidFieldError(SinkPath, s.Name, err.Error()) - } - - // Validate sink URL is a cluster local URL. - if !strings.HasSuffix(trimmedHost, ClusterLocalURLSuffix) { - return MakeInvalidFieldError(SinkPath, s.Name, SuffixMissingErrDetail) - } - - // We expected a sink in the format "service.namespace.svc.cluster.local". - if len(subDomains) != subdomainSegments { - return MakeInvalidFieldError(SinkPath, s.Name, SubDomainsErrDetail+trimmedHost) - } - - // Assumption: Subscription CR and Subscriber should be deployed in the same namespace. - svcNs := subDomains[1] - if s.Namespace != svcNs { - return MakeInvalidFieldError(NSPath, s.Name, NSMismatchErrDetail+svcNs) - } - - return nil -} - -func (s *Subscription) ifKeyExistsInConfig(key string) bool { - _, ok := s.Spec.Config[key] - return ok -} - -func isNotInt(value string) bool { - if _, err := strconv.Atoi(value); err != nil { - return true - } - return false -} - -func IsInvalidCE(source, eventType string) bool { - if source == "" { - return false - } - newEvent := utils.GetCloudEvent(eventType) - newEvent.SetSource(source) - err := newEvent.Validate() - return err != nil -} diff --git a/api/operator/v1alpha1/eventing_types.go b/api/operator/v1alpha1/eventing_types.go index f05654bf8..f15b13418 100644 --- a/api/operator/v1alpha1/eventing_types.go +++ b/api/operator/v1alpha1/eventing_types.go @@ -37,7 +37,6 @@ const ( ConditionBackendAvailable ConditionType = "BackendAvailable" ConditionPublisherProxyReady ConditionType = "PublisherProxyReady" - ConditionWebhookReady ConditionType = "WebhookReady" ConditionSubscriptionManagerReady ConditionType = "SubscriptionManagerReady" ConditionDeleted ConditionType = "Deleted" @@ -52,15 +51,12 @@ const ( ConditionReasonNATSNotAvailable ConditionReason = "NATSUnavailable" ConditionReasonBackendNotSpecified ConditionReason = "BackendNotSpecified" ConditionReasonForbidden ConditionReason = "Forbidden" - ConditionReasonWebhookFailed ConditionReason = "WebhookFailed" - ConditionReasonWebhookReady ConditionReason = "Ready" ConditionReasonDeletionError ConditionReason = "DeletionError" ConditionReasonEventMeshConfigAvailable ConditionReason = "EventMeshConfigAvailable" ConditionPublisherProxyReadyMessage = "Publisher proxy is deployed" ConditionPublisherProxyDeletedMessage = "Publisher proxy is deleted" ConditionNATSAvailableMessage = "NATS is available" - ConditionWebhookReadyMessage = "Webhook is available" ConditionPublisherProxyProcessingMessage = "Eventing publisher proxy deployment is in progress" ConditionSubscriptionManagerReadyMessage = "Subscription manager is ready" ConditionSubscriptionManagerStoppedMessage = "Subscription manager is stopped" @@ -77,7 +73,6 @@ func getSupportedConditionsTypes() map[ConditionType]interface{} { return map[ConditionType]interface{}{ ConditionBackendAvailable: nil, ConditionPublisherProxyReady: nil, - ConditionWebhookReady: nil, ConditionSubscriptionManagerReady: nil, ConditionDeleted: nil, } diff --git a/api/operator/v1alpha1/eventing_types_test.go b/api/operator/v1alpha1/eventing_types_test.go index 7bf182a47..c3314b315 100644 --- a/api/operator/v1alpha1/eventing_types_test.go +++ b/api/operator/v1alpha1/eventing_types_test.go @@ -103,7 +103,6 @@ func Test_getSupportedConditionsTypes(t *testing.T) { want := map[ConditionType]interface{}{ ConditionBackendAvailable: nil, ConditionPublisherProxyReady: nil, - ConditionWebhookReady: nil, ConditionSubscriptionManagerReady: nil, ConditionDeleted: nil, } diff --git a/api/operator/v1alpha1/status.go b/api/operator/v1alpha1/status.go index 0f6fb92bc..7430e610a 100644 --- a/api/operator/v1alpha1/status.go +++ b/api/operator/v1alpha1/status.go @@ -37,20 +37,6 @@ func (es *EventingStatus) UpdateConditionPublisherProxyReady(status kmetav1.Cond meta.SetStatusCondition(&es.Conditions, condition) } -func (es *EventingStatus) UpdateConditionWebhookReady(status kmetav1.ConditionStatus, reason ConditionReason, - message string, -) { - condition := kmetav1.Condition{ - Type: string(ConditionWebhookReady), - Status: status, - LastTransitionTime: kmetav1.Now(), - Reason: string(reason), - Message: message, - } - // meta.SetStatusCondition will update LastTransitionTime only when `Status` is changed. - meta.SetStatusCondition(&es.Conditions, condition) -} - func (es *EventingStatus) UpdateConditionSubscriptionManagerReady(status kmetav1.ConditionStatus, reason ConditionReason, message string, ) { @@ -119,10 +105,6 @@ func (es *EventingStatus) SetPublisherProxyReadyToTrue() { es.UpdateConditionPublisherProxyReady(kmetav1.ConditionTrue, ConditionReasonDeployed, ConditionPublisherProxyReadyMessage) } -func (es *EventingStatus) SetWebhookReadyConditionToTrue() { - es.UpdateConditionWebhookReady(kmetav1.ConditionTrue, ConditionReasonWebhookReady, ConditionWebhookReadyMessage) -} - func (es *EventingStatus) SetStateProcessing() { es.State = StateProcessing } diff --git a/api/operator/v1alpha1/status_test.go b/api/operator/v1alpha1/status_test.go index 00e955593..9177cc034 100644 --- a/api/operator/v1alpha1/status_test.go +++ b/api/operator/v1alpha1/status_test.go @@ -124,14 +124,6 @@ func TestRemoveUnsupportedConditions(t *testing.T) { Reason: "PublisherProxyReadyReason", Message: "PublisherProxyReadyMessage", } - webhookReadyCondition = kmetav1.Condition{ - Type: "WebhookReady", - Status: kmetav1.ConditionStatus("WebhookReadyStatus"), - ObservedGeneration: int64(3), - LastTransitionTime: kmetav1.Time{Time: time.Date(2003, 0o3, 0o3, 0o3, 0o3, 0o3, 0o00000003, time.UTC)}, - Reason: "WebhookReadyReason", - Message: "WebhookReadyMessage", - } subscriptionManagerReadyCondition = kmetav1.Condition{ Type: "SubscriptionManagerReady", Status: kmetav1.ConditionStatus("SubscriptionManagerReadyStatus"), @@ -220,7 +212,6 @@ func TestRemoveUnsupportedConditions(t *testing.T) { Conditions: []kmetav1.Condition{ backendAvailableCondition, publisherProxyReadyCondition, - webhookReadyCondition, subscriptionManagerReadyCondition, deletedCondition, }, @@ -229,7 +220,6 @@ func TestRemoveUnsupportedConditions(t *testing.T) { Conditions: []kmetav1.Condition{ backendAvailableCondition, publisherProxyReadyCondition, - webhookReadyCondition, subscriptionManagerReadyCondition, deletedCondition, }, @@ -257,7 +247,6 @@ func TestRemoveUnsupportedConditions(t *testing.T) { unsupportedTypeCondition3, backendAvailableCondition, publisherProxyReadyCondition, - webhookReadyCondition, subscriptionManagerReadyCondition, deletedCondition, }, @@ -266,7 +255,6 @@ func TestRemoveUnsupportedConditions(t *testing.T) { Conditions: []kmetav1.Condition{ backendAvailableCondition, publisherProxyReadyCondition, - webhookReadyCondition, subscriptionManagerReadyCondition, deletedCondition, }, diff --git a/api/operator/v1alpha1/zz_generated.deepcopy.go b/api/operator/v1alpha1/zz_generated.deepcopy.go index c227b3446..25bcddd82 100644 --- a/api/operator/v1alpha1/zz_generated.deepcopy.go +++ b/api/operator/v1alpha1/zz_generated.deepcopy.go @@ -21,7 +21,7 @@ limitations under the License. package v1alpha1 import ( - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) diff --git a/cmd/main.go b/cmd/main.go index b28138479..e98c9b95c 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -37,7 +37,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/metrics/server" - "sigs.k8s.io/controller-runtime/pkg/webhook" eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" operatorv1alpha1 "github.com/kyma-project/eventing-manager/api/operator/v1alpha1" @@ -69,7 +68,6 @@ func registerSchemas(scheme *runtime.Scheme) { const ( defaultMetricsPort = 9443 - webhookServerPort = 9443 ) func syncLogger(logger *logger.Logger) { @@ -113,7 +111,6 @@ func main() { //nolint:funlen // main function needs to initialize many object HealthProbeBindAddress: opts.ProbeAddr, LeaderElection: enableLeaderElection, LeaderElectionID: leaderElectionID, - WebhookServer: webhook.NewServer(webhook.Options{Port: webhookServerPort}), Cache: cache.Options{SyncPeriod: &opts.ReconcilePeriod}, Metrics: server.Options{BindAddress: opts.MetricsAddr}, NewCache: controllercache.New, @@ -200,13 +197,6 @@ func main() { //nolint:funlen // main function needs to initialize many object } //+kubebuilder:scaffold:builder - // Setup webhooks. - if err = (&eventingv1alpha2.Subscription{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "Failed to create webhook") - syncLogger(ctrLogger) - os.Exit(1) - } - // sync PeerAuthentications err = peerauthentication.SyncPeerAuthentications(ctx, kubeClient, ctrLogger.WithContext().Named("main")) if err != nil { diff --git a/config/crd/bases/eventing.kyma-project.io_subscriptions.yaml b/config/crd/bases/eventing.kyma-project.io_subscriptions.yaml index e69d77c82..0c48ea2f9 100644 --- a/config/crd/bases/eventing.kyma-project.io_subscriptions.yaml +++ b/config/crd/bases/eventing.kyma-project.io_subscriptions.yaml @@ -49,6 +49,8 @@ spec: config: additionalProperties: type: string + default: + maxInFlightMessages: "10" description: Map of configuration options that will be applied on the backend. type: object @@ -64,6 +66,7 @@ spec: description: Defines the origin of the event. type: string typeMatching: + default: standard description: |- Defines how types should be handled.
- `standard`: backend-specific logic will be applied to the configured source and types.
diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 0955426d4..5de96644d 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -6,20 +6,8 @@ resources: - bases/eventing.kyma-project.io_subscriptions.yaml #+kubebuilder:scaffold:crdkustomizeresource -patchesStrategicMerge: -# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix. -# patches here are for enabling the conversion webhook for each CRD -#- patches/webhook_in_eventings.yaml -- patches/webhook_in_subscriptions.yaml -#- patches/webhook_in_subscriptions.yaml -#+kubebuilder:scaffold:crdkustomizewebhookpatch - # [CERTMANAGER] To enable cert-manager, uncomment all the sections with [CERTMANAGER] prefix. # patches here are for enabling the CA injection for each CRD #- patches/cainjection_in_eventings.yaml #- patches/cainjection_in_subscriptions.yaml #+kubebuilder:scaffold:crdkustomizecainjectionpatch - -# the following config is for teaching kustomize how to do kustomization for CRDs. -configurations: -- kustomizeconfig.yaml diff --git a/config/crd/kustomizeconfig.yaml b/config/crd/kustomizeconfig.yaml deleted file mode 100644 index ec5c150a9..000000000 --- a/config/crd/kustomizeconfig.yaml +++ /dev/null @@ -1,19 +0,0 @@ -# This file is for teaching kustomize how to substitute name and namespace reference in CRD -nameReference: -- kind: Service - version: v1 - fieldSpecs: - - kind: CustomResourceDefinition - version: v1 - group: apiextensions.k8s.io - path: spec/conversion/webhook/clientConfig/service/name - -namespace: -- kind: CustomResourceDefinition - version: v1 - group: apiextensions.k8s.io - path: spec/conversion/webhook/clientConfig/service/namespace - create: false - -varReference: -- path: metadata/annotations diff --git a/config/crd/patches/webhook_in_eventings.yaml b/config/crd/patches/webhook_in_eventings.yaml deleted file mode 100644 index 7e8ee9544..000000000 --- a/config/crd/patches/webhook_in_eventings.yaml +++ /dev/null @@ -1,16 +0,0 @@ -# The following patch enables a conversion webhook for the CRD -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - name: eventings.operator.kyma-project.io -spec: - conversion: - strategy: Webhook - webhook: - clientConfig: - service: - namespace: system - name: webhook-service - path: /convert - conversionReviewVersions: - - v1 diff --git a/config/crd/patches/webhook_in_subscriptions.yaml b/config/crd/patches/webhook_in_subscriptions.yaml deleted file mode 100644 index cd0470431..000000000 --- a/config/crd/patches/webhook_in_subscriptions.yaml +++ /dev/null @@ -1,16 +0,0 @@ -# The following patch enables a conversion webhook for the CRD -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - name: subscriptions.eventing.kyma-project.io -spec: - conversion: - strategy: Webhook - webhook: - clientConfig: - service: - namespace: kyma-system - name: eventing-manager-webhook-service - path: /convert - conversionReviewVersions: - - v1 diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index 01a91ee94..c3d9701ea 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -24,129 +24,9 @@ resources: - ../crd - ../rbac - ../manager -- ../webhook - ../ui-extensions -# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in -# crd/kustomization.yaml -#- ../webhook -# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required. -#- ../certmanager -# [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'. -#- ../prometheus - # Protect the /metrics endpoint by putting it behind auth. # If you want your controller-manager to expose the /metrics # endpoint w/o any authn/z, please comment the following line. #- manager_auth_proxy_patch.yaml - - - -# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in -# crd/kustomization.yaml -#- manager_webhook_patch.yaml - -# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. -# Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks. -# 'CERTMANAGER' needs to be enabled to use ca injection -#- webhookcainjection_patch.yaml - -# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER' prefix. -# Uncomment the following replacements to add the cert-manager CA injection annotations -#replacements: -# - source: # Add cert-manager annotation to ValidatingWebhookConfiguration, MutatingWebhookConfiguration and CRDs -# kind: Certificate -# group: cert-manager.io -# version: v1 -# name: serving-cert # this name should match the one in certificate.yaml -# fieldPath: .metadata.namespace # namespace of the certificate CR -# targets: -# - select: -# kind: ValidatingWebhookConfiguration -# fieldPaths: -# - .metadata.annotations.[cert-manager.io/inject-ca-from] -# options: -# delimiter: '/' -# index: 0 -# create: true -# - select: -# kind: MutatingWebhookConfiguration -# fieldPaths: -# - .metadata.annotations.[cert-manager.io/inject-ca-from] -# options: -# delimiter: '/' -# index: 0 -# create: true -# - select: -# kind: CustomResourceDefinition -# fieldPaths: -# - .metadata.annotations.[cert-manager.io/inject-ca-from] -# options: -# delimiter: '/' -# index: 0 -# create: true -# - source: -# kind: Certificate -# group: cert-manager.io -# version: v1 -# name: serving-cert # this name should match the one in certificate.yaml -# fieldPath: .metadata.name -# targets: -# - select: -# kind: ValidatingWebhookConfiguration -# fieldPaths: -# - .metadata.annotations.[cert-manager.io/inject-ca-from] -# options: -# delimiter: '/' -# index: 1 -# create: true -# - select: -# kind: MutatingWebhookConfiguration -# fieldPaths: -# - .metadata.annotations.[cert-manager.io/inject-ca-from] -# options: -# delimiter: '/' -# index: 1 -# create: true -# - select: -# kind: CustomResourceDefinition -# fieldPaths: -# - .metadata.annotations.[cert-manager.io/inject-ca-from] -# options: -# delimiter: '/' -# index: 1 -# create: true -# - source: # Add cert-manager annotation to the webhook Service -# kind: Service -# version: v1 -# name: webhook-service -# fieldPath: .metadata.name # namespace of the service -# targets: -# - select: -# kind: Certificate -# group: cert-manager.io -# version: v1 -# fieldPaths: -# - .spec.dnsNames.0 -# - .spec.dnsNames.1 -# options: -# delimiter: '.' -# index: 0 -# create: true -# - source: -# kind: Service -# version: v1 -# name: webhook-service -# fieldPath: .metadata.namespace # namespace of the service -# targets: -# - select: -# kind: Certificate -# group: cert-manager.io -# version: v1 -# fieldPaths: -# - .spec.dnsNames.0 -# - .spec.dnsNames.1 -# options: -# delimiter: '.' -# index: 1 -# create: true diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 66c29630e..af2e29759 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -122,12 +122,6 @@ spec: value: "-1" - name: JS_STREAM_MAX_BYTES value: "700Mi" - - name: WEBHOOK_SECRET_NAME - value: "eventing-manager-webhook-server-cert" - - name: MUTATING_WEBHOOK_NAME - value: "subscription-mutating-webhook-configuration" - - name: VALIDATING_WEBHOOK_NAME - value: "subscription-validating-webhook-configuration" - name: EVENTING_WEBHOOK_AUTH_SECRET_NAME value: "eventing-webhook-auth" - name: EVENTING_WEBHOOK_AUTH_SECRET_NAMESPACE @@ -161,14 +155,5 @@ spec: requests: cpu: 10m memory: 128Mi - volumeMounts: - - mountPath: /tmp/k8s-webhook-server/serving-certs - name: cert - readOnly: true serviceAccountName: eventing-manager terminationGracePeriodSeconds: 10 - volumes: - - name: cert - secret: - defaultMode: 420 - secretName: "eventing-manager-webhook-server-cert" diff --git a/config/rbac/kustomization.yaml b/config/rbac/kustomization.yaml index a433b2016..53a79522b 100644 --- a/config/rbac/kustomization.yaml +++ b/config/rbac/kustomization.yaml @@ -9,9 +9,6 @@ resources: - service_account.yaml - role.yaml - role_binding.yaml -- webhook_service_account.yaml -- webhook_role.yaml -- webhook_role_binding.yaml #- leader_election_role.yaml #- leader_election_role_binding.yaml # Comment the following 4 lines if you want to disable diff --git a/config/rbac/webhook_role.yaml b/config/rbac/webhook_role.yaml deleted file mode 100644 index 47611ff6a..000000000 --- a/config/rbac/webhook_role.yaml +++ /dev/null @@ -1,11 +0,0 @@ -kind: ClusterRole -apiVersion: rbac.authorization.k8s.io/v1 -metadata: - name: eventing-manager-cert-handler -rules: - - apiGroups: ["apiextensions.k8s.io"] - resources: ["customresourcedefinitions"] - verbs: ["get", "patch", "list", "watch", "update"] - - apiGroups: [""] - resources: ["secrets"] - verbs: ["create", "get", "patch", "list", "watch", "update"] diff --git a/config/rbac/webhook_role_binding.yaml b/config/rbac/webhook_role_binding.yaml deleted file mode 100644 index c8e7793d0..000000000 --- a/config/rbac/webhook_role_binding.yaml +++ /dev/null @@ -1,17 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding -metadata: - labels: - app.kubernetes.io/component: rbac - app.kubernetes.io/created-by: eventing-manager - app.kubernetes.io/part-of: eventing-manager - app.kubernetes.io/managed-by: kustomize - name: eventing-manager-cert-handler -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: eventing-manager-cert-handler -subjects: - - kind: ServiceAccount - name: eventing-manager-cert-handler - namespace: system diff --git a/config/rbac/webhook_service_account.yaml b/config/rbac/webhook_service_account.yaml deleted file mode 100644 index 2034a9978..000000000 --- a/config/rbac/webhook_service_account.yaml +++ /dev/null @@ -1,10 +0,0 @@ -apiVersion: v1 -kind: ServiceAccount -metadata: - labels: - app: eventing-manager-cert-handler - app.kubernetes.io/component: eventing-manager - app.kubernetes.io/created-by: eventing-manager - app.kubernetes.io/part-of: eventing-manager - name: eventing-manager-cert-handler - namespace: system diff --git a/config/webhook/cronjob.yaml b/config/webhook/cronjob.yaml deleted file mode 100644 index 918016a0e..000000000 --- a/config/webhook/cronjob.yaml +++ /dev/null @@ -1,33 +0,0 @@ -apiVersion: batch/v1 -kind: CronJob -metadata: - name: eventing-manager-cert-handler - labels: - app: eventing-manager-cert-handler - app.kubernetes.io/component: eventing-manager - app.kubernetes.io/created-by: eventing-manager - app.kubernetes.io/part-of: eventing-manager - annotations: - sidecar.istio.io/inject: "false" -spec: - # Run cronjob two times per week on Sunday and on Thursday - schedule: "0 0 * * 0,4" - jobTemplate: - spec: - template: - metadata: - annotations: - sidecar.istio.io/inject: "false" - spec: - priorityClassName: eventing-manager-priority-class - restartPolicy: Never - containers: - - name: api-gateway - image: api-gateway:latest - imagePullPolicy: IfNotPresent - env: - - name: CRD_NAME - value: "subscriptions.eventing.kyma-project.io" - - name: SECRET_NAME - value: "eventing-manager-webhook-server-cert" - serviceAccountName: eventing-manager-cert-handler diff --git a/config/webhook/job.yaml b/config/webhook/job.yaml deleted file mode 100644 index 023684329..000000000 --- a/config/webhook/job.yaml +++ /dev/null @@ -1,29 +0,0 @@ -apiVersion: batch/v1 -kind: Job -metadata: - name: eventing-manager-cert-handler - labels: - app: eventing-manager-cert-handler - app.kubernetes.io/component: eventing-manager - app.kubernetes.io/created-by: eventing-manager - app.kubernetes.io/part-of: eventing-manager - annotations: - sidecar.istio.io/inject: "false" -spec: - template: - metadata: - annotations: - sidecar.istio.io/inject: "false" - spec: - priorityClassName: eventing-manager-priority-class - restartPolicy: Never - containers: - - name: api-gateway - image: api-gateway:latest - imagePullPolicy: IfNotPresent - env: - - name: CRD_NAME - value: "subscriptions.eventing.kyma-project.io" - - name: SECRET_NAME - value: "eventing-manager-webhook-server-cert" - serviceAccountName: eventing-manager-cert-handler diff --git a/config/webhook/kustomization.yaml b/config/webhook/kustomization.yaml deleted file mode 100644 index 92cde7f93..000000000 --- a/config/webhook/kustomization.yaml +++ /dev/null @@ -1,13 +0,0 @@ -resources: -- job.yaml -- cronjob.yaml -- webhook_configs.yaml -- service.yaml -- secret.yaml - -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization -images: -- name: api-gateway - newName: europe-docker.pkg.dev/kyma-project/prod/eventing-webhook-certificates - newTag: 1.7.0 diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml deleted file mode 100644 index 8c033de4e..000000000 --- a/config/webhook/manifests.yaml +++ /dev/null @@ -1,52 +0,0 @@ ---- -apiVersion: admissionregistration.k8s.io/v1 -kind: MutatingWebhookConfiguration -metadata: - name: mutating-webhook-configuration -webhooks: -- admissionReviewVersions: - - v1beta1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /mutate-eventing-kyma-project-io-v1alpha2-subscription - failurePolicy: Fail - name: msubscription.kb.io - rules: - - apiGroups: - - eventing.kyma-project.io - apiVersions: - - v1alpha2 - operations: - - CREATE - - UPDATE - resources: - - subscriptions - sideEffects: None ---- -apiVersion: admissionregistration.k8s.io/v1 -kind: ValidatingWebhookConfiguration -metadata: - name: validating-webhook-configuration -webhooks: -- admissionReviewVersions: - - v1beta1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /validate-eventing-kyma-project-io-v1alpha2-subscription - failurePolicy: Fail - name: vsubscription.kb.io - rules: - - apiGroups: - - eventing.kyma-project.io - apiVersions: - - v1alpha2 - operations: - - CREATE - - UPDATE - resources: - - subscriptions - sideEffects: None diff --git a/config/webhook/secret.yaml b/config/webhook/secret.yaml deleted file mode 100644 index 6342d2a3b..000000000 --- a/config/webhook/secret.yaml +++ /dev/null @@ -1,11 +0,0 @@ -# secret without any data. The cronjob will insert the cert into this file. -apiVersion: v1 -kind: Secret -metadata: - name: eventing-manager-webhook-server-cert - labels: - app.kubernetes.io/name: eventing-manager-webhook-service - app.kubernetes.io/component: eventing-manager - app.kubernetes.io/created-by: eventing-manager - app.kubernetes.io/part-of: eventing-manager -type: Opaque diff --git a/config/webhook/service.yaml b/config/webhook/service.yaml deleted file mode 100644 index 45175a466..000000000 --- a/config/webhook/service.yaml +++ /dev/null @@ -1,20 +0,0 @@ -apiVersion: v1 -kind: Service -metadata: - name: eventing-manager-webhook-service - labels: - app.kubernetes.io/name: eventing-manager-webhook-service - app.kubernetes.io/component: eventing-manager - app.kubernetes.io/created-by: eventing-manager - app.kubernetes.io/part-of: eventing-manager -spec: - selector: - control-plane: eventing-manager - app.kubernetes.io/name: eventing-manager - app.kubernetes.io/instance: eventing-manager - app.kubernetes.io/component: eventing-manager - ports: - - name: http-convert - port: 443 - protocol: TCP - targetPort: 9443 diff --git a/config/webhook/webhook_configs.yaml b/config/webhook/webhook_configs.yaml deleted file mode 100644 index 3e838d61c..000000000 --- a/config/webhook/webhook_configs.yaml +++ /dev/null @@ -1,63 +0,0 @@ -apiVersion: admissionregistration.k8s.io/v1 -kind: MutatingWebhookConfiguration -metadata: - creationTimestamp: null - name: subscription-mutating-webhook-configuration - labels: - app.kubernetes.io/name: subscription-mutating-webhook-configuration - app.kubernetes.io/component: eventing-manager - app.kubernetes.io/created-by: eventing-manager - app.kubernetes.io/part-of: eventing-manager -webhooks: -- admissionReviewVersions: - - v1 - clientConfig: - service: - name: eventing-manager-webhook-service - namespace: kyma-system - path: /mutate-eventing-kyma-project-io-v1alpha2-subscription - failurePolicy: Fail - name: msubscription.kb.io - rules: - - apiGroups: - - eventing.kyma-project.io - apiVersions: - - v1alpha2 - operations: - - CREATE - - UPDATE - resources: - - subscriptions - sideEffects: None ---- -apiVersion: admissionregistration.k8s.io/v1 -kind: ValidatingWebhookConfiguration -metadata: - creationTimestamp: null - name: subscription-validating-webhook-configuration - labels: - app.kubernetes.io/name: subscription-validating-webhook-configuration - app.kubernetes.io/component: eventing-manager - app.kubernetes.io/created-by: eventing-manager - app.kubernetes.io/part-of: eventing-manager -webhooks: -- admissionReviewVersions: - - v1 - clientConfig: - service: - name: eventing-manager-webhook-service - namespace: kyma-system - path: /validate-eventing-kyma-project-io-v1alpha2-subscription - failurePolicy: Fail - name: vsubscription.kb.io - rules: - - apiGroups: - - eventing.kyma-project.io - apiVersions: - - v1alpha2 - operations: - - CREATE - - UPDATE - resources: - - subscriptions - sideEffects: None diff --git a/hack/ci/render-sec-scanners-config.sh b/hack/ci/render-sec-scanners-config.sh index c53981dba..0a6befcd6 100755 --- a/hack/ci/render-sec-scanners-config.sh +++ b/hack/ci/render-sec-scanners-config.sh @@ -10,14 +10,7 @@ set -o pipefail # prevents errors in a pipeline from being masked TAG=$1 OUTPUT_FILE=${2:-"sec-scanners-config.yaml"} -WEBHOOK_FILE=${3-"config/webhook/kustomization.yaml"} PUBLISHER_FILE=${4-"config/manager/manager.yaml"} -# Fetch Webhook Image. -echo "fetching webhook image from ${WEBHOOK_FILE}" -WEBHOOK_IMAGE=$(yq eval '.images[0].newName' <"$WEBHOOK_FILE") -WEBHOOK_TAG=$(yq eval '.images[0].newTag' <"$WEBHOOK_FILE") -WEBHOOK_IMAGE="${WEBHOOK_IMAGE}:$WEBHOOK_TAG" -echo -e "webhook image is ${WEBHOOK_IMAGE} \n" # Fetch Publisher Image. echo "fetching publisher image from ${PUBLISHER_FILE}" @@ -29,13 +22,11 @@ echo -e "generating to ${OUTPUT_FILE} \n" cat < timeoutRetryActiveEmsStatus { - return false, errors.Errorf("timeout waiting for the subscription to be active: %s", subscription.Name) + return false, pkgerrors.Errorf("timeout waiting for the subscription to be active: %s", subscription.Name) } return false, nil @@ -807,7 +822,7 @@ func checkLastFailedDelivery(subscription *eventingv1alpha2.Subscription) (bool, var err error var lastFailedDeliveryTime time.Time if lastFailedDeliveryTime, err = time.Parse(time.RFC3339, lastFailed); err != nil { - return true, errors.Errorf("failed to parse LastFailedDelivery: %v", err) + return true, pkgerrors.Errorf("failed to parse LastFailedDelivery: %v", err) } // Check if LastSuccessfulDelivery exists. If not, LastFailedDelivery happened last. @@ -819,7 +834,7 @@ func checkLastFailedDelivery(subscription *eventingv1alpha2.Subscription) (bool, // Try to parse LastSuccessfulDelivery. var lastSuccessfulDeliveryTime time.Time if lastSuccessfulDeliveryTime, err = time.Parse(time.RFC3339, lastSuccessful); err != nil { - return true, errors.Errorf("failed to parse LastSuccessfulDelivery: %v", err) + return true, pkgerrors.Errorf("failed to parse LastSuccessfulDelivery: %v", err) } return lastFailedDeliveryTime.After(lastSuccessfulDeliveryTime), nil 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 173c2c8ae..5a53c3e43 100644 --- a/internal/controller/eventing/subscription/eventmesh/reconciler_internal_integration_test.go +++ b/internal/controller/eventing/subscription/eventmesh/reconciler_internal_integration_test.go @@ -21,13 +21,15 @@ import ( kctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" + "github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/validator" + subscriptionvalidatormocks "github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/validator/mocks" "github.com/kyma-project/eventing-manager/pkg/backend/cleaner" "github.com/kyma-project/eventing-manager/pkg/backend/eventmesh" "github.com/kyma-project/eventing-manager/pkg/backend/eventmesh/mocks" "github.com/kyma-project/eventing-manager/pkg/backend/metrics" - "github.com/kyma-project/eventing-manager/pkg/backend/sink" backendutils "github.com/kyma-project/eventing-manager/pkg/backend/utils" "github.com/kyma-project/eventing-manager/pkg/ems/api/events/types" "github.com/kyma-project/eventing-manager/pkg/env" @@ -54,6 +56,7 @@ func TestReconciler_Reconcile(t *testing.T) { eventingtesting.WithEventType(eventingtesting.OrderCreatedEventType), eventingtesting.WithValidSink("test", "test-svc"), eventingtesting.WithEmsSubscriptionStatus(string(types.SubscriptionStatusActive)), + eventingtesting.WithMaxInFlight(10), ) // A subscription marked for deletion. testSubUnderDeletion := eventingtesting.NewSubscription("sub2", "test", @@ -71,13 +74,14 @@ func TestReconciler_Reconcile(t *testing.T) { eventingtesting.WithEventType(eventingtesting.OrderCreatedEventType), eventingtesting.WithValidSink("test", "test-svc"), eventingtesting.WithEmsSubscriptionStatus(string(types.SubscriptionStatusPaused)), + eventingtesting.WithMaxInFlight(10), ) backendSyncErr := errors.New("backend sync error") backendDeleteErr := errors.New("backend delete error") validatorErr := errors.New("invalid sink") - happyValidator := sink.ValidatorFunc(func(_ context.Context, s *eventingv1alpha2.Subscription) error { return nil }) - unhappyValidator := sink.ValidatorFunc(func(_ context.Context, s *eventingv1alpha2.Subscription) error { return validatorErr }) + happyValidator := validator.SubscriptionValidatorFunc(func(_ context.Context, _ eventingv1alpha2.Subscription) error { return nil }) + unhappyValidator := validator.SubscriptionValidatorFunc(func(_ context.Context, _ eventingv1alpha2.Subscription) error { return validatorErr }) testCases := []struct { name string @@ -197,7 +201,7 @@ func TestReconciler_Reconcile(t *testing.T) { utils.Domain) }, wantReconcileResult: kctrl.Result{}, - wantReconcileError: validatorErr, + wantReconcileError: reconcile.TerminalError(validatorErr), }, { name: "Return nil and RequeueAfter when the EventMesh subscription is Paused", @@ -256,9 +260,10 @@ func TestReconciler_APIRuleConfig(t *testing.T) { eventingtesting.WithEventType(eventingtesting.OrderCreatedEventType), eventingtesting.WithConditions(eventingv1alpha2.MakeSubscriptionConditions()), eventingtesting.WithEmsSubscriptionStatus(string(types.SubscriptionStatusActive)), + eventingtesting.WithMaxInFlight(10), ) - validator := sink.ValidatorFunc(func(_ context.Context, s *eventingv1alpha2.Subscription) error { return nil }) + subscriptionValidator := validator.SubscriptionValidatorFunc(func(_ context.Context, _ eventingv1alpha2.Subscription) error { return nil }) col := metrics.NewCollector() @@ -288,7 +293,7 @@ func TestReconciler_APIRuleConfig(t *testing.T) { testenv.backend, testenv.credentials, testenv.mapper, - validator, + subscriptionValidator, col, utils.Domain), testenv.fakeClient @@ -318,7 +323,7 @@ func TestReconciler_APIRuleConfig(t *testing.T) { testenv.backend, testenv.credentials, testenv.mapper, - validator, + subscriptionValidator, col, utils.Domain), testenv.fakeClient @@ -386,9 +391,10 @@ func TestReconciler_APIRuleConfig_Upgrade(t *testing.T) { eventingtesting.WithEventType(eventingtesting.OrderCreatedEventType), eventingtesting.WithConditions(eventingv1alpha2.MakeSubscriptionConditions()), eventingtesting.WithEmsSubscriptionStatus(string(types.SubscriptionStatusActive)), + eventingtesting.WithMaxInFlight(10), ) - validator := sink.ValidatorFunc(func(_ context.Context, s *eventingv1alpha2.Subscription) error { return nil }) + subscriptionValidator := validator.SubscriptionValidatorFunc(func(_ context.Context, _ eventingv1alpha2.Subscription) error { return nil }) col := metrics.NewCollector() testCases := []struct { @@ -418,7 +424,7 @@ func TestReconciler_APIRuleConfig_Upgrade(t *testing.T) { testenv.backend, testenv.credentials, testenv.mapper, - validator, + subscriptionValidator, col, utils.Domain), testenv.fakeClient @@ -454,7 +460,7 @@ func TestReconciler_APIRuleConfig_Upgrade(t *testing.T) { testenv.backend, testenv.credentials, testenv.mapper, - validator, + subscriptionValidator, col, utils.Domain), testenv.fakeClient @@ -574,7 +580,7 @@ func TestReconciler_APIRuleConfig_Upgrade(t *testing.T) { func TestReconciler_PreserveBackendHashes(t *testing.T) { ctx := context.Background() collector := metrics.NewCollector() - validator := sink.ValidatorFunc(func(_ context.Context, s *eventingv1alpha2.Subscription) error { return nil }) + subscriptionValidator := validator.SubscriptionValidatorFunc(func(_ context.Context, _ eventingv1alpha2.Subscription) error { return nil }) const ( ev2hash = int64(118518533334734) @@ -605,6 +611,9 @@ func TestReconciler_PreserveBackendHashes(t *testing.T) { WebhookAuthHash: webhookAuthHash, EventMeshLocalHash: eventMeshLocalHash, }), + eventingtesting.WithMaxInFlight(10), + eventingtesting.WithSource(eventingtesting.EventSourceClean), + eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), ) }(), givenReconcilerSetup: func(s *eventingv1alpha2.Subscription) (*Reconciler, client.Client) { @@ -612,7 +621,7 @@ func TestReconciler_PreserveBackendHashes(t *testing.T) { te.backend.On("Initialize", mock.Anything).Return(nil) te.backend.On("SyncSubscription", mock.Anything, mock.Anything, mock.Anything).Return(true, nil) return NewReconciler(te.fakeClient, te.logger, te.recorder, te.cfg, te.cleaner, - te.backend, te.credentials, te.mapper, validator, collector, utils.Domain), te.fakeClient + te.backend, te.credentials, te.mapper, subscriptionValidator, collector, utils.Domain), te.fakeClient }, wantEv2Hash: ev2hash, wantEventMeshHash: eventMeshHash, @@ -632,6 +641,9 @@ func TestReconciler_PreserveBackendHashes(t *testing.T) { WebhookAuthHash: webhookAuthHash, EventMeshLocalHash: eventMeshLocalHash, }), + eventingtesting.WithMaxInFlight(10), + eventingtesting.WithSource(eventingtesting.EventSourceClean), + eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), ) }(), givenReconcilerSetup: func(s *eventingv1alpha2.Subscription) (*Reconciler, client.Client) { @@ -639,7 +651,7 @@ func TestReconciler_PreserveBackendHashes(t *testing.T) { te.backend.On("Initialize", mock.Anything).Return(nil) te.backend.On("SyncSubscription", mock.Anything, mock.Anything, mock.Anything).Return(true, nil) return NewReconciler(te.fakeClient, te.logger, te.recorder, te.cfg, te.cleaner, - te.backend, te.credentials, te.mapper, validator, collector, utils.Domain), te.fakeClient + te.backend, te.credentials, te.mapper, subscriptionValidator, collector, utils.Domain), te.fakeClient }, wantEv2Hash: ev2hash, wantEventMeshHash: eventMeshHash, @@ -772,12 +784,14 @@ func Test_getRequiredConditions(t *testing.T) { {Type: eventingv1alpha2.ConditionSubscriptionActive, Status: kcorev1.ConditionFalse}, {Type: eventingv1alpha2.ConditionAPIRuleStatus, Status: kcorev1.ConditionUnknown}, {Type: eventingv1alpha2.ConditionWebhookCallStatus, Status: kcorev1.ConditionFalse}, + {Type: eventingv1alpha2.ConditionSubscriptionSpecValid, Status: kcorev1.ConditionFalse}, }, wantConditions: []eventingv1alpha2.Condition{ {Type: eventingv1alpha2.ConditionSubscribed, Status: kcorev1.ConditionTrue}, {Type: eventingv1alpha2.ConditionSubscriptionActive, Status: kcorev1.ConditionFalse}, {Type: eventingv1alpha2.ConditionAPIRuleStatus, Status: kcorev1.ConditionUnknown}, {Type: eventingv1alpha2.ConditionWebhookCallStatus, Status: kcorev1.ConditionFalse}, + {Type: eventingv1alpha2.ConditionSubscriptionSpecValid, Status: kcorev1.ConditionFalse}, }, }, { @@ -791,6 +805,7 @@ func Test_getRequiredConditions(t *testing.T) { {Type: eventingv1alpha2.ConditionSubscriptionActive, Status: kcorev1.ConditionFalse}, {Type: eventingv1alpha2.ConditionAPIRuleStatus, Status: kcorev1.ConditionUnknown}, {Type: eventingv1alpha2.ConditionWebhookCallStatus, Status: kcorev1.ConditionUnknown}, + {Type: eventingv1alpha2.ConditionSubscriptionSpecValid, Status: kcorev1.ConditionUnknown}, }, }, { @@ -804,6 +819,7 @@ func Test_getRequiredConditions(t *testing.T) { {Type: eventingv1alpha2.ConditionSubscriptionActive, Status: kcorev1.ConditionFalse}, {Type: eventingv1alpha2.ConditionAPIRuleStatus, Status: kcorev1.ConditionUnknown}, {Type: eventingv1alpha2.ConditionWebhookCallStatus, Status: kcorev1.ConditionUnknown}, + {Type: eventingv1alpha2.ConditionSubscriptionSpecValid, Status: kcorev1.ConditionUnknown}, }, }, } @@ -1361,6 +1377,52 @@ func Test_checkLastFailedDelivery(t *testing.T) { } } +func Test_validateSubscription(t *testing.T) { + // given + subscription := eventingtesting.NewSubscription("test-subscription", "test", + eventingtesting.WithConditions(eventingv1alpha2.MakeSubscriptionConditions()), + eventingtesting.WithFinalizers([]string{eventingv1alpha2.Finalizer}), + eventingtesting.WithDefaultSource(), + eventingtesting.WithEventType(eventingtesting.OrderCreatedEventType), + eventingtesting.WithValidSink("test", "test-svc"), + eventingtesting.WithEmsSubscriptionStatus(string(types.SubscriptionStatusActive)), + eventingtesting.WithMaxInFlight(10), + ) + + // Set up the test environment. + testEnv := setupTestEnvironment(t, subscription) + testEnv.backend.On("Initialize", mock.Anything).Return(nil) + testEnv.backend.On("SyncSubscription", mock.Anything, mock.Anything, mock.Anything).Return(true, nil) + + // Set up the SubscriptionValidator mock. + validatorMock := &subscriptionvalidatormocks.SubscriptionValidator{} + validatorMock.On("Validate", mock.Anything, mock.Anything).Return(nil) + + reconciler := NewReconciler( + testEnv.fakeClient, + testEnv.logger, + testEnv.recorder, + testEnv.cfg, + testEnv.cleaner, + testEnv.backend, + testEnv.credentials, + testEnv.mapper, + validatorMock, + metrics.NewCollector(), + utils.Domain, + ) + + // when + request := kctrl.Request{NamespacedName: ktypes.NamespacedName{Namespace: subscription.Namespace, Name: subscription.Name}} + res, err := reconciler.Reconcile(context.Background(), request) + + // then + require.Equal(t, kctrl.Result{}, res) + require.NoError(t, err) + validatorMock.AssertExpectations(t) + testEnv.backend.AssertExpectations(t) +} + // helper functions and structs // testEnvironment provides mocked resources for tests. diff --git a/internal/controller/eventing/subscription/eventmesh/test/reconciler_integration_test.go b/internal/controller/eventing/subscription/eventmesh/test/reconciler_integration_test.go index 0ee61110f..cabe7bcf2 100644 --- a/internal/controller/eventing/subscription/eventmesh/test/reconciler_integration_test.go +++ b/internal/controller/eventing/subscription/eventmesh/test/reconciler_integration_test.go @@ -25,8 +25,6 @@ import ( ) const ( - invalidSinkErrMsg = "failed to validate subscription sink URL. " + - "It is not a valid cluster local svc: Service \"invalid\" not found" testName = "test" ) @@ -51,92 +49,11 @@ func TestMain(m *testing.M) { os.Exit(code) } -func Test_ValidationWebhook(t *testing.T) { - t.Parallel() - testCases := []struct { - name string - givenSubscriptionFunc func(namespace string) *eventingv1alpha2.Subscription - wantError error - }{ - { - name: "should fail to create subscription with invalid event source", - givenSubscriptionFunc: func(namespace string) *eventingv1alpha2.Subscription { - return eventingtesting.NewSubscription(testName, namespace, - eventingtesting.WithStandardTypeMatching(), - eventingtesting.WithSource(""), - eventingtesting.WithOrderCreatedV1Event(), - eventingtesting.WithValidSink(namespace, "svc"), - ) - }, - wantError: GenerateInvalidSubscriptionError(testName, - eventingv1alpha2.EmptyErrDetail, eventingv1alpha2.SourcePath), - }, - { - name: "should fail to create subscription with invalid event types", - givenSubscriptionFunc: func(namespace string) *eventingv1alpha2.Subscription { - return eventingtesting.NewSubscription(testName, namespace, - eventingtesting.WithStandardTypeMatching(), - eventingtesting.WithSource("source"), - eventingtesting.WithTypes([]string{}), - eventingtesting.WithValidSink(namespace, "svc"), - ) - }, - wantError: GenerateInvalidSubscriptionError(testName, - eventingv1alpha2.EmptyErrDetail, eventingv1alpha2.TypesPath), - }, - { - name: "should fail to create subscription with invalid sink", - givenSubscriptionFunc: func(namespace string) *eventingv1alpha2.Subscription { - return eventingtesting.NewSubscription(testName, namespace, - eventingtesting.WithStandardTypeMatching(), - eventingtesting.WithSource("source"), - eventingtesting.WithOrderCreatedV1Event(), - eventingtesting.WithSink("https://svc2.test.local"), - ) - }, - wantError: GenerateInvalidSubscriptionError(testName, - eventingv1alpha2.SuffixMissingErrDetail, eventingv1alpha2.SinkPath), - }, - { - name: "should fail to create subscription with invalid protocol settings", - givenSubscriptionFunc: func(namespace string) *eventingv1alpha2.Subscription { - return eventingtesting.NewSubscription(testName, namespace, - eventingtesting.WithStandardTypeMatching(), - eventingtesting.WithSource("source"), - eventingtesting.WithOrderCreatedV1Event(), - eventingtesting.WithInvalidWebhookAuthGrantType(), - eventingtesting.WithValidSink(namespace, "svc"), - ) - }, - wantError: GenerateInvalidSubscriptionError(testName, - eventingv1alpha2.InvalidGrantTypeErrDetail, eventingv1alpha2.ConfigPath), - }, - } - - for _, tc := range testCases { - testcase := tc - t.Run(testcase.name, func(t *testing.T) { - t.Parallel() - ctx := context.Background() - - // create unique namespace for this test run - testNamespace := getTestNamespace() - ensureNamespaceCreated(ctx, t, testNamespace) - - // update namespace information in given test assets - givenSubscription := testcase.givenSubscriptionFunc(testNamespace) - - // attempt to create subscription - ensureK8sResourceNotCreated(ctx, t, givenSubscription, testcase.wantError) - }) - } -} - func Test_CreateSubscription(t *testing.T) { testCases := []struct { name string givenSubscriptionFunc func(namespace string) *eventingv1alpha2.Subscription - wantSubscriptionMatchers gomegatypes.GomegaMatcher + wantSubscriptionMatchers func(namespace string) gomegatypes.GomegaMatcher wantEventMeshSubMatchers gomegatypes.GomegaMatcher wantEventMeshSubCheck bool wantAPIRuleCheck bool @@ -156,13 +73,15 @@ func Test_CreateSubscription(t *testing.T) { eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, "invalid")), ) }, - wantSubscriptionMatchers: gomega.And( - eventingtesting.HaveSubscriptionNotReady(), - eventingtesting.HaveCondition(eventingv1alpha2.MakeCondition( - eventingv1alpha2.ConditionAPIRuleStatus, - eventingv1alpha2.ConditionReasonAPIRuleStatusNotReady, - kcorev1.ConditionFalse, invalidSinkErrMsg)), - ), + wantSubscriptionMatchers: func(namespace string) gomegatypes.GomegaMatcher { + return gomega.And( + eventingtesting.HaveSubscriptionNotReady(), + eventingtesting.HaveCondition(eventingv1alpha2.MakeCondition( + eventingv1alpha2.ConditionSubscriptionSpecValid, + eventingv1alpha2.ConditionReasonSubscriptionSpecHasValidationErrors, + kcorev1.ConditionFalse, fmt.Sprintf("Subscription validation failed: Sink validation failed: service invalid.%s not found in the cluster", namespace))), + ) + }, }, { name: "should succeed to create subscription if types are non-empty", @@ -177,19 +96,21 @@ func Test_CreateSubscription(t *testing.T) { eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, testName)), ) }, - wantSubscriptionMatchers: gomega.And( - eventingtesting.HaveSubscriptionFinalizer(eventingv1alpha2.Finalizer), - eventingtesting.HaveSubscriptionActiveCondition(), - eventingtesting.HaveCleanEventTypes([]eventingv1alpha2.EventType{ - { - OriginalType: fmt.Sprintf("%s0", eventingtesting.OrderCreatedV1EventNotClean), - CleanType: fmt.Sprintf("%s0", eventingtesting.OrderCreatedV1Event), - }, { - OriginalType: fmt.Sprintf("%s1", eventingtesting.OrderCreatedV1EventNotClean), - CleanType: fmt.Sprintf("%s1", eventingtesting.OrderCreatedV1Event), - }, - }), - ), + wantSubscriptionMatchers: func(namespace string) gomegatypes.GomegaMatcher { + return gomega.And( + eventingtesting.HaveSubscriptionFinalizer(eventingv1alpha2.Finalizer), + eventingtesting.HaveSubscriptionActiveCondition(), + eventingtesting.HaveCleanEventTypes([]eventingv1alpha2.EventType{ + { + OriginalType: fmt.Sprintf("%s0", eventingtesting.OrderCreatedV1EventNotClean), + CleanType: fmt.Sprintf("%s0", eventingtesting.OrderCreatedV1Event), + }, { + OriginalType: fmt.Sprintf("%s1", eventingtesting.OrderCreatedV1EventNotClean), + CleanType: fmt.Sprintf("%s1", eventingtesting.OrderCreatedV1Event), + }, + }), + ) + }, wantEventMeshSubMatchers: gomega.And( eventmeshsubmatchers.HaveEvents(emstypes.Events{ { @@ -216,9 +137,11 @@ func Test_CreateSubscription(t *testing.T) { eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, testName)), ) }, - wantSubscriptionMatchers: gomega.And( - eventingtesting.HaveSubscriptionActiveCondition(), - ), + wantSubscriptionMatchers: func(namespace string) gomegatypes.GomegaMatcher { + return gomega.And( + eventingtesting.HaveSubscriptionActiveCondition(), + ) + }, wantEventMeshSubMatchers: gomega.And( // should have default values for protocol and webhook auth eventmeshsubmatchers.HaveContentMode(emTestEnsemble.envConfig.ContentMode), @@ -244,16 +167,18 @@ func Test_CreateSubscription(t *testing.T) { eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, testName)), ) }, - wantSubscriptionMatchers: gomega.And( - eventingtesting.HaveSubscriptionFinalizer(eventingv1alpha2.Finalizer), - eventingtesting.HaveSubscriptionActiveCondition(), - eventingtesting.HaveCleanEventTypes([]eventingv1alpha2.EventType{ - { - OriginalType: eventingtesting.EventMeshExactType, - CleanType: eventingtesting.EventMeshExactType, - }, - }), - ), + wantSubscriptionMatchers: func(namespace string) gomegatypes.GomegaMatcher { + return gomega.And( + eventingtesting.HaveSubscriptionFinalizer(eventingv1alpha2.Finalizer), + eventingtesting.HaveSubscriptionActiveCondition(), + eventingtesting.HaveCleanEventTypes([]eventingv1alpha2.EventType{ + { + OriginalType: eventingtesting.EventMeshExactType, + CleanType: eventingtesting.EventMeshExactType, + }, + }), + ) + }, wantEventMeshSubMatchers: gomega.And( eventmeshsubmatchers.HaveEvents(emstypes.Events{ { @@ -277,16 +202,18 @@ func Test_CreateSubscription(t *testing.T) { eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, testName)), ) }, - wantSubscriptionMatchers: gomega.And( - eventingtesting.HaveSubscriptionFinalizer(eventingv1alpha2.Finalizer), - eventingtesting.HaveSubscriptionActiveCondition(), - eventingtesting.HaveCleanEventTypes([]eventingv1alpha2.EventType{ - { - OriginalType: eventingtesting.OrderCreatedV1EventNotClean, - CleanType: eventingtesting.OrderCreatedV1Event, - }, - }), - ), + wantSubscriptionMatchers: func(namespace string) gomegatypes.GomegaMatcher { + return gomega.And( + eventingtesting.HaveSubscriptionFinalizer(eventingv1alpha2.Finalizer), + eventingtesting.HaveSubscriptionActiveCondition(), + eventingtesting.HaveCleanEventTypes([]eventingv1alpha2.EventType{ + { + OriginalType: eventingtesting.OrderCreatedV1EventNotClean, + CleanType: eventingtesting.OrderCreatedV1Event, + }, + }), + ) + }, wantEventMeshSubMatchers: gomega.And( eventmeshsubmatchers.HaveEvents(emstypes.Events{ { @@ -322,7 +249,7 @@ func Test_CreateSubscription(t *testing.T) { ensureK8sResourceCreated(ctx, t, givenSubscription) // check if the subscription is as required - getSubscriptionAssert(ctx, g, givenSubscription).Should(testcase.wantSubscriptionMatchers) + getSubscriptionAssert(ctx, g, givenSubscription).Should(testcase.wantSubscriptionMatchers(testNamespace)) if testcase.wantAPIRuleCheck { // check if an APIRule was created for the subscription @@ -358,6 +285,84 @@ func Test_CreateSubscription(t *testing.T) { } } +func Test_defaulting(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + givenSubscriptionFunc func(namespace string) *eventingv1alpha2.Subscription + wantSubscriptionMatchers gomegatypes.GomegaMatcher + }{ + // TypeMatching + { + name: "should default the TypeMatching to standard if it is not configured", + givenSubscriptionFunc: func(namespace string) *eventingv1alpha2.Subscription { + return eventingtesting.NewSubscription(testName, namespace, + eventingtesting.WithDefaultSource(), + eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), + eventingtesting.WithWebhookAuthForEventMesh(), + eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, testName)), + ) + }, + wantSubscriptionMatchers: gomega.And( + eventingtesting.HaveTypeMatching(eventingv1alpha2.TypeMatchingStandard), + ), + }, + { + name: "should not change the TypeMatching from exact if it is configured", + givenSubscriptionFunc: func(namespace string) *eventingv1alpha2.Subscription { + return eventingtesting.NewSubscription(testName, namespace, + eventingtesting.WithTypeMatchingExact(), + eventingtesting.WithDefaultSource(), + eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), + eventingtesting.WithWebhookAuthForEventMesh(), + eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, testName)), + ) + }, + wantSubscriptionMatchers: gomega.And( + eventingtesting.HaveTypeMatching(eventingv1alpha2.TypeMatchingExact), + ), + }, + { + name: "should not change the TypeMatching from standard if it is configured", + givenSubscriptionFunc: func(namespace string) *eventingv1alpha2.Subscription { + return eventingtesting.NewSubscription(testName, namespace, + eventingtesting.WithTypeMatchingStandard(), + eventingtesting.WithDefaultSource(), + eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), + eventingtesting.WithWebhookAuthForEventMesh(), + eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, testName)), + ) + }, + wantSubscriptionMatchers: gomega.And( + eventingtesting.HaveTypeMatching(eventingv1alpha2.TypeMatchingStandard), + ), + }, + } + + for _, testcase := range testCases { + t.Run(testcase.name, func(t *testing.T) { + t.Parallel() + + g := gomega.NewGomegaWithT(t) + ctx := context.Background() + + // given + testNamespace := getTestNamespace() + givenSubscription := testcase.givenSubscriptionFunc(testNamespace) + subscriberSvc := eventingtesting.NewSubscriberSvc(givenSubscription.Name, testNamespace) + + // when + ensureNamespaceCreated(ctx, t, testNamespace) + ensureK8sResourceCreated(ctx, t, subscriberSvc) + ensureK8sResourceCreated(ctx, t, givenSubscription) + + // then + getSubscriptionAssert(ctx, g, givenSubscription).Should(testcase.wantSubscriptionMatchers) + }) + } +} + func Test_UpdateSubscription(t *testing.T) { t.Parallel() testCases := []struct { @@ -618,14 +623,16 @@ func Test_FixingSinkAndApiRule(t *testing.T) { ) } - wantSubscriptionWithoutSinkMatchers := gomega.And( - eventingtesting.HaveCondition(eventingv1alpha2.MakeCondition( - eventingv1alpha2.ConditionAPIRuleStatus, - eventingv1alpha2.ConditionReasonAPIRuleStatusNotReady, - kcorev1.ConditionFalse, - invalidSinkErrMsg, - )), - ) + wantSubscriptionWithoutSinkMatchers := func(namespace string) gomegatypes.GomegaMatcher { + return gomega.And( + eventingtesting.HaveCondition(eventingv1alpha2.MakeCondition( + eventingv1alpha2.ConditionSubscriptionSpecValid, + eventingv1alpha2.ConditionReasonSubscriptionSpecHasValidationErrors, + kcorev1.ConditionFalse, + fmt.Sprintf("Subscription validation failed: Sink validation failed: service invalid.%s not found in the cluster", namespace), + )), + ) + } givenUpdateSubscriptionWithSinkFunc := func(namespace, name, sinkFormat, path string) *eventingv1alpha2.Subscription { return eventingtesting.NewSubscription(name, namespace, @@ -683,7 +690,7 @@ func Test_FixingSinkAndApiRule(t *testing.T) { ensureK8sResourceCreated(ctx, t, givenSubscription) createdSubscription := givenSubscription.DeepCopy() // check if the created subscription is correct - getSubscriptionAssert(ctx, g, createdSubscription).Should(wantSubscriptionWithoutSinkMatchers) + getSubscriptionAssert(ctx, g, createdSubscription).Should(wantSubscriptionWithoutSinkMatchers(testNamespace)) // update subscription with valid sink givenUpdateSubscription.ResourceVersion = createdSubscription.ResourceVersion @@ -814,7 +821,7 @@ func Test_APIRuleReUseAfterUpdatingSink(t *testing.T) { // phase 1: Create the first Subscription with ready APIRule and ready status. // create a subscriber service - sub1Name := fmt.Sprintf("test-sink-%s", testNamespace) + sub1Name := fmt.Sprintf("test-sink-%s-1", testNamespace) subscriberSvc1 := eventingtesting.NewSubscriberSvc(sub1Name, testNamespace) ensureK8sResourceCreated(ctx, t, subscriberSvc1) // create subscription @@ -827,7 +834,10 @@ func Test_APIRuleReUseAfterUpdatingSink(t *testing.T) { createdSubscription1 := givenSubscription1.DeepCopy() // wait until the APIRule is assigned to the created subscription - getSubscriptionAssert(ctx, g, createdSubscription1).Should(eventingtesting.HaveNoneEmptyAPIRuleName()) + getSubscriptionAssert(ctx, g, createdSubscription1).Should(gomega.And( + eventingtesting.HaveNoneEmptyAPIRuleName(), + eventingtesting.HaveSubscriptionActiveCondition(), + )) // fetch the APIRule and update the status of the APIRule to ready (mocking APIGateway controller) // and wait until the created Subscription becomes ready @@ -857,8 +867,13 @@ func Test_APIRuleReUseAfterUpdatingSink(t *testing.T) { createdSubscription2 := givenSubscription2.DeepCopy() // wait until the APIRule is assigned to the created subscription - getSubscriptionAssert(ctx, g, createdSubscription2).Should(eventingtesting.HaveNoneEmptyAPIRuleName()) - getSubscriptionAssert(ctx, g, createdSubscription2).ShouldNot(eventingtesting.HaveAPIRuleName(apiRule1.Name)) + getSubscriptionAssert(ctx, g, createdSubscription2).Should(gomega.And( + eventingtesting.HaveNoneEmptyAPIRuleName(), + eventingtesting.HaveSubscriptionActiveCondition(), + )) + getSubscriptionAssert(ctx, g, createdSubscription2).ShouldNot( + eventingtesting.HaveAPIRuleName(apiRule1.Name), + ) // fetch the APIRule and update the status of the APIRule to ready (mocking APIGateway controller) // and wait until the created Subscription becomes ready @@ -883,6 +898,7 @@ func Test_APIRuleReUseAfterUpdatingSink(t *testing.T) { getSubscriptionAssert(ctx, g, updatedSubscription2).Should(gomega.And( eventingtesting.HaveSubscriptionReady(), eventingtesting.HaveAPIRuleName(apiRule1.Name), + eventingtesting.HaveSubscriptionActiveCondition(), )) // check if the EventMesh Subscription has the correct webhook URL diff --git a/internal/controller/eventing/subscription/eventmesh/test/utils.go b/internal/controller/eventing/subscription/eventmesh/test/utils.go index 4038b79d3..26e9a8bdf 100644 --- a/internal/controller/eventing/subscription/eventmesh/test/utils.go +++ b/internal/controller/eventing/subscription/eventmesh/test/utils.go @@ -1,12 +1,9 @@ -//nolint:gosec //this is just a test, and security issues found here will not result in code used in a prod environment package test import ( "context" - "crypto/tls" "fmt" "log" - "net" "net/http" "path/filepath" "strings" @@ -23,7 +20,6 @@ import ( kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" klabels "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" kctrl "sigs.k8s.io/controller-runtime" @@ -33,14 +29,13 @@ import ( kctrllog "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/metrics/server" - "sigs.k8s.io/controller-runtime/pkg/webhook" eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" subscriptioncontrollereventmesh "github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/eventmesh" + "github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/validator" "github.com/kyma-project/eventing-manager/pkg/backend/cleaner" backendeventmesh "github.com/kyma-project/eventing-manager/pkg/backend/eventmesh" "github.com/kyma-project/eventing-manager/pkg/backend/metrics" - "github.com/kyma-project/eventing-manager/pkg/backend/sink" backendutils "github.com/kyma-project/eventing-manager/pkg/backend/utils" "github.com/kyma-project/eventing-manager/pkg/constants" emstypes "github.com/kyma-project/eventing-manager/pkg/ems/api/events/types" @@ -89,7 +84,6 @@ func setupSuite() error { emTestEnsemble = &eventMeshTestEnsemble{} // define logger - var err error defaultLogger, err := logger.New(string(kymalogger.JSON), string(kymalogger.INFO)) if err != nil { return err @@ -116,7 +110,7 @@ func setupSuite() error { // +kubebuilder:scaffold:scheme // setup eventMesh manager instance - k8sManager, webhookInstallOptions, err := setupManager(cfg) + k8sManager, err := setupManager(cfg) if err != nil { return err } @@ -127,11 +121,13 @@ func setupSuite() error { // setup eventMesh reconciler recorder := k8sManager.GetEventRecorderFor("eventing-controller") - sinkValidator := sink.NewValidator(k8sManager.GetClient(), recorder) emTestEnsemble.envConfig = getEnvConfig() eventMesh, credentials := setupEventMesh(defaultLogger) + // Init the Subscription validator. + subscriptionValidator := validator.NewSubscriptionValidator(k8sManager.GetClient()) + col := metrics.NewCollector() testReconciler := subscriptioncontrollereventmesh.NewReconciler( k8sManager.GetClient(), @@ -142,7 +138,7 @@ func setupSuite() error { eventMesh, credentials, emTestEnsemble.nameMapper, - sinkValidator, + subscriptionValidator, col, testutils.Domain, ) @@ -163,28 +159,22 @@ func setupSuite() error { emTestEnsemble.k8sClient = k8sManager.GetClient() - return startAndWaitForWebhookServer(k8sManager, webhookInstallOptions) + return nil } -func setupManager(cfg *rest.Config) (manager.Manager, *envtest.WebhookInstallOptions, error) { +func setupManager(cfg *rest.Config) (manager.Manager, 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 nil, err } - return k8sManager, webhookInstallOptions, nil + return k8sManager, nil } func setupEventMesh(defaultLogger *logger.Logger) (*backendeventmesh.EventMesh, *backendeventmesh.OAuth2ClientCredentials) { @@ -198,23 +188,6 @@ func setupEventMesh(defaultLogger *logger.Logger) (*backendeventmesh.EventMesh, return eventMesh, credentials } -func startAndWaitForWebhookServer(k8sManager manager.Manager, webhookInstallOpts *envtest.WebhookInstallOptions) error { - if err := (&eventingv1alpha2.Subscription{}).SetupWebhookWithManager(k8sManager); err != nil { - return err - } - dialer := &net.Dialer{Timeout: time.Second} - addrPort := fmt.Sprintf("%s:%d", webhookInstallOpts.LocalServingHost, webhookInstallOpts.LocalServingPort) - // wait for the webhook server to get ready - err := retry.Do(func() error { - conn, connErr := tls.DialWithDialer(dialer, "tcp", addrPort, &tls.Config{InsecureSkipVerify: true}) - if connErr != nil { - return connErr - } - return conn.Close() - }, retry.Attempts(maxReconnects)) - return err -} - func startTestEnv() (*rest.Config, error) { useExistingCluster := useExistingCluster emTestEnsemble.testEnv = &envtest.Environment{ @@ -224,9 +197,6 @@ func startTestEnv() (*rest.Config, error) { }, AttachControlPlaneOutput: attachControlPlaneOutput, UseExistingCluster: &useExistingCluster, - WebhookInstallOptions: envtest.WebhookInstallOptions{ - Paths: []string{filepath.Join("../../../../../../", "config", "webhook")}, - }, } var cfg *rest.Config @@ -282,15 +252,6 @@ func startNewEventMeshMock() *eventingtesting.EventMeshMock { return emMock } -func GenerateInvalidSubscriptionError(subName, errType string, path *field.Path) error { - webhookErr := "admission webhook \"vsubscription.kb.io\" denied the request: " - givenError := kerrors.NewInvalid( - eventingv1alpha2.GroupKind, subName, - field.ErrorList{eventingv1alpha2.MakeInvalidFieldError(path, subName, errType)}) - givenError.ErrStatus.Message = webhookErr + givenError.ErrStatus.Message - return givenError -} - func getTestNamespace() string { return fmt.Sprintf("ns-%s", utils.GetRandString(namespacePrefixLength)) } @@ -326,11 +287,6 @@ func ensureK8sResourceCreated(ctx context.Context, t *testing.T, obj client.Obje 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)) diff --git a/internal/controller/eventing/subscription/eventmesh/testwebhookauth/utils.go b/internal/controller/eventing/subscription/eventmesh/testwebhookauth/utils.go index e3c515056..e9ed43ffa 100644 --- a/internal/controller/eventing/subscription/eventmesh/testwebhookauth/utils.go +++ b/internal/controller/eventing/subscription/eventmesh/testwebhookauth/utils.go @@ -3,10 +3,8 @@ package testwebhookauth import ( "context" - "crypto/tls" "fmt" "log" - "net" "path/filepath" "testing" "time" @@ -29,14 +27,13 @@ import ( kctrllog "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/metrics/server" - "sigs.k8s.io/controller-runtime/pkg/webhook" eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" subscriptioncontrollereventmesh "github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/eventmesh" + "github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/validator" "github.com/kyma-project/eventing-manager/pkg/backend/cleaner" backendeventmesh "github.com/kyma-project/eventing-manager/pkg/backend/eventmesh" "github.com/kyma-project/eventing-manager/pkg/backend/metrics" - "github.com/kyma-project/eventing-manager/pkg/backend/sink" backendutils "github.com/kyma-project/eventing-manager/pkg/backend/utils" emstypes "github.com/kyma-project/eventing-manager/pkg/ems/api/events/types" "github.com/kyma-project/eventing-manager/pkg/env" @@ -89,7 +86,6 @@ func setupSuite() (*eventMeshTestEnsemble, error) { emTestEnsemble := &eventMeshTestEnsemble{} // define logger - var err error defaultLogger, err := logger.New(string(kymalogger.JSON), string(kymalogger.INFO)) if err != nil { return nil, err @@ -116,7 +112,7 @@ func setupSuite() (*eventMeshTestEnsemble, error) { // +kubebuilder:scaffold:scheme // start eventMesh manager instance - k8sManager, webhookInstallOptions, err := setupManager(emTestEnsemble, cfg) + k8sManager, err := setupManager(cfg) if err != nil { return nil, err } @@ -127,10 +123,13 @@ func setupSuite() (*eventMeshTestEnsemble, error) { // setup eventMesh reconciler recorder := k8sManager.GetEventRecorderFor("eventing-controller") - sinkValidator := sink.NewValidator(k8sManager.GetClient(), recorder) emTestEnsemble.envConfig = getEnvConfig(emTestEnsemble) eventMeshBackend = backendeventmesh.NewEventMesh(credentials, emTestEnsemble.nameMapper, defaultLogger) col := metrics.NewCollector() + + // Init the Subscription validator. + subscriptionValidator := validator.NewSubscriptionValidator(k8sManager.GetClient()) + testReconciler = subscriptioncontrollereventmesh.NewReconciler( k8sManager.GetClient(), defaultLogger, @@ -140,7 +139,7 @@ func setupSuite() (*eventMeshTestEnsemble, error) { eventMeshBackend, credentials, emTestEnsemble.nameMapper, - sinkValidator, + subscriptionValidator, col, testutils.Domain, ) @@ -161,41 +160,18 @@ func setupSuite() (*eventMeshTestEnsemble, error) { emTestEnsemble.k8sClient = k8sManager.GetClient() - return emTestEnsemble, startAndWaitForWebhookServer(k8sManager, webhookInstallOptions) + return emTestEnsemble, err } -func setupManager(ensemble *eventMeshTestEnsemble, cfg *rest.Config) (manager.Manager, *envtest.WebhookInstallOptions, error) { +func setupManager(cfg *rest.Config) (manager.Manager, error) { syncPeriod := syncPeriodSeconds * time.Second - webhookInstallOptions := &ensemble.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 - } - dialer := &net.Dialer{Timeout: time.Second} - addrPort := fmt.Sprintf("%s:%d", installOpts.LocalServingHost, installOpts.LocalServingPort) - // wait for the webhook server to get ready - err := retry.Do(func() error { - conn, connErr := tls.DialWithDialer(dialer, "tcp", addrPort, &tls.Config{InsecureSkipVerify: true}) - if connErr != nil { - return connErr - } - return conn.Close() - }, retry.Attempts(maxReconnects)) - return err + return k8sManager, err } func startTestEnv(ensemble *eventMeshTestEnsemble) (*rest.Config, error) { @@ -206,9 +182,6 @@ func startTestEnv(ensemble *eventMeshTestEnsemble) (*rest.Config, error) { }, AttachControlPlaneOutput: attachControlPlaneOutput, UseExistingCluster: utils.BoolPtr(useExistingCluster), - WebhookInstallOptions: envtest.WebhookInstallOptions{ - Paths: []string{filepath.Join("../../../../../../", "config", "webhook")}, - }, } var cfg *rest.Config diff --git a/internal/controller/eventing/subscription/jetstream/reconciler.go b/internal/controller/eventing/subscription/jetstream/reconciler.go index 907c655e5..199f2f027 100644 --- a/internal/controller/eventing/subscription/jetstream/reconciler.go +++ b/internal/controller/eventing/subscription/jetstream/reconciler.go @@ -2,6 +2,7 @@ package jetstream import ( "context" + "errors" "reflect" "time" @@ -15,16 +16,17 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" + "github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/validator" "github.com/kyma-project/eventing-manager/internal/controller/events" "github.com/kyma-project/eventing-manager/pkg/backend/cleaner" "github.com/kyma-project/eventing-manager/pkg/backend/jetstream" "github.com/kyma-project/eventing-manager/pkg/backend/metrics" - "github.com/kyma-project/eventing-manager/pkg/backend/sink" backendutils "github.com/kyma-project/eventing-manager/pkg/backend/utils" - "github.com/kyma-project/eventing-manager/pkg/errors" + emerrors "github.com/kyma-project/eventing-manager/pkg/errors" "github.com/kyma-project/eventing-manager/pkg/logger" "github.com/kyma-project/eventing-manager/pkg/object" "github.com/kyma-project/eventing-manager/pkg/utils" @@ -38,28 +40,28 @@ const ( type Reconciler struct { client.Client - Backend jetstream.Backend - recorder record.EventRecorder - logger *logger.Logger - cleaner cleaner.Cleaner - sinkValidator sink.Validator - customEventsChannel chan event.GenericEvent - collector *metrics.Collector + Backend jetstream.Backend + recorder record.EventRecorder + logger *logger.Logger + cleaner cleaner.Cleaner + subscriptionValidator validator.SubscriptionValidator + customEventsChannel chan event.GenericEvent + collector *metrics.Collector } func NewReconciler(client client.Client, jsBackend jetstream.Backend, logger *logger.Logger, recorder record.EventRecorder, cleaner cleaner.Cleaner, - defaultSinkValidator sink.Validator, collector *metrics.Collector, + subscriptionValidator validator.SubscriptionValidator, collector *metrics.Collector, ) *Reconciler { reconciler := &Reconciler{ - Client: client, - Backend: jsBackend, - recorder: recorder, - logger: logger, - cleaner: cleaner, - sinkValidator: defaultSinkValidator, - customEventsChannel: make(chan event.GenericEvent), - collector: collector, + Client: client, + Backend: jsBackend, + recorder: recorder, + logger: logger, + cleaner: cleaner, + subscriptionValidator: subscriptionValidator, + customEventsChannel: make(chan event.GenericEvent), + collector: collector, } return reconciler } @@ -105,8 +107,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req kctrl.Request) (kctrl.Re // fetch current subscription object and ensure the object was not deleted in the meantime currentSubscription := &eventingv1alpha2.Subscription{} - err := r.Client.Get(ctx, req.NamespacedName, currentSubscription) - if err != nil { + if err := r.Client.Get(ctx, req.NamespacedName, currentSubscription); err != nil { return kctrl.Result{}, client.IgnoreNotFound(err) } @@ -129,32 +130,25 @@ func (r *Reconciler) Reconcile(ctx context.Context, req kctrl.Request) (kctrl.Re return r.addFinalizer(ctx, desiredSubscription) } - // update the cleanEventTypes and config values in the subscription status, if changed - if err = r.syncEventTypes(desiredSubscription); err != nil { - if syncErr := r.syncSubscriptionStatus(ctx, desiredSubscription, err, log); syncErr != nil { - return kctrl.Result{}, err + // Validate subscription. + if validationErr := r.validateSubscription(ctx, desiredSubscription); validationErr != nil { + if errors.Is(validationErr, validator.ErrSinkValidationFailed) { + if deleteErr := r.Backend.DeleteSubscriptionsOnly(desiredSubscription); deleteErr != nil { + log.Errorw("Failed to delete JetStream subscriptions", "error", deleteErr) + return kctrl.Result{}, deleteErr + } } - return kctrl.Result{}, err - } - - // Check for valid sink - if err := r.sinkValidator.Validate(ctx, desiredSubscription); err != nil { - if deleteErr := r.Backend.DeleteSubscriptionsOnly(desiredSubscription); deleteErr != nil { - r.namedLogger().Errorw( - "Failed to delete JetStream subscriptions", - "namespace", desiredSubscription.Namespace, - "name", desiredSubscription.Name, - "error", deleteErr, - ) - return kctrl.Result{}, deleteErr + if updateErr := r.updateSubscriptionStatus(ctx, desiredSubscription, log); updateErr != nil { + return kctrl.Result{}, errors.Join(validationErr, updateErr) } + return kctrl.Result{}, reconcile.TerminalError(validationErr) + } - // No point in reconciling as the sink is invalid, - // return latest error to requeue the reconciliation request + // update the cleanEventTypes and config values in the subscription status, if changed + if err := r.syncEventTypes(desiredSubscription); err != nil { if syncErr := r.syncSubscriptionStatus(ctx, desiredSubscription, err, log); syncErr != nil { return kctrl.Result{}, err } - // No point in reconciling as the sink is invalid, return latest error to requeue the reconciliation request return kctrl.Result{}, err } @@ -177,6 +171,18 @@ func (r *Reconciler) Reconcile(ctx context.Context, req kctrl.Request) (kctrl.Re return kctrl.Result{}, r.syncSubscriptionStatus(ctx, desiredSubscription, nil, log) } +func (r *Reconciler) validateSubscription(ctx context.Context, subscription *eventingv1alpha2.Subscription) error { + var err error + if err = r.subscriptionValidator.Validate(ctx, *subscription); err != nil { + subscription.Status.SetNotReady() + subscription.Status.ClearTypes() + subscription.Status.ClearBackend() + subscription.Status.ClearConditions() + } + subscription.Status.SetSubscriptionSpecValidCondition(err) + return err +} + func (r *Reconciler) updateSubscriptionMetrics(current, desired *eventingv1alpha2.Subscription) { for _, currentType := range current.Status.Backend.Types { found := false @@ -239,7 +245,7 @@ func (r *Reconciler) handleSubscriptionDeletion(ctx context.Context, } if err := r.Backend.DeleteSubscription(subscription); err != nil { - deleteSubErr := errors.MakeError(errFailedToDeleteSub, err) + deleteSubErr := emerrors.MakeError(errFailedToDeleteSub, err) // if failed to delete the external dependency here, return with error // so that it can be retried if syncErr := r.syncSubscriptionStatus(ctx, subscription, deleteSubErr, log); syncErr != nil { @@ -255,7 +261,7 @@ func (r *Reconciler) handleSubscriptionDeletion(ctx context.Context, // update the subscription's finalizers in k8s if err := r.Update(ctx, subscription); err != nil { - return kctrl.Result{}, errors.MakeError(errFailedToUpdateFinalizers, err) + return kctrl.Result{}, emerrors.MakeError(errFailedToUpdateFinalizers, err) } for _, t := range types { @@ -278,8 +284,8 @@ func (r *Reconciler) syncSubscriptionStatus(ctx context.Context, // set ready state desiredSubscription.Status.Ready = err == nil - // compile the desired conditions - desiredSubscription.Status.Conditions = eventingv1alpha2.GetSubscriptionActiveCondition(desiredSubscription, err) + // set the desired conditions + eventingv1alpha2.SetSubscriptionActiveCondition(&desiredSubscription.Status, err) // Update the subscription return r.updateSubscriptionStatus(ctx, desiredSubscription, log) @@ -306,7 +312,7 @@ func (r *Reconciler) updateSubscriptionStatus(ctx context.Context, // sync subscription status with k8s if err := r.updateStatus(ctx, actualSubscription, desiredSubscription, logger); err != nil { - return errors.MakeError(errFailedToUpdateStatus, err) + return emerrors.MakeError(errFailedToUpdateStatus, err) } return nil @@ -325,7 +331,7 @@ func (r *Reconciler) updateStatus(ctx context.Context, oldSubscription, if err := r.Status().Update(ctx, newSubscription); err != nil { events.Warn(r.recorder, newSubscription, events.ReasonUpdateFailed, "Update Subscription status failed %s", newSubscription.Name) - return errors.MakeError(errFailedToUpdateStatus, err) + return emerrors.MakeError(errFailedToUpdateStatus, err) } events.Normal(r.recorder, newSubscription, events.ReasonUpdate, "Update Subscription status succeeded %s", newSubscription.Name) @@ -342,7 +348,7 @@ func (r *Reconciler) addFinalizer(ctx context.Context, sub *eventingv1alpha2.Sub // update the subscription's finalizers in k8s if err := r.Update(ctx, sub); err != nil { - return kctrl.Result{}, errors.MakeError(errFailedToUpdateFinalizers, err) + return kctrl.Result{}, emerrors.MakeError(errFailedToUpdateFinalizers, err) } return kctrl.Result{}, nil diff --git a/internal/controller/eventing/subscription/jetstream/reconciler_integration_test.go b/internal/controller/eventing/subscription/jetstream/reconciler_integration_test.go index d481df71d..314894b58 100644 --- a/internal/controller/eventing/subscription/jetstream/reconciler_integration_test.go +++ b/internal/controller/eventing/subscription/jetstream/reconciler_integration_test.go @@ -11,7 +11,6 @@ import ( gomegatypes "github.com/onsi/gomega/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - kcorev1 "k8s.io/api/core/v1" eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" eventingtesting "github.com/kyma-project/eventing-manager/testing" @@ -38,83 +37,6 @@ func TestMain(m *testing.M) { os.Exit(code) } -func Test_ValidationWebhook(t *testing.T) { - t.Parallel() - testCases := []struct { - name string - givenSubscriptionOpts []eventingtesting.SubscriptionOpt - wantError func(subName string) error - }{ - { - name: "should fail to create subscription with invalid event source", - givenSubscriptionOpts: []eventingtesting.SubscriptionOpt{ - eventingtesting.WithStandardTypeMatching(), - eventingtesting.WithSource(""), - eventingtesting.WithOrderCreatedV1Event(), - eventingtesting.WithSinkURLFromSvc(jsTestEnsemble.SubscriberSvc), - }, - wantError: func(subName string) error { - return GenerateInvalidSubscriptionError(subName, - eventingv1alpha2.EmptyErrDetail, eventingv1alpha2.SourcePath) - }, - }, - { - name: "should fail to create subscription with invalid event types", - givenSubscriptionOpts: []eventingtesting.SubscriptionOpt{ - eventingtesting.WithStandardTypeMatching(), - eventingtesting.WithSource("source"), - eventingtesting.WithTypes([]string{}), - eventingtesting.WithSinkURLFromSvc(jsTestEnsemble.SubscriberSvc), - }, - wantError: func(subName string) error { - return GenerateInvalidSubscriptionError(subName, - eventingv1alpha2.EmptyErrDetail, eventingv1alpha2.TypesPath) - }, - }, - { - name: "should fail to create subscription with invalid config", - givenSubscriptionOpts: []eventingtesting.SubscriptionOpt{ - eventingtesting.WithStandardTypeMatching(), - eventingtesting.WithSource("source"), - eventingtesting.WithOrderCreatedV1Event(), - eventingtesting.WithSinkURLFromSvc(jsTestEnsemble.SubscriberSvc), - eventingtesting.WithMaxInFlightMessages("invalid"), - }, - wantError: func(subName string) error { - return GenerateInvalidSubscriptionError(subName, - eventingv1alpha2.StringIntErrDetail, eventingv1alpha2.ConfigPath) - }, - }, - { - name: "should fail to create subscription with invalid sink", - givenSubscriptionOpts: []eventingtesting.SubscriptionOpt{ - eventingtesting.WithStandardTypeMatching(), - eventingtesting.WithSource("source"), - eventingtesting.WithOrderCreatedV1Event(), - eventingtesting.WithSink("https://svc2.test.local"), - }, - wantError: func(subName string) error { - return GenerateInvalidSubscriptionError(subName, - eventingv1alpha2.SuffixMissingErrDetail, eventingv1alpha2.SinkPath) - }, - }, - } - - for _, tc := range testCases { - testcase := tc - t.Run(testcase.name, func(t *testing.T) { - t.Parallel() - t.Log("creating the k8s subscription") - sub := NewSubscription(jsTestEnsemble.Ensemble, testcase.givenSubscriptionOpts...) - - EnsureNamespaceCreatedForSub(t, jsTestEnsemble.Ensemble, sub) - - // attempt to create subscription - EnsureK8sResourceNotCreated(t, jsTestEnsemble.Ensemble, sub, testcase.wantError(sub.Name)) - }) - } -} - // TestUnavailableNATSServer tests if a subscription is reconciled properly when the NATS backend is unavailable. func Test_UnavailableNATSServer(t *testing.T) { g := gomega.NewGomegaWithT(t) @@ -258,17 +180,15 @@ func Test_CreateSubscription(t *testing.T) { eventingtesting.WithSinkURL( eventingtesting.ValidSinkURL(jsTestEnsemble.SubscriberSvc.Namespace, "testapp"), ), + eventingtesting.WithMaxInFlight(10), }, want: Want{ K8sSubscription: []gomegatypes.GomegaMatcher{ eventingtesting.HaveCondition( ConditionInvalidSink( - "failed to validate subscription sink URL. It is not a valid cluster local svc: Service \"testapp\" not found", + "Subscription validation failed: Sink validation failed: service testapp.test not found in the cluster", )), }, - K8sEvents: []kcorev1.Event{ - EventInvalidSink("Sink does not correspond to a valid cluster local svc"), - }, }, }, { @@ -331,6 +251,110 @@ func Test_CreateSubscription(t *testing.T) { } } +func Test_defaulting(t *testing.T) { + t.Parallel() + + const ( + maxInFlightMessages = 3 + defaultMaxInFlightMessages = 10 + ) + + testCases := []struct { + name string + givenSubscriptionOpts []eventingtesting.SubscriptionOpt + want Want + }{ + // TypeMatching + { + name: "should default the TypeMatching to standard if it is not configured", + givenSubscriptionOpts: []eventingtesting.SubscriptionOpt{ + eventingtesting.WithSourceAndType(eventingtesting.EventSource, eventingtesting.OrderCreatedEventType), + eventingtesting.WithSinkURLFromSvc(jsTestEnsemble.SubscriberSvc), + eventingtesting.WithMaxInFlight(defaultMaxInFlightMessages), + eventingtesting.WithFinalizers([]string{}), + }, + want: Want{ + K8sSubscription: []gomegatypes.GomegaMatcher{ + eventingtesting.HaveTypeMatching(eventingv1alpha2.TypeMatchingStandard), + }, + }, + }, + { + name: "should not change the TypeMatching from exact if it is configured", + givenSubscriptionOpts: []eventingtesting.SubscriptionOpt{ + eventingtesting.WithTypeMatchingExact(), + eventingtesting.WithSourceAndType(eventingtesting.EventSource, eventingtesting.OrderCreatedEventType), + eventingtesting.WithSinkURLFromSvc(jsTestEnsemble.SubscriberSvc), + eventingtesting.WithMaxInFlight(defaultMaxInFlightMessages), + eventingtesting.WithFinalizers([]string{}), + }, + want: Want{ + K8sSubscription: []gomegatypes.GomegaMatcher{ + eventingtesting.HaveTypeMatching(eventingv1alpha2.TypeMatchingExact), + }, + }, + }, + { + name: "should not change the TypeMatching from standard if it is configured", + givenSubscriptionOpts: []eventingtesting.SubscriptionOpt{ + eventingtesting.WithTypeMatchingStandard(), + eventingtesting.WithSourceAndType(eventingtesting.EventSource, eventingtesting.OrderCreatedEventType), + eventingtesting.WithSinkURLFromSvc(jsTestEnsemble.SubscriberSvc), + eventingtesting.WithMaxInFlight(defaultMaxInFlightMessages), + eventingtesting.WithFinalizers([]string{}), + }, + want: Want{ + K8sSubscription: []gomegatypes.GomegaMatcher{ + eventingtesting.HaveTypeMatching(eventingv1alpha2.TypeMatchingStandard), + }, + }, + }, + // MaxInFlightMessages + { + name: "should default the MaxInFlightMessages if it is not configured", + givenSubscriptionOpts: []eventingtesting.SubscriptionOpt{ + eventingtesting.WithTypeMatchingStandard(), + eventingtesting.WithSourceAndType(eventingtesting.EventSource, eventingtesting.OrderCreatedEventType), + eventingtesting.WithSinkURLFromSvc(jsTestEnsemble.SubscriberSvc), + eventingtesting.WithFinalizers([]string{}), + }, + want: Want{ + K8sSubscription: []gomegatypes.GomegaMatcher{ + eventingtesting.HaveMaxInFlight(defaultMaxInFlightMessages), + }, + }, + }, + { + name: "should not change the MaxInFlightMessages if it is configured", + givenSubscriptionOpts: []eventingtesting.SubscriptionOpt{ + eventingtesting.WithTypeMatchingStandard(), + eventingtesting.WithSourceAndType(eventingtesting.EventSource, eventingtesting.OrderCreatedEventType), + eventingtesting.WithSinkURLFromSvc(jsTestEnsemble.SubscriberSvc), + eventingtesting.WithMaxInFlight(maxInFlightMessages), + eventingtesting.WithFinalizers([]string{}), + }, + want: Want{ + K8sSubscription: []gomegatypes.GomegaMatcher{ + eventingtesting.HaveMaxInFlight(maxInFlightMessages), + }, + }, + }, + } + + for _, testcase := range testCases { + t.Run(testcase.name, func(t *testing.T) { + t.Parallel() + g := gomega.NewGomegaWithT(t) + + // when + subscription := CreateSubscription(t, jsTestEnsemble.Ensemble, testcase.givenSubscriptionOpts...) + + // then + CheckSubscriptionOnK8s(g, jsTestEnsemble.Ensemble, subscription, testcase.want.K8sSubscription...) + }) + } +} + // Test_ChangeSubscription tests if existing subscriptions are reconciled properly after getting changed. func Test_ChangeSubscription(t *testing.T) { t.Parallel() 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 b87121da3..7212dce73 100644 --- a/internal/controller/eventing/subscription/jetstream/reconciler_internal_unit_test.go +++ b/internal/controller/eventing/subscription/jetstream/reconciler_internal_unit_test.go @@ -17,13 +17,15 @@ import ( kctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" + "github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/validator" + subscriptionvalidatormocks "github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/validator/mocks" "github.com/kyma-project/eventing-manager/pkg/backend/cleaner" "github.com/kyma-project/eventing-manager/pkg/backend/jetstream" backendjetstreammocks "github.com/kyma-project/eventing-manager/pkg/backend/jetstream/mocks" "github.com/kyma-project/eventing-manager/pkg/backend/metrics" - "github.com/kyma-project/eventing-manager/pkg/backend/sink" "github.com/kyma-project/eventing-manager/pkg/env" "github.com/kyma-project/eventing-manager/pkg/logger" eventingtesting "github.com/kyma-project/eventing-manager/testing" @@ -48,6 +50,8 @@ func Test_Reconcile(t *testing.T) { eventingtesting.WithFinalizers([]string{eventingv1alpha2.Finalizer}), eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), + eventingtesting.WithMaxInFlight(10), + eventingtesting.WithSink("http://test.test.svc.cluster.local"), ) // A subscription marked for deletion. testSubUnderDeletion := eventingtesting.NewSubscription("sub2", namespaceName, @@ -61,8 +65,8 @@ func Test_Reconcile(t *testing.T) { missingSubSyncErr := jetstream.ErrMissingSubscription backendDeleteErr := errors.New("backend delete error") validatorErr := errors.New("invalid sink") - happyValidator := sink.ValidatorFunc(func(_ context.Context, s *eventingv1alpha2.Subscription) error { return nil }) - unhappyValidator := sink.ValidatorFunc(func(_ context.Context, s *eventingv1alpha2.Subscription) error { return validatorErr }) + happyValidator := validator.SubscriptionValidatorFunc(func(_ context.Context, _ eventingv1alpha2.Subscription) error { return nil }) + unhappyValidator := validator.SubscriptionValidatorFunc(func(_ context.Context, _ eventingv1alpha2.Subscription) error { return validatorErr }) collector := metrics.NewCollector() testCases := []struct { @@ -214,7 +218,7 @@ func Test_Reconcile(t *testing.T) { testenv.Backend }, wantReconcileResult: kctrl.Result{}, - wantReconcileError: validatorErr, + wantReconcileError: reconcile.TerminalError(validatorErr), }, } @@ -234,7 +238,9 @@ func Test_Reconcile(t *testing.T) { // then req.Equal(testcase.wantReconcileResult, res) req.ErrorIs(err, testcase.wantReconcileError) - mockedBackend.AssertExpectations(t) + if testcase.wantReconcileError == nil { + mockedBackend.AssertExpectations(t) + } }) } } @@ -717,6 +723,47 @@ func Test_updateSubscription(t *testing.T) { } } +func Test_validateSubscription(t *testing.T) { + // given + subscription := eventingtesting.NewSubscription("test-subscription", namespaceName, + eventingtesting.WithFinalizers([]string{eventingv1alpha2.Finalizer}), + eventingtesting.WithSource(eventingtesting.EventSourceClean), + eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), + eventingtesting.WithMaxInFlight(10), + eventingtesting.WithSink("http://test.test.svc.cluster.local"), + ) + + // Set up the test environment. + testEnv := setupTestEnvironment(t, subscription) + testEnv.Backend.On("SyncSubscription", mock.Anything).Return(nil) + testEnv.Backend.On("GetJetStreamSubjects", mock.Anything, mock.Anything, mock.Anything).Return([]string{eventingtesting.JetStreamSubject}) + testEnv.Backend.On("GetConfig", mock.Anything).Return(env.NATSConfig{JSStreamName: "sap"}) + + // Set up the SubscriptionValidator mock. + validatorMock := &subscriptionvalidatormocks.SubscriptionValidator{} + validatorMock.On("Validate", mock.Anything, mock.Anything).Return(nil) + + reconciler := NewReconciler( + testEnv.Client, + testEnv.Backend, + testEnv.Logger, + testEnv.Recorder, + testEnv.Cleaner, + validatorMock, + metrics.NewCollector(), + ) + + // when + request := kctrl.Request{NamespacedName: types.NamespacedName{Namespace: subscription.Namespace, Name: subscription.Name}} + res, err := reconciler.Reconcile(context.Background(), request) + + // then + require.Equal(t, kctrl.Result{}, res) + require.NoError(t, err) + validatorMock.AssertExpectations(t) + testEnv.Backend.AssertExpectations(t) +} + // helper functions and structs // TestEnvironment provides mocked resources for tests. @@ -741,16 +788,18 @@ func setupTestEnvironment(t *testing.T, objs ...client.Object) *TestEnvironment t.Fatalf("initialize logger failed: %v", err) } jsCleaner := cleaner.NewJetStreamCleaner(defaultLogger) - defaultSinkValidator := sink.NewValidator(fakeClient, recorder) + + // Init the Subscription validator. + subscriptionValidator := validator.NewSubscriptionValidator(fakeClient) reconciler := Reconciler{ - Backend: mockedBackend, - Client: fakeClient, - logger: defaultLogger, - recorder: recorder, - sinkValidator: defaultSinkValidator, - cleaner: jsCleaner, - collector: metrics.NewCollector(), + Backend: mockedBackend, + Client: fakeClient, + logger: defaultLogger, + recorder: recorder, + subscriptionValidator: subscriptionValidator, + cleaner: jsCleaner, + collector: metrics.NewCollector(), } return &TestEnvironment{ diff --git a/internal/controller/eventing/subscription/jetstream/test_utils_test.go b/internal/controller/eventing/subscription/jetstream/test_utils_test.go index 4805432f1..fa434d62e 100644 --- a/internal/controller/eventing/subscription/jetstream/test_utils_test.go +++ b/internal/controller/eventing/subscription/jetstream/test_utils_test.go @@ -2,10 +2,8 @@ package jetstream_test import ( "context" - "crypto/tls" "fmt" "log" - "net" "path/filepath" "testing" "time" @@ -21,24 +19,21 @@ import ( kerrors "k8s.io/apimachinery/pkg/api/errors" kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" kctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" - "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/metrics/server" - "sigs.k8s.io/controller-runtime/pkg/webhook" eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" subscriptioncontrollerjetstream "github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/jetstream" + "github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/validator" "github.com/kyma-project/eventing-manager/internal/controller/events" backendcleaner "github.com/kyma-project/eventing-manager/pkg/backend/cleaner" "github.com/kyma-project/eventing-manager/pkg/backend/jetstream" "github.com/kyma-project/eventing-manager/pkg/backend/metrics" - "github.com/kyma-project/eventing-manager/pkg/backend/sink" "github.com/kyma-project/eventing-manager/pkg/env" "github.com/kyma-project/eventing-manager/pkg/logger" eventingtesting "github.com/kyma-project/eventing-manager/testing" @@ -111,11 +106,6 @@ func setupSuite() error { }, AttachControlPlaneOutput: attachControlPlaneOutput, UseExistingCluster: &useExistingCluster, - WebhookInstallOptions: envtest.WebhookInstallOptions{ - Paths: []string{ - filepath.Join("../../../../../", "config", "webhook", "webhook_configs.yaml"), - }, - }, }, } @@ -143,17 +133,11 @@ func startReconciler() error { } syncPeriod := syncPeriod - webhookInstallOptions := &jsTestEnsemble.TestEnv.WebhookInstallOptions k8sManager, err := kctrl.NewManager(jsTestEnsemble.Cfg, kctrl.Options{ Scheme: scheme.Scheme, HealthProbeBindAddress: "0", // disable Cache: cache.Options{SyncPeriod: &syncPeriod}, Metrics: server.Options{BindAddress: "0"}, // disable - WebhookServer: webhook.NewServer(webhook.Options{ - Host: webhookInstallOptions.LocalServingHost, - Port: webhookInstallOptions.LocalServingPort, - CertDir: webhookInstallOptions.LocalServingCertDir, - }), }) if err != nil { return err @@ -188,13 +172,16 @@ func startReconciler() error { k8sClient := k8sManager.GetClient() recorder := k8sManager.GetEventRecorderFor("eventing-controller-jetstream") + // Init the Subscription validator. + subscriptionValidator := validator.NewSubscriptionValidator(k8sClient) + jsTestEnsemble.Reconciler = subscriptioncontrollerjetstream.NewReconciler( k8sClient, jetStreamHandler, defaultLogger, recorder, cleaner, - sink.NewValidator(k8sClient, recorder), + subscriptionValidator, metricsCollector, ) @@ -219,10 +206,6 @@ func startReconciler() error { return errors.New("K8sClient cannot be nil") } - if err := StartAndWaitForWebhookServer(k8sManager, webhookInstallOptions); err != nil { - return err - } - return nil } @@ -378,8 +361,8 @@ func IsSubscriptionDeletedOnK8s(g *gomega.WithT, ens *Ensemble, func ConditionInvalidSink(msg string) eventingv1alpha2.Condition { return eventingv1alpha2.MakeCondition( - eventingv1alpha2.ConditionSubscriptionActive, - eventingv1alpha2.ConditionReasonNATSSubscriptionNotActive, + eventingv1alpha2.ConditionSubscriptionSpecValid, + eventingv1alpha2.ConditionReasonSubscriptionSpecHasValidationErrors, kcorev1.ConditionFalse, msg) } @@ -545,35 +528,6 @@ func NewCleanEventType(ending string) string { return fmt.Sprintf("%s%s", eventingtesting.OrderCreatedEventType, ending) } -func GenerateInvalidSubscriptionError(subName, errType string, path *field.Path) error { - webhookErr := "admission webhook \"vsubscription.kb.io\" denied the request: " - givenError := kerrors.NewInvalid( - eventingv1alpha2.GroupKind, subName, - field.ErrorList{eventingv1alpha2.MakeInvalidFieldError(path, subName, errType)}) - givenError.ErrStatus.Message = webhookErr + givenError.ErrStatus.Message - return givenError -} - -func StartAndWaitForWebhookServer(k8sManager manager.Manager, webhookInstallOpts *envtest.WebhookInstallOptions) error { - if err := (&eventingv1alpha2.Subscription{}).SetupWebhookWithManager(k8sManager); err != nil { - return err - } - // wait for the webhook server to get ready - dialer := &net.Dialer{Timeout: time.Second} - addrPort := fmt.Sprintf("%s:%d", webhookInstallOpts.LocalServingHost, webhookInstallOpts.LocalServingPort) - err := retry.Do(func() error { - conn, connErr := tls.DialWithDialer(dialer, "tcp", addrPort, - &tls.Config{ - InsecureSkipVerify: true, //nolint:gosec // TLS check can be skipped for this integration test suit - }) - if connErr != nil { - return connErr - } - return conn.Close() - }, retry.Attempts(MaxReconnects)) - return err -} - func doRetry(function func() error, errString string) error { err := retry.Do(function, retry.Delay(time.Minute), diff --git a/internal/controller/eventing/subscription/validator/errors.go b/internal/controller/eventing/subscription/validator/errors.go new file mode 100644 index 000000000..c07cbc8b1 --- /dev/null +++ b/internal/controller/eventing/subscription/validator/errors.go @@ -0,0 +1,43 @@ +package validator + +import ( + "fmt" + "strconv" + + "k8s.io/apimachinery/pkg/util/validation/field" + + eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" + "github.com/kyma-project/eventing-manager/pkg/ems/api/events/types" +) + +//nolint:gochecknoglobals // these are required for testing +var ( + sourcePath = field.NewPath("spec").Child("source") + typesPath = field.NewPath("spec").Child("types") + configPath = field.NewPath("spec").Child("config") + sinkPath = field.NewPath("spec").Child("sink") + typeMatchingPath = field.NewPath("spec").Child("typeMatching") + namespacePath = field.NewPath("metadata").Child("namespace") + + emptyErrDetail = "must not be empty" + invalidURIErrDetail = "must be valid as per RFC 3986" + duplicateTypesErrDetail = "must not have duplicate types" + lengthErrDetail = "must not be of length zero" + minSegmentErrDetail = fmt.Sprintf("must have minimum %s segments", strconv.Itoa(minEventTypeSegments)) + invalidPrefixErrDetail = fmt.Sprintf("must not have %s as type prefix", validPrefix) + stringIntErrDetail = fmt.Sprintf("%s must be a stringified int value", eventingv1alpha2.MaxInFlightMessages) + + invalidQosErrDetail = fmt.Sprintf("must be a valid QoS value %s or %s", types.QosAtLeastOnce, types.QosAtMostOnce) + invalidAuthTypeErrDetail = fmt.Sprintf("must be a valid Auth Type value %s", types.AuthTypeClientCredentials) + invalidGrantTypeErrDetail = fmt.Sprintf("must be a valid Grant Type value %s", types.GrantTypeClientCredentials) + invalidTypeMatchingErrDetail = fmt.Sprintf("must be a valid TypeMatching value %s or %s", eventingv1alpha2.TypeMatchingExact, eventingv1alpha2.TypeMatchingStandard) + + missingSchemeErrDetail = "must have URL scheme 'http' or 'https'" + suffixMissingErrDetail = fmt.Sprintf("must have valid sink URL suffix %s", clusterLocalURLSuffix) + subDomainsErrDetail = fmt.Sprintf("must have sink URL with %d sub-domains: ", subdomainSegments) + namespaceMismatchErrDetail = "must have the same namespace as the subscriber: " +) + +func makeInvalidFieldError(path *field.Path, subName, detail string) *field.Error { + return field.Invalid(path, subName, detail) +} diff --git a/internal/controller/eventing/subscription/validator/mocks/SubscriptionValidator.go b/internal/controller/eventing/subscription/validator/mocks/SubscriptionValidator.go new file mode 100644 index 000000000..8805be501 --- /dev/null +++ b/internal/controller/eventing/subscription/validator/mocks/SubscriptionValidator.go @@ -0,0 +1,84 @@ +// Code generated by mockery v2.42.0. DO NOT EDIT. + +package mocks + +import ( + context "context" + + v1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" + mock "github.com/stretchr/testify/mock" +) + +// SubscriptionValidator is an autogenerated mock type for the SubscriptionValidator type +type SubscriptionValidator struct { + mock.Mock +} + +type SubscriptionValidator_Expecter struct { + mock *mock.Mock +} + +func (_m *SubscriptionValidator) EXPECT() *SubscriptionValidator_Expecter { + return &SubscriptionValidator_Expecter{mock: &_m.Mock} +} + +// Validate provides a mock function with given fields: ctx, subscription +func (_m *SubscriptionValidator) Validate(ctx context.Context, subscription v1alpha2.Subscription) error { + ret := _m.Called(ctx, subscription) + + if len(ret) == 0 { + panic("no return value specified for Validate") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, v1alpha2.Subscription) error); ok { + r0 = rf(ctx, subscription) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// SubscriptionValidator_Validate_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Validate' +type SubscriptionValidator_Validate_Call struct { + *mock.Call +} + +// Validate is a helper method to define mock.On call +// - ctx context.Context +// - subscription v1alpha2.Subscription +func (_e *SubscriptionValidator_Expecter) Validate(ctx interface{}, subscription interface{}) *SubscriptionValidator_Validate_Call { + return &SubscriptionValidator_Validate_Call{Call: _e.mock.On("Validate", ctx, subscription)} +} + +func (_c *SubscriptionValidator_Validate_Call) Run(run func(ctx context.Context, subscription v1alpha2.Subscription)) *SubscriptionValidator_Validate_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(v1alpha2.Subscription)) + }) + return _c +} + +func (_c *SubscriptionValidator_Validate_Call) Return(_a0 error) *SubscriptionValidator_Validate_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *SubscriptionValidator_Validate_Call) RunAndReturn(run func(context.Context, v1alpha2.Subscription) error) *SubscriptionValidator_Validate_Call { + _c.Call.Return(run) + return _c +} + +// NewSubscriptionValidator creates a new instance of SubscriptionValidator. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewSubscriptionValidator(t interface { + mock.TestingT + Cleanup(func()) +}) *SubscriptionValidator { + mock := &SubscriptionValidator{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/internal/controller/eventing/subscription/validator/sink.go b/internal/controller/eventing/subscription/validator/sink.go new file mode 100644 index 000000000..7f528df3d --- /dev/null +++ b/internal/controller/eventing/subscription/validator/sink.go @@ -0,0 +1,63 @@ +package validator + +import ( + "context" + "fmt" + + pkgerrors "github.com/pkg/errors" + kcorev1 "k8s.io/api/core/v1" + ktypes "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kyma-project/eventing-manager/pkg/utils" +) + +var ErrSinkValidationFailed = pkgerrors.New("Sink validation failed") + +type sinkValidator interface { + validate(ctx context.Context, sink string) error +} + +type sinkServiceValidator struct { + client client.Client +} + +// Perform a compile-time check. +var _ sinkValidator = &sinkServiceValidator{} + +func newSinkValidator(client client.Client) sinkValidator { + return &sinkServiceValidator{client: client} +} + +func (sv sinkServiceValidator) validate(ctx context.Context, sink string) error { + _, subDomains, err := utils.GetSinkData(sink) + if err != nil { + return fmt.Errorf("%w: %w", ErrSinkValidationFailed, err) + } + + const minSubDomains = 2 + if len(subDomains) < minSubDomains { + return fmt.Errorf("%w: sink format should contain the service name.namespace", ErrSinkValidationFailed) + } + + namespace, name := subDomains[1], subDomains[0] + if !sv.serviceExists(ctx, namespace, name) { + return fmt.Errorf("%w: service %s.%s not found in the cluster", ErrSinkValidationFailed, name, namespace) + } + + return nil +} + +func (sv sinkServiceValidator) serviceExists(ctx context.Context, namespace, name string) bool { + return sv.client.Get(ctx, ktypes.NamespacedName{Namespace: namespace, Name: name}, &kcorev1.Service{}) == nil +} + +// sinkValidatorFunc implements the sinkValidator interface. +type sinkValidatorFunc func(ctx context.Context, sink string) error + +// Perform a compile-time check. +var _ sinkValidator = sinkValidatorFunc(func(_ context.Context, _ string) error { return nil }) + +func (svf sinkValidatorFunc) validate(ctx context.Context, sink string) error { + return svf(ctx, sink) +} diff --git a/internal/controller/eventing/subscription/validator/sink_test.go b/internal/controller/eventing/subscription/validator/sink_test.go new file mode 100644 index 000000000..8694650a4 --- /dev/null +++ b/internal/controller/eventing/subscription/validator/sink_test.go @@ -0,0 +1,83 @@ +package validator + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/require" + kcorev1 "k8s.io/api/core/v1" + kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestSinkValidator(t *testing.T) { + // given + const ( + namespaceName = "test-namespace" + serviceName = "test-service" + ) + + ctx := context.Background() + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() + validator := newSinkValidator(fakeClient) + + tests := []struct { + name string + givenSink string + givenService *kcorev1.Service + wantErr error + }{ + { + name: "With empty sink", + givenSink: "", + givenService: nil, + wantErr: ErrSinkValidationFailed, + }, + { + name: "With invalid sink URL", + givenSink: "[:invalid:url:]", + givenService: nil, + wantErr: ErrSinkValidationFailed, + }, + { + name: "With invalid sink format", + givenSink: "http://insuffecient", + givenService: nil, + wantErr: ErrSinkValidationFailed, + }, + { + name: "With non-existing service", + givenSink: fmt.Sprintf("https://%s.%s", serviceName, namespaceName), + givenService: nil, + wantErr: ErrSinkValidationFailed, + }, + { + name: "With existing service", + givenSink: fmt.Sprintf("https://%s.%s", serviceName, namespaceName), + givenService: &kcorev1.Service{ + ObjectMeta: kmetav1.ObjectMeta{Name: serviceName, Namespace: namespaceName}, + }, + wantErr: nil, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // create service if needed + if test.givenService != nil { + require.NoError(t, fakeClient.Create(ctx, test.givenService)) + } + + // when + gotErr := validator.validate(ctx, test.givenSink) + + // then + if test.wantErr == nil { + require.NoError(t, gotErr) + } else { + require.ErrorIs(t, gotErr, ErrSinkValidationFailed) + } + }) + } +} diff --git a/internal/controller/eventing/subscription/validator/spec.go b/internal/controller/eventing/subscription/validator/spec.go new file mode 100644 index 000000000..644120b3d --- /dev/null +++ b/internal/controller/eventing/subscription/validator/spec.go @@ -0,0 +1,174 @@ +package validator + +import ( + "strconv" + "strings" + + "k8s.io/apimachinery/pkg/util/validation/field" + + eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" + "github.com/kyma-project/eventing-manager/pkg/ems/api/events/types" + "github.com/kyma-project/eventing-manager/pkg/utils" +) + +const ( + minEventTypeSegments = 2 + subdomainSegments = 5 + validPrefix = "sap.kyma.custom" + clusterLocalURLSuffix = "svc.cluster.local" +) + +func validateSpec(subscription eventingv1alpha2.Subscription) field.ErrorList { + var allErrs field.ErrorList + if err := validateTypeMatching(subscription.Spec.TypeMatching); err != nil { + allErrs = append(allErrs, err) + } + if err := validateSource(subscription.Spec.Source, subscription.Spec.TypeMatching); err != nil { + allErrs = append(allErrs, err) + } + if err := validateTypes(subscription.Spec.Types, subscription.Spec.TypeMatching); err != nil { + allErrs = append(allErrs, err) + } + if err := validateConfig(subscription.Spec.Config); err != nil { + allErrs = append(allErrs, err...) + } + if err := validateSink(subscription.Spec.Sink, subscription.Namespace); err != nil { + allErrs = append(allErrs, err) + } + if len(allErrs) == 0 { + return nil + } + return allErrs +} + +func validateTypeMatching(typeMatching eventingv1alpha2.TypeMatching) *field.Error { + if typeMatching != eventingv1alpha2.TypeMatchingExact && typeMatching != eventingv1alpha2.TypeMatchingStandard { + return makeInvalidFieldError(typeMatchingPath, string(typeMatching), invalidTypeMatchingErrDetail) + } + return nil +} + +func validateSource(source string, typeMatching eventingv1alpha2.TypeMatching) *field.Error { + if source == "" && typeMatching != eventingv1alpha2.TypeMatchingExact { + return makeInvalidFieldError(sourcePath, source, emptyErrDetail) + } + // Check only if the source is valid for the cloud event, with a valid event type. + if isInvalidCE(source, "") { + return makeInvalidFieldError(sourcePath, source, invalidURIErrDetail) + } + return nil +} + +func validateTypes(types []string, typeMatching eventingv1alpha2.TypeMatching) *field.Error { + if len(types) == 0 { + return makeInvalidFieldError(typesPath, "", emptyErrDetail) + } + if duplicates := getDuplicates(types); len(duplicates) > 0 { + return makeInvalidFieldError(typesPath, strings.Join(duplicates, ","), duplicateTypesErrDetail) + } + for _, eventType := range types { + if len(eventType) == 0 { + return makeInvalidFieldError(typesPath, eventType, lengthErrDetail) + } + if segments := strings.Split(eventType, "."); len(segments) < minEventTypeSegments { + return makeInvalidFieldError(typesPath, eventType, minSegmentErrDetail) + } + if typeMatching != eventingv1alpha2.TypeMatchingExact && strings.HasPrefix(eventType, validPrefix) { + return makeInvalidFieldError(typesPath, eventType, invalidPrefixErrDetail) + } + // Check only is the event type is valid for the cloud event, with a valid source. + const validSource = "source" + if isInvalidCE(validSource, eventType) { + return makeInvalidFieldError(typesPath, eventType, invalidURIErrDetail) + } + } + return nil +} + +func validateConfig(config map[string]string) field.ErrorList { + var allErrs field.ErrorList + if ifKeyExistsInConfig(config, eventingv1alpha2.MaxInFlightMessages) && isNotInt(config[eventingv1alpha2.MaxInFlightMessages]) { + allErrs = append(allErrs, makeInvalidFieldError(configPath, config[eventingv1alpha2.MaxInFlightMessages], stringIntErrDetail)) + } + if ifKeyExistsInConfig(config, eventingv1alpha2.ProtocolSettingsQos) && types.IsInvalidQoS(config[eventingv1alpha2.ProtocolSettingsQos]) { + allErrs = append(allErrs, makeInvalidFieldError(configPath, config[eventingv1alpha2.ProtocolSettingsQos], invalidQosErrDetail)) + } + if ifKeyExistsInConfig(config, eventingv1alpha2.WebhookAuthType) && types.IsInvalidAuthType(config[eventingv1alpha2.WebhookAuthType]) { + allErrs = append(allErrs, makeInvalidFieldError(configPath, config[eventingv1alpha2.WebhookAuthType], invalidAuthTypeErrDetail)) + } + if ifKeyExistsInConfig(config, eventingv1alpha2.WebhookAuthGrantType) && types.IsInvalidGrantType(config[eventingv1alpha2.WebhookAuthGrantType]) { + allErrs = append(allErrs, makeInvalidFieldError(configPath, config[eventingv1alpha2.WebhookAuthGrantType], invalidGrantTypeErrDetail)) + } + return allErrs +} + +func validateSink(sink, namespace string) *field.Error { + if sink == "" { + return makeInvalidFieldError(sinkPath, sink, emptyErrDetail) + } + + if !utils.IsValidScheme(sink) { + return makeInvalidFieldError(sinkPath, sink, missingSchemeErrDetail) + } + + trimmedHost, subDomains, err := utils.GetSinkData(sink) + if err != nil { + return makeInvalidFieldError(sinkPath, sink, err.Error()) + } + + // Validate sink URL is a cluster local URL. + if !strings.HasSuffix(trimmedHost, clusterLocalURLSuffix) { + return makeInvalidFieldError(sinkPath, sink, suffixMissingErrDetail) + } + + // We expected a sink in the format "service.namespace.svc.cluster.local". + if len(subDomains) != subdomainSegments { + return makeInvalidFieldError(sinkPath, sink, subDomainsErrDetail+trimmedHost) + } + + // Assumption: Subscription CR and Subscriber should be deployed in the same namespace. + svcNs := subDomains[1] + if namespace != svcNs { + return makeInvalidFieldError(namespacePath, sink, namespaceMismatchErrDetail+svcNs) + } + + return nil +} + +func ifKeyExistsInConfig(config map[string]string, key string) bool { + _, ok := config[key] + return ok +} + +func isNotInt(value string) bool { + if _, err := strconv.Atoi(value); err != nil { + return true + } + return false +} + +func isInvalidCE(source, eventType string) bool { + if source == "" { + return false + } + newEvent := utils.GetCloudEvent(eventType) + newEvent.SetSource(source) + err := newEvent.Validate() + return err != nil +} + +// getDuplicates returns the duplicate items from the given list. +func getDuplicates(items []string) []string { + if len(items) == 0 { + return items + } + const duplicatesCount = 2 + itemsMap := make(map[string]int, len(items)) + duplicates := make([]string, 0, len(items)) + for _, t := range items { + if itemsMap[t]++; itemsMap[t] == duplicatesCount { + duplicates = append(duplicates, t) + } + } + return duplicates +} diff --git a/api/eventing/v1alpha2/subscription_webhook_unit_test.go b/internal/controller/eventing/subscription/validator/spec_test.go similarity index 56% rename from api/eventing/v1alpha2/subscription_webhook_unit_test.go rename to internal/controller/eventing/subscription/validator/spec_test.go index 9708884d2..7d291d1ef 100644 --- a/api/eventing/v1alpha2/subscription_webhook_unit_test.go +++ b/internal/controller/eventing/subscription/validator/spec_test.go @@ -1,116 +1,83 @@ -package v1alpha2_test +package validator import ( "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/util/validation/field" - "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" + eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" eventingtesting "github.com/kyma-project/eventing-manager/testing" ) -const ( - subName = "sub" - subNamespace = "test" - sink = "https://eventing-nats.test.svc.cluster.local:8080" -) - -func Test_Default(t *testing.T) { +func Test_validateSpec(t *testing.T) { t.Parallel() + + const ( + subName = "sub" + subNamespace = "test" + maxInFlightMessages = "10" + sink = "https://eventing-nats.test.svc.cluster.local:8080" + ) + type TestCase struct { name string - givenSub *v1alpha2.Subscription - wantSub *v1alpha2.Subscription + givenSub *eventingv1alpha2.Subscription + wantErr field.ErrorList } testCases := []TestCase{ { - name: "Add TypeMatching Standard and default MaxInFlightMessages value", - givenSub: eventingtesting.NewSubscription(subName, subNamespace, - eventingtesting.WithSource(eventingtesting.EventSourceClean), - eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - ), - wantSub: eventingtesting.NewSubscription(subName, subNamespace, - eventingtesting.WithSource(eventingtesting.EventSourceClean), - eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithTypeMatchingStandard(), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), - ), - }, - { - name: "Add TypeMatching Standard only", + name: "A valid subscription should not have errors", givenSub: eventingtesting.NewSubscription(subName, subNamespace, eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithMaxInFlightMessages("20"), - ), - wantSub: eventingtesting.NewSubscription(subName, subNamespace, - eventingtesting.WithSource(eventingtesting.EventSourceClean), - eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithMaxInFlightMessages("20"), - eventingtesting.WithTypeMatchingStandard(), + eventingtesting.WithWebhookAuthForEventMesh(), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), + eventingtesting.WithSink(sink), + eventingtesting.WithTypeMatching(eventingv1alpha2.TypeMatchingStandard), ), + wantErr: nil, }, { - name: "Add default MaxInFlightMessages value only", + name: "empty TypeMatching should return error", givenSub: eventingtesting.NewSubscription(subName, subNamespace, eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithTypeMatchingExact(), - ), - wantSub: eventingtesting.NewSubscription(subName, subNamespace, - eventingtesting.WithSource(eventingtesting.EventSourceClean), - eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithTypeMatchingExact(), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithWebhookAuthForEventMesh(), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), + eventingtesting.WithSink(sink), + eventingtesting.WithTypeMatching(""), ), + wantErr: field.ErrorList{ + makeInvalidFieldError(typeMatchingPath, "", invalidTypeMatchingErrDetail), + }, }, - } - - for _, tc := range testCases { - testcase := tc - t.Run(testcase.name, func(t *testing.T) { - t.Parallel() - testcase.givenSub.Default() - require.Equal(t, testcase.wantSub, testcase.givenSub) - }) - } -} - -func Test_validateSubscription(t *testing.T) { - t.Parallel() - type TestCase struct { - name string - givenSub *v1alpha2.Subscription - wantErr error - } - - testCases := []TestCase{ { - name: "A valid subscription should not have errors", + name: "unsupported TypeMatching should return error", givenSub: eventingtesting.NewSubscription(subName, subNamespace, eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), eventingtesting.WithWebhookAuthForEventMesh(), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink(sink), + eventingtesting.WithTypeMatching("unsupported"), ), - wantErr: nil, + wantErr: field.ErrorList{ + makeInvalidFieldError(typeMatchingPath, "unsupported", invalidTypeMatchingErrDetail), + }, }, { name: "empty source and TypeMatching Standard should return error", givenSub: eventingtesting.NewSubscription(subName, subNamespace, eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SourcePath, - subName, v1alpha2.EmptyErrDetail)}), + wantErr: field.ErrorList{makeInvalidFieldError(sourcePath, + "", emptyErrDetail)}, }, { name: "valid source and TypeMatching Standard should not return error", @@ -118,7 +85,7 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink(sink), ), wantErr: nil, @@ -128,7 +95,7 @@ func Test_validateSubscription(t *testing.T) { givenSub: eventingtesting.NewSubscription(subName, subNamespace, eventingtesting.WithTypeMatchingExact(), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink(sink), ), wantErr: nil, @@ -139,26 +106,22 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource("s%ourc%e"), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SourcePath, - subName, v1alpha2.InvalidURIErrDetail)}), + wantErr: field.ErrorList{makeInvalidFieldError(sourcePath, + "s%ourc%e", invalidURIErrDetail)}, }, { name: "nil types field should return error", givenSub: eventingtesting.NewSubscription(subName, subNamespace, eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource(eventingtesting.EventSourceClean), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, - subName, v1alpha2.EmptyErrDetail)}), + wantErr: field.ErrorList{makeInvalidFieldError(typesPath, + "", emptyErrDetail)}, }, { name: "empty types field should return error", @@ -166,13 +129,11 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithTypes([]string{}), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, - subName, v1alpha2.EmptyErrDetail)}), + wantErr: field.ErrorList{makeInvalidFieldError(typesPath, + "", emptyErrDetail)}, }, { name: "duplicate types should return error", @@ -183,13 +144,11 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.OrderCreatedV1Event, eventingtesting.OrderCreatedV1Event, }), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, - subName, v1alpha2.DuplicateTypesErrDetail)}), + wantErr: field.ErrorList{makeInvalidFieldError(typesPath, + "order.created.v1", duplicateTypesErrDetail)}, }, { name: "empty event type should return error", @@ -197,13 +156,11 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithTypes([]string{eventingtesting.OrderCreatedV1Event, ""}), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, - subName, v1alpha2.LengthErrDetail)}), + wantErr: field.ErrorList{makeInvalidFieldError(typesPath, + "", lengthErrDetail)}, }, { name: "lower than min segments should return error", @@ -211,35 +168,32 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithTypes([]string{"order"}), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, - subName, v1alpha2.MinSegmentErrDetail)}), + wantErr: field.ErrorList{makeInvalidFieldError(typesPath, + "order", minSegmentErrDetail)}, }, { name: "invalid prefix should return error", givenSub: eventingtesting.NewSubscription(subName, subNamespace, eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource(eventingtesting.EventSourceClean), - eventingtesting.WithTypes([]string{v1alpha2.InvalidPrefix}), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithTypes([]string{validPrefix}), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, - subName, v1alpha2.InvalidPrefixErrDetail)}), + wantErr: field.ErrorList{ + makeInvalidFieldError(typesPath, "sap.kyma.custom", invalidPrefixErrDetail), + }, }, { name: "invalid prefix with exact should not return error", givenSub: eventingtesting.NewSubscription(subName, subNamespace, eventingtesting.WithTypeMatchingExact(), eventingtesting.WithSource(eventingtesting.EventSourceClean), - eventingtesting.WithTypes([]string{v1alpha2.InvalidPrefix}), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithTypes([]string{validPrefix}), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink(sink), ), wantErr: nil, @@ -253,10 +207,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithMaxInFlightMessages("invalid"), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.ConfigPath, - subName, v1alpha2.StringIntErrDetail)}), + wantErr: field.ErrorList{makeInvalidFieldError(configPath, + "invalid", stringIntErrDetail)}, }, { name: "invalid QoS value should return error", @@ -264,14 +216,12 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithInvalidProtocolSettingsQos(), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.ConfigPath, - subName, v1alpha2.InvalidQosErrDetail)}), + wantErr: field.ErrorList{makeInvalidFieldError(configPath, + "AT_INVALID_ONCE", invalidQosErrDetail)}, }, { name: "invalid webhook auth type value should return error", @@ -279,14 +229,12 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithInvalidWebhookAuthType(), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.ConfigPath, - subName, v1alpha2.InvalidAuthTypeErrDetail)}), + wantErr: field.ErrorList{makeInvalidFieldError(configPath, + "abcd", invalidAuthTypeErrDetail)}, }, { name: "invalid webhook grant type value should return error", @@ -294,14 +242,12 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithInvalidWebhookAuthGrantType(), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.ConfigPath, - subName, v1alpha2.InvalidGrantTypeErrDetail)}), + wantErr: field.ErrorList{makeInvalidFieldError(configPath, + "invalid", invalidGrantTypeErrDetail)}, }, { name: "missing sink should return error", @@ -309,12 +255,10 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SinkPath, - subName, v1alpha2.EmptyErrDetail)}), + wantErr: field.ErrorList{makeInvalidFieldError(sinkPath, + "", emptyErrDetail)}, }, { name: "sink with invalid scheme should return error", @@ -322,13 +266,11 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink(subNamespace), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SinkPath, - subName, v1alpha2.MissingSchemeErrDetail)}), + wantErr: field.ErrorList{makeInvalidFieldError(sinkPath, + "test", missingSchemeErrDetail)}, }, { name: "sink with invalid URL should return error", @@ -336,14 +278,12 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink("http://invalid Sink"), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SinkPath, - subName, "failed to parse subscription sink URL: "+ - "parse \"http://invalid Sink\": invalid character \" \" in host name")}), + wantErr: field.ErrorList{makeInvalidFieldError(sinkPath, + "http://invalid Sink", "failed to parse subscription sink URL: "+ + "parse \"http://invalid Sink\": invalid character \" \" in host name")}, }, { name: "sink with invalid suffix should return error", @@ -351,13 +291,11 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink("https://svc2.test.local"), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SinkPath, - subName, v1alpha2.SuffixMissingErrDetail)}), + wantErr: field.ErrorList{makeInvalidFieldError(sinkPath, + "https://svc2.test.local", suffixMissingErrDetail)}, }, { name: "sink with invalid suffix and port should return error", @@ -365,13 +303,11 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink("https://svc2.test.local:8080"), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SinkPath, - subName, v1alpha2.SuffixMissingErrDetail)}), + wantErr: field.ErrorList{makeInvalidFieldError(sinkPath, + "https://svc2.test.local:8080", suffixMissingErrDetail)}, }, { name: "sink with invalid number of subdomains should return error", @@ -379,13 +315,11 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink("https://svc.cluster.local:8080"), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SinkPath, - subName, v1alpha2.SubDomainsErrDetail+"svc.cluster.local")}), + wantErr: field.ErrorList{makeInvalidFieldError(sinkPath, + "https://svc.cluster.local:8080", subDomainsErrDetail+"svc.cluster.local")}, }, { name: "sink with different namespace should return error", @@ -393,13 +327,11 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithTypeMatchingStandard(), eventingtesting.WithSource(eventingtesting.EventSourceClean), eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), - eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), eventingtesting.WithSink("https://eventing-nats.kyma-system.svc.cluster.local"), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.NSPath, - subName, v1alpha2.NSMismatchErrDetail+"kyma-system")}), + wantErr: field.ErrorList{makeInvalidFieldError(namespacePath, + "https://eventing-nats.kyma-system.svc.cluster.local", namespaceMismatchErrDetail+"kyma-system")}, }, { name: "multiple errors should be reported if exists", @@ -409,14 +341,12 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithMaxInFlightMessages("invalid"), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{ - v1alpha2.MakeInvalidFieldError(v1alpha2.SourcePath, - subName, v1alpha2.EmptyErrDetail), - v1alpha2.MakeInvalidFieldError(v1alpha2.ConfigPath, - subName, v1alpha2.StringIntErrDetail), - }), + wantErr: field.ErrorList{ + makeInvalidFieldError(sourcePath, + "", emptyErrDetail), + makeInvalidFieldError(configPath, + "invalid", stringIntErrDetail), + }, }, } @@ -424,13 +354,13 @@ func Test_validateSubscription(t *testing.T) { tc := testCase t.Run(tc.name, func(t *testing.T) { t.Parallel() - _, err := tc.givenSub.ValidateSubscription() + err := validateSpec(*tc.givenSub) require.Equal(t, tc.wantErr, err) }) } } -func Test_IsInvalidCESource(t *testing.T) { +func Test_isInvalidCE(t *testing.T) { t.Parallel() type TestCase struct { name string @@ -470,8 +400,86 @@ func Test_IsInvalidCESource(t *testing.T) { tc := testCase t.Run(tc.name, func(t *testing.T) { t.Parallel() - gotIsInvalid := v1alpha2.IsInvalidCE(tc.givenSource, tc.givenType) + gotIsInvalid := isInvalidCE(tc.givenSource, tc.givenType) require.Equal(t, tc.wantIsInvalid, gotIsInvalid) }) } } + +func Test_getDuplicates(t *testing.T) { + tests := []struct { + name string + givenTypes []string + wantDuplicateTypes []string + }{ + { + name: "with nil types", + givenTypes: nil, + wantDuplicateTypes: nil, + }, + { + name: "with empty types", + givenTypes: []string{}, + wantDuplicateTypes: []string{}, + }, + { + name: "with one type", + givenTypes: []string{ + "type0", + }, + wantDuplicateTypes: []string{}, + }, + { + name: "with multiple types and no duplicates", + givenTypes: []string{ + "type0", + "type1", + "type2", + }, + wantDuplicateTypes: []string{}, + }, + { + name: "with one type which is a duplicate", + givenTypes: []string{ + "type0", "type0", + }, + wantDuplicateTypes: []string{ + "type0", + }, + }, + { + name: "with multiple types and consequent duplicates", + givenTypes: []string{ + "type0", + "type1", "type1", "type1", // duplicates + "type2", "type2", // duplicates + "type3", + "type4", "type4", "type4", "type4", // duplicates + "type5", + }, + wantDuplicateTypes: []string{ + "type1", "type2", "type4", + }, + }, + { + name: "with multiple types and non-consequent duplicates", + givenTypes: []string{ + "type5", "type0", "type1", "type2", + "type1", // duplicate + "type3", "type4", + "type5", // duplicate + "type6", + "type4", "type2", // duplicates + }, + wantDuplicateTypes: []string{ + "type1", "type5", "type4", "type2", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotDuplicateTypes := getDuplicates(tt.givenTypes) + assert.Equal(t, tt.wantDuplicateTypes, gotDuplicateTypes) + }) + } +} diff --git a/internal/controller/eventing/subscription/validator/subscription.go b/internal/controller/eventing/subscription/validator/subscription.go new file mode 100644 index 000000000..719672d84 --- /dev/null +++ b/internal/controller/eventing/subscription/validator/subscription.go @@ -0,0 +1,49 @@ +package validator + +import ( + "context" + "fmt" + + pkgerrors "github.com/pkg/errors" + "sigs.k8s.io/controller-runtime/pkg/client" + + eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" +) + +var ErrSubscriptionValidationFailed = pkgerrors.New("Subscription validation failed") + +//go:generate go run github.com/vektra/mockery/v2 --name=SubscriptionValidator --outpkg=mocks +type SubscriptionValidator interface { + Validate(ctx context.Context, subscription eventingv1alpha2.Subscription) error +} + +type subscriptionValidator struct { + sinkValidator sinkValidator +} + +// Perform a compile-time check. +var _ SubscriptionValidator = &subscriptionValidator{} + +func NewSubscriptionValidator(client client.Client) SubscriptionValidator { + return &subscriptionValidator{sinkValidator: newSinkValidator(client)} +} + +func (sv *subscriptionValidator) Validate(ctx context.Context, subscription eventingv1alpha2.Subscription) error { + if errs := validateSpec(subscription); len(errs) > 0 { + return fmt.Errorf("%w: %w", ErrSubscriptionValidationFailed, errs.ToAggregate()) + } + if err := sv.sinkValidator.validate(ctx, subscription.Spec.Sink); err != nil { + return fmt.Errorf("%w: %w", ErrSubscriptionValidationFailed, err) + } + return nil +} + +// SubscriptionValidatorFunc implements the SinkValidator interface. +type SubscriptionValidatorFunc func(ctx context.Context, subscription eventingv1alpha2.Subscription) error + +// Perform a compile-time check. +var _ SubscriptionValidator = SubscriptionValidatorFunc(func(_ context.Context, _ eventingv1alpha2.Subscription) error { return nil }) + +func (svf SubscriptionValidatorFunc) Validate(ctx context.Context, subscription eventingv1alpha2.Subscription) error { + return svf(ctx, subscription) +} diff --git a/internal/controller/eventing/subscription/validator/subscription_test.go b/internal/controller/eventing/subscription/validator/subscription_test.go new file mode 100644 index 000000000..6c0cf5f61 --- /dev/null +++ b/internal/controller/eventing/subscription/validator/subscription_test.go @@ -0,0 +1,83 @@ +package validator + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" + eventingtesting "github.com/kyma-project/eventing-manager/testing" +) + +func TestValidate(t *testing.T) { + ctx := context.TODO() + + const ( + subName = "sub" + subNamespace = "test" + maxInFlightMessages = "10" + sink = "https://eventing-nats.test.svc.cluster.local:8080" + ) + + happySinkValidator := sinkValidatorFunc(func(ctx context.Context, sinkURI string) error { return nil }) + unhappySinkValidator := sinkValidatorFunc(func(ctx context.Context, sinkURI string) error { return ErrSinkValidationFailed }) + + tests := []struct { + name string + givenSubscription *eventingv1alpha2.Subscription + givenSinkValidator sinkValidator + wantErr error + }{ + { + name: "simulate no validation errors", + givenSubscription: eventingtesting.NewSubscription( + subName, + subNamespace, + eventingtesting.WithTypeMatchingExact(), + eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), + eventingtesting.WithSink(sink), + ), + givenSinkValidator: happySinkValidator, + wantErr: nil, + }, + { + name: "simulate spec validation error", + givenSubscription: eventingtesting.NewSubscription( + subName, + subNamespace, + eventingtesting.WithTypeMatchingExact(), + eventingtesting.WithTypes([]string{}), // Trigger validation error. + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), + eventingtesting.WithSink(sink), + ), + givenSinkValidator: happySinkValidator, + wantErr: ErrSubscriptionValidationFailed, + }, + { + name: "simulate sink validation error", + givenSubscription: eventingtesting.NewSubscription( + subName, + subNamespace, + eventingtesting.WithTypeMatchingExact(), + eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), + eventingtesting.WithMaxInFlightMessages(maxInFlightMessages), + eventingtesting.WithSink(sink), + ), + givenSinkValidator: unhappySinkValidator, // Trigger sink validation error. + wantErr: ErrSinkValidationFailed, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + validator := &subscriptionValidator{sinkValidator: test.givenSinkValidator} + gotErr := validator.Validate(ctx, *test.givenSubscription) + if test.wantErr == nil { + require.NoError(t, gotErr) + } else { + require.ErrorIs(t, gotErr, test.wantErr) + } + }) + } +} diff --git a/internal/controller/operator/eventing/controller.go b/internal/controller/operator/eventing/controller.go index 5843776ff..54846398a 100644 --- a/internal/controller/operator/eventing/controller.go +++ b/internal/controller/operator/eventing/controller.go @@ -472,13 +472,6 @@ func (r *Reconciler) handleEventingReconcile(ctx context.Context, eventing, log) } - // sync webhooks CABundle. - if err := r.reconcileWebhooksWithCABundle(ctx); err != nil { - return kctrl.Result{}, r.syncStatusWithWebhookErr(ctx, eventing, err, log) - } - // set webhook condition to true. - eventing.Status.SetWebhookReadyConditionToTrue() - // handle backend switching. if err := r.handleBackendSwitching(ctx, eventing, log); err != nil { return kctrl.Result{}, r.syncStatusWithSubscriptionManagerErr(ctx, eventing, err, log) diff --git a/internal/controller/operator/eventing/integrationtests/controller/integration_test.go b/internal/controller/operator/eventing/integrationtests/controller/integration_test.go index d0026dda6..9042aae6b 100644 --- a/internal/controller/operator/eventing/integrationtests/controller/integration_test.go +++ b/internal/controller/operator/eventing/integrationtests/controller/integration_test.go @@ -214,8 +214,6 @@ func Test_CreateEventingCR_NATS(t *testing.T) { if testcase.wantEnsureK8sObjects && testcase.givenEventing.Spec.Backend != nil { // check if EPP resources exists. ensureK8sResources(t, testcase.givenEventing, testEnvironment) - // check if webhook configurations are updated with correct CABundle. - testEnvironment.EnsureCABundleInjectedIntoWebhooks(t) } // check the publisher service in the Eventing CR status @@ -711,8 +709,6 @@ func Test_CreateEventingCR_EventMesh(t *testing.T) { if testcase.wantEnsureK8sObjects { // check if other EPP resources exists and values are reflected. ensureK8sResources(t, testcase.givenEventing, testEnvironment) - // check if webhook configurations are updated with correct CABundle. - testEnvironment.EnsureCABundleInjectedIntoWebhooks(t) } // check the publisher service in the Eventing CR status diff --git a/internal/controller/operator/eventing/status.go b/internal/controller/operator/eventing/status.go index 953547e8e..c67546a6d 100644 --- a/internal/controller/operator/eventing/status.go +++ b/internal/controller/operator/eventing/status.go @@ -122,17 +122,6 @@ func (r *Reconciler) syncStatusWithSubscriptionManagerFailedCondition(ctx contex return r.syncEventingStatus(ctx, eventing, log) } -func (r *Reconciler) syncStatusWithWebhookErr(ctx context.Context, - eventing *operatorv1alpha1.Eventing, err error, log *zap.SugaredLogger, -) error { - // Set error state in status - eventing.Status.SetStateError() - eventing.Status.UpdateConditionWebhookReady(kmetav1.ConditionFalse, operatorv1alpha1.ConditionReasonWebhookFailed, - err.Error()) - - return errors.Join(err, r.syncEventingStatus(ctx, eventing, log)) -} - func (r *Reconciler) syncStatusWithDeletionErr(ctx context.Context, eventing *operatorv1alpha1.Eventing, err error, log *zap.SugaredLogger, ) error { diff --git a/internal/controller/operator/eventing/webhook.go b/internal/controller/operator/eventing/webhook.go deleted file mode 100644 index 6c0edeb14..000000000 --- a/internal/controller/operator/eventing/webhook.go +++ /dev/null @@ -1,80 +0,0 @@ -package eventing - -import ( - "bytes" - "context" - - "github.com/pkg/errors" - "sigs.k8s.io/controller-runtime/pkg/client" - - emerrors "github.com/kyma-project/eventing-manager/pkg/errors" -) - -const ( - TLSCertField = "tls.crt" -) - -var ( - errObjectNotFound = errors.New("object not found") - errInvalidObject = errors.New("invalid object") -) - -// reconcileWebhooksWithCABundle injects the CABundle into mutating and validating webhooks. -func (r *Reconciler) reconcileWebhooksWithCABundle(ctx context.Context) error { - // get the secret containing the certificate - secretKey := client.ObjectKey{ - Namespace: r.backendConfig.Namespace, - Name: r.backendConfig.WebhookSecretName, - } - certificateSecret, err := r.kubeClient.GetSecret(ctx, secretKey.String()) - if err != nil { - return emerrors.MakeError(errObjectNotFound, err) - } - - // get the mutating WH config. - mutatingWH, err := r.kubeClient.GetMutatingWebHookConfiguration(ctx, r.backendConfig.MutatingWebhookName) - if err != nil { - return emerrors.MakeError(errObjectNotFound, err) - } - - // get the validation WH config. - validatingWH, err := r.kubeClient.GetValidatingWebHookConfiguration(ctx, r.backendConfig.ValidatingWebhookName) - if err != nil { - return emerrors.MakeError(errObjectNotFound, err) - } - - // check that the mutating and validating WH config are valid - if len(mutatingWH.Webhooks) == 0 { - return emerrors.MakeError(errInvalidObject, - errors.Errorf("mutatingWH %s does not have associated webhooks", - r.backendConfig.MutatingWebhookName)) - } - if len(validatingWH.Webhooks) == 0 { - return emerrors.MakeError(errInvalidObject, - errors.Errorf("validatingWH %s does not have associated webhooks", - r.backendConfig.ValidatingWebhookName)) - } - - // check if the CABundle present is valid - if !(mutatingWH.Webhooks[0].ClientConfig.CABundle != nil && - bytes.Equal(mutatingWH.Webhooks[0].ClientConfig.CABundle, certificateSecret.Data[TLSCertField])) { - // update the ClientConfig for mutating WH config - mutatingWH.Webhooks[0].ClientConfig.CABundle = certificateSecret.Data[TLSCertField] - // update the mutating WH on k8s. - if err = r.Client.Update(ctx, mutatingWH); err != nil { - return errors.Wrap(err, "while updating mutatingWH with caBundle") - } - } - - if !(validatingWH.Webhooks[0].ClientConfig.CABundle != nil && - bytes.Equal(validatingWH.Webhooks[0].ClientConfig.CABundle, certificateSecret.Data[TLSCertField])) { - // update the ClientConfig for validating WH config - validatingWH.Webhooks[0].ClientConfig.CABundle = certificateSecret.Data[TLSCertField] - // update the validating WH on k8s. - if err = r.Client.Update(ctx, validatingWH); err != nil { - return errors.Wrap(err, "while updating validatingWH with caBundle") - } - } - - return nil -} diff --git a/internal/controller/operator/eventing/webhook_test.go b/internal/controller/operator/eventing/webhook_test.go deleted file mode 100644 index 03f647a44..000000000 --- a/internal/controller/operator/eventing/webhook_test.go +++ /dev/null @@ -1,251 +0,0 @@ -package eventing - -import ( - "context" - "crypto/rand" - "testing" - - "github.com/stretchr/testify/require" - kadmissionregistrationv1 "k8s.io/api/admissionregistration/v1" - kcorev1 "k8s.io/api/core/v1" - kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/kyma-project/eventing-manager/pkg/env" -) - -func Test_ReconcileWebhooksWithCABundle(t *testing.T) { - t.Parallel() - // given - ctx := context.Background() - dummyCABundle := make([]byte, 20) - _, err := rand.Read(dummyCABundle) - require.NoError(t, err) - newCABundle := make([]byte, 20) - _, err = rand.Read(newCABundle) - require.NoError(t, err) - - testCases := []struct { - name string - givenObjects []client.Object - wantMutatingWH *kadmissionregistrationv1.MutatingWebhookConfiguration - wantValidatingWH *kadmissionregistrationv1.ValidatingWebhookConfiguration - wantError error - }{ - { - name: "secret does not exist", - wantError: errObjectNotFound, - }, - { - name: "secret exits but mutatingWH does not exist", - givenObjects: []client.Object{ - getSecretWithTLSSecret(nil), - }, - wantError: errObjectNotFound, - }, - { - name: "mutatingWH exists, validatingWH does not exist", - givenObjects: []client.Object{ - getSecretWithTLSSecret(nil), - getMutatingWebhookConfig([]kadmissionregistrationv1.MutatingWebhook{ - { - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{}, - }, - }), - }, - wantError: errObjectNotFound, - }, - { - name: "mutatingWH, validatingWH exists but does not contain webhooks", - givenObjects: []client.Object{ - getSecretWithTLSSecret(nil), - getMutatingWebhookConfig(nil), - getValidatingWebhookConfig(nil), - }, - wantError: errInvalidObject, - }, - { - name: "validatingWH does not contain webhooks", - givenObjects: []client.Object{ - getSecretWithTLSSecret(nil), - getMutatingWebhookConfig([]kadmissionregistrationv1.MutatingWebhook{ - { - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{}, - }, - }), - getValidatingWebhookConfig(nil), - }, - wantError: errInvalidObject, - }, - { - name: "WHs do not contain valid CABundle", - givenObjects: []client.Object{ - getSecretWithTLSSecret(dummyCABundle), - getMutatingWebhookConfig([]kadmissionregistrationv1.MutatingWebhook{ - { - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{}, - }, - }), - getValidatingWebhookConfig([]kadmissionregistrationv1.ValidatingWebhook{ - { - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{}, - }, - }), - }, - wantMutatingWH: getMutatingWebhookConfig([]kadmissionregistrationv1.MutatingWebhook{ - { - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{ - CABundle: dummyCABundle, - }, - }, - }), - wantValidatingWH: getValidatingWebhookConfig([]kadmissionregistrationv1.ValidatingWebhook{ - { - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{ - CABundle: dummyCABundle, - }, - }, - }), - wantError: nil, - }, - { - name: "WHs contains valid CABundle", - givenObjects: []client.Object{ - getSecretWithTLSSecret(dummyCABundle), - getMutatingWebhookConfig([]kadmissionregistrationv1.MutatingWebhook{ - { - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{ - CABundle: dummyCABundle, - }, - }, - }), - getValidatingWebhookConfig([]kadmissionregistrationv1.ValidatingWebhook{ - { - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{ - CABundle: dummyCABundle, - }, - }, - }), - }, - wantMutatingWH: getMutatingWebhookConfig([]kadmissionregistrationv1.MutatingWebhook{ - { - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{ - CABundle: dummyCABundle, - }, - }, - }), - wantValidatingWH: getValidatingWebhookConfig([]kadmissionregistrationv1.ValidatingWebhook{ - { - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{ - CABundle: dummyCABundle, - }, - }, - }), - wantError: nil, - }, - { - name: "WHs contains outdated valid CABundle", - givenObjects: []client.Object{ - getSecretWithTLSSecret(newCABundle), - getMutatingWebhookConfig([]kadmissionregistrationv1.MutatingWebhook{ - { - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{ - CABundle: dummyCABundle, - }, - }, - }), - getValidatingWebhookConfig([]kadmissionregistrationv1.ValidatingWebhook{ - { - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{ - CABundle: dummyCABundle, - }, - }, - }), - }, - wantMutatingWH: getMutatingWebhookConfig([]kadmissionregistrationv1.MutatingWebhook{ - { - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{ - CABundle: newCABundle, - }, - }, - }), - wantValidatingWH: getValidatingWebhookConfig([]kadmissionregistrationv1.ValidatingWebhook{ - { - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{ - CABundle: newCABundle, - }, - }, - }), - wantError: nil, - }, - } - - for _, tc := range testCases { - testcase := tc - t.Run(testcase.name, func(t *testing.T) { - t.Parallel() - // given - testEnv := NewMockedUnitTestEnvironment(t, testcase.givenObjects...) - testEnv.Reconciler.backendConfig = getTestBackendConfig() - - // when - err := testEnv.Reconciler.reconcileWebhooksWithCABundle(ctx) - - // then - require.ErrorIs(t, err, testcase.wantError) - if testcase.wantError == nil { - mutatingWH, newErr := testEnv.Reconciler.kubeClient.GetMutatingWebHookConfiguration(ctx, - testEnv.Reconciler.backendConfig.MutatingWebhookName) - require.NoError(t, newErr) - validatingWH, newErr := testEnv.Reconciler.kubeClient.GetValidatingWebHookConfiguration(ctx, - testEnv.Reconciler.backendConfig.ValidatingWebhookName) - require.NoError(t, newErr) - require.Equal(t, mutatingWH.Webhooks[0], testcase.wantMutatingWH.Webhooks[0]) - require.Equal(t, validatingWH.Webhooks[0], testcase.wantValidatingWH.Webhooks[0]) - } - }) - } -} - -func getSecretWithTLSSecret(dummyCABundle []byte) *kcorev1.Secret { - return &kcorev1.Secret{ - TypeMeta: kmetav1.TypeMeta{ - Kind: "Secret", - APIVersion: "v1", - }, - ObjectMeta: kmetav1.ObjectMeta{ - Name: getTestBackendConfig().WebhookSecretName, - Namespace: getTestBackendConfig().Namespace, - }, - Data: map[string][]byte{ - TLSCertField: dummyCABundle, - }, - } -} - -func getMutatingWebhookConfig(webhook []kadmissionregistrationv1.MutatingWebhook) *kadmissionregistrationv1.MutatingWebhookConfiguration { - return &kadmissionregistrationv1.MutatingWebhookConfiguration{ - ObjectMeta: kmetav1.ObjectMeta{ - Name: getTestBackendConfig().MutatingWebhookName, - }, - Webhooks: webhook, - } -} - -func getValidatingWebhookConfig(webhook []kadmissionregistrationv1.ValidatingWebhook) *kadmissionregistrationv1.ValidatingWebhookConfiguration { - return &kadmissionregistrationv1.ValidatingWebhookConfiguration{ - ObjectMeta: kmetav1.ObjectMeta{ - Name: getTestBackendConfig().ValidatingWebhookName, - }, - Webhooks: webhook, - } -} - -func getTestBackendConfig() env.BackendConfig { - return env.BackendConfig{ - WebhookSecretName: "webhookSecret", - MutatingWebhookName: "mutatingWH", - ValidatingWebhookName: "validatingWH", - Namespace: "kyma-system", - } -} diff --git a/pkg/backend/sink/validator.go b/pkg/backend/sink/validator.go deleted file mode 100644 index bf55f8b11..000000000 --- a/pkg/backend/sink/validator.go +++ /dev/null @@ -1,70 +0,0 @@ -package sink - -import ( - "context" - - "golang.org/x/xerrors" - kcorev1 "k8s.io/api/core/v1" - kerrors "k8s.io/apimachinery/pkg/api/errors" - ktypes "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/tools/record" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" - "github.com/kyma-project/eventing-manager/internal/controller/events" - "github.com/kyma-project/eventing-manager/pkg/utils" -) - -type Validator interface { - Validate(ctx context.Context, subscription *v1alpha2.Subscription) error -} - -// ValidatorFunc implements the Validator interface. -type ValidatorFunc func(context.Context, *v1alpha2.Subscription) error - -func (vf ValidatorFunc) Validate(ctx context.Context, sub *v1alpha2.Subscription) error { - return vf(ctx, sub) -} - -type defaultSinkValidator struct { - client client.Client - recorder record.EventRecorder -} - -// Perform a compile-time check. -var _ Validator = &defaultSinkValidator{} - -func NewValidator(client client.Client, recorder record.EventRecorder) Validator { - return &defaultSinkValidator{client: client, recorder: recorder} -} - -func (s defaultSinkValidator) Validate(ctx context.Context, subscription *v1alpha2.Subscription) error { - _, subDomains, err := utils.GetSinkData(subscription.Spec.Sink) - if err != nil { - return err - } - svcNs := subDomains[1] - svcName := subDomains[0] - - // Validate svc is a cluster-local one - if _, err := GetClusterLocalService(ctx, s.client, svcNs, svcName); err != nil { - if kerrors.IsNotFound(err) { - events.Warn(s.recorder, subscription, events.ReasonValidationFailed, "Sink does not correspond to a valid cluster local svc") - return xerrors.Errorf("failed to validate subscription sink URL. It is not a valid cluster local svc: %v", err) - } - - events.Warn(s.recorder, subscription, events.ReasonValidationFailed, "Fetch cluster-local svc failed namespace %s name %s", svcNs, svcName) - return xerrors.Errorf("failed to fetch cluster-local svc for namespace '%s' and name '%s': %v", svcNs, svcName, err) - } - - return nil -} - -func GetClusterLocalService(ctx context.Context, client client.Client, svcNs, svcName string) (*kcorev1.Service, error) { - svcLookupKey := ktypes.NamespacedName{Name: svcName, Namespace: svcNs} - svc := &kcorev1.Service{} - if err := client.Get(ctx, svcLookupKey, svc); err != nil { - return nil, err - } - return svc, nil -} diff --git a/pkg/backend/sink/validator_test.go b/pkg/backend/sink/validator_test.go deleted file mode 100644 index e2a7f5295..000000000 --- a/pkg/backend/sink/validator_test.go +++ /dev/null @@ -1,95 +0,0 @@ -package sink - -import ( - "context" - "strings" - "testing" - - "github.com/stretchr/testify/require" - kcorev1 "k8s.io/api/core/v1" - kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - - eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" - eventingtesting "github.com/kyma-project/eventing-manager/testing" -) - -func TestSinkValidator(t *testing.T) { - // given - namespaceName := "test" - fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() - ctx := context.Background() - recorder := &record.FakeRecorder{} - sinkValidator := NewValidator(fakeClient, recorder) - - testCases := []struct { - name string - givenSubscriptionSink string - givenSvcNameToCreate string - wantErrString string - }{ - { - name: "With invalid URL", - givenSubscriptionSink: "http://invalid Sink", - wantErrString: "failed to parse subscription sink URL", - }, - { - name: "With no existing svc in the cluster", - givenSubscriptionSink: "https://eventing-nats.test.svc.cluster.local:8080", - wantErrString: "failed to validate subscription sink URL. It is not a valid cluster local svc", - }, - { - name: "With no existing svc in the cluster, service has the wrong name", - givenSubscriptionSink: "https://eventing-nats.test.svc.cluster.local:8080", - givenSvcNameToCreate: "test", // wrong name - wantErrString: "failed to validate subscription sink URL. It is not a valid cluster local svc", - }, - { - name: "With a valid sink", - givenSubscriptionSink: "https://eventing-nats.test.svc.cluster.local:8080", - givenSvcNameToCreate: "eventing-nats", - wantErrString: "", - }, - } - - for _, tC := range testCases { - testCase := tC - t.Run(testCase.name, func(t *testing.T) { - // given - sub := eventingtesting.NewSubscription( - "foo", namespaceName, - eventingtesting.WithConditions([]eventingv1alpha2.Condition{}), - eventingtesting.WithStatus(true), - eventingtesting.WithSink(testCase.givenSubscriptionSink), - ) - - // create the service if required for test - if testCase.givenSvcNameToCreate != "" { - svc := &kcorev1.Service{ - ObjectMeta: kmetav1.ObjectMeta{ - Name: testCase.givenSvcNameToCreate, - Namespace: namespaceName, - }, - } - - err := fakeClient.Create(ctx, svc) - require.NoError(t, err) - } - - // when - // call the defaultSinkValidator function - err := sinkValidator.Validate(ctx, sub) - - // then - // given error should match expected error - if testCase.wantErrString == "" { - require.NoError(t, err) - } else { - substringResult := strings.Contains(err.Error(), testCase.wantErrString) - require.True(t, substringResult) - } - }) - } -} diff --git a/pkg/env/backend_config.go b/pkg/env/backend_config.go index 6eedcc048..3f7a5745b 100644 --- a/pkg/env/backend_config.go +++ b/pkg/env/backend_config.go @@ -16,11 +16,6 @@ type BackendConfig struct { EventingCRName string `default:"eventing" envconfig:"EVENTING_CR_NAME"` EventingCRNamespace string `default:"kyma-system" envconfig:"EVENTING_CR_NAMESPACE"` - WebhookSecretName string `default:"eventing-manager-webhook-server-cert" envconfig:"WEBHOOK_SECRET_NAME"` - MutatingWebhookName string `default:"subscription-mutating-webhook-configuration" envconfig:"MUTATING_WEBHOOK_NAME"` - //nolint:lll - ValidatingWebhookName string `default:"subscription-validating-webhook-configuration" envconfig:"VALIDATING_WEBHOOK_NAME"` - DefaultSubscriptionConfig DefaultSubscriptionConfig //nolint:lll diff --git a/pkg/k8s/client.go b/pkg/k8s/client.go index 813c43328..a8c63720f 100644 --- a/pkg/k8s/client.go +++ b/pkg/k8s/client.go @@ -7,7 +7,6 @@ import ( natsv1alpha1 "github.com/kyma-project/nats-manager/api/v1alpha1" istiopkgsecurityv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1" - kadmissionregistrationv1 "k8s.io/api/admissionregistration/v1" kappsv1 "k8s.io/api/apps/v1" kcorev1 "k8s.io/api/core/v1" krbacv1 "k8s.io/api/rbac/v1" @@ -47,9 +46,6 @@ type Client interface { GetNATSResources(ctx context.Context, namespace string) (*natsv1alpha1.NATSList, error) PatchApply(ctx context.Context, object client.Object) error GetSecret(ctx context.Context, namespacedName string) (*kcorev1.Secret, error) - GetMutatingWebHookConfiguration(ctx context.Context, name string) (*kadmissionregistrationv1.MutatingWebhookConfiguration, error) - GetValidatingWebHookConfiguration(ctx context.Context, - name string) (*kadmissionregistrationv1.ValidatingWebhookConfiguration, error) GetCRD(ctx context.Context, name string) (*kapiextensionsv1.CustomResourceDefinition, error) ApplicationCRDExists(ctx context.Context) (bool, error) PeerAuthenticationCRDExists(ctx context.Context) (bool, error) @@ -240,35 +236,6 @@ func (c *KubeClient) APIRuleCRDExists(ctx context.Context) (bool, error) { return true, nil } -// GetMutatingWebHookConfiguration returns the MutatingWebhookConfiguration k8s resource. -func (c *KubeClient) GetMutatingWebHookConfiguration(ctx context.Context, - name string, -) (*kadmissionregistrationv1.MutatingWebhookConfiguration, error) { - var mutatingWH kadmissionregistrationv1.MutatingWebhookConfiguration - mutatingWHKey := client.ObjectKey{ - Name: name, - } - if err := c.client.Get(ctx, mutatingWHKey, &mutatingWH); err != nil { - return nil, err - } - - return &mutatingWH, nil -} - -// GetValidatingWebHookConfiguration returns the ValidatingWebhookConfiguration k8s resource. -func (c *KubeClient) GetValidatingWebHookConfiguration(ctx context.Context, - name string, -) (*kadmissionregistrationv1.ValidatingWebhookConfiguration, error) { - var validatingWH kadmissionregistrationv1.ValidatingWebhookConfiguration - validatingWHKey := client.ObjectKey{ - Name: name, - } - if err := c.client.Get(ctx, validatingWHKey, &validatingWH); err != nil { - return nil, err - } - return &validatingWH, nil -} - func (c *KubeClient) GetSubscriptions(ctx context.Context) (*eventingv1alpha2.SubscriptionList, error) { subscriptions := &eventingv1alpha2.SubscriptionList{} err := c.client.List(ctx, subscriptions) diff --git a/pkg/k8s/client_test.go b/pkg/k8s/client_test.go index b79420cf8..85402897a 100644 --- a/pkg/k8s/client_test.go +++ b/pkg/k8s/client_test.go @@ -2,11 +2,9 @@ package k8s import ( "context" - "crypto/rand" "testing" "github.com/stretchr/testify/require" - kadmissionregistrationv1 "k8s.io/api/admissionregistration/v1" kappsv1 "k8s.io/api/apps/v1" kcorev1 "k8s.io/api/core/v1" krbacv1 "k8s.io/api/rbac/v1" @@ -378,148 +376,6 @@ func Test_GetSecret(t *testing.T) { } } -//nolint:dupl // not the same as validating webhook -func Test_GetMutatingWebHookConfiguration(t *testing.T) { - t.Parallel() - - // given - newCABundle := make([]byte, 20) - _, readErr := rand.Read(newCABundle) - require.NoError(t, readErr) - - // Define test cases as a table. - testCases := []struct { - name string - givenName string - wantMutatingWebhook *kadmissionregistrationv1.MutatingWebhookConfiguration - wantNotFoundError bool - }{ - { - name: "success", - givenName: "test-wh", - wantMutatingWebhook: &kadmissionregistrationv1.MutatingWebhookConfiguration{ - ObjectMeta: kmetav1.ObjectMeta{ - Name: "test-wh", - }, - Webhooks: []kadmissionregistrationv1.MutatingWebhook{ - { - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{ - CABundle: newCABundle, - }, - }, - }, - }, - }, - { - name: "not found", - givenName: "test-wh", - wantMutatingWebhook: nil, - wantNotFoundError: true, - }, - } - - for _, tc := range testCases { - testcase := tc - t.Run(testcase.name, func(t *testing.T) { - t.Parallel() - // given - ctx := context.Background() - fakeClient := fake.NewClientBuilder().Build() - kubeClient := &KubeClient{ - client: fakeClient, - } - - // Create the MutatingWebHookConfiguration if it should exist - if testcase.wantMutatingWebhook != nil { - require.NoError(t, fakeClient.Create(ctx, testcase.wantMutatingWebhook)) - } - - // when - gotWebhook, err := kubeClient.GetMutatingWebHookConfiguration(context.Background(), testcase.givenName) - - // then - if !testcase.wantNotFoundError { - require.NoError(t, err) - require.Equal(t, testcase.wantMutatingWebhook.Webhooks, gotWebhook.Webhooks) - } else { - require.Error(t, err) - require.True(t, kerrors.IsNotFound(err)) - } - }) - } -} - -//nolint:dupl // not the same as mutating webhook -func Test_GetValidatingWebHookConfiguration(t *testing.T) { - t.Parallel() - - // given - newCABundle := make([]byte, 20) - _, readErr := rand.Read(newCABundle) - require.NoError(t, readErr) - - // Define test cases as a table. - testCases := []struct { - name string - givenName string - wantValidatingWebhook *kadmissionregistrationv1.ValidatingWebhookConfiguration - wantNotFoundError bool - }{ - { - name: "success", - givenName: "test-wh", - wantValidatingWebhook: &kadmissionregistrationv1.ValidatingWebhookConfiguration{ - ObjectMeta: kmetav1.ObjectMeta{ - Name: "test-wh", - }, - Webhooks: []kadmissionregistrationv1.ValidatingWebhook{ - { - ClientConfig: kadmissionregistrationv1.WebhookClientConfig{ - CABundle: newCABundle, - }, - }, - }, - }, - }, - { - name: "not found", - givenName: "test-wh", - wantValidatingWebhook: nil, - wantNotFoundError: true, - }, - } - - for _, tc := range testCases { - testcase := tc - t.Run(testcase.name, func(t *testing.T) { - t.Parallel() - // given - ctx := context.Background() - fakeClient := fake.NewClientBuilder().Build() - kubeClient := &KubeClient{ - client: fakeClient, - } - - // Create the ValidatingWebhookConfiguration if it should exist - if testcase.wantValidatingWebhook != nil { - require.NoError(t, fakeClient.Create(ctx, testcase.wantValidatingWebhook)) - } - - // when - gotWebhook, err := kubeClient.GetValidatingWebHookConfiguration(context.Background(), testcase.givenName) - - // then - if !testcase.wantNotFoundError { - require.NoError(t, err) - require.Equal(t, testcase.wantValidatingWebhook.Webhooks, gotWebhook.Webhooks) - } else { - require.Error(t, err) - require.True(t, kerrors.IsNotFound(err)) - } - }) - } -} - func Test_GetCRD(t *testing.T) { t.Parallel() diff --git a/pkg/k8s/mocks/client.go b/pkg/k8s/mocks/client.go index e0e3d3cdd..772ec36dc 100644 --- a/pkg/k8s/mocks/client.go +++ b/pkg/k8s/mocks/client.go @@ -3,13 +3,11 @@ package mocks import ( - admissionregistrationv1 "k8s.io/api/admissionregistration/v1" - appsv1 "k8s.io/api/apps/v1" + context "context" + appsv1 "k8s.io/api/apps/v1" client "sigs.k8s.io/controller-runtime/pkg/client" - context "context" - corev1 "k8s.io/api/core/v1" mock "github.com/stretchr/testify/mock" @@ -578,65 +576,6 @@ func (_c *Client_GetDeploymentDynamic_Call) RunAndReturn(run func(context.Contex return _c } -// GetMutatingWebHookConfiguration provides a mock function with given fields: ctx, name -func (_m *Client) GetMutatingWebHookConfiguration(ctx context.Context, name string) (*admissionregistrationv1.MutatingWebhookConfiguration, error) { - ret := _m.Called(ctx, name) - - if len(ret) == 0 { - panic("no return value specified for GetMutatingWebHookConfiguration") - } - - var r0 *admissionregistrationv1.MutatingWebhookConfiguration - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string) (*admissionregistrationv1.MutatingWebhookConfiguration, error)); ok { - return rf(ctx, name) - } - if rf, ok := ret.Get(0).(func(context.Context, string) *admissionregistrationv1.MutatingWebhookConfiguration); ok { - r0 = rf(ctx, name) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*admissionregistrationv1.MutatingWebhookConfiguration) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { - r1 = rf(ctx, name) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// Client_GetMutatingWebHookConfiguration_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetMutatingWebHookConfiguration' -type Client_GetMutatingWebHookConfiguration_Call struct { - *mock.Call -} - -// GetMutatingWebHookConfiguration is a helper method to define mock.On call -// - ctx context.Context -// - name string -func (_e *Client_Expecter) GetMutatingWebHookConfiguration(ctx interface{}, name interface{}) *Client_GetMutatingWebHookConfiguration_Call { - return &Client_GetMutatingWebHookConfiguration_Call{Call: _e.mock.On("GetMutatingWebHookConfiguration", ctx, name)} -} - -func (_c *Client_GetMutatingWebHookConfiguration_Call) Run(run func(ctx context.Context, name string)) *Client_GetMutatingWebHookConfiguration_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string)) - }) - return _c -} - -func (_c *Client_GetMutatingWebHookConfiguration_Call) Return(_a0 *admissionregistrationv1.MutatingWebhookConfiguration, _a1 error) *Client_GetMutatingWebHookConfiguration_Call { - _c.Call.Return(_a0, _a1) - return _c -} - -func (_c *Client_GetMutatingWebHookConfiguration_Call) RunAndReturn(run func(context.Context, string) (*admissionregistrationv1.MutatingWebhookConfiguration, error)) *Client_GetMutatingWebHookConfiguration_Call { - _c.Call.Return(run) - return _c -} - // GetNATSResources provides a mock function with given fields: ctx, namespace func (_m *Client) GetNATSResources(ctx context.Context, namespace string) (*v1alpha1.NATSList, error) { ret := _m.Called(ctx, namespace) @@ -813,65 +752,6 @@ func (_c *Client_GetSubscriptions_Call) RunAndReturn(run func(context.Context) ( return _c } -// GetValidatingWebHookConfiguration provides a mock function with given fields: ctx, name -func (_m *Client) GetValidatingWebHookConfiguration(ctx context.Context, name string) (*admissionregistrationv1.ValidatingWebhookConfiguration, error) { - ret := _m.Called(ctx, name) - - if len(ret) == 0 { - panic("no return value specified for GetValidatingWebHookConfiguration") - } - - var r0 *admissionregistrationv1.ValidatingWebhookConfiguration - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string) (*admissionregistrationv1.ValidatingWebhookConfiguration, error)); ok { - return rf(ctx, name) - } - if rf, ok := ret.Get(0).(func(context.Context, string) *admissionregistrationv1.ValidatingWebhookConfiguration); ok { - r0 = rf(ctx, name) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*admissionregistrationv1.ValidatingWebhookConfiguration) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { - r1 = rf(ctx, name) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// Client_GetValidatingWebHookConfiguration_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetValidatingWebHookConfiguration' -type Client_GetValidatingWebHookConfiguration_Call struct { - *mock.Call -} - -// GetValidatingWebHookConfiguration is a helper method to define mock.On call -// - ctx context.Context -// - name string -func (_e *Client_Expecter) GetValidatingWebHookConfiguration(ctx interface{}, name interface{}) *Client_GetValidatingWebHookConfiguration_Call { - return &Client_GetValidatingWebHookConfiguration_Call{Call: _e.mock.On("GetValidatingWebHookConfiguration", ctx, name)} -} - -func (_c *Client_GetValidatingWebHookConfiguration_Call) Run(run func(ctx context.Context, name string)) *Client_GetValidatingWebHookConfiguration_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string)) - }) - return _c -} - -func (_c *Client_GetValidatingWebHookConfiguration_Call) Return(_a0 *admissionregistrationv1.ValidatingWebhookConfiguration, _a1 error) *Client_GetValidatingWebHookConfiguration_Call { - _c.Call.Return(_a0, _a1) - return _c -} - -func (_c *Client_GetValidatingWebHookConfiguration_Call) RunAndReturn(run func(context.Context, string) (*admissionregistrationv1.ValidatingWebhookConfiguration, error)) *Client_GetValidatingWebHookConfiguration_Call { - _c.Call.Return(run) - return _c -} - // PatchApply provides a mock function with given fields: ctx, object func (_m *Client) PatchApply(ctx context.Context, object client.Object) error { ret := _m.Called(ctx, object) diff --git a/pkg/subscriptionmanager/eventmesh/eventmesh.go b/pkg/subscriptionmanager/eventmesh/eventmesh.go index 0065e5799..4245cf70d 100644 --- a/pkg/subscriptionmanager/eventmesh/eventmesh.go +++ b/pkg/subscriptionmanager/eventmesh/eventmesh.go @@ -20,10 +20,10 @@ import ( eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" "github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/eventmesh" + "github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/validator" "github.com/kyma-project/eventing-manager/pkg/backend/cleaner" backendeventmesh "github.com/kyma-project/eventing-manager/pkg/backend/eventmesh" "github.com/kyma-project/eventing-manager/pkg/backend/metrics" - "github.com/kyma-project/eventing-manager/pkg/backend/sink" backendutils "github.com/kyma-project/eventing-manager/pkg/backend/utils" "github.com/kyma-project/eventing-manager/pkg/env" "github.com/kyma-project/eventing-manager/pkg/logger" @@ -116,6 +116,9 @@ func (c *SubscriptionManager) Start(_ env.DefaultSubscriptionConfig, params subm client := c.mgr.GetClient() recorder := c.mgr.GetEventRecorderFor("eventing-controller-beb") + // Init the Subscription validator. + subscriptionValidator := validator.NewSubscriptionValidator(client) + // Initialize v1alpha2 handler for EventMesh eventMeshHandler := backendeventmesh.NewEventMesh(oauth2credential, nameMapper, c.logger) eventMeshcleaner := cleaner.NewEventMeshCleaner(c.logger) @@ -128,7 +131,7 @@ func (c *SubscriptionManager) Start(_ env.DefaultSubscriptionConfig, params subm eventMeshHandler, oauth2credential, nameMapper, - sink.NewValidator(client, recorder), + subscriptionValidator, c.collector, c.domain, ) diff --git a/pkg/subscriptionmanager/jetstream/jetstream.go b/pkg/subscriptionmanager/jetstream/jetstream.go index 68795b972..d6f2a734e 100644 --- a/pkg/subscriptionmanager/jetstream/jetstream.go +++ b/pkg/subscriptionmanager/jetstream/jetstream.go @@ -18,10 +18,10 @@ import ( eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" subscriptioncontrollerjetstream "github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/jetstream" + "github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/validator" "github.com/kyma-project/eventing-manager/pkg/backend/cleaner" backendjetstream "github.com/kyma-project/eventing-manager/pkg/backend/jetstream" backendmetrics "github.com/kyma-project/eventing-manager/pkg/backend/metrics" - "github.com/kyma-project/eventing-manager/pkg/backend/sink" backendutils "github.com/kyma-project/eventing-manager/pkg/backend/utils" "github.com/kyma-project/eventing-manager/pkg/env" "github.com/kyma-project/eventing-manager/pkg/logger" @@ -91,6 +91,9 @@ func (sm *SubscriptionManager) Start(defaultSubsConfig env.DefaultSubscriptionCo client := sm.mgr.GetClient() recorder := sm.mgr.GetEventRecorderFor("eventing-controller-jetstream") + // Init the Subscription validator. + subscriptionValidator := validator.NewSubscriptionValidator(client) + // Initialize v1alpha2 event type cleaner jsCleaner := cleaner.NewJetStreamCleaner(sm.logger) jetStreamHandler := backendjetstream.NewJetStream(sm.envCfg, @@ -101,7 +104,7 @@ func (sm *SubscriptionManager) Start(defaultSubsConfig env.DefaultSubscriptionCo sm.logger, recorder, jsCleaner, - sink.NewValidator(client, recorder), + subscriptionValidator, sm.metricsCollector, ) sm.backendv2 = jetStreamReconciler.Backend diff --git a/test/utils/integration/integration.go b/test/utils/integration/integration.go index 560a53cc7..23ad01200 100644 --- a/test/utils/integration/integration.go +++ b/test/utils/integration/integration.go @@ -1,9 +1,7 @@ package integration import ( - "bytes" "context" - "crypto/rand" "fmt" "log" "path/filepath" @@ -19,7 +17,6 @@ import ( "github.com/onsi/gomega" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - kadmissionregistrationv1 "k8s.io/api/admissionregistration/v1" kappsv1 "k8s.io/api/apps/v1" kautoscalingv1 "k8s.io/api/autoscaling/v1" kcorev1 "k8s.io/api/core/v1" @@ -38,7 +35,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" "sigs.k8s.io/controller-runtime/pkg/metrics/server" - "sigs.k8s.io/controller-runtime/pkg/webhook" eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" "github.com/kyma-project/eventing-manager/api/operator/v1alpha1" @@ -147,16 +143,10 @@ func NewTestEnvironment(config TestEnvironmentConfig, connMock *natsconnectionmo } // setup ctrl manager - metricsPort, err := natstestutils.GetFreePort() - if err != nil { - return nil, err - } - ctrlMgr, err := kctrl.NewManager(envTestKubeCfg, kctrl.Options{ Scheme: scheme.Scheme, HealthProbeBindAddress: "0", // disable Metrics: server.Options{BindAddress: "0"}, // disable - WebhookServer: webhook.NewServer(webhook.Options{Port: metricsPort}), }) if err != nil { return nil, err @@ -238,17 +228,6 @@ func NewTestEnvironment(config TestEnvironmentConfig, connMock *natsconnectionmo return nil, err } - // create webhook cert secret. - const caBundleSize = 40 - newCABundle := make([]byte, caBundleSize) - if _, err := rand.Read(newCABundle); err != nil { - return nil, err - } - err = k8sClient.Create(ctx, newSecretWithTLSSecret(newCABundle)) - if err != nil { - return nil, err - } - return &TestEnvironment{ k8sClient: k8sClient, KubeClient: kubeClient, @@ -264,39 +243,6 @@ func NewTestEnvironment(config TestEnvironmentConfig, connMock *natsconnectionmo } 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: webhookClientConfig, - SideEffects: &sideEffectClassNone, - AdmissionReviewVersions: []string{"v1beta1"}, - }, - }) - mwh.Name = getTestBackendConfig().MutatingWebhookName - - // setup dummy validating webhook - vwh := getValidatingWebhookConfig([]kadmissionregistrationv1.ValidatingWebhook{ - { - Name: "reconciler.eventing.test", - ClientConfig: webhookClientConfig, - SideEffects: &sideEffectClassNone, - AdmissionReviewVersions: []string{"v1beta1"}, - }, - }) - vwh.Name = getTestBackendConfig().ValidatingWebhookName - // define CRDs to include. includedCRDs := []string{ filepath.Join(config.ProjectRootDir, "config", "crd", "bases"), @@ -321,10 +267,6 @@ func SetupAndStartEnvTest(config TestEnvironmentConfig) (*envtest.Environment, * ErrorIfCRDPathMissing: true, AttachControlPlaneOutput: attachControlPlaneOutput, UseExistingCluster: &uec, - WebhookInstallOptions: envtest.WebhookInstallOptions{ - MutatingWebhooks: []*kadmissionregistrationv1.MutatingWebhookConfiguration{mwh}, - ValidatingWebhooks: []*kadmissionregistrationv1.ValidatingWebhookConfiguration{vwh}, - }, } args := testEnv.ControlPlane.GetAPIServer().Configure() @@ -364,13 +306,8 @@ func (env TestEnvironment) TearDown() error { env.TestCancelFn() } - // clean-up created resources - err := env.DeleteSecretFromK8s(getTestBackendConfig().WebhookSecretName, getTestBackendConfig().Namespace) - if err != nil { - log.Printf("couldn't clean the webhook secret: %s", err) - } - // retry to stop the api-server + var err error sleepTime := 1 * time.Second const retries = 20 for range retries { @@ -871,50 +808,6 @@ func (env TestEnvironment) EnsureEPPClusterRoleBindingCorrect(t *testing.T, even }, SmallTimeOut, SmallPollingInterval, "failed to ensure ClusterRoleBinding correctness") } -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, - getTestBackendConfig().Namespace) - if err != nil { - env.Logger.WithContext().Error(err) - return false - } - - // get Mutating and validating webhook configurations from k8s. - mwh, err := env.KubeClient.GetMutatingWebHookConfiguration(context.Background(), - getTestBackendConfig().MutatingWebhookName) - if err != nil { - env.Logger.WithContext().Error(err) - return false - } - - vwh, err := env.KubeClient.GetValidatingWebHookConfiguration(context.Background(), - getTestBackendConfig().ValidatingWebhookName) - if err != nil { - env.Logger.WithContext().Error(err) - return false - } - - if len(mwh.Webhooks) == 0 || len(vwh.Webhooks) == 0 { - env.Logger.WithContext().Error("Invalid mutating and validating webhook configurations") - return false - } - - if !bytes.Equal(mwh.Webhooks[0].ClientConfig.CABundle, certSecret.Data[eventingcontroller.TLSCertField]) { - env.Logger.WithContext().Error("CABundle of mutating configuration is not correct") - return false - } - - if !bytes.Equal(vwh.Webhooks[0].ClientConfig.CABundle, certSecret.Data[eventingcontroller.TLSCertField]) { - env.Logger.WithContext().Error("CABundle of validating configuration is not correct") - return false - } - return true - }, SmallTimeOut, SmallPollingInterval, "failed to ensure correctness of CABundle in Webhooks") -} - func (env TestEnvironment) EnsureEventMeshSecretCreated(t *testing.T, eventing *v1alpha1.Eventing) { t.Helper() subarr := strings.Split(eventing.Spec.Backend.Config.EventMeshSecret, "/") @@ -1041,46 +934,9 @@ func (env TestEnvironment) GetNATSFromK8s(name, namespace string) (*natsv1alpha1 return nats, err } -func newSecretWithTLSSecret(dummyCABundle []byte) *kcorev1.Secret { - return &kcorev1.Secret{ - TypeMeta: kmetav1.TypeMeta{ - Kind: "Secret", - APIVersion: "v1", - }, - ObjectMeta: kmetav1.ObjectMeta{ - Name: getTestBackendConfig().WebhookSecretName, - Namespace: getTestBackendConfig().Namespace, - }, - Data: map[string][]byte{ - eventingcontroller.TLSCertField: dummyCABundle, - }, - } -} - -func getMutatingWebhookConfig(webhook []kadmissionregistrationv1.MutatingWebhook) *kadmissionregistrationv1.MutatingWebhookConfiguration { - return &kadmissionregistrationv1.MutatingWebhookConfiguration{ - ObjectMeta: kmetav1.ObjectMeta{ - Name: getTestBackendConfig().MutatingWebhookName, - }, - Webhooks: webhook, - } -} - -func getValidatingWebhookConfig(webhook []kadmissionregistrationv1.ValidatingWebhook) *kadmissionregistrationv1.ValidatingWebhookConfiguration { - return &kadmissionregistrationv1.ValidatingWebhookConfiguration{ - ObjectMeta: kmetav1.ObjectMeta{ - Name: getTestBackendConfig().ValidatingWebhookName, - }, - Webhooks: webhook, - } -} - func getTestBackendConfig() env.BackendConfig { return env.BackendConfig{ - WebhookSecretName: "eventing-manager-webhook-server-cert", - MutatingWebhookName: "subscription-mutating-webhook-configuration", - ValidatingWebhookName: "subscription-validating-webhook-configuration", - Namespace: "kyma-system", + Namespace: "kyma-system", } } diff --git a/testing/matchers.go b/testing/matchers.go index 17b7aea09..b963ba16f 100644 --- a/testing/matchers.go +++ b/testing/matchers.go @@ -191,6 +191,12 @@ func HaveMaxInFlight(maxInFlight int) gomegatypes.GomegaMatcher { }, BeTrue()) } +func HaveTypeMatching(typeMatching eventingv1alpha2.TypeMatching) gomegatypes.GomegaMatcher { + return WithTransform(func(s *eventingv1alpha2.Subscription) bool { + return s.Spec.TypeMatching == typeMatching + }, BeTrue()) +} + func HaveSubscriptionNotReady() gomegatypes.GomegaMatcher { return WithTransform(func(s *eventingv1alpha2.Subscription) bool { return s.Status.Ready