From 341b6cc2f64a414807936b4becb450dae111a8d1 Mon Sep 17 00:00:00 2001 From: marcobebway Date: Thu, 11 Apr 2024 16:35:41 +0200 Subject: [PATCH] feat: remove the mutating and validating webhooks for v1alpha2 Subscription. --- .golangci.yaml | 2 + Makefile | 4 +- api/eventing/v1alpha2/condition.go | 136 ++++++++-- api/eventing/v1alpha2/condition_unit_test.go | 10 +- api/eventing/v1alpha2/subscription_types.go | 21 +- .../v1alpha2/subscription_types_test.go | 70 +++++ ..._webhook.go => subscription_validation.go} | 93 ++----- ...est.go => subscription_validation_test.go} | 197 ++++---------- api/operator/v1alpha1/eventing_types.go | 5 - api/operator/v1alpha1/eventing_types_test.go | 1 - api/operator/v1alpha1/status.go | 17 -- api/operator/v1alpha1/status_test.go | 12 - cmd/main.go | 20 +- ...venting.kyma-project.io_subscriptions.yaml | 6 + 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/kustomization.yaml | 4 +- config/manager/manager.yaml | 15 -- config/rbac/kustomization.yaml | 3 - config/rbac/role.yaml | 12 + 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 ----- docs/user/02-configuration.md | 123 ++++++--- hack/ci/render-sec-scanners-config.sh | 9 - hack/e2e/common/fixtures/fixtures.go | 20 +- hack/e2e/setup/setup_test.go | 63 ----- .../subscription/eventmesh/reconciler.go | 45 +++- .../reconciler_internal_integration_test.go | 21 +- .../test/reconciler_integration_test.go | 127 +++------ .../subscription/eventmesh/test/utils.go | 53 +--- .../eventmesh/testwebhookauth/utils.go | 66 ++--- .../subscription/jetstream/reconciler.go | 75 ++++-- .../jetstream/reconciler_integration_test.go | 80 +----- .../reconciler_internal_unit_test.go | 11 +- .../subscription/jetstream/test_utils_test.go | 55 +--- .../operator/eventing/controller.go | 9 +- .../controller/integration_test.go | 8 +- .../controller/operator/eventing/status.go | 11 - .../controller/operator/eventing/webhook.go | 80 ------ .../operator/eventing/webhook_test.go | 251 ------------------ internal/webhook/cleanup.go | 75 ++++++ pkg/backend/sink/validator.go | 26 +- pkg/backend/sink/validator_test.go | 4 +- pkg/env/backend_config.go | 5 - pkg/k8s/client.go | 35 +-- pkg/k8s/client_test.go | 144 ---------- pkg/k8s/mocks/client.go | 124 +-------- test/utils/integration/integration.go | 152 +---------- 59 files changed, 674 insertions(+), 2068 deletions(-) rename api/eventing/v1alpha2/{subscription_webhook.go => subscription_validation.go} (51%) rename api/eventing/v1alpha2/{subscription_webhook_unit_test.go => subscription_validation_test.go} (66%) 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 delete mode 100644 internal/controller/operator/eventing/webhook.go delete mode 100644 internal/controller/operator/eventing/webhook_test.go create mode 100644 internal/webhook/cleanup.go diff --git a/.golangci.yaml b/.golangci.yaml index bb95d7e5c..e7353ed81 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -72,6 +72,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 diff --git a/Makefile b/Makefile index 5a0c18bdb..283016341 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..0ad5949e8 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 } - 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} - } + return Condition{ + Type: ConditionSubscriptionSpecValid, + Status: status, + LastTransitionTime: kmetav1.Now(), + Reason: reason, + Message: message, + } +} + +// 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 _, c := range s.Conditions { + if c.Type != condition.Type { + conditions = append(conditions, c) + continue + } + isFound = true + if !ConditionEquals(c, 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_unit_test.go b/api/eventing/v1alpha2/condition_unit_test.go index e3f2b2ca8..de2910eef 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,12 +544,12 @@ 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) } }) } diff --git a/api/eventing/v1alpha2/subscription_types.go b/api/eventing/v1alpha2/subscription_types.go index 6adff5845..59e414f9a 100644 --- a/api/eventing/v1alpha2/subscription_types.go +++ b/api/eventing/v1alpha2/subscription_types.go @@ -30,6 +30,8 @@ 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" + // +kubebuilder:validation:XValidation:rule="self=='standard' || self=='exact'", message="typeMatching can only be set to standard or exact" TypeMatching TypeMatching `json:"typeMatching,omitempty"` // Defines the origin of the event. @@ -40,6 +42,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"` } @@ -114,6 +117,21 @@ func (s *Subscription) GetUniqueTypes() []string { return result } +// GetDuplicateTypes returns the duplicate types from subscription spec. +func (s *Subscription) GetDuplicateTypes() []string { + if len(s.Spec.Types) == 0 { + return s.Spec.Types + } + types := make(map[string]int, len(s.Spec.Types)) + duplicates := make([]string, 0, len(s.Spec.Types)) + for _, t := range s.Spec.Types { + if types[t]++; types[t] == 2 { + duplicates = append(duplicates, t) + } + } + return duplicates +} + func (s *Subscription) DuplicateWithStatusDefaults() *Subscription { desiredSub := s.DeepCopy() desiredSub.Status = SubscriptionStatus{} @@ -140,6 +158,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_types_test.go b/api/eventing/v1alpha2/subscription_types_test.go index a4bb989d3..b6b3df3d9 100644 --- a/api/eventing/v1alpha2/subscription_types_test.go +++ b/api/eventing/v1alpha2/subscription_types_test.go @@ -69,3 +69,73 @@ func TestGetMaxInFlightMessages(t *testing.T) { }) } } + +func TestGetDuplicateTypes(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 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) { + s := v1alpha2.Subscription{Spec: v1alpha2.SubscriptionSpec{Types: tt.givenTypes}} + gotDuplicateTypes := s.GetDuplicateTypes() + assert.Equal(t, tt.wantDuplicateTypes, gotDuplicateTypes) + }) + } +} diff --git a/api/eventing/v1alpha2/subscription_webhook.go b/api/eventing/v1alpha2/subscription_validation.go similarity index 51% rename from api/eventing/v1alpha2/subscription_webhook.go rename to api/eventing/v1alpha2/subscription_validation.go index 2b76fea61..15be68e4b 100644 --- a/api/eventing/v1alpha2/subscription_webhook.go +++ b/api/eventing/v1alpha2/subscription_validation.go @@ -4,12 +4,7 @@ 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" @@ -24,51 +19,8 @@ const ( 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) { +func (s *Subscription) ValidateSpec() field.ErrorList { var allErrs field.ErrorList - if err := s.validateSubscriptionSource(); err != nil { allErrs = append(allErrs, err) } @@ -82,43 +34,42 @@ func (s *Subscription) ValidateSubscription() (admission.Warnings, error) { allErrs = append(allErrs, err) } if len(allErrs) == 0 { - return nil, nil + return nil } - - return nil, kerrors.NewInvalid(GroupKind, s.Name, allErrs) + return allErrs } func (s *Subscription) validateSubscriptionSource() *field.Error { if s.Spec.Source == "" && s.Spec.TypeMatching != TypeMatchingExact { - return MakeInvalidFieldError(SourcePath, s.Name, EmptyErrDetail) + return MakeInvalidFieldError(SourcePath, s.Spec.Source, 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 MakeInvalidFieldError(SourcePath, s.Spec.Source, 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) + return MakeInvalidFieldError(TypesPath, "", EmptyErrDetail) } - if len(s.GetUniqueTypes()) != len(s.Spec.Types) { - return MakeInvalidFieldError(TypesPath, s.Name, DuplicateTypesErrDetail) + if duplicates := s.GetDuplicateTypes(); len(duplicates) > 0 { + return MakeInvalidFieldError(TypesPath, strings.Join(duplicates, ","), DuplicateTypesErrDetail) } for _, etype := range s.Spec.Types { if len(etype) == 0 { - return MakeInvalidFieldError(TypesPath, s.Name, LengthErrDetail) + return MakeInvalidFieldError(TypesPath, etype, LengthErrDetail) } if segments := strings.Split(etype, "."); len(segments) < minEventTypeSegments { - return MakeInvalidFieldError(TypesPath, s.Name, MinSegmentErrDetail) + return MakeInvalidFieldError(TypesPath, etype, MinSegmentErrDetail) } if s.Spec.TypeMatching != TypeMatchingExact && strings.HasPrefix(etype, InvalidPrefix) { - return MakeInvalidFieldError(TypesPath, s.Name, InvalidPrefixErrDetail) + return MakeInvalidFieldError(TypesPath, etype, 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 MakeInvalidFieldError(TypesPath, etype, InvalidURIErrDetail) } } return nil @@ -127,48 +78,48 @@ func (s *Subscription) validateSubscriptionTypes() *field.Error { func (s *Subscription) validateSubscriptionConfig() field.ErrorList { var allErrs field.ErrorList if isNotInt(s.Spec.Config[MaxInFlightMessages]) { - allErrs = append(allErrs, MakeInvalidFieldError(ConfigPath, s.Name, StringIntErrDetail)) + allErrs = append(allErrs, MakeInvalidFieldError(ConfigPath, s.Spec.Config[MaxInFlightMessages], StringIntErrDetail)) } if s.ifKeyExistsInConfig(ProtocolSettingsQos) && types.IsInvalidQoS(s.Spec.Config[ProtocolSettingsQos]) { - allErrs = append(allErrs, MakeInvalidFieldError(ConfigPath, s.Name, InvalidQosErrDetail)) + allErrs = append(allErrs, MakeInvalidFieldError(ConfigPath, s.Spec.Config[ProtocolSettingsQos], InvalidQosErrDetail)) } if s.ifKeyExistsInConfig(WebhookAuthType) && types.IsInvalidAuthType(s.Spec.Config[WebhookAuthType]) { - allErrs = append(allErrs, MakeInvalidFieldError(ConfigPath, s.Name, InvalidAuthTypeErrDetail)) + allErrs = append(allErrs, MakeInvalidFieldError(ConfigPath, s.Spec.Config[WebhookAuthType], InvalidAuthTypeErrDetail)) } if s.ifKeyExistsInConfig(WebhookAuthGrantType) && types.IsInvalidGrantType(s.Spec.Config[WebhookAuthGrantType]) { - allErrs = append(allErrs, MakeInvalidFieldError(ConfigPath, s.Name, InvalidGrantTypeErrDetail)) + allErrs = append(allErrs, MakeInvalidFieldError(ConfigPath, s.Spec.Config[WebhookAuthGrantType], InvalidGrantTypeErrDetail)) } return allErrs } func (s *Subscription) validateSubscriptionSink() *field.Error { if s.Spec.Sink == "" { - return MakeInvalidFieldError(SinkPath, s.Name, EmptyErrDetail) + return MakeInvalidFieldError(SinkPath, s.Spec.Sink, EmptyErrDetail) } if !utils.IsValidScheme(s.Spec.Sink) { - return MakeInvalidFieldError(SinkPath, s.Name, MissingSchemeErrDetail) + return MakeInvalidFieldError(SinkPath, s.Spec.Sink, MissingSchemeErrDetail) } trimmedHost, subDomains, err := utils.GetSinkData(s.Spec.Sink) if err != nil { - return MakeInvalidFieldError(SinkPath, s.Name, err.Error()) + return MakeInvalidFieldError(SinkPath, s.Spec.Sink, err.Error()) } // Validate sink URL is a cluster local URL. if !strings.HasSuffix(trimmedHost, ClusterLocalURLSuffix) { - return MakeInvalidFieldError(SinkPath, s.Name, SuffixMissingErrDetail) + return MakeInvalidFieldError(SinkPath, s.Spec.Sink, SuffixMissingErrDetail) } // We expected a sink in the format "service.namespace.svc.cluster.local". if len(subDomains) != subdomainSegments { - return MakeInvalidFieldError(SinkPath, s.Name, SubDomainsErrDetail+trimmedHost) + return MakeInvalidFieldError(SinkPath, s.Spec.Sink, 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 MakeInvalidFieldError(NSPath, s.Spec.Sink, NSMismatchErrDetail+svcNs) } return nil diff --git a/api/eventing/v1alpha2/subscription_webhook_unit_test.go b/api/eventing/v1alpha2/subscription_validation_test.go similarity index 66% rename from api/eventing/v1alpha2/subscription_webhook_unit_test.go rename to api/eventing/v1alpha2/subscription_validation_test.go index 9708884d2..533fcfb2a 100644 --- a/api/eventing/v1alpha2/subscription_webhook_unit_test.go +++ b/api/eventing/v1alpha2/subscription_validation_test.go @@ -4,7 +4,6 @@ import ( "testing" "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" @@ -17,74 +16,12 @@ const ( sink = "https://eventing-nats.test.svc.cluster.local:8080" ) -func Test_Default(t *testing.T) { - t.Parallel() - type TestCase struct { - name string - givenSub *v1alpha2.Subscription - wantSub *v1alpha2.Subscription - } - - 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", - 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(), - ), - }, - { - name: "Add default MaxInFlightMessages value only", - 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), - ), - }, - } - - 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 + wantErr field.ErrorList } testCases := []TestCase{ @@ -107,10 +44,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SourcePath, - subName, v1alpha2.EmptyErrDetail)}), + wantErr: field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SourcePath, + "", v1alpha2.EmptyErrDetail)}, }, { name: "valid source and TypeMatching Standard should not return error", @@ -142,10 +77,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SourcePath, - subName, v1alpha2.InvalidURIErrDetail)}), + wantErr: field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SourcePath, + "s%ourc%e", v1alpha2.InvalidURIErrDetail)}, }, { name: "nil types field should return error", @@ -155,10 +88,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, - subName, v1alpha2.EmptyErrDetail)}), + wantErr: field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, + "", v1alpha2.EmptyErrDetail)}, }, { name: "empty types field should return error", @@ -169,10 +100,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, - subName, v1alpha2.EmptyErrDetail)}), + wantErr: field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, + "", v1alpha2.EmptyErrDetail)}, }, { name: "duplicate types should return error", @@ -186,10 +115,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, - subName, v1alpha2.DuplicateTypesErrDetail)}), + wantErr: field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, + "order.created.v1", v1alpha2.DuplicateTypesErrDetail)}, }, { name: "empty event type should return error", @@ -200,10 +127,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, - subName, v1alpha2.LengthErrDetail)}), + wantErr: field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, + "", v1alpha2.LengthErrDetail)}, }, { name: "lower than min segments should return error", @@ -214,10 +139,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, - subName, v1alpha2.MinSegmentErrDetail)}), + wantErr: field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, + "order", v1alpha2.MinSegmentErrDetail)}, }, { name: "invalid prefix should return error", @@ -228,10 +151,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, - subName, v1alpha2.InvalidPrefixErrDetail)}), + wantErr: field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.TypesPath, + "sap.kyma.custom", v1alpha2.InvalidPrefixErrDetail)}, }, { name: "invalid prefix with exact should not return error", @@ -253,10 +174,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{v1alpha2.MakeInvalidFieldError(v1alpha2.ConfigPath, + "invalid", v1alpha2.StringIntErrDetail)}, }, { name: "invalid QoS value should return error", @@ -268,10 +187,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithInvalidProtocolSettingsQos(), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.ConfigPath, - subName, v1alpha2.InvalidQosErrDetail)}), + wantErr: field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.ConfigPath, + "AT_INVALID_ONCE", v1alpha2.InvalidQosErrDetail)}, }, { name: "invalid webhook auth type value should return error", @@ -283,10 +200,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithInvalidWebhookAuthType(), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.ConfigPath, - subName, v1alpha2.InvalidAuthTypeErrDetail)}), + wantErr: field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.ConfigPath, + "abcd", v1alpha2.InvalidAuthTypeErrDetail)}, }, { name: "invalid webhook grant type value should return error", @@ -298,10 +213,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithInvalidWebhookAuthGrantType(), eventingtesting.WithSink(sink), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.ConfigPath, - subName, v1alpha2.InvalidGrantTypeErrDetail)}), + wantErr: field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.ConfigPath, + "invalid", v1alpha2.InvalidGrantTypeErrDetail)}, }, { name: "missing sink should return error", @@ -311,10 +224,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithEventType(eventingtesting.OrderCreatedV1Event), eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SinkPath, - subName, v1alpha2.EmptyErrDetail)}), + wantErr: field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SinkPath, + "", v1alpha2.EmptyErrDetail)}, }, { name: "sink with invalid scheme should return error", @@ -325,10 +236,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), eventingtesting.WithSink(subNamespace), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SinkPath, - subName, v1alpha2.MissingSchemeErrDetail)}), + wantErr: field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SinkPath, + "test", v1alpha2.MissingSchemeErrDetail)}, }, { name: "sink with invalid URL should return error", @@ -339,11 +248,9 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), 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{v1alpha2.MakeInvalidFieldError(v1alpha2.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", @@ -354,10 +261,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), eventingtesting.WithSink("https://svc2.test.local"), ), - wantErr: kerrors.NewInvalid( - v1alpha2.GroupKind, subName, - field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SinkPath, - subName, v1alpha2.SuffixMissingErrDetail)}), + wantErr: field.ErrorList{v1alpha2.MakeInvalidFieldError(v1alpha2.SinkPath, + "https://svc2.test.local", v1alpha2.SuffixMissingErrDetail)}, }, { name: "sink with invalid suffix and port should return error", @@ -368,10 +273,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), 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{v1alpha2.MakeInvalidFieldError(v1alpha2.SinkPath, + "https://svc2.test.local:8080", v1alpha2.SuffixMissingErrDetail)}, }, { name: "sink with invalid number of subdomains should return error", @@ -382,10 +285,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), 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{v1alpha2.MakeInvalidFieldError(v1alpha2.SinkPath, + "https://svc.cluster.local:8080", v1alpha2.SubDomainsErrDetail+"svc.cluster.local")}, }, { name: "sink with different namespace should return error", @@ -396,10 +297,8 @@ func Test_validateSubscription(t *testing.T) { eventingtesting.WithMaxInFlightMessages(v1alpha2.DefaultMaxInFlightMessages), 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{v1alpha2.MakeInvalidFieldError(v1alpha2.NSPath, + "https://eventing-nats.kyma-system.svc.cluster.local", v1alpha2.NSMismatchErrDetail+"kyma-system")}, }, { name: "multiple errors should be reported if exists", @@ -409,14 +308,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{ + v1alpha2.MakeInvalidFieldError(v1alpha2.SourcePath, + "", v1alpha2.EmptyErrDetail), + v1alpha2.MakeInvalidFieldError(v1alpha2.ConfigPath, + "invalid", v1alpha2.StringIntErrDetail), + }, }, } @@ -424,7 +321,7 @@ func Test_validateSubscription(t *testing.T) { tc := testCase t.Run(tc.name, func(t *testing.T) { t.Parallel() - _, err := tc.givenSub.ValidateSubscription() + err := tc.givenSub.ValidateSpec() require.Equal(t, tc.wantErr, err) }) } 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 f1d74d18c..ed233f7dc 100644 --- a/api/operator/v1alpha1/status.go +++ b/api/operator/v1alpha1/status.go @@ -35,19 +35,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(&es.Conditions, condition) -} - func (es *EventingStatus) UpdateConditionSubscriptionManagerReady(status kmetav1.ConditionStatus, reason ConditionReason, message string, ) { @@ -114,10 +101,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 bf240ffe1..a0a5b0359 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/cmd/main.go b/cmd/main.go index b28138479..72e883410 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -18,13 +18,13 @@ package main import ( "context" + "errors" "flag" "log" "os" "time" "github.com/go-logr/zapr" - apigatewayv1beta1 "github.com/kyma-project/api-gateway/apis/gateway/v1beta1" natsio "github.com/nats-io/nats.go" kapiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" kapixclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" @@ -37,14 +37,15 @@ 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" + apigatewayv1beta1 "github.com/kyma-project/api-gateway/apis/gateway/v1beta1" eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" operatorv1alpha1 "github.com/kyma-project/eventing-manager/api/operator/v1alpha1" natsconnection "github.com/kyma-project/eventing-manager/internal/connection/nats" controllercache "github.com/kyma-project/eventing-manager/internal/controller/cache" controllerclient "github.com/kyma-project/eventing-manager/internal/controller/client" eventingcontroller "github.com/kyma-project/eventing-manager/internal/controller/operator/eventing" + "github.com/kyma-project/eventing-manager/internal/webhook" "github.com/kyma-project/eventing-manager/options" backendmetrics "github.com/kyma-project/eventing-manager/pkg/backend/metrics" "github.com/kyma-project/eventing-manager/pkg/env" @@ -69,7 +70,6 @@ func registerSchemas(scheme *runtime.Scheme) { const ( defaultMetricsPort = 9443 - webhookServerPort = 9443 ) func syncLogger(logger *logger.Logger) { @@ -113,7 +113,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 +199,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 { @@ -226,6 +218,12 @@ func main() { //nolint:funlen // main function needs to initialize many object os.Exit(1) } + if errs := webhook.CleanupResources(ctx, k8sClient); len(errs) > 0 { + setupLog.Error(errors.Join(errs...), "unable to cleanup kubernetes webhook resources") + syncLogger(ctrLogger) + os.Exit(1) + } + setupLog.Info("starting manager") if err = mgr.Start(kctrl.SetupSignalHandler()); err != nil { setupLog.Error(err, "problem running manager") diff --git a/config/crd/bases/eventing.kyma-project.io_subscriptions.yaml b/config/crd/bases/eventing.kyma-project.io_subscriptions.yaml index e69d77c82..2f3abb9e6 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,11 +66,15 @@ 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.
- `exact`: no further processing will be applied to the configured source and types. type: string + x-kubernetes-validations: + - message: typeMatching can only be set to standard or exact + rule: self=='standard' || self=='exact' types: description: List of event types that will be used for subscribing on the backend. 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/kustomization.yaml b/config/manager/kustomization.yaml index c6ac7188c..3c89f0527 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -7,5 +7,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization images: - name: controller - newName: europe-docker.pkg.dev/kyma-project/prod/eventing-manager - newTag: v20231102-88a52f17 + newName: marcobebway/eventing-manager + newTag: dev0 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/role.yaml b/config/rbac/role.yaml index f36694b53..95a08f5fc 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -127,6 +127,18 @@ rules: - patch - update - watch +- apiGroups: + - batch + resources: + - cronjobs + verbs: + - delete +- apiGroups: + - batch + resources: + - jobs + verbs: + - delete - apiGroups: - eventing.kyma-project.io resources: 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/docs/user/02-configuration.md b/docs/user/02-configuration.md index 52788f49f..30121e143 100644 --- a/docs/user/02-configuration.md +++ b/docs/user/02-configuration.md @@ -52,47 +52,92 @@ Use the following sample CRs as guidance. Each can be applied immediately when y **Spec:** -| Parameter | Type | Description | -|----------------------------------------------------------|-----------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| **annotations** | map\[string\]string | Annotations allows to add annotations to resources. | -| **backend** | object | Backend defines the active backend used by Eventing. | -| **backend.​config** | object | Config defines configuration for eventing backend. | -| **backend.​config.​domain** | string | Domain defines the cluster public domain used to configure the EventMesh Subscriptions and their corresponding ApiRules. | -| **backend.​config.​eventMeshSecret** | string | EventMeshSecret defines the namespaced name of K8s Secret containing EventMesh credentials. The format of name is "namespace/name". | -| **backend.​config.​eventTypePrefix** | string | | -| **backend.​config.​natsMaxMsgsPerTopic** | integer | NATSMaxMsgsPerTopic limits how many messages in the NATS stream to retain per subject. | -| **backend.​config.​natsStreamMaxSize** | \{integer or string\} | NATSStreamMaxSize defines the maximum storage size for stream data. | -| **backend.​config.​natsStreamReplicas** | integer | NATSStreamReplicas defines the number of replicas for stream. | -| **backend.​config.​natsStreamStorageType** | string | NATSStreamStorageType defines the storage type for stream data. | -| **backend.​type** (required) | string | Type defines which backend to use. The value is either `EventMesh`, or `NATS`. | -| **labels** | map\[string\]string | Labels allows to add Labels to resources. | -| **logging** | object | Logging defines the log level for eventing-manager. | -| **logging.​logLevel** | string | LogLevel defines the log level. | -| **publisher** | object | Publisher defines the configurations for eventing-publisher-proxy. | -| **publisher.​replicas** | object | Replicas defines the scaling min/max for eventing-publisher-proxy. | -| **publisher.​replicas.​max** | integer | Max defines maximum number of replicas. | -| **publisher.​replicas.​min** | integer | Min defines minimum number of replicas. | -| **publisher.​resources** | object | Resources defines resources for eventing-publisher-proxy. | -| **publisher.​resources.​claims** | \[\]object | Claims lists the names of resources, defined in spec.resourceClaims, that are used by this container. This is an alpha field and requires enabling the DynamicResourceAllocation feature gate. This field is immutable. It can only be set for containers. | -| **publisher.​resources.​claims.​name** (required) | string | Name must match the name of one entry in pod.spec.resourceClaims of the Pod where this field is used. It makes that resource available inside a container. | -| **publisher.​resources.​limits** | map\[string\]\{integer or string\} | Limits describes the maximum amount of compute resources allowed. For more information, check [Resource Management for Pods and Containers](https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/). | -| **publisher.​resources.​requests** | map\[string\]\{integer or string\} | Requests describes the minimum amount of compute resources required. If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value. Requests cannot exceed Limits. For more information, check [Resource Management for Pods and Containers](https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/). | +| Parameter | Type | Description | +| ---- | ----------- | ---- | +| **annotations** | map\[string\]string | Annotations allows to add annotations to resources. | +| **backend** | object | Backend defines the active backend used by Eventing. | +| **backend.​config** | object | Config defines configuration for eventing backend. | +| **backend.​config.​domain** | string | Domain defines the cluster public domain used to configure the EventMesh Subscriptions +and their corresponding ApiRules. | +| **backend.​config.​eventMeshSecret** | string | EventMeshSecret defines the namespaced name of K8s Secret containing EventMesh credentials. The format of name is "namespace/name". | +| **backend.​config.​eventTypePrefix** | string | | +| **backend.​config.​natsMaxMsgsPerTopic** | integer | NATSMaxMsgsPerTopic limits how many messages in the NATS stream to retain per subject. | +| **backend.​config.​natsStreamMaxSize** | \{integer or string\} | NATSStreamMaxSize defines the maximum storage size for stream data. | +| **backend.​config.​natsStreamReplicas** | integer | NATSStreamReplicas defines the number of replicas for stream. | +| **backend.​config.​natsStreamStorageType** | string | NATSStreamStorageType defines the storage type for stream data. | +| **backend.​type** (required) | string | Type defines which backend to use. The value is either `EventMesh`, or `NATS`. | +| **labels** | map\[string\]string | Labels allows to add Labels to resources. | +| **logging** | object | Logging defines the log level for eventing-manager. | +| **logging.​logLevel** | string | LogLevel defines the log level. | +| **publisher** | object | Publisher defines the configurations for eventing-publisher-proxy. | +| **publisher.​replicas** | object | Replicas defines the scaling min/max for eventing-publisher-proxy. | +| **publisher.​replicas.​max** | integer | Max defines maximum number of replicas. | +| **publisher.​replicas.​min** | integer | Min defines minimum number of replicas. | +| **publisher.​resources** | object | Resources defines resources for eventing-publisher-proxy. | +| **publisher.​resources.​claims** | \[\]object | Claims lists the names of resources, defined in spec.resourceClaims, +that are used by this container. + + +This is an alpha field and requires enabling the +DynamicResourceAllocation feature gate. + + +This field is immutable. It can only be set for containers. | +| **publisher.​resources.​claims.​name** (required) | string | Name must match the name of one entry in pod.spec.resourceClaims of +the Pod where this field is used. It makes that resource available +inside a container. | +| **publisher.​resources.​limits** | map\[string\]\{integer or string\} | Limits describes the maximum amount of compute resources allowed. +More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ | +| **publisher.​resources.​requests** | map\[string\]\{integer or string\} | Requests describes the minimum amount of compute resources required. +If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, +otherwise to an implementation-defined value. Requests cannot exceed Limits. +More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ | **Status:** -| Parameter | Type | Description | -|------------------------------------------------------|------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| **activeBackend** (required) | string | | -| **conditions** | \[\]object | Condition contains details for one aspect of the current state of this API Resource. --- This struct is intended for direct use as an array at the field path .status.conditions. For example, type FooStatus struct{ // Represents the observations of a foo's current state. // Known .status.conditions.type are: "Available", "Progressing", and "Degraded" // +patchMergeKey=type // +patchStrategy=merge // +listType=map // +listMapKey=type Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"` | -| // other fields } | | | -| **conditions.​lastTransitionTime** (required) | string | lastTransitionTime is the last time the condition transitioned from one status to another. This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. | -| **conditions.​message** (required) | string | message is a human readable message indicating details about the transition. This may be an empty string. | -| **conditions.​observedGeneration** | integer | observedGeneration represents the .metadata.generation that the condition was set based upon. For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date with respect to the current state of the instance. | -| **conditions.​reason** (required) | string | reason contains a programmatic identifier indicating the reason for the condition's last transition. Producers of specific condition types may define expected values and meanings for this field, and whether the values are considered a guaranteed API. The value should be a CamelCase string. This field may not be empty. | -| **conditions.​status** (required) | string | status of the condition, one of `True`, `False`, `Unknown`. | -| **conditions.​type** (required) | string | type of condition in CamelCase or in foo.example.com/CamelCase. --- Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be useful (see .node.status.conditions), the ability to deconflict is important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) | -| **publisherService** | string | | -| **specHash** (required) | integer | | -| **state** (required) | string | Can have one of the following values: `Ready`, `Error`, `Processing`, `Warning`. `Ready` state is set when all the resources are deployed successfully and backend is connected. It gets `Warning` state if backend is not specified, NATS module is not installed or EventMesh Secret is missing in the cluster. `Error` state is set when there is an error not caused by a user. `Processing` state is set when eventing is initiliazed or backend is being switched. | +| Parameter | Type | Description | +| ---- | ----------- | ---- | +| **activeBackend** (required) | string | | +| **conditions** | \[\]object | Condition contains details for one aspect of the current state of this API Resource. +--- +This struct is intended for direct use as an array at the field path .status.conditions. For example, + + + type FooStatus struct{ + // Represents the observations of a foo's current state. + // Known .status.conditions.type are: "Available", "Progressing", and "Degraded" + // +patchMergeKey=type + // +patchStrategy=merge + // +listType=map + // +listMapKey=type + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"` + + + // other fields + } | +| **conditions.​lastTransitionTime** (required) | string | lastTransitionTime is the last time the condition transitioned from one status to another. +This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. | +| **conditions.​message** (required) | string | message is a human readable message indicating details about the transition. +This may be an empty string. | +| **conditions.​observedGeneration** | integer | observedGeneration represents the .metadata.generation that the condition was set based upon. +For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date +with respect to the current state of the instance. | +| **conditions.​reason** (required) | string | reason contains a programmatic identifier indicating the reason for the condition's last transition. +Producers of specific condition types may define expected values and meanings for this field, +and whether the values are considered a guaranteed API. +The value should be a CamelCase string. +This field may not be empty. | +| **conditions.​status** (required) | string | status of the condition, one of True, False, Unknown. | +| **conditions.​type** (required) | string | type of condition in CamelCase or in foo.example.com/CamelCase. +--- +Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be +useful (see .node.status.conditions), the ability to deconflict is important. +The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) | +| **publisherService** | string | | +| **specHash** (required) | integer | | +| **state** (required) | string | Can have one of the following values: Ready, Error, Processing, Warning. Ready state is set +when all the resources are deployed successfully and backend is connected. +It gets Warning state in case backend is not specified, NATS module is not installed, or EventMesh secret is missing in the cluster. +Error state is set when there is an error. Processing state is set if recources are being created or changed. | 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 < 0 { + return errors.Join(errList.ToAggregate()) + } + return r.sinkValidator.Validate(ctx, subscription) +} + // updateSubscription updates the subscription changes to k8s. func (r *Reconciler) updateSubscription(ctx context.Context, sub *eventingv1alpha2.Subscription, logger *zap.SugaredLogger) error { namespacedName := &ktypes.NamespacedName{ @@ -281,7 +308,7 @@ func (r *Reconciler) syncEventMeshSubscription(subscription *eventingv1alpha2.Su logger.Debug("Syncing subscription with EventMesh") if apiRule == nil { - return false, errors.Errorf("APIRule is required") + return false, pkgerrors.Errorf("APIRule is required") } if _, err := r.Backend.SyncSubscription(subscription, r.cleaner, apiRule); err != nil { @@ -357,10 +384,6 @@ func syncConditionWebhookCallStatus(subscription *eventingv1alpha2.Subscription) func (r *Reconciler) syncAPIRule(ctx context.Context, subscription *eventingv1alpha2.Subscription, logger *zap.SugaredLogger, ) (*apigatewayv1beta1.APIRule, error) { - if err := r.sinkValidator.Validate(ctx, subscription); err != nil { - return nil, err - } - sURL, err := url.ParseRequestURI(subscription.Spec.Sink) if err != nil { events.Warn(r.recorder, subscription, events.ReasonValidationFailed, @@ -389,7 +412,7 @@ func (r *Reconciler) syncAPIRule(ctx context.Context, subscription *eventingv1al return apiRule, nil } - return apiRule, controllererrors.NewSkippable(errors.Errorf("apiRule %s is not ready", apiRule.Name)) + return apiRule, controllererrors.NewSkippable(pkgerrors.Errorf("apiRule %s is not ready", apiRule.Name)) } // createOrUpdateAPIRule create new or update existing APIRule for the given subscription. @@ -789,7 +812,7 @@ func (r *Reconciler) checkStatusActive(subscription *eventingv1alpha2.Subscripti if activationTime, err := time.Parse(time.RFC3339, subscription.Status.Backend.FailedActivation); err != nil { return false, err } else if now.Sub(activationTime) > 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 +830,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 +842,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..fb99d1b6c 100644 --- a/internal/controller/eventing/subscription/eventmesh/reconciler_internal_integration_test.go +++ b/internal/controller/eventing/subscription/eventmesh/reconciler_internal_integration_test.go @@ -6,8 +6,6 @@ import ( "testing" "time" - apigatewayv1beta1 "github.com/kyma-project/api-gateway/apis/gateway/v1beta1" - kymalogger "github.com/kyma-project/kyma/common/logging/logger" "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -21,7 +19,9 @@ 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" + apigatewayv1beta1 "github.com/kyma-project/api-gateway/apis/gateway/v1beta1" eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" "github.com/kyma-project/eventing-manager/pkg/backend/cleaner" "github.com/kyma-project/eventing-manager/pkg/backend/eventmesh" @@ -36,6 +36,7 @@ import ( "github.com/kyma-project/eventing-manager/pkg/object" "github.com/kyma-project/eventing-manager/test/utils" eventingtesting "github.com/kyma-project/eventing-manager/testing" + kymalogger "github.com/kyma-project/kyma/common/logging/logger" ) // TestReconciler_Reconcile tests the return values of the Reconcile() method of the reconciler. @@ -54,6 +55,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,6 +73,7 @@ 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") @@ -197,7 +200,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,6 +259,7 @@ 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 }) @@ -386,6 +390,7 @@ 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 }) @@ -605,6 +610,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) { @@ -632,6 +640,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) { @@ -772,12 +783,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 +804,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 +818,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}, }, }, } 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..97af2c608 100644 --- a/internal/controller/eventing/subscription/eventmesh/test/reconciler_integration_test.go +++ b/internal/controller/eventing/subscription/eventmesh/test/reconciler_integration_test.go @@ -9,7 +9,6 @@ import ( "testing" "time" - apigatewayv1beta1 "github.com/kyma-project/api-gateway/apis/gateway/v1beta1" "github.com/onsi/gomega" gomegatypes "github.com/onsi/gomega/types" "github.com/stretchr/testify/assert" @@ -17,6 +16,7 @@ import ( kcorev1 "k8s.io/api/core/v1" kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + apigatewayv1beta1 "github.com/kyma-project/api-gateway/apis/gateway/v1beta1" eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" emstypes "github.com/kyma-project/eventing-manager/pkg/ems/api/events/types" "github.com/kyma-project/eventing-manager/pkg/object" @@ -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,87 +49,6 @@ 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 @@ -154,14 +71,15 @@ func Test_CreateSubscription(t *testing.T) { }), eventingtesting.WithWebhookAuthForEventMesh(), eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, "invalid")), + eventingtesting.WithMaxInFlight(10), ) }, wantSubscriptionMatchers: gomega.And( eventingtesting.HaveSubscriptionNotReady(), eventingtesting.HaveCondition(eventingv1alpha2.MakeCondition( - eventingv1alpha2.ConditionAPIRuleStatus, - eventingv1alpha2.ConditionReasonAPIRuleStatusNotReady, - kcorev1.ConditionFalse, invalidSinkErrMsg)), + eventingv1alpha2.ConditionSubscriptionSpecValid, + eventingv1alpha2.ConditionReasonSubscriptionSpecHasValidationErrors, + kcorev1.ConditionFalse, "Sink validation failed: Service \"invalid\" not found")), ), }, { @@ -175,6 +93,7 @@ func Test_CreateSubscription(t *testing.T) { }), eventingtesting.WithWebhookAuthForEventMesh(), eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, testName)), + eventingtesting.WithMaxInFlight(10), ) }, wantSubscriptionMatchers: gomega.And( @@ -275,6 +194,7 @@ func Test_CreateSubscription(t *testing.T) { eventingtesting.WithNotCleanType(), eventingtesting.WithWebhookAuthForEventMesh(), eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, testName)), + eventingtesting.WithMaxInFlight(10), ) }, wantSubscriptionMatchers: gomega.And( @@ -377,6 +297,7 @@ func Test_UpdateSubscription(t *testing.T) { }), eventingtesting.WithWebhookAuthForEventMesh(), eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, testName)), + eventingtesting.WithMaxInFlight(10), ) }, wantSubscriptionMatchers: gomega.And( @@ -397,6 +318,7 @@ func Test_UpdateSubscription(t *testing.T) { }), eventingtesting.WithWebhookAuthForEventMesh(), eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, testName)), + eventingtesting.WithMaxInFlight(10), ) }, wantUpdateSubscriptionMatchers: gomega.And( @@ -423,6 +345,7 @@ func Test_UpdateSubscription(t *testing.T) { }), eventingtesting.WithWebhookAuthForEventMesh(), eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, testName)), + eventingtesting.WithMaxInFlight(10), ) }, wantSubscriptionMatchers: gomega.And( @@ -446,6 +369,7 @@ func Test_UpdateSubscription(t *testing.T) { }), eventingtesting.WithWebhookAuthForEventMesh(), eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, testName)), + eventingtesting.WithMaxInFlight(10), ) }, wantUpdateSubscriptionMatchers: gomega.And( @@ -472,6 +396,7 @@ func Test_UpdateSubscription(t *testing.T) { }), eventingtesting.WithWebhookAuthForEventMesh(), eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, testName)), + eventingtesting.WithMaxInFlight(10), ) }, wantSubscriptionMatchers: gomega.And( @@ -494,6 +419,7 @@ func Test_UpdateSubscription(t *testing.T) { }), eventingtesting.WithWebhookAuthForEventMesh(), eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, testName)), + eventingtesting.WithMaxInFlight(10), ) }, wantUpdateSubscriptionMatchers: gomega.And( @@ -615,15 +541,16 @@ func Test_FixingSinkAndApiRule(t *testing.T) { eventingtesting.WithWebhookAuthForEventMesh(), // The following sink is invalid because it has an invalid svc name eventingtesting.WithSinkURL(eventingtesting.ValidSinkURL(namespace, "invalid")), + eventingtesting.WithMaxInFlight(10), ) } wantSubscriptionWithoutSinkMatchers := gomega.And( eventingtesting.HaveCondition(eventingv1alpha2.MakeCondition( - eventingv1alpha2.ConditionAPIRuleStatus, - eventingv1alpha2.ConditionReasonAPIRuleStatusNotReady, + eventingv1alpha2.ConditionSubscriptionSpecValid, + eventingv1alpha2.ConditionReasonSubscriptionSpecHasValidationErrors, kcorev1.ConditionFalse, - invalidSinkErrMsg, + "Sink validation failed: Service \"invalid\" not found", )), ) @@ -633,6 +560,7 @@ func Test_FixingSinkAndApiRule(t *testing.T) { eventingtesting.WithOrderCreatedV1Event(), eventingtesting.WithWebhookAuthForEventMesh(), eventingtesting.WithSink(fmt.Sprintf(sinkFormat, name, namespace, path)), + eventingtesting.WithMaxInFlight(10), ) } @@ -814,7 +742,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 @@ -822,12 +750,16 @@ func Test_APIRuleReUseAfterUpdatingSink(t *testing.T) { eventingtesting.WithDefaultSource(), eventingtesting.WithOrderCreatedV1Event(), eventingtesting.WithSinkURL(eventingtesting.ValidSinkURLWithPath(testNamespace, sub1Name, "path1")), + eventingtesting.WithMaxInFlight(10), ) ensureK8sResourceCreated(ctx, t, givenSubscription1) 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 @@ -852,13 +784,19 @@ func Test_APIRuleReUseAfterUpdatingSink(t *testing.T) { eventingtesting.WithDefaultSource(), eventingtesting.WithOrderCreatedV1Event(), eventingtesting.WithSinkURL(eventingtesting.ValidSinkURLWithPath(testNamespace, sub2Name, "path2")), + eventingtesting.WithMaxInFlight(10), ) ensureK8sResourceCreated(ctx, t, givenSubscription2) 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 +821,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..e7ef6414a 100644 --- a/internal/controller/eventing/subscription/eventmesh/test/utils.go +++ b/internal/controller/eventing/subscription/eventmesh/test/utils.go @@ -3,10 +3,8 @@ package test import ( "context" - "crypto/tls" "fmt" "log" - "net" "net/http" "path/filepath" "strings" @@ -15,15 +13,12 @@ import ( "github.com/avast/retry-go/v3" "github.com/go-logr/zapr" - apigatewayv1beta1 "github.com/kyma-project/api-gateway/apis/gateway/v1beta1" - kymalogger "github.com/kyma-project/kyma/common/logging/logger" "github.com/stretchr/testify/require" kcorev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" 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,8 +28,8 @@ 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" + apigatewayv1beta1 "github.com/kyma-project/api-gateway/apis/gateway/v1beta1" 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/pkg/backend/cleaner" @@ -50,6 +45,7 @@ import ( "github.com/kyma-project/eventing-manager/pkg/utils" testutils "github.com/kyma-project/eventing-manager/test/utils" eventingtesting "github.com/kyma-project/eventing-manager/testing" + kymalogger "github.com/kyma-project/kyma/common/logging/logger" ) type eventMeshTestEnsemble struct { @@ -116,7 +112,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 } @@ -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)) } diff --git a/internal/controller/eventing/subscription/eventmesh/testwebhookauth/utils.go b/internal/controller/eventing/subscription/eventmesh/testwebhookauth/utils.go index e3c515056..45d6d0263 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" @@ -14,6 +12,20 @@ import ( "github.com/avast/retry-go/v3" "github.com/go-logr/zapr" apigatewayv1beta1 "github.com/kyma-project/api-gateway/apis/gateway/v1beta1" + 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/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" + "github.com/kyma-project/eventing-manager/pkg/featureflags" + "github.com/kyma-project/eventing-manager/pkg/logger" + "github.com/kyma-project/eventing-manager/pkg/utils" + testutils "github.com/kyma-project/eventing-manager/test/utils" + eventingtesting "github.com/kyma-project/eventing-manager/testing" kymalogger "github.com/kyma-project/kyma/common/logging/logger" "github.com/stretchr/testify/require" kcorev1 "k8s.io/api/core/v1" @@ -29,22 +41,6 @@ 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/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" - "github.com/kyma-project/eventing-manager/pkg/featureflags" - "github.com/kyma-project/eventing-manager/pkg/logger" - "github.com/kyma-project/eventing-manager/pkg/utils" - testutils "github.com/kyma-project/eventing-manager/test/utils" - eventingtesting "github.com/kyma-project/eventing-manager/testing" ) type eventMeshTestEnsemble struct { @@ -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 } @@ -161,41 +157,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 +179,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..58b7a1b2c 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,6 +16,7 @@ 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" @@ -24,7 +26,7 @@ import ( "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" @@ -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,41 @@ 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.validate(ctx, desiredSubscription); validationErr != nil { + if errors.Is(validationErr, sink.ErrSinkValidationFailed) { + 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 + } } - 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 + // Update subscription status accordingly. + desiredSubscription.Status.SetNotReady() + desiredSubscription.Status.ClearTypes() + desiredSubscription.Status.ClearBackend() + desiredSubscription.Status.ClearConditions() + desiredSubscription.Status.SetSubscriptionSpecValidCondition(validationErr) + if updateErr := r.updateSubscriptionStatus(ctx, desiredSubscription, log); updateErr != nil { + return kctrl.Result{}, errors.Join(validationErr, updateErr) } - // No point in reconciling as the sink is invalid, - // return latest error to requeue the reconciliation request + return kctrl.Result{}, reconcile.TerminalError(validationErr) + } else { + var noError error = nil + desiredSubscription.Status.SetSubscriptionSpecValidCondition(noError) + } + + // 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 +187,13 @@ func (r *Reconciler) Reconcile(ctx context.Context, req kctrl.Request) (kctrl.Re return kctrl.Result{}, r.syncSubscriptionStatus(ctx, desiredSubscription, nil, log) } +func (r *Reconciler) validate(ctx context.Context, subscription *eventingv1alpha2.Subscription) error { + if errList := subscription.ValidateSpec(); len(errList) > 0 { + return errors.Join(errList.ToAggregate()) + } + return r.sinkValidator.Validate(ctx, subscription) +} + func (r *Reconciler) updateSubscriptionMetrics(current, desired *eventingv1alpha2.Subscription) { for _, currentType := range current.Status.Backend.Types { found := false @@ -239,7 +256,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 +272,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 +295,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 +323,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 +342,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 +359,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..1f117d8b1 100644 --- a/internal/controller/eventing/subscription/jetstream/reconciler_integration_test.go +++ b/internal/controller/eventing/subscription/jetstream/reconciler_integration_test.go @@ -38,83 +38,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,12 +181,13 @@ 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", + "Sink validation failed: Service \"testapp\" not found", )), }, K8sEvents: []kcorev1.Event{ 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..5e9b2df07 100644 --- a/internal/controller/eventing/subscription/jetstream/reconciler_internal_unit_test.go +++ b/internal/controller/eventing/subscription/jetstream/reconciler_internal_unit_test.go @@ -4,7 +4,6 @@ import ( "context" "testing" - kymalogger "github.com/kyma-project/kyma/common/logging/logger" "github.com/pkg/errors" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -17,6 +16,7 @@ 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/pkg/backend/cleaner" @@ -27,6 +27,7 @@ import ( "github.com/kyma-project/eventing-manager/pkg/env" "github.com/kyma-project/eventing-manager/pkg/logger" eventingtesting "github.com/kyma-project/eventing-manager/testing" + kymalogger "github.com/kyma-project/kyma/common/logging/logger" ) const ( @@ -48,6 +49,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, @@ -214,7 +217,7 @@ func Test_Reconcile(t *testing.T) { testenv.Backend }, wantReconcileResult: kctrl.Result{}, - wantReconcileError: validatorErr, + wantReconcileError: reconcile.TerminalError(validatorErr), }, } @@ -234,7 +237,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) + } }) } } diff --git a/internal/controller/eventing/subscription/jetstream/test_utils_test.go b/internal/controller/eventing/subscription/jetstream/test_utils_test.go index e447e0623..e1bf154f4 100644 --- a/internal/controller/eventing/subscription/jetstream/test_utils_test.go +++ b/internal/controller/eventing/subscription/jetstream/test_utils_test.go @@ -2,16 +2,13 @@ package jetstream_test import ( "context" - "crypto/tls" "fmt" "log" - "net" "path/filepath" "testing" "time" "github.com/avast/retry-go/v3" - kymalogger "github.com/kyma-project/kyma/common/logging/logger" natsioserver "github.com/nats-io/nats-server/v2/server" "github.com/onsi/gomega" gomegatypes "github.com/onsi/gomega/types" @@ -21,16 +18,13 @@ 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" @@ -42,6 +36,7 @@ import ( "github.com/kyma-project/eventing-manager/pkg/env" "github.com/kyma-project/eventing-manager/pkg/logger" eventingtesting "github.com/kyma-project/eventing-manager/testing" + kymalogger "github.com/kyma-project/kyma/common/logging/logger" ) const ( @@ -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 @@ -219,10 +203,6 @@ func startReconciler() error { return errors.New("K8sClient cannot be nil") } - if err := StartAndWaitForWebhookServer(k8sManager, webhookInstallOptions); err != nil { - return err - } - return nil } @@ -378,8 +358,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) } @@ -546,35 +526,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/operator/eventing/controller.go b/internal/controller/operator/eventing/controller.go index 5843776ff..1a09c85e8 100644 --- a/internal/controller/operator/eventing/controller.go +++ b/internal/controller/operator/eventing/controller.go @@ -172,6 +172,8 @@ func NewReconciler( // +kubebuilder:rbac:groups="eventing.kyma-project.io",resources=subscriptions,verbs=get;list;watch;update;patch;create;delete // +kubebuilder:rbac:groups=eventing.kyma-project.io,resources=subscriptions/status,verbs=get;update;patch // +kubebuilder:rbac:groups=security.istio.io,resources=peerauthentications,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups="batch",resources=jobs,verbs=delete +// +kubebuilder:rbac:groups="batch",resources=cronjobs,verbs=delete // Generate required RBAC to emit kubernetes events in the controller. // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch @@ -472,13 +474,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 e284913ec..2e822a32d 100644 --- a/internal/controller/operator/eventing/integrationtests/controller/integration_test.go +++ b/internal/controller/operator/eventing/integrationtests/controller/integration_test.go @@ -8,8 +8,6 @@ import ( "strings" "testing" - natsv1alpha1 "github.com/kyma-project/nats-manager/api/v1alpha1" - natstestutils "github.com/kyma-project/nats-manager/testutils" "github.com/onsi/gomega" gomegatypes "github.com/onsi/gomega/types" "github.com/pkg/errors" @@ -29,6 +27,8 @@ import ( "github.com/kyma-project/eventing-manager/test/matchers" "github.com/kyma-project/eventing-manager/test/utils" testutilsintegration "github.com/kyma-project/eventing-manager/test/utils/integration" + natsv1alpha1 "github.com/kyma-project/nats-manager/api/v1alpha1" + natstestutils "github.com/kyma-project/nats-manager/testutils" ) const ( @@ -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 b7b10f286..cd746b5d8 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/internal/webhook/cleanup.go b/internal/webhook/cleanup.go new file mode 100644 index 000000000..2d8665776 --- /dev/null +++ b/internal/webhook/cleanup.go @@ -0,0 +1,75 @@ +package webhook + +import ( + "context" + + kadmissionregistrationv1 "k8s.io/api/admissionregistration/v1" + kbatchv1 "k8s.io/api/batch/v1" + kcorev1 "k8s.io/api/core/v1" + kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + kctrlclient "sigs.k8s.io/controller-runtime/pkg/client" +) + +// CleanupResources removes the mutating and validating webhook resources. +func CleanupResources(ctx context.Context, client kctrlclient.Client) []error { + const ( + namespace = "kyma-system" + service = "eventing-manager-webhook-service" + cronjob = "eventing-manager-cert-handler" + job = "eventing-manager-cert-handler" + mutatingWebhookConfiguration = "subscription-mutating-webhook-configuration" + validatingWebhookConfiguration = "subscription-validating-webhook-configuration" + ) + return []error{ + deleteService(ctx, client, namespace, service), + deleteCronJob(ctx, client, namespace, cronjob), + deleteJob(ctx, client, namespace, job), + deleteMutatingWebhookConfiguration(ctx, client, namespace, mutatingWebhookConfiguration), + deleteValidatingWebhookConfiguration(ctx, client, namespace, validatingWebhookConfiguration), + } +} + +func deleteService(ctx context.Context, client kctrlclient.Client, namespace, name string) error { + return client.Delete(ctx, &kcorev1.Service{ + ObjectMeta: kmetav1.ObjectMeta{ + Namespace: namespace, + Name: name, + }, + }) +} + +func deleteCronJob(ctx context.Context, client kctrlclient.Client, namespace, name string) error { + return client.Delete(ctx, &kbatchv1.CronJob{ + ObjectMeta: kmetav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + }) +} + +func deleteJob(ctx context.Context, client kctrlclient.Client, namespace, name string) error { + return client.Delete(ctx, &kbatchv1.Job{ + ObjectMeta: kmetav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + }) +} + +func deleteMutatingWebhookConfiguration(ctx context.Context, client kctrlclient.Client, namespace, name string) error { + return client.Delete(ctx, &kadmissionregistrationv1.MutatingWebhookConfiguration{ + ObjectMeta: kmetav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + }) +} + +func deleteValidatingWebhookConfiguration(ctx context.Context, client kctrlclient.Client, namespace, name string) error { + return client.Delete(ctx, &kadmissionregistrationv1.ValidatingWebhookConfiguration{ + ObjectMeta: kmetav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + }) +} diff --git a/pkg/backend/sink/validator.go b/pkg/backend/sink/validator.go index bf55f8b11..fcc5e6439 100644 --- a/pkg/backend/sink/validator.go +++ b/pkg/backend/sink/validator.go @@ -2,10 +2,10 @@ package sink import ( "context" + "fmt" "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" @@ -15,6 +15,8 @@ import ( "github.com/kyma-project/eventing-manager/pkg/utils" ) +var ErrSinkValidationFailed = xerrors.New("Sink validation failed") + type Validator interface { Validate(ctx context.Context, subscription *v1alpha2.Subscription) error } @@ -39,22 +41,20 @@ func NewValidator(client client.Client, recorder record.EventRecorder) Validator } func (s defaultSinkValidator) Validate(ctx context.Context, subscription *v1alpha2.Subscription) error { - _, subDomains, err := utils.GetSinkData(subscription.Spec.Sink) - if err != nil { + var ( + svcNs, svcName string + ) + + if _, subDomains, err := utils.GetSinkData(subscription.Spec.Sink); err != nil { return err + } else { + svcNs = subDomains[1] + svcName = subDomains[0] } - 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) + events.Warn(s.recorder, subscription, events.ReasonValidationFailed, "Sink does not correspond to a valid cluster local svc") + return fmt.Errorf("%w: %w", ErrSinkValidationFailed, err) } return nil diff --git a/pkg/backend/sink/validator_test.go b/pkg/backend/sink/validator_test.go index e2a7f5295..a6ae8559d 100644 --- a/pkg/backend/sink/validator_test.go +++ b/pkg/backend/sink/validator_test.go @@ -38,13 +38,13 @@ func TestSinkValidator(t *testing.T) { { 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", + wantErrString: "Sink validation failed", }, { 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", + wantErrString: "Sink validation failed", }, { name: "With a valid sink", 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..81d4cae23 100644 --- a/pkg/k8s/client.go +++ b/pkg/k8s/client.go @@ -5,9 +5,7 @@ import ( "errors" "strings" - 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" @@ -22,6 +20,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" eventingv1alpha2 "github.com/kyma-project/eventing-manager/api/eventing/v1alpha2" + natsv1alpha1 "github.com/kyma-project/nats-manager/api/v1alpha1" ) func NatsGVK() schema.GroupVersionResource { @@ -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/test/utils/integration/integration.go b/test/utils/integration/integration.go index 7bc9d0196..ba0876324 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" @@ -14,12 +12,9 @@ import ( "github.com/avast/retry-go/v3" "github.com/go-logr/zapr" - natsv1alpha1 "github.com/kyma-project/nats-manager/api/v1alpha1" - natstestutils "github.com/kyma-project/nats-manager/testutils" "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 +33,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" @@ -55,6 +49,8 @@ import ( submgrmocks "github.com/kyma-project/eventing-manager/pkg/subscriptionmanager/mocks" "github.com/kyma-project/eventing-manager/test" testutils "github.com/kyma-project/eventing-manager/test/utils" + natsv1alpha1 "github.com/kyma-project/nats-manager/api/v1alpha1" + natstestutils "github.com/kyma-project/nats-manager/testutils" ) const ( @@ -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 i := 0; i < retries; i++ { @@ -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", } }