| Message ID | 20180106152833.10111-1-yann.morin.1998@free.fr |
|---|---|
| State | Accepted |
| Headers | show |
| Series | support/dockerfile: add directives to run as non-root | expand |
Hello, On Sat, 6 Jan 2018 16:28:33 +0100, Yann E. MORIN wrote: > Currently, our jobs on the gitlab-ci infra are running as root, which is > problematic for two reasons: > > - this is not the usual way Buildroot is built; > - it may miss issues where running as non-root is problematic. > > So, complement our Dockerfile with directives to add a new user and run > everything as that user, as demonstrated by this build job: > https://gitlab.com/ymorin/buildroot-ci/-/jobs/46929562 > > Additional, enforce an UTF-8 locale while running. > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > Cc: Arnout Vandecappelle <arnout@mind.be> > Cc: Peter Korsgaard <peter@korsgaard.com> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > support/docker/Dockerfile | 7 +++++++ > 1 file changed, 7 insertions(+) Applied to master, thanks. Could you update the official Buildroot Docker image, so that the Gitlab CI testing benefits from this ? Thanks! Thomas
Thomas, All, On 2018-01-12 22:54 +0100, Thomas Petazzoni spake thusly: > On Sat, 6 Jan 2018 16:28:33 +0100, Yann E. MORIN wrote: > > Currently, our jobs on the gitlab-ci infra are running as root, which is > > problematic for two reasons: > > > > - this is not the usual way Buildroot is built; > > - it may miss issues where running as non-root is problematic. > > > > So, complement our Dockerfile with directives to add a new user and run > > everything as that user, as demonstrated by this build job: > > https://gitlab.com/ymorin/buildroot-ci/-/jobs/46929562 > > > > Additional, enforce an UTF-8 locale while running. > > > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > Cc: Arnout Vandecappelle <arnout@mind.be> > > Cc: Peter Korsgaard <peter@korsgaard.com> > > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > > --- > > support/docker/Dockerfile | 7 +++++++ > > 1 file changed, 7 insertions(+) > > Applied to master, thanks. Could you update the official Buildroot > Docker image, so that the Gitlab CI testing benefits from this ? I would if I could, but I am not supposed to have push-access to that image, have I? Regards, Yann E. MORIN. > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com
Hello, On Sat, 13 Jan 2018 12:28:43 +0100, Yann E. MORIN wrote: > On 2018-01-12 22:54 +0100, Thomas Petazzoni spake thusly: > > On Sat, 6 Jan 2018 16:28:33 +0100, Yann E. MORIN wrote: > > > Currently, our jobs on the gitlab-ci infra are running as root, which is > > > problematic for two reasons: > > > > > > - this is not the usual way Buildroot is built; > > > - it may miss issues where running as non-root is problematic. > > > > > > So, complement our Dockerfile with directives to add a new user and run > > > everything as that user, as demonstrated by this build job: > > > https://gitlab.com/ymorin/buildroot-ci/-/jobs/46929562 > > > > > > Additional, enforce an UTF-8 locale while running. > > > > > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > > Cc: Arnout Vandecappelle <arnout@mind.be> > > > Cc: Peter Korsgaard <peter@korsgaard.com> > > > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > > > --- > > > support/docker/Dockerfile | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > Applied to master, thanks. Could you update the official Buildroot > > Docker image, so that the Gitlab CI testing benefits from this ? > > I would if I could, but I am not supposed to have push-access to that > image, have I? I am not sure how that works. Hopefully Arnout can give some details about this :) Best regards, Thomas
On 14-01-18 14:02, Thomas Petazzoni wrote: > Hello, > > On Sat, 13 Jan 2018 12:28:43 +0100, Yann E. MORIN wrote: > >> On 2018-01-12 22:54 +0100, Thomas Petazzoni spake thusly: >>> On Sat, 6 Jan 2018 16:28:33 +0100, Yann E. MORIN wrote: >>>> Currently, our jobs on the gitlab-ci infra are running as root, which is >>>> problematic for two reasons: >>>> >>>> - this is not the usual way Buildroot is built; >>>> - it may miss issues where running as non-root is problematic. >>>> >>>> So, complement our Dockerfile with directives to add a new user and run >>>> everything as that user, as demonstrated by this build job: >>>> https://gitlab.com/ymorin/buildroot-ci/-/jobs/46929562 >>>> >>>> Additional, enforce an UTF-8 locale while running. >>>> >>>> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> >>>> Cc: Arnout Vandecappelle <arnout@mind.be> >>>> Cc: Peter Korsgaard <peter@korsgaard.com> >>>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> >>>> --- >>>> support/docker/Dockerfile | 7 +++++++ >>>> 1 file changed, 7 insertions(+) >>> >>> Applied to master, thanks. Could you update the official Buildroot >>> Docker image, so that the Gitlab CI testing benefits from this ? >> >> I would if I could, but I am not supposed to have push-access to that >> image, have I? > > I am not sure how that works. Hopefully Arnout can give some details > about this :) The idea was that more people than just me would create an account on hub.docker.io, and I would add them to the buildroot organisation. For now, I've pushed the updated docker image. Regards, Arnout
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: > Currently, our jobs on the gitlab-ci infra are running as root, which is > problematic for two reasons: > - this is not the usual way Buildroot is built; > - it may miss issues where running as non-root is problematic. > So, complement our Dockerfile with directives to add a new user and run > everything as that user, as demonstrated by this build job: > https://gitlab.com/ymorin/buildroot-ci/-/jobs/46929562 > Additional, enforce an UTF-8 locale while running. > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > Cc: Arnout Vandecappelle <arnout@mind.be> > Cc: Peter Korsgaard <peter@korsgaard.com> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > support/docker/Dockerfile | 7 +++++++ > 1 file changed, 7 insertions(+) > diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile > index d45b23dc23..ebb471f7e5 100644 > --- a/support/docker/Dockerfile > +++ b/support/docker/Dockerfile > @@ -28,3 +28,10 @@ RUN apt-get -q -y clean > RUN sed -i 's/# \(en_US.UTF-8\)/\1/' /etc/locale.gen > RUN /usr/sbin/locale-gen > +RUN useradd -ms /bin/bash br-user > +RUN chown -R br-user:br-user /home/br-user I don't know much about Docker, but I was of the understanding that each run statement creates a new layer and the number of layers should be minimized, which is why you normally see stuff like: RUN foo && \ bar && \ foz && \ baz E.G. from the official documentation: https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#minimize-the-number-of-layers
Peter, All, On 2018-02-03 22:47 +0100, Peter Korsgaard spake thusly: > >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: [--SNIP--] > > diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile > > index d45b23dc23..ebb471f7e5 100644 > > --- a/support/docker/Dockerfile > > +++ b/support/docker/Dockerfile > > @@ -28,3 +28,10 @@ RUN apt-get -q -y clean > > RUN sed -i 's/# \(en_US.UTF-8\)/\1/' /etc/locale.gen > > RUN /usr/sbin/locale-gen > > > +RUN useradd -ms /bin/bash br-user > > +RUN chown -R br-user:br-user /home/br-user > > I don't know much about Docker, but I was of the understanding that each > run statement creates a new layer and the number of layers should be > minimized, which is why you normally see stuff like: > > RUN foo && \ > bar && \ > foz && \ > baz Well, I am no docker expert either, and I just mimicked whatever the file already looked like, as you can see for the previous two lines... > E.G. from the official documentation: > https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#minimize-the-number-of-layers It's always time for a docker expert to send a fixup patch, eh! ;-) Or I can have a look if I again get bored not to be in BRU... ;-] Regards, Yann E. MORIN.
Peter, All, On 2018-02-04 11:04 +0100, Yann E. MORIN spake thusly: > On 2018-02-03 22:47 +0100, Peter Korsgaard spake thusly: > > I don't know much about Docker, but I was of the understanding that each > > run statement creates a new layer and the number of layers should be > > minimized, which is why you normally see stuff like: [--SNIP--] > It's always time for a docker expert to send a fixup patch, eh! ;-) > Or I can have a look if I again get bored not to be in BRU... ;-] There, I *really* got bored not being at FOSDEM... https://patchwork.ozlabs.org/project/buildroot/list/?series=26863 Regards, Yann E. MORIN.
On 4 February 2018 at 21:04, Yann E. MORIN <yann.morin.1998@free.fr> wrote: > Peter, All, > > On 2018-02-03 22:47 +0100, Peter Korsgaard spake thusly: >> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: > [--SNIP--] >> > diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile >> > index d45b23dc23..ebb471f7e5 100644 >> > --- a/support/docker/Dockerfile >> > +++ b/support/docker/Dockerfile >> > @@ -28,3 +28,10 @@ RUN apt-get -q -y clean >> > RUN sed -i 's/# \(en_US.UTF-8\)/\1/' /etc/locale.gen >> > RUN /usr/sbin/locale-gen >> >> > +RUN useradd -ms /bin/bash br-user >> > +RUN chown -R br-user:br-user /home/br-user >> >> I don't know much about Docker, but I was of the understanding that each >> run statement creates a new layer and the number of layers should be >> minimized, which is why you normally see stuff like: >> >> RUN foo && \ >> bar && \ >> foz && \ >> baz > > Well, I am no docker expert either, and I just mimicked whatever the > file already looked like, as you can see for the previous two lines... Minimizing the number of layers for simple layers like the "RUN useradd" and "RUN chown" really doesn't save very much. Where the big win comes in (and is not mentioned in the official documentation) is when you keep the "install && build && clean" commands in a single RUN command. This matters because if you split it up over multiple layers, the earlier layers still contain all the stuff the later layers try to clean. The end result is a fat image that contains the stuff you wanted to remove in a lower layer, with whiteout entries in a higher layer, so it only looks like the files have been removed. Your patch set does the right thing WRT the debian install/clean, so all good, but I thought I'd mention this because it did not come up in any discussions here or in the linked docs.
Cam, All, On 2018-02-05 14:52 +1100, Cam Hutchison spake thusly: > On 4 February 2018 at 21:04, Yann E. MORIN <yann.morin.1998@free.fr> wrote: [--SNIP--] > >> I don't know much about Docker, but I was of the understanding that each > >> run statement creates a new layer and the number of layers should be > >> minimized, which is why you normally see stuff like: [--SNIP--] > Minimizing the number of layers for simple layers like the "RUN useradd" > and "RUN chown" really doesn't save very much. > > Where the big win comes in (and is not mentioned in the official > documentation) is when you keep the "install && build && clean" commands > in a single RUN command. This matters because if you split it up over > multiple layers, the earlier layers still contain all the stuff the > later layers try to clean. The end result is a fat image that contains > the stuff you wanted to remove in a lower layer, with whiteout entries > in a higher layer, so it only looks like the files have been removed. > > Your patch set does the right thing WRT the debian install/clean, so all > good, but I thought I'd mention this because it did not come up in any > discussions here or in the linked docs. OK, so we *do* have a Docker expert, now! ;-) Thank you for your explanations, that was very instructive. I gues that's because aufs is used underneath, or something like that? I would have expected that, instead of implicit, a layer would have been explicit, like with a COMMIT keyword or something like that... Anyway, it all makes sense in hindsight, indeed. Thanks! Regards, Yann E. MORIN.
On 5 February 2018 at 18:18, Yann E. MORIN <yann.morin.1998@free.fr> wrote: > Cam, All, > > On 2018-02-05 14:52 +1100, Cam Hutchison spake thusly: >> >> Where the big win comes in (and is not mentioned in the official >> documentation) is when you keep the "install && build && clean" commands >> in a single RUN command. This matters because if you split it up over >> multiple layers, the earlier layers still contain all the stuff the >> later layers try to clean. The end result is a fat image that contains >> the stuff you wanted to remove in a lower layer, with whiteout entries >> in a higher layer, so it only looks like the files have been removed. >> >> Your patch set does the right thing WRT the debian install/clean, so all >> good, but I thought I'd mention this because it did not come up in any >> discussions here or in the linked docs. > > OK, so we *do* have a Docker expert, now! ;-) Hah. I know enough to be dangerous, but I'm hardly an expert. I am experimenting with docker in buildroot at the moment though, so I'm sure to learn more. > Thank you for your explanations, that was very instructive. I gues > that's because aufs is used underneath, or something like that? Overlayfs these days, but yes. Each layer is another overlay and once a layer is made, it is immutable. Higher layers just modify the ultimate view. > I would have expected that, instead of implicit, a layer would have been > explicit, like with a COMMIT keyword or something like that... I've found Dockerfiles to be pretty basic really and this one layer per command stuff seems like a shortcut that was never fixed. A commit of some sort would make so much more sense. They have recently added the ability to build multiple images where only the final image is used as the output. This allows you to build up a "development" image and build an executable, and just copy that to the final image, leaving behind all the development infrastructure. That can make Dockerfiles a lot cleaner, removing the need for all the && chains.
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: Hi, >> Your patch set does the right thing WRT the debian install/clean, so all >> good, but I thought I'd mention this because it did not come up in any >> discussions here or in the linked docs. > OK, so we *do* have a Docker expert, now! ;-) ;) > Thank you for your explanations, that was very instructive. I gues > that's because aufs is used underneath, or something like that? > I would have expected that, instead of implicit, a layer would have been > explicit, like with a COMMIT keyword or something like that... > Anyway, it all makes sense in hindsight, indeed. Thanks! One more thing to keep in mind, is that Docker expects the commands to be idempotent, so docker will afaik cache each layer and reuse the cache when you rerun the dockerfile later on and not actually execute those commands: https://thenewstack.io/understanding-the-docker-cache-for-faster-builds/
diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile index d45b23dc23..ebb471f7e5 100644 --- a/support/docker/Dockerfile +++ b/support/docker/Dockerfile @@ -28,3 +28,10 @@ RUN apt-get -q -y clean RUN sed -i 's/# \(en_US.UTF-8\)/\1/' /etc/locale.gen RUN /usr/sbin/locale-gen +RUN useradd -ms /bin/bash br-user +RUN chown -R br-user:br-user /home/br-user + +USER br-user +WORKDIR /home/br-user +ENV HOME /home/br-user +ENV LC_ALL en_US.UTF-8
Currently, our jobs on the gitlab-ci infra are running as root, which is problematic for two reasons: - this is not the usual way Buildroot is built; - it may miss issues where running as non-root is problematic. So, complement our Dockerfile with directives to add a new user and run everything as that user, as demonstrated by this build job: https://gitlab.com/ymorin/buildroot-ci/-/jobs/46929562 Additional, enforce an UTF-8 locale while running. Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Cc: Arnout Vandecappelle <arnout@mind.be> Cc: Peter Korsgaard <peter@korsgaard.com> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- support/docker/Dockerfile | 7 +++++++ 1 file changed, 7 insertions(+)