-
Notifications
You must be signed in to change notification settings - Fork 244
Disable snapshot read endpoints by-default, require a per-interface opt-in to enable new OperatorFeature
#7440
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disable snapshot read endpoints by-default, require a per-interface opt-in to enable new OperatorFeature
#7440
Conversation
…e_serving_interfaces
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the network interface configuration system and adds opt-in feature gating for snapshot-serving endpoints to prevent public DoS attacks. The changes introduce a new FILE_SERVING_RPC_INTERFACE with FileAccess feature enabled by default, and modify the test infrastructure to use a more fluent API pattern for configuring nodes.
Key changes:
- Introduces opt-in feature gating for endpoints, specifically for file access/snapshot serving
- Refactors
RPCInterface.from_args()toapply_args()and addsHostSpec.with_args()for better composability - Adds a dedicated file-serving interface to all nodes by default with FileAccess feature enabled
- Moves snapshot-serving handlers from
node_frontend.hto newfile_serving_handlers.hfile
Reviewed Changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/infra/interfaces.py | Refactors interface configuration with apply_args() and with_args() methods, adds FILE_SERVING_RPC_INTERFACE |
| tests/infra/e2e_args.py | Simplifies nodes() function to use HostSpec.with_args() |
| tests/infra/node.py | Improves interface lookup with error handling |
| tests/infra/network.py | Updates create_node() to accept optional host parameter |
| tests/e2e_operations.py | Updates snapshot access tests to use FILE_SERVING_RPC_INTERFACE |
| src/node/rpc/file_serving_handlers.h | New file containing extracted snapshot-serving handlers with feature gating |
| src/node/rpc/frontend.h | Adds feature checking logic for endpoints |
| src/endpoints/endpoint.cpp | Adds require_optin_feature() method |
| include/ccf/endpoint.h | Defines OptInFeatures and adds required_optin_features field |
| doc/host_config_schema/cchost_config.json | Documents enabled_optin_features configuration option |
Comments suppressed due to low confidence (1)
tests/infra/basicperf.py:213
- Call to function nodes with too few arguments; should be no fewer than 2.
hosts = args.nodes or infra.e2e_args.nodes(1)
Co-authored-by: Copilot <[email protected]>
…e_serving_interfaces
…e_serving_interfaces
…e_serving_interfaces
FileAccess feature as requirement for calling /snapshot endpointsOperatorFeature
This has a bunch of knock-on effects. The related diffs may not all be strictly required, I've lost track, but they lead to a cleaner API (for things like "create a new node" in the Python infra) and I think for backporting purposes they all land together.