Skip to content

fix(nats-auth-callout): default to server subcommand when invoked with no args - #790

Open
priyaselvaganesan wants to merge 2 commits into
mainfrom
fix/nats-default-server-cmd
Open

fix(nats-auth-callout): default to server subcommand when invoked with no args#790
priyaselvaganesan wants to merge 2 commits into
mainfrom
fix/nats-default-server-cmd

Conversation

@priyaselvaganesan

@priyaselvaganesan priyaselvaganesan commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

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 no args to the container (the CRD has no field for it), so every pod crashes immediately.

Additional Details

  • Adds a commandArgs() helper that returns ["server"] when os.Args has no subcommand, and os.Args[1:] otherwise.
  • Execute() calls rootCmd.SetArgs(commandArgs()) before running.
  • All explicit subcommands (server, config, version, completion) work as before.
  • The old 0.1.9 image (a separate binary lineage) auto-started its server on invocation with no args; the monorepo binary requires an explicit server subcommand.

Testing

Observed on staging — pod nvcf-nats-auth-callout-service-b97d9975-hrwnd in astro-tenant-nvcf-nats-auth-callout-service, 288 restarts, exit code 0 each time:

$ kubectl logs nvcf-nats-auth-callout-service-b97d9975-hrwnd \
    -n astro-tenant-nvcf-nats-auth-callout-service \
    -c nvcf-nats-auth-callout-service --previous

provide ping and healthz http server

Usage:
  nvcf-nats-auth-callout-service [command]

Available Commands:
  completion  Generate the autocompletion script for the specified shell
  config      Configuration management and debugging
  help        Help about any command
  server      Simple HTTP server with ping and healthz endpoints
  version     Show version information

Flags:
  -h, --help   help for nvcf-nats-auth-callout-service

Use "nvcf-nats-auth-callout-service [command] --help" for more information about a command.

Confirmed fix routes to server locally (NKey config error expected without Vault secrets):

$ nvcf-nats-auth-callout-service
Error: invalid configuration: NKey seed cannot be empty
exit code: 2

New tests pass:

$ go test ./cmd/nvcf-nats-auth-callout-service/cli/... -run "TestNewRootCommand|TestCommandArgs" -v
--- PASS: TestNewRootCommand (0.00s)
--- PASS: TestCommandArgsDefaultsToServer (0.00s)
--- PASS: TestCommandArgsPassesExplicitArgs (0.00s)

References

Relates to #315

Summary by CodeRabbit

  • Improvements

    • The command-line tool now starts the server automatically when launched without a subcommand.
    • Explicit subcommands, such as version, continue to work as entered.
  • Bug Fixes

    • Improved command-line startup behavior to prevent failures when no arguments are provided.
  • Tests

    • Added coverage for default startup and explicit subcommand handling.

@priyaselvaganesan
priyaselvaganesan requested a review from a team as a code owner August 12, 2026 17:49
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 979855a3-4e07-4d0b-9a26-8d7402239f65

📥 Commits

Reviewing files that changed from the base of the PR and between 05639ce and fe9f67d.

📒 Files selected for processing (1)
  • src/control-plane-services/nats-auth-callout/cmd/nvcf-nats-auth-callout-service/main_test.go

📝 Walkthrough

Walkthrough

The CLI now defaults to the server subcommand when invoked without arguments. Explicit arguments remain unchanged. Tests cover both command-selection paths and safe main execution.

Changes

NATS Auth Callout CLI

Layer / File(s) Summary
Server command default and validation
src/control-plane-services/nats-auth-callout/cmd/nvcf-nats-auth-callout-service/cli/root.go, src/control-plane-services/nats-auth-callout/cmd/nvcf-nats-auth-callout-service/cli/root_test.go, src/control-plane-services/nats-auth-callout/cmd/nvcf-nats-auth-callout-service/main_test.go
commandArgs returns server when no arguments exist and preserves explicit arguments. Tests verify command selection and main execution with the version subcommand.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score: ⚪ Minimal · up to fe9f6

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: balajinvda

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title uses the required fix(scope): subject format and accurately describes the bug fix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/nats-default-server-cmd

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between 8989d60 and 71838de.

📒 Files selected for processing (2)
  • src/control-plane-services/nats-auth-callout/cmd/nvcf-nats-auth-callout-service/cli/root.go
  • src/control-plane-services/nats-auth-callout/cmd/nvcf-nats-auth-callout-service/cli/root_test.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between 71838de and 6189d67.

📒 Files selected for processing (1)
  • src/control-plane-services/nats-auth-callout/go.mod

Comment thread src/control-plane-services/nats-auth-callout/go.mod Outdated
…h no args

Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
@priyaselvaganesan
priyaselvaganesan force-pushed the fix/nats-default-server-cmd branch from 6189d67 to 05639ce Compare August 12, 2026 21:41
…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>
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