feat: add X-Docker-Agent-Version and X-Docker-Desktop-Version headers to built-in tools #2795
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| package desktop | ||
|
|
||
| import ( | ||
| "context" | ||
| "time" | ||
|
|
||
| "github.com/kofalt/go-memoize" | ||
| ) | ||
|
|
||
| // versionMemoizer caches the Docker Desktop version lookup with a TTL so: | ||
| // - if docker-agent starts before Desktop is ready, version detection | ||
| // recovers automatically once Desktop comes up; | ||
| // - if Desktop is upgraded mid-session, the new version is picked up | ||
| // within at most one TTL. | ||
| var versionMemoizer = memoize.NewMemoizer(5*time.Minute, 10*time.Minute) | ||
|
|
||
| // GetVersion returns the running Docker Desktop version (e.g. "4.74.0") or | ||
| // an empty string if Docker Desktop is not running or the call fails. | ||
| // | ||
| // The lookup is bounded by a short internal timeout so a stale or missing | ||
| // backend socket cannot stall callers on hot paths (it is queried on every | ||
| // outbound built-in tool HTTP request). ctx is used for the underlying | ||
| // HTTP call on a cache miss; on a cache hit the cached value is returned | ||
| // without consulting ctx. | ||
| func GetVersion(ctx context.Context) string { | ||
| v, _, _ := memoize.Call(versionMemoizer, "desktopVersion", func() (string, error) { | ||
| ctx, cancel := context.WithTimeout(ctx, 2*time.Second) | ||
| defer cancel() | ||
|
|
||
| var info struct { | ||
| CurrentVersion string `json:"currentVersion"` | ||
| } | ||
| _ = ClientBackend.Get(ctx, "/update", &info) | ||
|
Contributor
There was a problem hiding this comment. Non-blocking: the If we wanted to differentiate, returning a non-nil error from the closure on |
||
| return info.CurrentVersion, nil | ||
|
There was a problem hiding this comment. [MEDIUM] Empty Desktop version cached for full 5-minute TTL when Desktop is unavailable at first call
This contradicts the PR description's claim that the lookup "recovers automatically if Desktop starts after docker-agent": if Fix: return a non-nil error when the lookup fails so if err := ClientBackend.Get(ctx, "/update", &info); err != nil {
return "", err // not cached; next caller retries
}
return info.CurrentVersion, nilThis way a transient failure (Desktop not yet up) never poisons the cache. |
||
| }) | ||
| return v | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,35 @@ | ||
| // Package useragent centralizes the HTTP identity headers that built-in | ||
| // tools (api, fetch, openapi, ...) attach to every outbound request. | ||
| package useragent | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "net/http" | ||
| "runtime" | ||
|
|
||
| "github.com/docker/docker-agent/pkg/desktop" | ||
| "github.com/docker/docker-agent/pkg/version" | ||
| ) | ||
|
|
||
| // Header is the User-Agent value sent by built-in HTTP tools. It also | ||
| // doubles as the agent name fed to robotstxt.RobotsData.TestAgent so | ||
| // site operators see the same identity in both places. | ||
| var Header = fmt.Sprintf("Cagent/%s (%s; %s)", version.Version, runtime.GOOS, runtime.GOARCH) | ||
|
|
||
| // HTTP header names emitted by [SetIdentity] in addition to User-Agent. | ||
| const ( | ||
| HeaderAgentVersion = "X-Docker-Agent-Version" | ||
| HeaderDesktopVersion = "X-Docker-Desktop-Version" | ||
| ) | ||
|
|
||
| // SetIdentity stamps the request with User-Agent, X-Docker-Agent-Version, | ||
| // and (when Docker Desktop is reachable) X-Docker-Desktop-Version. Callers | ||
| // that want operator-supplied overrides should apply those headers AFTER | ||
| // calling SetIdentity. | ||
| func SetIdentity(req *http.Request) { | ||
| req.Header.Set("User-Agent", Header) | ||
| req.Header.Set(HeaderAgentVersion, version.Version) | ||
| if v := desktop.GetVersion(req.Context()); v != "" { | ||
| req.Header.Set(HeaderDesktopVersion, v) | ||
|
Contributor
There was a problem hiding this comment. Non-blocking (privacy/scope):
Neither blocks this PR. |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| package useragent | ||
|
|
||
| import ( | ||
| "net/http" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/docker/docker-agent/pkg/version" | ||
| ) | ||
|
|
||
| func TestSetIdentity(t *testing.T) { | ||
| req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, "http://example.com/", http.NoBody) | ||
| require.NoError(t, err) | ||
|
|
||
| SetIdentity(req) | ||
|
|
||
| assert.Equal(t, Header, req.Header.Get("User-Agent")) | ||
| assert.Equal(t, version.Version, req.Header.Get(HeaderAgentVersion)) | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Non-blocking: two behaviours that the PR description calls out as load-bearing don't have a unit test here:
Both are quick to add and would lock in the invariants the PR description leans on. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: what drove the 5 min TTL (and the 10 min cleanup interval)? The PR description mentions "recovers automatically … upgraded mid-session" which argues for some refresh, but 5 minutes is a fairly arbitrary middle ground — Desktop upgrades are rare (once a week), the lookup is cheap-ish (one local socket call with a 2 s ceiling), and the value almost never changes within a session.
A short rationale comment near the
NewMemoizercall (e.g. "5 min keeps the steady-state cost ≤ 1 socket call per 5 min per process while still picking up Desktop upgrades within a few minutes") would save the next reader the same question. Happy to leave the value as-is.