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

Fixing reproducibility error introduced by 24-CNB vs 24-Heroku #332

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions heroku-24-build/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,7 @@ ENV CNB_GROUP_ID=1000
# Stack IDs are deprecated, but we still set this for backwards compatibility:
# https://github.com/buildpacks/spec/blob/platform/0.13/platform.md#iobuildpacksstack-labels
ENV CNB_STACK_ID="heroku-24"

# Match HOME operation done in [heroku-22](https://github.com/usiegj00/base-images/blob/3bed84a6cf88bc6535b1ef18acbc454f88c854d8/heroku-22-cnb/Dockerfile#L17).
# This is related to [the user HOME issue exposed here](https://github.com/heroku/base-images/pull/332).
ENV HOME=/workspace
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a breaking change for existing Heroku-24 users.

It's also not safe to assume that the workspace dir is /workspace, since the CNB spec says this directory can vary and is user/platform provided. For example if I run pack build --workspace /my-app. See the CNB_APP_DIR references in:
https://github.com/buildpacks/spec/blob/main/platform.md

4 changes: 2 additions & 2 deletions heroku-24/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ test "$(file --brief /etc/ssl/certs/java/cacerts)" = "Java KeyStore"
# that we have to remove before creating our own (`userdel` will remove the group too).
userdel ubuntu --remove

groupadd heroku --gid 1000
useradd heroku --uid 1000 --gid 1000 --shell /bin/bash --create-home
groupadd dyno --gid 1000
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our aim with CNBs is to make the run/build/builder images be as platform independent as possible, and to try and avoid Heroku-specific terminology where possible (eg preferring "base images" instead of "stack-images" etc) - and "dyno" is a Heroku-specific product name. Plus people may use the images in non-Heroku environments, in which case they won't be running on a dyno.

As such this username change was intentional.

useradd dyno --uid 1000 --gid 1000 --shell /bin/bash --home /app --create-home
Copy link
Member

@edmorley edmorley Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're intentionally moving back to having a standard home directory location for the default user to (a) align with standards, (b) match the CNB spec and upstream CNB expectations.

In an ideal world we could make this change for Heroku-22 and older, though it would be a breaking change for those users, so we'll have to let those stacks age out.


rm -rf /root/*
rm -rf /tmp/*
Expand Down
Loading