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

Pack silently fails to use project.toml if the schema-version is not specified #1922

Closed
edmorley opened this issue Sep 27, 2023 · 3 comments · Fixed by #2042
Closed

Pack silently fails to use project.toml if the schema-version is not specified #1922

edmorley opened this issue Sep 27, 2023 · 3 comments · Fixed by #2042
Labels
status/ready Issue ready to be worked on. type/bug Issue that reports an unexpected behaviour.
Milestone

Comments

@edmorley
Copy link
Contributor

edmorley commented Sep 27, 2023

Summary

If a project.toml file does not contain an appropriately set schema-version, Pack silently ignores values set in the project.toml file, leading to a frustrating to debug experience.


Reproduction

Steps
  1. mkdir testcase && cd $_
  2. echo -e "[io.buildpacks]\nbuilder = \"some-builder\"" > project.toml
  3. pack build testcase
Current behavior

The pack build tries to perform a build using whatever is set as the default builder.

The cause of this is that the builder property is only supported as of project.toml schema version 0.2, but Pack CLI silently defaults to v0.1 if no schema version was specified, and ignores knowns keys rather than failing with a schema validation error.

If I add this, then the expected behaviour occurs:

[_]
schema-version = "0.2"
Expected behavior

For either:

  1. Pack to default The build to be performed using the specified builder (which would fail with an invalid builder error, but this is just a testcase)
  2. Pack to output a warning or error saying "no schema-version specified, defaulting to schema v0.1", along with perhaps a schema v0.1 validation error about the builder property not being recognised.

Environment

pack info
$ pack report
Pack:
  Version:  0.31.0+git-3a994bd.build-5086
  OS/Arch:  darwin/arm64

Default Lifecycle Version:  0.17.1

Supported Platform APIs:  0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 0.10, 0.11, 0.12

Config:
  default-builder-image = "[REDACTED]"
  experimental = true
  
  [[trusted-builders]]
    name = "[REDACTED]"
  
  [[trusted-builders]]
    name = "[REDACTED]"
@edmorley edmorley added status/triage Issue or PR that requires contributor attention. type/bug Issue that reports an unexpected behaviour. labels Sep 27, 2023
@edmorley
Copy link
Contributor Author

edmorley commented Sep 27, 2023

I think that either Pack should either:

  1. Reject project.toml files without an explicit schema-version entirely
  2. Always warn if an explicit schema-version isn't set (and state what it is defaulting to)

If the decision is made to allow implicit schema-version (ie option 2), I also think that the default version should be raised from v0.1 to whatever the latest version is. (This will be a breaking change, but it could be mitigated by adding the warning the release prior etc.)

Lastly, wrt schema validation, IMO Pack should either:

  • Reject unknown fields (within tables specified explicitly in the spec) entirely
  • Warn if an unknown field is found during parsing (and for bonus points, hint that maybe the schema-version needs changing)

@edmorley
Copy link
Contributor Author

In fact, the spec states:

It MUST contain schema-version to denote which schema version the descriptor is using.

(https://github.com/buildpacks/spec/blob/main/extensions/project-descriptor.md#_)

So I guess option (1) above is the right choice here.

The docs here also need fixing, since they say the [_] table is optional, when it's not according to the spec:
https://buildpacks.io/docs/reference/config/project-descriptor/#_-_table-optional_

@jjbustamante jjbustamante added status/ready Issue ready to be worked on. and removed status/triage Issue or PR that requires contributor attention. labels Oct 13, 2023
@jjbustamante jjbustamante modified the milestones: 0.32.0, 0.33.0 Oct 13, 2023
@edmorley
Copy link
Contributor Author

This caused confusion for another user recently in:
https://cloud-native.slack.com/archives/C033DV9EBDF/p1701906234287769

@natalieparellano natalieparellano modified the milestones: 0.33.0, 0.34.0 Dec 13, 2023
colincasey added a commit to colincasey/pack that referenced this issue Jan 30, 2024
- Fails with error if project.toml does not contain an `_.schema-version` key (required as per the spec)
- Warns if there are keys present in project.toml which are not supported by the targeted schema

Fixes buildpacks#1922

Signed-off-by: Colin Casey <casey.colin@gmail.com>
colincasey added a commit to colincasey/pack that referenced this issue Jan 30, 2024
- also notifies when the schema version is missing and defaults to `0.1`

Fixes buildpacks#1922

Signed-off-by: Colin Casey <casey.colin@gmail.com>
Pratham1812 pushed a commit to Pratham1812/pack that referenced this issue Mar 10, 2024
- also notifies when the schema version is missing and defaults to `0.1`

Fixes buildpacks#1922

Signed-off-by: Colin Casey <casey.colin@gmail.com>
Signed-off-by: Pratham Agarwal <agarwalpratham1812@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/ready Issue ready to be worked on. type/bug Issue that reports an unexpected behaviour.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants