Split out SolveResponse to avoid key collision with multiple exporters/cache exporters
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 10.3k
- Forks
- 1.5k
- Avg merge
- 1d 23h
- Merged PRs (30d)
- 48
Description
Follow-up to https://github.com/moby/buildkit/pull/4134#discussion_r1289910449.
With #3024 #4134 we subtly introduced an issue when multiple cache exporters and exporters are used, if multiple of the same type are used.
The issue is that we unconditionally just merge all the exporter responses together (even when there are possibly conflicting keys): https://github.com/moby/buildkit/blob/15b7b542ae352a1650716c58f8eeec41f5b38c28/solver/llbsolver/solver.go#L573-L582
My proposed solution is to split out the SolveResponse struct from:
message SolveResponse {
map<string, string> ExporterResponse = 1;
}
into:
message SolveResponse {
map<string, string> ExporterResponseDeprecated = 1;
repeated ExporterResponse ExporterResponses = 2;
repeated ExporterCacheResponse CacheExporterResponses = 2;
FrontendResponse FrontendRessponse = 3;
}
We would keep returning the combined responses in the deprecated field, but would then also return these new fields that have all the data correctly split out.
This then gives clients the option to easily distinguish between which exporters did what and produced which results. One of the open questions is how to address the --metadata-file option:
To really solve this issue, we need to make sure that the solve response stays separated out here - not just falling back to the deprecated field. However: we should avoid breaking existing usage (that relies on the current format) of the --metadata-file.
One possible option is to nest the exporter responses into the file (we should do this transformation client-side):
{
"<deprecated key>": "<deprecated value>",
"exporters": [
{
"<new key>": "<new value>"
}
],
"cache": [
{
"<new key>": "<new value>"
}
],
"frontend": {
"<new key>": "<new value>"
]
}
Essentially we can keep the format as is, but add some new fields that have the split out properties correctly. So new usages can rely on those, and can fallback to the old properties (e.g. the github actions, cc @crazy-max), while users relying on the old usage wouldn't break. This would allow us to deprecate and then remove the old format eventually, though I'm unsure of exact timelines.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with solver/llbsolver/solver.go at the linked response-merging code, then inspect the protobuf SolveResponse definition and cmd/buildctl/build.go around the --metadata-file handling. Resolve how separated exporter, cache-exporter, and frontend responses should be represented while preserving the deprecated combined field and existing metadata-file users. Done means clients can distinguish response sources without breaking the current format.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api, build-system
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100