feat(helm-reval): serve /info on the api port with consistent version and commit

[priyaselvaganesan]

Why

The /info build-info endpoint was mounted on the management port, so it was not reachable through the ingress, which routes to the traffic/API port. Consumers outside the cluster could not query the deployed build. The reported metadata was also inconsistent: /info read the shared go-lib version package while logs and the OTel service.version read main.Version, and the commit field had regressed to unknown.

What changed

• Serve GET /info (unauthenticated) on the API/traffic router via a serveInfo helper so it is reachable through the ingress.
• Run /info through the shared observability middleware (metrics, tracing, logging, recovery) without authz, so it stays unauthenticated but is still metered, traced, logged, and panic-safe.
• Derive main.Version and main.GitCommit from the shared go-lib version package, so /info, logs, and the OTel service.version all report the same build from a single link-time stamp.
• Add the STABLE_GIT_COMMIT_FULL stamp key (full hash) to the shared tools/workspace_status.sh, with a test. The reval version.GitHash x_def references it, but the feat(bazel): consolidate every service into the root Bazel module #593 consolidation to a shared status script left it unemitted, so /info reported commit: unknown.

Customer Release Notes

The helm-reval /info endpoint is reachable externally and reports service name, build version, and full git commit consistently with logs and traces.

Usage

curl http://<reval-host>/info returns {"service": ..., "version": ..., "commit": ...}.

Testing

• bazel test //src/control-plane-services/helm-reval/cmd/reval/cli:cli_test
• bash tools/scripts/test/test-workspace-status.sh
• Built the stamped OCI image at this commit, deployed it to a local k3d cluster, and curled /info through the Envoy gateway. Confirmed version equals the OTel service.version and commit equals git rev-parse HEAD (full hash), and that an unknown path returns reval's own 404, proving the request reaches the service unauthenticated through the ingress.

Issues

Relates to #315

[coderabbitai]

📝 Walkthrough

Walkthrough

The reval service now loads shared build metadata for Version and GitCommit. The unauthenticated /info endpoint moves to the external API router. Workspace status output includes the full Git commit hash. Tests cover metadata and HTTP methods.

Changes

Version metadata and external /info endpoint

Layer / File(s)	Summary
Build version metadata tools/workspace_status.sh, tools/scripts/test/test-workspace-status.sh, src/control-plane-services/helm-reval/cmd/reval-service/...	Workspace status now emits the full Git commit hash. The reval service loads shared version and commit values with "unknown" fallbacks.
External info endpoint src/control-plane-services/helm-reval/cmd/reval/cli/server.go	The API router applies public observability and recovery middleware to /info. The management router no longer registers /info.
Info endpoint validation src/control-plane-services/helm-reval/cmd/reval/cli/server_internal_test.go	Tests verify management-router 404 behavior, populated response fields, and 405 Method Not Allowed responses with Allow: GET for non-GET requests.

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

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant API_Router
  participant PublicMiddleware
  participant golibversion_Handler
  Client->>API_Router: GET /info
  API_Router->>PublicMiddleware: Apply observability and recovery middleware
  PublicMiddleware->>golibversion_Handler: ServeHTTP
  golibversion_Handler-->>Client: service, version, commit

Loading

Suggested reviewers: max-nv, dmikhaylovnv

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title follows Conventional Commits format and accurately describes the primary feature of serving /info on the API port with shared version and commit metadata.

✨ 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 feat/info-external-consolidate

---

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

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

tools/scripts/test/test-workspace-status.sh (1)

35-39: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Test the -dirty suffix.

The test covers only a clean fixture. It does not verify the new dirty-worktree behavior. A regression that removes -dirty still passes.

Modify the fixture after capturing full_sha, then assert that STABLE_GIT_COMMIT_FULL equals ${full_sha}-dirty.

As per coding guidelines, changed tool behavior requires focused tests.

Suggested assertion

 full_sha="$(git -C "$fixture" rev-parse HEAD)"
 got_full="$( (cd "$fixture" && env -u NVCF_VERSION bash "$script") | awk '/^STABLE_GIT_COMMIT_FULL /{print $2}' )"
 [ "$got_full" = "$full_sha" ] || fail "STABLE_GIT_COMMIT_FULL -> '$got_full' (want $full_sha)"
 
+dirty_marker="$fixture/.dirty-marker"
+printf 'dirty\n' > "$dirty_marker"
+got_dirty="$( (cd "$fixture" && env -u NVCF_VERSION bash "$script") | awk '/^STABLE_GIT_COMMIT_FULL /{print $2}' )"
+[ "$got_dirty" = "${full_sha}-dirty" ] || fail "dirty STABLE_GIT_COMMIT_FULL -> '$got_dirty' (want ${full_sha}-dirty)"

🤖 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 `@tools/scripts/test/test-workspace-status.sh` around lines 35 - 39, Extend the
test around full_sha in the workspace-status script to modify the fixture after
capturing the clean commit hash, then invoke the script again and assert
STABLE_GIT_COMMIT_FULL equals the original hash with the -dirty suffix. Preserve
the existing clean-worktree assertion and use the existing fixture and
extraction flow.

Source: Coding guidelines

🤖 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/helm-reval/cmd/reval-service/main.go`:
- Around line 28-33: Update the version metadata initialization around Version
and GitCommit so explicit fallbacks are resolved once when build stamping is
absent, using the same values for NewRootCommand, CLI output, telemetry, and
/info. Preserve stamped values when present and reuse the resolved metadata
instead of allowing each consumer to resolve it independently.

In `@src/control-plane-services/helm-reval/cmd/reval/cli/server_internal_test.go`:
- Around line 114-122: Add a test covering the router wiring performed by
runServer: verify GET /info returns 200 through the API server and 404 through
the separate management-server handler, rather than testing serveInfo directly.
Use the repository-native test runner to execute the new coverage.

In `@src/control-plane-services/helm-reval/cmd/reval/cli/server.go`:
- Line 122: Update the route registration around serveInfo and the public
middleware setup so /info remains unauthenticated but passes through the same
structured logging, tracing, RED metrics, and recovery middleware as /v1,
explicitly excluding authzMiddleware. Ensure the middleware-wrapped route is
used for /info rather than registering it directly on the bare router.

---

Nitpick comments:
In `@tools/scripts/test/test-workspace-status.sh`:
- Around line 35-39: Extend the test around full_sha in the workspace-status
script to modify the fixture after capturing the clean commit hash, then invoke
the script again and assert STABLE_GIT_COMMIT_FULL equals the original hash with
the -dirty suffix. Preserve the existing clean-worktree assertion and use the
existing fixture and extraction flow.

🪄 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: cb9cdcca-0010-4f7f-a4c2-b523bbe60397

📥 Commits

Reviewing files that changed from the base of the PR and between c73b36e and 27053dc.

📒 Files selected for processing (6)

• src/control-plane-services/helm-reval/cmd/reval-service/BUILD.bazel
• src/control-plane-services/helm-reval/cmd/reval-service/main.go
• src/control-plane-services/helm-reval/cmd/reval/cli/server.go
• src/control-plane-services/helm-reval/cmd/reval/cli/server_internal_test.go
• tools/scripts/test/test-workspace-status.sh
• tools/workspace_status.sh

[dmikhaylovnv]

Do we need to protect new endpoint with some rate limiting feature as it does not require auth anymore?

[priyaselvaganesan]

Do we need to protect new endpoint with some rate limiting feature as it does not require auth anymore?

reval has no rate limiting on any endpoint today, and /info joins an already-open surface where the NotFound handler is externally reachable and /v1/validate and /v1/render run before authz rejects them, so limiting /info alone wouldn't cover it. A BackendTrafficPolicy in Envoy Gateway on reval's route is a better fit since it needs no code change and protects the whole surface uniformly.

I would prefer keeping this PR scoped to the endpoint move and adding that as a follow-up.