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

make ready "Prepare Phase RFC" #1

Closed
wants to merge 5 commits into from
Closed
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
100 changes: 100 additions & 0 deletions text/0000-prepare-phase.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
# Meta
[meta]: #meta
- Name: Prepare Phase
- Start Date: 2021-06-08
- Author(s): [@jkutner](github.com/jkutner/) & [@natalieparellano](github.com/natalieparellano/) & [@haliliceylan](github.com/haliliceylan/)
- RFC Pull Request: (leave blank)
- CNB Pull Request: [buildpacks/spec#176](https://github.com/buildpacks/spec/pull/176)
- CNB Issue: (leave blank)
- Supersedes: N/A

# Summary
[summary]: #summary

This is a proposal for a new Lifecycle phase, called "prepare", that would run before all other phases.

# Definitions
[definitions]: #definitions

* __stack descriptor__ - an enhanced version of the [`stack.toml`](https://github.com/buildpacks/spec/blob/main/platform.md#stacktoml-toml)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should describe how it's enhanced. (I'm not sure myself...)

* __project descriptor__ - the [`project.toml`](https://github.com/buildpacks/spec/blob/main/extensions/project-descriptor.md) extension specification

# Motivation
[motivation]: #motivation

Parity with Pack: A Lifecycle Prepare phase should make it easier for Platform Implementers to achieve parity with features of Pack. Today, features like project.toml are only supported by Pack, and a new platform would need to write its own parser.

The prepare phase would support the following features and capabilities:
* [Inline buildpacks](https://github.com/buildpacks/rfcs/blob/main/text/0048-inline-buildpack.md), which require parsing of the `project.toml` in the lifecycle
haliliceylan marked this conversation as resolved.
Show resolved Hide resolved
* [Lifecycle configuration](https://github.com/buildpacks/rfcs/pull/128)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be good to list exactly what would be configured. Though perhaps it's worth noting, the linked RFC was closed (I forget exactly why). Maybe it isn't a concern anymore.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Actually i am little bit confusing about what would be configured. This is Draft RFC is little bit old and a lot of links are already merged or closed. The issue is not providing any technical information (except your comment). What should i do to find correct list for here ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's take it out for now, if there's a need surely others will suggest it :)


# What it is
[what-it-is]: #what-it-is
A new Lifecycle phase and associated binary should be available to Platform Implementers, and should be executed by Pack. The prepare phase will run before all other phases, and prepare the execution environment for a buildpack build. This phase will have access to secrets and credentials used to access registries and other services.

## Responsibilities

* Parsing the project descriptor and performance various operations based on its contents, include:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be possible to list all of the features? It might be good to break them out by:

  • whether they are currently implemented in pack (yes/no)
  • whether they are "platform" things vs. "buildpack" things (things a buildpack could do - e.g., modifying environment variables)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here is the project descriptor spec: https://github.com/buildpacks/spec/blob/main/extensions/project-descriptor.md
And here is where pack parses the project descriptor: https://github.com/buildpacks/pack/blob/f6b78274f8f6aa5c659db509a14230f1d1610abc/internal/commands/build.go#L66
If you're having any questions about the part, the folks in #pack-cli will be able to help :)

- downloading buildpacks
- creating ephemeral buildpacks
- applying include and exclude rules
- modifying environment variables

## Inputs

* Log level
* Run-image
* Stack ID
Comment on lines +46 to +47
Copy link
Collaborator

Choose a reason for hiding this comment

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

These might not make sense if we are removing some responsibilities from the prepare phase (see comments re: analyze reading run-image mixins, etc.)

* Project descriptor (optional)
* App source code

## Output

* Exit status
* Info-level logs to `stdout`
* Error-level logs to `stderr`
* Stack descriptor
* Analysis metadata
Comment on lines +56 to +57
Copy link
Collaborator

Choose a reason for hiding this comment

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

* Buildpacks (derived from inline buildpacks in project descriptor, or buildpacks in project descriptor that are not present in the builder)
* Order definition (derived from buildpacks configuration in project descriptor)
* Lifecycle configuration (derived from configuration in project descriptor)
* Mutated app source code (applying include and exclude rules in project descriptor)

# How it Works
[how-it-works]: #how-it-works

The prepare phase would be implemented as the `/cnb/lifecycle/preparer` binary. A platform MAY execute this phases either by invoking the `/cnb/lifecycle/preparer` binary or by executing `/cnb/lifecycle/creator`.

The `preparer` binary will have access to the [`Keychain`](https://github.com/buildpacks/lifecycle/blob/main/auth/env_keychain.go), and MUST NOT execute arbitrary code provided by either the buildpack user or buildpack author.
Copy link
Collaborator

@natalieparellano natalieparellano Jun 7, 2021

Choose a reason for hiding this comment

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

(deleted)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it's also worth noting if the preparer will run as privileged or not (I don't think it needs to).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Removed my suggestion, I forgot that we need registry auth to download buildpacks.


The [logic in `pack` that parses a `project.toml`](https://github.com/buildpacks/pack/blob/main/project/project.go) would be copied or moved into the `preparer`.

The app source code (which may be provided to the prepare either as a directory, volume, or tarball) would be mutated (either by copying it to a new location, or making changes directory against it). The `preparer` may delete files to apply the include and exclude rules from `project.toml`.

# Drawbacks
[drawbacks]: #drawbacks

* Yet another lifecycle phase

# Alternatives
[alternatives]: #alternatives
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think both of the listed alternatives could be removed, because they are dealing with the problem of "who is reading the run-image mixins".


- [Reverse the order of analyze and detect phases](https://github.com/buildpacks/spec/pull/172)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the one we went with to accomplish a subset of what's listed :)

- Split analyze into two parts (one to write `analyzed.toml` and one to analyze layers). The first (`analyzed.toml`) would be moved before the detect phase, and the second part (analyzing layers) would be merged into the restore phase.

# Prior Art
[prior-art]: #prior-art

- [Tekton prepare step](https://github.com/tektoncd/catalog/blob/11a17cfe87779099b0b61be3f1e496dfa79646b3/task/buildpacks-phases/0.1/buildpacks-phases.yaml#L61-L78)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be worth noting that this "prepare" step doesn't really do any of the things we want the preparer to do. I think it's just included here because it's called prepare :)


# Unresolved Questions
[unresolved-questions]: #unresolved-questions

- Do we still need `analyzer`, or can the remaining parts of analyze phase be rolled into restore phase?
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one could be removed I think.

- Does `pack` still need to parse `project.toml`, or is there value in reading it early on (before lifecycle runs)?
- Should we create a shared library for `project.toml` parsing?

# Spec. Changes
[spec-changes]: #spec-changes

See [buildpacks/spec PR #176](https://github.com/buildpacks/spec/pull/176)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Outdated now, we could just remove it. But maybe it's worth noting that this will require a platform API bump, with no impact to the buildpack API.