Re: [meta-virtualization] [PATCH v2 00/13] Container Improvements

[Bruce Ashfield <bruce.ashfield@gmail.com>]

Hi Tim,

Thanks for the substantial rework. Reviewed the whole series against the
v1 comments, and the two new pieces you factored out (container-volatile-fixup
and container-dev-mode) look great. Series has been applied on master.

Here's my summary, just so we can have it archived and use it as a reference
later if something breaks:

v1 items — status
-----------------

Addressed:

  * EXTRA_USERS_PARAMS uses += now (was = in v1)
  * PYEOF single-quoted heredoc has the "bitbake pre-expansion" comment
  * NONROOT_USER bare-identifier requirement documented
  * fix_oci_home_perms has the index.json guard + OCI_IMAGE_TAR_OUTPUT
    gate + bbfatal if the layout ever changes
  * IMAGE_INSTALL is no longer duplicated against OCI_LAYERS packages
    across the image recipes (image-oci auto-derivation on master
    handles it)
  * rootfs_fixup_var_volatile is factored into
    container-volatile-fixup.bbclass and app-container-python inherits
    it (was missing in v1)
  * container-dev-mode.bbclass cleans up the '-dev' boilerplate that
    each recipe previously repeated
  * app-container-valkey's persistence paths (/data, /var/lib/valkey,
    /var/log/valkey, /run/valkey) are in NONROOT_OWNED_DIRS
  * The valkey security-caveats item is moot in v2 — the
    container-entrypoint.sh design supersedes the always-on
    protected-mode override I had asked to be flagged in DESCRIPTION

I did notice a few things on the way through, but it was more efficient
if I just did them myself.

All three are behavior-neutral cleanups:

1. app-container-valkey: drop redundant OCI_IMAGE_RUNTIME_UID

   The recipe carries a verbatim copy of the same
     OCI_IMAGE_RUNTIME_UID = "${@bb.utils.contains('PACKAGECONFIG',
                                'dev', '0', '${NONROOT_UID}', d)}"
   line that container-dev-mode.bbclass now supplies. Leftover from v1.

2. app-container-mosquitto: fix PACKAGECONFIG 'dev' comment

   The block above PACKAGECONFIG was copied from app-container-python
   and mentions python3-pip / site-packages /
   pn-app-container-python — none of which apply. Rewrote it per-recipe.
   nginx and curl in the same series already have the correct wording.

3. container-nonroot-user.bbclass: document + assert inherit order

   oci_nonroot_inject_user() is a do_image_oci prefunc that reads
   OCI_LAYER_*_ROOTFS. Those vars are populated by image-oci's OWN
   do_image_oci prefunc (oci_multilayer_install_packages), and prefunc
   execution follows inherit order. If a future recipe inherits
   container-nonroot-user before image-oci, injection runs against
   empty per-layer state, no layer ships /etc/passwd, and the only
   signal is a bb.warn that reviewers can miss. Added a header comment
   spelling out the requirement + an anonymous-python guard that
   raises bb.fatal at parse time. All five in-tree consumers already
   inherit in the right order, so this catches a future mistake rather
   than fixing a current one.

Also — the cover letter says "verify '-dev' mode switches from root
user to mosquitto user", but the recipe doesn't set
NONROOT_USER = "mosquitto" (unlike nginx which does set it to "nginx").
So either the cover letter is aspirational or something is happening
implicitly that I'm missing. Any changes we can do on top of the
series. I've put it on master-next for visibility.

Bruce