cloudposse / bastion

đź”’Secure Bastion implemented as Docker Container running Alpine Linux with Google Authenticator & DUO MFA support
https://cloudposse.com/accelerate
Apache License 2.0
643 stars 112 forks source link

[dockerfile] remove build deps from final image #31

Closed alebabai closed 5 years ago

alebabai commented 5 years ago

what

remove build dependencies from final image when all make install steps are done

why

30

osterman commented 5 years ago

My concern with this is we're not really optimizing the image size by doing the multi-stage. Can you try just adding make as a dep from a clean alpine base and seeing if you can successfully install?

osterman commented 5 years ago

In an ideal world, we would set the base path in the configure steps to be /dist and do the make install within the builder. Then from the inheritor, we'd COPY --from=builder /dist /

alebabai commented 5 years ago

In an ideal world, we would set the base path in the configure steps to be /dist and do the make install within the builder. Then from the inheritor, we'd COPY --from=builder /dist /

in this case we should know names on all binaries that make produces, setting permissions for them.

osterman commented 5 years ago

in this case we should know names on all binaries that make produces, setting permissions for them.

so, I guess what you call /dist today I was thinking of /usr/src; I mean /dist as files for distribution. Basically, install to a synthetic root folder (e.g. /install).

Then we just COPY --from=builder /install /

alebabai commented 5 years ago

so, I guess what you call /dist today I was thinking of /usr/src; I mean /dist as files for distribution. Basically, install to a synthetic root folder (e.g. /install).

Not certainly in that way. For now /dist folder contains src files and all files produced by make. image

alebabai commented 5 years ago

@osterman Everything worked out , it seems to me. Final image size has been reduced from 335 MB to 41.7 MB. Could you help me with smoke test, how to do it without credentials?

alebabai commented 5 years ago

@osterman it didn't fix issue for me, but may would be useful So we have automake with some standard variables image

So we can remove redundant params for openssh-portable, because prefix is applied to them by default

...
        --prefix=/dist/usr \
        --sysconfdir=/etc/ssh \
        --datadir=/usr/share/openssh \
        --libexecdir=/usr/lib/ssh \
        --mandir=/usr/share/man \
...

final version could look like this

...
--prefix=/dist/usr \
...
alebabai commented 5 years ago

seems to be working now

osterman commented 5 years ago
--prefix=/dist/usr \

What about /etc? ...don't want /usr/etc

alebabai commented 5 years ago

What about /etc?

We don't need prefix here because this property is used during bastion runtime. So we can use it as is --sysconfdir=/etc/ssh

alebabai commented 5 years ago

In current variant i preserved all options, just added prefix where it needed. Seems like working variant.

mhwelsh commented 5 years ago

Pretty sure this Patch breaks duo integration. Current repo version of this does have the duo libs. I built fromscratch and I think it’s because the prefix command doesn’t work for the duo config script. The libs are built but aren’t copied to the fresh image as intended.

osterman commented 5 years ago

@alebabai can you take a look at this on Monday?

osterman commented 5 years ago

@mhwelsh can you open an issue so we can track it? thanks!

alebabai commented 5 years ago

@osterman

@alebabai can you take a look at this on Monday?

Sure.