commit f505c7be44b1484eb72daf3ebcbce7a485a60f94
author Emil Lundmark <lndmrk@webrtc.org> Wed Apr 10 13:02:32 2024
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Apr 10 19:09:34 2024
tree c743e4c3f86fe207ec616cb456268514b0c4a724
parent 4dfe7ea5afa4cbfde1bf0e5579aec362ea1a4a09

Add safeguard for modifying POLICY_EXEMPT_FIELD_TRIALS

This will make it harder to inadvertently register new field trials in
the wrong collection. This has happened before, see 88a8e44a51 ("Remove
nonexempt field trials from POLICY_EXEMPT_FIELD_TRIALS") for example.

Additionally, field trials will now also be validated by default before
a C++ header is generated.

Bug: None
Change-Id: I298c1345d48a522ecb95fd0f0e09834c8bdff40a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/346543
Reviewed-by: Jeremy Leconte <jleconte@google.com>
Commit-Queue: Emil Lundmark <lndmrk@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42034}

experiments/field_trials.py

diff --git a/experiments/field_trials.py b/experiments/field_trials.py
index a174367..3e65411 100755
--- a/experiments/field_trials.py
+++ b/experiments/field_trials.py
@@ -10,6 +10,7 @@
 
 import datetime
 from datetime import date
+import hashlib
 import sys
 from typing import FrozenSet, List, Set
 
@@ -162,6 +163,8 @@
 
 # These field trials precedes the policy in `g3doc/field-trials.md` and are
 # therefore not required to follow it. Do not add any new field trials here.
+# If you remove an entry you should also update
+# POLICY_EXEMPT_FIELD_TRIALS_DIGEST.
 POLICY_EXEMPT_FIELD_TRIALS: FrozenSet[FieldTrial] = frozenset([
     # keep-sorted start
     FieldTrial('UseTwccPlrForAna',
@@ -913,6 +916,9 @@
     # keep-sorted end
 ])  # yapf: disable
 
+POLICY_EXEMPT_FIELD_TRIALS_DIGEST: str = \
+    '023f4ce749a699f0ab811093b9f568d604da28a8'
+
 REGISTERED_FIELD_TRIALS: FrozenSet[FieldTrial] = ACTIVE_FIELD_TRIALS.union(
     POLICY_EXEMPT_FIELD_TRIALS)
 
@@ -1013,6 +1019,17 @@
       A list of explanations for invalid field trials.
     """
     invalid = []
+
+    sha1 = hashlib.sha1()
+    for trial in sorted(POLICY_EXEMPT_FIELD_TRIALS, key=lambda f: f.key):
+        sha1.update(trial.key.encode('ascii'))
+    if sha1.hexdigest() != POLICY_EXEMPT_FIELD_TRIALS_DIGEST:
+        invalid.append(
+            'POLICY_EXEMPT_FIELD_TRIALS has been modified. Please note that '
+            'you must not add any new entries there. If you removed an entry '
+            'you should also update POLICY_EXEMPT_FIELD_TRIALS_DIGEST. The'
+            f'new digest is "{sha1.hexdigest()}".')
+
     for trial in field_trials:
         if not trial.key.startswith('WebRTC-'):
             invalid.append(f'{trial.key} does not start with "WebRTC-".')
@@ -1020,10 +1037,16 @@
             invalid.append(f'{trial.key} must have an associated bug.')
         if trial.end_date >= INDEFINITE:
             invalid.append(f'{trial.key} must have an end date.')
+
     return invalid
 
 
 def cmd_header(args: argparse.Namespace) -> None:
+    if not args.no_validation:
+        if errors := validate_field_trials():
+            print('\n'.join(sorted(errors)))
+            sys.exit(1)
+
     args.output.write(registry_header())
 
 
@@ -1070,6 +1093,12 @@
                                type=argparse.FileType('w'),
                                required=False,
                                help='output file')
+    parser_header.add_argument(
+        '--no-validation',
+        default=False,
+        action='store_true',
+        required=False,
+        help='whether to validate the field trials before writing')
     parser_header.set_defaults(cmd=cmd_header)
 
     parser_expired = subcommand.add_parser(