feat(api): add async query parameter support - #1609
Merged
Merged
Conversation
Implement support for the HTTP HEAD method on the update API. HEAD requests trigger the update process asynchronously and return HTTP 202 Accepted immediately, allowing clients to initiate updates without waiting for completion. - Register HEAD method in the API router - Update handler to process HEAD requests asynchronously - Ensure lock management and panic recovery in async goroutines - Add unit tests for HEAD request behavior (202 Accepted, 429 Too Many Requests, and blocking behavior)
Refactor the monolithic Handle function into 11 focused, well-documented helper methods to improve maintainability, testability, and code clarity. Preserve existing behavior — HEAD async updates (202 Accepted) and POST synchronous updates (200 OK). Changes: - Extract readRequestBody, extractImages, acquireLock, send429Response - Extract handleHead, handlePost, executeUpdateAsync, executeUpdate, writeSuccessResponse, releaseLock - Add consistent package-level and function doc comments with Parameters/Returns - Add 6 black-box error-path tests: oversized body rejection, cancellation handling, panic recovery in async goroutines, write error handling for both 429 and 200 paths, and early return validation - Increase test coverage from ~87% to 95% - Fix noctx lint violations by using NewRequestWithContext throughout tests
- introduce lockResult struct for acquireLock return values - consolidate Token, Acquired, and RequestErr into single return type - update Handle method to use new lockResult struct fields
Fix the violation of RFC 9110 safe method semantics and use an async query parameter instead. - remove HEAD method registration from update handler endpoint - add async=true query parameter support for asynchronous updates - rename handleHead to handleAsync with updated logging - update tests to use POST with async parameter instead of HEAD method
…eter - add async query parameter documentation to update endpoint table - document HTTP 202 Accepted response for async updates - include curl examples for fire-and-forget update scenarios - update package documentation to reflect async execution support
2 tasks
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds async execution to POST /v1/update via ChangesAsync Update Handler Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 1 medium |
🟢 Metrics 19 duplication
Metric Results Duplication 19
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #1609 +/- ##
==========================================
+ Coverage 75.04% 75.24% +0.20%
==========================================
Files 58 58
Lines 9795 9830 +35
==========================================
+ Hits 7351 7397 +46
+ Misses 2183 2174 -9
+ Partials 261 259 -2
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/api/update/update.go (1)
156-160:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDrop blank
imageentries while parsing the query string.
?image=and values with trailing commas currently produce[]string{""}, which sends the request down the targeted-update path and changes lock behavior from immediate429to blocking. Filtering empty segments here avoids that edge-case semantic shift.Suggested fix
if found { for _, image := range imageQueries { - images = append(images, strings.Split(image, ",")...) + for _, part := range strings.Split(image, ",") { + part = strings.TrimSpace(part) + if part == "" { + continue + } + images = append(images, part) + } }🤖 Prompt for 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. In `@pkg/api/update/update.go` around lines 156 - 160, When parsing r.URL.Query()["image"] into imageQueries and appending to images, filter out empty segments so that inputs like "?image=" or trailing commas don't produce "" entries; update the loop that iterates over imageQueries (and the strings.Split(image, ",") result) to skip any empty strings before appending to images (e.g., filter split segments or use a split-and-filter helper) so images contains only non-empty values and avoids triggering the targeted-update path.
🤖 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 `@docs/advanced-features/http-api/index.md`:
- Around line 138-145: Add a 503 entry to the "Status Codes for Async Requests"
table (under the "Status Codes for Async Requests" heading) indicating "503
Service Unavailable — Client cancelled while waiting on update lock" and also
add the same 503 row to the main status-code table above so both tables mirror
each other; locate and update the tables in
docs/advanced-features/http-api/index.md where the current 202/401/429/500 rows
are defined.
- Around line 132-133: The curl example in
docs/advanced-features/http-api/index.md uses an unquoted URL with an ampersand
so most shells will treat & as a background operator and drop the async=true
parameter; update the POST example (the curl command shown) to quote the full
URL (e.g., "http://localhost:8080/v1/update?image=foo/bar:latest&async=true") or
escape the ampersand so the async=true query parameter is preserved when run in
a shell.
- Around line 12-15: The table in docs/advanced-features/http-api/index.md uses
intra-page anchors with underscores (e.g. [`image`](`#image_parameter_usage`),
[`async`](`#asynchronous_updates`)) which don't match the actual heading anchors;
update those fragment links to use hyphenated anchors that match the headings
(for example change `#image_parameter_usage` -> `#image-parameter-usage` and
`#asynchronous_updates` -> `#asynchronous-updates`) and verify the
[Update](`#http_api_update`) link matches the "http api update" heading format
(e.g., use `#http-api-update` or the exact generated slug for the "Update"
heading) so all table links resolve correctly.
---
Outside diff comments:
In `@pkg/api/update/update.go`:
- Around line 156-160: When parsing r.URL.Query()["image"] into imageQueries and
appending to images, filter out empty segments so that inputs like "?image=" or
trailing commas don't produce "" entries; update the loop that iterates over
imageQueries (and the strings.Split(image, ",") result) to skip any empty
strings before appending to images (e.g., filter split segments or use a
split-and-filter helper) so images contains only non-empty values and avoids
triggering the targeted-update path.
🪄 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: d162e685-ab09-402d-b734-66b4c3158614
📒 Files selected for processing (4)
docs/advanced-features/http-api/index.mdpkg/api/update/doc.gopkg/api/update/update.gopkg/api/update/update_test.go
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/advanced-features/http-api/index.md (1)
12-15:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the broken intra-page anchor links in the endpoints table.
The anchor fragments in line 14 use underscores (
#http_api_update,#image_parameter_usage,#asynchronous_updates), but Markdown generates hyphenated heading IDs. These links won't resolve to the actual headings.🔗 Proposed fix
-| [Update](`#http_api_update`) | `POST` | `/v1/update` | [`image`](`#image_parameter_usage`), [`async`](`#asynchronous_updates`) | Triggers container updates and returns JSON results of the operation | +| [Update](`#http-api-update`) | `POST` | `/v1/update` | [`image`](`#image-parameter-usage`), [`async`](`#asynchronous-updates`) | Triggers container updates and returns JSON results of the operation |🤖 Prompt for 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. In `@docs/advanced-features/http-api/index.md` around lines 12 - 15, The table's intra-page anchor links use underscores and thus don't match Markdown's hyphenated heading IDs; update the anchor fragments in the endpoints table so [Update](`#http_api_update`) → [Update](`#http-api-update`), [`image`](`#image_parameter_usage`) → [`image`](`#image-parameter-usage`), and [`async`](`#asynchronous_updates`) → [`async`](`#asynchronous-updates`) so they resolve to the actual headings (update the exact strings shown in the diff).
🧹 Nitpick comments (1)
docs/advanced-features/http-api/index.md (1)
121-121: 💤 Low valueConsider quoting the URL for consistency.
While this URL is safe without quotes (no
&character), quoting it would match the style used in line 134 and provide a consistent pattern across all curl examples.📝 Suggested change
-curl -X POST -H "Authorization: Bearer mytoken" localhost:8080/v1/update?async=true +curl -X POST -H "Authorization: Bearer mytoken" "localhost:8080/v1/update?async=true"🤖 Prompt for 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. In `@docs/advanced-features/http-api/index.md` at line 121, Update the curl example that currently reads: curl -X POST -H "Authorization: Bearer mytoken" localhost:8080/v1/update?async=true so the request URL is quoted (e.g., "... \"localhost:8080/v1/update?async=true\"") to match the quoting style used in other examples; locate the curl example text in the markdown and wrap the whole URL argument in quotes.
🤖 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.
Duplicate comments:
In `@docs/advanced-features/http-api/index.md`:
- Around line 12-15: The table's intra-page anchor links use underscores and
thus don't match Markdown's hyphenated heading IDs; update the anchor fragments
in the endpoints table so [Update](`#http_api_update`) →
[Update](`#http-api-update`), [`image`](`#image_parameter_usage`) →
[`image`](`#image-parameter-usage`), and [`async`](`#asynchronous_updates`) →
[`async`](`#asynchronous-updates`) so they resolve to the actual headings (update
the exact strings shown in the diff).
---
Nitpick comments:
In `@docs/advanced-features/http-api/index.md`:
- Line 121: Update the curl example that currently reads: curl -X POST -H
"Authorization: Bearer mytoken" localhost:8080/v1/update?async=true so the
request URL is quoted (e.g., "... \"localhost:8080/v1/update?async=true\"") to
match the quoting style used in other examples; locate the curl example text in
the markdown and wrap the whole URL argument in quotes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 300ebc92-9c29-4662-b187-8b513b1b41c9
📒 Files selected for processing (1)
docs/advanced-features/http-api/index.md
3 tasks
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
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.
This PR adds an
?async=truequery parameter to the/v1/updateendpoint, allowing clients to trigger updates without waiting for completion.Problem
As noted by issue #1575, Watchtower's HTTP API currently maintains a long-lived connection when a POST request is sent to trigger updates. This is problematic for execution environments, such as GitHub Actions, where connection time is billed and/or it's not necessary to wait for the update cycle to finish and receive a final response.
Solution
Issue #1575 proposed using the HEAD method for this; however, that would have been noncompliant with RFC 9110 Section 9.3.2 (HEAD) and Section 9.2.1 (Safe Methods).
Instead of using a noncompliant HEAD endpoint, an
?async=truequery parameter was added to the existing POST endpoint. When present, the handler spawns the update in a background goroutine and returns 202 Accepted immediately.POST /v1/update— synchronous, returns 200 with JSON results (unchanged)POST /v1/update?async=true— asynchronous, returns 202 Accepted, runs update in backgroundChanges
pkg/api/update/update.go— Added?async=truebranching inHandle; extracted handler logic into focused helper methodsinternal/api/api.go— Removed HEAD method registrationpkg/api/update/update_test.go— Added async parameter tests; added tests for body size limits, context cancellation, and panic recoverydocs/advanced-features/http-api/index.md— Documented?async=trueparameter and asynchronous behaviorpkg/api/update/doc.go— Updated package descriptionSummary by CodeRabbit
New Features
Documentation
Tests