Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config: turning down v1 config #4678

Merged
merged 5 commits into from
Oct 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Version history
1.9.0 (pending)
===============
* admin: added support for displaying subject alternate names in :ref:`certs<operations_admin_interface_certs>` end point.
* config: removed support for the v1 API.
* fault: removed integer percentage support.
* router: added ability to configure arbitrary :ref:`retriable status codes. <envoy_api_field_route.RouteAction.RetryPolicy.retriable_status_codes>`
* router: added ability to set attempt count in upstream requests, see :ref:`virtual host's include request
Expand All @@ -28,7 +29,7 @@ Version history
health check/weight/metadata updates within the given duration.
* config: regex validation added to limit to a maximum of 1024 characters.
* config: v1 disabled by default. v1 support remains available until October via flipping --v2-config-only=false.
* config: v1 disabled by default. v1 support remains available until October via setting :option:`--allow-deprecated-v1-api`.
* config: v1 disabled by default. v1 support remains available until October via deprecated flag --allow-deprecated-v1-api.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should modify earlier release notes, this is a bit like rewriting history. Ideally once we release, they are immutable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately I had to - the :option: tag was breaking the docs build now that the flag didn't exist.
I figured as long as I was removing the tagging I should explain why.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So be it.

* config: Fixed stat inconsistency between xDS and ADS implementation. :ref:`update_failure <config_cluster_manager_cds>`
stat is incremented in case of network failure and :ref:`update_rejected <config_cluster_manager_cds>` stat is incremented
in case of schema/validation error.
Expand Down
17 changes: 3 additions & 14 deletions docs/root/operations/cli.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,11 @@ following are the command line options that Envoy supports.

.. option:: -c <path string>, --config-path <path string>

*(optional)* The path to the v1 or v2 :ref:`JSON/YAML/proto3 configuration
*(optional)* The path to the v2 :ref:`JSON/YAML/proto3 configuration
file <config>`. If this flag is missing, :option:`--config-yaml` is required.
This will be parsed as a :ref:`v2 bootstrap configuration file
<config_overview_v2_bootstrap>`. On failure, if :option:`--allow-deprecated-v1-api`,
is set, it will be considered as a :ref:`v1 JSON
configuration file <config_overview_v1>`. For v2 configuration files, valid
extensions are ``.json``, ``.yaml``, ``.pb`` and ``.pb_text``, which indicate
<config_overview_v2_bootstrap>`.
Valid extensions are ``.json``, ``.yaml``, ``.pb`` and ``.pb_text``, which indicate
JSON, YAML, `binary proto3
<https://developers.google.com/protocol-buffers/docs/encoding>`_ and `text
proto3
Expand All @@ -26,7 +24,6 @@ following are the command line options that Envoy supports.
*(optional)* The YAML string for a v2 bootstrap configuration. If :option:`--config-path` is also set,
the values in this YAML string will override and merge with the bootstrap loaded from :option:`--config-path`.
Because YAML is a superset of JSON, a JSON string may also be passed to :option:`--config-yaml`.
:option:`--config-yaml` is not compatible with bootstrap v1.

Example overriding the node id on the command line:

Expand All @@ -37,14 +34,6 @@ following are the command line options that Envoy supports.
*(deprecated)* This flag used to allow opting into only using a
:ref:`v2 bootstrap configuration file <config_overview_v2_bootstrap>`. This is now set by default.

.. option:: --allow-deprecated-v1-api

*(optional)* This flag determines whether the configuration file should only
be parsed as a :ref:`v2 bootstrap configuration file
<config_overview_v2_bootstrap>`. If specified when a v2 bootstrap
config parse fails, a second attempt to parse the config as a :ref:`v1 JSON
configuration file <config_overview_v1>` will be made.

.. option:: --mode <string>

*(optional)* One of the operating modes for Envoy:
Expand Down
4 changes: 0 additions & 4 deletions source/server/options_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv,
// Deprecated and unused.
TCLAP::SwitchArg v2_config_only("", "v2-config-only", "deprecated", cmd, true);

TCLAP::SwitchArg allow_v1_config("", "allow-deprecated-v1-api", "allow use of legacy v1 config",
cmd, false);

TCLAP::SwitchArg allow_unknown_fields("", "allow-unknown-fields",
"allow unknown fields in the configuration", cmd, false);

Expand Down Expand Up @@ -194,7 +191,6 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv,
concurrency_ = std::max(1U, concurrency.getValue());
config_path_ = config_path.getValue();
config_yaml_ = config_yaml.getValue();
v2_config_only_ = !allow_v1_config.getValue();
if (allow_unknown_fields.getValue()) {
MessageUtil::proto_unknown_fields = ProtoUnknownFieldsMode::Allow;
}
Expand Down
4 changes: 1 addition & 3 deletions source/server/options_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ class OptionsImpl : public Server::Options {
void setConcurrency(uint32_t concurrency) { concurrency_ = concurrency; }
void setConfigPath(const std::string& config_path) { config_path_ = config_path; }
void setConfigYaml(const std::string& config_yaml) { config_yaml_ = config_yaml; }
void setV2ConfigOnly(bool v2_config_only) { v2_config_only_ = v2_config_only; }
void setAdminAddressPath(const std::string& admin_address_path) {
admin_address_path_ = admin_address_path;
}
Expand Down Expand Up @@ -73,7 +72,7 @@ class OptionsImpl : public Server::Options {
uint32_t concurrency() const override { return concurrency_; }
const std::string& configPath() const override { return config_path_; }
const std::string& configYaml() const override { return config_yaml_; }
bool v2ConfigOnly() const override { return v2_config_only_; }
bool v2ConfigOnly() const override { return true; }
const std::string& adminAddressPath() const override { return admin_address_path_; }
Network::Address::IpVersion localAddressIpVersion() const override {
return local_address_ip_version_;
Expand Down Expand Up @@ -107,7 +106,6 @@ class OptionsImpl : public Server::Options {
uint32_t concurrency_;
std::string config_path_;
std::string config_yaml_;
bool v2_config_only_;
std::string admin_address_path_;
Network::Address::IpVersion local_address_ip_version_;
spdlog::level::level_enum log_level_;
Expand Down
14 changes: 0 additions & 14 deletions test/server/options_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,6 @@ TEST_F(OptionsImplTest, InvalidCommandLine) {
"Couldn't find match for argument");
}

TEST_F(OptionsImplTest, v1Allowed) {
std::unique_ptr<OptionsImpl> options = createOptionsImpl(
"envoy --mode validate --concurrency 2 -c hello --admin-address-path path --restart-epoch 1 "
"--local-address-ip-version v6 -l info --service-cluster cluster --service-node node "
"--service-zone zone --file-flush-interval-msec 9000 --drain-time-s 60 --log-format [%v] "
"--parent-shutdown-time-s 90 --log-path /foo/bar --allow-deprecated-v1-api "
"--disable-hot-restart");
EXPECT_EQ(Server::Mode::Validate, options->mode());
EXPECT_FALSE(options->v2ConfigOnly());
}

TEST_F(OptionsImplTest, v1Disallowed) {
std::unique_ptr<OptionsImpl> options = createOptionsImpl(
"envoy --mode validate --concurrency 2 -c hello --admin-address-path path --restart-epoch 1 "
Expand Down Expand Up @@ -111,7 +100,6 @@ TEST_F(OptionsImplTest, All) {

TEST_F(OptionsImplTest, SetAll) {
std::unique_ptr<OptionsImpl> options = createOptionsImpl("envoy -c hello");
bool v2_config_only = options->v2ConfigOnly();
bool hot_restart_disabled = options->hotRestartDisabled();
Stats::StatsOptionsImpl stats_options;
stats_options.max_obj_name_length_ = 54321;
Expand All @@ -121,7 +109,6 @@ TEST_F(OptionsImplTest, SetAll) {
options->setConcurrency(42);
options->setConfigPath("foo");
options->setConfigYaml("bogus:");
options->setV2ConfigOnly(!options->v2ConfigOnly());
options->setAdminAddressPath("path");
options->setLocalAddressIpVersion(Network::Address::IpVersion::v6);
options->setDrainTime(std::chrono::seconds(42));
Expand All @@ -143,7 +130,6 @@ TEST_F(OptionsImplTest, SetAll) {
EXPECT_EQ(42U, options->concurrency());
EXPECT_EQ("foo", options->configPath());
EXPECT_EQ("bogus:", options->configYaml());
EXPECT_EQ(!v2_config_only, options->v2ConfigOnly());
EXPECT_EQ("path", options->adminAddressPath());
EXPECT_EQ(Network::Address::IpVersion::v6, options->localAddressIpVersion());
EXPECT_EQ(std::chrono::seconds(42), options->drainTime());
Expand Down