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

Fix buildkitd panic when frontend input is nil. #3659

Merged
merged 1 commit into from
Feb 22, 2023

Conversation

sipsma
Copy link
Collaborator

@sipsma sipsma commented Feb 22, 2023

See integ test case for repro, it causes a server-side panic.

Probably a bunch of possible fixes on different levels but opted for the "lowest-level" one by handling this case in NewDefinitionOp for now; reduces what was a panic to an err return there. Let me know if it's preferable to handle this elsewhere and make NewDefinitionOp return nil, nil in this case or something like that @tonistiigi

Signed-off-by: Erik Sipsma <erik@sipsma.dev>
@tonistiigi
Copy link
Member

Can this appear in some conditions where functions are chained or does user need to explicitly send nil value?

@sipsma
Copy link
Collaborator Author

sipsma commented Feb 22, 2023

Can this appear in some conditions where functions are chained or does user need to explicitly send nil value?

@tonistiigi It seems like for the bridgeClient implementation of Inputs the client has to pretty explicitly just pass nil based on the call paths I looked at.

However I guess looking at the grpcClient implementation of Inputs (not covered by the repro we encountered) maybe there are more complicated situations a similar error could be encountered? But if so, I think the fix to handle this in NewDefinitionOp should handle that better now too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants