[Experimental] `KReference.Group` to avoid being explicit on the `APIVersion`
- Dominant language
- Go
- Stars
- 1.6k
- Forks
- 631
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 6
Description
**Description**
I propose to add a new field to `KReference`, called `Group`. For example, in order to refer to a `KafkaChannel`, more than using this one:
```
Spec:
Channel:
API Version: messaging.knative.dev/v1alpha1
Kind: KafkaChannel
Name: testchannel
```
The user will be able to just use this:
```
Spec:
Channel:
Group: messaging.knative.dev
Kind: KafkaChannel
Name: testchannel
```
This approach brings the following benefits:
* User doesn't need to care about api versions when referencing to other resources, so definitely better UX
* Simpler upgrades when some api versions disappear (like https://github.com/knative-sandbox/eventing-kafka/issues/426#issuecomment-800991695)
When the api version is needed, an algorithm can just retrieve the latest for the provided api group and kind and then query the api server using such version.
**Background**
This came out while working on the KafkaChannel v1alpha1 removal: https://github.com/knative-sandbox/eventing-kafka/issues/426#issuecomment-800991695. This upgrade ended up being quite painful and we even got it wrong with brokers https://github.com/knative-sandbox/eventing-kafka/issues/624.
Not tieing to the api version seems to me logically more correct, because the api version is a detail of the resource. With the reference system we have today, every time we need to remove an old api, we'll have to ask users to accept some **downtime** to fix the api version manually, or we need to end up with **hacks** like https://github.com/knative/eventing/pull/5085/files#diff-74513e8fb938944cdb1b786c568c843a282eecd46ba5440a4374763245e2ffeaR30. Both are in my opinion unacceptable.
**Exit Criteria**
Users are now able to use references without explicitly define the api version, but just using the api group.
**Experimental flag name**: `kreference-group`
**Experimental feature stages plan**
Below the proposed plan for the feature stages (this list implicitly includes the requirements defined in the process)
* Alpha: **Included in 0.24**
* [x] Add to `KReference` the `Group` field and implement the logic to resolve the api version. My idea is to add a method like `KReference.Resolve(crdLister)` that overwrites, if needed, the `KReference.APIVersion` field with the served CRD version (similar to #5131): https://github.com/knative/pkg/pull/2127
* [x] Support the `Group` field inside the `Subscription.Spec.Subscriber`, adding the `x-preserve-unknown-fields` to the CRD and the required code to implement it: #5440
* [x] User documentation
* Beta graduation as soon as 1 release after the inception
* Beta:
* [ ] Support the new `KReference.Group` field in every place we use this type:
* [ ] Enable in all the usages of Destination duck https://github.com/knative/pkg/pull/2179
* [x] Subscriber channel: https://github.com/knative/eventing/pull/5520
* [ ] User documentation stabilization and improvements
* [ ] More e2e tests
* [ ] Add conformance tests
* Stable graduation as as soon as 2 releases after the Beta graduation
* Stable:
* [ ] Add the requirement to the spec
* [ ] Suggest in all documentation pages to use the new `Group` field more than `APIVersion`
**Affected WG**
* Eventing WG
**Prior discussions**
* https://github.com/knative-sandbox/eventing-kafka/issues/426
* https://github.com/knative-sandbox/eventing-kafka/issues/624
* #5131
Contributor guide
Research direction
Start with KReference and its uses in Subscription.Spec.Subscriber and Destination duck, then review the referenced CRD changes and existing resolution work in #5131. Done means group-only references resolve a served API version across all listed usages, with user documentation, e2e tests, and conformance coverage addressing the remaining plan items.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kubernetes
- Domain
- api, backend, distributed-systems
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100