Introduce ingress rule #120
Conversation
proxy/v1/config/ingress_rule.proto
Outdated
| // proxy serves as an ingress proxy for the entire mesh, but can also be | ||
| // addressed from inside the mesh. Each ingress rule defines a destination | ||
| // service and port. Rules that do not resolve to a service or a port in the | ||
| // mesh are ignored. |
There was a problem hiding this comment.
should we change "are ignored" to "should be ignored"?
proxy/v1/config/ingress_rule.proto
Outdated
| // precedence is unspecified. | ||
| int32 precedence = 4; | ||
|
|
||
| // Match condtions to be satisfied for the ingress rule to be |
There was a problem hiding this comment.
s/condtions/conditions/
proxy/v1/config/ingress_rule.proto
Outdated
| int32 port = 2; | ||
|
|
||
| // Optional TLS secret path to apply server-side TLS context on the port. | ||
| string tls_secret = 3; |
There was a problem hiding this comment.
Does path here mean filesystem path or reference to a secret?
There was a problem hiding this comment.
This is very vague. For a generic ingress rule, TLS secret would have a lot of things, such as CA certs, private keys, etc etc. The above case seems to imply K8S secrets, in which case, this ingress becomes specific to K8S.
There was a problem hiding this comment.
it's meant to be vague since Istio doesn't have secret store API. I'm not going to design an API in this tiny commit.
There was a problem hiding this comment.
Need examples. At a high-level, I am not convinced about the need for a separate ingress_rule object. Is this for use internally in the code? or is this going to be exposed in the Docs to end users, such that they would write route-rule, and ingress-rule ?
If its just to codify the interfaces in the code, then I don't understand the need for a proto.
Is this for VMs? If so, then the frontend load balancer stuff requires more thought than the k8s ingress type stuff. We need DNS names, certs, and a host of other things that are applicable to only the frontend load balancer.
proxy/v1/config/ingress_rule.proto
Outdated
|
|
||
| package istio.proxy.v1.config; | ||
|
|
||
| // Ingress rules are routing rules applied to the mediating middle proxy. This |
There was a problem hiding this comment.
I would say, "applied to the Pool of frontend load balancers that receive traffic for the entire mesh. The load balancer can also be addressed directly from within the cluster if needed".
There was a problem hiding this comment.
We also need to provide justification for why Ingress rules are different from normal route rules. And how they relate to ingress rules in K8S. I don't think Mesos or Swarm or CF has any notion of Ingress Rule.
There was a problem hiding this comment.
Done regarding the pool specifically for ingress.
| // The routing rules for the destination service are applied at the ingress | ||
| // proxy. That means the routing rule match conditions are composed and its | ||
| // actions are enforced. The traffic splitting for the destination service is | ||
| // also effective. |
There was a problem hiding this comment.
We need an example of IngressRule here. PTAL at the RouteRule for reference.
There was a problem hiding this comment.
This is not even alpha. Once it stabilizes, we'll get examples.
proxy/v1/config/ingress_rule.proto
Outdated
| int32 port = 2; | ||
|
|
||
| // Optional TLS secret path to apply server-side TLS context on the port. | ||
| string tls_secret = 3; |
There was a problem hiding this comment.
This is very vague. For a generic ingress rule, TLS secret would have a lot of things, such as CA certs, private keys, etc etc. The above case seems to imply K8S secrets, in which case, this ingress becomes specific to K8S.
| int32 destination_port = 7; | ||
|
|
||
| // Identifies the destination service port by name | ||
| string destination_port_name = 8; |
There was a problem hiding this comment.
Why do we need to worry about port_name here? This will be cause for confusion.
There was a problem hiding this comment.
Because you do port translation here unlike route rule.
There was a problem hiding this comment.
In k8s, ports are can be called by name.
|
Placed a warning that this API is experimental. |
Signed-off-by: Shriram Rajagopalan <[email protected]>
This definition captures the effective use of the route rules in the ingress controller in Pilot. The route rules fields that are missing are simply ignored by the implementation. Hoping to get this merged so we can transition to one-proto-per-type in the config system.