Conversation
imeyer
requested review from
Flo4604,
chronark and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughUpdated multiple go.mod files to pin the toolchain (go 1.25 / toolchain go1.25.1), bumped several telemetry/observability/spiffe/tls dependency versions, removed some local replace directives in the root go.mod, adjusted .gitignore entries for builderd/metald, and removed header comments from one CLI main.go. Changes
Sequence Diagram(s)No sequence diagram included — changes are manifest/configuration and minor comment/.gitignore edits with no control-flow or runtime behavior modifications. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewersPre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate 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. Comment |
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
go/go.mod (1)
223-224: Typo in module path: go.yaml.in → gopkg.in (duplicates also exist below).go.yaml.in/yaml is invalid and duplicates the correct gopkg.in entries (Lines 237–238). Remove the erroneous ones.
Apply:
-go.yaml.in/yaml/v2 v2.4.2 // indirect -go.yaml.in/yaml/v3 v3.0.4 // indirect
🧹 Nitpick comments (3)
go/deploy/pkg/spiffe/go.mod (1)
7-19: Minor version drift with grpc/protobuf vs telemetry/root.This module pins grpc v1.75.0 and protobuf v1.36.7, while telemetry uses grpc v1.75.1 and protobuf v1.36.9; the build will pick the highest, but aligning reduces churn.
Suggested tweak:
- google.golang.org/grpc v1.75.0 // indirect - google.golang.org/protobuf v1.36.7 // indirect + google.golang.org/grpc v1.75.1 // indirect + google.golang.org/protobuf v1.36.9 // indirectgo/go.mod (1)
57-62: Unify core versions with telemetry to avoid mixed patch levels.Align to latest already used by submodules.
Apply:
-golang.org/x/net v0.43.0 +golang.org/x/net v0.44.0 @@ -google.golang.org/protobuf v1.36.8 +google.golang.org/protobuf v1.36.9 @@ - google.golang.org/grpc v1.75.0 // indirect + google.golang.org/grpc v1.75.1 // indirectAlso applies to: 233-233
go/deploy/pkg/tls/go.mod (1)
11-22: Keep grpc/protobuf versions in sync with telemetry.Optional alignment; not functionally required due to MVS.
Apply:
- google.golang.org/grpc v1.75.0 // indirect - google.golang.org/protobuf v1.36.7 // indirect + google.golang.org/grpc v1.75.1 // indirect + google.golang.org/protobuf v1.36.9 // indirect
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
go/deploy/pkg/observability/interceptors/go.sumis excluded by!**/*.sumgo/deploy/pkg/spiffe/go.sumis excluded by!**/*.sumgo/deploy/pkg/telemetry/go.sumis excluded by!**/*.sumgo/deploy/pkg/tls/go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
go/apps/builderd/.gitignore(1 hunks)go/apps/builderd/cmd/builderd-cli/main.go(0 hunks)go/apps/metald/.gitignore(1 hunks)go/deploy/pkg/health/go.mod(1 hunks)go/deploy/pkg/observability/interceptors/go.mod(1 hunks)go/deploy/pkg/spiffe/go.mod(1 hunks)go/deploy/pkg/telemetry/go.mod(1 hunks)go/deploy/pkg/tls/go.mod(1 hunks)go/deploy/pkg/tracing/go.mod(1 hunks)go/go.mod(1 hunks)
💤 Files with no reviewable changes (1)
- go/apps/builderd/cmd/builderd-cli/main.go
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/job_detect_changes.yaml:111-114
Timestamp: 2025-08-08T16:10:00.224Z
Learning: Repo: unkeyed/unkey — When imeyer writes “make me an issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/actions/setup-node/action.yaml:0-0
Timestamp: 2025-08-08T15:10:46.436Z
Learning: Repo: unkeyed/unkey — Preference: If imeyer comments “issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/runbook-freshness-check.yaml:157-173
Timestamp: 2025-08-08T14:59:52.283Z
Learning: Repo unkeyed/unkey: When a CI/workflow fix is deferred, imeyer prefers a thorough GitHub issue be opened with sections (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References) and assigned to imeyer, including backlinks to the originating PR and comment.
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/actions/setup-node/action.yaml:0-0
Timestamp: 2025-08-08T15:10:46.436Z
Learning: For repo unkeyed/unkey and PR review workflows: When imeyer comments "issue" on a thread, automatically create a thorough GitHub issue (sections: Summary, Impact, Where, Repro/Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and the specific comment, and assign the issue to imeyer.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test Packages / Test
- GitHub Check: Build / Build
- GitHub Check: Test API / API Test Local
🔇 Additional comments (7)
go/apps/builderd/.gitignore (1)
89-91: Ignore additions for local binaries: good hardening.Prevents accidental commits of builderd artifacts. Mirrors metald pattern.
go/deploy/pkg/tracing/go.mod (1)
3-5: Go/toolchain pin: consistent and correct.go/go.mod (1)
253-256: Replace removal for apps: verify local dev/CI still resolve modules.rg output showed "No files were searched" and only go.yaml entries were found; cannot confirm whether any imports still reference github.com/unkeyed/unkey/go/apps or whether replace directives remain. Run and paste results:
rg -nP 'github\.com/unkeyed/unkey/go/apps/' -g '!**/vendor/**' -S -C2 rg -nP '^replace\s+github\.com/unkeyed/unkey/go/apps/' go/**/go.mod -S -n sed -n '240,270p' go/go.mod # inspect the replace block around lines ~253-256 rg -n 'go\.yaml\.in/yaml' go -S # if rg still reports "No files were searched", run: rg --debug 'github\.com/unkeyed/unkey/go/apps/' -g '!**/vendor/**' -S -C2go/deploy/pkg/telemetry/go.mod (1)
7-16: Telemetry stack upgrades look consistent — run module tidies and verify grpc/protobuf versions repo-wide.go/deploy/pkg/telemetry/go.mod contains google.golang.org/grpc v1.75.1 and google.golang.org/protobuf v1.36.9 (indirect). Ripgrep-based repo search skipped files, so repo-wide winners weren’t confirmed. Run:
find . -type f -name 'go.mod' -print0 | xargs -0 grep -nE 'google.golang.org/(grpc|protobuf)\b' || true
cd go/deploy/pkg/telemetry && go mod tidy && go list -m all | grep 'google.golang.org' || truego/deploy/pkg/health/go.mod (1)
3-5: CI will install Go 1.25.1 — confirmedWorkflows call ./.github/actions/setup-go → it uses actions/setup-go@v5 with go-version-file: ./go/go.mod; go/go.mod contains "toolchain go1.25.1", so setup-go will select/install Go 1.25.1 on CI runners.
go/deploy/pkg/observability/interceptors/go.mod (2)
9-9: Pseudo-version for tracing is fine with a replace; ensure a real tag exists before publishing.If you plan to publish this module independently, cut a tag in
go/deploy/pkg/tracingand switch the requirement to that tag once the replace is moved to go.work.
22-22: Move local replace in go.mod into go.work (don’t commit local paths)go/deploy/pkg/observability/interceptors/go.mod:22 currently contains:
replace github.com/unkeyed/unkey/go/deploy/pkg/tracing => ../../tracingRemove that line and add to the repository go.work instead (at repo root):
replace github.com/unkeyed/unkey/go/deploy/pkg/tracing => ./go/deploy/pkg/tracingApply (example):
- replace github.com/unkeyed/unkey/go/deploy/pkg/tracing => ../../tracing + // replaced in go.work
imeyer
force-pushed
the
push-qyzuykyqlwzz
branch
from
c151f18 to
0b76e73
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
go/apps/metald/cmd/metald-init/go.mod (2)
3-5: Toolchain pin looks good; ensure builders/CI images have Go 1.25.1Pinning toolchain is fine; verify all Docker images/builders and CI use >=1.21 (supports toolchain directive) and have 1.25.1 available to avoid fetch-at-build surprises.
Also confirm go.work (if any) and root toolchain/go directives are consistent across modules.
7-9: Static binary note: confirm build flagsGiven the static binary requirement, ensure CGO is disabled or musl toolchain is used in the build for this module (e.g., CGO_ENABLED=0 and -ldflags '-s -w').
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
go/deploy/pkg/observability/interceptors/go.sumis excluded by!**/*.sumgo/deploy/pkg/spiffe/go.sumis excluded by!**/*.sumgo/deploy/pkg/telemetry/go.sumis excluded by!**/*.sumgo/deploy/pkg/tls/go.sumis excluded by!**/*.sum
📒 Files selected for processing (12)
go/apps/builderd/.gitignore(1 hunks)go/apps/builderd/cmd/builderd-cli/main.go(0 hunks)go/apps/metald/.gitignore(1 hunks)go/apps/metald/cmd/metald-init/go.mod(1 hunks)go/demo_api/go.mod(1 hunks)go/deploy/pkg/health/go.mod(1 hunks)go/deploy/pkg/observability/interceptors/go.mod(1 hunks)go/deploy/pkg/spiffe/go.mod(1 hunks)go/deploy/pkg/telemetry/go.mod(1 hunks)go/deploy/pkg/tls/go.mod(1 hunks)go/deploy/pkg/tracing/go.mod(1 hunks)go/go.mod(1 hunks)
💤 Files with no reviewable changes (1)
- go/apps/builderd/cmd/builderd-cli/main.go
✅ Files skipped from review due to trivial changes (1)
- go/demo_api/go.mod
🚧 Files skipped from review as they are similar to previous changes (9)
- go/deploy/pkg/health/go.mod
- go/apps/builderd/.gitignore
- go/deploy/pkg/tracing/go.mod
- go/deploy/pkg/observability/interceptors/go.mod
- go/go.mod
- go/deploy/pkg/telemetry/go.mod
- go/apps/metald/.gitignore
- go/deploy/pkg/spiffe/go.mod
- go/deploy/pkg/tls/go.mod
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/job_detect_changes.yaml:111-114
Timestamp: 2025-08-08T16:10:00.224Z
Learning: Repo: unkeyed/unkey — When imeyer writes “make me an issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/actions/setup-node/action.yaml:0-0
Timestamp: 2025-08-08T15:10:46.436Z
Learning: Repo: unkeyed/unkey — Preference: If imeyer comments “issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
📚 Learning: 2025-07-15T14:59:30.212Z
Learnt from: chronark
PR: unkeyed/unkey#3560
File: go/deploy/metald/internal/database/repository.go:0-0
Timestamp: 2025-07-15T14:59:30.212Z
Learning: go/deploy/metald cannot currently import helpers from go/pkg/db because it is not yet part of the main Go module; avoid suggesting such imports until the modules are unified.
Applied to files:
go/apps/metald/cmd/metald-init/go.mod
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Go API Local / Test
- GitHub Check: Build / Build
- GitHub Check: Test Packages / Test
imeyer
force-pushed
the
push-qyzuykyqlwzz
branch
from
0b76e73 to
0f5f841
Compare
|
you know you can just close comments and not reply, right? 😄 |
You're absolutely right! |
|
lmao |
|
someone needs to move the fucking "Close with comment" button... |
|
|
Graphite Automations"Post a GIF when PR approved" took an action on this PR • (09/15/25)1 gif was posted to this PR based on Andreas Thomas's automation. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
go/deploy/pkg/spiffe/go.mod (1)
7-18: Optional: align grpc/protobuf/genproto versions in go/deploy/pkg/spiffe/go.mod with telemetryRepo scan shows go/deploy/pkg/spiffe/go.mod currently uses genproto v0.0.0-20250707201910-8d1bb00bc6a7, grpc v1.75.0, protobuf v1.36.7 while go/deploy/pkg/telemetry/go.mod uses genproto v0.0.0-20250908214217-97024824d090, grpc v1.75.1, protobuf v1.36.9 — aligning will reduce module-graph churn.
- require github.com/spiffe/go-spiffe/v2 v2.6.0 + require github.com/spiffe/go-spiffe/v2 v2.6.0 @@ - google.golang.org/genproto/googleapis/rpc v0.0.0-20250707201910-8d1bb00bc6a7 // indirect - google.golang.org/grpc v1.75.0 // indirect - google.golang.org/protobuf v1.36.7 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20250908214217-97024824d090 // indirect + google.golang.org/grpc v1.75.1 // indirect + google.golang.org/protobuf v1.36.9 // indirect
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
go/deploy/pkg/observability/interceptors/go.sumis excluded by!**/*.sumgo/deploy/pkg/spiffe/go.sumis excluded by!**/*.sumgo/deploy/pkg/telemetry/go.sumis excluded by!**/*.sumgo/deploy/pkg/tls/go.sumis excluded by!**/*.sum
📒 Files selected for processing (12)
go/apps/builderd/.gitignore(1 hunks)go/apps/builderd/cmd/builderd-cli/main.go(0 hunks)go/apps/metald/.gitignore(1 hunks)go/apps/metald/cmd/metald-init/go.mod(1 hunks)go/demo_api/go.mod(1 hunks)go/deploy/pkg/health/go.mod(1 hunks)go/deploy/pkg/observability/interceptors/go.mod(1 hunks)go/deploy/pkg/spiffe/go.mod(1 hunks)go/deploy/pkg/telemetry/go.mod(1 hunks)go/deploy/pkg/tls/go.mod(1 hunks)go/deploy/pkg/tracing/go.mod(1 hunks)go/go.mod(1 hunks)
💤 Files with no reviewable changes (1)
- go/apps/builderd/cmd/builderd-cli/main.go
🚧 Files skipped from review as they are similar to previous changes (9)
- go/demo_api/go.mod
- go/apps/builderd/.gitignore
- go/deploy/pkg/health/go.mod
- go/deploy/pkg/tracing/go.mod
- go/apps/metald/cmd/metald-init/go.mod
- go/go.mod
- go/deploy/pkg/tls/go.mod
- go/apps/metald/.gitignore
- go/deploy/pkg/observability/interceptors/go.mod
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/job_detect_changes.yaml:111-114
Timestamp: 2025-08-08T16:10:00.224Z
Learning: Repo: unkeyed/unkey — When imeyer writes “make me an issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/actions/setup-node/action.yaml:0-0
Timestamp: 2025-08-08T15:10:46.436Z
Learning: Repo: unkeyed/unkey — Preference: If imeyer comments “issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
📚 Learning: 2025-08-08T14:59:52.283Z
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/runbook-freshness-check.yaml:157-173
Timestamp: 2025-08-08T14:59:52.283Z
Learning: Repo unkeyed/unkey: When a CI/workflow fix is deferred, imeyer prefers a thorough GitHub issue be opened with sections (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References) and assigned to imeyer, including backlinks to the originating PR and comment.
Applied to files:
go/deploy/pkg/telemetry/go.mod
📚 Learning: 2025-08-08T16:10:00.224Z
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/workflows/job_detect_changes.yaml:111-114
Timestamp: 2025-08-08T16:10:00.224Z
Learning: Repo: unkeyed/unkey — When imeyer writes “make me an issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Applied to files:
go/deploy/pkg/telemetry/go.mod
📚 Learning: 2025-08-08T15:10:46.436Z
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/actions/setup-node/action.yaml:0-0
Timestamp: 2025-08-08T15:10:46.436Z
Learning: Repo: unkeyed/unkey — Preference: If imeyer comments “issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Applied to files:
go/deploy/pkg/telemetry/go.mod
📚 Learning: 2025-08-08T15:10:46.436Z
Learnt from: imeyer
PR: unkeyed/unkey#3755
File: .github/actions/setup-node/action.yaml:0-0
Timestamp: 2025-08-08T15:10:46.436Z
Learning: For repo unkeyed/unkey and PR review workflows: When imeyer comments "issue" on a thread, automatically create a thorough GitHub issue (sections: Summary, Impact, Where, Repro/Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and the specific comment, and assign the issue to imeyer.
Applied to files:
go/deploy/pkg/telemetry/go.mod
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Test Packages / Test
- GitHub Check: Test Go API Local / Test
🔇 Additional comments (4)
go/deploy/pkg/telemetry/go.mod (3)
3-6: Toolchain pin LGTM.Consistent with the repo move to Go 1.25 + toolchain go1.25.1.
8-16: OTel/Prometheus stack is coherently aligned.v1.38.0 across otel core/metric/trace/sdk and matching exporter versions looks consistent. No action.
20-44: Sanity-check newly introduced/updated indirects; verify yaml import path resolution.
Observed (go/deploy/pkg/telemetry/go.mod): genproto googleapis/api & rpc → v0.0.0-20250908214217-97024824d090, google.golang.org/grpc → v1.75.1, google.golang.org/protobuf → v1.36.9. Other modules contain mixed older versions (examples: agent protobuf v1.36.5 & grpc v1.70.0; chproxy genproto 2025-04-14 & grpc v1.71.1; spiffe genproto 2025-07-07 & grpc v1.75.0; repo root genproto 2025-08-26 & grpc v1.75.0).
go.yaml.in/yaml/v2@v2.4.3: sandbox attempt produced "failed to start telemetry sidecar: os.Executable: readlink /proc/self/exe: no such file or directory" — unresolved in this environment; run the included go mod download command locally or in CI to confirm resolution.
govulncheck: not runnable in sandbox ("govulncheck: command not found"); run locally/CI to complete the vulnerability check.
Optional: if alignment is desired, standardize on a single genproto/grpc/protobuf set (mirror spiffe or pick the newest) and run go mod tidy across modules.
go/deploy/pkg/spiffe/go.mod (1)
3-5: Toolchain pin OK — confirm CI uses Go 1.25go/deploy/pkg/spiffe/go.mod pins go 1.25 with toolchain go1.25.1. CI workflows use actions/setup-go with go-version-file: ./go/go.mod (see .github/actions/setup-go/action.yaml and .github/workflows/* that use ./.github/actions/setup-go). Ensure the top-level go/go.mod pins 1.25.x or update workflows to read this module's go.mod so CI and local/toolchain match.
3 participants
What does this PR do?
Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit