Skip to content

Conversation

@Xunzhuo
Copy link
Member

@Xunzhuo Xunzhuo commented Aug 30, 2025

What type of PR is this?

What this PR does / why we need it:

unify docker images

Which issue(s) this PR fixes:

Fixes #

Release Notes: Yes/No

Signed-off-by: bitliu <bitliu@tencent.com>
@Xunzhuo Xunzhuo requested a review from rootfs August 30, 2025 14:32
@github-actions
Copy link

👥 Owner Notification

The following owners have been identified for the changed files in this PR and have been automatically assigned:

📁 Root Directory

Owners: @rootfs, @Xunzhuo
Files changed:

  • .github/workflows/docker-publish.yml
  • .github/workflows/docker-release.yml

📁 deploy

Owners: @rootfs, @Xunzhuo
Files changed:

  • deploy/kubernetes/deployment.yaml
  • deploy/kubernetes/kustomization.yaml

This comment was automatically generated based on the OWNER files in the repository.

@Xunzhuo Xunzhuo merged commit 6824d28 into main Aug 30, 2025
4 checks passed
@rootfs rootfs deleted the unify-images branch August 30, 2025 14:36
srini-abhiram added a commit to srini-abhiram/semantic-router that referenced this pull request Dec 2, 2025
This commit addresses 6 critical issues identified during code review,
plus build improvements for better user experience.

Critical fixes:
- Issue vllm-project#1: PID file race condition - kill router process if PID file
  write fails to prevent untrackable processes
- Issue vllm-project#2: Cross-platform compatibility - replace hardcoded /tmp paths
  with os.TempDir() and add user-specific suffixes using os.Getuid()
- Issue vllm-project#3: Signal handling - add graceful shutdown for port-forward
  process on Ctrl+C to prevent orphaned kubectl processes
- Issue vllm-project#4: HTTP timeout - add 30-second timeout to prevent hanging
  requests in test command
- Issue vllm-project#5: Security - restrict log/PID file permissions to 0600
- Issue vllm-project#6: Input validation - enforce 10k character limit on prompts

Build improvements:
- Embed library path using -ldflags="-r" to eliminate need for users
  to manually set LD_LIBRARY_PATH when running vsr binary

Files modified:
- src/semantic-router/pkg/cli/deployment/deployment.go
- src/semantic-router/pkg/cli/deployment/upgrade.go
- src/semantic-router/cmd/vsr/commands/dashboard.go
- src/semantic-router/cmd/vsr/commands/test.go
- tools/make/build-run-test.mk
- src/semantic-router/pkg/cli/deployment/deployment_test.go

All fixes have been verified with standalone test programs.

Signed-off-by: Srinivas A <56465971+srini-abhiram@users.noreply.github.com>
srini-abhiram added a commit to srini-abhiram/semantic-router that referenced this pull request Dec 2, 2025
This commit addresses 6 critical issues identified during code review,
plus build improvements for better user experience.

Critical fixes:
- Issue vllm-project#1: PID file race condition - kill router process if PID file
  write fails to prevent untrackable processes
- Issue vllm-project#2: Cross-platform compatibility - replace hardcoded /tmp paths
  with os.TempDir() and add user-specific suffixes using os.Getuid()
- Issue vllm-project#3: Signal handling - add graceful shutdown for port-forward
  process on Ctrl+C to prevent orphaned kubectl processes
- Issue vllm-project#4: HTTP timeout - add 30-second timeout to prevent hanging
  requests in test command
- Issue vllm-project#5: Security - restrict log/PID file permissions to 0600
- Issue vllm-project#6: Input validation - enforce 10k character limit on prompts

Build improvements:
- Embed library path using -ldflags="-r" to eliminate need for users
  to manually set LD_LIBRARY_PATH when running vsr binary

Files modified:
- src/semantic-router/pkg/cli/deployment/deployment.go
- src/semantic-router/pkg/cli/deployment/upgrade.go
- src/semantic-router/cmd/vsr/commands/dashboard.go
- src/semantic-router/cmd/vsr/commands/test.go
- tools/make/build-run-test.mk
- src/semantic-router/pkg/cli/deployment/deployment_test.go

All fixes have been verified with standalone test programs.

Signed-off-by: Srinivas A <56465971+srini-abhiram@users.noreply.github.com>
srini-abhiram added a commit to srini-abhiram/semantic-router that referenced this pull request Dec 2, 2025
Add unit tests to verify all 6 critical production fixes implemented
in the previous commit. Tests ensure cross-platform compatibility,
security, and robustness of the VSR CLI tool.

Test coverage added:
- Cross-platform path handling (Issue vllm-project#2)
  * Absolute path validation
  * User-specific path generation
  * Temp directory usage

- PID file management and security (Issues vllm-project#1, vllm-project#5)
  * PID file race condition handling
  * Process termination on write failure
  * Restrictive 0600 file permissions
  * Proper cleanup on exit

- HTTP client timeout (Issue vllm-project#4)
  * 30-second timeout prevents hanging
  * Fast responses unaffected
  * Timeout configuration validation

- Input validation (Issue vllm-project#6)
  * 10k character limit enforcement
  * Boundary condition testing

- Signal handling (Issue vllm-project#3)
  * Graceful shutdown on interrupt
  * Process cleanup verification
  * No orphaned processes

All tests compile and validate the production fixes.

Signed-off-by: Srinivas A <56465971+srini-abhiram@users.noreply.github.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.

3 participants