Skip to content

refactor: switch admin ops from protobuf to binary encoding#10

Merged
vieiralucas merged 2 commits intomainfrom
refactor/binary-admin-encoding
Mar 27, 2026
Merged

refactor: switch admin ops from protobuf to binary encoding#10
vieiralucas merged 2 commits intomainfrom
refactor/binary-admin-encoding

Conversation

@vieiralucas
Copy link
Copy Markdown
Member

@vieiralucas vieiralucas commented Mar 27, 2026

Summary

  • Remove protobuf dependency (google.golang.org/protobuf) and all generated proto files (filav1/, proto/)
  • Rewrite admin operations (CreateQueue, DeleteQueue, GetQueueStats, ListQueues, Redrive, SetConfig, GetConfig, ListConfig) to use binary wire format matching data ops
  • Add new Go types: QueueConfig, QueueStats, QueueInfo, RedriveResult, ConfigEntry
  • Add admin lifecycle integration test (create + list + delete)

Test plan

  • go vet ./... passes (verified locally)
  • Integration tests pass against a FIBP server with binary admin encoding (PR #153)
  • No protobuf imports remain in the codebase

Summary by cubic

Switch admin operations to the binary wire format used by data ops, removing the google.golang.org/protobuf dependency and all proto artifacts; requires a server that supports binary admin encoding. Also fixes a bounds check in GetQueueStats parsing to avoid panics on valid 48-byte responses.

  • Refactors

    • Rewrote admin ops: CreateQueue, DeleteQueue, GetQueueStats, ListQueues, Redrive, SetConfig, GetConfig, ListConfig.
    • Added Go types: QueueConfig, QueueStats, QueueInfo, RedriveResult, ConfigEntry.
    • Added admin lifecycle integration test (create → list → delete).
  • Bug Fixes

    • Corrected bounds check in GetQueueStats response parsing to match the 48-byte layout.

Written for commit 7b28306. Summary will update on new commits.

remove protobuf dependency and generated proto files. admin operations
(create_queue, delete_queue, queue_stats, list_queues, redrive,
config_set/get/list) now use the same binary wire format as data ops.
add admin lifecycle integration test (create + list + delete).
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 10 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="admin.go">

<violation number="1" location="admin.go:147">
P1: Bounds check is `< 40` but the code reads 48 bytes (`p[44:48]`). A short response (40–47 bytes) will pass the check and cause an index-out-of-range panic at `ReplicationCount` or `ClusterNodeCount`.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread admin.go Outdated
the check was < 40 but the parser reads through p[44:48], which would
panic on a valid 48-byte response that arrived as exactly 48 bytes.
@vieiralucas
Copy link
Copy Markdown
Member Author

Fixed in 7b28306. The bounds check was but the response layout includes (p[40:44]) and (p[44:48]), requiring 48 bytes. Updated to .

@vieiralucas vieiralucas merged commit 10e5d65 into main Mar 27, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant