feat: remove the mutating and validating webhooks for v1alpha2 Subscr…

…iption (#549)

* feat: remove the mutating and validating webhooks for v1alpha2 Subscription.

* Fix lint issues

* Revert image update

* Fix tests

* Refactor subscription validation

* Fix lint issues

* Refactor subscription validation

* Refactoring

* Add subscription validation tests

* Test subscription validation in the reconciler using mocks

* Fix lint issues

* Refactoring

* Refactor type matching validation

* Refactoring validation

* Undo doc changes

* Update validation tests

* Add defaulting tests

* Remove not needed setup

* Update defaulting expr

* Fix lint issues

* Add tests for webhook resources cleanup

* Add tests for subscription spec valid conditions

* Restrict delete job and cronjobs only to the target resources

* Fix lint issues

* Preserve the old subscription status in case of the EventMesh backend

* Fix generated config

* Remove webhook cleanup logic

.golangci.yaml

@@ -74,6 +74,8 @@ linters-settings:
         alias: kappsv1
       - pkg: k8s.io/api/rbac/v1
         alias: krbacv1
+      - pkg: k8s.io/api/batch/v1
+        alias: kbatchv1
       - pkg: k8s.io/apimachinery/pkg/runtime/schema
         alias: kschema
       - pkg: k8s.io/apimachinery/pkg/labels
@@ -194,6 +196,8 @@ linters-settings:
         alias: pkgerrors
       - pkg: github.com/kyma-project/eventing-manager/testing/eventmeshsub
         alias: eventmeshsubmatchers
+      - pkg: github.com/kyma-project/eventing-manager/internal/controller/eventing/subscription/validator/mocks
+        alias: subscriptionvalidatormocks
 
   ireturn:
     allow:

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

api/eventing/v1alpha2/condition.go

@@ -10,10 +10,11 @@ import (
 type ConditionType string
 
 const (
-	ConditionSubscribed         ConditionType = "Subscribed"
-	ConditionSubscriptionActive ConditionType = "Subscription active"
-	ConditionAPIRuleStatus      ConditionType = "APIRule status"
-	ConditionWebhookCallStatus  ConditionType = "Webhook call status"
+	ConditionSubscribed            ConditionType = "Subscribed"
+	ConditionSubscriptionActive    ConditionType = "Subscription active"
+	ConditionSubscriptionSpecValid ConditionType = "Subscription spec valid"
+	ConditionAPIRuleStatus         ConditionType = "APIRule status"
+	ConditionWebhookCallStatus     ConditionType = "Webhook call status"
 
 	ConditionPublisherProxyReady ConditionType = "Publisher Proxy Ready"
 	ConditionControllerReady     ConditionType = "Subscription Controller Ready"
@@ -37,6 +38,9 @@ type Condition struct {
 type ConditionReason string
 
 const (
+	ConditionReasonSubscriptionSpecHasValidationErrors   ConditionReason = "Subscription spec has validation errors"
+	ConditionReasonSubscriptionSpecHasNoValidationErrors ConditionReason = "Subscription spec has no validation errors"
+
 	// JetStream Conditions.
 	ConditionReasonNATSSubscriptionActive    ConditionReason = "NATS Subscription active"
 	ConditionReasonNATSSubscriptionNotActive ConditionReason = "NATS Subscription not active"
@@ -133,6 +137,11 @@ func MakeSubscriptionConditions() []Condition {
 			LastTransitionTime: kmetav1.Now(),
 			Status:             kcorev1.ConditionUnknown,
 		},
+		{
+			Type:               ConditionSubscriptionSpecValid,
+			LastTransitionTime: kmetav1.Now(),
+			Status:             kcorev1.ConditionUnknown,
+		},
 	}
 	return conditions
 }
@@ -241,7 +250,7 @@ func ConditionsEquals(existing, expected []Condition) bool {
 	return true
 }
 
-// ConditionsEquals checks if two conditions are equal.
+// ConditionEquals checks if two conditions are equal.
 func ConditionEquals(existing, expected Condition) bool {
 	isTypeEqual := existing.Type == expected.Type
 	isStatusEqual := existing.Status == expected.Status
@@ -259,29 +268,112 @@ func CreateMessageForConditionReasonSubscriptionCreated(eventMeshName string) st
 	return fmt.Sprintf("EventMesh subscription name is: %s", eventMeshName)
 }
 
-// GetSubscriptionActiveCondition updates the ConditionSubscriptionActive condition based on the given error value.
-func GetSubscriptionActiveCondition(sub *Subscription, err error) []Condition {
-	subscriptionActiveCondition := Condition{
+// makeSubscriptionActiveCondition returns a new active condition based on the given error.
+func makeSubscriptionActiveCondition(err error) Condition {
+	var (
+		status  kcorev1.ConditionStatus
+		reason  ConditionReason
+		message string
+	)
+	if err != nil {
+		status = kcorev1.ConditionFalse
+		reason = ConditionReasonNATSSubscriptionNotActive
+		message = err.Error()
+	} else {
+		status = kcorev1.ConditionTrue
+		reason = ConditionReasonNATSSubscriptionActive
+	}
+	return Condition{
 		Type:               ConditionSubscriptionActive,
+		Status:             status,
 		LastTransitionTime: kmetav1.Now(),
+		Reason:             reason,
+		Message:            message,
 	}
-	if err == nil {
-		subscriptionActiveCondition.Status = kcorev1.ConditionTrue
-		subscriptionActiveCondition.Reason = ConditionReasonNATSSubscriptionActive
+}
+
+// makeSubscriptionSpecValidCondition returns a new validation condition based on the given error.
+func makeSubscriptionSpecValidCondition(err error) Condition {
+	var (
+		status  kcorev1.ConditionStatus
+		reason  ConditionReason
+		message string
+	)
+	if err != nil {
+		status = kcorev1.ConditionFalse
+		reason = ConditionReasonSubscriptionSpecHasValidationErrors
+		message = err.Error()
 	} else {
-		subscriptionActiveCondition.Message = err.Error()
-		subscriptionActiveCondition.Reason = ConditionReasonNATSSubscriptionNotActive
-		subscriptionActiveCondition.Status = kcorev1.ConditionFalse
+		status = kcorev1.ConditionTrue
+		reason = ConditionReasonSubscriptionSpecHasNoValidationErrors
+	}
+	return Condition{
+		Type:               ConditionSubscriptionSpecValid,
+		Status:             status,
+		LastTransitionTime: kmetav1.Now(),
+		Reason:             reason,
+		Message:            message,
 	}
-	for _, activeCond := range sub.Status.Conditions {
-		if activeCond.Type == ConditionSubscriptionActive {
-			if subscriptionActiveCondition.Status == activeCond.Status &&
-				subscriptionActiveCondition.Reason == activeCond.Reason &&
-				subscriptionActiveCondition.Message == activeCond.Message {
-				return []Condition{activeCond}
-			}
+}
+
+// setCondition sets the given condition in the Subscription status.
+// If the condition is already present, it will be updated.
+// If the condition is not present, it will be added.
+func (s *SubscriptionStatus) setCondition(condition Condition) {
+	isFound, isSet := false, false
+	conditions := make([]Condition, 0, len(s.Conditions))
+	for _, cond := range s.Conditions {
+		if cond.Type != condition.Type {
+			conditions = append(conditions, cond)
+			continue
+		}
+		isFound = true
+		if !ConditionEquals(cond, condition) {
+			isSet = true
+			conditions = append(conditions, condition)
 		}
 	}
+	if !isFound {
+		isSet = true
+		conditions = append(conditions, condition)
+	}
+	if isSet {
+		s.Conditions = conditions
+	}
+}
+
+// SetSubscriptionActiveCondition sets a subscription active condition based on the given error.
+// If the given error is nil, the status will have the Subscription active condition set to true,
+// otherwise it will have the Subscription active condition set to false and the error as the message.
+func SetSubscriptionActiveCondition(status *SubscriptionStatus, err error) {
+	condition := makeSubscriptionActiveCondition(err)
+	status.setCondition(condition)
+}
+
+// SetSubscriptionSpecValidCondition sets a subscription spec valid condition based on the given error.
+// If the given error is nil, the status will have the Subscription spec valid condition set to true,
+// otherwise it will have the Subscription spec valid condition set to false and the error as the message.
+func (s *SubscriptionStatus) SetSubscriptionSpecValidCondition(err error) {
+	condition := makeSubscriptionSpecValidCondition(err)
+	s.setCondition(condition)
+}
+
+// SetNotReady sets the Subscription status to not ready.
+func (s *SubscriptionStatus) SetNotReady() {
+	s.Ready = false
+}
+
+// ClearConditions sets the Subscription conditions to an empty list.
+func (s *SubscriptionStatus) ClearConditions() {
+	s.Conditions = []Condition{}
+}
+
+// ClearBackend sets the Subscription Backend to an empty struct.
+func (s *SubscriptionStatus) ClearBackend() {
+	s.Backend = Backend{}
+}
 
-	return []Condition{subscriptionActiveCondition}
+// ClearTypes sets the Subscription Types to an empty list.
+func (s *SubscriptionStatus) ClearTypes() {
+	s.Types = []EventType{}
 }

api/eventing/v1alpha2/condition_test.go

@@ -0,0 +1,62 @@
+package v1alpha2
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	kcorev1 "k8s.io/api/core/v1"
+	kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func Test_makeSubscriptionSpecValidCondition(t *testing.T) {
+	t.Parallel()
+
+	var (
+		// subscription spec valid condition
+		subscriptionSpecValidTrueCondition = Condition{
+			Type:               ConditionSubscriptionSpecValid,
+			Status:             kcorev1.ConditionTrue,
+			LastTransitionTime: kmetav1.Now(),
+			Reason:             ConditionReasonSubscriptionSpecHasNoValidationErrors,
+			Message:            "",
+		}
+
+		// subscription spec invalid condition
+		subscriptionSpecValidFalseCondition = Condition{
+			Type:               ConditionSubscriptionSpecValid,
+			Status:             kcorev1.ConditionFalse,
+			LastTransitionTime: kmetav1.Now(),
+			Reason:             ConditionReasonSubscriptionSpecHasValidationErrors,
+			Message:            "some error",
+		}
+	)
+
+	tests := []struct {
+		name                               string
+		givenError                         error
+		wantSubscriptionSpecValidCondition Condition
+	}{
+		{
+			name:                               "no error",
+			givenError:                         nil,
+			wantSubscriptionSpecValidCondition: subscriptionSpecValidTrueCondition,
+		},
+		{
+			name:                               "error",
+			givenError:                         errors.New("some error"), //nolint: goerr113 // used for testing only
+			wantSubscriptionSpecValidCondition: subscriptionSpecValidFalseCondition,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			// when
+			gotCondition := makeSubscriptionSpecValidCondition(test.givenError)
+
+			// then
+			require.True(t, ConditionEquals(gotCondition, test.wantSubscriptionSpecValidCondition))
+		})
+	}
+}