knative / knative/eventing

[Experimental] `KReference.Group` to avoid being explicit on the `APIVersion`

Open
#5,086 33 comments 0 reactions 0 assignees View on GitHub
kind/bug roadmap triage/accepted
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.