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

feat: Add peerbroker proxy for agent connections #349

Merged
merged 3 commits into from
Feb 23, 2022
Merged

feat: Add peerbroker proxy for agent connections #349

merged 3 commits into from
Feb 23, 2022

Conversation

kylecarbs
Copy link
Member

Agents will connect using this proxy. Eventually we'll intercept
some of these messages for validation, but that's not necessary right now.

Agents will connect using this proxy. Eventually we'll intercept
some of these messages for validation, but that's not necessary right now.
@kylecarbs kylecarbs self-assigned this Feb 22, 2022
@codecov
Copy link

codecov bot commented Feb 22, 2022

Codecov Report

Merging #349 (4499367) into main (a053fe8) will increase coverage by 0.04%.
The diff coverage is 57.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #349      +/-   ##
==========================================
+ Coverage   67.80%   67.85%   +0.04%     
==========================================
  Files         147      146       -1     
  Lines        7905     7886      -19     
  Branches       72       72              
==========================================
- Hits         5360     5351       -9     
+ Misses       2002     1990      -12     
- Partials      543      545       +2     
Flag Coverage Δ
unittest-go-macos-latest 66.25% <57.23%> (-0.18%) ⬇️
unittest-go-ubuntu-latest 67.15% <55.97%> (-0.46%) ⬇️
unittest-go-windows-2022 ?
unittest-js 66.10% <ø> (ø)
Impacted Files Coverage Δ
peerbroker/proxy.go 57.23% <57.23%> (ø)
pty/ptytest/ptytest.go 85.18% <0.00%> (-5.56%) ⬇️
peer/channel.go 81.87% <0.00%> (-0.59%) ⬇️
peer/conn.go 78.26% <0.00%> (ø)
pty/pty_windows.go
pty/start_windows.go
provisionerd/provisionerd.go 70.11% <0.00%> (+0.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a053fe8...4499367. Read the comment docs.

Comment on lines 39 to 40
// PubSub is used to geodistribute in a simple way. All message payloads
// are small in size <=8KB, and we don't require delivery guarantees.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding this context, very helpful for me!

The "don't require delivery guarantees" part I was unsure about - is it because the agent is giving information like stats that come periodically and will get renewed? Just worried about losing things like shell commands from the customer -> proxy -> agent or losing output from agent -> proxy -> customer.

Copy link
Member Author

Choose a reason for hiding this comment

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

I improved the comment here to explain what data flows through.

I added the ASCII chart too, which should help to explain this kinda confusing code.

peerbroker/proxy.go Outdated Show resolved Hide resolved
// └─────────────────────┘ │<agent-id> channel: │ │from payloads to create new │
// │ │ │NegotiateConnection() streams│
// │<stream-id><payload>│ │or write to existing ones. │
// └────────────────────┘ └─────────────────────────────┘
Copy link
Contributor

Choose a reason for hiding this comment

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

I love me some inline diagrams like this! 🖼️

@kylecarbs kylecarbs enabled auto-merge (squash) February 23, 2022 17:10
@kylecarbs kylecarbs merged commit b58e168 into main Feb 23, 2022
@kylecarbs kylecarbs deleted the peerproxy branch February 23, 2022 17:15
@bpmct
Copy link
Member

bpmct commented Nov 15, 2023

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.

None yet

4 participants