fix(nats-auth-callout): default to server subcommand when invoked with no args - #790
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe CLI now defaults to the ChangesNATS Auth Callout CLI
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to The change makes no-argument invocation start the server while preserving explicit subcommands, with focused tests covering the behavior. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/control-plane-services/nats-auth-callout/cmd/nvcf-nats-auth-callout-service/cli/root_test.go`:
- Around line 49-63: Update TestExecuteDefaultsToServer to exercise the
production default-routing logic instead of duplicating it locally: extract the
argument-selection branch from Execute into a testable helper such as
commandArgs, then test that a binary-only argument list produces ["server"] and
pass those returned arguments to rootCmd.Traverse. Keep the test focused on the
helper’s default behavior and run the repository-native Go tests.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 96eb2efb-6b72-480f-9318-aac7baec7201
📒 Files selected for processing (2)
src/control-plane-services/nats-auth-callout/cmd/nvcf-nats-auth-callout-service/cli/root.gosrc/control-plane-services/nats-auth-callout/cmd/nvcf-nats-auth-callout-service/cli/root_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/control-plane-services/nats-auth-callout/go.mod`:
- Line 111: Align the google.golang.org/grpc requirement with the replacement
version: either remove or update the replacement for the intentional v1.80.0
upgrade, or change the requirement to v1.79.3 and document the pin. Refresh
go.sum to match the selected version.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c2e723fe-bff5-410c-a3d1-1400b5fd46fe
📒 Files selected for processing (1)
src/control-plane-services/nats-auth-callout/go.mod
…h no args Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
6189d67 to
05639ce
Compare
…unction main() now routes to server by default when no args are given, so the test must use an explicit safe subcommand to avoid a config panic. Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
|
🎉 This PR is included in version nvcf-nats-auth-callout-service-v0.8.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
3 participants
TL;DR
Fixes nvcf-nats-auth-callout-service 0.8.0 pods going into CrashLoopBackOff on staging. The binary's cobra root command has no
RunE, so invoking it with no arguments prints help and exits 0. The Astro FargateDeployment config passes noargsto the container (the CRD has no field for it), so every pod crashes immediately. Until this merges, the monorepo image cannot deploy to Astro and staging stays on the old0.1.9binary lineage.Additional Details
commandArgs()helper that returns["server"]whenos.Argshas no subcommand, andos.Args[1:]otherwise.Execute()callsrootCmd.SetArgs(commandArgs())before running.server,config,version,completion) work as before.serversubcommand.Testing
Reproduced the failure and verified the fix end-to-end on a local k3d cluster.
Failure, observed on staging first: pod
nvcf-nats-auth-callout-service-b97d9975-hrwnd, 288 restarts, exit code 0 each time, logs showing the cobra help text instead of a running server:Reproduced the same crashloop locally by deploying the pre-fix image with no
args(mimicking the FargateDeployment): pod went CrashLoopBackOff with identical help-text logs.Verified the fix with a stamped build. The image entrypoint stays the bare binary (
/usr/bin/app, noserver), so the default-routing lives in the binary and covers every launch path. Deployed with noargs:1/1 Running, connects to NATS, starts the HTTP serverGET /inforeturns 200{"service":"nvcf-nats-auth-callout-service","version":"...","commit":"..."}POST /inforeturns 405 withAllow: GETGET /healthreturns 200New unit tests pass:
References
Relates to #315
Summary by CodeRabbit
Improvements
version, continue to work as entered.Bug Fixes
Tests