1Add per-attempt timeout and idempotency-aware retries to the REST client

[ximpanz]

The upstream SDK uses http.DefaultClient (no timeout) and a single Do() with no retries, so a stalled NetBird management API hangs plan/apply indefinitely, and transient 429/5xx/connection errors fail the whole run. The SDK also leaks the response body on its error path, which can wedge the connection pool after a burst of error responses.

• internal/provider/httpclient.go: retryTransport, an http.RoundTripper with a per-attempt context timeout and bounded exponential backoff. 429 retries any method; 5xx and transport/timeout errors retry only idempotent methods (GET/HEAD/PUT/DELETE/OPTIONS) to avoid duplicate POST/PATCH side effects. Drains+closes bodies between attempts, honors Retry-After, and stops on outer context cancellation.
• internal/provider/provider.go: pass the client via WithHttpClient(); new optional request_timeout (NB_REQUEST_TIMEOUT, default 30s) and max_retries (NB_MAX_RETRIES, default 4) provider settings.
• internal/provider/httpclient_test.go: retry-policy unit tests.
• internal/provider/settings_test.go: intSetting precedence tests (config > env > default, negative/invalid fallback).
• internal/provider/provider_test.go: include the two new attributes in TestProviderUserAgent's config so it matches the updated schema.
• docs/index.md: document request_timeout and max_retries.

Summary by CodeRabbit

• New Features

  • Added max_retries configuration option to control Management API retry behavior (default: 4, configurable via NB_MAX_RETRIES)
  • Added request_timeout configuration option to set per-attempt request timeout in seconds (default: 30, configurable via NB_REQUEST_TIMEOUT)

• Documentation

  • Updated provider documentation with new configuration parameters

[coderabbitai]

Warning

Review limit reached

@ximpanz, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 50 minutes and 21 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 470d6f2d-4c76-4245-b25f-6769e14e13e7

📥 Commits

Reviewing files that changed from the base of the PR and between b2db970 and b63ac63.

📒 Files selected for processing (6)

• docs/index.md
• internal/provider/httpclient.go
• internal/provider/httpclient_test.go
• internal/provider/provider.go
• internal/provider/provider_test.go
• internal/provider/settings_test.go

📝 Walkthrough

Walkthrough

This PR adds configurable HTTP retry logic with per-attempt timeouts to the Terraform NetBird provider. It implements a custom http.RoundTripper that retries idempotent requests on 5xx errors and all requests on 429 status codes, applies exponential backoff with jitter, and exposes request_timeout and max_retries as Terraform provider settings with environment-variable overrides.

Changes

HTTP Retry and Configuration

Layer / File(s)	Summary
HTTP retry transport core internal/provider/httpclient.go	Implements retryTransport with per-attempt context timeouts, exponential backoff with jitter, idempotency-aware retry decisions (5xx only for idempotent GET/HEAD/DELETE, 429 for all methods), request-body buffering for replay, drain-and-cleanup before each retry, and cancelOnCloseBody wrapper to release per-attempt context when the response is consumed.
HTTP retry transport tests internal/provider/httpclient_test.go	Tests verify GET retries on 5xx (two attempts, 200 on second), POST does not retry on 5xx (one attempt, 500), POST retries on 429 (two attempts, 201 on second), and per-attempt timeout is bounded (error on timeout, 1 + maxRetries total attempts).
Configuration setting resolution and provider integration internal/provider/provider.go, internal/provider/settings_test.go	Introduces intSetting helper to resolve optional integer settings with precedence: explicit Terraform value, environment variable (NB_REQUEST_TIMEOUT, NB_MAX_RETRIES), then default. Extends NetBirdProviderModel with RequestTimeout and MaxRetries fields, updates provider schema with new optional attributes, modifies Configure to compute settings and pass a retry-enabled HTTP client to the NetBird client. Includes unit tests for intSetting covering precedence, zero/negative/null values, and fallback behavior.
Test and documentation updates internal/provider/provider_test.go, docs/index.md	Extends TestProviderUserAgent to include request_timeout and max_retries in the test config payload. Documents the new settings in the provider schema with environment-variable names, defaults (timeout: 30s, max retries: 4), and note that retries can be disabled by setting max_retries to 0.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 A retry transport hops through the code,
With timeouts and backoff down the road,
Five-hundred errors fade away,
Idempotent GET gets another day!
Configuration, settings, defaults all in place—
Terraform providers now retry with grace. 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% 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 accurately describes the main change: adding per-attempt timeout and idempotency-aware retries to the REST client, which is the core functionality across all modified files.
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

---

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.}

[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 `@internal/provider/provider.go`:
- Around line 83-98: intSetting currently accepts 0 and that 0 is passed through
as perTryTimeout into newRetryingHTTPClient causing immediate per-attempt
timeouts and possible retry storms; update intSetting (or the caller that reads
request_timeout) to treat 0 as invalid/default by enforcing request_timeout >= 1
(or mapping 0 -> def) so perTryTimeout is never 0, and add unit tests covering
perTryTimeout==0 behaviour and interaction with max_retries and idempotent
methods to verify retries/backoff; reference intSetting, request_timeout,
perTryTimeout, newRetryingHTTPClient and max_retries when making the change and
in test names.

🪄 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: 2e29b5b0-28a4-4d7a-9b6e-c92b73a69a85

📥 Commits

Reviewing files that changed from the base of the PR and between f865f5c and b2db970.

📒 Files selected for processing (6)

• docs/index.md
• internal/provider/httpclient.go
• internal/provider/httpclient_test.go
• internal/provider/provider.go
• internal/provider/provider_test.go
• internal/provider/settings_test.go