config: sync with recent API changes in bootstrap and TLS.#1548
config: sync with recent API changes in bootstrap and TLS.#1548mattklein123 merged 2 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
|
@mattklein123 Not sure what we want to do with renamed fields in the long-term. Should we leave the old names in JSON and use the new names only with YAML / Proto? Should we rename them in JSON, or maybe support both there? |
|
Sigh, macOS test failed on |
|
That test is flakey on OS X. I recommend ignoring it for now. I'll look at the test tomorrow. |
|
@PiotrSikora I would not bother renaming in v1 JSON. I don't think it's worth it personally. For things that are just straight renames, we can use the new names in code, and in v2 config. For things that are new functionality, we can use judgement in terms of whether to bother supporting in v1 or not. For example, I don't think we ever have to support multiple client certs in v1. However, I think we should support SNI in v1 because it's such a highly requested feature. |
| void ClusterManagerImpl::initializeClustersFromV2Proto(const envoy::api::v2::Bootstrap& bootstrap) { | ||
| for (const auto& cluster : bootstrap.bootstrap_clusters()) { | ||
| loadCluster(cluster, false); | ||
| if (bootstrap.has_static_resources()) { |
There was a problem hiding this comment.
nit: I think @htuch has only been doing has* when it's really necessary. Here I think you can just call static_resources() to construct a default, the clusters() will just return empty list.
| // We can now potentially create the CDS API once the backing cluster exists. | ||
| if (bootstrap.has_cds_config()) { | ||
| cds_api_ = factory_.createCds(bootstrap.cds_config(), sds_config_, *this); | ||
| if (bootstrap.has_dynamic_resources() && bootstrap.dynamic_resources().has_cds_config()) { |
There was a problem hiding this comment.
nit: same here with first clause. Will stop commenting now for other things. @htuch is out right now but looking at other code I think this is what he has been doing. You can also see: https://github.com/lyft/envoy/pull/1521/files for some of the code he started to write. (That change needs mega merging so might as well just continue with this one since it's much smaller).
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
No description provided.