search

heroku / libcnb.rs

A framework for writing Cloud Native Buildpacks in Rust
BSD 3-Clause "New" or "Revised" License
34 stars 6 forks source link

Move to buildpack API `0.10` #773

Closed Malax closed 6 months ago

Malax commented 7 months ago

Buildpack API changes

Stack and Mixin Removal (RFC 096, Spec PR)

The stack field is sill in the spec as a deprecated field. I decided to not keep it around for libcnb. libcnb always just supported one specific version and when moving to the new buildpack API, uses should fully migrate to the target concept. This simplifies the implementation greatly (fewer permutations). If a blocker emerges, we can add support for deprecated stacks later down the line.

Open Questions

This seems to be an oversight in the spec. pack does not allow for this (code) and it seems like a to big of a change to silently make. Semantics of this are also unclear. I added code comments to the respective areas to clarify why libcnb.rs differs from the spec. I'm in contact with the maintainers to fix this spec issue upstream.

Unimplemented Buildpack API changes

Runtime Base Image Extensions (RFC 105, Spec PR)

Will be implemented at a later time. The spec is marked as experimental and the feature is basically a new kind of buildpack that shares some of the spec for regular buildpacks, but with slight differences. See the image extension spec for details. Tracking issue for this feature: #776

Build Image Environment Variables (RFC 109, Spec PR)

This does not affect the implementation in libcnb.rs. These new environment variables will always be set (regardless of the clear-env value and cannot be read manually from a directory. This is different from $CNB_PLATFORM_DIR/env/. However, this might affect buildpacks in other ways. For example, If a buildpack choses to set clear-env to true to ensure that no Maven specific environment variables are set that might mess with buildpack execution, build image environment variables can now mess with this assumption.

However, there are many other ways environment variables can end up in the buildpack's executable environment (from outside of $CNB_PLATFORM_DIR/env/). Buildpack authors should not rely on clear-env to avoid clobbering of important environment variables in any case.

Notes for reviewers

Closes #772.

edmorley commented 6 months ago

Build Image Environment Variables (RFC 109, https://github.com/buildpacks/spec/pull/349)

This does not affect the implementation in libcnb.rs. These new environment variables will always be set (regardless of the clear-env value and cannot be read manually from a directory. This is different from $CNB_PLATFORM_DIR/env/.

Is this correct? Iirc there was a bit of uncertainty around this (inconsistency between the rfc, spec and implementation) - though it's been so long I can't quite remember the outcome. See: https://github.com/buildpacks/spec/issues/330#issuecomment-1634336710 https://github.com/buildpacks/spec/pull/367

(Either way, we would want to add support to libcnb in a separate PR, so a tracking issue in the backlog to untangle that uncertainty would be fine)

Malax commented 6 months ago

@edmorley:

Is this correct?

The buildpack API spec 0.10 says:

During the detection and build phases, the lifecycle MUST provide as environment variables any operator-provided files in \<build-config>/env with environment variable names and contents matching the file names and contents. This applies for all values of clear-env or if clear-env is undefined in buildpack.toml.

Which is quite clear IMO. The <build-config> placeholder is only mentioned once (not defined, nor re-used) in the Buildpack API spec and there is no environment variable for it like for the other directories such as CNB_PLATFORM_DIR or CNB_LAYERS_DIR. I interpret this as the strong intent for the "operator defined environment variables" to be magic from the buildpack POV.

The PR that implements this feature into lifecycle does not seem to make any attempt to expose build config environments separately. The current implementation in main isn't different in that regard, platform env and build config env are merged together: https://github.com/buildpacks/lifecycle/blob/751c7a2737be2e6a8f66bfcc2e0cf0058661d6ff/env/env.go#L81-L112

With the Buildpack API spec and lifecycle implementation agreeing, there doesn't seem to be any follow-up required for libcnb.rs here. If anyone has contradicting information I'm happy to stand corrected! :)