fix(rest-api): fix make kind-reset for local development#3239
Conversation
WalkthroughThis PR adds ChangesRest API Makefile Changes
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rest-api/Makefile (1)
512-541: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider hoisting the repeated TLS/address flags into a variable.
The frontend address plus the four
--tls-*flags are duplicated across all threetemporal operator namespaceinvocations. A shared Make variable would keep them in lockstep and reduce the recipe noise.♻️ Optional — extract shared connection flags
TEMPORAL_ADMIN_FLAGS := --address temporal-frontend.temporal:7233 \ --tls-cert-path /var/secrets/temporal/certs/server-interservice/tls.crt \ --tls-key-path /var/secrets/temporal/certs/server-interservice/tls.key \ --tls-ca-path /var/secrets/temporal/certs/server-interservice/ca.crt \ --tls-server-name interservice.server.temporal.localThen each call collapses to
temporal operator namespace describe --namespace $$ns $(TEMPORAL_ADMIN_FLAGS).🤖 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 `@rest-api/Makefile` around lines 512 - 541, The Temporal namespace registration recipe repeats the same frontend address and TLS flags across the describe, create, and verify calls, so hoist that shared connection configuration into a Make variable in the Makefile. Update the commands in the namespace loop to reuse that variable with the `temporal operator namespace` invocations so the settings stay consistent and the recipe is easier to maintain.
🤖 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 `@rest-api/Makefile`:
- Around line 533-541: The final namespace verification in the Makefile is still
a single-shot `kubectl ... temporal operator namespace describe` check, so it
can fail during Temporal propagation lag. Update the namespace-check block that
iterates over `cloud` and `site` to use the same bounded retry behavior as the
create loop, or fold the describe into that existing retry path, so transient
`NOT_FOUND` responses are retried before failing.
---
Nitpick comments:
In `@rest-api/Makefile`:
- Around line 512-541: The Temporal namespace registration recipe repeats the
same frontend address and TLS flags across the describe, create, and verify
calls, so hoist that shared connection configuration into a Make variable in the
Makefile. Update the commands in the namespace loop to reuse that variable with
the `temporal operator namespace` invocations so the settings stay consistent
and the recipe is easier to maintain.
🪄 Autofix (Beta)
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: d0084632-b4c1-421c-aaa2-0286ace1f10e
📒 Files selected for processing (1)
rest-api/Makefile
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-07-08 01:00:46 UTC | Commit: 09acf6d |
Signed-off-by: Patrice Breton <pbreton@nvidia.com>
…ion lag The final namespace check in kind-reset-infra was a single-shot `temporal operator namespace describe`, which could return a transient NOT_FOUND during Temporal propagation lag right after a successful `create` and abort the local reset with a false failure. Fold verification into the existing create/retry loop: the loop now succeeds only on a confirmed `describe` (tracked via `ok`) rather than a successful `create`, so a not-yet-visible namespace is re-checked and transient NOT_FOUND responses are retried before failing loudly. Removes the redundant separate verify block. Local-only target; no chart or production behavior changes. Signed-off-by: Patrice Breton <pbreton@nvidia.com>
335b136 to
d4cdfe6
Compare
|
Thanks @pbreton |
thossain-nv
left a comment
There was a problem hiding this comment.
Thanks @pbreton for the improvement.
Ensure
make kind-resetis able to run and bring rest-api for local development.Type of Change
Breaking Changes
Testing