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

Add RFC to define mixins for the Bionic stack #40

Merged
merged 10 commits into from Mar 29, 2020
Merged

Conversation

sclevine
Copy link
Member

Signed-off-by: Stephen Levine <stephen.levine@gmail.com>
@sclevine
Copy link
Member Author

@jonjonsonjr as discussed

text/0000-bionic-mixins.md Outdated Show resolved Hide resolved
Signed-off-by: Stephen Levine <stephen.levine@gmail.com>
text/0000-bionic-mixins.md Outdated Show resolved Hide resolved
text/0000-bionic-mixins.md Outdated Show resolved Hide resolved
@zmackie
Copy link
Contributor

zmackie commented Jan 3, 2020

Think @sclevine you meant @jonjohnsonjr

Copy link
Contributor

@nebhale nebhale left a comment

Choose a reason for hiding this comment

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

Cannot wait to see this!


- Neither the spec nor this RFC suggest whether or not stack authors or app developers are allowed to make additional changes to stack images without using mixin metadata.
- This RFC doesn't propose a way for the project to describe changes to `ubuntu:bionic` that are not official LTS Ubuntu 18.04 packages.
- When package groups (like `build-essential`) or dependent packages are installed, must stack images always specify all the packages added? Or is it only necessary when stack image authors want buildpacks to be able to depend on the dependent packages?
Copy link
Contributor

Choose a reason for hiding this comment

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

The latter. In order to make a mixin as broadly compatible as possible you'll probably want to list both the package group and all the packages that it includes.

Copy link
Member Author

Choose a reason for hiding this comment

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

So your suggestion is that stack authors should add all packages to the mixin list, but are not required to? Would this imply that a given stack run/build image can have arbitrary contents that aren't specified by the ID or mixins, as long as those contents don't disappear when newer versions are published?

@nebhale nebhale added the Project label Jan 9, 2020
jabrown85 added a commit to heroku/nodejs-engine-buildpack that referenced this pull request Jan 14, 2020
[This RFC](buildpacks/rfcs#40) is working it's way through and it seems useful to be able to use Heroku buildpacks on `cloudfoundry/cnb:bionic`. This also aligns with how riff is compatible with [multiple stacks](https://github.com/projectriff/node-function-buildpack/blob/master/buildpack.toml#L22).
jabrown85 added a commit to heroku/nodejs-npm-buildpack that referenced this pull request Jan 14, 2020
[This RFC](buildpacks/rfcs#40) is working it's way through and it seems useful to be able to use Heroku buildpacks on `cloudfoundry/cnb:bionic`. This also aligns with how riff is compatible with [multiple stacks](https://github.com/projectriff/node-function-buildpack/blob/master/buildpack.toml#L22).
jabrown85 added a commit to heroku/cnb-builder-images that referenced this pull request Jan 14, 2020
[This RFC](buildpacks/rfcs#40) is working it's way through and it seems useful to be able to use Heroku buildpacks on `cloudfoundry/cnb:bionic`. This also aligns with how riff is compatible with [multiple stacks](https://github.com/projectriff/node-function-buildpack/blob/master/buildpack.toml#L22).
jabrown85 added a commit to jabrown85/nodejs-sf-fx-buildpack that referenced this pull request Jan 14, 2020
[This RFC](buildpacks/rfcs#40) is working its way through and it seems useful to be able to use Heroku buildpacks on `cloudfoundry/cnb:bionic`. This also aligns with how riff is compatible with [multiple stacks](https://github.com/projectriff/node-function-buildpack/blob/master/buildpack.toml#L22).
jabrown85 added a commit to heroku/node-function-buildpack that referenced this pull request Jan 14, 2020
[This RFC](buildpacks/rfcs#40) is working its way through and it seems useful to be able to use Heroku buildpacks on `cloudfoundry/cnb:bionic`. This also aligns with how riff is compatible with [multiple stacks](https://github.com/projectriff/node-function-buildpack/blob/master/buildpack.toml#L22).
jabrown85 added a commit to heroku/java-function-buildpack that referenced this pull request Jan 14, 2020
[This RFC](buildpacks/rfcs#40) is working its way through and it seems useful to be able to use Heroku buildpacks on `cloudfoundry/cnb:bionic`. This also aligns with how riff is compatible with [multiple stacks](https://github.com/projectriff/node-function-buildpack/blob/master/buildpack.toml#L22).
jabrown85 added a commit to heroku/heroku-buildpack-jvm-common that referenced this pull request Jan 14, 2020
[This RFC](buildpacks/rfcs#40) is working its way through and it seems useful to be able to use Heroku buildpacks on `cloudfoundry/cnb:bionic`. This also aligns with how riff is compatible with [multiple stacks](https://github.com/projectriff/node-function-buildpack/blob/master/buildpack.toml#L22).
jabrown85 added a commit to heroku/heroku-buildpack-ruby that referenced this pull request Jan 14, 2020
[This RFC](buildpacks/rfcs#40) is working its way through and it seems useful to be able to use Heroku buildpacks on `cloudfoundry/cnb:bionic`. This also aligns with how riff is compatible with [multiple stacks](https://github.com/projectriff/node-function-buildpack/blob/master/buildpack.toml#L22).
jabrown85 added a commit to heroku/buildpacks-procfile that referenced this pull request Jan 14, 2020
[This RFC](buildpacks/rfcs#40) is working its way through and it seems useful to be able to use Heroku buildpacks on `cloudfoundry/cnb:bionic`. This also aligns with how riff is compatible with [multiple stacks](https://github.com/projectriff/node-function-buildpack/blob/master/buildpack.toml#L22).
scothis pushed a commit to projectriff/node-function-buildpack that referenced this pull request Jan 31, 2020
It would be nice to support another `bionic` based stack to prevent Heroku from forking all the language buildpacks.

Note: While [this RFC](buildpacks/rfcs#40) is being discussed, it hasn't landed yet.
@jabrown85
Copy link
Contributor

Here are some more thoughts after our chat @sclevine @hone @jkutner

I think we all agreed that the io.buildpacks.stacks.bionic stack really needs to be bionic + a few essentials (e.g. ca-certificates, libssl).

I think we could enhance stack.id to give buildpack developers a better experience without them needing to understand the complexity.

An example might be a buildpack author for company/ruby would be encouraged to target io.buildpacks.stacks.bionic#language-essentials. language-essentials in this case could be libxml, libreadline, and libuv. Resolution of the stack still resolves to io.buildpacks.stacks.bionic but the definition of those buildpack mixin packs? would be part of the platform to ensure the best compatibility.

A #bash-essentials pack of mixins maybe includes jq and yj. This could be the recommended stack for buildpack developers who aren't compiling code as a part of their buildpack.

I would limit buildpack authors to a single pack identifier when defining their stack id. I wouldn't want io.buildpacks.stacks.bionic#language-essentials#bash-essentials to be accepted. The limitation of a single pack of mixins could be combined with additional mixins. So you could require imagemagick in addition to the ones provided by the mixin pack. I also would expect a stack of io.buildpacks.stacks.bionic and mixins of io.buildpacks.mixinpacks.bionic.language-essentials,io.buildpacks.mixinpacks.bionic.bash-essentials to work just fine.

Later, if a buildpack is failing to build on a new stack that doesn't work because the buildpack author is targeting a pack that includes something they are not actually required to use, the author could then go learn what exactly is in that pack and start specifying the mixins in a more selective way. They could stop requiring yj but start detecting when it's available during execution to improve compatibility across stacks. This is the sort of work I would expect buildpack authors at Heroku/Pivotal would do in order to target tiny stacks but only because we are paid to work on them :)

Anyway, that is what I was thinking while we were chatting earlier. I think having a few mixin packs targeted at the buildpack author's purpose could lower the barrier. Once they hit a limitation, the existing granularity of the mixinx would allow the buildpack author to take that dive into understanding stacks and mixins.

Signed-off-by: Stephen Levine <stephen.levine@gmail.com>
@sclevine sclevine requested a review from a team as a code owner February 20, 2020 02:13
@sclevine
Copy link
Member Author

@hone @jkutner @jabrown85 updated based on the feedback above.

I recorded the io.buildpacks.stacks.bionic#language-essentials suggestion as an unresolved question for a future RFC because it would require significant changes to the spec, lifecycle, pack, etc. Let me know if my understanding of that suggestion is inaccurate.

1. The base set of packages is `ubuntu:bionic` with `ca-certificates`, `libssl1.1`, and `openssl` added.
2. Mixins without an `=` are Ubuntu 18.04 LTS package names without version or architecture info from official package mirrors.
3. Mixins that begin with `set=` are defined by the project as such:
a. `build:set=shell-utils` represents the change set: `build:curl`, `build:jq`, and yj CLI at build-time.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
a. `build:set=shell-utils` represents the change set: `build:curl`, `build:jq`, and yj CLI at build-time.
a. `build:set=shell-utils` represents the change set: `build:curl`, `build:jq`, and `build:yj` at build-time.

Copy link
Member Author

Choose a reason for hiding this comment

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

yj is not available as an Ubuntu package, so I went with "yj CLI at build-time" instead of "build:yj."

Maybe it's worth defining build:set=yj as well?

@sclevine sclevine requested review from jkutner and hone March 1, 2020 01:50
# Proposal
[proposal]: #proposal

This RFC proposes that we define `io.buildpacks.stacks.bionic` such that:
Copy link
Member

Choose a reason for hiding this comment

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

Does it expand the scope too much to set a framework for adding new stacks under the io.buildpacks.stacks namespace or is that just a separate proposal. For instance, I think setting the set= pattern is probably more valuable then just defining what all the sets are in this proposal. @sclevine

Copy link
Member Author

Choose a reason for hiding this comment

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

We could define set= for io.buildpacks.stacks.*, but it might be safer to hold off until we have additional stacks to define it for. The set= convention may be applicable to most base images that use OS packages today, but container base images could change in unexpected ways in the future.

Copy link
Member

@hone hone left a comment

Choose a reason for hiding this comment

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

Since the location of where this stuff needs to live or how changes are made is being considered out of scope for this RFC, we should add this to the list of unresolved questions.

text/0000-bionic-mixins.md Outdated Show resolved Hide resolved
The Cloud Native Buildpacks project informally defines stack build and run images with ID `io.buildpacks.stacks.bionic` as containing exactly the set of Ubuntu packages in the `ubuntu:bionic` image on Docker Hub.
This RFC proposes that we formalize the existing definition of `io.buildpacks.stacks.bionic` and define mixins on that stack to be Ubuntu 18.04 LTS packages.

In general, a stack is a contract that should be more like "Ubuntu Bionic" and less like "Platform XYZ's Re-distribution of Ubuntu Binoic."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
In general, a stack is a contract that should be more like "Ubuntu Bionic" and less like "Platform XYZ's Re-distribution of Ubuntu Binoic."
In general, a stack is a contract that should be more like "Ubuntu Bionic" and less like "Platform XYZ's Re-distribution of Ubuntu Bionic."

Comment on lines 16 to 17
In general, a stack is a contract that should be more like "Ubuntu Bionic" and less like "Platform XYZ's Re-distribution of Ubuntu Binoic."
Stack images can still be provided by platforms/vendors, but they should use the standard CNB stack contracts where applicable.
Copy link
Contributor

Choose a reason for hiding this comment

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

I find this language a bit confusing. On first read, I thought it meant that the images must be derived from ubuntu:bionic, but I think it really means provides equivalent functionality to ubuntu:bionic. Otherwise images like gcr.io/gcp-runtimes/ubuntu_18_0_4 won't qualify. And it may be worth expanding functionality: does it mean that /var/lib/dpkg should be retained and that new packages can be installed with apt-get?

Comment on lines +23 to +24
1. The base set of packages is the set of packages distributed in `ubuntu:bionic` with `ca-certificates`, `libssl1.1`, and `openssl` added. The base image does not have to be derived from the `ubuntu:bionic` image, but must contain packages that provide exactly the same functionality to processes running as the CNB-specified, non-root user.
2. When invoked as root, `apt-get` must be configured to use at minimum the set of package repositories that `ubuntu:bionic` is configured to use. `apt-get update` may be needed to download the package repository indices before `apt-get install` is functional.
Copy link
Member Author

Choose a reason for hiding this comment

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

@briandealwis do these clarifications address your concerns?

Copy link
Contributor

Choose a reason for hiding this comment

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

lgtm, thanks

@sclevine
Copy link
Member Author

@hone added the final location of the definition of io.buildpacks.stacks.bionic as an unresolved question, as requested.

Copy link
Member

@hone hone left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for updating. Let's move to FCP and make sure @briandealwis has a chance to chime back in based on your changes.

@sclevine
Copy link
Member Author

Given Brian's response, FCP will end and the RFC will merge on 2020-03-26 unless there is additional feedback.

Signed-off-by: Joe Kutner <jpkutner@gmail.com>
@jkutner jkutner merged commit c0182aa into master Mar 29, 2020
@jkutner jkutner deleted the bionic-mixins branch March 29, 2020 15:26
nebhale added a commit that referenced this pull request May 27, 2020
[#70]

Signed-off-by: Ben Hale <bhale@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants