Skip to content

Bound NGINX Plus API requests with a timeout#1775

Open
CVanF5 wants to merge 1 commit into
nginx:mainfrom
CVanF5:fix/plus-api-client-timeout
Open

Bound NGINX Plus API requests with a timeout#1775
CVanF5 wants to merge 1 commit into
nginx:mainfrom
CVanF5:fix/plus-api-client-timeout

Conversation

@CVanF5

@CVanF5 CVanF5 commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

The NGINX Plus API HTTP client had no timeout, so a Plus API call that never responds (e.g. an unresponsive Plus API socket) blocks the agent's message bus indefinitely. The agent then never sends a DataPlaneResponse, leaving the management plane waiting on it.

  • Set the HTTP client timeout from Client.HTTP.Timeout (falling back to a default when unset), following the existing datasource/config pattern.
  • Timeout the unix socket dial with net.Dialer.Timeout and pass the per-request context through, following internal/grpc/proxy_dialer.go, so a stuck socket cannot block the dial.
  • Return the original error from createPlusAPIError when it cannot be parsed into the structured Plus API format (e.g. a timeout). Previously it returned nil, so a failed apply was reported to the management plane as COMMAND_STATUS_OKinstead of a failure.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING document
  • I have run make install-tools and have attached any dependency changes to this pull request
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • If applicable, I have checked that any relevant tests pass after adding my changes
  • If applicable, I have updated any relevant documentation (README.md)
  • If applicable, I have tested my cross-platform changes on Ubuntu 22, Redhat 8, SUSE 15 and FreeBSD 13

The NGINX Plus API HTTP client had no timeout, so a Plus API call that
never responds (e.g. an unresponsive Plus API socket) blocks the agent's
single-threaded message bus indefinitely. The agent then never sends a
DataPlaneResponse, leaving the management plane waiting on it.

- Set the HTTP client timeout from Client.HTTP.Timeout (falling back to a
  default when unset), following the existing datasource/config pattern.
- Bound the unix socket dial with net.Dialer.Timeout and pass the
  per-request context through, following internal/grpc/proxy_dialer.go,
  so a stuck socket cannot block the dial.
- Return the original error from createPlusAPIError when it cannot be
  parsed into the structured Plus API format (e.g. a timeout). Previously
  it returned nil, so a failed apply was reported to the management plane
  as COMMAND_STATUS_OK instead of a failure.
@CVanF5 CVanF5 requested a review from a team as a code owner June 30, 2026 20:34
@github-actions github-actions Bot added bug Something isn't working chore Pull requests for routine tasks labels Jun 30, 2026
@CVanF5 CVanF5 changed the title fix: bound NGINX Plus API requests with a timeout Bound NGINX Plus API requests with a timeout Jun 30, 2026
@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.23077% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.92%. Comparing base (006f54b) to head (53ffe37).

Files with missing lines Patch % Lines
internal/nginx/nginx_service.go 69.23% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1775      +/-   ##
==========================================
+ Coverage   84.89%   84.92%   +0.02%     
==========================================
  Files         105      105              
  Lines       13656    13663       +7     
==========================================
+ Hits        11593    11603      +10     
+ Misses       1540     1538       -2     
+ Partials      523      522       -1     
Files with missing lines Coverage Δ
internal/nginx/nginx_service.go 61.41% <69.23%> (+2.43%) ⬆️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 006f54b...53ffe37. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@CVanF5 CVanF5 closed this Jul 1, 2026
@CVanF5 CVanF5 deleted the fix/plus-api-client-timeout branch July 1, 2026 09:37
@CVanF5 CVanF5 restored the fix/plus-api-client-timeout branch July 1, 2026 09:38
@CVanF5 CVanF5 reopened this Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working chore Pull requests for routine tasks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants