Channel spec: iteration 1 by matzew · Pull Request #1420 · knative/eventing

matzew · 2019-06-18T10:25:26Z

Fixes #1213

Proposed Changes

adding 0.6.0 (historic) status
adding 0.7.0raw spec

Conformance Question

what do we want in the conformance test section ?

Let's mention it requires

controller
dispatcher
ServiceAccounts and ClusterRoles (for both)
presence of labels:
- "channable:true" on the channelable-manipulator ClusterRole
- "subscribable:true" on the CRD definition file
usage of CloudEvents (version 0.2?) for in/out

Some of this is already in the spec doc... but perhaps worth to explicitly add it to some conformance section?

I'd think we word these out, and have a clear, human-readable section. From there we can easily create some TCK / test-conformance-kit ?

Please have a look:
@lberk @aslom @alanconway @n3wscott @Harwayne @nachocano

matzew · 2019-06-18T10:25:59Z

+
+A channel logically receives events on its input domain and forwards them to its subscribers. Below is a specification for the generic parts of each _Channel_.
+
+A typical channel consists of a _Controller_ and a _Dispatcher_ pod.


@nachocano what about the webhook, that we have in Kafka?

matzew · 2019-06-18T10:26:20Z

ping @Abd4llA

sjwoodman · 2019-06-18T10:54:19Z

+    name: my-channel
+```
+
+Each _Channel Controller_ ensures the required tasks on the backing technology are applied. In this case a Kafka topic with the desired configuration is being created, backing all messages from the channel.


Do you want to define the behaviour of the status field to indicate that the channel has created the backing resources and is ready to receive messages?

yes please. There's this bug to address the fact that Channelable Duck is not currently very useful for deciphering the Status of the Channel CRDs
#1375

lionelvillard · 2019-06-18T13:51:30Z

+
+Different Channels handle failures in different ways:
+ * gcp-pubsub - Exponential backoff, up to five minutes. Will continue retrying forever.
+ * kafka - Immediate retry, no backoff. Will continue retrying forever.


Is this right? I don't see it retrying: https://github.com/knative/eventing/blob/master/contrib/kafka/pkg/dispatcher/dispatcher.go#L292

The error is always ignored.

This might be something we want to configure genrically as part of the "endpoint" configuration for a Channel or Source.
A related issue is reliability - for any form of reliable delivery then we will need to be able to configure QoS settings on endpoints.

Should then retry and other QoS speced for 0.8+? And tested?

vaikas · 2019-06-19T11:06:16Z

Thanks for putting this together. I focused mainly on the 0.7 and jotted down some thoughts / questions.

vaikas · 2019-06-19T10:45:20Z

+
+```
+apiVersion: messaging.knative.dev/v1alpha1
+  kind: InMemoryChannel


indent is off here.

Suggested change

kind: InMemoryChannel

kind: InMemoryChannel

vaikas · 2019-06-19T10:47:08Z

+    name: my-channel
+```
+
+Each _Channel Controller_ ensures the required tasks on the backing technology are applied. In this case a Kafka topic with the desired configuration is being created, backing all messages from the channel.


yes please. There's this bug to address the fact that Channelable Duck is not currently very useful for deciphering the Status of the Channel CRDs
#1375

vaikas · 2019-06-19T10:47:37Z

+
+Each _Channel Controller_ ensures the required tasks on the backing technology are applied. In this case a Kafka topic with the desired configuration is being created, backing all messages from the channel.
+
+#### Aggregated Channelable ClusterRole


nit: maybe:
Aggregated Channelable Manipulator ClusterRole

vaikas · 2019-06-19T10:48:32Z

+
+#### Aggregated Channelable ClusterRole
+
+Every CRD must create a corresponding ClusterRole, that will be aggregated into the `channelable-manipulator` ClusterRole. This ClusterRole must include permissions to create, read, patch, and update the CRD's custom objects. Below is an example for the `KafkaChannel`:


maybe read=> list, watch?
Also maybe:
... update the CRD's custom objects and their status.

vaikas · 2019-06-19T11:01:08Z

+
+Channels MAY NOT alter an event that goes through them. All CloudEvent attributes, including the data attribute, MUST be received at the subscriber identical to how they were received by the Channel. 
+
+Channels MUST attach a bearer token to all outgoing requests, likely in the form of a JWT. This bearer token MUST use an identity associated with the Channel, not the individual Subscription. 


How does this jive with the security requirements that @mikehelmick outlined in:
#705
That's geared towards Eventing constructs, and just want to make sure that this is compatible with that approach.

Would this be something that we rely on the (optional) mesh to handle for us instead?
Just curious who handles the auth and how the creds get distributed, etc.

vaikas · 2019-06-19T11:03:46Z

+
+TODO
+
+### Data Plane


Should we say something about (even if a TODO for the first draft) about metrics that a channel should expose (failed deliveries, malformed incoming events, etc.). as well as some sort of perf metrics?

vaikas · 2019-06-19T12:00:46Z

Looks like there's a dangling link?
I0619 08:48:10.948] docs/spec/channel/version_060.md
I0619 08:48:10.948] ERROR ../broker/README.md
I0619 08:48:10.949] Stat docs/spec/broker/README.md: no such file or directory

alanconway · 2019-06-24T17:26:37Z

+
+### The ClusterChannelProvisioner
+
+Describes an abstract configuration of a Source system which produces events or a Channel system that receives and delivers events.


Should "Source" be "Importer" or is that change still pending?
Are we folding configuration of source and channel together? If so it seems like it might be time to identify some common "endpoint" configuration type that describes either the client or server end of some protocol connection, where the direction of event flow is not assumed to be client->server (it will be for HTTP, but not for other protocols)

that's the old spec ... matching old terms ?

alanconway · 2019-06-24T17:35:28Z

+
+### The ClusterChannelProvisioner
+
+Describes an abstract configuration of a Source system which produces events or a Channel system that receives and delivers events.


Maybe clarify what the provisioner does - it's not just descriptive, it creates new channel instances does it not?

alanconway · 2019-06-24T17:43:21Z

+
+Different Channels handle failures in different ways:
+ * gcp-pubsub - Exponential backoff, up to five minutes. Will continue retrying forever.
+ * kafka - Immediate retry, no backoff. Will continue retrying forever.


This might be something we want to configure genrically as part of the "endpoint" configuration for a Channel or Source.
A related issue is reliability - for any form of reliable delivery then we will need to be able to configure QoS settings on endpoints.

lionelvillard · 2019-06-25T14:01:42Z

+
+##### Generic
+
+If a Channel receives an event queueing request and is unable to parse a valid CloudEvent, then it MUST reject the request.


Last time I checked there was no validation done (at least for the InMemoryChannel)

Should be for 0.8?

Harwayne · 2019-06-25T22:01:09Z

+
+```
+apiVersion: messaging.knative.dev/v1alpha1
+  kind: InMemoryChannel


Suggested change

kind: InMemoryChannel

kind: InMemoryChannel

Harwayne · 2019-06-25T22:01:15Z

+```
+apiVersion: messaging.knative.dev/v1alpha1
+  kind: InMemoryChannel
+  metadata:


Suggested change

metadata:

metadata:

Harwayne · 2019-06-25T22:01:21Z

+apiVersion: messaging.knative.dev/v1alpha1
+  kind: InMemoryChannel
+  metadata:
+    name: my-channel


Suggested change

name: my-channel

name: my-channel

Harwayne · 2019-06-25T22:05:07Z

+
+Every event queueing request to the Channel will come with a bearer token, likely a JWT. The bearer token MUST be validated before any other work is done on the request. The specifics of how and what to validate will be identical to Broker ingress verification, which is being [defined](https://github.com/knative/eventing/issues/705#issuecomment-496722527).
+
+The Channel MUST pass through all tracing information as CloudEvents attributes. In particular, it MUST translate any incoming OpenTracing or B3 headers to the [Distributed Tracing Extension](https://github.com/cloudevents/spec/blob/v0.2/extensions/distributed-tracing.md). The Channel SHOULD sample and write traces to the location specified in [`config-tracing`](https://github.com/cloudevents/spec/blob/v0.2/extensions/distributed-tracing.md).


config-tracing link is incorrect, although I don't know the correct one for now.

mikehelmick · 2019-06-27T16:43:54Z

+
+If a Channel receives an event queueing request and is unable to parse a valid CloudEvent, then it MUST reject the request.
+
+Every event queueing request to the Channel will come with a bearer token, likely a JWT. The bearer token MUST be validated before any other work is done on the request. The specifics of how and what to validate will be identical to Broker ingress verification, which is being [defined](https://github.com/knative/eventing/issues/705#issuecomment-496722527).


This is still a bit speculative. We'll have to think about how to craft correct policies in this area.

There is overlap in that the channel egress will need to be able to craft a new JWT with an audience of the subscriber.

we might want to mark that this will be defined for 0.8?

mikehelmick · 2019-06-27T16:44:55Z

+
+#### Output
+
+Channels MUST output CloudEvents. The output MUST be via a binding specified in the [CloudEvents specification](https://github.com/cloudevents/spec/tree/v0.2#cloudevents-documents). Every Channel MUST support sending events via Structured Content Mode HTTP Transport Binding.


when will be update channel spec to cloudevents 0.3?

aslom · 2019-07-02T14:54:29Z

Should version_070.md be renamed to 071 or 080 as changes are made or we just keep channel/spec.md with list of changes?

aslom · 2019-07-02T14:55:33Z

Should there be some kind of migration guide form 0.6? Especially about default channels?

mattmoor-sockpuppet · 2019-07-03T10:18:21Z

+
+Channels MUST output CloudEvents. The output MUST be via a binding specified in the [CloudEvents specification](https://github.com/cloudevents/spec/tree/v0.2#cloudevents-documents). Every Channel MUST support sending events via Structured Content Mode HTTP Transport Binding.
+
+Channels MAY NOT alter an event that goes through them. All CloudEvent attributes, including the data attribute, MUST be received at the subscriber identical to how they were received by the Channel. 


Suggested change

Channels MAY NOT alter an event that goes through them. All CloudEvent attributes, including the data attribute, MUST be received at the subscriber identical to how they were received by the Channel.

Channels MAY NOT alter an event that goes through them. All CloudEvent attributes, including the data attribute, MUST be received at the subscriber identical to how they were received by the Channel.

matzew · 2019-07-03T10:28:18Z

/test pull-knative-eventing-build-tests

Harwayne · 2019-07-08T16:41:13Z

/lgtm
/approve

Let's submit this as-is and continue improving it here, rather than it @matzew's personal repo. It is marked as in-progress, so should not be used by others for now.

knative-prow-robot · 2019-07-08T16:41:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Harwayne, matzew

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [Harwayne]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>

knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 18, 2019

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jun 18, 2019

knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 18, 2019

mattmoor-sockpuppet reviewed Jun 18, 2019

View reviewed changes

knative-prow-robot requested review from Harwayne and n3wscott June 18, 2019 10:25

matzew commented Jun 18, 2019

View reviewed changes

sjwoodman reviewed Jun 18, 2019

View reviewed changes

lionelvillard reviewed Jun 18, 2019

View reviewed changes

mattmoor-sockpuppet reviewed Jun 19, 2019

View reviewed changes

vaikas reviewed Jun 19, 2019

View reviewed changes

alanconway reviewed Jun 24, 2019

View reviewed changes

mattmoor-sockpuppet reviewed Jun 25, 2019

View reviewed changes

lionelvillard reviewed Jun 25, 2019

View reviewed changes

Harwayne reviewed Jun 25, 2019

View reviewed changes

mikehelmick reviewed Jun 27, 2019

View reviewed changes

mattmoor-sockpuppet reviewed Jul 3, 2019

View reviewed changes

Adding channel spec/draft, based on feedback and comments

e5f6b56

matzew force-pushed the channel_spec branch from 715aecf to e5f6b56 Compare July 3, 2019 13:27

matzew changed the title ~~WIP: Channel spec~~ Channel spec: iteration 1 Jul 3, 2019

knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 3, 2019

knative-prow-robot assigned Harwayne Jul 8, 2019

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 8, 2019

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 8, 2019

knative-prow-robot merged commit 4299512 into knative:master Jul 8, 2019

lberk pushed a commit to lberk/eventing that referenced this pull request Jul 15, 2019

Adding channel spec/draft, based on feedback and comments (knative#1420)

7ca5692

lberk mentioned this pull request Jul 26, 2019

Channel spec revisions iterations #1572

Merged

lionelvillard mentioned this pull request Dec 12, 2019

adding lionelvillard to approvers #2314

Merged

matzew added a commit to matzew/eventing that referenced this pull request Sep 28, 2021

Using 1.18 product brnach (knative#1420)

56a556b

Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>


		A channel logically receives events on its input domain and forwards them to its subscribers. Below is a specification for the generic parts of each _Channel_.

		A typical channel consists of a _Controller_ and a _Dispatcher_ pod.


		Each _Channel Controller_ ensures the required tasks on the backing technology are applied. In this case a Kafka topic with the desired configuration is being created, backing all messages from the channel.

		#### Aggregated Channelable ClusterRole


		#### Aggregated Channelable ClusterRole

		Every CRD must create a corresponding ClusterRole, that will be aggregated into the `channelable-manipulator` ClusterRole. This ClusterRole must include permissions to create, read, patch, and update the CRD's custom objects. Below is an example for the `KafkaChannel`:


		Channels MAY NOT alter an event that goes through them. All CloudEvent attributes, including the data attribute, MUST be received at the subscriber identical to how they were received by the Channel.

		Channels MUST attach a bearer token to all outgoing requests, likely in the form of a JWT. This bearer token MUST use an identity associated with the Channel, not the individual Subscription. No newline at end of file


		### The ClusterChannelProvisioner

		Describes an abstract configuration of a Source system which produces events or a Channel system that receives and delivers events.


		##### Generic

		If a Channel receives an event queueing request and is unable to parse a valid CloudEvent, then it MUST reject the request.


		Every event queueing request to the Channel will come with a bearer token, likely a JWT. The bearer token MUST be validated before any other work is done on the request. The specifics of how and what to validate will be identical to Broker ingress verification, which is being [defined](https://github.com/knative/eventing/issues/705#issuecomment-496722527).

		The Channel MUST pass through all tracing information as CloudEvents attributes. In particular, it MUST translate any incoming OpenTracing or B3 headers to the [Distributed Tracing Extension](https://github.com/cloudevents/spec/blob/v0.2/extensions/distributed-tracing.md). The Channel SHOULD sample and write traces to the location specified in [`config-tracing`](https://github.com/cloudevents/spec/blob/v0.2/extensions/distributed-tracing.md).


		#### Output

		Channels MUST output CloudEvents. The output MUST be via a binding specified in the [CloudEvents specification](https://github.com/cloudevents/spec/tree/v0.2#cloudevents-documents). Every Channel MUST support sending events via Structured Content Mode HTTP Transport Binding.

Conversation

matzew commented Jun 18, 2019

Proposed Changes

Conformance Question

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matzew commented Jun 18, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vaikas commented Jun 19, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vaikas commented Jun 19, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aslom commented Jul 2, 2019

Uh oh!

aslom commented Jul 2, 2019

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

matzew commented Jul 3, 2019