Allow --includeIPRanges to be passed to Istio

[olhtbr]

Extend the istio field in service.yaml to a map, which supports the --includeIPRanges option.

[plombardi89]

Thanks for the initial contribution @olhtbr. I feel like we want this to be a bit more generic? @rhs and @olhtbr I was looking at spawning issue GH-121 and it seems like the ask is really the ability to override a a value in service.yaml from the CLI.

I'm a bit weary of adding vendor specific CLI extensions for single use-case overrides. It seems we would be better off having a way to pass in values via the CLI that override the configuration values. For example, -d 'istio.includeIPRanges=["10.0.0.0/16", "11.0.0.0/16"]' would configure the equivalent YAML:

istio:
  enable: true
  includeIPRanges:
    - 10.0.0.0/16
    - 11.0.0.0/16

Thoughts @rhs ?

[plombardi89]

FWIW. We've stumbled across the desire to be able to do CLI overrides as well in the past. Also there's an open question as to how to handle a project with multiple service.yaml files.

[rhs]

I may be missing some context, but I wasn't thinking of this as a temporary single use override, but rather permanent configuration info about the service, i.e. it always needs to be deployed with those includeIPRanges or it won't work, in which case I think you really want the info in service.yaml rather than on the command line.

[olhtbr]

@rhs the original context for me is exactly that. My service needs to connect to an external HTTPS API, so I always want to deploy it without envoy intercepting this external traffic.

[rhs]

Just chatted with @plombardi89 offline, but want to summarize here. The point he was making is that the argument to --includeIPRanges may need to be different for differently configured clusters. This makes hardcoding it in service.yaml potentially brittle, e.g. if you need the same source code to deploy to multiple clusters that happen to be configured to use different ip ranges for their services.

@olhtbr This may not matter in your situation, but wanted to make you aware of that as I believe it is a valid point.

My thought is to stick with the PR as is, and then if this is a problem we can add a mechanism to make cluster level configuration available to service.yaml as template variables so your service could use a variable rather than hardcoding the ip range.

[rhs]

I've merged a slightly modified version of this PR into master. The only real difference was to add backwards compatibility with the old istio syntax and add a few more tests.