Skip to content

execserver: add protocol and handler for running remote commands - #2551

Draft
burgerdev wants to merge 1 commit into
mainfrom
burgerdev/exec-downstream
Draft

execserver: add protocol and handler for running remote commands#2551
burgerdev wants to merge 1 commit into
mainfrom
burgerdev/exec-downstream

Conversation

@burgerdev

Copy link
Copy Markdown
Member

Draft: just the protocol for now, before I wire nix and image. Recommended reading order:

  • protocol.go
  • codec.go
  • server.go / handler.go
  • runner.go
  • tests

@burgerdev
burgerdev requested a review from charludo July 28, 2026 18:04
@burgerdev burgerdev added the no changelog PRs not listed in the release notes label Jul 28, 2026
//
// It forwards stdin to the remote command, and pipes the remote output to the local writers.
// After the command completes, Run returns the exit code.
func Run(_ context.Context, log *slog.Logger, conn net.Conn, cmd []string, stdin io.Reader, stdout, stderr io.WriteCloser) (int, error) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why ignore the Context here? Shouldn't cancellation also abort this function?

// According to io.Reader, we might encounter n>0 and EOF in the same read.
if n > 0 {
if sendErr := WriteMessage(conn, &Request{Stdin: &StreamEvent{Data: buf[:n]}}); sendErr != nil {
log.Error("Encountered error while forwarding stdin", "error", err)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong error logged, same for the other two logs

return
}
if len(req.Cmd) == 0 {
slog.Warn("First request did not contain a command")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use slog here?

}

// ErrMessageTooLarge is returned if the serialized message exceeds the maximum size.
var ErrMessageTooLarge = errors.New("message to large")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var ErrMessageTooLarge = errors.New("message to large")
var ErrMessageTooLarge = errors.New("message too large")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog PRs not listed in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants