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

Registry Design #1015

Merged
merged 27 commits into from
May 6, 2019
Merged
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
9c8f86a
Registry proposal
nachocano Apr 1, 2019
5f7e91a
Updates after comments
nachocano Apr 2, 2019
a5e4762
adding broker ingress policies for reference
nachocano Apr 2, 2019
269205e
More clarifications. Restructuring the document a bit.
nachocano Apr 8, 2019
928fd2e
started a FAQ list at the bottom, for further clarifications.
nachocano Apr 9, 2019
9292ccf
Merge remote-tracking branch 'upstream/master' into registry-design
nachocano Apr 9, 2019
d4e5a7d
updates after Ville's comments
nachocano Apr 9, 2019
0a4b49c
addressing some of Doug's comments.
nachocano Apr 9, 2019
6155ebb
typos
nachocano Apr 9, 2019
90dfa07
removing the custom extension from, as we will be able to use
nachocano Apr 18, 2019
0fc13dd
Merge remote-tracking branch 'upstream/master' into registry-design
nachocano Apr 18, 2019
6b852ac
typo
nachocano Apr 18, 2019
71432c0
Merge remote-tracking branch 'upstream/master' into registry-design
nachocano Apr 23, 2019
78ee9c8
some updates
nachocano Apr 24, 2019
dd69ba6
adding Grant's example
nachocano Apr 24, 2019
61c46db
cosmetics
nachocano Apr 24, 2019
ce49a62
cosmetic
nachocano Apr 24, 2019
b3a07fb
updating types
nachocano Apr 24, 2019
4beac7d
typo
nachocano Apr 24, 2019
a94b623
removing broker ingress policies
nachocano Apr 26, 2019
64f7709
Merge remote-tracking branch 'upstream/master' into registry-design
nachocano Apr 26, 2019
ca8c221
Merge remote-tracking branch 'upstream/master' into registry-design
nachocano Apr 26, 2019
46a6b87
changing CR to CO
nachocano Apr 26, 2019
302ba24
Merge remote-tracking branch 'upstream/master' into registry-design
nachocano May 3, 2019
83b8820
updating source values
nachocano May 3, 2019
94a1891
adding the protocol to match the examples in CloudEvents spec
nachocano May 3, 2019
b311523
Merge remote-tracking branch 'upstream/master' into registry-design
nachocano May 6, 2019
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
68 changes: 64 additions & 4 deletions docs/registry/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ This is also known as the `discoverability` use case, and is the main focus of t

Design an **initial** version of the **Registry** for the **MVP** that can support discoverability of
the different event types that can be consumed from the eventing mesh. For details on the different user stories
that this proposal enables, please refer to the
that this proposal touches, please refer to the
[User stories and personas for Knative eventing](https://docs.google.com/document/d/15uhyqQvaomxRX2u8s0i6CNhA86BQTNztkdsLUnPmvv4/edit?usp=sharing) document.

#### Out of scope
Expand All @@ -21,7 +21,8 @@ takes care of setting up Secrets for connecting to a GitHub repo, and so on.
- Registry synchronization with `Event Producers`. We assume that if new GitHub events are created by
GitHub after our cluster has been configured (i.e., our GitHub CRD Source installed) and the appropriate webhooks
have been created, we will need to create new webhooks (and update the GitHub CRD Source) if we want to listen for
those new events. Such task will again be in charge of the `Cluster Configurator`.
those new events. Until doing so, those new events shouldn't be listed in the Registry.
Such task will again be in charge of the `Cluster Configurator`.

## Requirements

Expand Down Expand Up @@ -118,7 +119,7 @@ By applying the above file, two EventTypes will be registered, with types `dev.k
`dev.knative.source.github.pull_request`, source `my-other-user/my-other-repo`, for the `default` Broker in the `default`
namespace, and with owner `github-source-sample`.

Note that the `Cluster Configurator` is the person in charge of taken care of authentication-related matters. E.g., if a new `Event Consumer`
Note that the `Cluster Configurator` is the person in charge of taking care of authentication-related matters. E.g., if a new `Event Consumer`
wants to listen for events from a different GitHub repo, the `Cluster Configurator` will take care of the necessary secrets generation,
and new Source instantiation.

Expand Down Expand Up @@ -315,7 +316,7 @@ spec:
- Auto Add

By setting the ingress policy to auto add, the Broker will accept any event and will add its EventType
to the Registry (in case is not present).
to the Registry (in case it is not present).

Copy link

Choose a reason for hiding this comment

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

I'd put this feature at a MAY level of support in a spec; I think you should be able to be an event broker without it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That seems right, will do!

```yaml
apiVersion: eventing.knative.dev/v1alpha1
Expand All @@ -330,3 +331,62 @@ spec:
Note that more policies should probably need to be configured, e.g., allow auto-registration of EventTypes received
from within the cluster (e.g., from a response of a Service running in the cluster) as opposed to external services, and so on.
We just enumerate three simple cases here.

## FAQ

Here is a list of frequently asked questions that may help clarify the scope of the Registry.

- Is the Registry meant to be used just for creating Triggers or for also setting up Event Sources?

It's mainly intended for helping the `Event Consumer` with the `discoverability` use case. Therefore,
it is meant for helping out creating Triggers.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, for MVP of the registry, the expectation is that in order for a user to create a Trigger, they will need to know what event types are available, and hence there should be a way to discover them.
For MVP, these are Event Types that have been configured to be sent to a given broker, hence a Trigger for them can be created.

I think Event Source CRD is a "Potential" Event. Without instantiating one, you have no events. Hence the "Potential". Once an Event Source CR has been created, then those events are available to create Triggers for.

Now, I could certainly see a world (not in MVP), where an Event Source CRD could be pointed to by a registry.
So, Event Source CRD could be added to a registry, roughly looking like this:
{Broker, Source, Type} -> Ref To the CRD.

This way if a user looks at the Registry, they would see the Event Types available. Now, in order to actually create the CR, they will probably need to specify additional information (Like Credentials, etc.), and this is where things get wonky because you don't want to necessarily put those into the Trigger spec.

So, I think that' goal is doable, I just think by providing the MVP experience that focuses on the discovery of event types available (not potentially available) would allow us to provide value to the user and then add more niceties on top of it later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Furthermore, it's not only the matter of say providing credentials, there needs to be a way to describe the "subscription schema" that a user can inspect and understand things like, what format the credentials are (unless we come up with a standard, convention, or something like that), but in any case, I just think that providing a Registry as a concept makes users lives better, then in the next release we could start tackling these additional trickier bits.

- If I have a simple use case where I'm just setting up an Event Source and my KnService is its Sink (i.e., no Triggers involved),
is there a need/use for the Registry?

In this case, we believe there is no need for the Registry. As you can see in the EventType CRD, there is a mandatory
`broker` field. If you are not sending events to a Broker, then there is no need to use a Registry.
nachocano marked this conversation as resolved.
Show resolved Hide resolved
Implementation-wise, we can check whether the Source's sink kind is `Broker`, and if so, then register its EventTypes.

- Is the Registry meant to be used in a single-user environment where the same person is setting up both the Event Source and
the destination Sink?

We believe is mainly intended for multi-user environment. A `Cluster Configurator` persona is in charge of setting up
the Sources, and `Event Consumers` are the ones that create the Triggers. Having said that, it can also be used in
a single-user environment, but the Registry might not add much value compared to what we have right now in terms of
`discoverability`, but it surely does in terms of, for example, `admission control`.

- Does a user need to know which type of environment they're in before they should know if they should look at the Registry?
In other words, is a Registry always going to be there and if not under what conditions will it be?

If there are Sources pointing to Brokers, then there should be a Registry. `Event Consumers` will always be able to
Copy link
Contributor

Choose a reason for hiding this comment

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

Since registry is made of CR entries, there's always a registry, but there just might not be any CR => empty registry?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is the right way of putting it. You are right, you can always kubectl get eventtypes, in case there is no CR, then the registry will be empty. But there will always be one.
Will update that. Thanks for the comment!

`kubectl get eventtypes -n <namespace>`.

- Once an Event Source is created, how is a new one created with different auth in an env where the user is really just meant
to deal with Triggers? This may not be a Registry specific question but if one of the goals of the Registry is to make it
so that the user only deals with Triggers using the info in the Registry, I think this aspect comes into play.
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, so I think we should approach this in two steps. Provide the MVP where the Source needs to be created separately and say in .7 release tackle something like possibly being able to create a Trigger from a Source CRD. Namely the credentials will take some work, etc.


We believe the Event Source instantiation with different credentials should be handled by the `Cluster Configurer`. If the
`Cluster Configurer` persona happens to be the same person as the `Event Consumer` persona, then it will have to take care
of creating the Source. This is related to the question of a single-user, multi-user environment above.

- I've heard conflicting messages around whether the Registry is just a list of Event Types or it will also be a list of
Event Sources so that the user doesn't need to query the CRDs to get the list. We need to be clear about this.

The Registry is a list of EventTypes. Having said that, the `Event Consumer` could also (if it has the proper RBAC permissions)
list Event Sources (e.g., `kubectl get crds -l eventing.knative.dev/source=true`), but that list is not part of what we call
Registry here. The idea behind the fields in the EventType CRD is to have all the necessary information there in
order to create a Triggers, thus, in most cases, the `Event Consumer` shouldn't have to list Sources.

- I wonder if the Event Source populating the Registry should happen when the Event Source is loaded into the system,
meaning when the Event Source's CRD is installed (not when an instance of the CRD is created).

The problem with that is that you don't have a namespace (nor Broker, user/repo, etc.) at that point.
Which namespace the EventType should be created on? Pointing to which Broker?
Implementation-wise, one potential solution is to have a controller for source CRDs, whenever one is installed, search for all the namespaces with
eventing enabled (`kubectl get namespaces -l knative-eventing-injection=enabled`), and adding all the possible EventTypes from that CRD to each of
the Brokers in those namespaces. A downside of this is that the Registry information is not "accurate", in the sense that it only has info about EventTypes
that may eventually flow in the system. But actually, they will only be able to flow when a CR is created.

- ...