Skip to content

refactor(httpclient): unify HTTP client across all platforms#646

Open
frjcomp wants to merge 6 commits into
mainfrom
refactor/http-client-unification
Open

refactor(httpclient): unify HTTP client across all platforms#646
frjcomp wants to merge 6 commits into
mainfrom
refactor/http-client-unification

Conversation

@frjcomp
Copy link
Copy Markdown
Collaborator

@frjcomp frjcomp commented Jun 1, 2026

Summary

Centralises all HTTP client configuration in pkg/httpclient so every platform (Bitbucket, DevOps, Circle, Jenkins, Gitea) shares the same TLS, proxy, timeout, and retry settings — previously each used its own ad-hoc configuration.

Changes

pkg/httpclient/client.go — central factory

  • SetInsecureSkipVerify(bool) — toggleable TLS verification (default true, preserving existing behaviour)
  • SetSOCKSProxy(string) — SOCKS5 proxy support via golang.org/x/net/proxy
  • SetHTTPTimeout(duration) — configurable per-request timeout
  • GetPipeleekTransport() — new helper returning a configured *http.Transport for non-retryable clients (Resty)
  • Thread-safe global config via sync.RWMutex

internal/cmd/root.go — three new persistent flags

Flag Default Description
--insecure-skip-verify true Skip TLS certificate verification
--socks-proxy "" SOCKS5 proxy URL
--http-timeout 0 HTTP request timeout (0 = no timeout)

Platform injection

Platform Before After
Bitbucket (Resty) plain resty.New() .SetTransport(GetPipeleekTransport())
Azure DevOps (Resty) plain resty.New() .SetTransport(GetPipeleekTransport())
CircleCI &http.Client{Timeout: 45s} GetPipeleekHTTPClient().StandardClient()
Jenkins nil (SDK default) GetPipeleekHTTPClient().StandardClient()
Gitea SDK post-hoc transport mutation gitea.SetHTTPClient(GetPipeleekHTTPClient().StandardClient()) — clean injection, no mutation

Note: The GitHub SDK client uses a dedicated go-github-ratelimit transport and is intentionally left unchanged.

Tests

  • Expanded pkg/httpclient/client_test.go — 22 tests covering all new setter/getter functions
  • New pkg/bitbucket/scan/client_test.go — 6 tests (transport injection, cookie jar, TLS config)
  • New pkg/devops/scan/client_test.go — 4 tests (transport injection, URLs, TLS config)
  • New pkg/gitea/scan/scanner_test.go — 6 tests using httptest.Server mock (SDK injection, auth headers, cookie jar, no transport mutation, TLS config, invalid URL)

Docs

  • docs/introduction/configuration.md — new "HTTP Client Settings" section documenting all four global flags

Test results

  • Unit suite (go test ./... -race): all packages pass ✅
  • E2E (Jenkins, Bitbucket, Circle): all pass ✅

- Introduce configurable global state in pkg/httpclient:
  - SetInsecureSkipVerify(bool) — toggleable TLS verification (default true)
  - SetSOCKSProxy(string) — SOCKS5 proxy support via golang.org/x/net/proxy
  - SetHTTPTimeout(duration) — per-request timeout
  - GetPipeleekTransport() — shared *http.Transport for non-retryable clients

- Wire three new persistent root flags:
  --insecure-skip-verify (default true), --socks-proxy, --http-timeout

- Inject Pipeleek transport/client into all platforms:
  - Bitbucket/DevOps (Resty): .SetTransport(GetPipeleekTransport())
  - Circle: GetPipeleekHTTPClient().StandardClient()
  - Jenkins: GetPipeleekHTTPClient().StandardClient() (was nil)
  - Gitea SDK: gitea.SetHTTPClient(GetPipeleekHTTPClient().StandardClient())
    removing post-hoc transport mutation

- Add unit tests for new httpclient functions (22 tests)
- Add unit tests for Bitbucket NewClient, DevOps NewClient,
  and Gitea InitializeOptions (16 new tests with httptest mock servers)
- Update docs/introduction/configuration.md with HTTP Client Settings section
Copilot AI review requested due to automatic review settings June 1, 2026 11:59
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR centralizes HTTP client/transport configuration in pkg/httpclient and injects it into multiple platform scanners/SDK clients so they share consistent TLS, proxy/SOCKS, and timeout behavior, with new global CLI flags to control these settings.

Changes:

  • Added a global, mutex-protected HTTP client configuration (TLS verify toggle, SOCKS proxy, request timeout) and transport factory helpers in pkg/httpclient.
  • Updated platform clients (Bitbucket, Azure DevOps, CircleCI, Jenkins, Gitea SDK) to use the shared Pipeleek transport / standard client injection.
  • Added root-level persistent CLI flags and documented the new HTTP client settings.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/httpclient/client.go Introduces global HTTP client config and transport/client factories (TLS/SOCKS/proxy/timeout).
pkg/httpclient/client_test.go Adds unit tests for the new global setters and transport behavior.
internal/cmd/root.go Adds persistent flags and wires them into pkg/httpclient during PersistentPreRun.
pkg/bitbucket/scan/api.go Injects Pipeleek transport into Resty client.
pkg/bitbucket/scan/client_test.go Adds tests validating transport injection, cookie jar behavior, and TLS settings.
pkg/devops/scan/api.go Injects Pipeleek transport into Resty client.
pkg/devops/scan/client_test.go Adds tests validating transport injection and TLS settings.
pkg/circle/scan/scanner.go Switches CircleCI API client to Pipeleek’s standard HTTP client.
pkg/jenkins/scan/client.go Injects Pipeleek’s standard HTTP client into the Jenkins SDK client.
pkg/gitea/scan/scanner.go Uses Pipeleek clients and cleanly injects the standard client into the Gitea SDK.
pkg/gitea/scan/scanner_test.go Adds tests validating SDK injection, auth header behavior, cookie jar, and TLS settings.
docs/introduction/configuration.md Documents new global HTTP client flags and examples.

Comment thread pkg/httpclient/client.go Outdated
Comment thread docs/introduction/configuration.md Outdated
vscode added 5 commits June 2, 2026 06:43
- Set Timeout on SOCKS net.Dialer to prevent indefinite hangs on
  unreachable proxies; uses configured --http-timeout or 30s fallback
- Correct docs: --http-timeout only applies to retryable HTTP client
  users (GitLab, Gitea, Jenkins, CircleCI, NIST); Bitbucket and
  Azure DevOps use Resty transport injection and are unaffected
…ll platforms

- GetPipeleekHTTPClient now returns *resty.Client (was *retryablehttp.Client)
- All platform scan/util packages updated to use native Resty API:
  .R().Get()/.Post(), resp.StatusCode(), resp.Bytes(), client.Client()
- Platforms using SDK clients (GitLab, Gitea, gojenkins, CircleCI) call
  .Client() to extract *http.Client while still inheriting the shared transport
- Removes direct dependency on go-retryablehttp; retained as indirect
  (transitive via TruffleHog)
- Update docs/introduction/proxying.md: fix stale retryablehttp reference in
  http-timeout scope note
- Add pipeleek_test_build to .gitignore
… GitLab enum client

- github/scan: pass httpclient.GetPipeleekTransport() as base to
  github_ratelimit.New() instead of nil (which fell back to http.DefaultTransport,
  bypassing TLS/proxy config)
- gitlab/enum: replace bare resty.New() with httpclient.GetPipeleekHTTPClient()
  so --proxy, --tls-verification and --http-timeout flags apply to enum calls
…h GetPipeleekHTTPClient

Both clients were using resty.New().SetTransport(GetPipeleekTransport()) which
missed retry logic (count, wait, conditions) and --http-timeout. Switching to
GetPipeleekHTTPClient() restores full parity with other platforms.
Replace manual AddRetryConditions (429 / 5xx-except-501 / error logic)
with EnableRetryDefaultConditions(), which implements the same behaviour
including 501 exclusion, status-0, and transient URL errors.

Logging is preserved via AddRetryHooks. Update test to assert the new
configuration style.
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.

2 participants