api: field_handler: Prevent new "inside-chroot" APIs without Chroot
ChrootHandler is happy to skip over messages that unexpectedly are
missing a Chroot field, and in the primary entry path (handle_chroot()),
simply print a warning:
No chroot message found, falling back to defaults.
This hides the addition of malformed new APIs, since they can sometimes
get away with that due to technical debt in b/187787264.
Let's make it fatal instead.
BUG=b:187787264
TEST=./run_tests
Change-Id: Id81c49cec8a0d1fd287d5d5ad9412008dcfeb97b
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/4661997
Reviewed-by: Alex Klein <saklein@chromium.org>
Tested-by: Brian Norris <briannorris@chromium.org>
Auto-Submit: Brian Norris <briannorris@chromium.org>
Commit-Queue: Brian Norris <briannorris@chromium.org>
diff --git a/api/field_handler.py b/api/field_handler.py
index 0ff3bd4..9404b5f 100644
--- a/api/field_handler.py
+++ b/api/field_handler.py
@@ -31,6 +31,10 @@
"""Result path is invalid."""
+class MissingChrootMessage(Error):
+ """Message is missing Chroot field."""
+
+
class ChrootHandler:
"""Translate a Chroot message to chroot enter arguments and env."""
@@ -60,7 +64,8 @@
if chroot:
return chroot
- return None
+ # Complain loudly if a message is used without a Chroot field.
+ raise MissingChrootMessage("No chroot message found.")
def parse_chroot(
self, chroot_message: common_pb2.Chroot
@@ -74,12 +79,7 @@
) -> "chroot_lib.Chroot":
"""Find and parse the chroot field, returning the Chroot instance."""
handler = ChrootHandler(clear_field)
- chroot = handler.handle(message)
- if chroot:
- return chroot
-
- logging.warning("No chroot message found, falling back to defaults.")
- return handler.parse_chroot(common_pb2.Chroot())
+ return handler.handle(message)
def handle_goma(message, chroot_path, out_path):