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

Adds stack descriptor RFC #169

Merged
merged 1 commit into from
Apr 29, 2022
Merged

Adds stack descriptor RFC #169

merged 1 commit into from
Apr 29, 2022

Conversation

ryanmoran
Copy link
Member

Summary

Readable

Checklist

  • I have viewed, signed, and submitted the Contributor License Agreement.
  • I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
  • I have added an integration test, if necessary.
  • I have reviewed the styleguide for guidance on my code quality.
  • I'm happy with the commit history on this PR (I have rebased/squashed as needed).

@ryanmoran ryanmoran requested a review from a team as a code owner April 4, 2022 16:02
@sophiewigmore
Copy link
Member

sophiewigmore commented Apr 4, 2022

I can't see a reason not to do this. If anyone has one, I'd love to hear it. Any way we can make the stacks code more configurable and easier to use is a huge win in my eyes. I also feel like a well-defined configuration file demystifies some of the confusion around our stack creation.

@ryanmoran ryanmoran requested review from a team, sophiewigmore and fg-j and removed request for a team April 4, 2022 16:36
@ryanmoran ryanmoran self-assigned this Apr 4, 2022

## Rationale and Alternatives

We could abandon the `create-stack` codebase entirely and expect that anyone
Copy link
Member

Choose a reason for hiding this comment

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

As someone with essentially no exposure to the stack creation, can you briefly describe the downside to the Dockerfile approach? I can infer/assume but I'd like to hear your thoughts.

Copy link
Contributor

Choose a reason for hiding this comment

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

The process documented on the CNB site doesn't seem too onerous. I'm curious what's missing from that process or what advantage there is to using this tool?

Conversely, I think it would be nice if our process aligned with the documented process from the CNB. I can see an advantage there.

Copy link
Member Author

Choose a reason for hiding this comment

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

The biggest concern I have between the Dockerfile-only interface and what is proposed here is that we're co-mingling the parts that make up a "base image" (os-specific modifications to an image to make it a reasonable base to build on top of) and a "stack" (the parts of the image that are codified in the Buildpacks Platform Spec or in Paketo RFCs). The effort here is to try to separate those things so its a bit clearer where the line is.

Additionally, we have some features in the existing stacks that aren't part of the Buildpacks Platform Spec that are non-trivial to implement inline in a Dockerfile. Specifically, I am thinking of the code that generates our existing image label-based SBOM for run images. Ideally, I'd like to be able to move the existing stacks onto this new configuration format and tooling since we'll be maintaining them for the next year before they go out of support.

Comment on lines 30 to 34
name = "<os distro name>" # optional
version = "os distro version>" # optional
Copy link
Member

Choose a reason for hiding this comment

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

Should we be using values (ID and VERSION_ID) in /etc/os-release instead of user-provided values?

Or we could make these optional and fail if they do not match /etc/os-release.

Copy link
Member Author

Choose a reason for hiding this comment

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

It makes sense to just use what exists in /etc/os-release. We can probably omit these labels otherwise since the labels themselves are not required by the spec. I added some details about this in the Stack Spec Requirements section.

homepage = "<homepage uri>" # optional
maintainer = "<maintainer name>" # optional

platforms = ["linux/amd64", "linux/arm64", "<other-platform>"] # optional; default = ["linux/amd64"]; choices defined in https://docs.docker.com/desktop/multi-arch/
Copy link
Member

Choose a reason for hiding this comment

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

Aren't the platform(s) are dictated by the platform(s) base image(s) from which we construct the stack?

Maybe we should:

  1. require that the dockerfile accept the base image as a build arg
  2. require that provide a reference to the base image in this file
  3. If the reference is a single image, inherit the platform
  4. If the reference is a multi-arch image, build multiple times times and create a multi-arch image from the result

Copy link
Member Author

Choose a reason for hiding this comment

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

I outlined a bit how I expect the toolchain to handle multi-arch images. The outputs will basically always be formatted as an image index, but may only contain images for a single platform.

@ryanmoran ryanmoran merged commit b2da992 into main Apr 29, 2022
@ryanmoran ryanmoran deleted the stack-descriptor branch April 29, 2022 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants