Subscription.Spec.Channel should use KReference instead of corev1.ObjectReference

[slinkydeveloper]

Describe the bug
Subscription.Spec.Channel uses corev1.ObjectReference, which has fields we don't need, and we don't support. Per documentation:

eventing/pkg/apis/messaging/v1/subscription_types.go

Lines 76 to 92 in 0653ac4

	// Reference to a channel that will be used to create the subscription
	// You can specify only the following fields of the ObjectReference:
	// - Kind
	// - APIVersion
	// - Name
	// The resource pointed by this ObjectReference must meet the
	// contract to the ChannelableSpec duck type. If the resource does not
	// meet this contract it will be reflected in the Subscription's status.
	//
	// This field is immutable. We have no good answer on what happens to
	// the events that are currently in the channel being consumed from
	// and what the semantics there should be. For now, you can always
	// delete the Subscription and recreate it to point to a different
	// channel, giving the user more control over what semantics should
	// be used (drain the channel first, possibly have events dropped,
	// etc.)
	Channel corev1.ObjectReference `json:"channel"`

Also the CRD definition is wrong, because the schema contains those fields we don't support in that definition:

https://github.com/knative/eventing/blob/main/config/core/resources/subscription.yaml#L37-L61

We then perform the validation properly, with some custom logic:

eventing/pkg/apis/messaging/v1/subscribable_channelable_validation.go

Line 62 in 0653ac4

func checkDisallowedObjectReferenceFields(f corev1.ObjectReference) *apis.FieldError {

But still it doesn't make sense the "bigger type", to have the fields disabled. Neither it makes sense to have those fields documented in the CRD, if then they are disallowed.

Expected behavior
We should use KReference which contains just the fields we can support for the Channel field. We also need to trim the CRD schema to just the allowed fields.

Knative release version
all

Additional context
This was discovered while planning #5086. Because of that type, we can't even further extend the behaviour of that particular field.

[matzew]

Ughh... nice catch!

[vaikas]

Thanks for catching this. Yeah, KReference was added after that for the reasons you mention, and that particular field in subscription just apparently was never updated :(

"Nice thing" is that it's wire compatible for yaml so it won't break anything there, but if anybody is using client libraries they will need to change.