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

Distribution Spec (v3) - Build Image Artifact format #266

Merged

Conversation

jimil749
Copy link
Contributor

@jimil749 jimil749 commented Nov 1, 2021

This PR consists the artifact format and the delivery mechanism for Buildpacks' Build Image.

Major changes/addition involve:

  • Removing "Stack" related fields from the image manifest and instead using Image Config's os, architecture field.

Signed-off-by: Jimil Desai jimildesai42@gmail.com

@jimil749
Copy link
Contributor Author

jimil749 commented Nov 1, 2021

@jromero, should the current spec involve the Runtime Metadata (https://github.com/buildpacks/rfcs/blob/main/text/0096-remove-stacks-mixins.md#runtime-metadata) information? Because that information should be available at "build" time, hence it should belong with the build image, I guess!

distribution.md Outdated Show resolved Hide resolved
distribution.md Outdated Show resolved Hide resolved
@hone hone marked this pull request as ready for review November 3, 2021 18:19
@hone hone requested a review from a team as a code owner November 3, 2021 18:19
@hone hone changed the base branch from main to distribution/0.3 November 3, 2021 18:19
distribution.md Outdated Show resolved Hide resolved
distribution.md Outdated Show resolved Hide resolved
distribution.md Outdated Show resolved Hide resolved
@jimil749
Copy link
Contributor Author

Hi @sclevine! I have addressed your review, it'd be great if you can take a look.

Signed-off-by: Jimil Desai <jimildesai42@gmail.com>
Signed-off-by: Jimil Desai <jimildesai42@gmail.com>
Signed-off-by: Jimil Desai <jimildesai42@gmail.com>
Signed-off-by: Jimil Desai <jimildesai42@gmail.com>
@jimil749
Copy link
Contributor Author

Resolved the merge conflict. It'd be good to have a review on this.

distribution.md Outdated Show resolved Hide resolved
Signed-off-by: Javier Romero <rjavier@vmware.com>

The Build Image MUST contain the following configurations:

* Image Config's `config.User` field MUST be set to the user [†](README.md#operating-system-conventions)UID/[‡](README.md#operating-system-conventions)SID with a writable home directory.
Copy link
Member

Choose a reason for hiding this comment

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

Should this include a GID to match CNB_GROUP_ID?

Copy link
Member

Choose a reason for hiding this comment

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

FWIW, I believe this was transposed from the platform API which I don't see having that either.

Copy link
Member

Choose a reason for hiding this comment

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

Hm, I guess it can be optional if the primary group is the intended group.

Copy link
Member

Choose a reason for hiding this comment

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

Additional finding: Google and Heroku builders only use user (not even uid).

Google: "User": "cnb"
Heroku: "User": "heroku"


The Build Image MUST contain the following configurations:

* Image Config's `config.User` field MUST be set to the user [†](README.md#operating-system-conventions)UID/[‡](README.md#operating-system-conventions)SID with a writable home directory.
Copy link
Member

Choose a reason for hiding this comment

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

Hm, I guess it can be optional if the primary group is the intended group.

@sclevine sclevine merged commit edaee0c into buildpacks:distribution/0.3 Dec 1, 2021
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.

None yet

6 participants