Few small updates to the mesh config API#1194
Conversation
|
Happy with any name - whatever name is selected will also be used in the inject template for consistency. |
mesh/v1alpha1/config.proto
Outdated
| // Unix Domain Socket through which Envoy communicates with NodeAgent SDS to get key/cert for mTLS. | ||
| // Use secret-mount files instead of SDS if set to empty. | ||
| string sds_uds_path = 20; | ||
| // @deprecated - istiod agent will detect and send the path to envoy. |
There was a problem hiding this comment.
s/istiod agent/istio-agent
There was a problem hiding this comment.
Do not add docs here. It will show up in webpage. Just add a $hide_from_docs
mesh/v1alpha1/proxy.proto
Outdated
|
|
||
| // Additional env variables to inject into sidecar. | ||
| // Names starting with ISTIO_META_ will be included in the generated bootstrap and sent to the XDS server. | ||
| map<string,string> meta = 24; |
There was a problem hiding this comment.
name is a little opaque, 'sidecar_env' ?
I don't have a strong opinion about this, otherwise. Shriram / Lin are likely better TOC reviewers than I.
There was a problem hiding this comment.
sidecar_env sounds good - unless gRCP expects to use it as well.
Things like CA_ADDR will go here, might be relevant for sidecar-less.
env ? It'll go under container env: section
There was a problem hiding this comment.
To be clear: this is intended for break glass, vendor or advanced features - I will add caAddr as a top-level setting.
There was a problem hiding this comment.
Proxy config is processed by the pilot agent before generating envoy bootstrap. It seems confusing to specify env vars here that will be consumed by platform (added to process env var) before pilot agent starts.
There was a problem hiding this comment.
Does a user really care how it's used? It seems like more of an implementation detail - to the user it's all just configuring the proxy?
There was a problem hiding this comment.
The 'default' proxy has all the (default) settings for the sidecar. For now we do indeed do a crazy complicated thing - but the plan (according to the istiod doc) is to simplify things a bit.
One option is to have Pilot return the template and mesh.yaml to agent, so agent doesn't need the crazy stuff. Longer term Pilot can do the rendering of the bootstrap - and return a final bootstrap, plus some env variables needed by pilot-agent.
But in the end - logically this is config for the proxy, and seems reasonable to keep it here. DiscoveryAddress is the same.
There was a problem hiding this comment.
let us call this proxy_metadata as it applies to gateways as well.
There was a problem hiding this comment.
Can we call it proxy_env ? Metadata is a subset - not all the env are passed to envoy bootstrap and node metadata inside. Some apply to pilot-agent/istio-agent itself.
Not a big deal - I'll change it to proxy_metadata for now.
There was a problem hiding this comment.
@rshriram - to be clear, the gateway can't easily support it (right now), it's used in injection.
We are discussing adding a handler in Pilot to return the 'active' mesh config, and istio-agent using that - either in gateway or sidecar - instead of having the env and other options handled by injector. But in terms of implementation it'll take a bit longer, the use in injection is simpler.
|
PTAL. I have few other related changes so we can have injection without values.yaml and helm - trying to use small PRs to avoid problems... |
|
PTAL - I keep spending time resolving conflicts. |
|
lgtm after renaming to proxy_metadata |
Added a mechanism to specify the 'env' that gets injected - to be used instead of values.yaml.