Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 5 additions & 22 deletions pkg/apis/messaging/v1alpha1/parallel_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"knative.dev/pkg/apis"
duckv1 "knative.dev/pkg/apis/duck/v1"
duckv1alpha1 "knative.dev/pkg/apis/duck/v1alpha1"
"knative.dev/pkg/apis/v1alpha1"
"knative.dev/pkg/kmeta"
"knative.dev/pkg/webhook"
)
Expand Down Expand Up @@ -76,39 +77,21 @@ type ParallelSpec struct {

// Reply is a Reference to where the result of a case Subscriber gets sent to
// when the case does not have a Reply
//
// You can specify only the following fields of the ObjectReference:
// - Kind
// - APIVersion
// - Name
//
// The resource pointed by this ObjectReference must meet the Addressable contract
// with a reference to the Addressable duck type. If the resource does not meet this contract,
// it will be reflected in the Subscription's status.
// +optional
Reply *corev1.ObjectReference `json:"reply,omitempty"`
Reply *v1alpha1.Destination `json:"reply,omitempty"`
}

type ParallelBranch struct {
// Filter is the expression guarding the branch
Filter *SubscriberSpec `json:"filter,omitempty"`
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SubscriberSpec has a *string as URI, as opposed to our Destination, which is *apis.URL. Check if this can break...

Filter *v1alpha1.Destination `json:"filter,omitempty"`

// Subscriber receiving the event when the filter passes
Subscriber SubscriberSpec `json:"subscriber"`
Subscriber v1alpha1.Destination `json:"subscriber"`

// Reply is a Reference to where the result of Subscriber of this case gets sent to.
// If not specified, sent the result to the Parallel Reply
//
// You can specify only the following fields of the ObjectReference:
// - Kind
// - APIVersion
// - Name
//
// The resource pointed by this ObjectReference must meet the Addressable contract
// with a reference to the Addressable duck type. If the resource does not meet this contract,
// it will be reflected in the Subscription's status.
// +optional
Reply *corev1.ObjectReference `json:"reply,omitempty"`
Reply *v1alpha1.Destination `json:"reply,omitempty"`
}

// ParallelStatus represents the current state of a Parallel.
Expand Down
16 changes: 7 additions & 9 deletions pkg/apis/messaging/v1alpha1/parallel_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,11 @@ func (ps *ParallelSpec) Validate(ctx context.Context) *apis.FieldError {
}

for i, s := range ps.Branches {
if s.Filter != nil {
if err := IsValidSubscriberSpec(*s.Filter); err != nil {
errs = errs.Also(err.ViaField("filter"))
}
if err := s.Filter.ValidateDisallowDeprecated(ctx); err != nil {
errs = errs.Also(err.ViaField("filter"))
}

if e := IsValidSubscriberSpec(s.Subscriber); e != nil {
if e := s.Subscriber.ValidateDisallowDeprecated(ctx); e != nil {
errs = errs.Also(apis.ErrInvalidArrayValue(s, "branches", i))
}
}
Expand All @@ -57,10 +55,10 @@ func (ps *ParallelSpec) Validate(ctx context.Context) *apis.FieldError {
if len(ps.ChannelTemplate.Kind) == 0 {
errs = errs.Also(apis.ErrMissingField("channelTemplate.kind"))
}
if ps.Reply != nil {
if err := IsValidObjectReference(*ps.Reply); err != nil {
errs = errs.Also(err.ViaField("reply"))
}

if err := ps.Reply.Validate(ctx); err != nil {
errs = errs.Also(err.ViaField("reply"))
}

return errs
}
59 changes: 32 additions & 27 deletions pkg/apis/messaging/v1alpha1/parallel_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
eventingduck "knative.dev/eventing/pkg/apis/duck/v1alpha1"
"knative.dev/pkg/apis"
"knative.dev/pkg/apis/v1alpha1"
)

func TestParallelValidation(t *testing.T) {
Expand All @@ -46,13 +46,13 @@ func TestParallelValidation(t *testing.T) {
}

func TestParallelSpecValidation(t *testing.T) {
subscriberURI := "http://example.com"
subscriberURI := apis.HTTP("example.com")
validChannelTemplate := &eventingduck.ChannelTemplateSpec{
metav1.TypeMeta{
TypeMeta: metav1.TypeMeta{
Kind: "mykind",
APIVersion: "myapiversion",
},
&runtime.RawExtension{},
Spec: &runtime.RawExtension{},
}
tests := []struct {
name string
Expand All @@ -77,7 +77,7 @@ func TestParallelSpecValidation(t *testing.T) {
}, {
name: "missing channeltemplatespec",
ts: &ParallelSpec{
Branches: []ParallelBranch{{Subscriber: SubscriberSpec{URI: &subscriberURI}}},
Branches: []ParallelBranch{{Subscriber: v1alpha1.Destination{URI: subscriberURI}}},
},
want: func() *apis.FieldError {
fe := apis.ErrMissingField("channelTemplate")
Expand All @@ -86,8 +86,10 @@ func TestParallelSpecValidation(t *testing.T) {
}, {
name: "invalid channeltemplatespec missing APIVersion",
ts: &ParallelSpec{
ChannelTemplate: &eventingduck.ChannelTemplateSpec{metav1.TypeMeta{Kind: "mykind"}, &runtime.RawExtension{}},
Branches: []ParallelBranch{{Subscriber: SubscriberSpec{URI: &subscriberURI}}},
ChannelTemplate: &eventingduck.ChannelTemplateSpec{
TypeMeta: metav1.TypeMeta{Kind: "mykind"},
Spec: &runtime.RawExtension{}},
Branches: []ParallelBranch{{Subscriber: v1alpha1.Destination{URI: subscriberURI}}},
},
want: func() *apis.FieldError {
fe := apis.ErrMissingField("channelTemplate.apiVersion")
Expand All @@ -96,8 +98,10 @@ func TestParallelSpecValidation(t *testing.T) {
}, {
name: "invalid channeltemplatespec missing Kind",
ts: &ParallelSpec{
ChannelTemplate: &eventingduck.ChannelTemplateSpec{metav1.TypeMeta{APIVersion: "myapiversion"}, &runtime.RawExtension{}},
Branches: []ParallelBranch{{Subscriber: SubscriberSpec{URI: &subscriberURI}}},
ChannelTemplate: &eventingduck.ChannelTemplateSpec{
TypeMeta: metav1.TypeMeta{APIVersion: "myapiversion"},
Spec: &runtime.RawExtension{}},
Branches: []ParallelBranch{{Subscriber: v1alpha1.Destination{URI: subscriberURI}}},
},
want: func() *apis.FieldError {
fe := apis.ErrMissingField("channelTemplate.kind")
Expand All @@ -107,7 +111,7 @@ func TestParallelSpecValidation(t *testing.T) {
name: "valid parallel",
ts: &ParallelSpec{
ChannelTemplate: validChannelTemplate,
Branches: []ParallelBranch{{Subscriber: SubscriberSpec{URI: &subscriberURI}}},
Branches: []ParallelBranch{{Subscriber: v1alpha1.Destination{URI: subscriberURI}}},
},
want: func() *apis.FieldError {
return nil
Expand All @@ -116,7 +120,7 @@ func TestParallelSpecValidation(t *testing.T) {
name: "valid parallel with valid reply",
ts: &ParallelSpec{
ChannelTemplate: validChannelTemplate,
Branches: []ParallelBranch{{Subscriber: SubscriberSpec{URI: &subscriberURI}}},
Branches: []ParallelBranch{{Subscriber: v1alpha1.Destination{URI: subscriberURI}}},
Reply: makeValidReply("reply-channel"),
},
want: func() *apis.FieldError {
Expand All @@ -126,28 +130,29 @@ func TestParallelSpecValidation(t *testing.T) {
name: "valid parallel with invalid missing name",
ts: &ParallelSpec{
ChannelTemplate: validChannelTemplate,
Branches: []ParallelBranch{{Subscriber: SubscriberSpec{URI: &subscriberURI}}},
Reply: &corev1.ObjectReference{
APIVersion: "messaging.knative.dev/v1alpha1",
Kind: "inmemorychannel",
Branches: []ParallelBranch{{Subscriber: v1alpha1.Destination{URI: subscriberURI}}},
Reply: &v1alpha1.Destination{
DeprecatedAPIVersion: "messaging.knative.dev/v1alpha1",
DeprecatedKind: "inmemorychannel",
},
},
want: func() *apis.FieldError {
fe := apis.ErrMissingField("reply.name")
return fe
}(),
}, {
name: "valid parallel with invalid reply",
ts: &ParallelSpec{
ChannelTemplate: validChannelTemplate,
Branches: []ParallelBranch{{Subscriber: SubscriberSpec{URI: &subscriberURI}}},
Reply: makeInvalidReply("reply-channel"),
},
want: func() *apis.FieldError {
fe := apis.ErrDisallowedFields("reply.Namespace")
fe.Details = "only name, apiVersion and kind are supported fields"
return fe
}(),
// TODO check if destination should support DeprecatedNamespace and in its Ref.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment as in #2094

//}, {
// name: "valid parallel with invalid reply",
// ts: &ParallelSpec{
// ChannelTemplate: validChannelTemplate,
// Branches: []ParallelBranch{{Subscriber: v1alpha1.Destination{URI: subscriberURI}}},
// Reply: makeInvalidReply("reply-channel"),
// },
// want: func() *apis.FieldError {
// fe := apis.ErrDisallowedFields("reply.Namespace")
// fe.Details = "only name, apiVersion and kind are supported fields"
// return fe
// }(),
}}

for _, test := range tests {
Expand Down
12 changes: 2 additions & 10 deletions pkg/apis/messaging/v1alpha1/sequence_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"knative.dev/pkg/apis"
duckv1 "knative.dev/pkg/apis/duck/v1"
duckv1alpha1 "knative.dev/pkg/apis/duck/v1alpha1"
"knative.dev/pkg/apis/v1alpha1"
"knative.dev/pkg/kmeta"
"knative.dev/pkg/webhook"
)
Expand Down Expand Up @@ -76,17 +77,8 @@ type SequenceSpec struct {
ChannelTemplate *eventingduckv1alpha1.ChannelTemplateSpec `json:"channelTemplate,omitempty"`

// Reply is a Reference to where the result of the last Subscriber gets sent to.
//
// You can specify only the following fields of the ObjectReference:
// - Kind
// - APIVersion
// - Name
//
// The resource pointed by this ObjectReference must meet the Addressable contract
// with a reference to the Addressable duck type. If the resource does not meet this contract,
// it will be reflected in the Subscription's status.
// +optional
Reply *corev1.ObjectReference `json:"reply,omitempty"`
Reply *v1alpha1.Destination `json:"reply,omitempty"`
}

type SequenceChannelStatus struct {
Expand Down
8 changes: 4 additions & 4 deletions pkg/apis/messaging/v1alpha1/sequence_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ func (ps *SequenceSpec) Validate(ctx context.Context) *apis.FieldError {
if len(ps.ChannelTemplate.Kind) == 0 {
errs = errs.Also(apis.ErrMissingField("channelTemplate.kind"))
}
if ps.Reply != nil {
if err := IsValidObjectReference(*ps.Reply); err != nil {
errs = errs.Also(err.ViaField("reply"))
}

if err := ps.Reply.Validate(ctx); err != nil {
errs = errs.Also(err.ViaField("reply"))
}

return errs
}
56 changes: 30 additions & 26 deletions pkg/apis/messaging/v1alpha1/sequence_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
eventingduck "knative.dev/eventing/pkg/apis/duck/v1alpha1"
"knative.dev/pkg/apis"
"knative.dev/pkg/apis/v1alpha1"
)

func TestSequenceValidation(t *testing.T) {
Expand All @@ -45,20 +46,22 @@ func TestSequenceValidation(t *testing.T) {
})
}

func makeValidReply(channelName string) *corev1.ObjectReference {
return &corev1.ObjectReference{
APIVersion: "messaging.knative.dev/v1alpha1",
Kind: "inmemorychannel",
Name: channelName,
func makeValidReply(channelName string) *v1alpha1.Destination {
return &v1alpha1.Destination{
DeprecatedAPIVersion: "messaging.knative.dev/v1alpha1",
DeprecatedKind: "inmemorychannel",
DeprecatedName: channelName,
}
}

func makeInvalidReply(channelName string) *corev1.ObjectReference {
return &corev1.ObjectReference{
APIVersion: "messaging.knative.dev/v1alpha1",
Kind: "inmemorychannel",
Namespace: "notallowed",
Name: channelName,
func makeInvalidReply(channelName string) *v1alpha1.Destination {
return &v1alpha1.Destination{
Ref: &corev1.ObjectReference{
APIVersion: "messaging.knative.dev/v1alpha1",
Kind: "inmemorychannel",
Namespace: "notallowed",
Name: channelName,
},
}
}

Expand Down Expand Up @@ -144,27 +147,28 @@ func TestSequenceSpecValidation(t *testing.T) {
ts: &SequenceSpec{
ChannelTemplate: validChannelTemplate,
Steps: []SubscriberSpec{{URI: &subscriberURI}},
Reply: &corev1.ObjectReference{
APIVersion: "messaging.knative.dev/v1alpha1",
Kind: "inmemorychannel",
Reply: &v1alpha1.Destination{
DeprecatedAPIVersion: "messaging.knative.dev/v1alpha1",
DeprecatedKind: "inmemorychannel",
},
},
want: func() *apis.FieldError {
fe := apis.ErrMissingField("reply.name")
return fe
}(),
}, {
name: "valid sequence with invalid reply",
ts: &SequenceSpec{
ChannelTemplate: validChannelTemplate,
Steps: []SubscriberSpec{{URI: &subscriberURI}},
Reply: makeInvalidReply("reply-channel"),
},
want: func() *apis.FieldError {
fe := apis.ErrDisallowedFields("reply.Namespace")
fe.Details = "only name, apiVersion and kind are supported fields"
return fe
}(),
// TODO current destination ref allows setting the namespace, thus this fails.
//}, {
// name: "valid sequence with invalid reply",
// ts: &SequenceSpec{
// ChannelTemplate: validChannelTemplate,
// Steps: []SubscriberSpec{{URI: &subscriberURI}},
// Reply: makeInvalidReply("reply-channel"),
// },
// want: func() *apis.FieldError {
// fe := apis.ErrDisallowedFields("reply.Namespace")
// fe.Details = "only name, apiVersion and kind are supported fields"
// return fe
// }(),
}}

for _, test := range tests {
Expand Down
15 changes: 8 additions & 7 deletions pkg/apis/messaging/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading