refactor: HTTP client: unified support for HTTP/HTTPS and Unix sockets #131
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Created reusable http_client module with object-oriented API - Added support for Unix sockets (unix:/path/to/socket) - Implemented connection pooling and keep-alive via resty.http - Unified live and stream modules to use single API_CLIENT instance - Added mTLS support with parsed PEM objects (with file path fallback) - Moved APPSEC_CLIENT from runtime.conf to runtime object - Added request_base() method for AppSec (uses base path only) - Enhanced path building to merge query strings from URL and request - Removed unused get_remediation_http_request helper functions - Fixed unix socket connection format (scheme=nil, host=socket_path)
- Use tostring() to convert query table from url.parse() to string - Fixes error: attempt to concatenate field 'query' (a table value) - Query table has __tostring metatable that calls buildQuery()
…ts and always creating new httpc
LaurenceJJones
force-pushed
the
http-client-refactor
branch
from
ca1b988 to
ef076f4
Compare
The module-level request_uri function was kept for backward compatibility but is not used anywhere in the codebase. All code uses the object-oriented API (http_client.new() + client:request_uri()) instead. Signed-off-by: Laurence <[email protected]>
LaurenceJJones
force-pushed
the
http-client-refactor
branch
from
ef076f4 to
4d07533
Compare
Prevent 'bad request' errors when closing HTTP client connections that may already be in a closed or bad state. This matches the pattern used elsewhere in the file for safe connection cleanup.
- Test 17: Fix missing newline in TLS auth test - Test 18: Live mode with Unix socket LAPI - Test 19: Stream mode with Unix socket LAPI - Test 20: Live mode with Unix socket LAPI and metrics These tests verify that the bouncer can connect to the LAPI via Unix sockets for both decisions and metrics endpoints.
- Centralize API key, user agent, and TLS auth handling in http_client - Remove duplicate configuration storage from live, stream, and metrics modules - Add timeout validation and default to 3000ms if not configured - Add APPSEC client initialization with API key support (no mTLS) - Simplify metrics:sendMetrics() signature to remove unused parameters - Add debug logging for timeout configuration
- Remove self.httpc storage, always create new instance per request - Rename _get_httpc() to _create_httpc() to reflect create-per-request pattern - Update _release_httpc() to accept httpc parameter instead of using self.httpc - Fix GET request handling: only include body in request options when present - Follow resty.http pattern: create, use, keepalive (don't store)
…SON decode issues - Simplified http_client API: all complexity (unix/http/https, mTLS) handled automatically - Removed all logging from http_client module - errors returned to callers - Reordered operations: prepare headers/path before connecting for efficiency - Added proper error handling for JSON decode failures in stream.lua, live.lua, and crowdsec.lua - All callers now explicitly check for errors (err ~= nil or not res) - Fixed stack trace issue when response body is empty or invalid JSON
- Remove unused connection_key from client object - Use _release_httpc() consistently (set_keepalive handles closing automatically) - Support single timeout value (applied to all 3) or individual timeouts - Metrics uses resty.http default timeouts (60s) to match original behavior - Stream/Live use REQUEST_TIMEOUT for all three timeouts - AppSec uses individual timeout values (APPSEC_CONNECT/SEND/PROCESS_TIMEOUT)
- Pass single timeout value instead of table with duplicate values - http_client automatically applies single value to all three timeouts (connect, send, read)
- Use explicit if/else pattern matching USE_TLS_AUTH for clarity - Ensures it defaults to false for any value other than "true" - Moved conversion to after TLS certificate parsing for better organization
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WIP
supperceeds #54