fix: http connection transport TLS

[adityathebe]

Summary by CodeRabbit

• Improvements

  • Enhanced HTTP TLS/SSL connection handling with improved support for custom CA certificates and client certificate authentication.
  • Refined secure connection observability integration.

• Tests

  • Added comprehensive test coverage for TLS configuration validation and HTTPS connections, including scenarios with custom certificate authorities, client certificates, and skip-verify options.

[coderabbitai]

Warning

Rate limit exceeded

@adityathebe has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 46 minutes and 21 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 644b6db4-41f7-4086-b084-39b453530255

📥 Commits

Reviewing files that changed from the base of the PR and between b5c34a1 and 955a777.

📒 Files selected for processing (3)

• connection/http.go
• connection/http_test.go
• connection/prometheus.go

Walkthrough

This PR refactors HTTP TLS handling by introducing TLSConfig.clientConfig() to build TLS configurations and updating TLSConfig.IsEmpty() semantics. The HTTPConnection.TransportWithContext method is refactored to use a new h.transport() helper that applies TLS client settings, and feature-driven observability wiring replaces hardcoded transport selection. Comprehensive test coverage validates TLS config behavior and transport outcomes.

Changes

HTTP TLS Handling

Layer / File(s)	Summary
TLS Config Contract and Methods connection/http.go	Adds crypto/tls and crypto/x509 imports; updates TLSConfig.IsEmpty() to consider TLS enabled when InsecureSkipVerify, HandshakeTimeout, or any CA/cert/key material is present; introduces TLSConfig.clientConfig() to build a tls.Config with optional system CA augmentation and client certificate loading.
HTTP Transport and Observability Wiring connection/http.go	Refactors HTTPConnection.TransportWithContext to derive observability feature name from client options (default "http"), introduces h.transport() helper that clones net/http.DefaultTransport and applies TLS client settings with handshake timeout, and removes prior round-tripper code that conditionally assigned conn.TLS.
TLS and Transport Test Coverage connection/http_test.go	Extends test imports with encoding/pem, net/http, net/http/httptest, time; adds TestTLSConfigIsEmpty to verify config emptiness detection, TestHTTPConnectionTransportTLS to verify TLS outcomes against an httptest server under multiple scenarios (unknown CA, custom CA, insecure skip-verify), and TestCreateHTTPClientWithTLS to validate client creation with InsecureSkipVerify.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly describes the main change: refactoring HTTP TLS handling and transport implementation in the connection layer.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/http-connection-transport-tls

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch fix/http-connection-transport-tls

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Benchstat (Other)

Base: 369727c670ee578eac3e33f0b0294e8938f9873c
Head: 955a77766587cd28a40f21abb64ecff4e560b5e4

✅ 1 improvement(s)

Benchmark	Base	Head	Change	p-value
ResourceSelectorQueryBuild/tags-4	17.41µ	17.21µ	-1.13%	0.002

Full benchstat output

goos: linux
goarch: amd64
pkg: github.com/flanksource/duty/bench
cpu: AMD EPYC 7763 64-Core Processor                
                                                       │ bench-base.txt │           bench-head.txt           │
                                                       │     sec/op     │    sec/op     vs base              │
InsertionForRowsWithAliases/external_users.aliases-4       641.8µ ±  4%   645.5µ ±  7%       ~ (p=0.937 n=6)
InsertionForRowsWithAliases/config_items.external_id-4     1.140m ± 11%   1.146m ± 10%       ~ (p=1.000 n=6)
InsertionOfConfigsWithProperties-4                         3.775m ±  2%   3.770m ±  1%       ~ (p=0.699 n=6)
UpdateOfConfigsWithProperties-4                            7.504m ±  2%   7.507m ±  5%       ~ (p=0.699 n=6)
ResourceSelectorConfigs/name-4                             214.7µ ±  3%   211.2µ ±  3%       ~ (p=0.180 n=6)
ResourceSelectorConfigs/name_and_type-4                    227.5µ ±  5%   228.8µ ±  5%       ~ (p=0.937 n=6)
ResourceSelectorConfigs/tags-4                             28.83m ±  3%   28.78m ±  4%       ~ (p=0.589 n=6)
ResourceSelectorQueryBuild/name-4                          43.53µ ±  2%   42.82µ ±  4%       ~ (p=0.180 n=6)
ResourceSelectorQueryBuild/name_and_type-4                 64.45µ ±  2%   64.19µ ±  1%       ~ (p=0.485 n=6)
ResourceSelectorQueryBuild/tags-4                          17.41µ ±  1%   17.21µ ±  0%  -1.13% (p=0.002 n=6)
geomean                                                    519.3µ         517.5µ        -0.34%

[github-actions]

Benchstat (RLS)

Base: 369727c670ee578eac3e33f0b0294e8938f9873c
Head: 955a77766587cd28a40f21abb64ecff4e560b5e4

📊 6 minor regression(s) (all within 5% threshold)

Benchmark	Base	Head	Change	p-value
RLS/Sample-15000/config_names/Without_RLS-4	13.54m	14.15m	+4.54%	0.002
RLS/Sample-15000/config_detail/With_RLS-4	138.2m	142.6m	+3.24%	0.002
RLS/Sample-15000/config_summary/Without_RLS-4	101.4m	103.5m	+2.07%	0.026
RLS/Sample-15000/configs/With_RLS-4	138.2m	140.4m	+1.61%	0.004
RLS/Sample-15000/catalog_changes/Without_RLS-4	5.285m	5.362m	+1.46%	0.041
RLS/Sample-15000/config_types/With_RLS-4	139.4m	141.1m	+1.27%	0.004

✅ 4 improvement(s)

Benchmark	Base	Head	Change	p-value
RLS/Sample-15000/config_changes/Without_RLS-4	5.787m	5.322m	-8.04%	0.002
RLS/Sample-15000/config_types/Without_RLS-4	5.053m	4.777m	-5.45%	0.002
RLS/Sample-15000/config_names/With_RLS-4	141.6m	138.2m	-2.39%	0.002
RLS/Sample-15000/analysis_types/With_RLS-4	4.020m	3.953m	-1.67%	0.026

Full benchstat output

goos: linux
goarch: amd64
pkg: github.com/flanksource/duty/bench
cpu: AMD EPYC 7763 64-Core Processor                
                                               │ bench-base.txt │           bench-head.txt           │
                                               │     sec/op     │    sec/op     vs base              │
RLS/Sample-15000/catalog_changes/Without_RLS-4      5.285m ± 2%   5.362m ±  2%  +1.46% (p=0.041 n=6)
RLS/Sample-15000/catalog_changes/With_RLS-4         143.6m ± 1%   144.0m ±  2%       ~ (p=0.310 n=6)
RLS/Sample-15000/config_changes/Without_RLS-4       5.787m ± 4%   5.322m ±  1%  -8.04% (p=0.002 n=6)
RLS/Sample-15000/config_changes/With_RLS-4          145.7m ± 6%   145.1m ±  3%       ~ (p=0.394 n=6)
RLS/Sample-15000/config_detail/Without_RLS-4        4.829m ± 2%   4.944m ±  2%       ~ (p=0.093 n=6)
RLS/Sample-15000/config_detail/With_RLS-4           138.2m ± 1%   142.6m ±  3%  +3.24% (p=0.002 n=6)
RLS/Sample-15000/config_names/Without_RLS-4         13.54m ± 1%   14.15m ±  3%  +4.54% (p=0.002 n=6)
RLS/Sample-15000/config_names/With_RLS-4            141.6m ± 1%   138.2m ±  1%  -2.39% (p=0.002 n=6)
RLS/Sample-15000/config_summary/Without_RLS-4       101.4m ± 3%   103.5m ±  2%  +2.07% (p=0.026 n=6)
RLS/Sample-15000/config_summary/With_RLS-4          723.0m ± 1%   722.5m ±  3%       ~ (p=0.937 n=6)
RLS/Sample-15000/configs/Without_RLS-4              8.291m ± 6%   8.424m ± 15%       ~ (p=0.818 n=6)
RLS/Sample-15000/configs/With_RLS-4                 138.2m ± 1%   140.4m ±  1%  +1.61% (p=0.004 n=6)
RLS/Sample-15000/analysis_types/Without_RLS-4       3.932m ± 1%   3.980m ±  3%       ~ (p=0.132 n=6)
RLS/Sample-15000/analysis_types/With_RLS-4          4.020m ± 1%   3.953m ±  2%  -1.67% (p=0.026 n=6)
RLS/Sample-15000/analyzer_types/Without_RLS-4       3.829m ± 2%   3.789m ±  1%       ~ (p=0.240 n=6)
RLS/Sample-15000/analyzer_types/With_RLS-4          3.817m ± 4%   3.884m ±  4%       ~ (p=0.180 n=6)
RLS/Sample-15000/change_types/Without_RLS-4         5.584m ± 2%   5.372m ±  7%       ~ (p=0.065 n=6)
RLS/Sample-15000/change_types/With_RLS-4            5.551m ± 3%   5.445m ±  4%       ~ (p=0.310 n=6)
RLS/Sample-15000/config_classes/Without_RLS-4       4.304m ± 2%   4.301m ±  4%       ~ (p=1.000 n=6)
RLS/Sample-15000/config_classes/With_RLS-4          138.2m ± 1%   138.7m ±  1%       ~ (p=0.485 n=6)
RLS/Sample-15000/config_types/Without_RLS-4         5.053m ± 4%   4.777m ±  0%  -5.45% (p=0.002 n=6)
RLS/Sample-15000/config_types/With_RLS-4            139.4m ± 0%   141.1m ±  1%  +1.27% (p=0.004 n=6)
geomean                                             21.50m        21.46m        -0.18%

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@connection/http.go`:
- Around line 42-43: TLSConfig.IsEmpty currently treats a config with only Cert
or only Key as non-empty but the certificate-loading code (around the client
cert load at lines ~64-70) only creates a tls.Certificate when both PEMs are
present, allowing a partial mTLS config to silently fall back; update
TLSConfig.IsEmpty to treat a config as invalid/non-empty when exactly one of
Cert or Key is present (i.e. detect XOR) and change the certificate-loading
logic (the client cert construction path) to return an explicit error when Cert
is provided without Key or Key without Cert instead of skipping client cert
creation so partial client-certificate input is rejected.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9a02ce7a-013c-477a-84ae-10192f423b24

📥 Commits

Reviewing files that changed from the base of the PR and between 369727c and b5c34a1.

📒 Files selected for processing (2)

• connection/http.go
• connection/http_test.go

[github-actions]

Gavel results

Gavel exited with code .

View full results