Generate the SDK container's repo section, and hash the renderer

.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'

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.
 
 ---
 

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"]

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,
+    )

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.<func>; 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",

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.<func>; 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}

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.