build: adding an option to hard-fail when deprecated config is used.

CONTRIBUTING.md

@@ -72,7 +72,11 @@ maximize the chances of your PR being merged.
   could convert from the earlier API to the new API. A field may be deprecated
   if this tool would be able to perform the conversion. For example, removing a
   field to describe HTTP/2 window settings is valid if a more comprehensive
-  HTTP/2 protocol options field is being introduced to replace it.
+  HTTP/2 protocol options field is being introduced to replace it. The PR author
+  deprecating the old configuration is responsible for updating all tests and
+  canonical configuration, or guarding them with ifndef DISABLE_DEPRECATED_FEATURES.
+  This will be validated by the bazel.debug target, which will hard-fail when
+  deprecated configuration is used.
 * For configuration deprecations that are not covered by the above semantic
   replacement policy, any deprecation will only take place after
   community consultation on mailing lists, Slack and GitHub, over the period of

bazel/BUILD

@@ -120,6 +120,11 @@ config_setting(
     values = {"define": "object_dump_on_signal_trace=disabled"},
 )
 
+config_setting(
+    name = "disable_deprecated_features",
+    values = {"define": "deprecated_features=disabled"},
+)
+
 config_setting(
     name = "disable_hot_restart",
     values = {"define": "hot_restart=disabled"},

bazel/README.md

@@ -407,6 +407,8 @@ The following optional features can be disabled on the Bazel build command-line:
 * Backtracing on signals with `--define signal_trace=disabled`
 * Active stream state dump on signals with `--define signal_trace=disabled` or `--define disable_object_dump_on_signal_trace=disabled`
 * tcmalloc with `--define tcmalloc=disabled`
+* deprecated features with `--define deprecated_features=disabled`
+
 
 ## Enabling optional features
 

bazel/envoy_internal.bzl

@@ -56,6 +56,9 @@ def envoy_copts(repository, test = False):
            }) + select({
                repository + "//bazel:disable_object_dump_on_signal_trace": [],
                "//conditions:default": ["-DENVOY_OBJECT_TRACE_ON_DUMP"],
+           }) + select({
+               repository + "//bazel:disable_deprecated_features": ["-DDISABLE_DEPRECATED_FEATURES"],
+               "//conditions:default": [],
            }) + select({
                repository + "//bazel:enable_log_debug_assert_in_release": ["-DENVOY_LOG_DEBUG_ASSERT_IN_RELEASE"],
                "//conditions:default": [],

ci/do_ci.sh

@@ -133,7 +133,7 @@ elif [[ "$CI_TARGET" == "bazel.debug" ]]; then
   echo "bazel debug build with tests..."
   bazel_binary_build debug
   echo "Testing ${TEST_TARGETS}"
-  bazel test ${BAZEL_BUILD_OPTIONS} -c dbg ${TEST_TARGETS}
+  bazel test ${BAZEL_BUILD_OPTIONS} --define deprecated_features=disabled -c dbg ${TEST_TARGETS}
   exit 0
 elif [[ "$CI_TARGET" == "bazel.debug.server_only" ]]; then
   setup_clang_toolchain

source/common/runtime/runtime_impl.cc

@@ -182,9 +182,14 @@ bool SnapshotImpl::deprecatedFeatureEnabled(const std::string& key) const {
     // If either disallowed by default or configured off, the feature is not enabled.
     return false;
   }
+
   // The feature is allowed. It is assumed this check is called when the feature
   // is about to be used, so increment the feature use stat.
   stats_.deprecated_feature_use_.inc();
+#ifdef DISABLE_DEPRECATED_FEATURES
+  return false;
+#endif
+
   return true;
 }
 

test/common/protobuf/utility_test.cc

@@ -492,6 +492,7 @@ TEST_F(DeprecatedFieldsTest, NoErrorWhenDeprecatedFieldsUnused) {
   EXPECT_EQ(0, runtime_deprecated_feature_use_.value());
 }
 
+#ifndef DISABLE_DEPRECATED_FEATURES
 TEST_F(DeprecatedFieldsTest, IndividualFieldDeprecated) {
   envoy::test::deprecation_test::Base base;
   base.set_is_deprecated("foo");
@@ -617,6 +618,7 @@ TEST_F(DeprecatedFieldsTest, RepeatedMessageDeprecated) {
                       "'envoy.test.deprecation_test.Base.deprecated_repeated_message'",
                       MessageUtil::checkForDeprecation(base));
 }
+#endif
 
 class TimestampUtilTest : public testing::Test, public ::testing::WithParamInterface<int64_t> {};
 

test/common/runtime/runtime_impl_test.cc

@@ -179,6 +179,15 @@ TEST_F(DiskLoaderImplTest, All) {
   // test_feature_false is not in runtime_features.cc and so is false by default.
   EXPECT_EQ(false, snapshot->runtimeFeatureEnabled("envoy.reloadable_features.test_feature_false"));
 
+  // Deprecation
+#ifdef DISABLE_DEPRECATED_FEATURES
+  EXPECT_EQ(false, snapshot->deprecatedFeatureEnabled("random_string_should_be_enabled"));
+#else
+  EXPECT_EQ(true, snapshot->deprecatedFeatureEnabled("random_string_should_be_enabled"));
+#endif
+  EXPECT_EQ(false, snapshot->deprecatedFeatureEnabled(
+                       "envoy.deprecated_features.deprecated.proto:is_deprecated_fatal"));
+
   // Feature defaults via helper function.
   EXPECT_EQ(false, runtimeFeatureEnabled("envoy.reloadable_features.test_feature_false"));
   EXPECT_EQ(true, runtimeFeatureEnabled("envoy.reloadable_features.test_feature_true"));