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

Always set CNB_TARGET_* variables during detect, build, and generate #1309

Merged
merged 3 commits into from
Mar 4, 2024

Conversation

natalieparellano
Copy link
Member

when the Buildpack API version is at least 0.10

Summary

Previously, we only set these variables when the Platform API version was at least 0.12. But, newer Buildpack APIs expect these variables regardless of the Platform API version. If we are on an older platform, derive the target variables from the base image OS.

Release notes

The lifecycle always set CNB_TARGET_* variables during detect, build, and generate when the Buildpack API version is at least 0.10


Related

Resolves #1308 and #1186


Context

when the Buildpack API version is at least 0.10.

Previously, we only set these variables when the Platform API version was at least 0.12.
But, newer Buildpack APIs expect these variables regardless of the Platform API version.
If we are on an older platform, derive the target variables from the base image OS.

Fixes #1308

Signed-off-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Natalie Arellano <narellano@vmware.com>
Copy link

codecov bot commented Mar 4, 2024

Codecov Report

Attention: Patch coverage is 88.00000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 64.60%. Comparing base (f3f292c) to head (61fbfce).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1309      +/-   ##
==========================================
+ Coverage   64.59%   64.60%   +0.01%     
==========================================
  Files         101      101              
  Lines        6997     7007      +10     
==========================================
+ Hits         4519     4526       +7     
- Misses       2067     2069       +2     
- Partials      411      412       +1     
Flag Coverage Δ
os_linux 64.08% <88.00%> (+0.01%) ⬆️
os_windows 56.67% <72.00%> (+0.02%) ⬆️
unit 64.08% <88.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@natalieparellano natalieparellano merged commit 1148b71 into main Mar 4, 2024
10 checks passed
@natalieparellano natalieparellano deleted the fix/target-env branch March 4, 2024 17:33
@edmorley
Copy link
Contributor

@natalieparellano Thank you for fixing this! We're now using lifecycle 0.19.0 in our newer builders and it's resolved the issue.

However, I've just realised we have an older builder that's stuck on lifecycle 0.17.x (since it has to support buildpacks using Buildpack API 0.6) that's also affected. I see RFC-120 only covers CVE backporting to older lifecycle releases - do you have any thoughts as to whether this bug fix might be something that could/should be backported?

(I think in general backporting should be for exceptional circumstances only, however, I wonder if for this specific case it might be worth the effort, just so buildpack authors don't hold back from upgrading to newer Buildpack API versions and targets instead of stacks)

edmorley added a commit to heroku/cnb-builder-images that referenced this pull request Mar 11, 2024
The latest version of Pack CLI (v0.33.2) supports Platform API <= 0.12:
https://github.com/buildpacks/pack/blob/v0.33.2/internal/build/lifecycle_executor.go#L34

This means when it's used with any lifecycle version newer than 0.17.x,
a `pack build` will end up using Platform API 0.12.

Therefore all of our tests in our CNB repos, and the smoke tests in this
repo are all testing against Platform API 0.12.

However, Kodon is currently using Platform API 0.9, and whilst this is
in the process of being upgraded (GUS-W-15122354), it's unlikely that
the version will be upgraded for Kodon's `functions_eol` branch:
https://github.com/heroku/kodon/blob/functions_eol/internal/constants/constants.go#L75

As such, I've updated the functions tests here to use Pack CLI 0.27.0,
which similarly only supports Platform API <= 0.9, and therefore gives
us greater testing parity between CI and Kodon.

This test coverage will help catch things like:
buildpacks/lifecycle#1309 (comment)

Downgrading the Pack CLI version meant needing to remove the
`--force-color` arg, since it's only supported as of v0.33.0.

I've not changed the Pack CLI version used elsewhere, since it's helpful
for us to also have coverage of the actual Pack CLI and Platform API
versions everyone else will be using locally. (Plus hopefully Kodon's
`main` branch will be upgraded to newer Platform API soon anyway.)
edmorley added a commit to heroku/cnb-builder-images that referenced this pull request Mar 12, 2024
The latest version of Pack CLI (v0.33.2) supports Platform API <= 0.12:
https://github.com/buildpacks/pack/blob/v0.33.2/internal/build/lifecycle_executor.go#L34

This means when it's used with any lifecycle version newer than 0.17.x,
a `pack build` will end up using Platform API 0.12:
https://github.com/buildpacks/lifecycle#supported-apis

Therefore all of our tests in our CNB repos, and the smoke tests in this
repo are all testing against Platform API 0.12.

However, Kodon is currently using Platform API 0.9, and whilst this is
in the process of being upgraded (GUS-W-15122354), it's unlikely that
the version will be upgraded for Kodon's `functions_eol` branch:
https://github.com/heroku/kodon/blob/functions_eol/internal/constants/constants.go#L75

As such, I've updated the functions tests here to use Pack CLI 0.27.0,
which similarly only supports Platform API <= 0.9, and therefore gives
us greater testing parity between CI and Kodon.

This test coverage will help catch things like:
buildpacks/lifecycle#1309 (comment)

Downgrading the Pack CLI version meant needing to remove the
`--force-color` arg, since it's only supported as of v0.33.0.

I've not changed the Pack CLI version used elsewhere, since it's helpful
for us to also have coverage of the actual Pack CLI and Platform API
versions everyone else will be using locally. (Plus hopefully Kodon's
`main` branch will be upgraded to newer Platform API soon anyway.)

GUS-W-15226981.
@natalieparellano
Copy link
Member Author

@edmorley that sounds reasonable to me - I created #1317

@edmorley
Copy link
Contributor

Thank you - much appreciated! 😄

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

Successfully merging this pull request may close these issues.

CNB_TARGET_ environment variables are not set when expected
3 participants