Skip to content

Foreign services in Istio #273

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jan 11, 2018
Merged

Foreign services in Istio #273

merged 14 commits into from
Jan 11, 2018

Conversation

rshriram
Copy link
Member

Allow defining external services using the gateway spec. The location field allows server to be at the edge, edge+internal, or external to the mesh.

Shriram Rajagopalan added 2 commits December 11, 2017 23:52
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Dec 12, 2017
// and a specific internal workload by specifying gateways as well as the
// source match condition in HTTP/TCP routes.
// gateways. The selection condition imposed by this field is independant
// of the sourceLabels match condition in HTTP/TCP routes. It is possible
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a question about this, related to an earlier PR:

//       hosts:
//       - reviews.prod
//       - uk.bookinfo.com
//       - eu.bookinfo.com
//       gateways:
//       - my-gateway

Why there is a need to specify uk.bookinfo.com and eu.bookinfo.com - they appear at the gateway definition. Why the rule isn't:

//       hosts:
//       - reviews.prod
//       gateways:
//       - my-gateway

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm.. thats a good point. The thing is that the gateway has multiple domains, ports, etc. We need to pick a one of them for the route rule.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the semantics of this should be clarified. So currently it means:
If the request arrives from inside the mesh, and has the host reviews.prod or
If the request arrives thru my-gateway, and has the hosts uk.bookinfo.com, eu.bookinfo.com
...

And if the request arrives from inside the mesh and has the host uk.bookinfo.com, does the rule also match?

Maybe we should put the hosts of a gateway specifically into gateways? And mesh hosts inside some special field like mesh?

For example,

//       mesh:
//             hosts:
//                 - reviews.prod
//       gateways:
//            - my-gateway
//                     hosts:
//                            - uk.bookinfo.com
//                            - eu.bookinfo.com
//            - my-other-gateway
//                     hosts:
//                           - us.bookinfo.com
//                           - ca.bookinfo.com

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chatted with @rshriram about this. 'mesh' is really just a special type of a gateway (and in fact there may be others). We already have domain binding in gateways so we just need refinement of gateway bindings to route rules at the host and condition level.

@rshriram
Copy link
Member Author

cc @louiscryan / @ZackButcher ^^^

@vadimeisenbergibm
Copy link
Contributor

/lgtm

@@ -241,6 +269,11 @@ message Server {

// If set to "mutual", the gateway will use standard mTLS authentication.
MUTUAL = 2;

// Applicable only to external services. If set to originate, the
Copy link
Contributor

@ZackButcher ZackButcher Dec 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets rephrase this a bit to make it more clear we're talking about egress. Maybe something like:

If set to originate, the gateway implementation will use TLS on the connection to the service. Applicable only to services whose [Location][istio.routing.v1alpha2.Server.Location] is EXTERNAL to the mesh. TLS to services inside of the mesh is controlled by <link to mesh config>

And I think we want to talk about TLS, not HTTPS, since there's no requirement that the traffic egressing be HTTP. We can set up TLS just fine for TCP traffic too (right?).

Copy link
Contributor

@ZackButcher ZackButcher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@louiscryan
Copy link
Contributor

I think this is the wrong API model for this. As discussed with Shriram I think we would be better off expressing the intent using DestinationRule

Copy link
Contributor

@louiscryan louiscryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs further discussion

@rshriram
Copy link
Member Author

rshriram commented Jan 4, 2018

@louiscryan / @kyessenov Can you dump a simple config example to help me understand?

The way I am envisioning this (not this PR, but in general) is somethign like this:

  1. If you want to use ExternalName services from your orchestration system (e.g., define ExternalName service for apple, assign apple.com to it, and make all apps access apple.default.svc.cluster.local), then you use them just like any other kubernetes/nomad service. No need for egress rules of any sort. I know for a fact that NYTimes, Weather Company, and others are using this model.

  2. If you don't want to or cannot use model 1, and you need to be able to use external DNS names directly in your app (e.g., foo.watsoncloud.bar.net, baz.watsonprivate.zoo.com, and hundreds more domains), you need to either create egress rules per domain or wildcard rules (*.watsoncloud.bar.net:443). The goal here is not just routing, but simply basic stats, traces/logs, etc.

Case (2) is essentially defining a Listener and a HTTP virtual host or a TCP proxy on that listener using original_dst cluster for load balancing (in envoy terms). Such a listener has to exist on every sidecar (i.e., allow by default model). Route rules are insufficient to express a listener and the original_dst cluster load balancing mode. Gateway can describe a listener but not the fact that we need a original_dst cluster. DestinationRule can only describe cluster properties, not the listener.

The approach adopted in this PR is to use the reserved "mesh" Gateway (applicable to all sidecars) to describe the listeners (e.g., google.com:8080), and the HTTPS properties for TLS origination, and then with a simple flag (action: passthrough), we can automatically construct the original_dst cluster. Users can then override cluster properties (change circuit breaker or http2) or route properties (URL rewrite, etc.) using destination rules and route rules respectively. Note that weighted routing/load balancing settings do not apply to Egress rules and will be ignored.

For example

# The following is an example of a gateway abstracting an external
# service. The caller is expected to access the service using
# http://foo.watsoncloud.bar.com:80 and 123.123.123.123:31333

     apiVersion: config.istio.io/v1alpha2
     kind: Gateway
     metadata:
       name: mesh #note the use of reserved name
     spec:
       servers:
       - port:
           number: 443
           protocol: HTTP
         domains:
         - *.watsoncloud.bar.net #there could be 100s of domains under this
         tls:
           mode: originate
         action: passthrough
       - port:
           number: 31333
           protocol: MONGO
         domains:
         - 123.123.123.123/23
         tls:
           mode: originate
         action: passthrough

This is effectively the equivalent of egress rules today. If you wanted more control over whether the request is routed to, you could do something like

     apiVersion: config.istio.io/v1alpha2
     kind: Gateway
     metadata:
       name: mesh #note the use of reserved name
     spec:
       servers:
       - port:
           number: 443
           protocol: HTTP
         domains:
         - *.watsoncloud.bar.net #there could be 100s of domains under this
         tls:
           mode: originate
         action: forward # note the change
       - port:
           number: 31333
           protocol: MONGO
         domains:
         - 123.123.123.123/23
         tls:
           mode: originate
         action: passthrough

and a route rule..

     apiVersion: config.istio.io/v1alpha2
     kind: RouteRule
     metadata:
       name: my-egress-rule
     spec:
       hosts:
       - *.watsoncloud.bar.net
       gateways:
       - mesh
       http:
       - route:
          - destination:
                name: my-egress-proxy.foonamespace
                port:
                   number: 9199 #proxy port

The route rule above will cause traffic to be forwarded to internal egress proxy my-egress-proxy.foonamespace on port 9199 without modifying the hostnames. Future extensions will allow us to do more fancy stuff like tunneling, etc.

There are some nice side effects of the model above, which I won't go into detail..

@louiscryan
Copy link
Contributor

Using a declared resource for gateway:mesh as done above has the same effect as making the equivalent change in DestinationRule or using RouteRule. I'm just not sure the grain of edit is what works for this mode. Editing the gateway:mesh resource would require a fleetwide admin privilege. If I wanted that routing to occur only for a subset of workloads making that call then it would have to be specified in RouteRule and we'd have to deal with precedence.

I like the idea of using gateway:mesh to configure listeners, or at least allowed listeners etc so that's definitely worth exploring separately.

Consider the following alternative

     apiVersion: config.istio.io/v1alpha2
     kind: RouteRule
     metadata:
       name: watsoncloud
     spec:
       hosts:
          *.watsoncloud.bar.net # Captures the same range of hosts
       http: 
          route:
             destination:
                name: egress
                port: 9090
     apiVersion: config.istio.io/v1alpha2
     kind: Gateway
     metadata:
       name: egress
     spec:
       servers:
       - port:
           exposure: mesh | external | both   # hypothetical
           number: 443
           protocol: HTTPS
         domains:
       tls:
           mode: mutual # require the incoming connection to be mutual TLS (inside mesh)
       proxy: 
          mode: transparent | forward
          tls: simple | mutual | passthrough # specify constraint on outgoing traffic

This model has a number of advantages:

  • Can be rolled out incrementally using source selection on the route rule
  • Aligns with the desired units of edit
  • Can be inferred by protocol / port associations
  • Separates constraints on incoming and outgoing TLS

This does imply that a gateway is a referenceable destination so that needs to be thought through

@rshriram
Copy link
Member Author

rshriram commented Jan 4, 2018

It also implies that the gateway (egress) is a separate entity. If we want to materialize that gateway as part of the mesh (or do not want to), we need a way to specify that. I could certainly fold all transparent proxies into the sidecar, but this also implies that a given gateway will be partially implemented in sidecars and others in a separate pod (since this proxy-mode is part of servers, which is an array of listeners).

Second, this also creates some form of confusion for someone who is just looking to set up a hole in the firewall equivalent. is foo.watsoncloud.bar.net:80 going to be tunneled to 9090 and then going to go out? If so, do I have to ensure that the link is protected (due to compliance requirements)? Won't this be an extra unnecessary hop? [ IF we were to tell them that this goes out directly from the sidecar, the question would be, "why specify the port in the route.destination ?" . Unraveling this thread, we would now have to support a special port-less gateway to make the config look sane. Not sure if that is a clean solution].

To your other argument
Editing the gateway:mesh resource would require a fleetwide admin privilege. If I wanted that routing to occur only for a subset of workloads making that call then it would have to be specified in RouteRule and we'd have to deal with precedence.

I thought through this. Communication in side the mesh is of no issue to someone looking to secure the system. But any external communication is strictly monitored and policed. Atleast in the watson cloud, the admins want to know about each and every external service being accessed. So, having a central place to register all these external services is actually a nice thing, IMO.

Trying to allow a source-specific firewall-hole implies that we are fitting some form of policing into the routing rule itself. However, such policing is not robust because the proxy could be bypassed. You still need network policy to block outbound traffic from unwanted sources.

Given that, why bother to do any policing from route rule? If the user wants to restrict the access, they can write network policies to that effect, or better still, forward all traffic to an egress node where they can write mixer policies to the same effect. This keeps routing and enforcement separate.

There is a flexibility argument here: source specific firewall-holes allow teams to work independently, but I am not sure this is that much of an issue in practice. Most of the time, people don't have rbac/restrictions in place. So, editing a global mesh config is not that bad..

@louiscryan
Copy link
Contributor

Gateway is certainly a distinct logical entity even if it may not be a physical one. One of the reasons I added the 'exposure' field was to control the visibility of the ports. If a port was exposed internally that would imply that the gateway was now an addressable destination within the mesh, this was not the case when the port was only exposed externally.

The 'unnecessary hop' is sometimes considered to be a feature. I think do egress via a gateway will be somewhat rare but not unusual. Ensuring the link is protected on both sides of the gateway is why I have two 'tls' entries in the resource now, one for inbound which we already had and one for outbound which we did not (your 'originate' proposal seemed like it was confusing inbound and outbound).

For things that go out directly via the sidecar they can just use the actual destination service as the 'destination'. Port may or may not be required depending on inference rules so I thought it best to show it explicitly. In particular for TCP proxying I'd expect to tunnel many protocols through a single forwarding proxy port.

I agree w.r.t to scoping that capturing egress will largely be an admin function though it may be organizational (namespace) rather than global in some cases. RouteRule can still be used globally in this fashion and has the advantage that it has src selectors so policies can be rolled out gradually. Its not really about changing organizational practice, its more about supporting safe production practices.

@rshriram
Copy link
Member Author

rshriram commented Jan 4, 2018

I agree with the gateway, port exposure, tls stuff. these are natural outcomes of having the dedicated egress proxy and we had all of these in 0.1 when we had istio-egress. Keep in mind that we cannot support the standalone egress gateway today, because of Envoy restrictions. if I access *.google.com from service A, and I reroute this to an Egress Envoy, it will not be able to dynamically resolve the exact host (like maps.google vs plus.google) and route appropriately. The lack of wildcard support was one of the driving reasons behind the egress rule and use of orig_dst_clusters. Nor can we support proxying to TCP services via dedicated gateway (envoy based).

The same wildcard restriction applies for this as well For things that go out directly via the sidecar they can just use the actual destination service as the 'destination'. . Since there can be hundreds of domains behind *.bluemix.net, I cannot write a route rule for each. Curiously, this is the reason I had the "passthrough" option in the route rule :P.

@louiscryan
Copy link
Contributor

Why can't we support the standalone egress gateway? I believe we can:

  • A gateway can have a route rule bound to handle egress when we're not doing original_dest or some other transparent proxying
  • We can use ports to do TCP proxying and we can use the PROXY_PROTOCOL if we want to forward through a single port

We still need the ability to have different traffic policies for *.bluemix.net and *.google.com which translates into having different original_dest Envoy clusters right ? I agree that we need some way to model a dynamically-resolved destination in the API. I agree we can't ask people to denormalize DNS into route rules. DestinationRule seems like the logical place to specify that.

     apiVersion: config.istio.io/v1alpha2
     kind: DestinationRule
     metadata:
       name: allowed-external-dest
     spec:
       name: *.ibm.com # Captures the same range of hosts
       resolution: registry | local_dns # The latter implies use original_dest

@rshriram
Copy link
Member Author

rshriram commented Jan 5, 2018

Its not just that..
In the simplest of all cases, if I want to send to *.bluemix.net with a specific client cert, what do I do? setup a route rule and a gateway with the client cert? (and we now need a way in the gateway to say apply this gateway directly at the sidecar, I don't want the extra hop). This is the problem with the representation you have. Secondly, even if we pick the DestinationRule to do the passthrough thing (which is a bit too verbose for this), the destination under route will be non intuitive..

hosts:
- *.bluemix.net
http:
- route:
   - destination:
         name: *.bluemix.net #this could be shortened with a passthrough option in the route rule itself, but we will still miss TLS configs, special h2/alpn configs that gateway can offer
destinationRule:
name: *.bluemix.net
resolution: passthrough

The problem with the external gateway is the wildcard as you put it. if foo.bluemix.net resolves to 1.1.1.1, the original destination cluster is literally establishing a connection to 1.1.1.1 and sending the request to it. Essentially disables load balancing. IF we decided to forward the call to another envoy, we lose the original destination that was resolved in the caller envoy. And the receiving envoy will have a destination equal to its own IP address.

It doesn't matter whether we put this in Destination rule or gateway. The limitation still exists. Also, Envoy can process incoming proxy protocol requests. But not create proxy protocol requests in the outbound path. These are not insurmountable, just that its not possible as things exist today.

@nmnellis
Copy link

nmnellis commented Jan 5, 2018

I am one of the developers for The Weather Company mentioned by @rshriram above. I am confused about a few points and when i would want to use the proposed gateway.

so normally for an external service, lets say sqs.amazonaws.com i would have to create an egress rule.

apiVersion: config.istio.io/v1alpha2
kind: EgressRule
metadata:
  name: aws-egress-rule
spec:
  destination:
    service: "*.amazonaws.com"
  ports:
    - port: 443
      protocol: https

if i wanted it to be ssl i could still call it from my app as http://sqs.amazonaws.com:443/do/something.

What I am missing is why would i need to involve a "gateway" for anything that does not need route rules(special cases like url rewriting or forwarding)? Like below why would i want to specify something as "passthrough" when its already being passed through.

       - port:
           number: 443
           protocol: HTTP
         domains:
         - *.amazonaws.com
         tls:
           mode: originate
         action: passthrough  # what is this doing?

It would really help me understand the need for the above changes if some use cases could be provided, or when the "gateway" should / shouldn't be used because there are better alternatives.

@louiscryan
Copy link
Contributor

I believe you would specify the client-cert to use in the DestinationRule, seems straightforward enough and aligns with the Envoy model of Cluster

Why do you need the RouteRule in the example you provide? As long as a 'destination' exists and we're not customizing routing the sidecar proxy will deliver that traffic.

Coming back to your eariler comment

If you want to use ExternalName services from your orchestration system (e.g., define ExternalName service for apple, assign apple.com to it, and make all apps access apple.default.svc.cluster.local), then you use them just like any other kubernetes/nomad service. No need for egress rules of any sort. I know for a fact that NYTimes, Weather Company, and others are using this model.

We'd still want the ability to use a client-cert when talking to that service. Unless we put that into routing-rule (which seems like a bad idea) the only way to get that behavior would be to declare the TLS requirement in DestinationRule

FWIW - I think it's worth moving this discussion into a doc so we can capture the variations in a more structured way and iterate from there.

repeated string domains = 2;

message TLSOptions {
// If set to true, the load balancer will send a 302 redirect for all
// http connections, asking the clients to use HTTPS.
bool https_redirect = 1;

// TLS modes enforced by the gateway
// TLS modes enforced by the server
enum TLSmode {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we use strict CamelName convention?

// HTTP. The sidecar will perform TLS origination on behalf of the
// application.
//
// NOTE 1: that domains denoted by foreign services will be visible at all
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if you need to say this. I would simply state that 'foreign services are treated like other services in the registry and subject to the same visibility and scoping rules'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also need to cover precedence here if there is an equivalent name in the service registry and foreign. General problem in service registries anyway when there are multiple sources of truth being merged

// NOTE 1: that domains denoted by foreign services will be visible at all
// sidecars in the mesh.
//
// NOTE 2: There can be ONLY ONE ForeignServices configuration for the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why ? That seems odd. Why couldn't this be per namespace?

It might be OK to start with just one and relax this constraint later.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coz there is no such thing as pods per namespace in other environments. Api server may have namespaces but that’s just for organization.

// protocol: HTTPS
// resolution: DNS
//
// Route rules can be applied to services described in the ForeignServices
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an example of a route-rule capturing a domain, I think most folks would be interested in the example of using a foreign service as a destination. Rules reference destination with concrete names not wildcards so I think we need a 'name' above

repeated Service services = 1;
}

// Service describes the properties of the external service that should be made accessible from within the mesh. For example,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The seems a little redundant with the earlier example, what are you trying to show here ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is supposed to be redundant. An example per message so that the resulting doc is more readable, and self contained. Trying to work around the oddities of the doc gen.

message Service {
// REQUIRED. The destination address of the external service. Could be a
// DNS name with wildcard prefix or a CIDR prefix. Note that the hosts
// field applies to both HTTP and TCP services.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/HTTP and TCP/ all protocols

// - number: 27018
// protocol: MONGO
// name: mongo
// resolution: passthrough
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in enum below

repeated string domains = 2;

message TLSOptions {
// If set to true, the load balancer will send a 302 redirect for all
// http connections, asking the clients to use HTTPS.
bool https_redirect = 1;

// TLS modes enforced by the gateway
// TLS modes enforced by the server
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure these changes are necessary. Would just try to doc somewhere in gateway that its a way to 'define a proxy' and then use the term 'gateway' consistently

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the name Gateway is a little too general? Would things be easier to understand if it was named EdgeProxy instead?

Btw, this relates back to my comment about the special gateway reserved name mesh (#276 (comment)). I still think that this is a confusing design, especially given the fact that the Gateway doc says: "Gateway describes a load balancer operating at the edge of the mesh ...".

Since a gateway is really just a proxy for ingress/egress, ignoring the (IMO) kludgy mesh one, then calling it EdgeProxy instead of Gateway will require one less Istio term to be clearly defined and learned.

@@ -0,0 +1,160 @@
// Copyright 2017 Istio Authors
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2018


// The protocol exposed on the port.
// MUST BE one of HTTP|HTTPS|GRPC|HTTP2|MONGO|TCP.
string protocol = 2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this match any of the istio attribute?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know. But this is a way to "declare", and not match. It is in line with standard kubernetes port spec.

// mechanisms such as IP table REDIRECT/ eBPF. After performing any
// routing related transformations, the proxy will forward the
// connection to the IP address to which the connection was bound.
LOCAL = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default is error prone. Suggest to use RESOLUTION_UNSPECIFIED = 0 and makes it an error. Force choice is much safer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that this type of destination (original_dst_cluster) is very hard for an end user to understand. And 90% of time, people should not care about the resolution mode. So, a default is needed, otherwise, we have to explain envoy architecture, our packet trapping, and the rationale behind the original_dst_cluster, so that the user can explicitly pick this as default. Increases verbosity and also has bad user experience, (2 configs to setup a very simple thing).

// a gateway domain is a suffix of one of the route rule domains.
// A list of domains exposed by this server. While typically applicable
// to HTTP services, it can also be used for TCP services using TLS with
// SNI. Standard DNS wildcard prefix syntax is permitted.
repeated string domains = 2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What the difference between hosts and domains?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I leave that to @louiscryan to answer :). He was particular about using domains here and not hosts.

// rewrite:
// uri: /barcatalog
//
message ForeignServices {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't a plural resource name a bad idea? How do you refer to multiple ForeignServices? ForeignServicess?
To be consistent with RouteRule and DestinationRule, shouldn't it be named <something>Rule? What about EgressRule, which contains a list of ForeignService?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its not really a rule though, its declaring an entry in the service registry. I can live with ForeignService and remove the internal array as long as we allow repeated hosts. Would also need to relax the 'there can be only one constraint'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this message is needed. Why don't we define ForeignService and let the user to repeat it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should revisit the cardinality thing. Ill open an issue


package istio.routing.v1alpha2;

// Foreign services describe the properties of services outside Istio. It
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'describe the endpoints, ports and protocols of'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree.

// ports:
// - number: 443
// name: https
// protocol: HTTPS #application originates TLS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is at odds with the description above. This is not the application originating TLS, this is the service expecting it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also can elide 'protocol' in this example as we will infer for name/port. Only need protocol if we need to override IANA defaults (should be in doc for field in proto)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name is optional.. and people may not name the port properly. And things like mongo/redis - do they have iana entries?

//
// For example, the following foreign services configuration describes the
// set of services at https://*.foo.com:443 and http://example.bar.com:80. Calls
// to *.foo.com are treated as opaque TCP calls as the application
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is what we want. We want this to be magical. If the application originates TLS then the sidecar will pass it through, if it originates plaintext the sidecar will upgrade it for them. TLS sniffing will allow us to do that.

If we have that behavior we don't need the destination rule, we can assume 'simple' for the endpoint. We only need the dest rule if we need some special behavior, e.g. mutual tls

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.. but we can't support that behavior today. Until TLS sniffing is implemented. I am looking at the next 2 months, where this doc will get published and people will begin using this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was chatting with @PiotrSikora about this a bit. I think what we would ask folks to do is:

  • declare two ports on the foreign service, http & https
  • write a route rule which captures the host & http and sends it to the https one

They would do this even if the public service doesn't really have an http port.

Would be good to show that as a concrete and useful example here I think

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and yes we can ignore sniffing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It won't work with orig dst clusters. You cannot change the port :(.
Secondly, even if we make it work, the associated routing rule does not work, if the destination has wildcards.

hosts:
- *.foo.com
http:
- match:
   - port:
          number: 80
   route:
   - destination:
          name: *.foo.com
          port:
              number: 443

It won't make sense when you look at the config. We could probably tack a "passthrough" field in the destination.

// resolution: local
//
message Service {
// REQUIRED. The destination address of the external service. Could be a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are not always the destination addresses, These are the host (service) names

};

// DNS resolution mode for the hosts.
Resolution resolution = 3;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still missing endpoints and labels...

@rshriram rshriram merged commit ba8e7ca into master Jan 11, 2018

// Different ways of discovering the IP addresses associated with the
// service.
enum Discovery {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename type to 'Resolution'

// Service discovery mode for the hosts.
Discovery resolution = 3;

// Endpoint defines a network address (IP:port or hostname:port)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would note here that if endpoints are not specified then in the case of static or dns the host names will be resolved as the addresses. We can even support *.example.com as a host name by resolving 'example.com'

// qualified domain name without wildcards).
string address = 1;

// The port on which the endpoint is listening for network connections.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Document here that this is only required if the endpoint ports vary from the service ports.

@frankbu
Copy link
Contributor

frankbu commented Jan 11, 2018

Why has this been merged already? I thought we were in general agreement that the resource name ForeignServices should change (not be plural). Also the latest version of the comment at the very top of the file makes no sense:

Service registry entries describe describe the endpoints, ports and

Is that supposed to be something like:

A ForeignService is a service registry entry that describes the endpoints, ...

// The port on which the endpoint is listening for network connections.
Port port = 2;

// One or more labels associated with the endpoint.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note here that these labels can be used to define subsets just as for entries in the primary service registry.

string address = 1;

// The port on which the endpoint is listening for network connections.
Port port = 2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be plural

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is only one port per endpoint, it essentially serves as a mapping from service port to endpoint port.

@kyessenov
Copy link
Contributor

kyessenov commented Jan 11, 2018 via email

incfly pushed a commit to incfly/api that referenced this pull request Jun 13, 2018
nacx pushed a commit to nacx/api that referenced this pull request Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants