chore: move vmprovisioner protos to shared proto#3494
Merged
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Contributor
|
Caution Review failedThe pull request is closed. 📝 Walkthrough## Walkthrough
This change migrates the protobuf package for the VM provisioner service from a deployment-specific path to a centralized location under `go/gen/proto/metal/vmprovisioner/v1`. It removes local protobuf generation and Buf configuration, updates all affected Go import paths, adjusts module dependencies, and updates the protobuf package declaration accordingly.
## Changes
| Files/Paths | Change Summary |
|-------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------|
| go/deploy/metald/Makefile, go/deploy/metald/buf.gen.yaml, go/deploy/metald/buf.yaml | Removed protobuf generation, Buf config, and related Makefile targets. |
| go/deploy/metald/client/* , go/deploy/metald/cmd/metald/main.go, go/deploy/metald/internal/* | Updated import paths from `metald/gen/vmprovisioner/v1` to `gen/proto/metal/vmprovisioner/v1`. |
| go/deploy/metald/go.mod | Updated internal and indirect dependencies, added root module replace, and pinned versions to specific commits. |
| go/go.mod | Bumped Go version, added/updated internal and external dependencies, and refreshed indirect dependencies. |
| go/proto/metal/vmprovisioner/v1/vmprovisioner.proto | Changed protobuf package and `go_package` option to `metal.vmprovisioner.v1` and new import path. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Client
participant VMProvisionerAPI (Go)
participant ProtoTypes (go/gen/proto/metal/vmprovisioner/v1)
Client->>VMProvisionerAPI (Go): Import ProtoTypes
VMProvisionerAPI (Go)->>ProtoTypes: Use shared protobuf types and servicesSuggested reviewers
|
Collaborator
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Jul 9, 2025
chronark
changed the title
This was referenced Jul 9, 2025
Merged
Contributor
|
Thank you for following the naming conventions for pull request titles! 🙏 |
…ore_move_vmprovisioner_protos_to_shared_protochore
![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: 6
🔭 Outside diff range comments (1)
go/deploy/metald/internal/backend/cloudhypervisor/client.go (1)
62-66: Nil-check on embedded proto sub-messages
config.GetCpu().GetVcpuCount()andconfig.GetMemory().GetSizeBytes()assumeGetCpu()/GetMemory()always return non-nil; earlier code nil-checks elsewhere. Consider mirroring that check here to avoid a panic if callers forget to populate those fields.- slog.Int("vcpus", int(config.GetCpu().GetVcpuCount())), - slog.Int64("memory_bytes", config.GetMemory().GetSizeBytes()), + slog.Int("vcpus", func() int { + if cpu := config.GetCpu(); cpu != nil { + return int(cpu.GetVcpuCount()) + } + return 0 + }()), + slog.Int64("memory_bytes", func() int64 { + if mem := config.GetMemory(); mem != nil { + return mem.GetSizeBytes() + } + return 0 + }()),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (12)
go/deploy/metald/go.sumis excluded by!**/*.sumgo/gen/deploy/assetmanagerd/proto/asset/v1/asset.pb.gois excluded by!**/*.pb.go,!**/gen/**go/gen/deploy/assetmanagerd/proto/asset/v1/assetv1connect/asset.connect.gois excluded by!**/gen/**go/gen/deploy/billaged/proto/billing/v1/billing.pb.gois excluded by!**/*.pb.go,!**/gen/**go/gen/deploy/billaged/proto/billing/v1/billingv1connect/billing.connect.gois excluded by!**/gen/**go/gen/deploy/builderd/proto/builder/v1/builder.pb.gois excluded by!**/*.pb.go,!**/gen/**go/gen/deploy/builderd/proto/builder/v1/builderv1connect/builder.connect.gois excluded by!**/gen/**go/gen/deploy/metald/proto/vmprovisioner/v1/vm.pb.gois excluded by!**/*.pb.go,!**/gen/**go/gen/deploy/metald/proto/vmprovisioner/v1/vmprovisionerv1connect/vm.connect.gois excluded by!**/gen/**go/gen/proto/metal/vmprovisioner/v1/vmprovisioner.pb.gois excluded by!**/*.pb.go,!**/gen/**go/gen/proto/metal/vmprovisioner/v1/vmprovisionerv1connect/vmprovisioner.connect.gois excluded by!**/gen/**go/go.sumis excluded by!**/*.sum
📒 Files selected for processing (23)
go/deploy/metald/Makefile(4 hunks)go/deploy/metald/buf.gen.yaml(0 hunks)go/deploy/metald/buf.yaml(0 hunks)go/deploy/metald/client/client.go(1 hunks)go/deploy/metald/client/cmd/metald-cli/main.go(1 hunks)go/deploy/metald/client/config.go(1 hunks)go/deploy/metald/client/example_test.go(1 hunks)go/deploy/metald/client/types.go(1 hunks)go/deploy/metald/client/vmconfig.go(1 hunks)go/deploy/metald/cmd/metald/main.go(1 hunks)go/deploy/metald/go.mod(4 hunks)go/deploy/metald/internal/backend/cloudhypervisor/client.go(1 hunks)go/deploy/metald/internal/backend/cloudhypervisor/types.go(1 hunks)go/deploy/metald/internal/backend/firecracker/automatic_build_test.go(1 hunks)go/deploy/metald/internal/backend/firecracker/sdk_client_v4.go(1 hunks)go/deploy/metald/internal/backend/firecracker/sdk_client_v4_test.go(1 hunks)go/deploy/metald/internal/backend/types/backend.go(1 hunks)go/deploy/metald/internal/database/repository.go(1 hunks)go/deploy/metald/internal/reconciler/vm_reconciler.go(1 hunks)go/deploy/metald/internal/service/vm.go(1 hunks)go/deploy/metald/internal/service/vm_cleanup_bench_test.go(1 hunks)go/go.mod(5 hunks)go/proto/metal/vmprovisioner/v1/vmprovisioner.proto(2 hunks)
💤 Files with no reviewable changes (2)
- go/deploy/metald/buf.yaml
- go/deploy/metald/buf.gen.yaml
🧰 Additional context used
📓 Path-based instructions (7)
`go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,rb,java,c,cpp,h,cs,rs,php,html,css,scss,xml}`: All text, ASCII, and code files MUST end with a newline.
go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,rb,java,c,cpp,h,cs,rs,php,html,css,scss,xml}: All text, ASCII, and code files MUST end with a newline.
📄 Source: CodeRabbit Inference Engine (go/deploy/CLAUDE.md)
List of files the instruction was applied to:
go/deploy/metald/internal/backend/cloudhypervisor/types.gogo/deploy/metald/client/types.gogo/deploy/metald/client/example_test.gogo/deploy/metald/internal/service/vm_cleanup_bench_test.gogo/deploy/metald/internal/reconciler/vm_reconciler.gogo/deploy/metald/cmd/metald/main.gogo/deploy/metald/internal/backend/cloudhypervisor/client.gogo/deploy/metald/internal/backend/firecracker/sdk_client_v4_test.gogo/deploy/metald/client/client.gogo/deploy/metald/client/vmconfig.gogo/deploy/metald/internal/database/repository.gogo/deploy/metald/client/cmd/metald-cli/main.gogo/deploy/metald/internal/backend/firecracker/automatic_build_test.gogo/deploy/metald/client/config.gogo/deploy/metald/internal/backend/types/backend.gogo/deploy/metald/internal/backend/firecracker/sdk_client_v4.gogo/deploy/metald/internal/service/vm.go
`go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,rb,java,c,cpp,h,cs,...
go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,rb,java,c,cpp,h,cs,rs,php,html,css,scss,xml}: UseAIDEV-NOTE:,AIDEV-TODO:,AIDEV-BUSINESS_RULE:, orAIDEV-QUESTION:(all-caps prefix) as anchor comments aimed at AI and developers.
Update relevant anchors when modifying associated code.
Do not removeAIDEV-*s without explicit human instruction.
Make sure to add relevant anchor comments whenever a file or piece of code is too complex, very important, confusing, or could have a bug.
📄 Source: CodeRabbit Inference Engine (go/deploy/CLAUDE.md)
List of files the instruction was applied to:
go/deploy/metald/internal/backend/cloudhypervisor/types.gogo/deploy/metald/client/types.gogo/deploy/metald/client/example_test.gogo/deploy/metald/internal/service/vm_cleanup_bench_test.gogo/deploy/metald/internal/reconciler/vm_reconciler.gogo/deploy/metald/cmd/metald/main.gogo/deploy/metald/internal/backend/cloudhypervisor/client.gogo/deploy/metald/internal/backend/firecracker/sdk_client_v4_test.gogo/deploy/metald/client/client.gogo/deploy/metald/client/vmconfig.gogo/deploy/metald/internal/database/repository.gogo/deploy/metald/client/cmd/metald-cli/main.gogo/deploy/metald/internal/backend/firecracker/automatic_build_test.gogo/deploy/metald/client/config.gogo/deploy/metald/internal/backend/types/backend.gogo/deploy/metald/internal/backend/firecracker/sdk_client_v4.gogo/deploy/metald/internal/service/vm.go
`go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go`: When a service's `*.go` code changes significantly, increase the patch-level version number.
go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go: When a service's*.gocode changes significantly, increase the patch-level version number.
📄 Source: CodeRabbit Inference Engine (go/deploy/CLAUDE.md)
List of files the instruction was applied to:
go/deploy/metald/internal/backend/cloudhypervisor/types.gogo/deploy/metald/client/types.gogo/deploy/metald/client/example_test.gogo/deploy/metald/internal/service/vm_cleanup_bench_test.gogo/deploy/metald/internal/reconciler/vm_reconciler.gogo/deploy/metald/cmd/metald/main.gogo/deploy/metald/internal/backend/cloudhypervisor/client.gogo/deploy/metald/internal/backend/firecracker/sdk_client_v4_test.gogo/deploy/metald/client/client.gogo/deploy/metald/client/vmconfig.gogo/deploy/metald/internal/database/repository.gogo/deploy/metald/client/cmd/metald-cli/main.gogo/deploy/metald/internal/backend/firecracker/automatic_build_test.gogo/deploy/metald/client/config.gogo/deploy/metald/internal/backend/types/backend.gogo/deploy/metald/internal/backend/firecracker/sdk_client_v4.gogo/deploy/metald/internal/service/vm.go
`go/deploy/*/cmd/*/**`: Service binary code should be located at `/cmd/`.
go/deploy/*/cmd/*/**: Service binary code should be located at<service>/cmd/<service | command>.
📄 Source: CodeRabbit Inference Engine (go/deploy/CLAUDE.md)
List of files the instruction was applied to:
go/deploy/metald/cmd/metald/main.go
`go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile`: Never use `go ...
go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile: Never usego buildfor any of theassetmanagerd,billaged,builderd,metaldbinaries.
Usemake buildto test that the binary builds.
Usemake installto build and install the binary with systemd unit from$SERVICE/contrib/systemd.
📄 Source: CodeRabbit Inference Engine (go/deploy/CLAUDE.md)
List of files the instruction was applied to:
go/deploy/metald/Makefile
`go/deploy/*/Makefile`: Service-level makefile should be `/Makefile`.
go/deploy/*/Makefile: Service-level makefile should be<service>/Makefile.
📄 Source: CodeRabbit Inference Engine (go/deploy/CLAUDE.md)
List of files the instruction was applied to:
go/deploy/metald/Makefile
`go/deploy/**/Makefile`: Global makefile should be `Makefile` at the root.
go/deploy/**/Makefile: Global makefile should beMakefileat the root.
📄 Source: CodeRabbit Inference Engine (go/deploy/CLAUDE.md)
List of files the instruction was applied to:
go/deploy/metald/Makefile
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/*/Makefile : Service-level makefile should be `<service>/Makefile`.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/*/cmd/*/** : Service binary code should be located at `<service>/cmd/<service | command>`.
go/deploy/metald/internal/backend/cloudhypervisor/types.go (1)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
go/deploy/metald/client/types.go (1)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
go/deploy/metald/client/example_test.go (1)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
go/deploy/metald/internal/service/vm_cleanup_bench_test.go (1)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
go/deploy/metald/internal/reconciler/vm_reconciler.go (1)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
go/deploy/metald/cmd/metald/main.go (2)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Never use `go build` for any of the `assetmanagerd`, `billaged`, `builderd`, `metald` binaries.
go/deploy/metald/internal/backend/cloudhypervisor/client.go (2)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Never use `go build` for any of the `assetmanagerd`, `billaged`, `builderd`, `metald` binaries.
go/deploy/metald/internal/backend/firecracker/sdk_client_v4_test.go (2)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Never use `go build` for any of the `assetmanagerd`, `billaged`, `builderd`, `metald` binaries.
go/deploy/metald/client/client.go (1)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
go/proto/metal/vmprovisioner/v1/vmprovisioner.proto (2)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/*/cmd/*/** : Service binary code should be located at `<service>/cmd/<service | command>`.
go/deploy/metald/client/vmconfig.go (1)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
go/deploy/metald/internal/database/repository.go (2)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Never use `go build` for any of the `assetmanagerd`, `billaged`, `builderd`, `metald` binaries.
go/deploy/metald/client/cmd/metald-cli/main.go (1)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
go/deploy/metald/internal/backend/firecracker/automatic_build_test.go (3)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Use `make build` to test that the binary builds.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Never use `go build` for any of the `assetmanagerd`, `billaged`, `builderd`, `metald` binaries.
go/deploy/metald/client/config.go (1)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
go/deploy/metald/internal/backend/types/backend.go (1)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
go/deploy/metald/internal/backend/firecracker/sdk_client_v4.go (2)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Never use `go build` for any of the `assetmanagerd`, `billaged`, `builderd`, `metald` binaries.
go/go.mod (3)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,rb,java,c,cpp,h,cs,rs,php,html,css,scss,xml} : Update relevant anchors when modifying associated code.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Never use `go build` for any of the `assetmanagerd`, `billaged`, `builderd`, `metald` binaries.
go/deploy/metald/internal/service/vm.go (2)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Never use `go build` for any of the `assetmanagerd`, `billaged`, `builderd`, `metald` binaries.
go/deploy/metald/go.mod (4)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Never use `go build` for any of the `assetmanagerd`, `billaged`, `builderd`, `metald` binaries.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Use `make build` to test that the binary builds.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,rb,java,c,cpp,h,cs,rs,php,html,css,scss,xml} : Update relevant anchors when modifying associated code.
go/deploy/metald/Makefile (8)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/**/Makefile : Global makefile should be `Makefile` at the root.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Use `make build` to test that the binary builds.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Never use `go build` for any of the `assetmanagerd`, `billaged`, `builderd`, `metald` binaries.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/*/Makefile : Service-level makefile should be `<service>/Makefile`.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Use `make install` to build and install the binary with systemd unit from `$SERVICE/contrib/systemd`.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,rb,java,c,cpp,h,cs,rs,php,html,css,scss,xml} : Do not remove `AIDEV-*`s without explicit human instruction.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/*/cmd/*/** : Service binary code should be located at `<service>/cmd/<service | command>`.
🪛 Buf (1.54.0)
go/proto/metal/vmprovisioner/v1/vmprovisioner.proto
3-3: Files with package "metal.vmprovisioner.v1" must be within a directory "metal/vmprovisioner/v1" relative to root but were in directory "go/proto/metal/vmprovisioner/v1".
(PACKAGE_DIRECTORY_MATCH)
🪛 checkmake (0.2.2)
go/deploy/metald/Makefile
[warning] 18-18: Missing required phony target "all"
(minphony)
🔇 Additional comments (19)
go/deploy/metald/internal/backend/cloudhypervisor/types.go (1)
4-5: Confirm new proto path is reflected in go.modThe import path shift to
github.com/unkeyed/unkey/go/gen/proto/metal/vmprovisioner/v1looks correct, but please ensure:
go.mod/go.sumhave been updated (go mod tidy).- No lingering
replacedirectives for the old path remain in downstream modules.go/deploy/metald/internal/backend/firecracker/automatic_build_test.go (1)
11-14: 👍 Import path update looks correct
Nothing else changed in the test; compilation should succeed once the module graph is tidied.go/deploy/metald/internal/backend/firecracker/sdk_client_v4_test.go (1)
9-11: Path update acknowledgedLGTM. No further action required.
go/deploy/metald/internal/database/repository.go (1)
10-10: LGTM: Clean import path update for protobuf centralization.The import path change from deployment-specific to centralized protobuf location is well-executed and maintains the existing
metaldv1alias, ensuring no breaking changes to the codebase.go/deploy/metald/internal/service/vm_cleanup_bench_test.go (1)
16-16: LGTM: Benchmark tests updated with centralized protobuf path.The import path change maintains consistency with the broader refactoring while preserving the existing
metaldv1alias and test functionality.go/deploy/metald/client/config.go (1)
9-9: LGTM: Client configuration updated with centralized protobuf path.The import path change correctly updates the reference to the centralized protobuf location while maintaining the existing
vmprovisionerv1alias.go/deploy/metald/client/vmconfig.go (1)
6-6: LGTM: VM configuration builder updated with centralized protobuf path.The import path change correctly references the centralized protobuf location while preserving the existing
vmprovisionerv1alias and builder functionality.go/deploy/metald/client/example_test.go (1)
10-10: LGTM: Example tests updated with centralized protobuf path.The import path change maintains consistency with the refactoring effort while preserving the existing
vmprovisionerv1alias and example test functionality.go/deploy/metald/client/client.go (1)
11-12: LGTM! Import path updates are consistent and correct.The import paths have been properly updated to use the centralized proto location, which aligns with the PR objective of moving vmprovisioner protos to shared proto.
go/deploy/metald/internal/backend/types/backend.go (1)
7-7: LGTM! Import path correctly updated to centralized proto location.The import path has been properly updated while maintaining the existing alias, ensuring seamless integration with the new proto package location.
go/deploy/metald/internal/reconciler/vm_reconciler.go (1)
14-14: LGTM! Import path correctly updated to centralized proto location.The import path has been properly updated while maintaining the existing alias, ensuring seamless integration with the new proto package location.
go/deploy/metald/client/cmd/metald-cli/main.go (1)
13-13: LGTM! Import path correctly updated to centralized proto location.The import path has been properly updated while maintaining the existing alias, ensuring seamless integration with the new proto package location.
go/deploy/metald/cmd/metald/main.go (1)
30-30: LGTM! Import path correctly updated to centralized proto location.The import path has been properly updated to use the centralized proto location for the connect client, ensuring seamless integration with the new proto package location.
go/deploy/metald/internal/backend/firecracker/sdk_client_v4.go (1)
27-27: LGTM: Clean import path migration to centralized proto location.The import path update correctly reflects the migration from deployment-specific protobuf generation to the shared proto location, aligning with the PR's objective to centralize vmprovisioner proto management.
go/deploy/metald/internal/service/vm.go (1)
14-15: LGTM: Consistent import path migration for both protobuf packages.Both import path updates correctly reflect the migration from deployment-specific protobuf generation to the shared proto location. The changes include both the main
metaldv1package and thevmprovisionerv1connectservice handler package, maintaining consistency in the centralized proto organization.go/proto/metal/vmprovisioner/v1/vmprovisioner.proto (1)
3-3: Buf directory rule now failsStatic analysis (
PACKAGE_DIRECTORY_MATCH) points out that files for packagemetal.vmprovisioner.v1must live inmetal/vmprovisioner/v1, but the file sits undergo/proto/....
If Buf is still used elsewhere in the mono-repo this will fail CI. Either:
- Move the file to
proto/metal/vmprovisioner/v1, or- Disable the rule / adjust the workspace configuration.
go/deploy/metald/go.mod (1)
3-3:go 1.24.4targets an unreleased toolchainGo 1.24 is not out yet. Using a future minor version will fail on standard builders (1.22.x is current).
Please pin to the latest released version used in CI, or ensure the CI image installs the tip toolchain.go/go.mod (1)
3-3: Root module also moved togo 1.24.4Same concern as in
metald/go.mod: make sure CI and downstream users have a 1.24 toolchain available.go/deploy/metald/Makefile (1)
30-31:citarget depends onlintbut not onfmt/vetanymoreThe
checktarget still callsfmt vet lint, butcinow skipsfmt.
If the intention is to mirror the previous behaviour, addfmtandvetback to the dependency list.⛔ Skipped due to learnings
Learnt from: CR PR: unkeyed/unkey#0 File: go/deploy/CLAUDE.md:0-0 Timestamp: 2025-07-09T08:42:29.316Z Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Use `make build` to test that the binary builds.
Comment on lines
3
to
5
| import ( | ||
| vmprovisionerv1 "github.com/unkeyed/unkey/go/deploy/metald/gen/vmprovisioner/v1" | ||
| vmprovisionerv1 "github.com/unkeyed/unkey/go/gen/proto/metal/vmprovisioner/v1" | ||
| ) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Keep aliasing consistent across packages
You switched to the new central proto path—great. Double-check that every package still aliases it as vmprovisionerv1 (some older files used metaldv1). Consistency prevents accidental mixed imports.
🤖 Prompt for AI Agents
In go/deploy/metald/client/types.go around lines 3 to 5, ensure the imported
package alias for the proto path is consistently set to vmprovisionerv1. If any
instances use a different alias like metaldv1, update them to vmprovisionerv1 to
maintain uniformity across all files and avoid mixed imports.
Comment on lines
15
to
18
| "github.com/unkeyed/unkey/go/deploy/metald/internal/backend/types" | ||
| metaldv1 "github.com/unkeyed/unkey/go/gen/proto/metal/vmprovisioner/v1" | ||
|
|
||
| "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Import order now breaks goimports ordering
metaldv1 (external) is listed between local and std-lib imports. Running goimports will reshuffle; committing the formatted file avoids noisy diffs later.
🤖 Prompt for AI Agents
In go/deploy/metald/internal/backend/cloudhypervisor/client.go around lines 15
to 18, the import order breaks goimports ordering by placing the external import
'metaldv1' between local and standard library imports. Rearrange the imports to
follow the standard grouping: first standard library imports, then external
imports, and finally local imports. Run goimports to automatically format and
order the imports correctly before committing to avoid noisy diffs.
Comment on lines
+3
to
+5
| package metal.vmprovisioner.v1; | ||
|
|
||
| option go_package = "metald/gen/vmprovisioner/v1;vmprovisionerv1"; | ||
| option go_package = "github.com/unkeyed/unkey/go/gen/proto/metal/vmprovisioner/v1;vmprovisionerv1"; |
Contributor
There was a problem hiding this comment.
Package rename introduces wire-level breaking changes
Switching the protobuf package from vmprovisioner.v1 to metal.vmprovisioner.v1 (and the corresponding go_package path) changes:
- The fully-qualified gRPC service name (
/metal.vmprovisioner.v1.VmService/...). - Fully-qualified message type URLs (affects Any, google.api.http transcoding, etc.).
Unless every client and server is deployed in lock-step, this will break existing traffic.
Consider providing a deprecation/transition strategy (dual-register, alias proto, version bump, or a gateway shim) before merging.
🧰 Tools
🪛 Buf (1.54.0)
3-3: Files with package "metal.vmprovisioner.v1" must be within a directory "metal/vmprovisioner/v1" relative to root but were in directory "go/proto/metal/vmprovisioner/v1".
(PACKAGE_DIRECTORY_MATCH)
🤖 Prompt for AI Agents
In go/proto/metal/vmprovisioner/v1/vmprovisioner.proto around lines 3 to 5,
changing the protobuf package name to metal.vmprovisioner.v1 causes wire-level
breaking changes affecting service names and message type URLs. To fix this,
revert the package name to the original vmprovisioner.v1 or implement a
transition strategy such as dual-registering both package names, creating an
alias proto, bumping the version, or adding a gateway shim to maintain
compatibility during rollout.
Comment on lines
+11
to
15
| github.com/unkeyed/unkey/go v0.0.0-00010101000000-000000000000 | ||
| github.com/unkeyed/unkey/go/deploy/assetmanagerd v0.0.0-20250709084132-0dd195e4c51b | ||
| github.com/unkeyed/unkey/go/deploy/billaged v0.0.0-20250709084132-0dd195e4c51b | ||
| github.com/unkeyed/unkey/go/deploy/builderd v0.0.0-20250709084132-0dd195e4c51b | ||
| github.com/unkeyed/unkey/go/deploy/pkg/health v0.0.0-00010101000000-000000000000 |
Contributor
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Nested module now depends on parent root module – risk of cyclic upgrades
Requiring github.com/unkeyed/unkey/go (the repository root) from inside the metald sub-module can create upgrade loops and complicate tagging.
Double-check that nothing inside metald actually imports packages from the root module; if not, drop this require/replace pair.
🏁 Script executed:
#!/bin/bash
# List packages in metald that import the root module.
rg -n $'^import .*"github.com/unkeyed/unkey/go' go/deploy/metald | headLength of output: 72
Remove unnecessary root module requirement in metald/go.mod
We ran:
rg -n '^import .*"github.com/unkeyed/unkey/go' go/deploy/metaldand found no imports of the parent root module in go/deploy/metald. You can safely remove the require github.com/unkeyed/unkey/go v0.0.0-00010101000000-000000000000 line (and any associated replace directives) from:
- go/deploy/metald/go.mod
🤖 Prompt for AI Agents
In go/deploy/metald/go.mod around lines 11 to 15, remove the line requiring the
root module github.com/unkeyed/unkey/go at version
v0.0.0-00010101000000-000000000000, as it is not imported anywhere in the
go/deploy/metald package. Also remove any associated replace directives related
to this root module to clean up unnecessary dependencies.
Comment on lines
+31
to
+33
| github.com/unkeyed/unkey/go/deploy/assetmanagerd v0.0.0-20250709084132-0dd195e4c51b | ||
| github.com/unkeyed/unkey/go/deploy/billaged v0.0.0-20250709084132-0dd195e4c51b | ||
| github.com/unkeyed/unkey/go/deploy/builderd v0.0.0-20250709084132-0dd195e4c51b |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Internal sub-module pseudo-versions hard-pinned
All three internal services are pinned to the same pseudo-version/commit.
If they are meant to track the current branch, consider using replace ../<service> instead of publishing pseudo-versions to avoid manual bumps.
🤖 Prompt for AI Agents
In go/go.mod around lines 31 to 33, the internal sub-modules are hard-pinned to
specific pseudo-versions, which requires manual updates for each new commit. To
fix this, replace these pseudo-version dependencies with local replace
directives pointing to the relative paths of the internal services (e.g.,
replace github.com/unkeyed/unkey/go/deploy/assetmanagerd => ../assetmanagerd).
This will make the modules track the current branch automatically and avoid
manual version bumps.
Comment on lines
+20
to
22
| build: deps ## Build the binary | ||
| @mkdir -p $(BUILD_DIR) | ||
| @go build $(LDFLAGS) -o $(BUILD_DIR)/$(BINARY_NAME) ./cmd/metald |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
build target still uses go build, violating service Makefile guidelines
Project guidelines state: “Never use go build for any of the … metald binaries.”
Refactor to delegate compilation to an internal script or go install, e.g.:
- @go build $(LDFLAGS) -o $(BUILD_DIR)/$(BINARY_NAME) ./cmd/metald
+ @go install $(LDFLAGS) ./cmd/metald
+ @mkdir -p $(BUILD_DIR)
+ @cp "$(shell go env GOPATH)/bin/$(BINARY_NAME)" "$(BUILD_DIR)/$(BINARY_NAME)"🤖 Prompt for AI Agents
In go/deploy/metald/Makefile at lines 20 to 22, the build target uses `go
build`, which violates the project guideline that forbids using `go build` for
metald binaries. To fix this, replace the `go build` command with a call to an
internal build script or use `go install` to compile the binary, ensuring the
build process aligns with the service Makefile guidelines.
This was referenced Aug 31, 2025
18 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.
What does this PR do?
Moves the vmprovisioner proto files to the core.
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
Refactor
Chores