heroku / cnb-shim

This is a shim to run old buildpacks as Cloud Native Buildpacks
MIT License
18 stars 12 forks source link

Remove implicit requirement on bash (support direct=true) #74

Open edmorley opened 1 year ago

edmorley commented 1 year ago

The direct=false process type mode has been removed as of Buildpack API version 0.9.

This has a few implications:

At first glance, it might seem that cnb-shim would only be affected by this when it eventually upgrades to Buildpack API 0.9 (it's currently using 0.4; xref #69).

However, the problem is that if any other buildpack used alongside a cnb-shimmed buildpack choses to switch to direct=true for the processes they define (which they'll all have to do eventually, given Buildpack API 0.9+ removes support for the older direct=false mode), then the shimmed buildpack will just stop working since its .profile.d/ scripts will no longer be sourced.

This means we need to add support in cnb-shim before any other buildpack can switch to direct=true.

For more info, see: https://github.com/heroku/procfile-cnb/issues/151 https://github.com/heroku/procfile-cnb/pull/150#issuecomment-1531423521

GUS-W-13156998.

schneems commented 1 year ago

This means we need to add support in cnb-shim before any other buildpack can switch to direct=true.

Do you have any sketches of what we might need to do to move forward?

I see this:

Therefore, we still need to solve the issue for shimmed CNBs, which would require updating cnb-shim to make it add an exec.d binary as part of the build, that then at runtime runs the profile.d scripts (to ensure any non-env var side effects happen) and then extracts the env vars and exports them in the key-value pair format expected of the exec.d binaries.

From https://github.com/heroku/procfile-cnb/pull/150. I'm catching up on exec.d from https://github.com/buildpacks/spec/blob/main/buildpack.md#process-5

execute each file in each //exec.d as described in the Platform Interface Specification. execute each file in each //exec.d/ as described in the Platform Interface Specification.

It is very similar to a profile.d script but with a different interface. I'm not entirely following your wording and order. I'm interpreting the last part to actually need to come first:

extracts the env vars and exports them in the key-value pair format expected of the exec.d binaries

I think we need to do this in order to run the exec.d binaries based on this:

The lifecycle MUST set all buildpack-provided launch environment variables as described in the Environment section.

Or are you talking about something different? If I'm interpreting correctly then I think I have the needed information to work on updating the cnb-shim so we can update the procfile buildpack so we can update the ruby buildpack.

edmorley commented 1 year ago

I don't really understand the questions / where the confusion is? I'll try giving an overview and maybe that will help? (That said, part of the task for whomever works on this will be figuring out exactly how to design/implement this, since we don't have all the answers yet.)

Classic buildpacks can choose to use profile.d/ scripts, and these scripts can both set env vars and also have side effects (eg creating other files, starting other background processes etc).

Currently cnb-shim copies any classic profile.d/ scripts as-is, since older CNB API versions support profile.d/ scripts too: https://github.com/heroku/cnb-shim/blob/0b640962365ad8609250cd86d7a39ba9b180fe06/bin/build#L32-L38

With older CNB API versions when using direct=false, at runtime the launcher sources these profile.d/ scripts before running the actual process.

With newer CNB API versions direct=false and its associated automatic profile.d/ script sourcing has been removed.

That means cnb-shim will need to use exec.d/ binaries instead to fulfil the same purpose. The spec for these is here: https://github.com/buildpacks/spec/blob/main/buildpack.md#execd

The profile.d/ scripts being shimmed need to be interpreted at runtime (rather than in advance during the build), so they can't be converted to something exec.d friendly in advance.

As such, it seems like cnb-shim will need to:

At runtime, the exec.d binary will presumably need to:

libcnb.rs already has exec.d support which would simplify parts of this quite a bit, so that's one option.

Or if sticking to Go, then you need to bear in mind that cnb-shim is stuck on a very old CNB helper library - see the libbuildpack references in: https://github.com/heroku/cnb-shim/issues/69

In general, this likely won't be a small amount of work, so buyer beware :-)