Hi Ludwig, On 2026-05-13T14:08:10, Ludwig Nussel <[email protected]> wrote: > test: vboot: handle CONFIG_FIT_REQUIRE_CONFIG_SIGS in test_vboot > > Make sure test_vboot works with CONFIG_FIT_REQUIRE_CONFIG_SIGS set > and unset. > Enable CONFIG_FIT_REQUIRE_CONFIG_SIGS in sandbox_defconfig and leave > it off in other sandbox configs > > Co-authored-by: Copilot <[email protected]> > > Signed-off-by: Ludwig Nussel <[email protected]> > > configs/sandbox_defconfig | 1 + > test/py/tests/test_vboot.py | 60 > ++++++++++++++++++++++++++++++++++++--------- > 2 files changed, 49 insertions(+), 12 deletions(-)
Reviewed-by: Simon Glass <[email protected]> > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig > @@ -21,6 +21,7 @@ > CONFIG_EFI_CAPSULE_CRT_FILE='board/sandbox/capsule_pub_key_good.crt' > CONFIG_BUTTON_CMD=y > CONFIG_FIT=y > CONFIG_FIT_SIGNATURE=y > +CONFIG_FIT_REQUIRE_CONFIG_SIGS=y > CONFIG_FIT_CIPHER=y > CONFIG_FIT_VERBOSE=y Turning this on in the main sandbox_defconfig means the 'Test FIT with signed images' section (behind 'if not require_config_sigs:') is no longer exercised in CI, and test_global_sign() is skipped entirely. We lose coverage of image-level signatures and the pre-load global signature path on the primary sandbox build, both still supported features. Please leave sandbox_defconfig alone and turn this on in a secondary config (eg sandbox_noinst or sandbox_spl) so both paths keep getting tested. Or restructure the test so the image-level path still runs when require_config_sigs is set. Worse cas I suppose we could provide a runtime mechanism which the tests can control. What do you think? > diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py > @@ -306,26 +306,54 @@ def test_vboot(ubman, name, sha_algo, padding, > sign_options, required, > + bcfg = ubman.config.buildconfig > + require_config_sigs = bcfg.get('config_fit_require_config_sigs', > False) The same idiom is repeated in test_global_sign() below. Please put this into the outer test_vboot() scope (or a small helper) so the build config is read once. > diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py > @@ -485,6 +513,14 @@ def test_vboot(ubman, name, sha_algo, padding, > sign_options, required, > + # test_fdt_add_pubkey reuses this tmpdir and needs > sandbox-kernel.dtb, > + # so compile it unconditionally before any early exit. > + dtc('sandbox-kernel.dts', ubman, dtc_args, datadir, tmpdir, None) > + > + bcfg = ubman.config.buildconfig > + if bcfg.get('config_fit_require_config_sigs', False): > + pytest.skip('simple-images.its has no config-level signatures; ' > + 'incompatible with CONFIG_FIT_REQUIRE_CONFIG_SIGS') Worth mentioning in the commit message that test_global_sign() gets skipped, otherwise it's easy to miss that we silently stop testing the pre-load global signature path. Also, the comment suggests the right fix may be to provide an .its with config signatures rather than skip - is there a reason that won't work? > diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py > @@ -306,26 +306,54 @@ def test_vboot(ubman, name, sha_algo, padding, > sign_options, required, > - ubman.log.action('%s: Test FIT with signed configuration' % sha_algo) > + ubman.log.action('%s: Test FIT with unsigned configuration' % > sha_algo) Good catch! Regards, Simon
