Pubsub Channel can adopt K8s Service or VirtualService owned by another resource/user

[grantr]

From #618 (comment)

Expected Behavior

K8s Services and VirtualServices owned or created by other actors never conflict with Channel Services.

Actual Behavior

At least in the GCP Pubsub implementation, the Channel controller adopts a Service with the expected name even if that Service is not owned by the Channel (it logs a warning if this happens). The adopted Service is set as the address of the Channel.

The effect is that all messages sent to that Channel will be rejected or lost, and the actual owner of the Channel will start receiving invalid requests. Depending on relative throughput and upstream retry policies, this might cause a DoS of the Service endpoints.

The VirtualService has a similar but worse problem in that it's spec is updated to be what the Channel controller expects. So in addition to the above, the routing configuration of the owning endpoints will be altered.

Steps to Reproduce the Problem

I haven't tried this, but I imagine the repro is as simple as:

1. Create a Knative Service named foo-channel.
2. Create a Channel named foo in the same namespace as the Knative Service.
3. Send a message to that Channel. The Knative Service will receive that message instead of the channel dispatcher.

Additional Info

Possible solutions include using generated names for Channel-owned objects (this may not be possible according to @Harwayne) or giving the Channel a terminal error state when encountering a resource that isn't owned by the Channel.

[Harwayne]

I believe the steps to reproduce this problem won't create the problem you expect.

My understanding of how the requests work:

1. In the user code, an HTTP request is made to the Channel's DNS name.
   • This is injected via the Channel's status.address.
   • Currently this is based on the Channel's K8s Service. e.g. 'qux-1-channel.default.svc.cluster.local'
   • Where this request is routed isn't important, as long as the user code is happy to send the request.
2. The request is intercepted by the Istio sidecar. Which notices the host header in the request and sees that it matches a VirtualService's spec.hosts entry. So it ignores where the request was originally sent and instead routes it to the destination K8s Service. It also rewrites the host header to the 'Channel DNS' (e.g. 'qux-1.default.channels.cluster.local').
3. The request is sent to a Pod on the destination K8s Service. Which uses the host header to determine which Channel this request is being made on.

So, any foo-channel K8s Service will allow it to work. However, the new (or altered) VirtualService will cause requests made from inside the Istio mesh (which I think includes Knative Serving) to route to the Channel's dispatcher, rather than the Knative Service. We intend to make the same sort of update spec if not what is expected request to the K8s Service soon too, which would break non-Istio mesh requests too (by sending them to the Channel's dispatcher, rather than the Knative Service).

We can easily change VirtualService to a generated name. But it doesn't really help.

We could also change the K8s Service to a generated name, as long as we update the Channel's status.address to the correct generated DNS. Note that making this change would cause a blip in Channel's when switching from the 'normal' name to the generated one while all Subscriptions get the new updated value and the Istio mesh updates. We might be able to do it without any outage by using label selectors to find the Service, which we can make match the existing Services (so that existing Channel continue to use the nicely named Service while all news ones use generated names).