feat: add init command and cleanup config#1
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughRemoves HTTP/1.1 flag and ACME/TLS ACME scaffolding from configuration, deletes a WebSocket proxy stub from the HTTP handler, adds an Init CLI subcommand that writes a default config, and updates/adds example scripts to start a test backend and perform functional /api/hello checks. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (CLI)
participant Main as main.rs
participant Init as init_config_command
User->>Main: Run "init" command
Main->>Init: init_config_command(config_path)
rect rgba(220, 240, 255, 0.3)
Note over Init: Check for existing file
Init->>Init: If file exists -> error
end
rect rgba(220, 255, 220, 0.3)
Note over Init: Generate & write
Init->>Init: Render default config
Init->>Init: Write file to path
Init-->>User: Success + guidance
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/scripts/proxy_single.sh(1 hunks)examples/scripts/proxy_single_rewrite.sh(1 hunks)src/adapters/http_handler.rs(0 hunks)src/config/models.rs(1 hunks)src/config/validation.rs(3 hunks)src/main.rs(4 hunks)
💤 Files with no reviewable changes (1)
- src/adapters/http_handler.rs
🧰 Additional context used
🧬 Code graph analysis (2)
examples/scripts/proxy_single.sh (1)
examples/scripts/_lib.sh (3)
cleanup_ports(8-25)wait_port_listen(29-42)timeout_guard(66-70)
examples/scripts/proxy_single_rewrite.sh (1)
examples/scripts/_lib.sh (4)
cleanup_ports(8-25)wait_port_listen(29-42)timeout_guard(66-70)wait_http_ok(46-63)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Agent
- GitHub Check: build-test-examples
🔇 Additional comments (12)
src/config/validation.rs (2)
9-9: LGTM: Import cleanup aligns with ACME removal.The removal of
AcmeConfigfrom imports is consistent with the PR's goal to remove ACME scaffolding from the codebase.
344-367: LGTM: TLS validation simplified to manual certificates only.The validation logic correctly enforces that both certificate and private key paths must be provided together. The error message is clear and actionable. This aligns with the removal of ACME support across the codebase.
examples/scripts/proxy_single_rewrite.sh (3)
1-9: LGTM: Script setup follows best practices.The script correctly uses strict mode (
set -euo pipefail) and properly sources the helper library. The configuration references the correct file for this rewrite test scenario.
10-16: LGTM: Backend setup is robust.The script properly cleans up ports, captures the backend PID, sets up exit traps, and waits for the backend to be listening before proceeding. This ensures reliable test execution.
18-24: LGTM: Gateway startup is properly managed.The gateway setup includes proper PID tracking, timeout protection, comprehensive trap handling, and functional HTTP readiness verification. This ensures the gateway is actually ready to handle requests before testing begins.
examples/scripts/proxy_single.sh (3)
8-8: LGTM: Config file update aligns with script split.The change from
proxy_single_rewrite.tomltoproxy_single.tomlcorrectly reflects the separation of basic proxy testing from rewrite testing, as described in the PR objectives.
12-21: LGTM: Temporary directory approach improves test isolation.The script now creates an isolated temporary directory with a controlled backend structure. This is a best practice for integration testing as it avoids filesystem dependencies and ensures cleanup. The trap handling properly removes the temporary directory on exit.
30-37: LGTM: Content validation is more robust.The test now verifies the actual response content rather than just HTTP status codes. The prefix match (
"hello from backend"*) appropriately handles potential trailing content from the HTTP server, and the error message provides actionable debugging information.src/config/models.rs (1)
307-314: LGTM: TLS configuration simplified correctly.The removal of ACME-related fields and the updated documentation clearly indicate this struct now only supports manual certificate/key pair configuration. The use of
Option<String>for both fields is appropriate and consistent with the validation logic that requires both to be present together.src/main.rs (3)
43-48: LGTM: Init command definition follows established patterns.The new
Initcommand is well-structured, follows the same pattern as existing commands (ValidateandServe), and provides a sensible default value for the config path. The documentation is clear and concise.
66-66: LGTM: Command dispatch logic is consistent.The Init command is properly integrated into the command dispatch flow, following the same early-return pattern as the Validate command. This correctly prevents server startup when running the init command.
Also applies to: 75-77
656-701: LGTM: Init command implementation is well-designed.The
init_config_commandfunction follows good practices:
- Prevents accidental overwrite by checking for existing files
- Provides a well-structured default configuration with examples
- Uses async I/O appropriately
- Includes helpful success messages guiding the user on next steps
- The default configuration template is comprehensive and includes commented examples for different route types
There was a problem hiding this comment.
Pull request overview
This PR introduces a new axon init command for improved onboarding and removes unused ACME/HTTP1 configuration fields to simplify the codebase.
- Adds
axon initcommand to generate a defaultconfig.tomlfile with sensible defaults - Removes unused
acmeandhttp1_enabledconfiguration fields fromServerConfigand associated validation logic - Removes dead WebSocket handler code that was never referenced
- Splits proxy test scripts to correctly match their respective configuration files
- Adds comprehensive helper library (
_lib.sh) for test scripts - Introduces new WebSocket integration test scripts for various scenarios
Reviewed changes
Copilot reviewed 6 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/main.rs | Adds new Init command variant and init_config_command function to generate default config files |
| src/config/models.rs | Removes http1_enabled field, AcmeConfig struct, and acme-related builder methods from ServerConfig |
| src/config/validation.rs | Removes InvalidAcme error variant, validate_acme_config function, and simplifies TLS validation logic |
| src/adapters/http_handler.rs | Removes unused handle_websocket_upgrade placeholder method |
| examples/scripts/_lib.sh | Adds shared helper functions for test scripts (port cleanup, waiting, timeout guards) |
| examples/scripts/proxy_single.sh | Updates to test basic proxy scenario without path rewriting |
| examples/scripts/proxy_single_rewrite.sh | New script to test proxy with path rewriting to match proxy_single_rewrite.toml config |
| examples/scripts/ws_echo.sh | Adds WebSocket echo test with subprotocol negotiation |
| examples/scripts/ws_binary.sh | Adds WebSocket binary frame test |
| examples/scripts/ws_close.sh | Adds WebSocket close frame test |
| examples/scripts/ws_large_payload.sh | Adds WebSocket large payload and idle timeout test |
| examples/scripts/ws_ping_pong.sh | Adds WebSocket ping/pong frame test |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This PR introduces several improvements to the Axon project:
Features
initcommand: Addedaxon initto generate a defaultconfig.tomlfile, improving the onboarding experience.Refactoring & Cleanup
acmeandhttp1_enabledfields fromServerConfiginsrc/config/models.rs.src/adapters/http_handler.rsrelated to the removed configuration.Tests & Examples
examples/scripts/.proxy_single.shintoproxy_single.sh(basic proxy) andproxy_single_rewrite.sh(proxy with rewrite) to correctly match the example configurations.examples/scripts/run_all.sh.All integration tests passed locally.
Summary by CodeRabbit
New Features
Refactor
Tests / Examples
✏️ Tip: You can customize this high-level summary in your review settings.