Re: [PATCH v3 6/6] If we use a custom keyring debootstrap may fall to https

[Henning Schild <henning.schild@siemens.com>]

Am Wed, 6 Mar 2019 17:26:18 +0100
schrieb "[ext] Andreas J. Reichel" <andreas.reichel.ext@siemens.com>:

> From: Andreas Reichel <andreas.reichel.ext@siemens.com>
> 
> See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=891009
> 
> So if we have something in aptkeyring, append https-support to
> OVERRIDES.
> 
> Furthermore, the conditional append for https-support was missing
> in qemuamd64-stretch.conf, thus, remove this from all the distros
> and put it into the isar-bootstrap.inc.
> 
> Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com>
> ---
>  meta-isar/conf/multiconfig/qemuamd64-buster.conf    |  1 -
>  meta-isar/conf/multiconfig/qemuamd64-jessie.conf    |  1 -
>  meta/recipes-core/isar-bootstrap/isar-bootstrap.inc | 11 +++++++++++
>  3 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/meta-isar/conf/multiconfig/qemuamd64-buster.conf
> b/meta-isar/conf/multiconfig/qemuamd64-buster.conf index
> 63df75c..da90993 100644 ---
> a/meta-isar/conf/multiconfig/qemuamd64-buster.conf +++
> b/meta-isar/conf/multiconfig/qemuamd64-buster.conf @@ -18,4 +18,3 @@
> QEMU_MACHINE ?= "q35" QEMU_CPU ?= ""
>  QEMU_DISK_ARGS ?= "-hda ##ROOTFS_IMAGE##
> -bios /usr/local/share/ovmf/OVMF.fd" 
> -DISTRO_BOOTSTRAP_BASE_PACKAGES_append_https-support = "
> apt-transport-https ca-certificates" diff --git
> a/meta-isar/conf/multiconfig/qemuamd64-jessie.conf
> b/meta-isar/conf/multiconfig/qemuamd64-jessie.conf index
> d1335ff..42c71df 100644 ---
> a/meta-isar/conf/multiconfig/qemuamd64-jessie.conf +++
> b/meta-isar/conf/multiconfig/qemuamd64-jessie.conf @@ -15,4 +15,3 @@
> QEMU_MACHINE ?= "pc" QEMU_CPU ?= "" QEMU_DISK_ARGS ?= "-hda
> ##ROOTFS_IMAGE##" 
> -DISTRO_BOOTSTRAP_BASE_PACKAGES_append_https-support = "
> apt-transport-https ca-certificates" diff --git
> a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
> 8002a53..64cefc6 100644 ---
> a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -25,6 +25,7
> @@ BASEAPTSRCS = "${WORKDIR}/base-apt-sources" APTKEYFILES = ""
> DEPLOY_ISAR_BOOTSTRAP ?= "" DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales
> gnupg2" +DISTRO_BOOTSTRAP_BASE_PACKAGES_append_https-support = "
> apt-transport-https ca-certificates" 
>  DISTRO_APT_PREMIRRORS ?= "${@ "http://ftp\.(\S+\.)?debian.org
> file:///${REPO_BASE_DIR} \n" if
> bb.utils.to_boolean(d.getVar('ISAR_USE_CACHED_BASE_REPO')) else "" }"
> @@ -41,6 +42,12 @@ python () { if own_pub_key:
>              aptkeys += own_pub_key.split()
>  
> +    if len(aptkeys) > 0:
> +        # debootstrap falls back to https if there is no
> +        # 'reliable' keyring, whatever that means, but it happened
> +        # https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=891009
> +        d.setVar("HAVE_CUSTOM_APT_KEYS", "True")

Why this indirection and the new variable? Can you not just check
len(aptkeys) where you check HAVE_CUSTOM_APT_KEYS?

In fact you probably want to look at DISTRO_APT_KEYS, which seems to be
the source feeding all the other variables. Whatever you come up with
in v3 4/6.

>      for key in aptkeys:
>          d.appendVar("SRC_URI", " %s" % key)
>          fetcher = bb.fetch2.Fetch([key], d)
> @@ -150,6 +157,10 @@ def get_distro_have_https_source(d,
> is_host=False): return any(source[2].startswith("https://") for
> source in generate_distro_sources(d, is_host)) 
>  def get_distro_needs_https_support(d, is_host=False):
> +    apt_keys = d.getVar("HAVE_CUSTOM_APT_KEYS", False)
> +    if apt_keys:
> +        return "https-support"
> +

Reusing the OVERRIDE seems like a good idea to avoid double adding the
packages. So gnupg implies https but does not add the packages again.
But i think i would do something like

get_gnupg_overrides() 
 ret = "gnupg"
 # blabla bug
 ret += " https-support"

Henning

>      if get_distro_have_https_source(d, is_host):
>          return "https-support"
>      else: