[ME-4981] Add IsV2 Flag for Tunnel Cert Requests #61
Conversation
WalkthroughAdds a boolean field is_v2 (field number 4) to the TunnelCertificateSignRequest message in connector/connector.proto. No other messages or services are modified. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
connector/connector.proto (3)
72-72: Consider presence semantics: optional vs default falseIf you need to distinguish “unset” from “explicitly false,” use presence. Two options:
- Prefer: proto3
optional(requires protoc and language plugins >= 3.12).- Alternative:
google.protobuf.BoolValuewrapper.Suggested change (if presence is needed):
- bool is_v2 = 4; + optional bool is_v2 = 4;If your build toolchain doesn’t support proto3 optional across all consumers, use the wrapper type instead and add the import:
- Field:
- bool is_v2 = 4;
- google.protobuf.BoolValue is_v2 = 4;
- Import (top of file):import "google/protobuf/wrappers.proto";
--- `72-72`: **Document the intent and default behavior of is_v2** A short comment makes the API clearer for downstream consumers and avoids guesswork. ```diff - bool is_v2 = 4; + // When true, request a v2-format tunnel certificate. Defaults to false for backward compatibility. + bool is_v2 = 4;
72-72: Future-proofing: use an enum for versioning instead of a booleanIf there’s any chance of a v3 (or variants), an enum avoids additive boolean flags (is_v3, is_v4, …) and scales better.
Minimal field change here:
- bool is_v2 = 4; + TunnelCertVersion version = 4; // defaults to VERSION_V1 if unsetAnd define elsewhere in this file:
enum TunnelCertVersion { VERSION_UNSPECIFIED = 0; // Treat as V1 for backward compatibility VERSION_V1 = 1; VERSION_V2 = 2; }This keeps wire-compatibility and gives clear semantics going forward.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
connector/connector.pb.gois excluded by!**/*.pb.gogen/kotlin/border0/v1/TunnelCertificateSignRequestKt.ktis excluded by!**/gen/**gen/swift/connector.pb.swiftis excluded by!**/gen/**
📒 Files selected for processing (1)
connector/connector.proto(1 hunks)
🔇 Additional comments (2)
connector/connector.proto (2)
68-73: LGTM: backward-compatible field addition with a safe tag numberAdding
bool is_v2 = 4;is wire-compatible (old clients ignore unknown fields). Tag 4 fills the next available slot in this message, which is ideal. No concerns with Go/JSON name mapping: it will generateIsV2in Go andisV2for JSON.
68-73: Stubs regenerated for Go/Kotlin/Swift — no downstream changes foundGenerated stubs in this repo already include the new field and I found no non-generated consumers that need updates; the proto bool default is false when the field is absent.
- Checked files showing the regen:
- connector/connector.proto — TunnelCertificateSignRequest { bool is_v2 = 4; }
- connector/connector.pb.go — TunnelCertificateSignRequest has IsV2 bool and GetIsV2()
- gen/kotlin/border0/v1/TunnelCertificateSignRequestKt.kt — isV2 property with get/set/clear
- gen/swift/connector.pb.swift — isV2 property and encode/decode handling
- No non-generated code (consumers) found that construct or inspect is_v2.
- No JSON/YAML/OpenAPI schemas referencing is_v2 were found in the repo.
- Server default behavior: absent boolean fields default to false (generated getters return false when not set).
Action: none required in this repo. If you publish stubs for any other language targets not present here, regenerate those targets as well.
[ME-4981] Add IsV2 Flag for Tunnel Cert Requests
Summary by CodeRabbit