Decouple eventing from Istio

[scothis]

Replace the Istio VirtualService used by Channels to send traffic to a
Bus.

The VirtualService did two things for us:

1. connected the Channel's Service to the Bus's Service
2. set the request's Host header to indicate the channel and namespace

An ExternalName Service can forward traffic effectively to the Bus's
Service, which handles item 1. Item 2 is addressed by dropping support
for short hostnames that do not include the namespace.

For a Channel named 'aloha' in the default namespace, an event producer
can make a request using any of these host names:

• aloha-channel.default
• aloha-channel.default.svc.cluster.local

While short names like aloha-channel will resolve to the correct bus,
ClusterBuses must be able to resolve the namespace in addition to the
channel since different namespaces can have channels with the same name.
A 404 will be returned if the Host header in the request does not
contain the namespace. Namespaced buses do not strictly require the
namespace to be included in the Host to resolve the correct Channel, but
will fail with a 404 for runtime consistency with ClusterBuses. We may
relax this requirement in the future.

Fixes #294

[scothis]

/test pull-knative-eventing-integration-tests

[scothis]

/test pull-knative-eventing-integration-tests

[scothis]

The e2e tests are failing because Istio doesn't recognize that the request from the receive adapter to the channel should be made on the mesh.

This could be solved by adding a ServiceEntry like:

apiVersion: networking.istio.io/v1alpha3
kind: ServiceEntry
metadata:
  name: bus-channels
spec:
  hosts:
  - "*-channel.e2etestfn.svc.cluster.local"
  ports:
  - number: 80
    name: http
    protocol: HTTP
  location: MESH_INTERNAL

...but this would need to be specified for every namespace that will host a channel that also originates a request to that channel from a pod that is Istio injected. Creating a less-than-ideal user experiance.

Another option would be to follow the pattern in serving by exposing buses via Ingress. I'm not a fan of this approach as buses should generally be an internal facing concept and not be exposed to the world.

The better approach may be to hold this PR and file issues with Istio to:

• support multiple wildcards for host names (like *-channel.*.svc.cluster.local)
• detect an intra-cluster ExternalName service and automatically use the mesh.

cc @tcnghia

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: scothis
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: evankanderson

If they are not already assigned, you can assign the PR to them by writing /assign @evankanderson in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[knative-metrics-robot]

The following is the coverage report on pkg/.
Say /test pull-knative-eventing-go-coverage to run the coverage report again

File	Old Coverage	New Coverage	Delta
pkg/buses/stub/main.go	5.4%	5.0%	-0.4

[scothis]

/assign @mattmoor
/assign @evankanderson
/assign @tcnghia

This PR is ready for discussion, but not complete.

I'm not happy with where it ended up in terms of requiring a ServiceEntry to be created in every namespace, but it seems like the best path for the moment. We can either:

• keep the dependency on Istio
• use the eventing-controller to create ServiceEntry records (preserving a dependency in the controller on Istio, although smaller)
• come up with an alternate approach to connect traffic for a Channel to a Bus

Serving is able to work around these issues by using Ingress. Channels, however, should not be exposed publicly. Perhaps there is way to use Ingress but limit it to traffic already in the cluster??

[tcnghia]

Istio has a Gateway of type internal load balancer, which is turned off by default (https://github.com/istio/istio/blob/8daa232df6ef7cdbb2987de1187255b14e3c3989/install/kubernetes/helm/istio/values.yaml#L271). I am looking for ways of how we can use something similar to expose internal Routes.

Another way is using RBAC to restrict ingress traffic, but I believe that would require mTLS, which is a deeper Istio integration. @ZhiminXiang

[scothis]

/test pull-knative-eventing-integration-tests

[scothis]

Rebased on top of master after the bus refactor

[n3wscott]

I don't know much about istio, but from what I understand, this looks reasonable and also inline with what serving is going to be moving to with the serving owned virtual service. ?

It looks like the mesh is opening internally for any traffic on :80. The downside to this PR I think means that we are giving up on the mesh getting to help auth control between inner-cluster components. (which we are not using at the moment but might in the future. (but we could also enable it via sidecars?))

/lgtm

But someone who really understands or usage of istio needs to review and approve.

[knative-prow-robot]

New changes are detected. LGTM label has been removed.

[scothis]

/close

There isn't enough here to be worthy since it's going away.

[knative-prow-robot]

@scothis: Closing this PR.

Details

In response to this:

/close

There isn't enough here to be worthy since it's going away.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.