Addition of experimental flag full-control-dockerfile

[jarnfast]

What does this change

Addition of experimental flag full-control-dockerfile

When used porter will use the file referenced by dockerfile without any changes (templating, CMD, et al)

Can be used for building truly custom invocation images. E.g.:

• multi-stage builds
• minimizing CVEs
• "flatten" image to improve archive performance

What issue does it fix

Closes #2802

Implemented as an experimental flag after discussion on Slack. The is a less intrusive solution than adding a CLI flag yet giving users the full control of the Dockerfile until a more thorough refactoring of the Dockerfile generation will be implemented.

Notes for the reviewer

Naming stuff is HARD - any suggestion on the experimental flag name will not be taken badly 😆

I opted to duplicate a bit of the "reading the Dockerfile" code in order to avoid injecting the IsFeatureEnabled conditional in multiple methods in the dockerfile-generator.go file. This also makes it easier to remove down the line.

Checklist

• Did you write tests?
• Did you write documentation?

[kichristensen]

Hi, thank you very much for the PR.

A small nit, it might be a good idea when installing a bundle with the experimental feature enabled, to write a warning saying that changes to the dockerfile is not automatically detected. But it is not very important to me, as it is an experimental feature.

@schristoff how do we want to handle the updated documentation? Do we want to wait with merging of the documentation until around the release, or do we accept that the documentation is updated right away?

[jarnfast]

A small nit, it might be a good idea when installing a bundle with the experimental feature enabled, to write a warning saying that changes to the dockerfile is not automatically detected. But it is not very important to me, as it is an experimental feature.

Ah, actually this is the current behavior when using the dockerfile option. Ie. it's not introduced by this experimental feature. I'd gladly propose a separate improvement to make ensureLocalBundleIsUpToDate aware of the dockerfile contents 😄

[kichristensen]

@jarnfast that makes sense, let us keep it out of this PR

[schristoff]

@schristoff how do we want to handle the updated documentation? Do we want to wait with merging of the documentation until around the release, or do we accept that the documentation is updated right away?

Let's just merge it in, we are a month out so I'm not too concerned. In the future maybe we need to start a docs branch though 🤔

[schristoff]

We should maybe document sane things Porter needs to run (like copying the bundle directory, or permissions that we have in the templated Dockerfile)

[jarnfast]

Hm.. maybe? I'm not super convinced - it is a "here be dragons" experimental flag. 🐲
How about linking to https://porter.sh/docs/introduction/concepts-and-components/intro-invocation-images/ which actually has some nice descriptions of what's going on. Would that be enough? 😺

[schristoff]

I think just pointing out you need the following for Porter to be able to run:

# PORTER_INIT

RUN rm -f /etc/apt/apt.conf.d/docker-clean; echo 'Binary::apt::APT::Keep-Downloaded-Packages "true";' > /etc/apt/apt.conf.d/keep-cache
RUN --mount=type=cache,target=/var/cache/apt --mount=type=cache,target=/var/lib/apt \
    apt-get update && apt-get install -y ca-certificates

# PORTER_MIXINS

# Use the BUNDLE_DIR build argument to copy files into the bundle's working directory
COPY --link . ${BUNDLE_DIR}

In your test example you are still copying bundleDir into /cnab/app in the container, and I think it's just a good call out to say "Porter will not run, with this flag, if you don't copy the bundle directory"
We can still link to the intro to invocation images, too. But it makes using the flag a little more "usable" to those who aren't Porter nerds!

[schristoff]

Left a couple of quick questions. Thank you so much, this looks great :)

[schristoff]

I'm going to approve because I'm only picking at documentation

[kichristensen]

I'm going to approve because I'm only picking at documentation

I will second that. It is an advanced flag that most people won't use, unless they really know what they are doing.

[jarnfast]

I'm going to approve because I'm only picking at documentation

I will second that. It is an advanced flag that most people won't use, unless they really know what they are doing.

Cool, thanks, then I won't update the PR any further ;)