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

Add support to allow credential helper to return of JSON payloads larger than 4K #21287

Closed
RiverPup opened this issue Feb 10, 2024 · 1 comment
Labels
help wanted Someone outside the Bazel team could own this P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: bug

Comments

@RiverPup
Copy link

Description of the feature request:

It seems that the current implementation of credential helper only works for the return of JSON payloads less than 4K. JSON payloads larger than 4K will cause the credential helper call to hang and timeout.

Which category does this issue belong to?

CLI, Remote Execution

What underlying problem are you trying to solve with this feature?

Credential helper JSON payloads greater than 4K are timing out.

Which operating system are you running Bazel on?

Windows and Linux

What is the output of bazel info release?

version 6.3.1 to 7.x

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@github-actions github-actions bot added team-CLI Console UI team-Remote-Exec Issues and PRs for the Execution (Remote) team labels Feb 10, 2024
@RiverPup RiverPup changed the title add support to allow credential helper to return of JSON payloads larger than 4K Add support to allow credential helper to return of JSON payloads larger than 4K Feb 10, 2024
@meisterT meisterT removed the team-CLI Console UI label Feb 13, 2024
@zhengwei143 zhengwei143 added the P2 We'll consider working on this in future. (Assignee optional) label Feb 13, 2024
@tjgq tjgq removed their assignment May 15, 2024
@tjgq tjgq added P3 We're not considering working on this, but happy to review a PR. (No assignee) help wanted Someone outside the Bazel team could own this and removed P2 We'll consider working on this in future. (Assignee optional) labels May 15, 2024
@tjgq
Copy link
Contributor

tjgq commented May 15, 2024

The issue here is that Bazel reads the credential helper output from a pipe, and we don't start reading from the pipe until the process terminates. So if the response is too big to fit in the pipe buffer, we deadlock and timeout.

I can think of two possible fixes: increase the size of the pipe buffer or read the response in a separate thread that does its own buffering. Either way, we definitely want to cap the response size at some sane value (say, 64KB).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Someone outside the Bazel team could own this P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: bug
Projects
None yet
Development

No branches or pull requests

7 participants