[Buildroot] [PATCH 0/5] utils/docker-run: cleanups and improvements (branch yem/docker-run)

[Buildroot] [PATCH 0/5] utils/docker-run: cleanups and improvements (branch yem/docker-run)

[Yann E. MORIN]

Hello All!

This 4-patch series improves on our docker-run wrapper:

  - utils/docker-run: introduce sorted list of mountpoints
  - utils/docker-run: mount the download directory if specified
  - utils/docker-run: bind mount .git/objects if needed
  - utils/docker-run: also mount current working directory
  - utils/docker-run: make it compatible with SELinux

It gathers changes from Thomas and Nicolas.

Regards,
Yann E. MORIN.


----------------------------------------------------------------
Nicolas Boichat (1):
      utils/docker-run: bind mount .git/objects if needed

Thomas Petazzoni (1):
      utils/docker-run: mount the download directory if specified

Yann E. MORIN (3):
      utils/docker-run: introduce sorted list of mountpoints
      utils/docker-run: also mount current working directory
      utils/docker-run: make it compatible with SELinux

 utils/docker-run | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 1/5] utils/docker-run: introduce sorted list of mountpoints

[Yann E. MORIN]

For now, we only ever mount two mountpoints, the main directory (i.e.
the working copy), and the git directory.

To pave the way for adding new mountpoints, we introduce a list of them,
that we sort to ensure that we never mount a shallower mounpoint after a
deeper one (that would shadow the deeper mountpoint).

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 utils/docker-run | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/utils/docker-run b/utils/docker-run
index ab95e61e84..2f689a1fbd 100755
--- a/utils/docker-run
+++ b/utils/docker-run
@@ -17,10 +17,11 @@ declare -a docker_opts=(
     -i
     --rm
     --user "$(id -u):$(id -g)"
-    --mount "type=bind,src=${MAIN_DIR},dst=${MAIN_DIR}"
     --workdir "${MAIN_DIR}"
 )
 
+declare -a mountpoints=( "${MAIN_DIR}" )
+
 # Empty GIT_DIR means that we are not in a workdir, *and* git is too old
 # to know about worktrees, so we're not in a worktree either. So it means
 # we're in the main git working copy, and thus we don't need to mount the
@@ -31,9 +32,14 @@ if [ "${GIT_DIR}" ]; then
     # not absolute, GIT_DIR is relative to MAIN_DIR. If it's an absolute
     # path already (in a wordir), then that's a noop.
     GIT_DIR="$(cd "${MAIN_DIR}"; readlink -e "${GIT_DIR}")"
-    docker_opts+=( --mount "type=bind,src=${GIT_DIR},dst=${GIT_DIR}" )
+    mountpoints+=( "${GIT_DIR}" )
 fi
 
+# shellcheck disable=SC2013 # can't use while-read because of the assignment
+for dir in $(printf '%s\n' "${mountpoints[@]}" |LC_ALL=C sort -u); do
+    docker_opts+=( --mount "type=bind,src=${dir},dst=${dir}" )
+done
+
 if tty -s; then
     docker_opts+=( -t )
 fi
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 2/5] utils/docker-run: mount the download directory if specified

[Yann E. MORIN]

From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

If the user has defined $BR2_DL_DIR in the environment, it would be
nice to have it accessible inside the Docker container, and the
BR2_DL_DIR environment variable set to access it.

This commit does exactly this: it mounts the host $BR2_DL_DIR as /dl
in the container, and sets BR2_DL_DIR=/dl in the container.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
[yann.morin.1998@free.fr: use the new mountpoints list]
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
 utils/docker-run | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/utils/docker-run b/utils/docker-run
index 2f689a1fbd..7b27a9b05c 100755
--- a/utils/docker-run
+++ b/utils/docker-run
@@ -35,6 +35,11 @@ if [ "${GIT_DIR}" ]; then
     mountpoints+=( "${GIT_DIR}" )
 fi
 
+if [ "${BR2_DL_DIR}" ]; then
+    mountpoints+=( "${BR2_DL_DIR}" )
+    docker_opts+=( --env BR2_DL_DIR )
+fi
+
 # shellcheck disable=SC2013 # can't use while-read because of the assignment
 for dir in $(printf '%s\n' "${mountpoints[@]}" |LC_ALL=C sort -u); do
     docker_opts+=( --mount "type=bind,src=${dir},dst=${dir}" )
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 3/5] utils/docker-run: bind mount .git/objects if needed

[Yann E. MORIN]

From: Nicolas Boichat <drinkcat@google.com>

If buildroot is checked out as part of a 'repo' manifest, docker-run
doesn't fully bind mount the .git directory, leading to commands such
as `utils/docker-run make check-package` to fail.

Signed-off-by: Nicolas Boichat <drinkcat@google.com>
[yann.morin.1998@free.fr: use newly introduced mountpoints list]
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
 utils/docker-run | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/utils/docker-run b/utils/docker-run
index 7b27a9b05c..ff903b96e9 100755
--- a/utils/docker-run
+++ b/utils/docker-run
@@ -33,6 +33,12 @@ if [ "${GIT_DIR}" ]; then
     # path already (in a wordir), then that's a noop.
     GIT_DIR="$(cd "${MAIN_DIR}"; readlink -e "${GIT_DIR}")"
     mountpoints+=( "${GIT_DIR}" )
+
+    # 'repo' stores .git/objects separately.
+    if [ -L "${GIT_DIR}/objects" ]; then
+        OBJECTS_DIR="$(cd "${MAIN_DIR}"; readlink -e "${GIT_DIR}/objects")"
+        mountpoints+=( "${OBJECTS_DIR}" )
+    fi
 fi
 
 if [ "${BR2_DL_DIR}" ]; then
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/5] utils/docker-run: bind mount .git/objects if needed

[Nicolas Boichat via buildroot]

[-- Attachment #1.1: Type: text/plain, Size: 1479 bytes --]

On Thu, Aug 10, 2023 at 5:24 AM Yann E. MORIN <yann.morin.1998@free.fr>
wrote:

> From: Nicolas Boichat <drinkcat@google.com>
>
> If buildroot is checked out as part of a 'repo' manifest, docker-run
> doesn't fully bind mount the .git directory, leading to commands such
> as `utils/docker-run make check-package` to fail.
>
> Signed-off-by: Nicolas Boichat <drinkcat@google.com>
> [yann.morin.1998@free.fr: use newly introduced mountpoints list]
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> ---
>  utils/docker-run | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/utils/docker-run b/utils/docker-run
> index 7b27a9b05c..ff903b96e9 100755
> --- a/utils/docker-run
> +++ b/utils/docker-run
> @@ -33,6 +33,12 @@ if [ "${GIT_DIR}" ]; then
>      # path already (in a wordir), then that's a noop.
>      GIT_DIR="$(cd "${MAIN_DIR}"; readlink -e "${GIT_DIR}")"
>      mountpoints+=( "${GIT_DIR}" )
> +
> +    # 'repo' stores .git/objects separately.
> +    if [ -L "${GIT_DIR}/objects" ]; then
> +        OBJECTS_DIR="$(cd "${MAIN_DIR}"; readlink -e
> "${GIT_DIR}/objects")"
>

I intentionally removed `cd "${MAIN_DIR}"` in my patch, as ${GIT_DIR} has
already been made an absolute path, just above, so the cd operation is
unnecessary.


> +        mountpoints+=( "${OBJECTS_DIR}" )
>

Otherwise, that's a nice cleanup ,-)

Thanks!


> +    fi
>  fi
>
>  if [ "${BR2_DL_DIR}" ]; then
> --
> 2.25.1
>
>

[-- Attachment #1.2: Type: text/html, Size: 2625 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/5] utils/docker-run: bind mount .git/objects if needed

[Yann E. MORIN]

Nicolas, All,

On 2023-08-10 08:23 +0800, Nicolas Boichat via buildroot spake thusly:
> On Thu, Aug 10, 2023 at 5:24 AM Yann E. MORIN < [1]yann.morin.1998@free.fr> wrote:
>   From: Nicolas Boichat < [2]drinkcat@google.com>
> 
>   If buildroot is checked out as part of a 'repo' manifest, docker-run
>   doesn't fully bind mount the .git directory, leading to commands such
>   as `utils/docker-run make check-package` to fail.
> 
>   Signed-off-by: Nicolas Boichat < [3]drinkcat@google.com>
>   [ [4]yann.morin.1998@free.fr: use newly introduced mountpoints list]
>   Signed-off-by: Yann E. MORIN < [5]yann.morin.1998@free.fr>
>   ---
>    utils/docker-run | 6 ++++++
>    1 file changed, 6 insertions(+)
> 
>   diff --git a/utils/docker-run b/utils/docker-run
>   index 7b27a9b05c..ff903b96e9 100755
>   --- a/utils/docker-run
>   +++ b/utils/docker-run
>   @@ -33,6 +33,12 @@ if [ "${GIT_DIR}" ]; then
>        # path already (in a wordir), then that's a noop.
>        GIT_DIR="$(cd "${MAIN_DIR}"; readlink -e "${GIT_DIR}")"
>        mountpoints+=( "${GIT_DIR}" )
>   +
>   +    # 'repo' stores .git/objects separately.
>   +    if [ -L "${GIT_DIR}/objects" ]; then
>   +        OBJECTS_DIR="$(cd "${MAIN_DIR}"; readlink -e "${GIT_DIR}/objects")"
> 
> I intentionally removed `cd "${MAIN_DIR}"` in my patch, as ${GIT_DIR}
> has already been made an absolute path, just above, so the cd
> operation is unnecessary.

That's right. But for symetry, I preferred to use the same construct.
True, that's superfluous, but at least we don;t have to think too hard
about it when we look at the code.

Thanks for the feedback; I should probably have mentioned that in the
commit log.

Regards,
Yann E. MORIN.

>   +        mountpoints+=( "${OBJECTS_DIR}" )
> 
> Otherwise, that's a nice cleanup ,-)
> Thanks!
>  
> 
>   +    fi
>    fi
> 
>    if [ "${BR2_DL_DIR}" ]; then
>   --
>   2.25.1
> 
> Links:
> 1. mailto:yann.morin.1998@free.fr
> 2. mailto:drinkcat@google.com
> 3. mailto:drinkcat@google.com
> 4. mailto:yann.morin.1998@free.fr
> 5. mailto:yann.morin.1998@free.fr

> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot


-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 4/5] utils/docker-run: also mount current working directory

[Yann E. MORIN]

Currently, using utils/docker-run expects that the current working
directory is the working copy. This means that it is not possible
to use docker-run with an out-of-tree build (one using O=).

Add the current working directory to the list of mountpoints, and
use that as working directory in the container.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 utils/docker-run | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/utils/docker-run b/utils/docker-run
index ff903b96e9..83938f8a7a 100755
--- a/utils/docker-run
+++ b/utils/docker-run
@@ -17,10 +17,13 @@ declare -a docker_opts=(
     -i
     --rm
     --user "$(id -u):$(id -g)"
-    --workdir "${MAIN_DIR}"
+    --workdir "$(pwd)"
 )
 
-declare -a mountpoints=( "${MAIN_DIR}" )
+declare -a mountpoints=(
+    "${MAIN_DIR}"
+    "$(pwd)"
+)
 
 # Empty GIT_DIR means that we are not in a workdir, *and* git is too old
 # to know about worktrees, so we're not in a worktree either. So it means
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 5/5] utils/docker-run: make it compatible with SELinux

[Yann E. MORIN]

After switching to a fresh Fedora 38 installation with SELinux disabled,
we noticed that utils/docker-run doesn't work as the applications
running inside the container are not allowed to accept the data mounted
through the bind mount.

Since we do not really need to isolate and confine the build, but rather
to provide a known environment, we don;t really need to enforce any
SELinux confinment in the container.

So, we tell docker to turn off label confinement for the container:

    https://manpages.org/docker-run

    --security-opt=[]
      Security Options
        [...]
        "label=disable"     : Turn off label confinement for the container

Suggested-by: Antoine Tenart <atenart@kernel.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
[yann.morin.1998@free.fr: use Antoine's proposal]
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
 utils/docker-run | 1 +
 1 file changed, 1 insertion(+)

diff --git a/utils/docker-run b/utils/docker-run
index 83938f8a7a..02cf68c946 100755
--- a/utils/docker-run
+++ b/utils/docker-run
@@ -18,6 +18,7 @@ declare -a docker_opts=(
     --rm
     --user "$(id -u):$(id -g)"
     --workdir "$(pwd)"
+    --security-opt label=disable
 )
 
 declare -a mountpoints=(
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 0/5] utils/docker-run: cleanups and improvements (branch yem/docker-run)

[Yann E. MORIN]

All,

On 2023-08-09 23:24 +0200, Yann E. MORIN spake thusly:
> Hello All!
> 
> This 4-patch series improves on our docker-run wrapper:
> 
>   - utils/docker-run: introduce sorted list of mountpoints
>   - utils/docker-run: mount the download directory if specified
>   - utils/docker-run: bind mount .git/objects if needed
>   - utils/docker-run: also mount current working directory
>   - utils/docker-run: make it compatible with SELinux
> 
> It gathers changes from Thomas and Nicolas.

Whole series applied to next, thanks.

Regards,
Yann E. MORIN.

> Regards,
> Yann E. MORIN.
> 
> 
> ----------------------------------------------------------------
> Nicolas Boichat (1):
>       utils/docker-run: bind mount .git/objects if needed
> 
> Thomas Petazzoni (1):
>       utils/docker-run: mount the download directory if specified
> 
> Yann E. MORIN (3):
>       utils/docker-run: introduce sorted list of mountpoints
>       utils/docker-run: also mount current working directory
>       utils/docker-run: make it compatible with SELinux
> 
>  utils/docker-run | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> -- 
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot