From 528b6f2eefa8ade049be41d024e447f568c09878 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 3 Sep 2026 16:05:02 -0600 Subject: [PATCH 1/3] meta-avocado/sdk: Hash the repo-config renderer into task signatures Editing repoconf.py does not invalidate any task hash, so a corrected gpgkey path or baseurl can be committed, built, and never reach the SDK: bitbake reports do_compile as sstate-valid, restores the old object, and the container ships the file rendered before the edit. Nothing warns. Two independent mechanisms have to line up before bitbake tracks an addpylib module, and neither was in place. PyLibNode.eval registers a submodule's functions only when the package names it in BBIMPORTS (bb/parse/ast.py), and this package declared none, so repoconf never entered modulecode_deps at all. Registration alone would not have been enough either: the registered key is the fully qualified avocado_sdk_metadata.repoconf., while called_node_name records the literal spelling at the call site (bb/codeparser.py) and bb/data.py intersects the two as exact sets. An aliased import therefore records a name that can never match its own registration. Declaring BBIMPORTS and dropping the alias closes both. The verbose call sites are the price of the second half; an `as repoconf` import reads better and silently unhashes the module. --- meta-avocado/lib/avocado_sdk_metadata/__init__.py | 7 +++++++ .../recipes-avocado/sdk/avocado-sdk-metadata.bb | 10 +++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/meta-avocado/lib/avocado_sdk_metadata/__init__.py b/meta-avocado/lib/avocado_sdk_metadata/__init__.py index 98813136..a9506fc9 100644 --- a/meta-avocado/lib/avocado_sdk_metadata/__init__.py +++ b/meta-avocado/lib/avocado_sdk_metadata/__init__.py @@ -1 +1,8 @@ # SPDX-License-Identifier: Apache-2.0 + +# BitBake hashes an addpylib module's functions into task signatures only for +# the submodules named here: bb/parse/ast.py's PyLibNode.eval reads BBIMPORTS +# and calls bb.codeparser.add_module_functions once per entry. A submodule left +# out is invisible to every task hash, so editing it leaves sstate valid and +# ships the previously generated file. +BBIMPORTS = ["repoconf"] diff --git a/meta-avocado/recipes-avocado/sdk/avocado-sdk-metadata.bb b/meta-avocado/recipes-avocado/sdk/avocado-sdk-metadata.bb index 94600bd7..d9e437dc 100644 --- a/meta-avocado/recipes-avocado/sdk/avocado-sdk-metadata.bb +++ b/meta-avocado/recipes-avocado/sdk/avocado-sdk-metadata.bb @@ -47,7 +47,11 @@ PACKAGE_ARCH = "all_avocadosdk" python do_compile() { import os import bb - import avocado_sdk_metadata.repoconf as repoconf + # Imported and called fully qualified, not aliased. BitBake records the + # literal call spelling and matches it against the registered name + # avocado_sdk_metadata.repoconf.; an alias never matches, so the + # renderer's source would drop out of this task's hash. + import avocado_sdk_metadata.repoconf deploy_dir_rpm = d.getVar('DEPLOY_DIR_RPM') machine = d.getVar('MACHINE') @@ -84,7 +88,7 @@ python do_compile() { # section or bump priority, but the arch is tracked above. return False written_sections.add(repo_section_name) - repo_f.write(repoconf.render_repo_section( + repo_f.write(avocado_sdk_metadata.repoconf.render_repo_section( section=repo_section_name, name=repo_name, baseurl_path=repo_url_path, @@ -159,7 +163,7 @@ python do_compile() { def _write_additional_target_repo(repo_f, priority): """Write the target-ext repo entry at the end.""" - repo_f.write(repoconf.render_repo_section( + repo_f.write(avocado_sdk_metadata.repoconf.render_repo_section( section=f"{machine_short_name}-target-ext", name=f"{machine_short_name}-target-ext", baseurl_path=f"$releasever/target/{machine_short_name}-ext", From 47cba8a5a1e9ef38242a79cd15a8a6c87311c4ba Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 3 Sep 2026 16:05:57 -0600 Subject: [PATCH 2/3] meta-avocado: Run the repo-config renderer tests in CI The suite under meta-avocado/tests is executed by nothing. The tree's only unittest invocation is scoped to meta-avocado-sbom/tests, so these tests have never run outside a developer's shell since they were added. That matters more here than the file count suggests. What a generated .repo section tells dnf to verify is only otherwise observable by building an SDK and reading the file out of the container, and these tests exist as the substitute for that build. A guard nobody runs is indistinguishable from one that always passes: weakening the priority check to a falsy test, for instance, would sort every section written at priority=0 to dnf's default while CI stayed green. Scoped to the lib, the tests and the SDK recipes rather than the whole layer, so an unrelated recipe edit does not queue a run that can only pass. No pip install step, unlike the sbom contract workflow - the suite is standard library only, so there is no dependency whose failure could shrink it rather than fail it. --- .github/workflows/pr-repoconf.yml | 46 +++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 .github/workflows/pr-repoconf.yml diff --git a/.github/workflows/pr-repoconf.yml b/.github/workflows/pr-repoconf.yml new file mode 100644 index 00000000..e3223468 --- /dev/null +++ b/.github/workflows/pr-repoconf.yml @@ -0,0 +1,46 @@ +name: PR-Repoconf + +# What a generated .repo section tells dnf to verify is a security decision, and +# it is only observable by building an SDK and reading the file out of it. These +# tests are the substitute for that build, so they have to run somewhere. + +on: + pull_request: + types: [opened, synchronize, reopened] + paths: + - 'meta-avocado/lib/**' + - 'meta-avocado/tests/**' + - 'meta-avocado/recipes-avocado/sdk/**' + - '.github/workflows/pr-repoconf.yml' + push: + branches: [scarthgap] + paths: + - 'meta-avocado/lib/**' + - 'meta-avocado/tests/**' + - 'meta-avocado/recipes-avocado/sdk/**' + workflow_dispatch: + +permissions: + contents: read + +# Cancel superseded runs so a burst of `synchronize` pushes does not stack up. +concurrency: + group: repoconf-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + repoconf: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: '3.12' + + # Standard library only - no install step, so a dependency failure cannot + # silently shrink the suite. + - name: Run the repo-config renderer tests + run: | + python -m unittest discover -v \ + -s meta-avocado/tests -p 'test_*.py' From 3180ad7e54d26dd47ccfccdfe1fd4ce80e41142e Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 3 Sep 2026 16:05:29 -0600 Subject: [PATCH 3/3] meta-avocado/sdk: Generate the SDK container's own repo section Enabling metadata verification would have verified everything except the one repository every SDK container reads. The [avocado-sdk] section was a checked-in file carrying a hardcoded gpgcheck=0 and no repo_gpgcheck, so the switch that reaches every generated section could not reach it. Nothing errors and nothing warns; the build simply looks verified. Rendering it through the same function makes one setting govern every .repo section the tree installs. The rendered block differs from the deleted file by an explicit repo_gpgcheck=0, which is dnf's own default, so behaviour is unchanged until the switch is set. Two smaller decisions the diff would otherwise leave unexplained. The literals live in render_sdk_repo_section rather than in the recipe because a recipe is a task the suite cannot import: spelled there, a typo in the section name or the path would pass every test and 404 on the container's first transaction. The deleted file was at least data a reviewer could diff, and that property should not be lost in the move. priority stays a required argument even though this caller passes None. Making it default to None would let a per-target section be added without one, rendering at dnf's default of 99 and sorting below every sibling ranked 1..N - a wrong package on a device, with no build error and no failing test. Passing None explicitly says the same thing and keeps the omission a TypeError. The {REPO_BASE} sed goes with the file it templated. It has matched nothing since the template switched to the ${repo_url} placeholder, which the client substitutes at install time instead. AVOCADO_REPO_BASE now has no bitbake consumer at all, though kas/base.yml and build-all-containers.sh still set and print it; that predates this change and is left alone rather than swept in. --- docs/local-package-feed.md | 24 +++-- .../lib/avocado_sdk_metadata/repoconf.py | 52 +++++++++-- .../recipes-avocado/sdk/avocado-sdk-repos.bb | 37 ++++++-- .../sdk/avocado-sdk-repos/avocado-sdk.repo | 5 - meta-avocado/tests/test_repoconf.py | 91 +++++++++++++++++++ 5 files changed, 181 insertions(+), 28 deletions(-) delete mode 100644 meta-avocado/recipes-avocado/sdk/avocado-sdk-repos/avocado-sdk.repo diff --git a/docs/local-package-feed.md b/docs/local-package-feed.md index 471c783c..99a36b20 100644 --- a/docs/local-package-feed.md +++ b/docs/local-package-feed.md @@ -200,13 +200,23 @@ curl -s -o /dev/null -w '%{http_code}\n' \ Package signing is a separate question and no part of the build signs RPMs today, so `AVOCADO_REPO_GPGCHECK` stays at `0` regardless of what the index does. -**These variables do not reach every repo the SDK reads.** They govern the -per-target sections generated by `avocado-sdk-metadata.bb`. The `[avocado-sdk]` -section is a static file installed by `avocado-sdk-repos.bb`, so it carries a -hardcoded `gpgcheck=0` and no `repo_gpgcheck` no matter what these are set to. -Enabling metadata verification therefore leaves that one repository unverified, -and it is the repository every SDK container reads. Bringing it under the same -renderer has to happen before verification can be called complete. +Both variables reach every `.repo` section the build installs: the per-target +sections generated by `avocado-sdk-metadata.bb`, and the `[avocado-sdk]` section +the SDK container itself reads, generated by `avocado-sdk-repos.bb`. Both render +through `meta-avocado/lib/avocado_sdk_metadata/repoconf.py`, so setting one +variable is enough - no `.repo` section holds a hardcoded value. + +One hardcoded value sits outside that set. `[main] gpgcheck=True` in +`avocado-sdk-repos`' `dnf.conf` is the container-wide default for a repo that +sets nothing. Every section rendered above sets `gpgcheck` explicitly and the +per-repo value wins, so it changes nothing today - but a `.repo` file added to +the container by someone else inherits it, and no build variable reaches it. + +The `[avocado-sdk]` section writes no `priority`, so it sits at dnf's default of +99. That is deliberate: `avocado-cli` points `reposdir` at both this file and +the per-target config, and dnf ranks priority across that merged set rather than +per file. The per-target sections hold 1 through N, so 99 keeps `[avocado-sdk]` +below them - where the static file this replaced effectively sat. --- diff --git a/meta-avocado/lib/avocado_sdk_metadata/repoconf.py b/meta-avocado/lib/avocado_sdk_metadata/repoconf.py index 8fff6d86..e631e109 100644 --- a/meta-avocado/lib/avocado_sdk_metadata/repoconf.py +++ b/meta-avocado/lib/avocado_sdk_metadata/repoconf.py @@ -23,13 +23,17 @@ signatures breaks every install, so the switch belongs with the publisher change, not ahead of it. -devtool-debt: only the sections generated by avocado-sdk-metadata.bb render -through here. The [avocado-sdk] section is a static file installed by -avocado-sdk-repos.bb and carries a hardcoded gpgcheck=0 with no repo_gpgcheck. -Ceiling: adequate only while metadata verification is off everywhere. -Upgrade trigger: bring that recipe through this renderer before anything sets -AVOCADO_REPO_METADATA_GPGCHECK=1, or the SDK repo every container reads stays -unverified while the generated ones are verified. +Every ``.repo`` section the tree installs renders through here: the per-target +sections from avocado-sdk-metadata.bb, and the SDK container's own +``[avocado-sdk]`` from avocado-sdk-repos.bb. That is what makes one switch +enough - a section rendered anywhere else would sit at whatever it was +hardcoded to while the rest moved. + +One hardcoded verification value remains and is deliberately out of scope here: +``[main] gpgcheck=True`` in avocado-sdk-repos' dnf.conf. It is dnf's default for +a repo that sets nothing, and every section rendered here sets ``gpgcheck`` +explicitly, so it decides nothing today - it would decide for a repo added to +the container by someone else. """ # Relative to the repository root, which is also what baseurl points at. @@ -55,6 +59,12 @@ def render_repo_section( ``baseurl_path`` is appended to the ``${repo_url}`` placeholder, which is substituted by the client at SDK install time rather than at build time. + ``priority`` of None writes no priority line, leaving the section at dnf's + default of 99. It stays a required argument so that forgetting it is a + TypeError rather than a section that silently sorts below its siblings - + the per-target file ranks its sections 1..N and depends on every one of + them carrying a priority. + Raises: ValueError: ``baseurl_path`` is empty, which would point the client at the feed root, or either verification switch is not a dnf boolean. @@ -83,6 +93,32 @@ def render_repo_section( # same step that publishes the signature, so referencing it while # verification is off points at a 404. lines.append(f"gpgkey={baseurl}/{_KEY_RELPATH}") - lines.append(f"priority={priority}") + if priority is not None: + lines.append(f"priority={priority}") return "\n".join(lines) + "\n\n" + + +def render_sdk_repo_section(*, gpgcheck="0", repo_gpgcheck="0"): + """Return the ``[avocado-sdk]`` block the SDK container's own dnf reads. + + The three literals live here rather than in the recipe because a recipe is + a BitBake task the test suite cannot import: spelled there, a typo in the + section name or the path would pass every test and 404 on the container's + first transaction. + + No priority. avocado-cli merges this file with the per-target repo config + in one transaction (it sets ``reposdir`` to both directories), and dnf + ranks priority globally across that merged set rather than per file. The + per-target sections occupy 1..N, so leaving this at dnf's default of 99 + ranks it below them - which is where the static file this replaced + effectively sat. + """ + return render_repo_section( + section="avocado-sdk", + name="Avocado SDK", + baseurl_path="$releasever/sdk/all", + priority=None, + gpgcheck=gpgcheck, + repo_gpgcheck=repo_gpgcheck, + ) diff --git a/meta-avocado/recipes-avocado/sdk/avocado-sdk-repos.bb b/meta-avocado/recipes-avocado/sdk/avocado-sdk-repos.bb index 49db988b..9688fa67 100644 --- a/meta-avocado/recipes-avocado/sdk/avocado-sdk-repos.bb +++ b/meta-avocado/recipes-avocado/sdk/avocado-sdk-repos.bb @@ -5,17 +5,11 @@ LIC_FILES_CHKSUM = "file://${COMMON_LICENSE_DIR}/Apache-2.0;md5=89aea4e17d99a7ca PV = "${SDK_VERSION}" SRC_URI = " \ - file://avocado-sdk.repo \ file://dnf.conf \ " S = "${WORKDIR}" -REPO_BASE = "${AVOCADO_REPO_BASE}" - -# Monitor AVOCADO_REPO_BASE for changes -vardeps += "AVOCADO_REPO_BASE" - FILES:${PN} += "${sysconfdir}/yum.repos.d/avocado-sdk.repo" inherit update-alternatives @@ -27,11 +21,38 @@ ALTERNATIVE_LINK_NAME[dnf_conf] = "${sysconfdir}/dnf/dnf.conf" ALTERNATIVE_LINK_NAME[rpm_platform] = "${sysconfdir}/rpm/platform" ALTERNATIVE_LINK_NAME[rpmrc] = "${sysconfdir}/rpmrc" +# The [avocado-sdk] section is generated rather than shipped as a static file +# so that the verification switches reach it. A hardcoded gpgcheck=0 would stay +# unverified while every generated section switched over, and the repository +# left out is the one every SDK container reads. +python do_compile() { + import os + # Imported and called fully qualified, not aliased. BitBake records the + # literal call spelling and matches it against the registered name + # avocado_sdk_metadata.repoconf.; an alias never matches, so the + # renderer's source would drop out of this task's hash. + import avocado_sdk_metadata.repoconf + + # Package signatures. Distinct from AVOCADO_REPO_METADATA_GPGCHECK, which + # is the one a signed repomd.xml needs. See + # meta-avocado/lib/avocado_sdk_metadata/repoconf.py for why they differ. + gpg_check = d.getVar('AVOCADO_REPO_GPGCHECK') or '0' + repo_metadata_gpg_check = d.getVar('AVOCADO_REPO_METADATA_GPGCHECK') or '0' + + out_dir = os.path.join(d.getVar('WORKDIR'), 'generated-files') + os.makedirs(out_dir, exist_ok=True) + + with open(os.path.join(out_dir, 'avocado-sdk.repo'), 'w') as repo_f: + repo_f.write(avocado_sdk_metadata.repoconf.render_sdk_repo_section( + gpgcheck=gpg_check, + repo_gpgcheck=repo_metadata_gpg_check, + )) +} + do_install() { # Add Avocado SDK repo install -d ${D}${sysconfdir}/yum.repos.d - install -m 0644 ${WORKDIR}/avocado-sdk.repo ${D}${sysconfdir}/yum.repos.d/avocado-sdk.repo - sed -i "s|{REPO_BASE}|${REPO_BASE}|g" ${D}${sysconfdir}/yum.repos.d/avocado-sdk.repo + install -m 0644 ${WORKDIR}/generated-files/avocado-sdk.repo ${D}${sysconfdir}/yum.repos.d/avocado-sdk.repo install -d ${D}${sysconfdir}/dnf install -m 644 ${WORKDIR}/dnf.conf ${D}${sysconfdir}/dnf/dnf.conf.${PN} diff --git a/meta-avocado/recipes-avocado/sdk/avocado-sdk-repos/avocado-sdk.repo b/meta-avocado/recipes-avocado/sdk/avocado-sdk-repos/avocado-sdk.repo deleted file mode 100644 index 1ca86dfd..00000000 --- a/meta-avocado/recipes-avocado/sdk/avocado-sdk-repos/avocado-sdk.repo +++ /dev/null @@ -1,5 +0,0 @@ -[avocado-sdk] -name=Avocado SDK -baseurl=${repo_url}/$releasever/sdk/all -enabled=1 -gpgcheck=0 diff --git a/meta-avocado/tests/test_repoconf.py b/meta-avocado/tests/test_repoconf.py index d55f51b4..3ba1f60c 100644 --- a/meta-avocado/tests/test_repoconf.py +++ b/meta-avocado/tests/test_repoconf.py @@ -115,6 +115,97 @@ def test_priority_is_rendered_from_an_integer(self): self.assertEqual(parse(self._render(priority=11))["priority"], "11") +class PriorityIsOmissibleButNotOptional(unittest.TestCase): + """A section can decline to rank itself; a caller cannot decline to say so. + + dnf ranks priority globally across every repo it loads, defaulting to 99. + The per-target file ranks its own sections 1..N and breaks if one of them + silently lands at 99, so omitting the argument stays a TypeError while + passing None explicitly writes no line. + """ + + def test_passing_none_writes_no_priority_line(self): + text = repoconf.render_repo_section( + section="s", name="n", baseurl_path="p", priority=None + ) + + self.assertNotIn("priority", parse(text)) + + def test_omitting_the_argument_is_still_an_error(self): + # The guard the per-target file depends on: a section added there + # without a priority must not render, it must fail. + with self.assertRaises(TypeError): + repoconf.render_repo_section(section="s", name="n", baseurl_path="p") + + def test_a_zero_priority_is_still_written(self): + # Only None omits the line. Zero is a real dnf priority and must not + # be swallowed by a falsy test. + text = repoconf.render_repo_section( + section="s", name="n", baseurl_path="p", priority=0 + ) + + self.assertEqual(parse(text)["priority"], "0") + + def test_the_separator_survives_the_missing_line(self): + # The priority line was last, so dropping it must not take the blank + # separator with it. + text = repoconf.render_repo_section( + section="s", name="n", baseurl_path="p", priority=None + ) + + self.assertTrue(text.endswith("\n\n")) + + +class SdkRepoSection(unittest.TestCase): + """Pin the block the SDK container's own dnf reads. + + These literals used to be a checked-in file a reviewer could diff. Now they + are code, so this is what stands between a typo in the section name or the + path and a container whose first dnf transaction 404s. + """ + + def test_the_whole_block_is_pinned(self): + self.assertEqual( + repoconf.render_sdk_repo_section(), + "[avocado-sdk]\n" + "name=Avocado SDK\n" + "baseurl=${repo_url}/$releasever/sdk/all\n" + "enabled=1\n" + "gpgcheck=0\n" + "repo_gpgcheck=0\n" + "\n", + ) + + def test_it_declines_to_rank_itself(self): + # avocado-cli merges this file with the per-target config in one + # transaction, where the target sections hold 1..N. Staying at dnf's + # default of 99 keeps this below them, as the static file it replaced + # effectively was. + self.assertNotIn("priority", parse(repoconf.render_sdk_repo_section())) + + def test_metadata_verification_reaches_it(self): + # The whole point of routing this section through the renderer: the + # switch has to reach the one repo every SDK container reads. + parsed = parse(repoconf.render_sdk_repo_section(repo_gpgcheck="1")) + + self.assertEqual(parsed["repo_gpgcheck"], "1") + self.assertEqual( + parsed["gpgkey"], + "${repo_url}/$releasever/sdk/all/repodata/repomd.xml.key", + ) + + def test_package_signature_checking_reaches_it_independently(self): + parsed = parse(repoconf.render_sdk_repo_section(gpgcheck="1")) + + self.assertEqual(parsed["gpgcheck"], "1") + self.assertEqual(parsed["repo_gpgcheck"], "0") + + def test_a_bad_switch_value_is_rejected_here_too(self): + # The wrapper must not become a way around the boolean check. + with self.assertRaises(ValueError): + repoconf.render_sdk_repo_section(repo_gpgcheck="yes") + + class RenderRepoSectionRejectsBadInput(unittest.TestCase): def test_an_empty_baseurl_path_is_rejected(self): # A section with no path silently points every client at the feed root.