tooling: flags fixups (envoyproxy#9312)

Changing runtime ASSERTs to ensure that all runtime guards are registered as true by default, or in the "special case" section (documented to be used with care, tracking issues etc.)
Changing governance to note we should look at that section when we cut releases.
Changing flags script to file code removal after guards have been true for 6 months.

Risk Level: Meduim (if folks are using the upstream naming paradigm they'll fail asserts)
Testing: existing tests
Docs Changes: governance update
Release Notes: n/a
Fixes envoyproxy#8992

Signed-off-by: Alyssa Wilk <[email protected]>

GOVERNANCE.md

@@ -106,6 +106,8 @@ or you can subscribe to the iCal feed [here](https://app.opsgenie.com/webcal/get
 * Run the deprecate_features.py script (e.g. `sh tools/deprecate_features/deprecate_features.sh`)
   to make the last release's deprecated features fatal-by-default. Submit the resultant PR and send
   an email to envoy-announce.
+* Check source/common/runtime/runtime_features.cc and see if any runtime guards in
+  disabled_runtime_features should be reassessed, and ping on the relevant issues.
 
 ## When does a maintainer lose maintainer status
 

source/common/runtime/runtime_features.cc

@@ -33,6 +33,21 @@ constexpr const char* runtime_features[] = {
     "envoy.reloadable_features.strict_authority_validation",
 };
 
+// This is a section for officially sanctioned runtime features which are too
+// high risk to be enabled by default. Examples where we have opted to land
+// features without enabling by default are swapping the underlying buffer
+// implementation or the HTTP/1.1 codec implementation. Most features should be
+// enabled by default.
+//
+// When features are added here, there should be a tracking bug assigned to the
+// code owner to flip the default after sufficient testing.
+constexpr const char* disabled_runtime_features[] = {
+    // Sentinel and test flag.
+    "envoy.reloadable_features.test_feature_false",
+    // Should be removed as part of https://github.com/envoyproxy/envoy/issues/8993
+    "envoy.reloadable_features.http2_protocol_options.stream_error_on_invalid_http_messaging",
+};
+
 // This is a list of configuration fields which are disallowed by default in Envoy
 //
 // By default, use of proto fields marked as deprecated in their api/.../*.proto file will result
@@ -70,6 +85,9 @@ RuntimeFeatures::RuntimeFeatures() {
   for (auto& feature : runtime_features) {
     enabled_features_.insert(feature);
   }
+  for (auto& feature : disabled_runtime_features) {
+    disabled_features_.insert(feature);
+  }
 }
 
 } // namespace Runtime

source/common/runtime/runtime_features.h

@@ -29,12 +29,16 @@ class RuntimeFeatures {
   bool enabledByDefault(absl::string_view feature) const {
     return enabled_features_.find(feature) != enabled_features_.end();
   }
+  bool existsButDisabled(absl::string_view feature) const {
+    return disabled_features_.find(feature) != disabled_features_.end();
+  }
 
 private:
   friend class RuntimeFeaturesPeer;
 
   absl::flat_hash_set<std::string> disallowed_features_;
   absl::flat_hash_set<std::string> enabled_features_;
+  absl::flat_hash_set<std::string> disabled_features_;
 };
 
 using RuntimeFeaturesDefaults = ConstSingleton<RuntimeFeatures>;

source/common/runtime/runtime_impl.cc

@@ -39,7 +39,8 @@ bool isLegacyFeature(absl::string_view feature) {
 }
 
 bool isRuntimeFeature(absl::string_view feature) {
-  return absl::StartsWith(feature, "envoy.reloadable_features.");
+  return RuntimeFeaturesDefaults::get().enabledByDefault(feature) ||
+         RuntimeFeaturesDefaults::get().existsButDisabled(feature);
 }
 
 } // namespace

tools/deprecate_version/deprecate_version.py

@@ -22,6 +22,8 @@
 
 from __future__ import print_function
 
+import datetime
+from datetime import date
 from collections import defaultdict
 import os
 import re
@@ -127,62 +129,56 @@ def CreateIssues(access_token, runtime_and_pr):
           raise
 
 
-def GetRuntimeAlreadyTrue():
-  """Returns a list of runtime flags already defaulted to true
-  """
-  runtime_already_true = []
-  runtime_features = re.compile(r'.*"(envoy.reloadable_features..*)",.*')
-  with open('source/common/runtime/runtime_features.cc', 'r') as features:
-    for line in features.readlines():
-      match = runtime_features.match(line)
-      if match and 'test_feature_true' not in match.group(1):
-        print("Found existing flag " + match.group(1))
-        runtime_already_true.append(match.group(1))
-
-  return runtime_already_true
-
-
 def GetRuntimeAndPr():
   """Returns a list of tuples of [runtime features to deprecate, PR the feature was added]
   """
   repo = Repo(os.getcwd())
 
-  runtime_already_true = GetRuntimeAlreadyTrue()
-
   # grep source code looking for reloadable features which are true to find the
   # PR they were added.
-  grep_output = subprocess.check_output('grep -r "envoy.reloadable_features\." source/', shell=True)
   features_to_flip = []
-  runtime_feature_regex = re.compile(r'.*(source.*cc).*"(envoy.reloadable_features\.[^"]+)".*')
-  for line in grep_output.splitlines():
-    match = runtime_feature_regex.match(str(line))
-    if match:
-      filename = (match.group(1))
-      runtime_guard = match.group(2)
-      # If this runtime guard isn't true, ignore it for this release.
-      if not runtime_guard in runtime_already_true:
-        continue
 
-      # For true runtime guards, walk the blame of the file they were added to,
-      # to find the pr the feature was added.
-      for commit, lines in repo.blame('HEAD', filename):
-        for line in lines:
-          if runtime_guard in line:
-            pr = (int(re.search('\(#(\d+)\)', commit.message).group(1)))
-            # Add the runtime guard and PR to the list to file issues about.
-            features_to_flip.append((runtime_guard, pr))
-            # Make sure if grep finds multiple spots we only do the work one time.
-            runtime_already_true.remove(runtime_guard)
+  runtime_features = re.compile(r'.*"(envoy.reloadable_features..*)",.*')
 
-    else:
-      print('no match in ' + str(line) + ' please address manually!')
+  removal_date = date.today() - datetime.timedelta(days=183)
+  found_test_feature_true = False
 
-  return features_to_flip
+  # Walk the blame of runtime_features and look for true runtime features older than 6 months.
+  for commit, lines in repo.blame('HEAD', 'source/common/runtime/runtime_features.cc'):
+    for line in lines:
+      match = runtime_features.match(line)
+      if match:
+        runtime_guard = match.group(1)
+        if runtime_guard == 'envoy.reloadable_features.test_feature_false':
+          print("Found end sentinel\n")
+          if not found_test_feature_true:
+            # The script depends on the cc file having the true runtime block
+            # before the false runtime block.  Fail if one isn't found.
+            print('Failed to find test_feature_true.  Script needs fixing')
+            sys.exit(1)
+          return features_to_flip
+        if runtime_guard == 'envoy.reloadable_features.test_feature_true':
+          found_test_feature_true = True
+          continue
+        pr = (int(re.search('\(#(\d+)\)', commit.message).group(1)))
+        pr_date = date.fromtimestamp(commit.committed_date)
+        removable = (pr_date < removal_date)
+        # Add the runtime guard and PR to the list to file issues about.
+        print('Flag ' + runtime_guard + ' added at ' + str(pr_date) + ' ' +
+              (removable and 'and is safe to remove' or 'is not ready to remove'))
+        if removable:
+          features_to_flip.append((runtime_guard, pr))
+  print('Failed to find test_feature_false.  Script needs fixing')
+  sys.exit(1)
 
 
 if __name__ == '__main__':
   runtime_and_pr = GetRuntimeAndPr()
 
+  if not runtime_and_pr:
+    print('No code is deprecated.')
+    sys.exit(0)
+
   access_token = os.getenv('GH_ACCESS_TOKEN')
   if not access_token:
     print(