[manuf] Make orchestrator resolve paths using runfiles library

This simplifies use of the packaged orchestrator script. Instead of having to extract the packaged runfiles manually, the rules_python machinery extracts it into a temp dir and the library handles path resolution. Some hackery was required to preserve the scheme used previously for referring to the main repo vs external repos. Signed-off-by: Noah Moroze <[email protected]>
lowRISC · Dec 17, 2024 · 351b36b · 351b36b
1 parent 99683cf
commit 351b36b
Show file tree

Hide file tree

Showing 10 changed files with 137 additions and 68 deletions.
diff --git a/sw/host/provisioning/orchestrator/README.md b/sw/host/provisioning/orchestrator/README.md
@@ -61,35 +61,14 @@ cp ${REPO_TOP}/bazel-bin/sw/host/provisioning/orchestrator/src/orchestrator.zip
 
 export ORCHESTRATOR_ZIP="${ORCHESTRATOR_RUN_DIR}/orchestrator.zip"
 
-# Extract runfile folders from orchestrator package.
-unzip ${ORCHESTRATOR_ZIP} \
-  "runfiles/lowrisc_opentitan/*" \
-  "runfiles/openocd/*" \
-  "runfiles/provisioning_exts/*" \
-  "runfiles/sc_hsm/*"
-
-# All external dependencies are mapped under
-# runfiles/lowrisc_opentitan/external.
-mkdir -p  runfiles/lowrisc_opentitan/external
-
-ln -fs $(pwd)/runfiles/openocd runfiles/lowrisc_opentitan/external
-
-# The following is needed if you are using the provisioning extensions
-# infrastructure.
-PROVISIONING_EXT_RUNFILES=$(pwd)/runfiles/provisioning_exts
-[ -d "${PROVISION_EXT_RUNFILES}" ] &&   \
-  ln -fs "${PROVISIONING_EXT_RUNFILES}" \
-    runfiles/lowrisc_opentitan/external/provisioning_exts
-
 # Run tool. The path to the --sku-config parameter is relative to the
-# runfiles-dir.
+# workspace root.
 export FPGA_TARGET=hyper310
 python3 ${ORCHESTRATOR_ZIP} \
   --sku-config=sw/host/provisioning/orchestrator/configs/skus/emulation.hjson \
   --test-unlock-token="0x11111111_11111111_11111111_11111111" \
   --test-exit-token="0x22222222_22222222_22222222_22222222" \
   --fpga=${FPGA_TARGET} \
   --non-interactive \
-  --runfiles-dir=$(pwd)/runfiles/lowrisc_opentitan \
-  --db-path=$(pwd)/provisioning.sqlite
+  --db-path=provisioning.sqlite
 ```
diff --git a/sw/host/provisioning/orchestrator/src/BUILD b/sw/host/provisioning/orchestrator/src/BUILD
@@ -54,7 +54,10 @@ py_library(
     name = "util",
     srcs = ["util.py"],
     imports = ["."],
-    deps = [requirement("hjson")],
+    deps = [
+        requirement("hjson"),
+        "@rules_python//python/runfiles",
+    ],
 )
 
 py_library(

diff --git a/sw/host/provisioning/orchestrator/src/ca_config.py b/sw/host/provisioning/orchestrator/src/ca_config.py
@@ -6,6 +6,8 @@
 from dataclasses import dataclass
 from pathlib import Path
 
+from util import resolve_runfile
+
 
 @dataclass
 class CaConfig:
@@ -18,9 +20,9 @@ class CaConfig:
 
     def __post_init__(self):
         # Update certificate and key members to Path objs if necessary.
-        self.certificate = Path(self.certificate)
+        self.certificate = Path(resolve_runfile(self.certificate))
         if self.key_type == "Raw":
-            self.key = Path(self.key)
+            self.key = Path(resolve_runfile(self.key))
         self.validate()
 
     def validate(self) -> None:

diff --git a/sw/host/provisioning/orchestrator/src/orchestrator.py b/sw/host/provisioning/orchestrator/src/orchestrator.py
@@ -5,7 +5,6 @@
 
 import argparse
 import logging
-import os
 import shlex
 import subprocess
 import sys
@@ -17,7 +16,7 @@
 from device_id import DeviceId
 from ot_dut import OtDut
 from sku_config import SkuConfig
-from util import confirm, parse_hexstring_to_int
+from util import confirm, parse_hexstring_to_int, resolve_runfile
 
 
 def get_user_confirmation(
@@ -110,11 +109,6 @@ def main(args_in):
         default=False,
         help="Skip all non-required user confirmations.",
     )
-    parser.add_argument(
-        "--runfiles-dir",
-        type=str,
-        help="Runfiles directory to use for provisioning.",
-    )
     parser.add_argument(
         "--log-dir",
         default="logs",
@@ -136,13 +130,10 @@ def main(args_in):
     if not args.cp_only and args.db_path is None:
         parser.error("--db-path is required when --cp-only is not provided")
 
-    # All relative paths are relative to the runfiles directory.
-    if args.runfiles_dir:
-        os.chdir(args.runfiles_dir)
-
     # Load and validate a SKU configuration file.
+    sku_config_path = resolve_runfile(args.sku_config)
     sku_config_args = {}
-    with open(args.sku_config, "r") as fp:
+    with open(sku_config_path, "r") as fp:
         sku_config_args = hjson.load(fp)
     sku_config = SkuConfig(**sku_config_args)
 

diff --git a/sw/host/provisioning/orchestrator/src/ot_dut.py b/sw/host/provisioning/orchestrator/src/ot_dut.py
@@ -14,7 +14,7 @@
 
 from device_id import DeviceId, DeviceIdentificationNumber
 from sku_config import SkuConfig
-from util import confirm, format_hex, run
+from util import confirm, format_hex, run, resolve_runfile
 
 # FPGA bitstream.
 _FPGA_UNIVERSAL_SPLICE_BITSTREAM = "hw/bitstream/universal/splice.bit"
@@ -99,27 +99,33 @@ def run_cp(self) -> None:
         host_flags = _BASE_PROVISIONING_FLAGS
         device_elf = _CP_DEVICE_ELF
         print(f"device_elf: {device_elf}")
+
+        openocd_bin = resolve_runfile(_OPENOCD_BIN)
+        openocd_cfg = resolve_runfile(_OPENOCD_ADAPTER_CONFIG)
         if self.fpga:
             # Set host flags and device binary for FPGA DUT.
             host_flags = host_flags.format(target=self.fpga,
-                                           openocd_bin=_OPENOCD_BIN,
-                                           openocd_cfg=_OPENOCD_ADAPTER_CONFIG)
+                                           openocd_bin=openocd_bin,
+                                           openocd_cfg=openocd_cfg)
             host_flags += " --clear-bitstream"
-            host_flags += f" --bitstream={_FPGA_UNIVERSAL_SPLICE_BITSTREAM}"
+            bitstream = resolve_runfile(_FPGA_UNIVERSAL_SPLICE_BITSTREAM)
+            host_flags += f" --bitstream={bitstream}"
             device_elf = device_elf.format(
                 base_dir=self._base_dev_dir(),
                 target=f"fpga_{self.fpga}_rom_with_fake_keys")
         else:
             # Set host flags and device binary for Silicon DUT.
             host_flags = host_flags.format(target="teacup",
-                                           openocd_bin=_OPENOCD_BIN,
-                                           openocd_cfg=_OPENOCD_ADAPTER_CONFIG)
+                                           openocd_bin=openocd_bin,
+                                           openocd_cfg=openocd_cfg)
             host_flags += " --disable-dft-on-reset"
             device_elf = device_elf.format(base_dir=self._base_dev_dir(),
                                            target="silicon_creator")
+        device_elf = resolve_runfile(device_elf)
 
         # Assemble CP command.
-        cmd = f"""{_CP_HOST_BIN} \
+        cp_host_bin = resolve_runfile(_CP_HOST_BIN)
+        cmd = f"""{cp_host_bin} \
         --rcfile= \
         --logging=info \
         {host_flags} \
@@ -179,6 +185,9 @@ def run_ft(self) -> None:
 
         # Set cmd args and device binaries.
         host_bin = _FT_HOST_BIN.format(sku=self.sku_config.name)
+        host_bin = resolve_runfile(host_bin)
+        openocd_bin = resolve_runfile(_OPENOCD_BIN)
+        openocd_cfg = resolve_runfile(_OPENOCD_ADAPTER_CONFIG)
         host_flags = _BASE_PROVISIONING_FLAGS
         individ_elf = _FT_INDIVID_DEVICE_ELF
         # Emulation perso bins are signed online with fake keys, and therefore
@@ -190,8 +199,8 @@ def run_ft(self) -> None:
             # No need to load another bitstream, we will take over where CP
             # stage above left off.
             host_flags = host_flags.format(target=self.fpga,
-                                           openocd_bin=_OPENOCD_BIN,
-                                           openocd_cfg=_OPENOCD_ADAPTER_CONFIG)
+                                           openocd_bin=openocd_bin,
+                                           openocd_cfg=openocd_cfg)
             individ_elf = individ_elf.format(
                 base_dir=self._base_dev_dir(),
                 otp=self.sku_config.otp,
@@ -207,8 +216,8 @@ def run_ft(self) -> None:
         else:
             # Set host flags and device binaries for Silicon DUT.
             host_flags = host_flags.format(target="teacup",
-                                           openocd_bin=_OPENOCD_BIN,
-                                           openocd_cfg=_OPENOCD_ADAPTER_CONFIG)
+                                           openocd_bin=openocd_bin,
+                                           openocd_cfg=openocd_cfg)
             host_flags += " --disable-dft-on-reset"
             individ_elf = individ_elf.format(base_dir=self._base_dev_dir(),
                                              otp=self.sku_config.otp,
@@ -220,6 +229,10 @@ def run_ft(self) -> None:
                                                  sku=self.sku_config.name,
                                                  target="silicon_creator")
 
+        individ_elf = resolve_runfile(individ_elf)
+        perso_bin = resolve_runfile(perso_bin)
+        fw_bundle_bin = resolve_runfile(fw_bundle_bin)
+
         # Write CA configs to a JSON tmpfile.
         ca_config_dict = {
             "dice": self.sku_config.dice_ca.to_dict_entry(),

diff --git a/sw/host/provisioning/orchestrator/src/sku_config.py b/sw/host/provisioning/orchestrator/src/sku_config.py
@@ -10,6 +10,7 @@
 import hjson
 
 from ca_config import CaConfig
+from util import resolve_runfile
 
 _PRODUCT_IDS_HJSON = "sw/host/provisioning/orchestrator/data/products.hjson"
 _PACKAGE_IDS_HJSON = "sw/host/provisioning/orchestrator/data/packages/earlgrey_a1.hjson"
@@ -37,13 +38,15 @@ def __post_init__(self):
             self.ext_ca = CaConfig(name="ext_ca", **self.ext_ca)
 
         # Load product IDs database.
+        product_ids_hjson = resolve_runfile(_PRODUCT_IDS_HJSON)
         self._product_ids = None
-        with open(_PRODUCT_IDS_HJSON, "r") as fp:
+        with open(product_ids_hjson, "r") as fp:
             self._product_ids = hjson.load(fp)
 
         # Load package IDs database.
+        package_ids_hjson = resolve_runfile(_PACKAGE_IDS_HJSON)
         self._package_ids = None
-        with open(_PACKAGE_IDS_HJSON, "r") as fp:
+        with open(package_ids_hjson, "r") as fp:
             self._package_ids = hjson.load(fp)
 
         # Validate inputs.
@@ -58,18 +61,23 @@ def __post_init__(self):
         if self.package in self._package_ids:
             self.package_id = int(self._package_ids[self.package], 16)
 
+        if self.token_encrypt_key:
+            self.token_encrypt_key = resolve_runfile(self.token_encrypt_key)
+
     @staticmethod
     def from_ids(product_id: int, si_creator_id: int,
                  package_id: int) -> "SkuConfig":
         """Creates a SKU configuration object from product, SiliconCreator, and package IDs."""
         # Load product IDs database.
+        product_ids_hjson = resolve_runfile(_PRODUCT_IDS_HJSON)
         product_ids = None
-        with open(_PRODUCT_IDS_HJSON, "r") as fp:
+        with open(product_ids_hjson, "r") as fp:
             product_ids = hjson.load(fp)
 
         # Load package IDs database.
+        package_ids_hjson = resolve_runfile(_PACKAGE_IDS_HJSON)
         package_ids = None
-        with open(_PACKAGE_IDS_HJSON, "r") as fp:
+        with open(package_ids_hjson, "r") as fp:
             package_ids = hjson.load(fp)
 
         # Create SKU configuration object.

diff --git a/sw/host/provisioning/orchestrator/src/util.py b/sw/host/provisioning/orchestrator/src/util.py
@@ -3,9 +3,12 @@
 # SPDX-License-Identifier: Apache-2.0
 
 import logging
+import os
 import shlex
 import subprocess
 
+from python.runfiles import Runfiles
+
 
 def parse_hexstring_to_int(x):
     """Accepts hexstrings with and without the 0x."""
@@ -64,3 +67,36 @@ def run(cmd, stdout_logfile, stderr_logfile):
     out_tee.stdin.close()
     err_tee.stdin.close()
     return res
+
+
+_runfiles = Runfiles.Create()
+
+
+def resolve_runfile(path):
+    """Resolves path to runfile.
+
+    Relative paths specified as external/<repo>/... will be resolved relative to
+    the external repository @<repo>. Otherwise, relative paths will be resolved
+    relative to the main workspace. Absolute paths are returned as-is.
+
+    Raises a ValueError if the path does not exist on the filesystem.
+    """
+
+    # orchestrator.py assumes the "old" style of runfiles tree, where paths to
+    # files within the main workspace do not include the repo name and external
+    # deps prepend external/.
+    #
+    # The old scheme does not work within a zipped py_binary, so this logic is a
+    # hack to fix up the supplied path.
+    #
+    # See https://docs.google.com/document/d/1skNx5o-8k5-YXUAyEETvr39eKoh9fecJbGUquPh5iy8/edit.
+    REPO = "lowrisc_opentitan"
+    if path.startswith("external/"):
+        corrected_path = path[len("external/"):]
+    else:
+        corrected_path = os.path.join(REPO, path)
+
+    resolved = _runfiles.Rlocation(corrected_path)
+    if resolved is None or not os.path.exists(resolved):
+        raise ValueError(f"Could not find runfile: {path}")
+    return resolved
diff --git a/sw/host/provisioning/orchestrator/tests/BUILD b/sw/host/provisioning/orchestrator/tests/BUILD
@@ -22,6 +22,7 @@ py_test(
     data = [
         "//sw/device/silicon_creator/manuf/keys/fake:dice_ca.pem",
         "//sw/device/silicon_creator/manuf/keys/fake:ext_ca.pem",
+        "//sw/device/silicon_creator/manuf/keys/fake:rma_unlock_enc_rsa3072.pub.der",
         "//sw/device/silicon_creator/manuf/keys/fake:sk.pkcs8.der",
         "//sw/host/provisioning/orchestrator/configs/skus:emulation.hjson",
     ],
@@ -33,6 +34,18 @@ py_test(
     ],
 )
 
+py_test(
+    name = "util_test",
+    srcs = ["util_test.py"],
+    data = [
+        "//sw/device/silicon_creator/manuf/keys/fake:dice_ca.pem",
+        "//third_party/openocd:jtag_cmsis_dap_adapter_cfg",
+    ],
+    deps = [
+        "//sw/host/provisioning/orchestrator/src:util",
+    ],
+)
+
 # This transition has the same effect as supplying the following flags to bazel test:
 #  --//hw/bitstream/universal:env=//hw/top_earlgrey:fpga_hyper310_rom_with_fake_keys
 #  --//hw/bitstream/universal:otp=//hw/ip/otp_ctrl/data/earlgrey_skus/emulation:otp_img_test_unlocked0_manuf_empty

diff --git a/sw/host/provisioning/orchestrator/tests/e2e.sh b/sw/host/provisioning/orchestrator/tests/e2e.sh
@@ -9,20 +9,11 @@ cp sw/host/provisioning/orchestrator/src/orchestrator.zip $TEST_TMPDIR
 
 ORCHESTRATOR_PATH=$TEST_TMPDIR/orchestrator.zip
 
-unzip $ORCHESTRATOR_PATH \
-  "runfiles/lowrisc_opentitan/*" \
-  "runfiles/openocd/*" \
-  -d $TEST_TMPDIR
-  # TODO: uncomment this line when we add the extensions in
-  # "runfiles/provisioning_exts/*" \
-
-mkdir -p $TEST_TMPDIR/runfiles/lowrisc_opentitan/external
-cp -r $TEST_TMPDIR/runfiles/openocd/ $TEST_TMPDIR/runfiles/lowrisc_opentitan/external/
-
-PROVISIONING_EXT_RUNFILES=$TEST_TMPDIR/runfiles/provisioning_exts
-[ -d "${PROVISION_EXT_RUNFILES}" ] && \
-  ln -fs "${PROVISIONING_EXT_RUNFILES}" \
-    $TEST_TMPDIR/runfiles/lowrisc_opentitan/external/provisioning_exts
+# This script is run by a Bazel sh_test rule, which sets RUNFILES_DIR to point
+# at the test's runfiles. However, if RUNFILES_DIR is set, orchestrator.zip will
+# inherit its value instead of setting it to the proper directory. This breaks
+# runfile resolution, so we unset this variable here.
+unset RUNFILES_DIR
 
 # Run tool. The path to the --sku-config parameter is relative to the
 # runfiles-dir. Note: "use-ext-clk" flag on FPGA does nothing, but this tests
@@ -34,5 +25,4 @@ $PYTHON ${ORCHESTRATOR_PATH} \
   --fpga=hyper310 \
   --use-ext-clk \
   --non-interactive \
-  --runfiles-dir=$TEST_TMPDIR/runfiles/lowrisc_opentitan \
   --db-path=$TEST_TMPDIR/registry.sqlite