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

RFC: Support for pack.toml #189

Closed
wants to merge 6 commits into from
Closed
Changes from 5 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
123 changes: 123 additions & 0 deletions text/0000-pack-toml.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
# Meta
[meta]: #meta
- Name: Support for `pack.toml`
- Start Date: 2021-11-16
- Author(s): [jkutner](http://github.com/jkutner)
- Status: Draft
- RFC Pull Request: (leave blank)
- CNB Pull Request: (leave blank)
- CNB Issue: (leave blank)
- Supersedes: N/A

# Summary
[summary]: #summary

This is a proposal for a `pack.toml` file that is a special case of the `project.toml` descriptor file. The `pack.toml` will include configuration options specific to the Pack CLI, which may not be honored by other platforms.

# Definitions
[definitions]: #definitions

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


# Motivation
[motivation]: #motivation

The `project.toml` file has been a useful and essential tool for buildpack users who need to codify certain arguments of their builds. However, there has been much debate around how to handle configuration options that may not be supported by all platforms.

We desire a descriptor file that is:
* specific to pack (i.e. not platform-neutral)
Copy link
Member

Choose a reason for hiding this comment

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

What do we get out of a config file specific to pack? I feel like you might have a use case in mind?

Copy link
Member Author

Choose a reason for hiding this comment

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

Two things:

  • ability to support pack specific configuration w/o putting it in the project.toml spec
  • a file that advertises that a repo can be used with pack (like a Dockerfile does for docker)

* advertises that the repo containing the file can be built with pack
* adheres to the best practices and learnings from [`project.toml`](https://github.com/buildpacks/spec/blob/main/extensions/project-descriptor.md)
jkutner marked this conversation as resolved.
Show resolved Hide resolved

# What it is
[what-it-is]: #what-it-is

The Pack CLI will support a new descriptor file, `pack.toml`, that is a superset of the project descriptor.
* An app MAY have a `pack.toml`
* An app MAY have a `project.toml`
* An app MAY have both a `pack.toml` and a `project.toml`, in which case the `project.toml` will be ignored.
jkutner marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* An app MAY have both a `pack.toml` and a `project.toml`, in which case the `project.toml` will be ignored.
* An app MAY have both a `pack.toml` and a `project.toml` where they will be merged and used.

Copy link
Member Author

Choose a reason for hiding this comment

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

@hone we've gone back and forth on this. Merging could actually be very complicated, and if we support merging we should only do it at the top level shared key (i.e. we're not going to try and merge lists of things for example)

Copy link
Contributor

Choose a reason for hiding this comment

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

@jkutner this is precisely why I have a problem with a platform specific descriptor covering platform agnostic configuration.

I would want to have as much as possible in an agnostic way, at least from the perspective of all platforms other than pack. Why? Because for 99% of the non pack platforms I can imagine, pack will be the platform for local execution. See kpack and our integration into project Piper as two examples and Tekton most likely as a third.

Also, as I pointed out in #189 (comment) the “project.toml will be ignored” statement contradicts other parts of this proposal.


The `pack.toml` will support an additional `[io.buildpacks.pack]` table that is NOT defined in the [project descriptor spec](https://github.com/buildpacks/spec/blob/main/extensions/project-descriptor.md). The `[io.buildpacks.pack]` table will have the following schema:
jkutner marked this conversation as resolved.
Show resolved Hide resolved

```toml
[io.buildpacks.pack]
builder = "<string (optional)>"
Copy link
Member

Choose a reason for hiding this comment

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

io.buildpacks.builder already seems to be supported in project.toml here: https://github.com/buildpacks/spec/blob/main/extensions/project-descriptor.md#iobuildpacksbuilder-optional. Is this different somehow?

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 think that may have been added since I first wrote this. I'll update with a different example that shows something that's specific to pack (like tag or maybe env-file)

```

Where:

* `builder` is a uri to a CNB Builder docker image

## Example

```
[io.buildpacks.build]
exclude = ["/README.md"]

[[io.buildpacks.group]]
id = "example/java"
version = "1.0.0"

[io.buildpacks.pack]
builder = "example/my-builder-image"
```

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

The Pack CLI will honor both the `pack.toml` and `project.toml` file descriptors with the following rules:

* If one or more descriptors are provided on the command line they will all be merged and used.
jkutner marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
* If one or more descriptors are provided on the command line they will all be merged and used.
* If one or more descriptors are provided on the command line, all top-level keys will all be merged and the resulting descriptor will be used.

* If a `pack.toml` file exists in the workspace and no descriptors are provided on the command line, it will be used.
* If a `project.toml` file exists in the workspace and no descriptors are provided on the command line, it will be merged with any `pack.toml` that is present in the workspace and the result will be used.
jkutner marked this conversation as resolved.
Show resolved Hide resolved
* Else no project descriptor will be used

When multiple descriptors are _merged_ the following precedence is given:

* Configuration in a file provided on the command line will take precedence over files provided on the command line after it.
* Configuration in a `pack.toml` will take precedence over configuration in a `project.toml`
Copy link

Choose a reason for hiding this comment

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

I would like some outputs from pack to indicate the config being used (even better if it could trace it back to the origin and say why). That way users can know at a glance if their changes have been picked up or if they're not being picked up, get a hint that maybe another file is taking precedence.

I'm unsure whether that desire can be encoded in the RFC in a sensible way.


# Drawbacks
[drawbacks]: #drawbacks

* Using a `pack.toml` and `project.toml` descriptor together in a `pack` build may be confusing. The rules of merging two files may not be obvious.

# Alternatives
[alternatives]: #alternatives

## Support `pack.toml` as a subset of `project.toml`

* If a path to a `pack.toml` is provided on the command line it will be used.
* Else if a `pack.toml` file exists in the workspace, it will be used.
* Else no project descriptor will be used

At the same time:

* If a path to a `project.toml` is provided on the command line it will be used.
* Else if a `project.toml` file exists in the workspace, it will be used.
* Else no project descriptor will be used

## Support `pack.toml` and `project.toml` as mutually exclusive descriptors

* If a descriptor is provided on the command line it will be used.
* Else If a `pack.toml` file exists in the workspace, it will be used. Only configuration in the `[io.buildpacks]` table and sub-tables will be honored. The root `[_]` will be optional.
* Else if a `project.toml` file exists in the workspace, it will be used. Any configuration in the `[io.buildpacks.pack]` table will be ignored.
* Else no project descriptor will be used

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

## Terraform Manifests

A repository may contain multiple [Terraform](https://www.terraform.io/) manifest, in which case the resources in those files will be merge into a single plan.

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

N/A

# Spec. Changes (OPTIONAL)
[spec-changes]: #spec-changes

N/A