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.

Sign up for GitHub

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

Jump to bottom

ut for trigger/broker updates check immutable #4843

Merged
merged 3 commits into from
Feb 10, 2021
Merged

ut for trigger/broker updates check immutable #4843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e363f4
ut for trigger/broker updates check immutable
vaikas Feb 5, 2021
f182539
ut are actually kind of cool... fix the brokenness
vaikas Feb 5, 2021
d11de38
fix conformance test
vaikas Feb 9, 2021
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
82 changes: 82 additions & 0 deletions pkg/apis/eventing/v1/broker_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,88 @@ func TestValidate(t *testing.T) {
}
}

func TestValidateUpdate(t *testing.T) {
tests := []struct {
name string
b Broker
bNew Broker
want *apis.FieldError
}{{
name: "valid config change",
b: Broker{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"},
},
Spec: BrokerSpec{
Config: &duckv1.KReference{
Namespace: "namespace",
Name: "name",
Kind: "kind",
APIVersion: "apiversion",
},
},
},
bNew: Broker{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"},
},
Spec: BrokerSpec{
Config: &duckv1.KReference{
Namespace: "namespace",
Name: "name2",
Kind: "kind",
APIVersion: "apiversion",
},
},
},
}, {
name: "invalid config change, broker.class",
b: Broker{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"},
},
Spec: BrokerSpec{
Config: &duckv1.KReference{
Namespace: "namespace",
Name: "name",
Kind: "kind",
APIVersion: "apiversion",
},
},
},
bNew: Broker{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{"eventing.knative.dev/broker.class": "SomeOtherBrokerClass"},
},
Spec: BrokerSpec{
Config: &duckv1.KReference{
Namespace: "namespace",
Name: "name",
Kind: "kind",
APIVersion: "apiversion",
},
},
},
want: &apis.FieldError{
Message: "Immutable fields changed (-old +new)",
Paths: []string{"annotations"},
Details: `{string}:
-: "MTChannelBasedBroker"
+: "SomeOtherBrokerClass"
`,
},
}}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
ctx := apis.WithinUpdate(context.Background(), &test.b)
got := test.bNew.Validate(ctx)
if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" {
t.Error("Broker.Validate (-want, +got) =", diff)
}
})
}
}

func TestValidSpec(t *testing.T) {
bop := eventingduckv1.BackoffPolicyExponential
tests := []struct {
Expand Down
47 changes: 47 additions & 0 deletions pkg/apis/eventing/v1/trigger_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,53 @@ func TestTriggerValidation(t *testing.T) {
}
}

func TestTriggerUpdateValidation(t *testing.T) {
tests := []struct {
name string
t *Trigger
tNew *Trigger
want *apis.FieldError
}{{
name: "invalid update, broker changed",
t: &Trigger{
ObjectMeta: v1.ObjectMeta{
Namespace: "test-ns",
},
Spec: TriggerSpec{
Broker: "test_broker",
Filter: validEmptyFilter,
Subscriber: validSubscriber,
}},
tNew: &Trigger{
ObjectMeta: v1.ObjectMeta{
Namespace: "test-ns",
},
Spec: TriggerSpec{
Broker: "anotherBroker",
Filter: validEmptyFilter,
Subscriber: validSubscriber,
}},
want: &apis.FieldError{
Message: "Immutable fields changed (-old +new)",
Paths: []string{"spec", "broker"},
Details: `{string}:
-: "test_broker"
+: "anotherBroker"
`,
},
}}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
ctx := apis.WithinUpdate(context.Background(), test.t)
got := test.tNew.Validate(ctx)
if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" {
t.Error("Trigger.Validate (-want, +got) =", diff)
}
})
}
}

func TestTriggerSpecValidation(t *testing.T) {
invalidString := "invalid time"
tests := []struct {
Expand Down
82 changes: 82 additions & 0 deletions pkg/apis/eventing/v1beta1/broker_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,88 @@ func TestValidate(t *testing.T) {
}
}

func TestValidateUpdate(t *testing.T) {
tests := []struct {
name string
b Broker
bNew Broker
want *apis.FieldError
}{{
name: "valid config change",
b: Broker{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"},
},
Spec: BrokerSpec{
Config: &duckv1.KReference{
Namespace: "namespace",
Name: "name",
Kind: "kind",
APIVersion: "apiversion",
},
},
},
bNew: Broker{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"},
},
Spec: BrokerSpec{
Config: &duckv1.KReference{
Namespace: "namespace",
Name: "name2",
Kind: "kind",
APIVersion: "apiversion",
},
},
},
}, {
name: "invalid config change, broker.class",
b: Broker{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"},
},
Spec: BrokerSpec{
Config: &duckv1.KReference{
Namespace: "namespace",
Name: "name",
Kind: "kind",
APIVersion: "apiversion",
},
},
},
bNew: Broker{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{"eventing.knative.dev/broker.class": "SomeOtherBrokerClass"},
},
Spec: BrokerSpec{
Config: &duckv1.KReference{
Namespace: "namespace",
Name: "name",
Kind: "kind",
APIVersion: "apiversion",
},
},
},
want: &apis.FieldError{
Message: "Immutable fields changed (-old +new)",
Paths: []string{"annotations"},
Details: `{string}:
-: "MTChannelBasedBroker"
+: "SomeOtherBrokerClass"
`,
},
}}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
ctx := apis.WithinUpdate(context.Background(), &test.b)
got := test.bNew.Validate(ctx)
if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" {
t.Error("Broker.Validate (-want, +got) =", diff)
}
})
}
}

func TestValidSpec(t *testing.T) {
bop := eventingduckv1beta1.BackoffPolicyExponential
tests := []struct {
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/eventing/v1beta1/trigger_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ func (t *Trigger) Validate(ctx context.Context) *apis.FieldError {
errs := t.Spec.Validate(ctx).ViaField("spec")
errs = t.validateAnnotation(errs, DependencyAnnotation, t.validateDependencyAnnotation)
errs = t.validateAnnotation(errs, InjectionAnnotation, t.validateInjectionAnnotation)
if apis.IsInUpdate(ctx) {
original := apis.GetBaseline(ctx).(*Trigger)
errs = errs.Also(t.CheckImmutableFields(ctx, original))
}
return errs
}

Expand Down
47 changes: 47 additions & 0 deletions pkg/apis/eventing/v1beta1/trigger_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,53 @@ func TestTriggerValidation(t *testing.T) {
}
}

func TestTriggerUpdateValidation(t *testing.T) {
tests := []struct {
name string
t *Trigger
tNew *Trigger
want *apis.FieldError
}{{
name: "invalid update, broker changed",
t: &Trigger{
ObjectMeta: v1.ObjectMeta{
Namespace: "test-ns",
},
Spec: TriggerSpec{
Broker: "test_broker",
Filter: validEmptyFilter,
Subscriber: validSubscriber,
}},
tNew: &Trigger{
ObjectMeta: v1.ObjectMeta{
Namespace: "test-ns",
},
Spec: TriggerSpec{
Broker: "anotherBroker",
Filter: validEmptyFilter,
Subscriber: validSubscriber,
}},
want: &apis.FieldError{
Message: "Immutable fields changed (-old +new)",
Paths: []string{"spec", "broker"},
Details: `{string}:
-: "test_broker"
+: "anotherBroker"
`,
},
}}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
ctx := apis.WithinUpdate(context.Background(), test.t)
got := test.tNew.Validate(ctx)
if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" {
t.Error("Trigger.Validate (-want, +got) =", diff)
}
})
}
}

func TestTriggerSpecValidation(t *testing.T) {
invalidString := "invalid time"

Expand Down
29 changes: 7 additions & 22 deletions test/conformance/helpers/broker_control_plane_test_helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package helpers

import (
"context"
"strings"
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -71,7 +72,7 @@ func BrokerV1Beta1ControlPlaneTest(
})

t.Run("Ready Trigger V1Beta1 (no Broker) set Broker and includes status.subscriber Uri", func(t *testing.T) {
triggerV1Beta1ReadyAfterBrokerIncludesSubURI(t, triggerNoBroker, brokerName, client)
triggerV1Beta1CanNotUpdateBroker(t, triggerNoBroker, brokerName, client)
})

t.Run("Ready Trigger V1Beta1 includes status.subscriber Uri", func(t *testing.T) {
Expand Down Expand Up @@ -120,7 +121,7 @@ func triggerV1Beta1ReadyBrokerReadyHelper(triggerName, brokerName string, client
client.WaitForResourceReadyOrFail(trigger.Name, testlib.TriggerTypeMeta)
}

func triggerV1Beta1ReadyAfterBrokerIncludesSubURI(t *testing.T, triggerName, brokerName string, client *testlib.Client) {
func triggerV1Beta1CanNotUpdateBroker(t *testing.T, triggerName, brokerName string, client *testlib.Client) {
err := reconciler.RetryUpdateConflicts(func(attempts int) (err error) {
tr, err := client.Eventing.EventingV1beta1().Triggers(client.Namespace).Get(context.Background(), triggerName, metav1.GetOptions{})
if err != nil {
Expand All @@ -130,28 +131,12 @@ func triggerV1Beta1ReadyAfterBrokerIncludesSubURI(t *testing.T, triggerName, bro
_, e := client.Eventing.EventingV1beta1().Triggers(client.Namespace).Update(context.Background(), tr, metav1.UpdateOptions{})
return e
})
if err != nil {
t.Fatalf("Error: Could not update trigger %s: %v", triggerName, err)
if err == nil {
t.Fatalf("Error: Was able to update the trigger.Spec.Broker %s", triggerName)
}

client.WaitForResourceReadyOrFail(triggerName, testlib.TriggerTypeMeta)

var tr *eventingv1beta1.Trigger
triggers := client.Eventing.EventingV1beta1().Triggers(client.Namespace)
err = client.RetryWebhookErrors(func(attempts int) (err error) {
var e error
client.T.Logf("Getting v1beta1 trigger %s", triggerName)
tr, e = triggers.Get(context.Background(), triggerName, metav1.GetOptions{})
if e != nil {
client.T.Logf("Failed to get trigger %q: %v", triggerName, e)
}
return err
})
if err != nil {
t.Fatalf("Error: Could not get trigger %s: %v", triggerName, err)
}
if tr.Status.SubscriberURI == nil {
t.Fatalf("Error: trigger.Status.SubscriberURI is nil but Broker Addressable & Ready")
if !strings.Contains(err.Error(), "Immutable fields changed (-old +new): broker, spec") {
t.Fatalf("Unexpected failure to update trigger, expected Immutable fields changed (-old +new): broker, spec But got: %v", err)
}
}

Expand Down