Proxy config file format about quilkin HOT 15 CLOSED

The idea with admin would be to group things like logging, metrics related configs that are operational concerns
while the idea with proxy is to have other config for the proxy

That makes perfect sense. SGTM!

Yeah that's the idea that only one is possible at the same time and its enforced by the config. dynamic.filters today is present since we don't do xds filters yet but if we wanted to after adding support we can remove that field for example

Perfect. Love it. 🚢 it. 👍

We might want to add a note to this effect, but this sounds great.

from quilkin.

I would argue for

admin:                   # Optional
  address:
    port: 8080           # Optional - known default

Just so that down the line if we ever wanted to, we could also do something like:

admin:                   # Optional
  address:
    bind: 0.0.0.0 
    port: 8080           # Optional - known default

If we want to also provide what port the admin interface is bound to down the line. (or other such address/socket type arguments).

e.g. Envoy does:

admin:
  access_log_path: /tmp/admin_access.log
  profile_path: /tmp/envoy.prof
  address:
    socket_address: { address: 127.0.0.1, port_value: 9901 }

from quilkin.

Here's a proposal for a proxy configuration:

version: v1alpha1
proxy:
  mode: CLIENT | SERVER  # Optional default SERVER
  id: my-proxy           # Optional default uuid
  port: 7000             # Optional - known default
admin:                   # Optional
  metrics:
    port: 8080           # Optional - known default
filters:                 # Optional - default is an empty filter chain
  - name: quilkin.extensions.filters.debug.v1alpha1.Debug
    config:
      id: debug-1
endpoints:
  - name: server-1       # Optional - likely should be removed instead since there is no XDS equivalent
    address: 127.0.0.1:9091
  - name: server-2
    address: 127.0.0.1:9092
management_servers:      # Control plane XDS servers
  - address: http://127.0.0.1:8082  # This is an object rather than string list since we'll likely need other configs later on.
  - address: http://127.0.0.1:8083

Only one of endpoints or management_servers is allowed.

You can have a minimal working config that logs all packets with:

version: v1alpha1
filters:
  - name: quilkin.extensions.filters.debug.v1alpha1.Debug # Log all messages proxied ...
endpoints:
  - address: 127.0.0.1:9091 # ... to this server.

from quilkin.

This looks good! Also consolidates things nicely between clients and servers.

A couple of thoughts:

• Re: #77 we should version this I think as we go down this path.
• Since it's somewhat related - any thoughts on how to add auth tokens to an endpoint so we can build a routing filter? Is there a metadata section on an endpoint we can take advantage of? Or will this need to be somehow attached to the filter configuration?

from quilkin.

Added an optional version field to the proposal, defaulting to the latest possible - it said apiVersion on #77 though not sure if that was intended? its not clear to me what the api would refer to otherwise.

Re auth tokens, yes we can attach arbitrary metadata to an XDS Endpoint resource which would be useful for it https://www.envoyproxy.io/docs/envoy/latest/api-v2/api/v2/endpoint/endpoint_components.proto#endpoint-lbendpoint

from quilkin.

Added an optional version field to the proposal, defaulting to the latest possible - it said apiVersion on #77 though not sure if that was intended? its not clear to me what the api would refer to otherwise.

I chose apiVersion basically because that's what Kubernetes uses:
https://kubernetes.io/docs/concepts/overview/working-with-objects/kubernetes-objects/#describing-a-kubernetes-object

So just being consistent with that prior art 🤷

Re auth tokens, yes we can attach arbitrary metadata to an XDS Endpoint resource which would be useful for it https://www.envoyproxy.io/docs/envoy/latest/api-v2/api/v2/endpoint/endpoint_components.proto#endpoint-lbendpoint

Awesome. Which key is used in the metadata could also be configurable down the line too. This sounds like it'll work really well 👍

from quilkin.

version: v1alpha1 # Optional default is latest possible version

I think this should be required - mainly because if people drop this, and we increment the default version, that would break things, and that seems like something we should avoid.

Also, just following K8s conventions there too.

from quilkin.

I think k8s specs describe different things than our proxy config though, I can map e.g apps/v1 to a rest endpoint for that object, which is why I'm not clear on what api could mean in our case - the versioning here is covering only the config file format?

Re making it optional, its expected that if no version is specified, things are allowed to break during upgrades. My idea was that since a default version is easily inferred, it shouldn't be required because it simplifies things when getting started and in cases where one would always want to use the latest available and not think about compatibility unless they needed to. Pinning to a version is trivial when its needed otherwise

from quilkin.

I think k8s specs describe different things than our proxy config though, I can map e.g apps/v1 to a rest endpoint for that object, which is why I'm not clear on what api could mean in our case - the versioning here is covering only the config file format?

That's an excellent point. Works for me. version it is.

Re making it optional, its expected that if no version is specified, things are allowed to break during upgrades.

As a user, this would freak me out. I would want a guarantee that if I upgrade, things will continue to work as expected with no code changes on my part -- unless I was explicitly told that it wouldn't, and very far in advance. While we're in alpha stage - sure, we can break things on upgrade, but if we step to v1 and we ever want to go to a v2, if the user is still using a v1 config, things shouldn't break for them.

By defaulting this value, the user may not even know what version previously worked for them until it actually breaks. They then have to go back and try and work it out, while dealing with a breakage at the same time.

So I feel pretty strongly that the version should be required, so its always explicitly available to the end user (and to us as well).

from quilkin.

updated version to be required. I'm thinking of updating the static vs dynamic parts of the config to be independent with e.g

proxy: ...
static:
  filters:  ...               # Optional - default is an empty filter chain
  endpoints: ...
dynamic:
  filters: ...                # Optional - but maybe will be removed instead when we add support for filters config via XDS
  management_servers: ...

the idea is that with the current setup, adding more config fields (e.g clusters) that are either static or dynamic will quickly lead to a situation where it's cumbersome to figure out which fields are compatible with each other and which aren't - this way its harder to make that mistake since we're explictly specifying whether we're working with static or dynamic configuration. WDYT?

from quilkin.

This is starting to look like Envoy's configuration options - https://www.envoyproxy.io/docs/envoy/latest/configuration/overview/examples

Which seems reasonable (they have dynamic_resources we have dynamic - no objections here). A few thoughts:

1. Totally unrelated, but should proxy (or parts of proxy) be under admin ?
2. Question: Can you only have static or dynamic -- i.e. you can't have both at the same time? This only gets confusing for me if you can have both at the same time.

Other than that, I think this makes a lot of sense, as I read through envoy's documentation and (I think) understanding what is happening here. 👍

from quilkin.

Totally unrelated, but should proxy (or parts of proxy) be under admin ?

probably not, perhaps the other way around that admin would be under proxy but not sure if that clarifies much.
The idea with admin would be to group things like logging, metrics related configs that are operational concerns
while the idea with proxy is to have other config for the proxy - not entirely sure of the scope of proxy at the moment e.g where something like session_timeout: 3s would fall somewhere under it

from quilkin.

Question: Can you only have static or dynamic -- i.e. you can't have both at the same time? This only gets confusing for me if you can have both at the same time.

Yeah that's the idea that only one is possible at the same time and its enforced by the config. dynamic.filters today is present since we don't do xds filters yet but if we wanted to after adding support we can remove that field for example

from quilkin.

Just had a quick thought - I was looking at writing a /health endpoint, and figured it would probably run on the same http port as metrics.

admin:                   # Optional
  metrics:
    port: 8080           # Optional - known default

Given above, should this be changed to something like:

admin:                   # Optional
  address:
    port: 8080           # Optional - known default

Since metrics, and health (and maybe other things?) would be served off this port?

from quilkin.

Ah yes, I had that in mind as well - should it be without the address object though? not sure anything other than port will be needed there or we can have it as e.g address: ':8080'

admin:
  port: 8080

from quilkin.