feat(nats-auth-callout): expose GET /info endpoint - #711
Open
priyaselvaganesan wants to merge 3 commits into
Open
feat(nats-auth-callout): expose GET /info endpoint#711priyaselvaganesan wants to merge 3 commits into
priyaselvaganesan wants to merge 3 commits into
Conversation
Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
priyaselvaganesan
requested review from
along-2017 and
balajinvda
and removed request for
along-2017
August 6, 2026 19:04
📝 WalkthroughWalkthroughThe NATS auth callout service now exposes a version-backed ChangesNATS auth callout service info endpoint
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant golibversion
Client->>Router: Request GET /info
Router->>golibversion: Serve version metadata
golibversion-->>Router: Return JSON metadata
Router-->>Client: Return HTTP response
Client->>Router: Request non-GET /info
Router-->>Client: Return 405 with Allow: GET
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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
🧹 Nitpick comments (1)
src/control-plane-services/nats-auth-callout/internal/router/info_test.go (1)
44-50: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftAssert the stamped metadata values.
The test accepts fallback values such as
"unknown". A missing or miswiredx_defsvalue can therefore pass. This does not verify the service name, semantic version, or full commit SHA returned by/info.Add a stamped test with controlled values or an injectable handler, then assert the exact
service,version, andcommitfields. Keep this test for fallback behavior only if that behavior is also required.🤖 Prompt for 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. In `@src/control-plane-services/nats-auth-callout/internal/router/info_test.go` around lines 44 - 50, Update the /info test around the response unmarshalling to inject controlled metadata values and assert exact service, version, and commit values instead of only checking non-empty fields. Use the existing handler or configuration injection point visible in the router tests, and retain the fallback assertion only if fallback behavior is separately required.
🤖 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 v1.80.0 requirement with its
replacement: either change the required version to v1.79.3 and document why the
replacement is intentional, or remove the replacement so v1.80.0 is used
directly.
---
Nitpick comments:
In `@src/control-plane-services/nats-auth-callout/internal/router/info_test.go`:
- Around line 44-50: Update the /info test around the response unmarshalling to
inject controlled metadata values and assert exact service, version, and commit
values instead of only checking non-empty fields. Use the existing handler or
configuration injection point visible in the router tests, and retain the
fallback assertion only if fallback behavior is separately required.
🪄 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: 45aa28a4-ea93-435e-a10c-161224c073c5
⛔ Files ignored due to path filters (1)
src/control-plane-services/nats-auth-callout/go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
src/control-plane-services/nats-auth-callout/cmd/nvcf-nats-auth-callout-service/BUILD.bazelsrc/control-plane-services/nats-auth-callout/go.modsrc/control-plane-services/nats-auth-callout/internal/router/BUILD.bazelsrc/control-plane-services/nats-auth-callout/internal/router/info_test.gosrc/control-plane-services/nats-auth-callout/internal/router/router.go
…ethodNotAllowed, align grpc require to replace Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
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/internal/router/info_test.go`:
- Around line 33-41: Update TestInfoEndpoint_GET to capture the existing
golibversion.Service, golibversion.Version, and golibversion.GitHash values
before overwriting them, then restore those saved values in t.Cleanup instead of
clearing the globals. Follow the restoration pattern used by
TestInfoEndpoint_GET_UnstampedFallback.
🪄 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: 7e5821f1-f467-493b-b2de-ec80bd8b9475
📒 Files selected for processing (4)
src/control-plane-services/nats-auth-callout/go.modsrc/control-plane-services/nats-auth-callout/internal/router/BUILD.bazelsrc/control-plane-services/nats-auth-callout/internal/router/info_test.gosrc/control-plane-services/nats-auth-callout/internal/router/router.go
🚧 Files skipped from review as they are similar to previous changes (2)
- src/control-plane-services/nats-auth-callout/internal/router/router.go
- src/control-plane-services/nats-auth-callout/internal/router/BUILD.bazel
Comment on lines
+33
to
+41
| func TestInfoEndpoint_GET(t *testing.T) { | ||
| golibversion.Service = "nvcf-nats-auth-callout-service" | ||
| golibversion.Version = "test-1.0.0" | ||
| golibversion.GitHash = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" | ||
| t.Cleanup(func() { | ||
| golibversion.Service = "" | ||
| golibversion.Version = "" | ||
| golibversion.GitHash = "" | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Restore the prior version metadata.
TestInfoEndpoint_GET changes process-global metadata. Its cleanup always clears the values. This can overwrite state set before the test and make later tests depend on execution order.
Save golibversion.Service, golibversion.Version, and golibversion.GitHash before assignment. Restore those saved values in t.Cleanup, as TestInfoEndpoint_GET_UnstampedFallback does.
Proposed fix
func TestInfoEndpoint_GET(t *testing.T) {
+ previousService := golibversion.Service
+ previousVersion := golibversion.Version
+ previousGitHash := golibversion.GitHash
golibversion.Service = "nvcf-nats-auth-callout-service"
golibversion.Version = "test-1.0.0"
golibversion.GitHash = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
t.Cleanup(func() {
- golibversion.Service = ""
- golibversion.Version = ""
- golibversion.GitHash = ""
+ golibversion.Service = previousService
+ golibversion.Version = previousVersion
+ golibversion.GitHash = previousGitHash
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func TestInfoEndpoint_GET(t *testing.T) { | |
| golibversion.Service = "nvcf-nats-auth-callout-service" | |
| golibversion.Version = "test-1.0.0" | |
| golibversion.GitHash = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" | |
| t.Cleanup(func() { | |
| golibversion.Service = "" | |
| golibversion.Version = "" | |
| golibversion.GitHash = "" | |
| }) | |
| func TestInfoEndpoint_GET(t *testing.T) { | |
| previousService := golibversion.Service | |
| previousVersion := golibversion.Version | |
| previousGitHash := golibversion.GitHash | |
| golibversion.Service = "nvcf-nats-auth-callout-service" | |
| golibversion.Version = "test-1.0.0" | |
| golibversion.GitHash = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" | |
| t.Cleanup(func() { | |
| golibversion.Service = previousService | |
| golibversion.Version = previousVersion | |
| golibversion.GitHash = previousGitHash | |
| }) |
🤖 Prompt for 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.
In `@src/control-plane-services/nats-auth-callout/internal/router/info_test.go`
around lines 33 - 41, Update TestInfoEndpoint_GET to capture the existing
golibversion.Service, golibversion.Version, and golibversion.GitHash values
before overwriting them, then restore those saved values in t.Cleanup instead of
clearing the globals. Follow the restoration pattern used by
TestInfoEndpoint_GET_UnstampedFallback.
…e global HandleMethodNotAllowed Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
along-2017
reviewed
Aug 7, 2026
| http.MethodTrace, | ||
| } { | ||
| r.engine.Handle(method, "/info", infoMethodNotAllowed) | ||
| } |
Contributor
There was a problem hiding this comment.
you may consider set engine.HandleMethodNotAllowed = true and gin handles 405 for all unregistered methods. In this case, this loop not needed anymore.
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
2 participants
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.
TL;DR
Adds
GET /infoto the nats-auth-callout gin router (port 8080), serving service name, version, and commit SHA via the shared go-lib handler.Additional Details
r.engine.GET("/info", ...)for the handler, with explicitr.engine.Handle(method, "/info", ...)registrations for HEAD/POST/PUT/PATCH/DELETE/OPTIONS/CONNECT/TRACE returning 405 +Allow: GET. Method enforcement is scoped to/infoonly — other routes are unaffected.Service,Version, andGitHashx_defs are injected at build time (--stamp) on thecmd/nvcf-nats-auth-callout-servicebinary.GitHashuses{STABLE_GIT_COMMIT_FULL}for the full 40-char SHA.require google.golang.org/grpc v1.80.0tov1.79.3to match the existing replace directive."unknown", and non-GET rejection on all 8 methods.Testing
Built with Bazel, imported into ncp-local k3d, patched
deployment/nvcf-nats-auth-callout-service, and verified via port-forward:Unit tests pass under both
go testandbazel test.References
GET /infoendpoint #275 (helm-reval), feat(grpc-proxy): expose GET /info endpoint #610 (grpc-proxy)Relates to #315
Summary by CodeRabbit
New Features
/infoendpoint that reports service version metadata.405 Method Not Allowedresponse and indicate that onlyGETis supported.Tests