Eventing: Adopt Destination in Subscriptions and Triggers.

[n3wscott]

Problem
adding uri and path to subscribers and sinks in Eventing/Messaging via Destination.

Also there is a new resolver helper for Destinations also in pkg.

[grantr]

This would be a breaking API change. Trigger and Subscription spec changes from:

spec:
  subscriber:
    ref:
      apiVersion: serving.knative.dev/v1
      kind: Service
      name: my-service

to:

spec:
  subscriber:
    apiVersion: serving.knative.dev/v1
    kind: Service
    name: my-service

Why is this breaking change necessary or useful, versus the non-breaking change that keeps ref?

[grantr]

I'm concerned that putting all the fields at the top level of the Destination struct makes validation complex for the user. Similar concerns were expressed in https://github.com/knative/pkg/pull/561. The validation message here considers the three fields apiVersion, kind, and name as a single object distinct from uri, but that's not reflected in the api structure.

Something like this would be backward compatible with trigger and subscription:

type Destination struct {
    Ref *corev1.ObjectReference `json:"ref,inline"`
    URI *apis.URL `json:"uri,omitempty"`
    Path *string `json:"path,omitempty"`
}

and is more obvious that Ref member fields are logically distinct from URI and Path fields.

[matzew]

Ref member fields are logically distinct from URI and Path fields.

After this statement and re-reading this comment, I tend to agree w/ @grantr

[lionelvillard]

• regarding breaking the API concern: it shouldn't be a concern for v1alpha1.
• regarding the validation concern: OpenAPI (and k8s) supports oneOf so I would expect users to be somewhat familiar with it. Maybe a bit confusing at first.

Also related to https://github.com/knative/eventing/issues/1936. For Sequence and Parallel, Reply is an ObjectReference (should be Destination), without ref. We should at least be consistent.

Finally, I expect that in many cases uri will be automatically populated and user should only care about apiVersion, kind and name. Adding the extra ref seems (and was for me) confusing.

[aslom]

Where is design documentation for it that explains https://github.com/knative/pkg/pull/561/files ? Will it change for v1 vs v1alpha1? If I have uri why do I need path? Any place that have examples how it would be used?

[nicolaferraro]

I agree that removing res from the current objects is an unnecessary complication.

Also from a semantic point of view, by keeping res you can say that a Destination contains either a ref or a uri, while if you flatten res into three fields that becomes not so obvious.

Another concern is: why calling it uri if it's a URL? Can it contain anything different from a resource locator? Addressable correctly uses url: https://github.com/knative/pkg/blob/9c320664c8c479daae224db1f8b5579235bbe567/apis/duck/v1/addressable_types.go#L37

[rhuss]

So if I understand correctly "ref" and "uri" are mutually exclusive (only one make sense), but "path" can be always added. I agree with @nicolaferraro , that for this aspect it makes sense to keep "ref" as it is easier to argues/visualize/validate the difference between ref and an alternate URI and describe the reference value as an "atom" (which has an own validation).

[rhuss]

If I have uri why do I need path

True, but if you have a "reference" then you might need a path if the addressed service exposes its endpoint not under the root context.

However, an alternative would be to move the path part to the ref: section, too (and let the URI stand alone for it, as it can have the path already included.

spec:
  subscriber:
    ref:
      apiVersion: serving.knative.dev/v1
      kind: Service
      name: my-service
      path: /hello

and

spec:
  subscriber:
    uri: "https://myexternalsvx.example.com/hello"

This would also solve the issue, whether when path and uri is given, whether the path should replace the URI path or be appended to the URI path (of course one could argue, that an absolute path would override and a relative path would append, but still I don't see the benefit for an additional path if an uri is given)

[rhuss]

Also 👍 for renaming uri to url.

[matzew]

I really like @rhuss suggestion:

spec:
  subscriber:
    ref:
      apiVersion: serving.knative.dev/v1
      kind: Service
      name: my-service
      path: /hello

Also :+1: for renaming uri to url.

Same here :+1:

[lionelvillard]

If we keep ref, +1 on path. I wished I had it a while back: https://github.com/knative/docs/blob/master/docs/eventing/samples/parallel/mutual-exclusivity/parallel.yaml#L11

[n3wscott]

Uri is the right name, it could be anything. Like a urn.

[n3wscott]

We need both uri and path to allow for root urls and an additional path. This is the case where a destination is a resolved uri plus a custom path. It could be non trivial to combine them at resolve time.

[n3wscott]

I assert subscriber.ref was a mistake. Destination resolves that mistake, and the API is not beta yet, so good time to break it.

[grantr]

It could be non trivial to combine them at resolve time.

This may be true, but can you give an example so we understand what you're thinking about? If the resolver is responsible for combining them, and the resolution is non-trivial, the user must still understand how the combination process works so they can predict what the resolved URL will be.

I assert subscriber.ref was a mistake

Can you explain why you think so? I'd like to understand your reasoning.

[akashrv]

I prefer @rhuss suggestion.

1. Path and URL at the same root level is confusing because a URL is already fully qualified with all the constructs "scheme:[//authority]path[?query][#fragment]"
2. Problems we are trying to solve with path (and this could extend with more pieces) is for resolved urls from object references such as addressable and k8s service, hence grouping them together in a single struct makes sense.
3. About URI v/s URL as the name, from what I read it is specifically a URL in destination. Now we can always say that all URLs are URIs, however a URI is an identifier, whereas a URL is specifically a URI that is a locator. We need an absolute locator in destination, and I prefer it to be explicitly called out without any confusion.

@n3wscott If you think that ref was a mistake and that you prefer url (and not uri), then please explain the rationale, else the statement seems like and opinion and doesn't help make the correct decision.

General note to all: Let's please close this asap and make part of v0.10.0 @capri-xiyue has already started working on adopting "destination" inside eventing core, can we close this thread early next week, or before our WG meeting? Please do not drag this till the end of the release.

[n3wscott]

@n3wscott If you think that ref was a mistake and that you prefer url (and not uri), then please explain the rationale, else the statement seems like and opinion and doesn't help make the correct decision.

I think the field ref as a named field is the mistake. It would also be a mistake to use url.

I would be fine doing as grant said:

type Destination struct {
    Ref *corev1.ObjectReference `json::"ref,omitempty"`
    URI *apis.URL `json:"uri,omitempty"`
    Path *string `json:"path,omitempty"`
}

if that makes it more clear what in the world is happening.

then validation is as follows:

ref resolves to a root url

OR

uri resolves to a root url

THEN

path is added to the root url. 

[akashrv]

I think the field ref as a named field is the mistake. It would also be a mistake to use url.

@n3wscott I believe that you must have spent time thinking about this and made the opinion. However as everyone else has explained the rationale, it helps to explain the rationale. That way we all help each other see things that we might have missed. so asking again - Could you consider the rationale provided by others in this thread (including myself), and then explain yours too? That will help us make the right decision and also learn from each other.
in other words - you have made it crystal clear that you think ref and url are wrong choices, could you explain why?

[capri-xiyue]

Currently I prefer

1. The field ref as a named field since it makes more clear that Ref member fields are logically distinct from URI and Path fields.
2. Regarding URL and URI, I think URL makes more sense since the object in the destination should be a locator.
   For point 1 and 2, @n3wscott I still don't understand why do you think Ref named field and URL naming is a mistake. Could you help explain it? Since it's duplicate efforts to adopt destination first and then devs still need to change relevant code if the interface of destination gets changed. I hope we can make an agreement ASAP.
3. I'm not sure how do we use Path. Will we use Path and URL with root URL and addition path? Or user will just use URL to define both the root URL and addition path? If we just use Path with Ref, I think it's a little bit confusing to put path in the same level as Ref and URL. It might be better to put path in Ref field.

[lionelvillard]

I personally prefer not having the ref field because:

• I understand oneOf
• It's easier to write and to read (less nesting)
• The learning curve is minimal: it's apiVersion, name, kind, path or ur(i|l)

I would keep path only as part to the inlined object ref to remove the confusion about why having both url and path.

uri is more flexible thanurl but I don't have a concrete use-case justifying why it's useful to have it. So I'm fine with url.

[n3wscott]

@akashrv my why is as @lionelvillard has commented.

a flat structure is better for the person writing the yaml. less nesting and clearer to understand from their point of view.

I argue for _uri_ to support it as a resolvable component, such as custom schemes that resolve to a url, that resolved url then is used with path. Path is optional and you could opt to just use uri with path embedded.

@capri-xiyue There is code _already_ in pkg to use path and add it to the resolved ref, or the uri. There is also code that validates it.

[evankanderson]

What if url/uri parameter is resolved in reference to a base URI of the Addressable in ref?

If it's an absolute URL, it overrides whatever is in ref, and we might want to validate that ref is empty. If it's relative, it works like path in some of the designs here.

This could give us a resource like:

ref:
  apiVersion: serving.knative.dev/v1
  kind: Service
  name: bar
url: /foo

[akashrv]

What if url/uri parameter is resolved in reference to a base URI of the Addressable in ref?

If it's an absolute URL, it overrides whatever is in ref, and we might want to validate that ref is empty. If it's relative, it works like path in some of the designs here.

This could give us a resource like:

ref:
  apiVersion: serving.knative.dev/v1
  kind: Service
  name: bar
url: /foo

I think this is a good suggestion and resolves all comments/concerns. Let's just make sure that all validations and how to resolve this struct to a url/uri is centralized in destination and all corner cases handled in one place (such as ref nil, uri relative; ref not nil, uri absotlute; etc)
And in this case, the variable name will be URI and not a URL as it can be relative and not always absolute

[n3wscott]

just having ref and uri breaks the usecase of having a root uri and a path, but it opens up fun cases like:

ref:
  apiVersion: serving.knative.dev/v1
  kind: Service
  name: bar
url: /?foo#baz

I could live with this.

[akashrv]

just having ref and uri breaks the usecase of having a root uri and a path, but it opens up fun cases like:

ref:
  apiVersion: serving.knative.dev/v1
  kind: Service
  name: bar
url: /?foo#baz

I could live with this.

Could you provide and example of which use it breaks? Couldn't the ref be root uri, and the uri=relative uri with path?

[n3wscott]

Here is an example: I am implementing a router with eventing, I have /a /b /c handlers. They are implemented in an off cluster FaaS:

uri: http://offcluster.example.com
path: /a

uri: http://offcluster.example.com
path: /b

uri: http://offcluster.example.com
path: /c

to make these I can make a template:

uri: {{host}}
path: /{{path}}

Now, if I need too change the off cluster url, I could just update {{host}}

But if I do not have path, then I would have routes like

uri: http://offcluster.example.com/c

And a template like:

uri: {{host}}/{{path}}

Which is more error prone because I need to be sure that things like http://exmaple.com///b work out.

[n3wscott]

Also, having uri assumed to be non-resolving means we miss out on cases like:

uri: lookup://this-is-not-a-url.look-up-this-key-in-a-lookup-table
path: /foo

Resolves to sending messages to http://example.com/foo

[grantr]

I think the official term for the field proposed by @evankanderson is _URI reference_ as summarized
at https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#URI_references:

A _URI reference_ is either a URI, or a _relative reference_ when it does not begin with a scheme component followed by a colon (:).

The same page further specifies in https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#URI_resolution that

_Resolving_ a URI reference against a _base URI_ results in a _target URI_.

and gives some informative examples of how a relative reference would be resolved against a base URI.

The doc for this field might be as follows:

A URI reference specifying the location of the sink. If this is a relative reference, the target URI is resolved with the URI retrieved from Ref as the base URI.

I think I agree with Akash that this field should be called uri instead of url if it's allowed to be a relative reference. Also, https://en.wikipedia.org/wiki/Uniform_Resource_Name points out that RFC3986 in 2005 requested that the URL and URN names be dropped in favor of just URI.

[n3wscott]

It complicates my usecase, but am not sure my usecase is real, and can be done with having ref and uri, plus adding path or others later could be an option.

I am still +1 on this idea for just uri and ref.

[akashrv]

```yaml
uri: {{host}}/{{path}}

```

I will agree with you previous comment that we can live with this.
And yes as @grantr pointed out I assumed them to be uri references.

So it seems like we have a consensus (thanks @evankanderson) - @lionelvillard, @matzew @rhuss - Would like to hear your views, before @capri-xiyue implements this.

[n3wscott]

implements this.

It is already all implemented in pkg in the original format. Make sure to update that impl, not reimpl.

https://github.com/knative/pkg/pull/606

https://github.com/knative/pkg/pull/572

[lionelvillard]

+1 for uri and neutral for ref. I have a preference for having a flat structure but not a deal breaker :-)

[lionelvillard]

To be clear on why I prefer a flat structure is because I believe specifying an object reference is the common case. And since uri replaces path, this makes perfect sense:

apiVersion: serving.knative.dev/v1
kind: Service
name: bar
uri: /foo

For sequence, it would look like this:

apiVersion: messaging.knative.dev/v1alpha1
kind: Sequence
metadata:
  name: sequence
spec:
  channelTemplate:
    apiVersion: messaging.knative.dev/v1alpha1
    kind: InMemoryChannel
  steps:
    - apiVersion: serving.knative.dev/v1alpha1
      kind: Service
      name: first
    - apiVersion: serving.knative.dev/v1alpha1
      kind: Service
      name: second
    - apiVersion: serving.knative.dev/v1alpha1
      kind: Service
      name: third
reply:
  kind: Service
  apiVersion: serving.knative.dev/v1alpha1
  name: event-display

instead of:

apiVersion: messaging.knative.dev/v1alpha1
kind: Sequence
metadata:
  name: sequence
spec:
  channelTemplate:
    apiVersion: messaging.knative.dev/v1alpha1
    kind: InMemoryChannel
  steps:
   - ref:
        apiVersion: serving.knative.dev/v1alpha1
        kind: Service
        name: first
    - ref:
        apiVersion: serving.knative.dev/v1alpha1
        kind: Service
        name: second
    - ref:
        apiVersion: serving.knative.dev/v1alpha1
        kind: Service
        name: third
reply:
  ref:
    kind: Service
    apiVersion: serving.knative.dev/v1alpha1
    name: event-display

But again preferences are very subjective. I'm fine with what the majority decides.

[n3wscott]

@lionelvillard I totally agree with you, and the only point that wins me over to "it's ok with ref" is to not break the current api...

[matzew]

Actually, the sequence is a good example against having „ref“, I agree with
that!

On Mon 14. Oct 2019 at 23:52, Lionel Villard [e-mail]
wrote:

To be clear on why I prefer a flat structure is because I believe
specifying an object reference is the common case. And since uri replaces
path, this makes perfect sense:

apiVersion: serving.knative.dev/v1
kind: Service
name: bar
uri: /foo

For sequence, it would look like this:

apiVersion: messaging.knative.dev/v1alpha1
kind: Sequence
metadata:
name: sequence
spec:
channelTemplate:
apiVersion: messaging.knative.dev/v1alpha1
kind: InMemoryChannel
steps:
- apiVersion: serving.knative.dev/v1alpha1
kind: Service
name: first
- apiVersion: serving.knative.dev/v1alpha1
kind: Service
name: second
- apiVersion: serving.knative.dev/v1alpha1
kind: Service
name: third
reply:
kind: Service
apiVersion: serving.knative.dev/v1alpha1
name: event-display

instead of:

apiVersion: messaging.knative.dev/v1alpha1
kind: Sequence
metadata:
name: sequence
spec:
channelTemplate:
apiVersion: messaging.knative.dev/v1alpha1
kind: InMemoryChannel
steps:

• ref:
  apiVersion: serving.knative.dev/v1alpha1
  kind: Service
  name: first
  
  
  
  • ref:
    
    apiVersion: serving.knative.dev/v1alpha1
    
    kind: Service
    
    name: second
  
  
  • ref:
    
    apiVersion: serving.knative.dev/v1alpha1
    
    kind: Service
    
    name: third
    
    reply:
    
    ref:
    
    kind: Service
    
    apiVersion: serving.knative.dev/v1alpha1
    
    name: event-display
  
  

But again preferences are very subjective. I'm fine with what the majority
decides.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/knative/eventing/issues/1918?email_source=notifications&email_token=AABGPTSJUZU5KS6A35PZEZLQOTSYXA5CNFSM4IYA6EBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBGWLGI#issuecomment-541943193,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABGPTQO3CVJL75D7F65MCTQOTSYXANCNFSM4IYA6EBA
.

>

Sent from Gmail Mobile

[vaikas]

Just trying to summarize the discussion to make sure we're not missing anything. So, Destination becomes like so:

`

type Destination struct {
corev1.ObjectReference json:",inline"
URI *apis.URL json:"uri,omitempty"
}

`

This is then decoded as:

1. If there's an ObjectReference
   A: There's no URI, then resolve ObjectReference duck and use that as is
   B: There's a URI, it's a relative to the resolved ObjectReference, so resolve ObjectReference duck and tack URI to end of it.
2. If there's no ObjectReference, URI is expected to be fully qualified.

My thought is that we should optimize for the long term with the API, so if we think that extra Ref is unnecessary (does not cause confusion, etc.), then my preference would be to drop the Ref for all the reasons discussed. Also, we should decouple the what/how as in, what do we want the API to look like, and how do we get there.
@mattmoor since he made the original comment in the PR referenced above so that he can raise his concerns directly.

There have been other breaking changes (in Serving and Eventing), so we should not go forward with the non-optimal API only on the basis of not breaking the API (expected with v1alpha1 versioning).

Could folks that had earlier comments about the shape of the API please comment on the latest proposal, so that we can discuss it.

I would ideally like to close this by tmw WG meeting, so please if you have concerns, raise them asap so that we can hopefully resolve this today :)

Thanks all for very thoughtful discussion and good ideas / examples!

[mattmoor]

According to the proposed Knative release principles we would adopt a minimum K8s version of 1.15 in 0.12, which is 3 months out. Could we make v1alpha1 backwards compatible and adopt a new shape for v1beta1+?

The relevance of 1.15 becoming the minimum supported version is that we can start to leverage conversion webhooks.

[vaikas]

@mattmoor yeah, sorry for not being clearer about this:
What I was trying to decouple was:

• What should the API look like (hence your concerns about the shape of the api)
• How it gets rolled out and how it's supported.

[grantr]

I still prefer ref from an API design point of view because it seems weird to have the object's validation rules change based on which fields are set:

1. Set uri: validation passes
2. Set apiVersion: validation fails, now you need kind and name
3. Remove apiVersion, set uid: validation succeeds?

My general feeling on the Kubernetes APIs that already exist is that consistency of shared objects is prioritized over nesting level. Job could have added fields like completions to PodSpec but chose nesting instead. CronJob could have added schedule to JobSpec but chose nesting instead. Choosing to alter the shared core API ObjectReference seems contrary to that.

I could be convinced that flattening ObjectReference and URI is ok given similar examples from other Kubernetes APIs.

(This is my opinion, but I won't complain if we go the other way. I'm just glad we're having this discussion)

[mattmoor]

I think that generally I agree with @grantr: if we are embedding corev1.ObjectReference, distinguishing it explicitly is probably preferable.

However, I'd also point out that there are quite a few more specialized reference types in K8s type system:

• ObjectReference: What we're using
• LocalObjectReference: meant to be namespace-local, but omits other useful fields (e.g. TypeMeta)
• CrossVersionObjectReference: explicitly filters the fields of ObjectReference for use in HPA resources' scaleTargetRef.

So to offer an alternative, perhaps we should simply create our own FooReference with explicitly documented semantics and not inline corev1.ObjectReference?

[vaikas]

oops :)

[mattmoor]

Shape aside, FWIW this makes sense to me:

This is then decoded as:

1. If there's an ObjectReference
   A: There's no URI, then resolve ObjectReference duck and use that as is
   B: There's a URI, it's a relative to the resolved ObjectReference, so resolve ObjectReference duck and tack URI to end of it.
2. If there's no ObjectReference, URI is expected to be fully qualified.

[vaikas]

After chatting with @grantr offline, I realized the validation concern is wrt OpenAPI validation, not in the code on the backend, I was missing that context. Mea culpa.

Leaving the Ref there makes it clearer that ObjectReference is optional (vs. just some fields of it if they are flattened). So, that would mosdef be a pro for having it there. I can live with the extra yaml indentation (though I still remember writing the docs for the sequence and cursing the extra indent). Another Pro for keeping it is that it will also keep backwards compatibility and with the first point mentioned, that seems like we should keep it after all.

So, keep the Ref is what the plan is, we'll finalize tmw WG meeting unless we can close this before then here.

[grantr]

regarding the validation concern: OpenAPI (and k8s) supports oneOf

I think to capture the validation rules of apiVersion, kind, and name as implemented in https://github.com/knative/pkg/blob/0a0c4972cd7134ce54b1ce6a6de8f189257a64fb/apis/v1alpha1/destination.go#L119-L133, the OpenAPI schema for destination would need to define a sub-schema that looks like ObjectReference, and make that part of the oneOf.

Something like this:

components:
  schemas:
    Destination:
      oneOf:
      - $ref: '#/components/schemas/ObjectReferenceOnly'
      - $ref: '#/components/schemas/URIOnly'
      - $ref: '#/components/schemas/Both'
    ObjectReferenceOnly:
      # require apiVersion, kind, name
    URIOnly:
      # require uri
    Both:
      # require apiVersion, kind, name, uri

Is this the right way to do it? Any OpenAPI experts out there? @lionelvillard

[lionelvillard]

Well it's not so easy now. oneOf worked when we had path inside ref.

Maybe we should so what @mattmoor suggests: create our own FooReference and call it Destination without embedding ObjectReference. :-)

[rhuss]

• If there's an ObjectReference
  A: There's no URI, then resolve ObjectReference duck and use that as is
  B: There's a URI, it's a relative to the resolved ObjectReference, so resolve ObjectReference duck and tack URI to end of it.
• If there's no ObjectReference, URI is expected to be fully qualified.

Does this imply it is an error condition if there is an ObjectReference _and_ a fully qualified URI ? Also will the be a difference for a relative URI reference "context" (relative path) and "/context" (absolute path) ? Are both supposed to be appended to a base URL coming from the ObjectReference Duck or does an absolute path override the path component of that URL ? (and only the relative URI is appended)

[lionelvillard]

If you look at section 5.4.1 it's clear that /context is appended to the base URL. //context overrides the path.

When there is an ObjectReference it is used as base URL. section 5.2.2 precisely defines how the URI reference is converted to the target URI. At first glance and past experience, then the URI reference is absolute, it becomes the target URI, effectively ignoring the base URI.

[rhuss]

If you look at section 5.4.1 it's clear that /context is appended to the base URL. //context overrides the path.

Hmm, for me this is not so clear. I.e. for the base URL http://a/b/c/d;p?q the examples in 5.4.1 say

 "g"             =  "http://a/b/c/g"
 "g/"            =  "http://a/b/c/g/"
 "/g"            =  "http://a/g"

So it's different for a path "/g" (which overrides the path "/b/c/d") wheras "g" appends to the current context (leading to a path "/b/c/g")

IMO //context is a _network path_ which is something totally different. But maybe I really profoundly misunderstand something here ... (and we are just one / off ;-)

[rhuss]

My reference is from 4.2:

A relative reference that begins with two slash characters is termed a network-path reference; such references are rarely used. A relative reference that begins with a single slash character is termed an absolute-path reference. A relative reference that does not begin with a slash character is termed a relative-path reference.

[rhuss]

At first glance and past experience, then the URI reference is absolute, it becomes the target URI, effectively ignoring the base URI.

Agreed if it is an _absolute URI reference_ (i.e. starting with a scheme), but for anything else which is a _relative URI reference_ as defined in 4.2. This includes //path (i think we should ignore those), /path and path but also ../path etc.)

[rhuss]

But my question really was what happens when an ObjectReference and an absolute URI reference are given at the same time:

• Go into an error state

_or_

• Define a priority that one absolute URI overwrites the other one, like that uri: overwrites the Addressable's URI if both are absolute (or vice versa)

[n3wscott]

to @rhuss 's points, having a uri does complicate it over path. We will not just tack it on, but have to merge the two. Both what resolves from ref and uri could have query params, fragments, and paths.

And there is an error case in the both because if you provide both, you should not have provided a host in uri

[capri-xiyue]

For me, having Ref and URI reference as the shape of Destination is confusing.
For URI reference, the wiki said it supports things as follows:
`Within a representation with a well defined base URI of

http://a/b/c/d;p?q

"g:h" -> "g:h"
"g" -> "http://a/b/c/g"
"./g" -> "http://a/b/c/g"
"g/" -> "http://a/b/c/g/"
"/g" -> "http://a/g"
"//g" -> "http://g"
"?y" -> "http://a/b/c/d;p?y"
"g?y" -> "http://a/b/c/g?y"
"#s" -> "http://a/b/c/d;p?q#s"
"g#s" -> "http://a/b/c/g#s"
"g?y#s" -> "http://a/b/c/g?y#s"
";x" -> "http://a/b/c/;x"
"g;x" -> "http://a/b/c/g;x"
"g;x?y#s" -> "http://a/b/c/g;x?y#s"
"" -> "http://a/b/c/d;p?q"
"." -> "http://a/b/c/"
"./" -> "http://a/b/c/"
".." -> "http://a/b/"
"../" -> "http://a/b/"
"../g" -> "http://a/b/g"
"../.." -> "http://a/"
"../../" -> "http://a/"
"../../g" -> "http://a/g"`

Do we need to support query and fragment in Destination?
Compared to Ref and URL reference, I prefer Ref, URI and Path since it's more clear to me and the usage of URI is clean since it's a distinct field different from Ref. I don't have a strong opinion regarding whether Path should be an individual field or Path is a subfield of Ref. Both are fine to me. Though Path and URL at the same level may be confusing, it brings more flexibility to user so that they can use URL as the root URL and path as Additional Path.

[rhuss]

to @rhuss 's points, having a uri does complicate it over path. We will not just tack it on, but have to merge the two. Both what resolves from ref and uri could have query params, fragments, and paths.

Good point, @n3wscott

So where do we stand now ? Having both path: and uri: added to the Destination (with or without some nesting) ? And path: needs to be relative but without any other URI adornments (like fragments or query params) and an absolute path (i.e. starting with /) will override any uri's path whereas a relative path (not starting with /) will be appended to the uri (which comes either from uri: or the Addressable's uri). And its an error to both specify an object reference ref: and a uri: at the same time (but a path can be given any time).

The other option would be to have only one uri: field and conflict resolution rules when an Addressable is given and the additionally specified uri: is either an absolute reference with a scheme, contains fragments or query params with clashing names.

Personally I would still prefer this second option as I consider having a relative URI reference with query params/fragments in uri: to be an edge case (as well as I don't expect query parameters/fragments in an resolved URI of an Addressable to be a common use case).

[lionelvillard]

why not using this to resolve the reference: https://golang.org/pkg/net/url/#URL.ResolveReference

More programming language implement the rfc 3986 resolution algorithm (not always 100% conformant).

[n3wscott]

ResolveReference seems to eat the root path in the case of ref--> http://example.com/foo and uri--> bar

I want http://example.com/foo/bar

ResolveReference gives http://example.com/bar

[grantr]

I did some experimentation with that method: https://play.golang.org/p/12IfWdV7siU

I'm not convinced it's the behavior users will expect. For example, if the resolved ref is http://base.com/foo and the uri is bar, I think given the discussion here, we'd want the resolved uri to be http://base.com/foo/bar. But actually the result is http://base.com/foo.

Looking at the examples in https://github.com/knative/eventing/issues/1918#issuecomment-542782937, it doesn't look like there's any way to append a path segment to the uri. The reference always replaces the right-most path segment.

[n3wscott]

We already implemented this using for uris and a path using path.Join: https://github.com/knative/pkg/blob/b55b842259e0723498a8792794928b232ca8f223/resolver/addressable_resolver.go#L135-L142

[n3wscott]

@rhuss I am in this camp

to have only one uri: field and conflict resolution rules when an Addressable is given and the additionally specified uri: is either an absolute reference with a scheme, contains fragments or query params with clashing names.

I think we are now working on the conflict resolution rules. The interesting thing comes from the Addressable _could_ have path and query params. I expect uri to be merged+appended to it. If query params clash, who wins... (i think the uri). Navigating the path seems scary and we should not allow it (uri: "../../foo").

[grantr]

Do we need to support query and fragment in Destination?

I haven't seen anyone mention a need for query and fragment support. The only use case I've heard for URI manipulation is "append path segments to a URI that may or may not contain path segments". Unfortunately it doesn't look like standard URI reference resolution can do that. Maybe URI templates can, but that's potentially a lot more complexity:

ref:
  ...
uri: {ref}/foo

[grantr]

I feel a little nervous about changing the reference resolution rules for a field called uri. If we do that, we need to be clear in the docs that this field doesn't follow the RFC3986 rules when used as a relative reference.

A field called path may be easier to define as non-standard.

[lionelvillard]

@grantr yup agree.

ResolveReference does the right thing I believe:

--- "foo"
IsAbs(): false
resolving to base "http://base.com": http://base.com/foo
resolving to base "http://base.com/bar/": http://base.com/bar/foo
resolving to base "http://base.com/bar?a=b": http://base.com/foo

Note the / after bar.

[n3wscott]

Note the / after bar.

🤯

ok in that case, I could live with ResolveReference as long as we hold it right.

[grantr]

Oh good, nice catch @lionelvillard.

The reference clobbers query params from the base uri, even with the trailing slash, but that seems like a reasonable limitation (and it's standardized):

--- "foo"
IsAbs(): false
resolving to base "http://base.com": http://base.com/foo
resolving to base "http://base.com/bar/": http://base.com/bar/foo
resolving to base "http://base.com/bar/?a=b": http://base.com/bar/foo

[capri-xiyue]

I agree with the proposal with "ref" and "uri" field and using ResolveReference to merge it with "ref" since it's more flexible than a subfield "path" inside "ref" . I searched some docs and it said whether URI has trailing slash or not depends on whether it's a collection node or not. And it's standard to have target URI like "http://base.com/foo/bar" when base URI is "http://base.com/foo/" and relative URI is "foo". (Though I'm not sure whether we have the use case that the addressable url of a resource will be a collection node whose uri is like "http://base.com/foo/". Even we have, the proposal still satisfies it)

[grantr]

Ok I'll summarize what I think is the final recommendation for those following along:

type Destination struct{
  // +optional
  Ref *corev1.ObjectReference
  // +optional
  URI *apis.URL
}

Validation rules for Destination:

• If Ref is absent and URI is absent, the Destination is invalid.
• If Ref is present and URI is absolute (URL.IsAbs() returns true), the Destination is invalid.
• If Ref is absent and URI is relative (URL.IsAbs() returns false), the Destination is invalid.
• All other Destinations are valid.

I believe we'd implement this in OpenAPI using anyOf on Ref and URI, or using oneOf like this:

{
  "oneOf": [
    {
      "required": ["ref"],
      "not": {"required": ["uri"]}
    },
    {
      "required": ["uri"],
      "not": {"required": ["ref"]}
    }
  ]
}

But I'm not an OpenAPI expert. Someone else may want to verify that this is expressible in OpenAPI. The absolute/relative checks would likely be in code only.

Target URI resolution rules:

• If URI field is absent, the target URI is the URI retrieved from Ref.
• If URI field is present and absolute, the target URI is the value of the URI field.
• If URI field is present and relative, the target URI is the result of calling URL.ResolveReference with the URI retrieved from Ref as the base URI and the value of the URI field as the URI reference.

@vaikas-google over to you for :heavy_check_mark:

[vaikas]

LGTM.

[lionelvillard]

Just to make sure: what people think about ref, considering that lot of things have changed since we first discussed about it.

[vaikas]

Let's just use it to make the validation easier as well as for more clear
what's optional and not as well as backwards compatibility.

On Thu, Oct 17, 2019 at 10:59 AM Lionel Villard [e-mail]
wrote:

Just to make sure: what people think about ref, considering that lot of
things have changed since we first discussed about it.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/knative/eventing/issues/1918?email_source=notifications&email_token=ACWB45HUKRCUTT27KG6TTPDQPCRWXA5CNFSM4IYA6EBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBQ7QKQ#issuecomment-543291434,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ACWB45AYPESU5W4TPTLY2R3QPCRWXANCNFSM4IYA6EBA
.

[aslom]

If I read correctly the algorithm https://github.com/knative/eventing/issues/1918#issuecomment-542941078 and https://github.com/knative/eventing/issues/1918#issuecomment-542303282 rules are:

• if Ref is present then
  
  
  
  • if there's no URI then resolve Ref and use that as URI
  
  
  • else if URI a relative then resolve Ref with relative URI and use resolved URI
  
  
  • else Error: URI can not be absolute if Ref is present
  
  
• else If there's no Ref then
  
  
  
  • if URI is absolute then use it
  
  
  • else Error: URI is not absolute
  
  
• else Error: at lease URI or Ref must be present

[vaikas]

Correct.

On Thu, Oct 17, 2019 at 11:41 AM Aleksander Slominski <
[email protected]> wrote:

If I read correctly the algorithm #1918 (comment)
https://github.com/knative/eventing/issues/1918#issuecomment-542941078
and #1918 (comment)
https://github.com/knative/eventing/issues/1918#issuecomment-542303282
rules are:

• if Ref is present then
  
  
  
  • if there's no URI then resolve Ref and use that as URI
  
  
  • else if URI a relative then resolve Ref with relative URI and use
    
    resolved URI
  
  
  • else Error: URI can not be absolute if Ref is present
  
  
• else If there's no Ref then
  
  
  
  • if URI is absolute then use it
  
  
  • else Error: URI is not absolute
  
  
• else Error: at lease URI or Ref must be present

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/knative/eventing/issues/1918?email_source=notifications&email_token=ACWB45B7NQHI445E4ANJZFLQPCWUTA5CNFSM4IYA6EBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBRDWFY#issuecomment-543308567,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ACWB45EBFRCR335FQOSBHK3QPCWUTANCNFSM4IYA6EBA
.

[evankanderson]

https://github.com/knative/pkg/pull/774 is related?

[capri-xiyue]

I'm working on adopting Destination in Knative eventing(https://github.com/knative/eventing/issues/1918). When I tried to adopt Destination in Sources(for example: ApiServerSource), I noticed that we will have breaking api change.
At first, the interface of sources looks like this:

apiVersion: sources.eventing.knative.dev/v1alpha1
kind: ApiServerSource
metadata:
  name: testevents
  namespace: default
spec:
  serviceAccountName: events-sa
  mode: Resource
  resources:
    - apiVersion: v1
      kind: Event
  sink:
    apiVersion: eventing.knative.dev/v1alpha1
    kind: Broker
    name: default`

After we adopt Destination, the interface will look like this:

apiVersion: sources.eventing.knative.dev/v1alpha1
kind: ApiServerSource
metadata:
  name: testevents
  namespace: default
spec:
  serviceAccountName: events-sa
  mode: Resource
  resources:
    - apiVersion: v1
      kind: Event
  sink:
    ref:
      apiVersion: eventing.knative.dev/v1alpha1
      kind: Broker
      name: default

Is that ok that we change the type of sink from &corev1.ObjectReference to &Destination and introduce the breaking api change now? Or should we deprecate sink and introduce a new json field called "destination" to avoid the breaking api change now(To clarify I mean deprecate "Sink" and create another field Destination *v1alpha1.Destination json:"destination,omitempty" to avoid the breaking api change now)?

[n3wscott]

given this datapoint that the modified destination will break sources, and how this looks:

  sink:
    ref:
      apiVersion: eventing.knative.dev/v1alpha1
      kind: Broker
      name: default

vs

  sink:
    apiVersion: eventing.knative.dev/v1alpha1
    kind: Broker
    name: default

I will vote again for dropping ref from from destination, and adjust the rules for uri to be it is an override using the builtin go method with no other qualifications. Meaning:

• it is valid to have a ref and a full uri, the uri wins.
• It is valid to have relative uri and ref, the resolved uri is augmented.
• it is valid to have just uri if it is full (host plus scheme), or just ref.
• it is not valid to have just uri if no host or scheme.

I am thinking there is not much value in limiting the case of overloading the scheme/host if we can already set a full uri. We can just assume the user understands what they are doing, and we report back what we have resolved and are using the the status.

[n3wscott]

to be clear: my vote is make an API breaking change for subscriptions and triggers.

This is something we could decide to adopt in a v1beta1 api.

[lionelvillard]

for 1.10 we could have both, ref and inlined ObjectReference, and we can "slowly" phase out ref.

[n3wscott]

how about this:

type Destination struct{
  // +optional
  DeprecatedRef *corev1.ObjectReference  `json:"ref,omitempty"`
  // +optional
  *corev1.ObjectReference `json:",inline"`
  // +optional
  URI *apis.URL `json:"uri,omitempty"`
}

[aslom]

My .02c I think simpler is better unless complexity (and ref) is really required? Why not make destination to be URI ? I was hoping users could write simple URI for sink:

  sink: broker:default

instead of

  sink:
    apiVersion: eventing.knative.dev/v1alpha1
    kind: Broker
    name: default

and similarly:

   sink: service:my-service

and with path /hello

   sink: service:my-service/hello

URI class should be able to parse custom URI schema with kind/version for service or broker if versioning is needed?

  sink: broker:eventing.knative.dev/v1alpha1:default

and if course regular URL could be used to

  sink: https://host:port/path

The scheme part would be valid in knative eventing for service: and broker: with few simple rules defined. Similar to custom schemes https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Examples

That would be along lines of @rhuss comment https://github.com/knative/eventing/issues/1918#issuecomment-540970309

[n3wscott]

@aslom I do like this idea, but it does not work so well with unknown types that having a ref does let you do. The ref is a core idea in the ducktyping system we have been building. We would have to have a gvk/gvr version of the ObjectRef as a uri. WHICH COULD WORK! but we are really inventing something very new...

like k8s://eventing.knative.dev/v1alpha1/brokers/default/default or something...

[aslom]

@n3wscott I like k8s:// URI scheme. Duck typing would work with no changes as you get the same components after parsing URI: apiVersion, kind, name, path.

That could be supported as generic URI resolved by parsing
k8s://[apiVersion/?]kind:name[/?path] (example as above k8s://eventing.knative.dev/v1alpha1/brokers/default/path) and shortcut with kind:[apiVersion/?]name[/?path] (examplebroker:default/pathorbroker://eventing.knative.dev-v1alpha1/default`

URI rules are well described and readability is excellent: https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Generic_syntax

[capri-xiyue]

I checked the codebase. We will need to adopt Destination in multiple resources as follows:

• Trigger (https://github.com/knative/eventing/blob/master/pkg/apis/eventing/v1alpha1/trigger_types.go)
• Parallel
  (https://github.com/knative/eventing/blob/master/pkg/apis/messaging/v1alpha1/parallel_types.go)
  including Filter, Subscriber and Reply
• Sequence
  (https://github.com/knative/eventing/blob/master/pkg/apis/messaging/v1alpha1/sequence_types.go)
• Subscription
  (https://github.com/knative/eventing/blob/master/pkg/apis/messaging/v1alpha1/subscription_types.go)
• Source
  (https://github.com/knative/eventing/tree/master/pkg/apis/sources/v1alpha1)

With this Destination API:

// Destination represents a target of an invocation over HTTP.
type Destination struct {
    // Ref points to an Addressable.
    // +optional
    Ref *corev1.ObjectReference `json:"ref,omitempty"`

    // URI can be an absolute URL(non-empty scheme and non-empty host) pointing to the target or a relative URI. Relative URIs will be resolved using the base URI retrieved from Ref.
    // +optional
    URI *apis.URL `json:"uri,omitempty"`
}

We will have breaking api change for sources.
However, without Ref field in Destination, we will have breaking api changes for Trigger, Parallel, Sequence and Subscription.

I'll summarize what I think is the final recommendation for those following along:
Considering the open api validation concerns for inline fields (apiVersion, Kind, Name), logic confusion for combining inline fields (apiVersion, Kind, Name) with URI, it might be better to still use Ref in Destination.
To avoid the breaking api change for sources,
we can change Destination API as follows:

type Destination struct{
  // +optional
  Ref *corev1.ObjectReference  `json:"ref,omitempty"`
  // +optional
  DeprecatedObjectReference *corev1.ObjectReference `json:",inline"`
  // +optional
  URI *apis.URL `json:"uri,omitempty"`
}

@vaikas-google over to you for ✔️

[vaikas]

@aslom that's definitely an interesting idea, each k8s object does have a selfLink already, like a broker would be:
/apis/eventing.knative.dev/v1alpha1/namespaces/my-namespace/brokers/default
just note the leading /apis/ otherwise, same as your example.

Not having thought about this quite as deeply yet, but mixing the URI to represent a Reference to object holding the actual URI feels a bit clunky, because we're really doing a Reference to an object and grabbing the Status.URI from there. I also feel like using ObjectReference to refer to other objects is the standard way to do this in k8s.

[grantr]

A URI format for object reference is interesting and seems useful for some cases, e.g. annotations. @n3wscott alluded to this in https://github.com/knative/eventing/issues/1918#issuecomment-541912459 when he brought up the use case of a URI that isn't a hostname.

But I still think we should retain ObjectReference. It's the standard Kubernetes (and Knative) way to refer to another object, and we need a very good reason to decide against that.

We can explore new URI schemes after this change is made. Adding something like k8s:// will be backward compatible.

[aslom]

@grantr @n3wscott @lionelvillard can configuration be "duck typed"? I think it is possible with JSON schema for CRD with oneOf but not sure if tooling supports it?

If supported we could have both ObjectReference and URI

{
  "oneOf": [
    { "type": "URI" },
    { "type": "ObjectReference" }
  ]
}

https://json-schema.org/understanding-json-schema/reference/combining.html#oneof

so then both would be valid:

  sink: k8s://eventing.knative.dev/v1alpha1/brokers/default/path

and

  sink:
    apiVersion: eventing.knative.dev/v1alpha1
    kind: Broker
    name: default
    path: /path

[grantr]

I would avoid a field that can be either a string or object. That can be difficult to deal with in languages with static typing.

[capri-xiyue]

I checked the codebase. We will need to adopt Destination in multiple resources as follows:

• Trigger (https://github.com/knative/eventing/blob/master/pkg/apis/eventing/v1alpha1/trigger_types.go)
• Parallel
  (https://github.com/knative/eventing/blob/master/pkg/apis/messaging/v1alpha1/parallel_types.go)
  including Filter, Subscriber and Reply
• Sequence
  (https://github.com/knative/eventing/blob/master/pkg/apis/messaging/v1alpha1/sequence_types.go)
• Subscription
  (https://github.com/knative/eventing/blob/master/pkg/apis/messaging/v1alpha1/subscription_types.go)
• Source
  (https://github.com/knative/eventing/tree/master/pkg/apis/sources/v1alpha1)

With this Destination API:

// Destination represents a target of an invocation over HTTP.
type Destination struct {
  // Ref points to an Addressable.
  // +optional
  Ref *corev1.ObjectReference `json:"ref,omitempty"`

  // URI can be an absolute URL(non-empty scheme and non-empty host) pointing to the target or a relative URI. Relative URIs will be resolved using the base URI retrieved from Ref.
  // +optional
  URI *apis.URL `json:"uri,omitempty"`
}

We will have breaking api change for sources.
However, without Ref field in Destination, we will have breaking api changes for Trigger, Parallel, Sequence and Subscription.

I'll summarize what I think is the final recommendation for those following along:
Considering the open api validation concerns for inline fields (apiVersion, Kind, Name), logic confusion for combining inline fields (apiVersion, Kind, Name) with URI, it might be better to still use Ref in Destination.
To avoid the breaking api change for sources,
we can change Destination API as follows:

type Destination struct{
  // +optional
  Ref *corev1.ObjectReference  `json:"ref,omitempty"`
  // +optional
  DeprecatedObjectReference *corev1.ObjectReference `json:",inline"`
  // +optional
  URI *apis.URL `json:"uri,omitempty"`
}

@vaikas-google over to you for ✔️

It looks like json:",inline" doesn't work when we have a naming DeprecatedObjectReference.
It only works with

type Destination struct{
  // +optional
  Ref *corev1.ObjectReference  `json:"ref,omitempty"`
  // +optional
  *corev1.ObjectReference `json:",inline"`
  // +optional
  URI *apis.URL `json:"uri,omitempty"`
}

FYI: https://goplay.space/#a8IYOIWh1iG
I tried to search the official docs of golang json tags. But I don't find it. Is there anyone familiar with the usage of json tags?

I did some tests. It seems like json:",inline" has no use at all. Unmarshall still works with

type Destination struct{
  // +optional
  Ref *corev1.ObjectReference  `json:"ref,omitempty"`
  // +optional
  *corev1.ObjectReference 
  // +optional
  URI *apis.URL `json:"uri,omitempty"`
}

Found the official doc: https://golang.org/pkg/encoding/json/#Marshal
https://golang.org/pkg/encoding/json/#Marshal
Anonymous struct fields are usually marshaled as if their inner exported fields were fields in the outer struct, subject to the usual Go visibility rules amended as described in the next paragraph. An anonymous struct field with a name given in its JSON tag is treated as having that name, rather than being anonymous. An anonymous struct field of interface type is treated the same as having that type as its name, rather than being anonymous.

[capri-xiyue]

I'm wondering whether it's necessary to support backwards compatibility. To support backwards compatibility, we will have struct like

    type Destination struct {
        // Ref points to an Addressable.
        // +optional
        Ref *corev1.ObjectReference `json:"ref,omitempty"`

        // +optional
        DeprecatedAPIVersion string `json:"apiVersion,omitempty"`

        DeprecatedKind string `json:"kind,omitempty"`

        DeprecatedName string `json:"name,omitempty"`

        // URI can be an absolute URL(non-empty scheme and non-empty host) pointing to the target or a relative URI. Relative URIs will be resolved using the base URI retrieved from Ref.
        // +optional
        URI *apis.URL `json:"uri,omitempty"`
    }

However, DeprecatedAPIVersion, DeprecatedKind and DeprecatedName are implementation details. Users won't see it unless they check the codebase. I think when we finally deprecate [apiVersion, kind, name], users will still feel it's a breaking api change.
In addition, by maintaining a Destination structure above, it makes the code(implementation and error message a little bit complex, FYI I implemented it in https://github.com/knative/pkg/pull/790).

Considering the fact that DeprecatedAPIVersion, DeprecatedKind and DeprecatedName won't have any meaning to the user experience, the workload of adopting Destination which supports [apiVersion, kind, name] into multiple resources like Trigger, Parallel, Sequence, Subscription, Source and then the workload to change all of them again when we finally deprecate [apiVersion, kind, name], I prefer introducing a breaking api change now instead of supporting the "fake" backwards compatibility

[rhuss]

I prefer introducing a breaking api change now instead supporting the "fake" backwards compatibility

me, too, as introducing new fields (i.e. formerly unknown) as _deprecated_ for Trigger, Parallel, Sequence and Subscription feels weird, too (and opens the chance that these fields are started to be used). Making the usage of a Destination symmetrically now in all Eventing without artificially introducing deprecated fields looks to me to be the right thing.

[vaikas]

The one problem is that we should allow existing objects to be operated on, by not having the fields present will break every existing object. So, without having any backwards compatible way to upgrade the existing objects to the newer version would be bad IMHO.
We're going to make the Destination a priority in our API task force to make sure that we can provide a sane user experience.

[capri-xiyue]

Another way to supports backwards compatibility and still alerts users about the deprecation might be adding a warning message when users use the deprecated fields.

[vaikas]

Yes, we have done this in the past for deprecated fields.

[capri-xiyue]

I did some investigation of how to deprecate fields.

1. OpenAPI 3.0 supports "deprecated". However, it looks like we use K8s OpenAPI 3.0 which only implements a subset of features of OpenAPI. Now K8s OpenAPI 3.0 doesn't support "deprecated".

1. For the webhook validation, it only returns*apis.FieldError. Therefore currently we can't use webhook validation to show deprecation warning

2. We can also use apis.Condition to mark deprecated(for example: https://github.com/knative/eventing/pull/1332/files#diff-0e0fe67d5b432f6523ce9ab2cce5dabcR191). But for Trigger, currently it doesn't have a condition type for subscriber URI. Maybe we can add one. But my concern is that if we use apis.Condition to mark deprecated, is it a little bit confusing since it involves some implementation details? It is not obvious that the usage of Destination will result in some deprecated warning of condition type of "sink" of "sources", "subscriberURI" of "trigger", and "filter" of "parallel".

3. We can use docs but we can't guarantee user will see it.

[aslom]

If we are doing non-backward compatible changes then why not radically improve operator and developer experience - if we can make YAML code much more readable without loss of functionality then why not do making writing and reading YAML with many destinations easy, compare current sequence and parallel example from
https://knative.dev/development/eventing/samples/sequence/sequence-reply-to-sequence/
https://knative.dev/development/eventing/samples/sequence/sequence-with-broker-trigger/
and https://knative.dev/development/eventing/samples/parallel/index.html
with removing ref and uri:

  steps:
    - service:first
    - service:second
    - service:third

  branches:
    - filter: service:even-filter
      subscriber: service:even-transformer
    - filter: service:odd-filter
      subscriber: service:odd-transformer
  reply: service:event-display

It should be also relatively easy to write converter that takes old YAML and convert to new URI-based YAMl (and vice versa)

[n3wscott]

We will track this adoption in a doc https://docs.google.com/document/d/1EAf9NLwDX3c9DS8lr6KxjtgRXUmkFGIcG6DeAYd5UHI/edit#

[vaikas]

For the reasons indicated above, we are going to use the .10 release to provide backwards compatibility, add the new agreed upon Ref field for all the reasons mentioned, and then get rid of the Deprecated fields (necessary for backwards compat) as.11 with the eye towards the v1beta1 api shape. I do not want to invent a new way to code up something at this point that will basically go against the k8s conventions for referring to another object. As @grantr mentioned, if we later on want to add more capabilities into the URI field, then we can make that an additive change.
Note that there can be tooling which is going to be built on top of the actual API (kn for example, there's probably others), where things like what you discuss here can (and I'd argue should) be built on and having a clear principled API that conforms and follows k8s apis is more important.

[rhuss]

Note that there can be tooling which is going to be built on top of the actual API (kn for example, there's probably others), where things like what you discuss here can (and I'd argue should) be built on and having a clear principled API that conforms and follows k8s apis is more important.

fwiw this is indeed planned for kn to allow kind of an URL scheme for referencing different types of sinks in a source (same would apply for triggers, subscriptions, etc.) to make it easier for the user to specify those. Challenge here is currently to find out the proper apiVersion in the mapping (but that's a different story :)