Open DaMandal0rian opened 2 months ago
PR Description updated to latest commit (https://github.com/subspace/infra/commit/500bbf8ab958260dd055e9dccd4afb8b051217be)
โฑ๏ธ Estimated effort to review [1-5] | 3, because the PR involves multiple configuration changes across different files and domains, which requires a detailed review to ensure consistency and correctness in the configurations. |
๐งช Relevant tests | No |
โก Possible issues | Possible Redundancy: The configuration for state and blocks pruning is repeated for both EVM and AUTO domains. This could be simplified or parameterized to avoid redundancy and potential errors in future updates. |
Configuration Clarity: The use of environment variables like `${DOMAIN_ID_EVM}` and `${DOMAIN_ID_AUTO}` needs clear documentation or examples to ensure they are set correctly in the environment. | |
๐ Security concerns | No |
relevant file | ansible/network/files/docker-compose-bootstrap-domain.yml |
suggestion | Consider parameterizing the repeated configurations for state and blocks pruning to reduce redundancy and potential errors. This can be achieved by defining these settings in a separate, included YAML file or by using a templating mechanism to apply the settings across different domains. [important] |
relevant line | "--state-pruning", "archive" |
relevant file | ansible/network/files/docker-compose-domain.yml |
suggestion | Add explicit environment variable definitions or checks to ensure that `${DOMAIN_ID_EVM}` and `${DOMAIN_ID_AUTO}` are defined before they are used in the configurations. This can prevent runtime errors due to undefined variables. [important] |
relevant line | "--domain-id", "${DOMAIN_ID_EVM}" |
relevant file | ansible/network/files/docker-compose-domain.yml |
suggestion | Consider adding health checks for the services defined in the Docker Compose files to ensure that they are running correctly and to facilitate easier debugging and monitoring. [medium] |
relevant line | - "traefik.http.services.archival-node.loadbalancer.server.port=8944" |
Category | Suggestions |
Security |
Enhance security by restricting the CORS policy to specific domains.___ **Consider using a more restrictive setting for--rpc-cors to enhance security. Using 'all' allows any website to make requests to your service, which might expose sensitive data or lead to other security vulnerabilities.** [ansible/network/files/docker-compose-bootstrap-domain.yml [150]](https://github.com/subspace/infra/pull/314/files#diff-665e6416494eeaad41822df0d2d07e9d1d2a386b6b8653e71d7a32b7b2fcc7b8R150-R150) ```diff -"--rpc-cors", "all", +"--rpc-cors", "domain-specific.com", ``` |
Improve security by binding the RPC server to specific network interfaces.___ **The--rpc-listen-on configuration is set to listen on all interfaces (0.0.0.0 ). It's recommended to bind to specific interfaces to reduce the risk of exposing the RPC server to potentially malicious traffic.** [ansible/network/files/docker-compose-bootstrap-domain.yml [151]](https://github.com/subspace/infra/pull/314/files#diff-665e6416494eeaad41822df0d2d07e9d1d2a386b6b8653e71d7a32b7b2fcc7b8R151-R151) ```diff -"--rpc-listen-on", "0.0.0.0:8944", +"--rpc-listen-on", "192.168.1.1:8944", # Assuming 192.168.1.1 is the intended interface ``` | |
Maintainability |
Remove duplicate configuration entries to avoid potential errors.___ **The--state-pruning and --blocks-pruning options are duplicated in the configuration. Ensure that these settings are intended to be repeated, or consider removing the duplicates to avoid configuration errors.** [ansible/network/files/docker-compose-bootstrap-domain.yml [146-159]](https://github.com/subspace/infra/pull/314/files#diff-665e6416494eeaad41822df0d2d07e9d1d2a386b6b8653e71d7a32b7b2fcc7b8R146-R159) ```diff "--state-pruning", "archive", -"--blocks-pruning", "archive", ``` |
Best practice |
Ensure consistent HTTPS redirection by applying the middleware to all routers.___ **The middlewareredirect-https is added to the archival-node-auto router but not to the archival-node router. To maintain consistency and ensure all traffic is redirected to HTTPS, consider adding this middleware to all relevant routers.** [ansible/network/files/docker-compose-domain.yml [97]](https://github.com/subspace/infra/pull/314/files#diff-186921172e472602ae26c578a019543f9775b6522ad5443c9c5815854112216bR97-R97) ```diff +- "traefik.http.routers.archival-node.middlewares=redirect-https" - "traefik.http.routers.archival-node-auto.middlewares=redirect-https" ``` |
Possible issue |
Standardize server ports across configurations to prevent misconfigurations.___ **The load balancer server port configuration forarchival-node-auto is set to 7944, which differs from the standard port 8944 used in other configurations. Verify if this is intentional and consider standardizing ports for uniformity and to avoid misconfiguration.** [ansible/network/files/docker-compose-domain.yml [98]](https://github.com/subspace/infra/pull/314/files#diff-186921172e472602ae26c578a019543f9775b6522ad5443c9c5815854112216bR98-R98) ```diff -- "traefik.http.services.archival-node-auto.loadbalancer.server.port=7944" +- "traefik.http.services.archival-node-auto.loadbalancer.server.port=8944" ``` |
User description
Extend docker compose manifests to include auto domain. This continues on https://github.com/subspace/infra/pull/311 and https://github.com/subspace/infra/pull/312
PR Type
enhancement, configuration changes
Description
Changes walkthrough ๐
docker-compose-bootstrap-domain.yml
Enhance Docker Compose Configurations for Bootstrap Domains
ansible/network/files/docker-compose-bootstrap-domain.yml
EVM and AUTO domains.
docker-compose-domain.yml
Update Traefik Configurations and Domain IDs in Docker Compose
ansible/network/files/docker-compose-domain.yml
AUTO.