Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove the mutating and validating webhooks for v1alpha2 Subscription #549

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
f436e77
feat: remove the mutating and validating webhooks for v1alpha2 Subscr…
marcobebway Apr 11, 2024
c3c3011
Fix lint issues
marcobebway Apr 11, 2024
4840e30
Revert image update
marcobebway Apr 12, 2024
b0a8a8e
Fix tests
marcobebway Apr 12, 2024
d7a0453
Refactor subscription validation
marcobebway Apr 15, 2024
cdc1324
Fix lint issues
marcobebway Apr 15, 2024
cc8593b
Refactor subscription validation
marcobebway Apr 15, 2024
ceef046
Refactoring
marcobebway Apr 15, 2024
a28c332
Add subscription validation tests
marcobebway Apr 15, 2024
c9882e8
Test subscription validation in the reconciler using mocks
marcobebway Apr 15, 2024
559fc92
Fix lint issues
marcobebway Apr 15, 2024
6d5063c
Refactoring
marcobebway Apr 16, 2024
46f4890
Refactor type matching validation
marcobebway Apr 16, 2024
618c360
Refactoring validation
marcobebway Apr 16, 2024
85ff6ae
Undo doc changes
marcobebway Apr 16, 2024
788e374
Update validation tests
marcobebway Apr 16, 2024
b6c2bde
Add defaulting tests
marcobebway Apr 16, 2024
d4fc940
Remove not needed setup
marcobebway Apr 16, 2024
fef264d
Update defaulting expr
marcobebway Apr 16, 2024
de5395f
Fix lint issues
marcobebway Apr 16, 2024
7b3799a
Add tests for webhook resources cleanup
marcobebway Apr 17, 2024
4491b69
Add tests for subscription spec valid conditions
marcobebway Apr 17, 2024
f7a5a0d
Restrict delete job and cronjobs only to the target resources
marcobebway Apr 17, 2024
c0b96ba
Fix lint issues
marcobebway Apr 17, 2024
13ee8be
Preserve the old subscription status in case of the EventMesh backend
marcobebway Apr 17, 2024
ed204a6
Fix generated config
marcobebway Apr 17, 2024
cb0c887
Remove webhook cleanup logic
marcobebway Apr 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
136 changes: 114 additions & 22 deletions api/eventing/v1alpha2/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
mfaizanse marked this conversation as resolved.
Show resolved Hide resolved

ConditionPublisherProxyReady ConditionType = "Publisher Proxy Ready"
ConditionControllerReady ConditionType = "Subscription Controller Ready"
Expand All @@ -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"
Expand Down Expand Up @@ -133,6 +137,11 @@ func MakeSubscriptionConditions() []Condition {
LastTransitionTime: kmetav1.Now(),
Status: kcorev1.ConditionUnknown,
},
{
Type: ConditionSubscriptionSpecValid,
LastTransitionTime: kmetav1.Now(),
Status: kcorev1.ConditionUnknown,
},
}
return conditions
}
Expand Down Expand Up @@ -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
Expand All @@ -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) {
mfaizanse marked this conversation as resolved.
Show resolved Hide resolved
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{}
}
62 changes: 62 additions & 0 deletions api/eventing/v1alpha2/condition_test.go
Original file line number Diff line number Diff line change
@@ -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))
})
}
}
Loading
Loading