Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix!: use client ip when creating connection logs for workspace proxied app accesses#19788

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ethanndickson merged 1 commit intomainfromethan/proxied-app-connection-log-fix
Sep 15, 2025

Conversation

ethanndickson
Copy link
Member

@ethanndicksonethanndickson commentedSep 12, 2025
edited
Loading

Breaking API Change for changelog:

The presence of theip field oncodersdk.ConnectionLog cannot be guaranteed, and so the field has been made optional. It may be omitted on API responses.

When running a scaletest, I noticed logs of the form:

2025-09-12 06:34:10.924 [erro]  coderd.workspaceapps: upsert connection log failed  trace=0xa17580  span=0xa17620  workspace_id=81b937d7-5777-4df5-b5cb-80241f30326f  agent_id=78b2ff6d-b4a6-4a4e-88a7-283e05455a88  app_id=00000000-0000-0000-0000-000000000000  user_id=00000000-0000-0000-0000-000000000000  user_agent=""  app_slug_or_port=terminal  status_code=404  request_id=67f03cf8-9523-444a-97bc-90de080a54c8 ...    error= 1 error occurred:           * pq: null value in column "ip" of relation "connection_logs" violates not-null constraint

to ensure logs are never omitted from the connection log due to a missing IP again (i.e. I'm not sure if we can always rely on a valid, parseable, IP from(http.Request).RemoteAddr), I've removed theNOT NULL constraint onip onconnection_logs, and madeip on the API response optional.

The specific cause for these null IPs was the/workspaceproxies/me/issue-signed-app-token [post] endpoint constructing it's ownhttp.Request without aRemoteAddr set, and then passing that to the token issuer.

To solve this, we'll have workspace proxies send the real IP of the client when calling/workspaceproxies/me/issue-signed-app-token [post] via the headerCoder-Workspace-Proxy-Real-IP.

@ethanndicksonGraphite App
Copy link
MemberAuthor

@ethanndicksonethanndicksonforce-pushed theethan/proxied-app-connection-log-fix branch fromdd9ae88 to9ea7924CompareSeptember 12, 2025 07:21
@ethanndicksonethanndickson marked this pull request as ready for reviewSeptember 12, 2025 07:29
@github-actionsgithub-actionsbot added the release/breakingThis label is applied to PRs to detect breaking changes as part of the release process labelSep 12, 2025
@ethanndicksonethanndicksonforce-pushed theethan/proxied-app-connection-log-fix branch from9ea7924 to3b8b864CompareSeptember 15, 2025 02:06
@ethanndicksonethanndickson merged commit6a9b896 intomainSep 15, 2025
35 checks passed
@ethanndicksonethanndickson deleted the ethan/proxied-app-connection-log-fix branchSeptember 15, 2025 02:30
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsSep 15, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@deansheatherdeansheatherdeansheather approved these changes

Assignees

@ethanndicksonethanndickson

Labels
release/breakingThis label is applied to PRs to detect breaking changes as part of the release process
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@ethanndickson@deansheather

[8]ページ先頭

©2009-2025 Movatter.jp