-
Notifications
You must be signed in to change notification settings - Fork 588
builder: validate buildkit configuration #2864
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
$ docker buildx create --buildkitd-config cfg.toml
ERROR: failed to parse buildkit config: (2, 1): Can't convert maybe(string) to bool
ac887c6 to
861f42a
Compare
| if err != nil { | ||
| return t, errors.Wrap(err, "failed to parse config") | ||
| return t, errors.Wrap(err, "failed to parse buildkit config") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering; will this be problematic when crating a builder using an older version of buildkit? (i.e. would validation only work correctly if the version matches the one that's vendored?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this will be an issue as we are backward compat. Also we are already using this logic to get the registry config in
buildx/store/storeutil/storeutil.go
Lines 125 to 128 in 567361d
| config, err := buildkitdconfig.Load(bytes.NewReader(dt)) | |
| if err != nil { | |
| return opt, err | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah, I wasn't sure, so thought I'd mention it. I recall that I ran into changes in the garbage-collect configuration that changed, and for which I needed to make changes in moby/moby, because it used those; moby/moby#48634 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a test for this, making sure a new unknown key doesn't cause a validation error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tonistiigi Done
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
861f42a to
07db2be
Compare

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.

fixes #2853
We should validate buildkit configuration before creating the builder.