Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
edmorley opened this issue May 5, 2023 · 2 comments
Open

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

edmorley opened this issue May 5, 2023 · 2 comments

Comments

@edmorley
Copy link
Member

edmorley commented May 5, 2023

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

This has a few implications:

  • process commands now need to be wrapped with bash (eg bash -c <original command)
  • the profile.d/ scripts of other buildpacks are no longer run before the command
  • any .profile script in the app source is no longer run before the command (though there is a compatibility buildpack upstream for this: https://github.com/buildpacks/profile)

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:
heroku/buildpacks-procfile#151
heroku/buildpacks-procfile#150 (comment)

GUS-W-13156998.

@schneems
Copy link
Contributor

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 heroku/buildpacks-procfile#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
Copy link
Member Author

edmorley commented Jun 30, 2023

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:

cnb-shim/bin/build

Lines 32 to 38 in 0b64096

# copy profile.d scripts into a layer so they will be sourced
if [[ -d .profile.d ]]; then
profile_dir="${layers_dir}/profile"
mkdir -p "${profile_dir}/profile.d"
cp .profile.d/* "${profile_dir}/profile.d/"
echo "launch = true" >"${profile_dir}.toml"
fi

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:

  • copy the classic buildpack profile.d/ scripts into a layer
  • also write an exec.d/ binary into that layer, which is then the thing that runs at runtime and performs the shimming of the profile.d scripts that have been bundled into the layer

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

  • launch the profile.d scripts using bash in a subprocess
  • somehow diff the env vars in that bash subprocess before/after the profile.d scripts ran
  • convert that diff of env vars into the key-value env var pairs described in the CNB exec.d spec
  • output those key-value pairs into FD3 per the spec

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:
#69

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants