Reconcile EventTypes within Source Duck controller

[nachocano]

Fixes #2484

Proposed Changes

• 🧽Creating a Source CRD reconciler that reconciles CRDs with the duck.knative.dev/source="true" label. This reconciler is in charge of dynamically creating controllers for a particular GVR.

• 🧽Creating a Source duck reconciler, which is dynamically created by the CRD one mentioned above, that reconciles duckv1.Source resources with a particular GVR. It currently creates their EventTypes but in the future it can do more things.

• In order to do this I needed to change the duckv1.SourceStatus to also contain a CloudEventAttributes list. This might be the most controversial change and I'm fully open for feedback. I couldn't just set the ceSource in the Status, because different Sources do different things. For example, the GithubSource allows you to select a subset of the eventTypes listed in the CRD (thus we need the ceTypes information as well and cannot just rely on the CRD registry annotation). The KafkaSource may contain multiple ceSources, e.g., if you specify two topics then the data plane constructs the ceSource based on the msg.Topic. In some Google Sources, we do not event know the ceSource at control-plane time, but rather on the data plane (thus I made eventType.spec.source optional), etc. To have a better idea of what I'm talking about, please refer Removing EventType dependency from eventing-contrib eventing-contrib#1036 in eventing-contrib. Also, here is the pkg PR Updating duckv1.SourceStatus to include CloudEventAttributes pkg#1165

• Added some TODOs in the code so that they are properly revisited once we address Improve EventType Registry #2750. IMO these changes puts us in a much better position for revisiting EventType. Also I made some mandatory parameters from EventType optional as well (not breaking change).

• Added E2E tests for both ApiServerSource and PingSource.

• Still missing some UTs, planning on adding them in a follow up...

Ack to @n3wscott for his autotrigger and discovery magic. This is basically the former.

Release Note

Docs

/assign @lionelvillard @vaikas @n3wscott @mattmoor @mikehelmick would be great to have your input

[knative-prow-robot]

@nachocano: GitHub didn't allow me to assign the following users: would, to, your, input, be, great, have.

Note that only knative members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

Details

In response to this:

Fixes #2484

Proposed Changes

• 🧽Creating a Source CRD reconciler that reconciles CRDs with the duck.knative.dev/source="true" label. This reconciler is in charge of dynamically creating controllers for a particular GVR.

• 🧽Creating a Source duck reconciler, which is dynamically created by the CRD one mentioned above, that reconciles duckv1.Source resources with a particular GVR. It currently creates their EventTypes but in the future it can do more things.

• In order to do this I needed to change the duckv1.SourceStatus to also contain a CloudEventAttributes list. This might be the most controversial change and I'm fully open for feedback. I couldn't just set the ceSource in the Status, because different Sources do different things. For example, the GithubSource allows you to select a subset of the eventTypes listed in the CRD (thus we need the ceTypes information as well and cannot just rely on the CRD registry annotation). The KafkaSource may contain multiple ceSources, e.g., if you specify two topics then the data plane constructs the ceSource based on the msg.Topic. In some Google Sources, we do not event know the ceSource at control-plane time, but rather on the data plane (thus I made eventType.spec.source optional), etc. To have a better idea of what I'm talking about, please refer Removing EventType dependency from eventing-contrib eventing-contrib#1036 in eventing-contrib.

• Added some TODOs in the code so that they are properly revisited once we address Improve EventType Registry #2750. IMO these changes puts us in a much better position for revisiting EventType. Also I made some mandatory parameters from EventType optional as well (not breaking change).

• Still missing: UTs, but who needs that :)

Ack to @n3wscott for his autotrigger and discovery magic. This is basically the former.

Release Note

Docs

/assign @lionelvillard @vaikas @n3wscott would be great to have your input

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[mattmoor]

Produced via:
gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -type f -name '*.go' -print)

[mattmoor]

Produced via:
gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -type f -name '*.go' -print)

[nachocano]

ping...
It would be great to get some feedback on this.
I'm going on paternity leave end of next week, and will be gone for 1.5 months. So if we can get this in before that, it'd be great. Especially because it cleans up a bunch of things in eventing-contrib. See knative/eventing-contrib#1036 and IMO puts us in a better position to addressing EventType improvements

[mattmoor]

Produced via:
gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -type f -name '*.go' -print)

[mattmoor]

Produced via:
gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -type f -name '*.go' -print)

[n3wscott]

Two changes I would like to see:

1. Drop the finalizers on the CRDs, there is no need.
2. Add a e2e test or two for this to confirm it works.

[nachocano]

2. confirm it works.

OK, will do! Thanks for reviewing Scott!

[knative-metrics-robot]

The following is the coverage report on the affected files.
Say /test pull-knative-eventing-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/eventing/v1alpha1/eventtype_validation.go	95.0%	92.3%	-2.7
pkg/apis/eventing/v1beta1/eventtype_validation.go	95.0%	92.3%	-2.7
pkg/apis/sources/v1alpha1/apiserver_lifecycle.go	59.3%	56.0%	-3.3
pkg/apis/sources/v1alpha1/ping_lifecycle.go	85.7%	84.2%	-1.5
pkg/reconciler/apiserversource/apiserversource.go	79.1%	80.2%	1.1
pkg/reconciler/apiserversource/controller.go	94.1%	93.3%	-0.8
pkg/reconciler/pingsource/controller/pingsource.go	63.9%	64.0%	0.1

[nachocano]

Changes done, RFAL

[n3wscott]

nice work, looks real good. This will clean up a lot of controllers. 👍

/lgtm
/approve

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: n3wscott, nachocano

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 [n3wscott,nachocano]

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