-
Notifications
You must be signed in to change notification settings - Fork 470
Add ConnectionClosedCallback #244
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
Comments
@SimoneDutto you can get around this by listening to the session context. Just spin off a goroutine in E.g.: ConnCallback: func(ctx ssh.Context, conn net.Conn) net.Conn {
go func() {
<-ctx.Done()
fmt.Println("Connection concluded")
}()
return conn
}, |
@wolveix i didn't know that, thank you so much! |
@SimoneDutto you're very welcome :) Yeah, context should always conclude. Looking at this library's source code, they don't set a timeout for the context itself; however, they initialize it with a cancel function and always call it. So unless a panic occurs within the library, the context will conclude :) |
…onnections #19236 In this PR we hook the max concurrent connections into the ssh server. It works by checking for the number of connections via an atomic.Int in the connectionCallback. It was originally suggested in a gliderlab ssh issue: gliderlabs/ssh#244 The error we write down the conn is not displayed by ssh clients, because they expect a ssh handshake. However if you telnet to a maxed ssh server to debug it, you could see the `too many connections` error, which is helpful. ## Checklist <!-- If an item is not applicable, use `~strikethrough~`. --> - [x] Code style: imports ordered, good names, simple structure, etc - [x] Comments saying why design decisions were made - [x] [Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing ## QA steps ```bash juju bootstrap lxd test-max-conns --build-agent juju controller-config ssh-max-concurrent-connections=10 #make it easier to qa juju show-controller #to get controller ip ``` ```console $ ssh -J admin@<controller_ip>:17022 bob@e12e3493-d2f7-42e2-8049-8cb624dd0a4e Your final destination is: e12e3493-d2f7-42e2-8049-8cb624dd0a4e as user: bob Connection to e12e3493-d2f7-42e2-8049-8cb624dd0a4e closed. ``` Send multiple concurrent ssh connections ```console $ seq 16 | parallel -j 16 "ssh -J admin@10.229.36.38:17022 -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null bob@e12e3493-d2f7-42e2-8049-8cb624dd0a4e" kex_exchange_identification: read: Connection reset by peer Connection reset by 10.229.36.38 port 17022 Connection closed by UNKNOWN port 65535 Connection closed by 10.229.36.38 port 17022 Connection closed by UNKNOWN port 65535 Connection closed by 10.229.36.38 port 17022 Connection closed by UNKNOWN port 65535 Connection closed by 10.229.36.38 port 17022 Connection closed by UNKNOWN port 65535 Connection closed by 10.229.36.38 port 17022 Connection closed by UNKNOWN port 65535 Connection closed by 10.229.36.38 port 17022 Connection closed by UNKNOWN port 65535 Your final destination is: e12e3493-d2f7-42e2-8049-8cb624dd0a4e as user: bob Warning: Permanently added 'e12e3493-d2f7-42e2-8049-8cb624dd0a4e' (RSA) to the list of known hosts. Your final destination is: e12e3493-d2f7-42e2-8049-8cb624dd0a4e as user: bob Warning: Permanently added 'e12e3493-d2f7-42e2-8049-8cb624dd0a4e' (RSA) to the list of known hosts. Your final destination is: e12e3493-d2f7-42e2-8049-8cb624dd0a4e as user: bob Warning: Permanently added 'e12e3493-d2f7-42e2-8049-8cb624dd0a4e' (RSA) to the list of known hosts. Your final destination is: e12e3493-d2f7-42e2-8049-8cb624dd0a4e as user: bob Warning: Permanently added 'e12e3493-d2f7-42e2-8049-8cb624dd0a4e' (RSA) to the list of known hosts. Your final destination is: e12e3493-d2f7-42e2-8049-8cb624dd0a4e as user: bob Warning: Permanently added 'e12e3493-d2f7-42e2-8049-8cb624dd0a4e' (RSA) to the list of known hosts. Your final destination is: e12e3493-d2f7-42e2-8049-8cb624dd0a4e as user: bob Warning: Permanently added 'e12e3493-d2f7-42e2-8049-8cb624dd0a4e' (RSA) to the list of known hosts. Your final destination is: e12e3493-d2f7-42e2-8049-8cb624dd0a4e as user: bob Warning: Permanently added 'e12e3493-d2f7-42e2-8049-8cb624dd0a4e' (RSA) to the list of known hosts. Your final destination is: e12e3493-d2f7-42e2-8049-8cb624dd0a4e as user: bob Warning: Permanently added 'e12e3493-d2f7-42e2-8049-8cb624dd0a4e' (RSA) to the list of known hosts. Your final destination is: e12e3493-d2f7-42e2-8049-8cb624dd0a4e as user: bob Warning: Permanently added 'e12e3493-d2f7-42e2-8049-8cb624dd0a4e' (RSA) to the list of known hosts. Your final destination is: e12e3493-d2f7-42e2-8049-8cb624dd0a4e as user: bob Warning: Permanently added 'e12e3493-d2f7-42e2-8049-8cb624dd0a4e' (RSA) to the list of known hosts. ``` (this output can vary depending on how many requests you actually managed to send concurrently) You should see as many "your final destination ..." as your max concurrent connections. To see the metric being exposed by the inspection worker you can: ```bash juju ssh 0 sudo curl --unix-socket /var/lib/juju/agents/machine-0/introspection.socket http://localhost/depengine ```
Hello,
I am trying to implement a MaxConcurrentConnections for the ssh server.
I've notice the library already offers a
ConnCallback
andConnectionFailedCallback
.However that seems not to be enough to create a guard around the number of maximum connection because we are missing the
ConnectionClosedCallback
.With something like
ConnectionClosedCallback
I could simply implement my rate limiter by doing:I'll put a PR up just to show you how I've tried it to implement the
ConnectionClosedCallback
, but I am of course open to suggestions!The text was updated successfully, but these errors were encountered: