Skip to content
Closed
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.

8 changes: 4 additions & 4 deletions pkg/apis/messaging/v1alpha1/parallel_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ func TestParallelValidation(t *testing.T) {
func TestParallelSpecValidation(t *testing.T) {
subscriberURI := "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 Down Expand Up @@ -117,7 +117,7 @@ func TestParallelSpecValidation(t *testing.T) {
ts: &ParallelSpec{
ChannelTemplate: validChannelTemplate,
Branches: []ParallelBranch{{Subscriber: SubscriberSpec{URI: &subscriberURI}}},
Reply: makeValidReply("reply-channel"),
Reply: makeValidReply("reply-channel").GetRef(),
},
want: func() *apis.FieldError {
return nil
Expand All @@ -141,7 +141,7 @@ func TestParallelSpecValidation(t *testing.T) {
ts: &ParallelSpec{
ChannelTemplate: validChannelTemplate,
Branches: []ParallelBranch{{Subscriber: SubscriberSpec{URI: &subscriberURI}}},
Reply: makeInvalidReply("reply-channel"),
Reply: makeInvalidReply("reply-channel").GetRef(),
},
want: func() *apis.FieldError {
fe := apis.ErrDisallowedFields("reply.Namespace")
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{
Comment thread
vaikas marked this conversation as resolved.
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.
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.

/hold

we have a DeprecatedNamespace in Destination, is that correct? I thought we weren't supporting namespaces in ObjectRef..
If that is the case, the Ref in Destination should actually validate that... as done here

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.

/hold cancel, it seems that we will let this through

//}, {
// 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
5 changes: 3 additions & 2 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.

2 changes: 1 addition & 1 deletion pkg/reconciler/sequence/resources/subscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func NewSubscription(stepNumber int, p *v1alpha1.Sequence) *v1alpha1.Subscriptio
},
}
} else if p.Spec.Reply != nil {
r.Spec.Reply = &v1alpha1.ReplyStrategy{Channel: p.Spec.Reply}
r.Spec.Reply = &v1alpha1.ReplyStrategy{Channel: p.Spec.Reply.GetRef()}
}
return r
}
5 changes: 4 additions & 1 deletion pkg/reconciler/testing/sequence.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
eventingduckv1alpha1 "knative.dev/eventing/pkg/apis/duck/v1alpha1"
"knative.dev/eventing/pkg/apis/messaging/v1alpha1"
pkgv1alpha1 "knative.dev/pkg/apis/v1alpha1"
)

// SequenceOption enables further configuration of a Sequence.
Expand Down Expand Up @@ -68,7 +69,9 @@ func WithSequenceSteps(steps []v1alpha1.SubscriberSpec) SequenceOption {

func WithSequenceReply(reply *corev1.ObjectReference) SequenceOption {
return func(p *v1alpha1.Sequence) {
p.Spec.Reply = reply
p.Spec.Reply = &pkgv1alpha1.Destination{
Ref: reply,
}
}
}

Expand Down