Repository navigation
Retry transient LDAP connection failures in binds and searches - #86
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 98c5825a-bd27-4391-a924-14e339aab71e
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues block the change adding bounded retries to initial LDAP connection setup.
No open issues 🎉
What changed in this PR
This PR makes initial LDAP connection setup resilient to transient connection refusals without replaying searches or mutations.
Changes:
- Retry
Errno::ECONNREFUSEDup to three total attempts, with 1- and 2-second delays and retry/exhaustion logging. - Create a fresh client for each attempt and cache only a successfully bound client.
- Add regression coverage for recovery, exhaustion, caching, and failures that must not be retried.
| File | Description |
|---|---|
spec/unit/entitlements/service/ldap_spec.rb |
Adds retry regressions and verifies non-retryable failures and operations remain unchanged. |
lib/entitlements/service/ldap.rb |
Adds bounded connection-refusal retries and logging around initial LDAP setup. |
Review effort: Balanced (auto)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 98c5825a-bd27-4391-a924-14e339aab71e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 98c5825a-bd27-4391-a924-14e339aab71e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 98c5825a-bd27-4391-a924-14e339aab71e
northrup
left a comment
There was a problem hiding this comment.
Retry shape looks right to me. The part I'd change is the classifier: exact-message matching depends on libc wording, and it misses a few transient failures. I ran it against real failures (net-ldap 0.19.0, Ruby 3.4.9):
| Failure | This PR (glibc) | This PR (musl) | Suggested |
|---|---|---|---|
| Connection refused | ✅ | ✅ | ✅ |
| Connect timeout | ✅ | ✅ | ✅ |
DNS EAI_AGAIN |
✅ | ❌ | ✅ |
| TLS handshake timeout | ❌ | ❌ | ✅ |
| Accept, then reset (LB with no backend) | ❌ | ❌ | ✅ |
| NXDOMAIN | – | – | ✅ |
| Bad cert | – | – | – |
✅ retried · ❌ should have been · – not retried
musl spells EAI_AGAIN as getaddrinfo: Try again, so the DNS retry never fires on Alpine.
The good news is net-ldap does the classifying for us. Every single-host connection-setup failure comes back as a bare Net::LDAP::Error (error.rb), and operation errors are subclasses. So key on the class and deny-list the permanent TLS failures. Getting it wrong that way costs ~3s; getting it wrong the current way fails the run.
Suggestions are inline. With them applied, RuboCop and the unit suite are unchanged, and the new specs fail against the current classifier.
Two smaller things:
- 1s + 2s covers a blip, not a restart or failover. It depends on what bit you.
- The runner pin is setup-ruby's documented workaround for
ubuntu-26.04. It might merge faster as its own PR.
Repro
# Usage: ruby repro.rb [dns] (requires: gem install net-ldap -v 0.19.0)
require "net/ldap"
require "socket"
require "openssl"
def pr86?(e) # PR #86 as written
return false unless [Errno::ECONNREFUSED, Errno::ETIMEDOUT, Net::LDAP::Error].any? { |k| k === e }
return true if e.is_a?(Errno::ECONNREFUSED) || e.is_a?(Errno::ETIMEDOUT)
return false unless e.instance_of?(Net::LDAP::Error)
["getaddrinfo: Temporary failure in name resolution",
"Connection timed out - user specified timeout",
"Operation timed out - user specified timeout"].include?(e.message)
end
def suggested?(e) # suggested rescue + retryable_connection_error?
return false unless [SystemCallError, Net::LDAP::Error].any? { |k| k === e }
case e
when Errno::ECONNREFUSED, Errno::ECONNRESET, Errno::EPIPE, Errno::ETIMEDOUT then true
when Net::LDAP::Error
e.instance_of?(Net::LDAP::Error) && !e.message.match?(/certificate verify failed|does not match the server certificate/)
else false
end
end
def bind(label, host, port, tls: false)
opts = { host: host, port: port, connect_timeout: 1, auth: { method: :simple, username: "cn=x", password: "x" } }
opts[:encryption] = { method: :simple_tls, tls_options: { verify_mode: OpenSSL::SSL::VERIFY_PEER } } if tls
Net::LDAP.new(opts).bind
rescue => e
puts format("%-20s pr86=%-5s suggested=%-5s %s: %s", label, pr86?(e), suggested?(e), e.class, e.message[0, 55])
end
def listener(&handler)
server = TCPServer.new("127.0.0.1", 0)
Thread.new { loop { handler.call(server.accept) rescue nil } }
server.addr[1]
end
if ARGV == ["dns"]
bind("dns-eai-again", "ldap.example.com", 389)
exit
end
closed = TCPServer.new("127.0.0.1", 0)
refused_port = closed.addr[1]
closed.close
bind("refused", "127.0.0.1", refused_port)
# A full accept queue makes Linux drop SYNs, giving a genuine connect timeout.
backlog = Socket.new(:INET, :STREAM)
backlog.bind(Addrinfo.tcp("127.0.0.1", 0))
backlog.listen(0)
fillers = 3.times.map { Socket.new(:INET, :STREAM).tap { |c| c.connect_nonblock(backlog.local_address, exception: false) } }
sleep 0.3
bind("connect-timeout", "127.0.0.1", backlog.local_address.ip_port)
bind("nxdomain", "nonexistent.invalid", 389)
bind("tls-handshake-stall", "127.0.0.1", listener { |c| Thread.new { sleep 30; c.close } }, tls: true)
bind("accept-then-reset", "127.0.0.1", listener(&:close))
key = OpenSSL::PKey::RSA.new(2048)
cert = OpenSSL::X509::Certificate.new
cert.version = 2
cert.serial = 1
cert.subject = cert.issuer = OpenSSL::X509::Name.parse("/CN=localhost")
cert.public_key = key.public_key
cert.not_before = Time.now - 60
cert.not_after = Time.now + 3600
cert.sign(key, OpenSSL::Digest.new("SHA256"))
ctx = OpenSSL::SSL::SSLContext.new
ctx.cert = cert
ctx.key = key
tls_server = OpenSSL::SSL::SSLServer.new(TCPServer.new("127.0.0.1", 0), ctx)
Thread.new { loop { tls_server.accept rescue nil } }
bind("cert-verify-failed", "127.0.0.1", tls_server.to_io.addr[1], tls: true)for img in ruby:3.4.9-slim ruby:3.4.9-alpine; do
docker run --rm -v "$PWD/repro.rb:/repro.rb:ro" "$img" sh -c '
gem install -q net-ldap -v 0.19.0 >/dev/null &&
ruby /repro.rb &&
printf "nameserver 192.0.2.1\noptions timeout:1 attempts:1\n" > /etc/resolv.conf &&
ruby /repro.rb dns'
doneCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 98c5825a-bd27-4391-a924-14e339aab71e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 98c5825a-bd27-4391-a924-14e339aab71e

Uh oh!
There was an error while loading. Please reload this page.