From 11886d7246f2c0f98117ab687ba627fcbf574244 Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 15 Nov 2024 18:48:42 +0000 Subject: [PATCH 01/56] Replace iptables with nftables --- Dockerfile | 11 +- node_cli/core/iptables.py | 19 +-- node_cli/core/nftables.py | 277 ++++++++++++++++++++++++++++++++++++ node_cli/core/node.py | 5 +- node_cli/operations/base.py | 9 +- node_cli/utils/helper.py | 12 +- setup.py | 4 +- tests/core/iptables_test.py | 3 +- 8 files changed, 312 insertions(+), 28 deletions(-) create mode 100644 node_cli/core/nftables.py diff --git a/Dockerfile b/Dockerfile index fbd5248c..15e36323 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM python:3.11-buster +FROM python:3.11-bookworm ENV DEBIAN_FRONTEND=noninteractive RUN apt-get update && apt-get install -y software-properties-common @@ -9,7 +9,12 @@ RUN apt-get install -y \ libssl-dev \ libffi-dev \ swig \ - iptables + iptables \ + nftables \ + python3-nftables \ + libxslt-dev \ + kmod + RUN mkdir /app WORKDIR /app @@ -17,6 +22,8 @@ WORKDIR /app COPY . . ENV PATH=/app/buildvenv/bin:$PATH +ENV PYTHONPATH="{PYTHONPATH}:/usr/lib/python3/dist-packages" + RUN python3.11 -m venv /app/buildvenv && \ pip install --upgrade pip && \ pip install wheel setuptools==63.2.0 && \ diff --git a/node_cli/core/iptables.py b/node_cli/core/iptables.py index fe072931..7f3ecd70 100644 --- a/node_cli/core/iptables.py +++ b/node_cli/core/iptables.py @@ -18,17 +18,12 @@ # along with this program. If not, see . import logging -import socket import sys from pathlib import Path -from node_cli.configs import ( - IPTABLES_DIR, - IPTABLES_RULES_STATE_FILEPATH, - ENV, - DEFAULT_SSH_PORT -) -from node_cli.utils.helper import run_cmd +from node_cli.configs import IPTABLES_DIR, IPTABLES_RULES_STATE_FILEPATH, ENV + +from node_cli.utils.helper import get_ssh_port, run_cmd logger = logging.getLogger(__name__) @@ -137,14 +132,6 @@ def drop_all_udp(chain: iptc.Chain) -> None: ensure_rule(chain, r) -def get_ssh_port(ssh_service_name='ssh'): - try: - return socket.getservbyname(ssh_service_name) - except OSError: - logger.exception('Cannot get ssh service port') - return DEFAULT_SSH_PORT - - def allow_ssh(chain: iptc.Chain) -> None: ssh_port = get_ssh_port() accept_incoming(chain, str(ssh_port), 'tcp') diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py new file mode 100644 index 00000000..65d2ba4c --- /dev/null +++ b/node_cli/core/nftables.py @@ -0,0 +1,277 @@ +import json +import logging +from typing import Optional +from dataclasses import dataclass + +import nftables + +from node_cli.utils.helper import get_ssh_port + +logger = logging.getLogger(__name__) + + +@dataclass +class Rule: + chain: str + protocol: str + port: Optional[int] = None + icmp_type: Optional[str] = None + action: str = 'accept' + + +class NFTablesError(Exception): + pass + + +class NFTablesManager: + def __init__(self, family: str = 'inet', table: str = 'filter', chain: str = 'INPUT') -> None: + self.nft = nftables.Nftables() + self.nft.set_json_output(True) + self.family = family + self.table = table + + def execute_cmd(self, json_cmd: dict) -> None: + try: + rc, output, error = self.nft.json_cmd(json_cmd) + if rc != 0: + raise NFTablesError(f'Command failed: {error}') + return output + except Exception as e: + logger.error('Failed to execute command: %s', e) + raise NFTablesError(e) + + def get_chains(self) -> list[str]: + try: + rc, output, error = self.nft.cmd(f'list chains {self.family}') + if rc != 0: + if 'No such file or directory' in error: + return [] + raise NFTablesError(f'Failed to list chains: {error}') + + chains = json.loads(output) + return [item['chain']['name'] for item in chains.get('nftables', []) if 'chain' in item] + except Exception as e: + logger.error('Failed to get chains: %s', e) + return [] + + def flush(self) -> None: + self.run_cmd('flush ruleset') + + def chain_exists(self, chain_name: str) -> bool: + return chain_name in self.get_chains() + + def create_chain_if_not_exists( + self, chain: str, hook: str, priority: int = 0, policy: str = 'accept' + ) -> None: + if not self.chain_exists(chain): + cmd = { + 'nftables': [ + { + 'add': { + 'chain': { + 'family': self.family, + 'table': self.table, + 'name': chain, + 'type': 'filter', + 'hook': hook, + 'priority': priority, + 'policy': policy, + } + } + } + ] + } + self.execute_cmd(cmd) + logger.info('Created new chain: %s', chain) + else: + logger.info('Chain already exists: %s', chain) + + def table_exists(self) -> bool: + try: + rc, output, error = self.nft.cmd(f'list table {self.family} {self.table}') + return rc == 0 + except Exception: + return False + + def create_table_if_not_exists(self) -> None: + """Create table only if it doesn't exist""" + if not self.table_exists(): + cmd = {'nftables': [{'add': {'table': {'family': self.family, 'name': self.table}}}]} + self.execute_cmd(cmd) + logger.info('Created new table: %s', self.table) + else: + logger.info('Table already exists: %s', self.table) + + def get_rules(self, chain: str) -> list[dict]: + """Get existing rules for a chain""" + try: + cmd = f'list chain {self.family} {self.table} {chain}' + rc, output, error = self.nft.cmd(cmd) + if rc != 0: + if 'No such file or directory' in error: + return [] + raise NFTablesError(f'Failed to list rules: {error}') + + rules = json.loads(output) + return [item['rule'] for item in rules.get('nftables', []) if 'rule' in item] + except Exception as e: + logger.error('Failed to get rules: %s', e) + return [] + + def rule_exists(self, chain: str, new_rule_expr: list[dict]) -> bool: + existing_rules = self.get_rules(chain) + + for rule in existing_rules: + if rule.get('expr') == new_rule_expr: + return True + return False + + def add_rule_if_not_exists(self, rule: Rule) -> None: + expr = [] + + if rule.protocol in ['tcp', 'udp']: + if rule.port: + expr.append( + { + 'match': { + 'left': {'payload': {'protocol': rule.protocol, 'field': 'dport'}}, + 'op': '==', + 'right': rule.port, + } + } + ) + elif rule.protocol == 'icmp' and rule.icmp_type: + expr.append( + { + 'match': { + 'left': {'payload': {'protocol': 'icmp', 'field': 'type'}}, + 'op': '==', + 'right': rule.icmp_type, + } + } + ) + + expr.append({rule.action: None}) + + if not self.rule_exists(rule.chain, expr): + cmd = { + 'nftables': [ + { + 'add': { + 'rule': { + 'family': self.family, + 'table': self.table, + 'chain': rule.chain, + 'expr': expr, + } + } + } + ] + } + self.execute_cmd(cmd) + logger.info( + 'Added new rule to chain %s: %s port %s', rule.chain, rule.protocol, rule.port + ) + else: + logger.info( + 'Rule already exists in chain %s: %s port %s', rule.chain, rule.protocol, rule.port + ) + + def add_connection_tracking_rule(self, chain: str) -> None: + expr = [ + { + 'match': { + 'left': {'ct': {'key': 'state'}}, + 'op': 'in', + 'right': ['established', 'related'], + } + }, + {'accept': None}, + ] + + if not self.rule_exists(chain, expr): + cmd = { + 'nftables': [ + { + 'add': { + 'rule': { + 'family': self.family, + 'table': self.table, + 'chain': chain, + 'expr': expr, + } + } + } + ] + } + self.execute_cmd(cmd) + logger.info('Added connection tracking rule to chain %s', chain) + else: + logger.info('Connection tracking rule already exists in chain %s', chain) + + def add_loopback_rule(self, chain) -> None: + expr = [ + {'match': {'left': {'meta': {'key': 'iifname'}}, 'op': '==', 'right': 'lo'}}, + {'accept': None}, + ] + if not self.rule_exists(chain, expr): + json_cmd = { + 'nftables': [ + { + 'add': { + 'rule': { + 'family': 'inet', + 'table': 'filter', + 'chain': 'INPUT', + 'expr': expr, + } + } + } + ] + } + self.execute_cmd(json_cmd) + else: + logger.info('Loopback rule already exists in chain %s', chain) + + def setup_firewall(self) -> None: + """Setup firewall rules""" + try: + self.create_table_if_not_exists() + + base_chains_config = { + 'INPUT': {'hook': 'input', 'policy': 'accept'}, + 'FORWARD': {'hook': 'forward', 'policy': 'drop'}, + 'OUTPUT': {'hook': 'output', 'policy': 'accept'}, + } + + for chain, config in base_chains_config.items(): + self.create_chain_if_not_exists( + chain=chain, hook=config['hook'], policy=config['policy'] + ) + chain = 'INPUT' + + self.add_connection_tracking_rule(chain) + + tcp_ports = [get_ssh_port(), 8080, 443, 53, 3009, 9100] + for port in tcp_ports: + self.add_rule_if_not_exists(Rule(chain=chain, protocol='tcp', port=port)) + + self.add_rule_if_not_exists(Rule(chain=chain, protocol='udp', port=53)) + self.add_loopback_rule(chain=chain) + + icmp_types = ['destination-unreachable', 'source-quench', 'time-exceeded'] + for icmp_type in icmp_types: + self.add_rule_if_not_exists(Rule(chain=chain, protocol='icmp', icmp_type=icmp_type)) + + self.add_rule_if_not_exists(Rule(chain=chain, protocol='tcp', action='drop')) + self.add_rule_if_not_exists(Rule(chain=chain, protocol='udp', action='drop')) + + except Exception as e: + logger.error('Failed to setup firewall: %s', e) + raise + + +def configure_nftables() -> None: + nft_mgr = NFTablesManager() + nft_mgr.setup_firewall() + logger.info('Firewall setup completed successfully') diff --git a/node_cli/core/node.py b/node_cli/core/node.py index fb213a79..83c38c83 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -45,7 +45,8 @@ from node_cli.configs.env import get_env_config from node_cli.configs.cli_logger import LOG_DATA_PATH as CLI_LOG_DATA_PATH -from node_cli.core.iptables import configure_iptables +from node_cli.core.nftables import configure_nftables +# from node_cli.core.iptables import configure_iptables from node_cli.core.host import ( is_node_inited, save_env_params, get_flask_secret_key ) @@ -529,5 +530,5 @@ def run_checks( def configure_firewall_rules() -> None: print('Configuring firewall ...') - configure_iptables() + configure_nftables() print('Done') diff --git a/node_cli/operations/base.py b/node_cli/operations/base.py index ea5ff162..68319fc6 100644 --- a/node_cli/operations/base.py +++ b/node_cli/operations/base.py @@ -46,7 +46,8 @@ from node_cli.operations.docker_lvmpy import lvmpy_install # noqa from node_cli.operations.skale_node import download_skale_node, sync_skale_node, update_images from node_cli.core.checks import CheckType, run_checks as run_host_checks -from node_cli.core.iptables import configure_iptables +# from node_cli.core.iptables import configure_iptables +from node_cli.core.nftables import configure_nftables from node_cli.core.schains import update_node_cli_schain_status, cleanup_sync_datadir from node_cli.utils.docker_utils import ( compose_rm, @@ -114,6 +115,8 @@ def update(env_filepath: str, env: Dict) -> None: backup_old_contracts() download_contracts(env) + configure_nftables() + lvmpy_install(env) generate_nginx_config() @@ -163,7 +166,7 @@ def init(env_filepath: str, env: dict) -> bool: configure_filebeat() configure_flask() - configure_iptables() + configure_nftables() generate_nginx_config() lvmpy_install(env) @@ -320,7 +323,7 @@ def restore(env, backup_path, config_only=False): configure_docker() link_env_file() - configure_iptables() + configure_nftables() lvmpy_install(env) init_shared_space_volume(env['ENV_TYPE']) diff --git a/node_cli/utils/helper.py b/node_cli/utils/helper.py index 71e559cf..39de440a 100644 --- a/node_cli/utils/helper.py +++ b/node_cli/utils/helper.py @@ -21,6 +21,7 @@ import json import os import re +import socket import sys import uuid from urllib.parse import urlparse @@ -54,7 +55,7 @@ ) from node_cli.configs import ( TEXT_FILE, ADMIN_HOST, ADMIN_PORT, HIDE_STREAM_LOG, GLOBAL_SKALE_DIR, - GLOBAL_SKALE_CONF_FILEPATH + GLOBAL_SKALE_CONF_FILEPATH, DEFAULT_SSH_PORT ) from node_cli.configs.routes import get_route from node_cli.utils.global_config import read_g_config, get_system_user @@ -66,7 +67,6 @@ logger = logging.getLogger(__name__) - HOST = f'http://{ADMIN_HOST}:{ADMIN_PORT}' DEFAULT_ERROR_DATA = { @@ -414,3 +414,11 @@ def get_tmp_path(path: str) -> str: base, ext = os.path.splitext(path) salt = uuid.uuid4().hex[:5] return base + salt + '.tmp' + ext + + +def get_ssh_port(ssh_service_name='ssh'): + try: + return socket.getservbyname(ssh_service_name) + except OSError: + logger.exception('Cannot get ssh service port') + return DEFAULT_SSH_PORT diff --git a/setup.py b/setup.py index 4cefd5da..1dea4f75 100644 --- a/setup.py +++ b/setup.py @@ -25,8 +25,8 @@ def find_version(*file_paths): ], 'dev': [ "bumpversion==0.6.0", - "pytest==7.2.2", - "pytest-cov==4.0.0", + "pytest==8.3.2", + "pytest-cov==5.0.0", "twine==4.0.2", "mock==4.0.3", "freezegun==1.2.2" diff --git a/tests/core/iptables_test.py b/tests/core/iptables_test.py index 03ae7c31..eaa097d9 100644 --- a/tests/core/iptables_test.py +++ b/tests/core/iptables_test.py @@ -2,7 +2,8 @@ import mock -from node_cli.core.iptables import allow_ssh, get_ssh_port +from node_cli.core.iptables import allow_ssh +from node_cli.utils.helper import get_ssh_port def test_get_ssh_port(): From dbc886ef3c9cc4cd38bae4ae28e62728194b23e2 Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 15 Nov 2024 18:50:06 +0000 Subject: [PATCH 02/56] Add nftables dependency to GA workflow --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0b1e24ef..1f765edc 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -23,7 +23,7 @@ jobs: - name: Install ubuntu dependencies run: | sudo apt-get update - sudo apt-get install python-setuptools iptables + sudo apt-get install python-setuptools iptables nftables python3-nftables - name: Install python dependencies run: | From 990484d7e93429bd00cbba7e604214fe0287d25c Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 15 Nov 2024 18:56:01 +0000 Subject: [PATCH 03/56] Update GA test pipeline --- .github/workflows/test.yml | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1f765edc..4ea10106 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -36,31 +36,12 @@ jobs: run: | flake8 . - - name: Build binary in Ubuntu 18.04 environment - normal - run: | - mkdir -p ./dist - docker build . -t node-cli-builder - docker run -v /home/ubuntu/dist:/app/dist node-cli-builder scripts/build.sh test test normal - docker rm -f $(docker ps -aq) - - - name: Check build - normal - run: sudo /home/ubuntu/dist/skale-test-Linux-x86_64 - - name: Build binary in Ubuntu 20.04 environment - normal run: | scripts/build.sh test test normal - - name: Check build - sync + - name: Check build - normal run: sudo /home/ubuntu/dist/skale-test-Linux-x86_64 - - name: Build sync binary in Ubuntu 18.04 environment - run: | - mkdir -p ./dist - docker build . -t node-cli-builder - docker run -v /home/ubuntu/dist:/app/dist node-cli-builder scripts/build.sh test test sync - docker rm -f $(docker ps -aq) - - - name: Check build - sync - run: sudo /home/ubuntu/dist/skale-test-Linux-x86_64-sync - name: Build sync binary in Ubuntu 20.04 environment run: | From 13dc9751150ad8aa0a396687782b622659958f5e Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 15 Nov 2024 19:24:24 +0000 Subject: [PATCH 04/56] Switch to ubuntu to 22.04 in GA pipeline --- .github/workflows/test.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4ea10106..35c240e9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -3,7 +3,7 @@ on: [push, pull_request] jobs: test: - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 strategy: matrix: python-version: [3.11] @@ -36,14 +36,14 @@ jobs: run: | flake8 . - - name: Build binary in Ubuntu 20.04 environment - normal + - name: Build binary in 22.04 environment - normal run: | scripts/build.sh test test normal - name: Check build - normal run: sudo /home/ubuntu/dist/skale-test-Linux-x86_64 - - name: Build sync binary in Ubuntu 20.04 environment + - name: Build sync binary in Ubuntu 22.04 environment run: | scripts/build.sh test test sync From 41ee41c126495d2c91bbf2a187b260f0555c0b11 Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 15 Nov 2024 19:54:34 +0000 Subject: [PATCH 05/56] Fix build --- .github/workflows/publish.yml | 2 +- .github/workflows/test.yml | 14 ++++++++++---- Dockerfile | 4 ++-- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 15407c80..b1ec5612 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -13,7 +13,7 @@ jobs: create_release: if: github.event.pull_request.merged name: Create release - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 outputs: upload_url: ${{ steps.create_release.outputs.upload_url }} version: ${{ steps.export_outputs.outputs.version }} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 35c240e9..6dcd06da 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -36,16 +36,22 @@ jobs: run: | flake8 . - - name: Build binary in 22.04 environment - normal + - name: Build binary - sync run: | - scripts/build.sh test test normal + mkdir -p ./dist + docker build . -t node-cli-builder + docker run -v /home/ubuntu/dist:/app/dist node-cli-builder scripts/build.sh test test normal + docker rm -f $(docker ps -aq) - name: Check build - normal run: sudo /home/ubuntu/dist/skale-test-Linux-x86_64 - - name: Build sync binary in Ubuntu 22.04 environment + - name: Build binary - sync run: | - scripts/build.sh test test sync + mkdir -p ./dist + docker build . -t node-cli-builder + docker run -v /home/ubuntu/dist:/app/dist node-cli-builder scripts/build.sh test test sync + docker rm -f $(docker ps -aq) - name: Check build - sync run: sudo /home/ubuntu/dist/skale-test-Linux-x86_64-sync diff --git a/Dockerfile b/Dockerfile index 15e36323..18f8b2f4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,10 +1,10 @@ FROM python:3.11-bookworm ENV DEBIAN_FRONTEND=noninteractive -RUN apt-get update && apt-get install -y software-properties-common -RUN apt-get install -y \ +RUN apt-get update && \ git \ build-essential \ + software-properties-common \ zlib1g-dev \ libssl-dev \ libffi-dev \ From 4badf7a6eeec667a5fdc133ec72aeb24a2eedd09 Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 18 Nov 2024 12:31:23 +0000 Subject: [PATCH 06/56] Fix Dockerfile deps installation --- Dockerfile | 2 +- node_cli/core/nftables.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index 18f8b2f4..97acbed3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ FROM python:3.11-bookworm ENV DEBIAN_FRONTEND=noninteractive -RUN apt-get update && \ +RUN apt-get update && sudo apt install \ git \ build-essential \ software-properties-common \ diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index 65d2ba4c..b295162e 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -55,7 +55,7 @@ def get_chains(self) -> list[str]: return [] def flush(self) -> None: - self.run_cmd('flush ruleset') + self.nft.cmd('flush ruleset') def chain_exists(self, chain_name: str) -> bool: return chain_name in self.get_chains() From 4c0b27b807d1d89f36cf12c078e7ab9f55a4077e Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 18 Nov 2024 12:41:50 +0000 Subject: [PATCH 07/56] Fix Dockerfile --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 97acbed3..c77efb4b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ FROM python:3.11-bookworm ENV DEBIAN_FRONTEND=noninteractive -RUN apt-get update && sudo apt install \ +RUN apt-get update && apt install -y \ git \ build-essential \ software-properties-common \ From e96a667f7ebef8cd0b8fcd9038a3a8399d40d589 Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 18 Nov 2024 13:10:25 +0000 Subject: [PATCH 08/56] Fix tests --- .github/workflows/publish.yml | 9 +++++---- .github/workflows/test.yml | 6 +++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index b1ec5612..abdf44a9 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -26,6 +26,7 @@ jobs: - name: Checkout submodules run: git submodule update --init + - name: Install ubuntu dependencies run: | sudo apt-get update @@ -68,7 +69,7 @@ jobs: strategy: matrix: include: - - os: ubuntu-20.04 + - os: ubuntu-22.04 asset_name: skale-${{ needs.create_release.outputs.version }}-Linux-x86_64 steps: - uses: actions/checkout@v2 @@ -78,7 +79,7 @@ jobs: python-version: 3.11 - name: Install ubuntu dependencies - if: matrix.os == 'ubuntu-20.04' + if: matrix.os == 'ubuntu-22.04' run: | sudo apt-get update @@ -127,7 +128,7 @@ jobs: strategy: matrix: include: - - os: ubuntu-20.04 + - os: ubuntu-22.04 asset_name: skale-${{ needs.create_release.outputs.version }}-Linux-x86_64-sync steps: - uses: actions/checkout@v2 @@ -137,7 +138,7 @@ jobs: python-version: 3.11 - name: Install ubuntu dependencies - if: matrix.os == 'ubuntu-20.04' + if: matrix.os == 'ubuntu-22.04' run: | sudo apt-get update diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6dcd06da..85bd73b5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,11 +32,15 @@ jobs: pip install -e .[dev] pip install --upgrade 'setuptools<45.0.0' + - name: Add nftables to PYTHONPATH + run: | + export PYTHONPATH=${PYTHONPATH}:/usr/lib/python3/dist-packages/ + - name: Lint with flake8 run: | flake8 . - - name: Build binary - sync + - name: Build binary - normal run: | mkdir -p ./dist docker build . -t node-cli-builder From 5da857c4b97ca29c8184f0fb3a7072fb1fb8dabc Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 18 Nov 2024 15:52:01 +0000 Subject: [PATCH 09/56] Fix tests pipeline --- .github/workflows/test.yml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 85bd73b5..b3546fc1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,6 +1,9 @@ name: Test on: [push, pull_request] +env: + PYTHONPATH: ${PYTHONPATH}:/usr/lib/python3/dist-packages/ + jobs: test: runs-on: ubuntu-22.04 @@ -32,10 +35,6 @@ jobs: pip install -e .[dev] pip install --upgrade 'setuptools<45.0.0' - - name: Add nftables to PYTHONPATH - run: | - export PYTHONPATH=${PYTHONPATH}:/usr/lib/python3/dist-packages/ - - name: Lint with flake8 run: | flake8 . @@ -60,5 +59,10 @@ jobs: - name: Check build - sync run: sudo /home/ubuntu/dist/skale-test-Linux-x86_64-sync + - name: Run prepare test build + run: | + scripts/build.sh test test normal + + - name: Run tests run: bash ./scripts/run_tests.sh From a657276702964a71c795a5229810dfc3ad437a63 Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 18 Nov 2024 16:29:48 +0000 Subject: [PATCH 10/56] Remove python-iptables from dependencies --- setup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/setup.py b/setup.py index 1dea4f75..04259d65 100644 --- a/setup.py +++ b/setup.py @@ -64,7 +64,6 @@ def find_version(*file_paths): "GitPython==3.1.41", "packaging==23.0", "python-debian==0.1.49", - "python-iptables==1.0.1", "PyYAML==6.0", "pyOpenSSL==22.0.0", "MarkupSafe==2.1.1", From cab4d91799236fbd2ecab726020827e0438d412b Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 18 Nov 2024 17:09:26 +0000 Subject: [PATCH 11/56] Update Markupsafe to 3.0.2 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 04259d65..cc775aec 100644 --- a/setup.py +++ b/setup.py @@ -66,7 +66,7 @@ def find_version(*file_paths): "python-debian==0.1.49", "PyYAML==6.0", "pyOpenSSL==22.0.0", - "MarkupSafe==2.1.1", + "MarkupSafe==3.0.2", 'Flask==2.3.3', 'itsdangerous==2.1.2', "cryptography==37.0.2", From c4ad6a99d637e6479d9ee88ed490e0456e2b72e8 Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 18 Nov 2024 19:01:12 +0000 Subject: [PATCH 12/56] Bump dependencies --- setup.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/setup.py b/setup.py index cc775aec..f335705d 100644 --- a/setup.py +++ b/setup.py @@ -20,7 +20,7 @@ def find_version(*file_paths): extras_require = { 'linter': [ - "flake8==6.0.0", + "flake8==7.1.1", "isort>=4.2.15,<5.10.2", ], 'dev': [ @@ -50,13 +50,13 @@ def find_version(*file_paths): author_email='support@skalelabs.com', url='https://github.com/skalenetwork/node-cli', install_requires=[ - "click==8.1.3", + "click==8.1.7", "PyInstaller==5.12.0", - "distro==1.4.0", + "distro==1.9.0", "docker==6.0.1", "texttable==1.6.7", "python-dateutil==2.8.2", - "Jinja2==3.1.2", + "Jinja2==3.1.4", "psutil==5.9.4", "python-dotenv==0.21.0", "terminaltables==3.1.10", @@ -65,11 +65,11 @@ def find_version(*file_paths): "packaging==23.0", "python-debian==0.1.49", "PyYAML==6.0", - "pyOpenSSL==22.0.0", + "pyOpenSSL==24.2.1", "MarkupSafe==3.0.2", 'Flask==2.3.3', 'itsdangerous==2.1.2', - "cryptography==37.0.2", + "cryptography==42.0.4", "filelock==3.0.12", 'sh==1.14.2', 'python-crontab==2.6.0' From f5ccf8b3d55eb405885b74e5fc489d00a667aaae Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 18 Nov 2024 19:15:43 +0000 Subject: [PATCH 13/56] Do not fail if cannot import iptc --- node_cli/core/iptables.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node_cli/core/iptables.py b/node_cli/core/iptables.py index 7f3ecd70..c122a7d3 100644 --- a/node_cli/core/iptables.py +++ b/node_cli/core/iptables.py @@ -30,7 +30,7 @@ try: import iptc -except (FileNotFoundError, AttributeError) as err: +except (FileNotFoundError, AttributeError, ModuleNotFoundError) as err: if "pytest" in sys.modules or ENV == 'dev': from collections import namedtuple # hotfix for tests iptc = namedtuple('iptc', ['Chain', 'Rule']) From 13fb2f2c46ac052c2466ae6199802073fab1231a Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 18 Nov 2024 20:17:02 +0000 Subject: [PATCH 14/56] Upgrade setuptools --- .github/workflows/test.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b3546fc1..a102aed9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -26,14 +26,13 @@ jobs: - name: Install ubuntu dependencies run: | sudo apt-get update - sudo apt-get install python-setuptools iptables nftables python3-nftables + sudo apt-get install iptables nftables python3-nftables - name: Install python dependencies run: | python -m pip install --upgrade pip - pip install -e . + pip install setuptools==75.5.0 pip install -e .[dev] - pip install --upgrade 'setuptools<45.0.0' - name: Lint with flake8 run: | From a7545781b8a2d4b575aa772794ca628481d2b3b2 Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 18 Nov 2024 20:36:11 +0000 Subject: [PATCH 15/56] Do not set pythonpath globally --- .github/workflows/test.yml | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a102aed9..a4c95ad4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,9 +1,6 @@ name: Test on: [push, pull_request] -env: - PYTHONPATH: ${PYTHONPATH}:/usr/lib/python3/dist-packages/ - jobs: test: runs-on: ubuntu-22.04 @@ -62,6 +59,7 @@ jobs: run: | scripts/build.sh test test normal - - name: Run tests - run: bash ./scripts/run_tests.sh + run: | + export PYTHONPATH=${PYTHONPATH}:/usr/lib/python3/dist-packages/ + bash ./scripts/run_tests.sh From 9cec75793615f01f50153984512aa81c67b380e7 Mon Sep 17 00:00:00 2001 From: badrogger Date: Wed, 20 Nov 2024 13:22:23 +0000 Subject: [PATCH 16/56] Try with no setuptools --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a4c95ad4..f3945029 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -28,7 +28,7 @@ jobs: - name: Install python dependencies run: | python -m pip install --upgrade pip - pip install setuptools==75.5.0 + # pip install setuptools==75.5.0 pip install -e .[dev] - name: Lint with flake8 From 28016b9af3beb0947b9729f4999229d3efbacaa0 Mon Sep 17 00:00:00 2001 From: badrogger Date: Wed, 20 Nov 2024 13:55:25 +0000 Subject: [PATCH 17/56] Remove libxtwrapper --- main.spec | 3 --- 1 file changed, 3 deletions(-) diff --git a/main.spec b/main.spec index a4dbe395..e3844bc1 100644 --- a/main.spec +++ b/main.spec @@ -2,15 +2,12 @@ import importlib.util -libxtwrapper_path = importlib.util.find_spec('libxtwrapper').origin - block_cipher = None a = Analysis( ['node_cli/main.py'], pathex=['.'], - binaries=[(libxtwrapper_path, '.')], datas=[ ("./text.yml", "data"), ("./datafiles/skaled-ssl-test", "data/datafiles") From 0006b95bb83896504f71120a23e86213ea927857 Mon Sep 17 00:00:00 2001 From: badrogger Date: Wed, 20 Nov 2024 15:29:28 +0000 Subject: [PATCH 18/56] Remove allow_ssh function --- tests/core/iptables_test.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/core/iptables_test.py b/tests/core/iptables_test.py index eaa097d9..2926b917 100644 --- a/tests/core/iptables_test.py +++ b/tests/core/iptables_test.py @@ -2,7 +2,6 @@ import mock -from node_cli.core.iptables import allow_ssh from node_cli.utils.helper import get_ssh_port @@ -11,9 +10,3 @@ def test_get_ssh_port(): assert get_ssh_port('http') == 80 with mock.patch.object(socket, 'getservbyname', side_effect=OSError): assert get_ssh_port() == 22 - - -def test_allow_ssh(): - chain = mock.Mock() - chain.rules = [] - allow_ssh(chain) From 8e976b04945260b88c3206f75b8477775c397253 Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 22 Nov 2024 16:18:34 +0000 Subject: [PATCH 19/56] Add firewall cleanup to update --- node_cli/core/node.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/node_cli/core/node.py b/node_cli/core/node.py index 83c38c83..a8a84e84 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -7,7 +7,6 @@ # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU Affero General Public License as published by # the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. # # This program is distributed in the hope that it will be useful, # but WITHOUT ANY WARRANTY; without even the implied warranty of @@ -42,6 +41,7 @@ SKALE_STATE_DIR, TM_INIT_TIMEOUT ) +from node_cli.cli import __version__ from node_cli.configs.env import get_env_config from node_cli.configs.cli_logger import LOG_DATA_PATH as CLI_LOG_DATA_PATH @@ -67,6 +67,7 @@ ) from node_cli.utils.helper import error_exit, get_request, post_request from node_cli.utils.helper import extract_env_params +from node_cli.utils.meta import get_meta_info from node_cli.utils.texts import Texts from node_cli.utils.exit_codes import CLIExitCodes from node_cli.utils.decorators import ( @@ -74,6 +75,7 @@ check_inited, check_user ) +from node_cli.migrations.focal_to_jammy import migrate as migrate_2_6 logger = logging.getLogger(__name__) @@ -303,6 +305,9 @@ def update(env_filepath: str, pull_config_for_schain: str, unsafe_ok: bool = Fal sync_schains=False, pull_config_for_schain=pull_config_for_schain ) + prev_version = get_meta_info()['version'] + if __version__.startswith('2.6') and prev_version == '2.5.0': + migrate_2_6() update_ok = update_op(env_filepath, env) if update_ok: logger.info('Waiting for containers initialization') From 8dc9b80fe03b7455cba3e96c44528a61f52c332a Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 25 Nov 2024 13:37:19 +0000 Subject: [PATCH 20/56] Apply migrations during update for sync node --- node_cli/core/node.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/node_cli/core/node.py b/node_cli/core/node.py index a8a84e84..23fec8ad 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -222,6 +222,9 @@ def init_sync( @check_user def update_sync(env_filepath: str, unsafe_ok: bool = False) -> None: logger.info('Node update started') + prev_version = get_meta_info()['version'] + if __version__.startswith('2.6') and prev_version == '2.5.0': + migrate_2_6() configure_firewall_rules() env = get_node_env(env_filepath, sync_node=True) update_ok = update_sync_op(env_filepath, env) From d405b3693bd2641ae156c2afd12d5617cdd5cbc1 Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 26 Nov 2024 13:36:26 +0000 Subject: [PATCH 21/56] Bump version --- node_cli/cli/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node_cli/cli/__init__.py b/node_cli/cli/__init__.py index 4b85052a..ed6a4da3 100644 --- a/node_cli/cli/__init__.py +++ b/node_cli/cli/__init__.py @@ -1,4 +1,4 @@ -__version__ = '2.5.0' +__version__ = '2.6.0' if __name__ == "__main__": print(__version__) From af55f11dc43c0f33a6a765fe562f13986f5d8ecf Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 26 Nov 2024 13:36:46 +0000 Subject: [PATCH 22/56] Remove deprecated iptables module --- node_cli/core/iptables.py | 199 -------------------------------------- 1 file changed, 199 deletions(-) delete mode 100644 node_cli/core/iptables.py diff --git a/node_cli/core/iptables.py b/node_cli/core/iptables.py deleted file mode 100644 index c122a7d3..00000000 --- a/node_cli/core/iptables.py +++ /dev/null @@ -1,199 +0,0 @@ -# -*- coding: utf-8 -*- -# -# This file is part of node-cli -# -# Copyright (C) 2021 SKALE Labs -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU Affero General Public License for more details. -# -# You should have received a copy of the GNU Affero General Public License -# along with this program. If not, see . - -import logging -import sys -from pathlib import Path - -from node_cli.configs import IPTABLES_DIR, IPTABLES_RULES_STATE_FILEPATH, ENV - -from node_cli.utils.helper import get_ssh_port, run_cmd - - -logger = logging.getLogger(__name__) - -try: - import iptc -except (FileNotFoundError, AttributeError, ModuleNotFoundError) as err: - if "pytest" in sys.modules or ENV == 'dev': - from collections import namedtuple # hotfix for tests - iptc = namedtuple('iptc', ['Chain', 'Rule']) - else: - logger.error(f'Unable to import iptc due to an error {err}') - - -ALLOWED_INCOMING_TCP_PORTS = [ - '80', # filestorage - '311', # watchdog https - '8080', # http - '443', # https - '53', # dns - '3009', # watchdog http - '9100' # node exporter -] - -ALLOWED_INCOMING_UDP_PORTS = [ - '53' # dns -] - - -def configure_iptables(): - """ - This is the main function used for the initial setup of the firewall rules on the SKALE Node - host machine - """ - logger.info('Configuring iptables...') - if not iptc: - raise ImportError('Unable to import iptc package') - Path(IPTABLES_DIR).mkdir(parents=True, exist_ok=True) - - tb = iptc.Table(iptc.Table.FILTER) - input_chain = iptc.Chain(tb, 'INPUT') - - set_base_policies() - allow_loopback(input_chain) - accept_icmp(input_chain) - allow_conntrack(input_chain) - allow_base_ports(input_chain) - drop_all_tcp(input_chain) - drop_all_udp(input_chain) - save_iptables_rules_state() - - -def save_iptables_rules_state(): - res = run_cmd(['iptables-save']) - plain_rules = res.stdout.decode('utf-8').rstrip() - with open(IPTABLES_RULES_STATE_FILEPATH, 'w') as state_file: - state_file.write(plain_rules) - - -def set_base_policies() -> None: - """Drop all incoming, allow all outcoming, drop all forwarding""" - logger.debug('Setting base policies...') - iptc.easy.set_policy(iptc.Table.FILTER, 'INPUT', 'ACCEPT') - iptc.easy.set_policy(iptc.Table.FILTER, 'OUTPUT', 'ACCEPT') - iptc.easy.set_policy(iptc.Table.FILTER, 'FORWARD', 'DROP') - - -def allow_loopback(chain: iptc.Chain) -> None: - """Allow local loopback services""" - logger.debug('Allowing loopback packages...') - rule = iptc.Rule() - rule.target = iptc.Target(rule, 'ACCEPT') - rule.in_interface = 'lo' - ensure_rule(chain, rule) - - -def allow_conntrack(chain: iptc.Chain) -> None: - """Allow conntrack established connections""" - logger.debug('Allowing conntrack...') - rule = iptc.Rule() - rule.target = iptc.Target(rule, 'ACCEPT') - match = iptc.Match(rule, 'conntrack') - chain = iptc.Chain(iptc.Table(iptc.Table.FILTER), 'INPUT') - match.ctstate = 'RELATED,ESTABLISHED' - rule.add_match(match) - ensure_rule(chain, rule) - - -def drop_all_tcp(chain: iptc.Chain) -> None: - """Drop the rest of tcp connections""" - logger.debug('Adding drop tcp rule ...') - r = iptc.Rule() - t = iptc.Target(r, 'DROP') - r.target = t - r.protocol = 'tcp' - ensure_rule(chain, r) - - -def drop_all_udp(chain: iptc.Chain) -> None: - """Drop the rest of udp connections """ - logger.debug('Adding drop udp rule ...') - r = iptc.Rule() - t = iptc.Target(r, 'DROP') - r.target = t - r.protocol = 'udp' - ensure_rule(chain, r) - - -def allow_ssh(chain: iptc.Chain) -> None: - ssh_port = get_ssh_port() - accept_incoming(chain, str(ssh_port), 'tcp') - - -def allow_base_ports(chain: iptc.Chain) -> None: - logger.info('Configuring ssh port') - allow_ssh(chain) - logger.info('Configuring incoming tcp ports') - for port in ALLOWED_INCOMING_TCP_PORTS: - accept_incoming(chain, port, 'tcp') - logger.info('Configuring incoming udp ports') - for port in ALLOWED_INCOMING_UDP_PORTS: - accept_incoming(chain, port, 'udp') - - -def accept_incoming(chain: iptc.Chain, port: str, protocol: str) -> None: - logger.debug('Going to allow %s traffic from %s port', protocol, port) - rule = iptc.Rule() - rule.protocol = protocol - match = iptc.Match(rule, protocol) - match.dport = port - t = iptc.Target(rule, 'ACCEPT') - rule.target = t - rule.add_match(match) - ensure_rule(chain, rule, insert=True) - - -def accept_icmp(chain: iptc.Chain) -> None: - add_icmp_rule(chain, 'destination-unreachable') - add_icmp_rule(chain, 'source-quench') - add_icmp_rule(chain, 'time-exceeded') - - -def add_icmp_rule(chain: iptc.Chain, icmp_type: str) -> None: - rule = iptc.Rule() - rule.protocol = 'icmp' - match = iptc.Match(rule, 'icmp') - match.icmp_type = icmp_type - t = iptc.Target(rule, 'ACCEPT') - rule.target = t - rule.add_match(match) - ensure_rule(chain, rule) - - -def ensure_rule(chain: iptc.Chain, rule: iptc.Rule, insert=False) -> None: - if rule not in chain.rules: - logger.debug(f'Adding rule: {rule_to_dict(rule)}, chain: {chain.name}') - if insert: - chain.insert_rule(rule) - else: - chain.append_rule(rule) - else: - logger.debug(f'Rule already present: {rule_to_dict(rule)}, chain: {chain.name}') - - -def rule_to_dict(rule): - return { - 'proto': rule.protocol, - 'src': rule.src, - 'dst': rule.dst, - 'in_interface': rule.in_interface, - 'out': rule.out_interface, - 'target': rule.target.name, - } From 8cde8b1d31685ff72fec395a1fb118a99f169cfe Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 26 Nov 2024 13:41:18 +0000 Subject: [PATCH 23/56] Use lowercase chains. Do not import if not needed --- node_cli/core/nftables.py | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index b295162e..431a10aa 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -1,15 +1,25 @@ import json import logging +import sys from typing import Optional from dataclasses import dataclass -import nftables - +from node_cli.configs import ENV from node_cli.utils.helper import get_ssh_port logger = logging.getLogger(__name__) +try: + import nftables +except (FileNotFoundError, AttributeError, ModuleNotFoundError) as err: + if "pytest" in sys.modules or ENV == 'dev': + from collections import namedtuple # hotfix for tests + iptc = namedtuple('nftables', ['Chain', 'Rule']) + else: + logger.error(f'Unable to import iptc due to an error {err}') + + @dataclass class Rule: chain: str @@ -24,11 +34,12 @@ class NFTablesError(Exception): class NFTablesManager: - def __init__(self, family: str = 'inet', table: str = 'filter', chain: str = 'INPUT') -> None: + def __init__(self, family: str = 'inet', table: str = 'filter', chain: str = 'input') -> None: self.nft = nftables.Nftables() self.nft.set_json_output(True) self.family = family self.table = table + self.chain = chain def execute_cmd(self, json_cmd: dict) -> None: try: @@ -222,7 +233,7 @@ def add_loopback_rule(self, chain) -> None: 'rule': { 'family': 'inet', 'table': 'filter', - 'chain': 'INPUT', + 'chain': self.chain, 'expr': expr, } } @@ -239,18 +250,17 @@ def setup_firewall(self) -> None: self.create_table_if_not_exists() base_chains_config = { - 'INPUT': {'hook': 'input', 'policy': 'accept'}, - 'FORWARD': {'hook': 'forward', 'policy': 'drop'}, - 'OUTPUT': {'hook': 'output', 'policy': 'accept'}, + self.chain: {'hook': 'input', 'policy': 'accept'}, + 'forward': {'hook': 'forward', 'policy': 'drop'}, + 'output': {'hook': 'output', 'policy': 'accept'}, } for chain, config in base_chains_config.items(): self.create_chain_if_not_exists( chain=chain, hook=config['hook'], policy=config['policy'] ) - chain = 'INPUT' - self.add_connection_tracking_rule(chain) + self.add_connection_tracking_rule(self.chain) tcp_ports = [get_ssh_port(), 8080, 443, 53, 3009, 9100] for port in tcp_ports: From fe34b6414d828469581e69c79a52625759487c72 Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 26 Nov 2024 13:41:57 +0000 Subject: [PATCH 24/56] Fix migration condition --- node_cli/core/node.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/node_cli/core/node.py b/node_cli/core/node.py index 23fec8ad..f4e54519 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -223,7 +223,7 @@ def init_sync( def update_sync(env_filepath: str, unsafe_ok: bool = False) -> None: logger.info('Node update started') prev_version = get_meta_info()['version'] - if __version__.startswith('2.6') and prev_version == '2.5.0': + if (__version__ == 'test' or __version__.startswith('2.6')) and prev_version == '2.5.0': migrate_2_6() configure_firewall_rules() env = get_node_env(env_filepath, sync_node=True) @@ -300,6 +300,9 @@ def update(env_filepath: str, pull_config_for_schain: str, unsafe_ok: bool = Fal error_msg = 'Cannot update safely' error_exit(error_msg, exit_code=CLIExitCodes.UNSAFE_UPDATE) + prev_version = get_meta_info().version + if (__version__ == 'test' or __version__.startswith('2.6')) and prev_version == '2.5.0': + migrate_2_6() logger.info('Node update started') configure_firewall_rules() env = get_node_env( @@ -308,9 +311,6 @@ def update(env_filepath: str, pull_config_for_schain: str, unsafe_ok: bool = Fal sync_schains=False, pull_config_for_schain=pull_config_for_schain ) - prev_version = get_meta_info()['version'] - if __version__.startswith('2.6') and prev_version == '2.5.0': - migrate_2_6() update_ok = update_op(env_filepath, env) if update_ok: logger.info('Waiting for containers initialization') From 8b871ba00f89b93bb52240e9dfa55f6c747d87c5 Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 26 Nov 2024 13:42:19 +0000 Subject: [PATCH 25/56] Fix core and sync tests --- tests/cli/sync_node_test.py | 4 ++++ tests/core/core_node_test.py | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/cli/sync_node_test.py b/tests/cli/sync_node_test.py index 4f0517a5..5e98cd41 100644 --- a/tests/cli/sync_node_test.py +++ b/tests/cli/sync_node_test.py @@ -22,6 +22,7 @@ import mock import logging +from node_cli.cli import __version__ from node_cli.configs import SKALE_DIR, NODE_DATA_PATH from node_cli.core.node_options import NodeOptions from node_cli.cli.sync_node import _init_sync, _update_sync @@ -46,6 +47,7 @@ def test_init_sync(mocked_g_config): result = run_command(_init_sync, ['./tests/test-env']) node_options = NodeOptions() + print(node_options) assert not node_options.archive assert not node_options.catchup assert not node_options.historic_state @@ -110,6 +112,8 @@ def test_update_sync(mocked_g_config): 'node_cli.core.resources.get_disk_size', return_value=BIG_DISK_SIZE ), mock.patch('node_cli.core.node.configure_firewall_rules'), mock.patch( 'node_cli.utils.decorators.is_node_inited', return_value=True + ), mock.patch( + 'node_cli.core.node.get_meta_info', return_value={'version': __version__} ): result = run_command(_update_sync, ['./tests/test-env', '--yes']) assert result.exit_code == 0 diff --git a/tests/core/core_node_test.py b/tests/core/core_node_test.py index b945f96f..c02cd734 100644 --- a/tests/core/core_node_test.py +++ b/tests/core/core_node_test.py @@ -9,6 +9,8 @@ import pytest import requests +from node_cli.cli import __version__ + from node_cli.configs import NODE_DATA_PATH from node_cli.configs.resource_allocation import RESOURCE_ALLOCATION_FILEPATH from node_cli.core.node import BASE_CONTAINERS_AMOUNT, is_base_containers_alive @@ -168,10 +170,8 @@ def test_update_node(mocked_g_config, resource_file): 'node_cli.utils.helper.post_request', resp_mock ), mock.patch('node_cli.core.resources.get_disk_size', return_value=BIG_DISK_SIZE), mock.patch( 'node_cli.core.host.init_data_dir' - ): - with mock.patch( - 'node_cli.utils.helper.requests.get', return_value=safe_update_api_response() - ): # noqa + ), mock.patch('node_cli.core.node.get_meta_info', return_value={'version': __version__}): + with mock.patch( 'node_cli.utils.helper.requests.get', return_value=safe_update_api_response()): # noqa result = update(env_filepath, pull_config_for_schain=None) assert result is None From faa92b100aba478f1e31902489c94097b056a486 Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 26 Nov 2024 13:42:35 +0000 Subject: [PATCH 26/56] Add missing migration and nftables tests --- tests/core/migration_test.py | 40 ++++++++++ tests/core/nftables_test.py | 141 +++++++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+) create mode 100644 tests/core/migration_test.py create mode 100644 tests/core/nftables_test.py diff --git a/tests/core/migration_test.py b/tests/core/migration_test.py new file mode 100644 index 00000000..29a19680 --- /dev/null +++ b/tests/core/migration_test.py @@ -0,0 +1,40 @@ +import pytest + +from node_cli.migrations.focal_to_jammy import migrate + +from node_cli.utils.helper import run_cmd + + +def add_base_rules(): + run_cmd('iptables -A INPUT -m conntrack --ctstate ESTABLISHED,RELATED -j ACCEPT'.split(' ')) + run_cmd('iptables -A INPUT -i lo -j ACCEPT'.split(' ')) + run_cmd('iptables -A INPUT -p tcp --dport 22 -j ACCEPT'.split(' ')) + run_cmd('iptables -A INPUT -p tcp --dport 8080 -j ACCEPT'.split(' ')) + run_cmd('iptables -A INPUT -p tcp --dport 443 -j ACCEPT'.split(' ')) + run_cmd('iptables -A INPUT -p tcp --dport 53 -j ACCEPT'.split(' ')) + run_cmd('iptables -A INPUT -p udp --dport 53 -j ACCEPT'.split(' ')) + run_cmd('iptables -A INPUT -p tcp --dport 3009 -j ACCEPT'.split(' ')) + # non skale related rule + run_cmd('iptables -A INPUT -p tcp --dport 2222 -j ACCEPT'.split(' ')) + run_cmd('iptables -A INPUT -p tcp --dport 9100 -j ACCEPT'.split(' ')) + run_cmd('iptables -A INPUT -p tcp -j DROP'.split(' ')) + run_cmd('iptables -A INPUT -p udp -j DROP'.split(' ')) + run_cmd('iptables -I INPUT -p icmp --icmp-type destination-unreachable -j ACCEPT'.split(' ')) + run_cmd('iptables -I INPUT -p icmp --icmp-type source-quench -j ACCEPT'.split(' ')) + run_cmd('iptables -I INPUT -p icmp --icmp-type time-exceeded -j ACCEPT'.split(' ')) + + +@pytest.fixture +def base_rules(): + try: + add_base_rules() + yield + finally: + run_cmd(['iptables', '-F']) + + +def test_migration(base_rules): + migrate() + res = run_cmd(['iptables', '-S']) + output = res.stdout.decode('utf-8') + assert output == '-P INPUT ACCEPT\n-P FORWARD ACCEPT\n-P OUTPUT ACCEPT\n-A INPUT -p tcp -m tcp --dport 2222 -j ACCEPT\n' # noqa diff --git a/tests/core/nftables_test.py b/tests/core/nftables_test.py new file mode 100644 index 00000000..e63e8d81 --- /dev/null +++ b/tests/core/nftables_test.py @@ -0,0 +1,141 @@ +import pytest +from unittest.mock import Mock, patch +import json +import nftables + + +from node_cli.core.nftables import NFTablesManager, Rule + + +@pytest.fixture(scope='module') +def nft_manager(): + """Returns a NFTablesManager instance""" + manager = NFTablesManager(family='inet', table='filter') + try: + yield manager + finally: + manager.flush() + + +@pytest.fixture +def mock_nft_output(): + """Fixture for mock nftables output""" + return { + 'nftables': [ + {'chain': {'family': 'inet', 'table': 'filter', 'name': 'INPUT', 'handle': 1}}, + { + 'rule': { + 'family': 'inet', + 'table': 'filter', + 'chain': 'INPUT', + 'handle': 2, + 'expr': [ + { + 'match': { + 'left': {'payload': {'protocol': 'tcp', 'field': 'dport'}}, + 'op': '==', + 'right': 80, + } + }, + {'accept': None}, + ], + } + }, + ] + } + + +def test_init(nft_manager): + """Test initialization""" + assert nft_manager.family == 'inet' + assert nft_manager.table == 'filter' + assert isinstance(nft_manager.nft, nftables.Nftables) + + +@patch('nftables.Nftables.json_cmd') +def test_execute_cmd_success(mock_json_cmd, nft_manager): + """Test successful command execution""" + mock_json_cmd.return_value = (0, '', '') + cmd = {'nftables': [{'add': {'table': {'family': 'inet', 'name': 'filter'}}}]} + + nft_manager.execute_cmd(cmd) + mock_json_cmd.assert_called_once_with(cmd) + + +@patch('nftables.Nftables.json_cmd') +def test_execute_cmd_failure(mock_json_cmd, nft_manager): + """Test command execution failure""" + mock_json_cmd.return_value = (1, '', 'Error message') + cmd = {'nftables': [{'add': {'table': {'family': 'inet', 'name': 'filter'}}}]} + + with pytest.raises(Exception) as exc_info: + nft_manager.execute_cmd(cmd) + assert 'Command failed: Error message' in str(exc_info.value) + + +@patch('nftables.Nftables.cmd') +def test_get_chains(mock_cmd, nft_manager, mock_nft_output): + """Test getting chains""" + mock_cmd.return_value = (0, json.dumps(mock_nft_output), '') + + chains = nft_manager.get_chains() + assert 'INPUT' in chains + mock_cmd.assert_called_once_with('list chains inet') + + +@patch('nftables.Nftables.cmd') +def test_chain_exists(mock_cmd, nft_manager, mock_nft_output): + """Test chain existence check""" + mock_cmd.return_value = (0, json.dumps(mock_nft_output), '') + + assert nft_manager.chain_exists('INPUT') + assert not nft_manager.chain_exists('nonexistent') + + +@patch.object(NFTablesManager, 'execute_cmd') +@patch.object(NFTablesManager, 'chain_exists') +def test_create_chain_if_not_exists(mock_exists, mock_execute, nft_manager): + """Test chain creation""" + mock_exists.return_value = False + + nft_manager.create_chain_if_not_exists('INPUT', 'input') + mock_execute.assert_called_once() + + +@pytest.mark.parametrize( + 'rule_data', + [ + {'chain': 'INPUT', 'protocol': 'tcp', 'port': 80, 'action': 'accept'}, + {'chain': 'INPUT', 'protocol': 'udp', 'port': 53, 'action': 'accept'}, + {'chain': 'INPUT', 'protocol': 'icmp', 'icmp_type': 'echo-request', 'action': 'accept'}, + ], +) +@patch.object(NFTablesManager, 'execute_cmd') +@patch.object(NFTablesManager, 'rule_exists') +def test_add_rule_if_not_exists(mock_exists, mock_execute, nft_manager, rule_data): + """Test rule addition with different types""" + mock_exists.return_value = False + + rule = Rule(**rule_data) + nft_manager.add_rule_if_not_exists(rule) + mock_execute.assert_called_once() + + +@patch.object(NFTablesManager, 'execute_cmd') +def test_setup_firewall(mock_execute, nft_manager): + """Test complete firewall setup""" + with patch.multiple( + NFTablesManager, + table_exists=Mock(return_value=False), + chain_exists=Mock(return_value=False), + rule_exists=Mock(return_value=False), + ): + nft_manager.setup_firewall() + assert mock_execute.called + + +def test_invalid_protocol(nft_manager): + """Test adding rule with invalid protocol""" + rule = Rule(chain='INPUT', protocol='invalid', port=80) + with pytest.raises(Exception): + nft_manager.add_rule_if_not_exists(rule) From d0d1efcecb6af5c56aa044d08518d0e4edbcda2a Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 26 Nov 2024 13:44:06 +0000 Subject: [PATCH 27/56] Extract nftables and migration tests into another action --- .dockerignore | 1 - .github/workflows/test.yml | 5 ++ node_cli/migrations/__init__.py | 0 node_cli/migrations/focal_to_jammy.py | 95 +++++++++++++++++++++++++++ scripts/run_nftables_test.sh | 13 ++++ scripts/run_tests.sh | 2 +- 6 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 node_cli/migrations/__init__.py create mode 100644 node_cli/migrations/focal_to_jammy.py create mode 100755 scripts/run_nftables_test.sh diff --git a/.dockerignore b/.dockerignore index 060cae42..99903e8d 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,4 +1,3 @@ -tests helper-scripts dist build diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f3945029..68ae7b26 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -63,3 +63,8 @@ jobs: run: | export PYTHONPATH=${PYTHONPATH}:/usr/lib/python3/dist-packages/ bash ./scripts/run_tests.sh + + - name: Run nftables tests + run: | + bash ./scripts/run_tests.sh + docker build . -t node-cli-tester && docker run --name tester --cap-add=NET_ADMIN --cap-add=NET_RAW --rm tester scripts/run_nftables_test.sh diff --git a/node_cli/migrations/__init__.py b/node_cli/migrations/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/node_cli/migrations/focal_to_jammy.py b/node_cli/migrations/focal_to_jammy.py new file mode 100644 index 00000000..0f7be46c --- /dev/null +++ b/node_cli/migrations/focal_to_jammy.py @@ -0,0 +1,95 @@ +import logging + +from node_cli.utils.helper import get_ssh_port, run_cmd + +logger = logging.getLogger(__name__) + + +ALLOWED_INCOMING_TCP_PORTS = [ + '80', # filestorage + '311', # watchdog https + '8080', # http + '443', # https + '53', # dns + '3009', # watchdog http + '9100' # node exporter +] + +ALLOWED_INCOMING_UDP_PORTS = [ + '53' # dns +] + + +def remove_tcp_rules(ssh_port: int) -> None: + tcp_rule_template = 'iptables -{} INPUT -p tcp -m tcp --dport {} -j ACCEPT' + for tcp_port in [*ALLOWED_INCOMING_TCP_PORTS, ssh_port]: + check_cmd = tcp_rule_template.format('C', tcp_port).split(' ') + remove_cmd = tcp_rule_template.format('D', tcp_port).split(' ') + result = run_cmd(check_cmd, check_code=False) + if result.returncode == 0: + result = run_cmd(remove_cmd) + + +def remove_udp_rules() -> None: + udp_rule_template = 'iptables -{} INPUT -p udp -m udp --dport {} -j ACCEPT' + for udp_port in [*ALLOWED_INCOMING_UDP_PORTS]: + check_cmd = udp_rule_template.format('C', udp_port).split(' ') + remove_cmd = udp_rule_template.format('D', udp_port).split(' ') + result = run_cmd(check_cmd, check_code=False) + if result.returncode == 0: + result = run_cmd(remove_cmd) + + +def remove_loopback_rules() -> None: + loopback_rule_template = 'iptables -{} INPUT -i lo -j ACCEPT' + check_cmd = loopback_rule_template.format('C').split(' ') + remove_cmd = loopback_rule_template.format('D').split(' ') + result = run_cmd(check_cmd, check_code=False) + if result.returncode == 0: + result = run_cmd(remove_cmd) + + +def remove_icmp_rules() -> None: + icmp_rule_template = 'iptables -{} INPUT -p icmp -m icmp --icmp-type {} -j ACCEPT' + for icmp_type in [3, 4, 11]: + check_cmd = icmp_rule_template.format('C', icmp_type).split(' ') + remove_cmd = icmp_rule_template.format('D', icmp_type).split(' ') + result = run_cmd(check_cmd, check_code=False) + if result.returncode == 0: + result = run_cmd(remove_cmd) + + +def remove_conntrack_rules() -> None: + track_rule_template = 'iptables -{} INPUT -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT' + check_cmd = track_rule_template.format('C').split(' ') + remove_cmd = track_rule_template.format('D').split(' ') + result = run_cmd(check_cmd, check_code=False) + if result.returncode == 0: + result = run_cmd(remove_cmd) + + +def remove_drop_rules() -> None: + drop_rule_template = 'iptables -{} INPUT -p {} -j DROP' + protocols = ['tcp', 'udp'] + for proto in protocols: + check_cmd = drop_rule_template.format('C', proto).split(' ') + remove_cmd = drop_rule_template.format('D', proto).split(' ') + result = run_cmd(check_cmd, check_code=False) + if result.returncode == 0: + result = run_cmd(remove_cmd) + + +def remove_old_firewall_rules(ssh_port: int) -> None: + remove_drop_rules() + remove_conntrack_rules() + remove_loopback_rules() + remove_udp_rules() + remove_tcp_rules(ssh_port) + remove_icmp_rules() + + +def migrate() -> None: + ssh_port = get_ssh_port() + logger.info('Running migration from focal to jammy') + remove_old_firewall_rules(ssh_port) + logger.info('Migration from focal to jammy completed') diff --git a/scripts/run_nftables_test.sh b/scripts/run_nftables_test.sh new file mode 100755 index 00000000..c8740093 --- /dev/null +++ b/scripts/run_nftables_test.sh @@ -0,0 +1,13 @@ +#!/usr/bin/env bash + +DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" +PROJECT_DIR=$(dirname $DIR) +echo $PROJECT_DIR +ls -altr $PROJECT_DIR + +LVMPY_LOG_DIR="$PROJECT_DIR/tests/" \ + HIDE_STREAM_LOG=true \ + TEST_HOME_DIR="$PROJECT_DIR/tests/" \ + GLOBAL_SKALE_DIR="$PROJECT_DIR/tests/etc/skale" \ + DOTENV_FILEPATH='tests/test-env' \ + py.test -vv tests/core/migration_test.py tests/core/nftables_test.py $@ diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index edc7fa73..1bea2824 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -8,4 +8,4 @@ LVMPY_LOG_DIR="$PROJECT_DIR/tests/" \ TEST_HOME_DIR="$PROJECT_DIR/tests/" \ GLOBAL_SKALE_DIR="$PROJECT_DIR/tests/etc/skale" \ DOTENV_FILEPATH='tests/test-env' \ - py.test --cov=$PROJECT_DIR/ tests/ $@ + py.test -vv --cov=$PROJECT_DIR/ --ignore=tests/core/nftables_test.py --ignore=tests/core/migration_test.py tests $@ From 3ce7856e933603d2622b2b2468ec903ad17c39d6 Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 26 Nov 2024 13:52:50 +0000 Subject: [PATCH 28/56] Reduce log level of pytest --- scripts/run_nftables_test.sh | 2 +- scripts/run_tests.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/run_nftables_test.sh b/scripts/run_nftables_test.sh index c8740093..6599c55c 100755 --- a/scripts/run_nftables_test.sh +++ b/scripts/run_nftables_test.sh @@ -10,4 +10,4 @@ LVMPY_LOG_DIR="$PROJECT_DIR/tests/" \ TEST_HOME_DIR="$PROJECT_DIR/tests/" \ GLOBAL_SKALE_DIR="$PROJECT_DIR/tests/etc/skale" \ DOTENV_FILEPATH='tests/test-env' \ - py.test -vv tests/core/migration_test.py tests/core/nftables_test.py $@ + py.test -v tests/core/migration_test.py tests/core/nftables_test.py $@ diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index 1bea2824..daf30179 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -8,4 +8,4 @@ LVMPY_LOG_DIR="$PROJECT_DIR/tests/" \ TEST_HOME_DIR="$PROJECT_DIR/tests/" \ GLOBAL_SKALE_DIR="$PROJECT_DIR/tests/etc/skale" \ DOTENV_FILEPATH='tests/test-env' \ - py.test -vv --cov=$PROJECT_DIR/ --ignore=tests/core/nftables_test.py --ignore=tests/core/migration_test.py tests $@ + py.test -v --cov=$PROJECT_DIR/ --ignore=tests/core/nftables_test.py --ignore=tests/core/migration_test.py tests $@ From 2c8930b8bf7b02974b778f46d5c204280b1f6dd2 Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 26 Nov 2024 19:20:12 +0000 Subject: [PATCH 29/56] Use ip family by default --- node_cli/core/nftables.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index 431a10aa..d3c000b6 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -34,7 +34,7 @@ class NFTablesError(Exception): class NFTablesManager: - def __init__(self, family: str = 'inet', table: str = 'filter', chain: str = 'input') -> None: + def __init__(self, family: str = 'ip', table: str = 'filter', chain: str = 'input') -> None: self.nft = nftables.Nftables() self.nft.set_json_output(True) self.family = family @@ -83,7 +83,7 @@ def create_chain_if_not_exists( 'family': self.family, 'table': self.table, 'name': chain, - 'type': 'filter', + 'type': self.table, 'hook': hook, 'priority': priority, 'policy': policy, @@ -231,8 +231,8 @@ def add_loopback_rule(self, chain) -> None: { 'add': { 'rule': { - 'family': 'inet', - 'table': 'filter', + 'family': self.family, + 'table': self.table, 'chain': self.chain, 'expr': expr, } @@ -250,7 +250,7 @@ def setup_firewall(self) -> None: self.create_table_if_not_exists() base_chains_config = { - self.chain: {'hook': 'input', 'policy': 'accept'}, + 'input': {'hook': 'input', 'policy': 'accept'}, 'forward': {'hook': 'forward', 'policy': 'drop'}, 'output': {'hook': 'output', 'policy': 'accept'}, } @@ -264,21 +264,21 @@ def setup_firewall(self) -> None: tcp_ports = [get_ssh_port(), 8080, 443, 53, 3009, 9100] for port in tcp_ports: - self.add_rule_if_not_exists(Rule(chain=chain, protocol='tcp', port=port)) + self.add_rule_if_not_exists(Rule(chain=self.chain, protocol='tcp', port=port)) - self.add_rule_if_not_exists(Rule(chain=chain, protocol='udp', port=53)) - self.add_loopback_rule(chain=chain) + self.add_rule_if_not_exists(Rule(chain=self.chain, protocol='udp', port=53)) + self.add_loopback_rule(chain=self.chain) icmp_types = ['destination-unreachable', 'source-quench', 'time-exceeded'] for icmp_type in icmp_types: - self.add_rule_if_not_exists(Rule(chain=chain, protocol='icmp', icmp_type=icmp_type)) + self.add_rule_if_not_exists(Rule(chain=self.chain, protocol='icmp', icmp_type=icmp_type)) - self.add_rule_if_not_exists(Rule(chain=chain, protocol='tcp', action='drop')) - self.add_rule_if_not_exists(Rule(chain=chain, protocol='udp', action='drop')) + self.add_rule_if_not_exists(Rule(chain=self.chain, protocol='tcp', action='drop')) + self.add_rule_if_not_exists(Rule(chain=self.chain, protocol='udp', action='drop')) except Exception as e: logger.error('Failed to setup firewall: %s', e) - raise + raise NFTablesError(e) def configure_nftables() -> None: From 202d68e2949704be46d718c1dc81bd93e96fe7e4 Mon Sep 17 00:00:00 2001 From: badrogger Date: Thu, 28 Nov 2024 22:11:00 +0000 Subject: [PATCH 30/56] Remove duplicate setup_firewall execution --- node_cli/operations/base.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/node_cli/operations/base.py b/node_cli/operations/base.py index 68319fc6..2a27807b 100644 --- a/node_cli/operations/base.py +++ b/node_cli/operations/base.py @@ -46,8 +46,6 @@ from node_cli.operations.docker_lvmpy import lvmpy_install # noqa from node_cli.operations.skale_node import download_skale_node, sync_skale_node, update_images from node_cli.core.checks import CheckType, run_checks as run_host_checks -# from node_cli.core.iptables import configure_iptables -from node_cli.core.nftables import configure_nftables from node_cli.core.schains import update_node_cli_schain_status, cleanup_sync_datadir from node_cli.utils.docker_utils import ( compose_rm, @@ -115,8 +113,6 @@ def update(env_filepath: str, env: Dict) -> None: backup_old_contracts() download_contracts(env) - configure_nftables() - lvmpy_install(env) generate_nginx_config() @@ -166,7 +162,6 @@ def init(env_filepath: str, env: dict) -> bool: configure_filebeat() configure_flask() - configure_nftables() generate_nginx_config() lvmpy_install(env) @@ -323,7 +318,6 @@ def restore(env, backup_path, config_only=False): configure_docker() link_env_file() - configure_nftables() lvmpy_install(env) init_shared_space_volume(env['ENV_TYPE']) From d0ef85d8341831bbf4a7bb0b04a1d3f3d4fe85fc Mon Sep 17 00:00:00 2001 From: badrogger Date: Thu, 28 Nov 2024 22:11:43 +0000 Subject: [PATCH 31/56] Add firewall configuration to restore --- node_cli/core/node.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/node_cli/core/node.py b/node_cli/core/node.py index f4e54519..bf88b47f 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -174,6 +174,8 @@ def restore(backup_path, env_filepath, no_snapshot=False, config_only=False): logger.info('Adding BACKUP_RUN to env ...') env['BACKUP_RUN'] = 'True' # should be str + configure_firewall_rules() + restored_ok = restore_op(env, backup_path, config_only=config_only) if not restored_ok: error_exit( From d420f5852c328117acc1ce5d2a6ea7b12e90e7e2 Mon Sep 17 00:00:00 2001 From: badrogger Date: Thu, 28 Nov 2024 22:12:10 +0000 Subject: [PATCH 32/56] Configure chain properly --- node_cli/core/nftables.py | 61 ++++++++++++++++++++++++++++++++++----- 1 file changed, 54 insertions(+), 7 deletions(-) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index d3c000b6..26e1bb8e 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -34,7 +34,7 @@ class NFTablesError(Exception): class NFTablesManager: - def __init__(self, family: str = 'ip', table: str = 'filter', chain: str = 'input') -> None: + def __init__(self, family: str = 'inet', table: str = 'firewall', chain: str = 'input') -> None: self.nft = nftables.Nftables() self.nft.set_json_output(True) self.family = family @@ -83,9 +83,9 @@ def create_chain_if_not_exists( 'family': self.family, 'table': self.table, 'name': chain, - 'type': self.table, + 'type': 'filter', 'hook': hook, - 'priority': priority, + 'prio': priority, 'policy': policy, } } @@ -93,7 +93,7 @@ def create_chain_if_not_exists( ] } self.execute_cmd(cmd) - logger.info('Created new chain: %s', chain) + logger.info('Created new chain: %s %s', chain, cmd) else: logger.info('Chain already exists: %s', chain) @@ -137,6 +137,44 @@ def rule_exists(self, chain: str, new_rule_expr: list[dict]) -> bool: return True return False + def add_drop_rule_if_node_exists(self, protocol: str) -> None: + expr = [ + { + "match": { + "op": "==", + "left": { + "payload": { + "protocol": "ip", + "field": "protocol" + } + }, + "right": protocol + } + }, + {'counter': None}, + {"drop": None} + ] + if not self.rule_exists(self.chain, expr): + # cmd = { + # 'nftables': [ + # { + # 'add': { + # 'rule': { + # 'family': self.family, + # 'table': self.table, + # 'chain': self.chain, + # 'expr': expr, + # } + # } + # } + # ] + # } + # self.execute_cmd(cmd) + cmd = f'add rule {self.family} {self.table} {self.chain} ip protocol {protocol} counter drop' + logger.info('CMD %s', cmd) + self.nft.cmd(cmd) + logger.info('Added drop rule for %s', protocol) + def add_rule_if_not_exists(self, rule: Rule) -> None: expr = [] @@ -162,6 +200,7 @@ def add_rule_if_not_exists(self, rule: Rule) -> None: } ) + expr.append({'counter': None}) expr.append({rule.action: None}) if not self.rule_exists(rule.chain, expr): @@ -197,6 +236,7 @@ def add_connection_tracking_rule(self, chain: str) -> None: 'right': ['established', 'related'], } }, + {'counter': None}, {'accept': None}, ] @@ -223,6 +263,7 @@ def add_connection_tracking_rule(self, chain: str) -> None: def add_loopback_rule(self, chain) -> None: expr = [ {'match': {'left': {'meta': {'key': 'iifname'}}, 'op': '==', 'right': 'lo'}}, + {'counter': None}, {'accept': None}, ] if not self.rule_exists(chain, expr): @@ -271,10 +312,16 @@ def setup_firewall(self) -> None: icmp_types = ['destination-unreachable', 'source-quench', 'time-exceeded'] for icmp_type in icmp_types: - self.add_rule_if_not_exists(Rule(chain=self.chain, protocol='icmp', icmp_type=icmp_type)) + self.add_rule_if_not_exists( + Rule( + chain=self.chain, + protocol='icmp', + icmp_type=icmp_type + ) + ) - self.add_rule_if_not_exists(Rule(chain=self.chain, protocol='tcp', action='drop')) - self.add_rule_if_not_exists(Rule(chain=self.chain, protocol='udp', action='drop')) + self.add_drop_rule_if_node_exists(protocol='tcp') + self.add_drop_rule_if_node_exists(protocol='udp') except Exception as e: logger.error('Failed to setup firewall: %s', e) From 7a7ffe1537475684f28c023cde9de2693967a696 Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 29 Nov 2024 20:10:06 +0000 Subject: [PATCH 33/56] Fix duplicates --- node_cli/core/nftables.py | 40 ++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index 26e1bb8e..fe3dfc77 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -133,7 +133,12 @@ def rule_exists(self, chain: str, new_rule_expr: list[dict]) -> bool: existing_rules = self.get_rules(chain) for rule in existing_rules: - if rule.get('expr') == new_rule_expr: + expr = rule.get('expr') + for i, statement in enumerate(expr): + if 'counter' in statement: + expr[i] = {'counter': None} + rule['counter'] = None + if expr == new_rule_expr: return True return False @@ -155,24 +160,21 @@ def add_drop_rule_if_node_exists(self, protocol: str) -> None: {"drop": None} ] if not self.rule_exists(self.chain, expr): - # cmd = { - # 'nftables': [ - # { - # 'add': { - # 'rule': { - # 'family': self.family, - # 'table': self.table, - # 'chain': self.chain, - # 'expr': expr, - # } - # } - # } - # ] - # } - # self.execute_cmd(cmd) - cmd = f'add rule {self.family} {self.table} {self.chain} ip protocol {protocol} counter drop' - logger.info('CMD %s', cmd) - self.nft.cmd(cmd) + cmd = { + 'nftables': [ + { + 'add': { + 'rule': { + 'family': self.family, + 'table': self.table, + 'chain': self.chain, + 'expr': expr, + } + } + } + ] + } + self.execute_cmd(cmd) logger.info('Added drop rule for %s', protocol) def add_rule_if_not_exists(self, rule: Rule) -> None: From 418f70a9d770eab50836b9f6009b720225a04c01 Mon Sep 17 00:00:00 2001 From: badrogger Date: Fri, 29 Nov 2024 20:23:49 +0000 Subject: [PATCH 34/56] Switch to new version of docker compose --- node_cli/core/checks.py | 12 ++++++------ node_cli/utils/docker_utils.py | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/node_cli/core/checks.py b/node_cli/core/checks.py index e094fe99..b4902b57 100644 --- a/node_cli/core/checks.py +++ b/node_cli/core/checks.py @@ -415,26 +415,26 @@ def docker_api(self) -> CheckResult: @preinstall def docker_compose(self) -> CheckResult: - name = 'docker-compose' - cmd = shutil.which('docker-compose') + name = 'docker' + cmd = shutil.which('docker') if cmd is None: - info = 'No such command: "docker-compose"' + info = 'No such command: "docker"' return self._failed(name=name, info=info) v_cmd_result = run_cmd( - ['docker-compose', '-v'], + ['docker compose', 'version'], check_code=False, separate_stderr=True ) output = v_cmd_result.stdout.decode('utf-8').rstrip() if v_cmd_result.returncode != 0: - info = f'Checking docker-compose version failed with: {output}' + info = f'Checking docker compose version failed with: {output}' return self._failed(name=name, info=output) actual_version = output.split(',')[0].split()[-1].strip() expected_version = self.requirements['docker-compose'] - info = f'Expected docker-compose version {expected_version}, actual {actual_version}' # noqa + info = f'Expected docker compose version {expected_version}, actual {actual_version}' # noqa if version_parse(actual_version) < version_parse(expected_version): return self._failed(name=name, info=info) else: diff --git a/node_cli/utils/docker_utils.py b/node_cli/utils/docker_utils.py index 2f5e56a8..f625e282 100644 --- a/node_cli/utils/docker_utils.py +++ b/node_cli/utils/docker_utils.py @@ -247,7 +247,7 @@ def compose_rm(env={}, sync_node: bool = False): compose_path = get_compose_path(sync_node) run_cmd( cmd=( - 'docker-compose', + 'docker compose', '-f', compose_path, 'down', '-t', str(COMPOSE_SHUTDOWN_TIMEOUT), @@ -261,7 +261,7 @@ def compose_pull(sync_node: bool = False): logger.info('Pulling compose containers') compose_path = get_compose_path(sync_node) run_cmd( - cmd=('docker-compose', '-f', compose_path, 'pull'), + cmd=('docker compose', '-f', compose_path, 'pull'), env={ 'SKALE_DIR': SKALE_DIR } @@ -272,7 +272,7 @@ def compose_build(sync_node: bool = False): logger.info('Building compose containers') compose_path = get_compose_path(sync_node) run_cmd( - cmd=('docker-compose', '-f', compose_path, 'build'), + cmd=('docker compose', '-f', compose_path, 'build'), env={ 'SKALE_DIR': SKALE_DIR } @@ -280,11 +280,11 @@ def compose_build(sync_node: bool = False): def get_up_compose_cmd(services): - return ('docker-compose', '-f', COMPOSE_PATH, 'up', '-d', *services) + return ('docker compose', '-f', COMPOSE_PATH, 'up', '-d', *services) def get_up_compose_sync_cmd(): - return ('docker-compose', '-f', SYNC_COMPOSE_PATH, 'up', '-d') + return ('docker compose', '-f', SYNC_COMPOSE_PATH, 'up', '-d') def get_compose_path(sync_node: bool) -> str: From 240f7b3c98d28462704cd27f1cb992be95d3de1d Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 3 Dec 2024 18:23:47 +0000 Subject: [PATCH 35/56] Improve run_nftables_tests script --- scripts/run_nftables_test.sh | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/scripts/run_nftables_test.sh b/scripts/run_nftables_test.sh index 6599c55c..98f3b874 100755 --- a/scripts/run_nftables_test.sh +++ b/scripts/run_nftables_test.sh @@ -1,13 +1,18 @@ #!/usr/bin/env bash +set -ea -DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" -PROJECT_DIR=$(dirname $DIR) -echo $PROJECT_DIR -ls -altr $PROJECT_DIR +# DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" +# PROJECT_DIR=$(dirname $DIR) +# export DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" + +docker rm -f ncli-tester || true +docker build . -t ncli-tester +docker run \ + -e LVMPY_LOG_DIR="$PROJECT_DIR/tests/" \ + -e HIDE_STREAM_LOG=true \ + -e TEST_HOME_DIR="$PROJECT_DIR/tests/" \ + -e GLOBAL_SKALE_DIR="$PROJECT_DIR/tests/etc/skale" \ + -e DOTENV_FILEPATH='tests/test-env' \ + --cap-add=NET_ADMIN --cap-add=NET_RAW \ + --name ncli-tester ncli-tester py.test -v tests/core/migration_test.py tests/core/nftables_test.py $@ -LVMPY_LOG_DIR="$PROJECT_DIR/tests/" \ - HIDE_STREAM_LOG=true \ - TEST_HOME_DIR="$PROJECT_DIR/tests/" \ - GLOBAL_SKALE_DIR="$PROJECT_DIR/tests/etc/skale" \ - DOTENV_FILEPATH='tests/test-env' \ - py.test -v tests/core/migration_test.py tests/core/nftables_test.py $@ From 0609568ca4385b0a7ef95d10ab753f9195467fc6 Mon Sep 17 00:00:00 2001 From: badrogger Date: Wed, 4 Dec 2024 16:54:34 +0000 Subject: [PATCH 36/56] Fix tests --- node_cli/core/node.py | 1 - scripts/run_nftables_test.sh | 2 +- scripts/run_tests.sh | 2 +- tests/cli/node_test.py | 80 ++++++++++++++++++++++++++---------- tests/core/core_node_test.py | 10 +++-- 5 files changed, 67 insertions(+), 28 deletions(-) diff --git a/node_cli/core/node.py b/node_cli/core/node.py index bf88b47f..a805cb8d 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -46,7 +46,6 @@ from node_cli.configs.cli_logger import LOG_DATA_PATH as CLI_LOG_DATA_PATH from node_cli.core.nftables import configure_nftables -# from node_cli.core.iptables import configure_iptables from node_cli.core.host import ( is_node_inited, save_env_params, get_flask_secret_key ) diff --git a/scripts/run_nftables_test.sh b/scripts/run_nftables_test.sh index 98f3b874..98827e3c 100755 --- a/scripts/run_nftables_test.sh +++ b/scripts/run_nftables_test.sh @@ -14,5 +14,5 @@ docker run \ -e GLOBAL_SKALE_DIR="$PROJECT_DIR/tests/etc/skale" \ -e DOTENV_FILEPATH='tests/test-env' \ --cap-add=NET_ADMIN --cap-add=NET_RAW \ - --name ncli-tester ncli-tester py.test -v tests/core/migration_test.py tests/core/nftables_test.py $@ + --name ncli-tester ncli-tester py.test tests/core/migration_test.py tests/core/nftables_test.py $@ diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index daf30179..97676b9a 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -8,4 +8,4 @@ LVMPY_LOG_DIR="$PROJECT_DIR/tests/" \ TEST_HOME_DIR="$PROJECT_DIR/tests/" \ GLOBAL_SKALE_DIR="$PROJECT_DIR/tests/etc/skale" \ DOTENV_FILEPATH='tests/test-env' \ - py.test -v --cov=$PROJECT_DIR/ --ignore=tests/core/nftables_test.py --ignore=tests/core/migration_test.py tests $@ + py.test --cov=$PROJECT_DIR/ --ignore=tests/core/nftables_test.py --ignore=tests/core/migration_test.py tests $@ diff --git a/tests/cli/node_test.py b/tests/cli/node_test.py index 9c86057c..bd2b19c3 100644 --- a/tests/cli/node_test.py +++ b/tests/cli/node_test.py @@ -40,6 +40,7 @@ ) from node_cli.utils.exit_codes import CLIExitCodes from node_cli.utils.helper import init_default_logger +from node_cli.utils.meta import CliMeta from tests.helper import ( response_mock, @@ -84,7 +85,9 @@ def test_register_node_with_error(resource_alloc, mocked_g_config): ) assert result.exit_code == 3 assert ( - result.output == f'Command failed with following errors:\n--------------------------------------------------\nStrange error\n--------------------------------------------------\nYou can find more info in {G_CONF_HOME}.skale/.skale-cli-log/debug-node-cli.log\n') # noqa + result.output + == f'Command failed with following errors:\n--------------------------------------------------\nStrange error\n--------------------------------------------------\nYou can find more info in {G_CONF_HOME}.skale/.skale-cli-log/debug-node-cli.log\n' + ) # noqa def test_register_node_with_prompted_ip(resource_alloc, mocked_g_config): @@ -98,7 +101,10 @@ def test_register_node_with_prompted_ip(resource_alloc, mocked_g_config): input='0.0.0.0\n', ) assert result.exit_code == 0 - assert result.output == 'Enter node public IP: 0.0.0.0\nNode registered in SKALE manager.\nFor more info run < skale node info >\n' # noqa + assert ( + result.output + == 'Enter node public IP: 0.0.0.0\nNode registered in SKALE manager.\nFor more info run < skale node info >\n' + ) # noqa def test_register_node_with_default_port(resource_alloc, mocked_g_config): @@ -112,7 +118,10 @@ def test_register_node_with_default_port(resource_alloc, mocked_g_config): input='0.0.0.0\n', ) assert result.exit_code == 0 - assert result.output == 'Enter node public IP: 0.0.0.0\nNode registered in SKALE manager.\nFor more info run < skale node info >\n' # noqa + assert ( + result.output + == 'Enter node public IP: 0.0.0.0\nNode registered in SKALE manager.\nFor more info run < skale node info >\n' + ) # noqa def test_register_with_no_alloc(mocked_g_config): @@ -125,7 +134,10 @@ def test_register_with_no_alloc(mocked_g_config): input='0.0.0.0\n', ) assert result.exit_code == 8 - assert result.output == f"Enter node public IP: 0.0.0.0\nCommand failed with following errors:\n--------------------------------------------------\nNode hasn't been inited before.\nYou should run < skale node init >\n--------------------------------------------------\nYou can find more info in {G_CONF_HOME}.skale/.skale-cli-log/debug-node-cli.log\n" # noqa + assert ( + result.output + == f"Enter node public IP: 0.0.0.0\nCommand failed with following errors:\n--------------------------------------------------\nNode hasn't been inited before.\nYou should run < skale node init >\n--------------------------------------------------\nYou can find more info in {G_CONF_HOME}.skale/.skale-cli-log/debug-node-cli.log\n" + ) # noqa def test_node_info_node_info(): @@ -150,7 +162,10 @@ def test_node_info_node_info(): resp_mock = response_mock(requests.codes.ok, json_data={'payload': payload, 'status': 'ok'}) result = run_command_mock('node_cli.utils.helper.requests.get', resp_mock, node_info) assert result.exit_code == 0 - assert result.output == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: Active\n--------------------------------------------------\n' # noqa + assert ( + result.output + == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: Active\n--------------------------------------------------\n' + ) # noqa def test_node_info_node_info_not_created(): @@ -200,7 +215,10 @@ def test_node_info_node_info_frozen(): resp_mock = response_mock(requests.codes.ok, json_data={'payload': payload, 'status': 'ok'}) result = run_command_mock('node_cli.utils.helper.requests.get', resp_mock, node_info) assert result.exit_code == 0 - assert result.output == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: Frozen\n--------------------------------------------------\n' # noqa + assert ( + result.output + == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: Frozen\n--------------------------------------------------\n' + ) # noqa def test_node_info_node_info_left(): @@ -225,7 +243,10 @@ def test_node_info_node_info_left(): resp_mock = response_mock(requests.codes.ok, json_data={'payload': payload, 'status': 'ok'}) result = run_command_mock('node_cli.utils.helper.requests.get', resp_mock, node_info) assert result.exit_code == 0 - assert result.output == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: Left\n--------------------------------------------------\n' # noqa + assert ( + result.output + == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: Left\n--------------------------------------------------\n' + ) # noqa def test_node_info_node_info_leaving(): @@ -250,7 +271,10 @@ def test_node_info_node_info_leaving(): resp_mock = response_mock(requests.codes.ok, json_data={'payload': payload, 'status': 'ok'}) result = run_command_mock('node_cli.utils.helper.requests.get', resp_mock, node_info) assert result.exit_code == 0 - assert result.output == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: Leaving\n--------------------------------------------------\n' # noqa + assert ( + result.output + == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: Leaving\n--------------------------------------------------\n' + ) # noqa def test_node_info_node_info_in_maintenance(): @@ -275,7 +299,10 @@ def test_node_info_node_info_in_maintenance(): resp_mock = response_mock(requests.codes.ok, json_data={'payload': payload, 'status': 'ok'}) result = run_command_mock('node_cli.utils.helper.requests.get', resp_mock, node_info) assert result.exit_code == 0 - assert result.output == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: In Maintenance\n--------------------------------------------------\n' # noqa + assert ( + result.output + == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: In Maintenance\n--------------------------------------------------\n' + ) # noqa def test_node_signature(): @@ -306,7 +333,10 @@ def test_restore(mocked_g_config): 'subprocess.run', new=subprocess_run_mock ), patch('node_cli.core.resources.get_disk_size', return_value=BIG_DISK_SIZE), patch( 'node_cli.utils.decorators.is_node_inited', return_value=False - ): + ), patch( + 'node_cli.core.node.get_meta_info', + return_value=CliMeta(version='2.4.0', config_stream='3.0.2'), + ), patch('node_cli.core.node.configure_firewall_rules'): result = run_command(restore_node, [backup_path, './tests/test-env']) assert result.exit_code == 0 assert 'Node is restored from backup\n' in result.output # noqa @@ -325,7 +355,10 @@ def test_restore_no_snapshot(mocked_g_config): 'subprocess.run', new=subprocess_run_mock ), patch('node_cli.core.resources.get_disk_size', return_value=BIG_DISK_SIZE), patch( 'node_cli.utils.decorators.is_node_inited', return_value=False - ): + ), patch( + 'node_cli.core.node.get_meta_info', + return_value=CliMeta(version='2.4.0', config_stream='3.0.2'), + ), patch('node_cli.core.node.configure_firewall_rules'): result = run_command(restore_node, [backup_path, './tests/test-env', '--no-snapshot']) assert result.exit_code == 0 assert 'Node is restored from backup\n' in result.output # noqa @@ -392,7 +425,7 @@ def test_turn_on_maintenance_off(mocked_g_config): 'node_cli.core.node.get_flask_secret_key' ), mock.patch('node_cli.core.node.turn_on_op'), mock.patch( 'node_cli.core.node.is_base_containers_alive' - ), mock.patch('node_cli.core.node.is_node_inited', return_value=True): + ), mock.patch('node_cli.utils.decorators.is_node_inited', return_value=True): result = run_command_mock( 'node_cli.utils.helper.requests.post', resp_mock, @@ -424,14 +457,17 @@ def test_set_domain_name(): def test_node_version(meta_file_v2): - result = run_command(version) - assert result.exit_code == 0 - assert result.output == '--------------------------------------------------\nVersion: 0.1.1\nConfig Stream: develop\nLvmpy stream: 1.1.2\n--------------------------------------------------\n' # noqa + with mock.patch('node_cli.utils.decorators.is_node_inited', return_value=True): + result = run_command(version) + assert result.exit_code == 0 + assert ( + result.output + == '--------------------------------------------------\nVersion: 0.1.1\nConfig Stream: develop\nLvmpy stream: 1.1.2\n--------------------------------------------------\n' + ) # noqa - result = run_command(version, ['--json']) - print(repr(result.output)) - assert result.exit_code == 0 - assert ( - result.output - == "{'version': '0.1.1', 'config_stream': 'develop', 'docker_lvmpy_stream': '1.1.2'}\n" - ) # noqa + result = run_command(version, ['--json']) + assert result.exit_code == 0 + assert ( + result.output + == "{'version': '0.1.1', 'config_stream': 'develop', 'docker_lvmpy_stream': '1.1.2'}\n" + ) # noqa diff --git a/tests/core/core_node_test.py b/tests/core/core_node_test.py index c02cd734..aeb35600 100644 --- a/tests/core/core_node_test.py +++ b/tests/core/core_node_test.py @@ -9,12 +9,11 @@ import pytest import requests -from node_cli.cli import __version__ - from node_cli.configs import NODE_DATA_PATH from node_cli.configs.resource_allocation import RESOURCE_ALLOCATION_FILEPATH from node_cli.core.node import BASE_CONTAINERS_AMOUNT, is_base_containers_alive from node_cli.core.node import init, pack_dir, update, is_update_safe, repair_sync +from node_cli.utils.meta import CliMeta from tests.helper import response_mock, safe_update_api_response, subprocess_run_mock from tests.resources_test import BIG_DISK_SIZE @@ -170,7 +169,12 @@ def test_update_node(mocked_g_config, resource_file): 'node_cli.utils.helper.post_request', resp_mock ), mock.patch('node_cli.core.resources.get_disk_size', return_value=BIG_DISK_SIZE), mock.patch( 'node_cli.core.host.init_data_dir' - ), mock.patch('node_cli.core.node.get_meta_info', return_value={'version': __version__}): + ), mock.patch( + 'node_cli.core.node.get_meta_info', + return_value=CliMeta( + version='2.5.0', config_stream='3.0.2' + ) + ): with mock.patch( 'node_cli.utils.helper.requests.get', return_value=safe_update_api_response()): # noqa result = update(env_filepath, pull_config_for_schain=None) assert result is None From 498511a91668913a2bf3698df9d2db4ad0b300f1 Mon Sep 17 00:00:00 2001 From: badrogger Date: Wed, 4 Dec 2024 17:44:39 +0000 Subject: [PATCH 37/56] Fix linter --- tests/cli/node_test.py | 50 +++++++++--------------------------------- 1 file changed, 10 insertions(+), 40 deletions(-) diff --git a/tests/cli/node_test.py b/tests/cli/node_test.py index bd2b19c3..0f41aaed 100644 --- a/tests/cli/node_test.py +++ b/tests/cli/node_test.py @@ -84,10 +84,7 @@ def test_register_node_with_error(resource_alloc, mocked_g_config): ['--name', 'test-node2', '--ip', '0.0.0.0', '--port', '80', '-d', 'skale.test'], ) assert result.exit_code == 3 - assert ( - result.output - == f'Command failed with following errors:\n--------------------------------------------------\nStrange error\n--------------------------------------------------\nYou can find more info in {G_CONF_HOME}.skale/.skale-cli-log/debug-node-cli.log\n' - ) # noqa + assert (result.output == f'Command failed with following errors:\n--------------------------------------------------\nStrange error\n--------------------------------------------------\nYou can find more info in {G_CONF_HOME}.skale/.skale-cli-log/debug-node-cli.log\n') # noqa def test_register_node_with_prompted_ip(resource_alloc, mocked_g_config): @@ -101,10 +98,7 @@ def test_register_node_with_prompted_ip(resource_alloc, mocked_g_config): input='0.0.0.0\n', ) assert result.exit_code == 0 - assert ( - result.output - == 'Enter node public IP: 0.0.0.0\nNode registered in SKALE manager.\nFor more info run < skale node info >\n' - ) # noqa + assert (result.output == 'Enter node public IP: 0.0.0.0\nNode registered in SKALE manager.\nFor more info run < skale node info >\n') # noqa def test_register_node_with_default_port(resource_alloc, mocked_g_config): @@ -118,10 +112,7 @@ def test_register_node_with_default_port(resource_alloc, mocked_g_config): input='0.0.0.0\n', ) assert result.exit_code == 0 - assert ( - result.output - == 'Enter node public IP: 0.0.0.0\nNode registered in SKALE manager.\nFor more info run < skale node info >\n' - ) # noqa + assert (result.output == 'Enter node public IP: 0.0.0.0\nNode registered in SKALE manager.\nFor more info run < skale node info >\n') # noqa def test_register_with_no_alloc(mocked_g_config): @@ -134,10 +125,7 @@ def test_register_with_no_alloc(mocked_g_config): input='0.0.0.0\n', ) assert result.exit_code == 8 - assert ( - result.output - == f"Enter node public IP: 0.0.0.0\nCommand failed with following errors:\n--------------------------------------------------\nNode hasn't been inited before.\nYou should run < skale node init >\n--------------------------------------------------\nYou can find more info in {G_CONF_HOME}.skale/.skale-cli-log/debug-node-cli.log\n" - ) # noqa + assert (result.output == f"Enter node public IP: 0.0.0.0\nCommand failed with following errors:\n--------------------------------------------------\nNode hasn't been inited before.\nYou should run < skale node init >\n--------------------------------------------------\nYou can find more info in {G_CONF_HOME}.skale/.skale-cli-log/debug-node-cli.log\n") # noqa def test_node_info_node_info(): @@ -162,10 +150,7 @@ def test_node_info_node_info(): resp_mock = response_mock(requests.codes.ok, json_data={'payload': payload, 'status': 'ok'}) result = run_command_mock('node_cli.utils.helper.requests.get', resp_mock, node_info) assert result.exit_code == 0 - assert ( - result.output - == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: Active\n--------------------------------------------------\n' - ) # noqa + assert (result.output == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: Active\n--------------------------------------------------\n') # noqa def test_node_info_node_info_not_created(): @@ -215,10 +200,7 @@ def test_node_info_node_info_frozen(): resp_mock = response_mock(requests.codes.ok, json_data={'payload': payload, 'status': 'ok'}) result = run_command_mock('node_cli.utils.helper.requests.get', resp_mock, node_info) assert result.exit_code == 0 - assert ( - result.output - == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: Frozen\n--------------------------------------------------\n' - ) # noqa + assert (result.output == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: Frozen\n--------------------------------------------------\n') # noqa def test_node_info_node_info_left(): @@ -243,10 +225,7 @@ def test_node_info_node_info_left(): resp_mock = response_mock(requests.codes.ok, json_data={'payload': payload, 'status': 'ok'}) result = run_command_mock('node_cli.utils.helper.requests.get', resp_mock, node_info) assert result.exit_code == 0 - assert ( - result.output - == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: Left\n--------------------------------------------------\n' - ) # noqa + assert (result.output == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: Left\n--------------------------------------------------\n') # noqa def test_node_info_node_info_leaving(): @@ -271,10 +250,7 @@ def test_node_info_node_info_leaving(): resp_mock = response_mock(requests.codes.ok, json_data={'payload': payload, 'status': 'ok'}) result = run_command_mock('node_cli.utils.helper.requests.get', resp_mock, node_info) assert result.exit_code == 0 - assert ( - result.output - == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: Leaving\n--------------------------------------------------\n' - ) # noqa + assert (result.output == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: Leaving\n--------------------------------------------------\n') # noqa def test_node_info_node_info_in_maintenance(): @@ -299,10 +275,7 @@ def test_node_info_node_info_in_maintenance(): resp_mock = response_mock(requests.codes.ok, json_data={'payload': payload, 'status': 'ok'}) result = run_command_mock('node_cli.utils.helper.requests.get', resp_mock, node_info) assert result.exit_code == 0 - assert ( - result.output - == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: In Maintenance\n--------------------------------------------------\n' - ) # noqa + assert (result.output == '--------------------------------------------------\nNode info\nName: test\nID: 32\nIP: 0.0.0.0\nPublic IP: 1.1.1.1\nPort: 10001\nDomain name: skale.test\nStatus: In Maintenance\n--------------------------------------------------\n') # noqa def test_node_signature(): @@ -460,10 +433,7 @@ def test_node_version(meta_file_v2): with mock.patch('node_cli.utils.decorators.is_node_inited', return_value=True): result = run_command(version) assert result.exit_code == 0 - assert ( - result.output - == '--------------------------------------------------\nVersion: 0.1.1\nConfig Stream: develop\nLvmpy stream: 1.1.2\n--------------------------------------------------\n' - ) # noqa + assert (result.output == '--------------------------------------------------\nVersion: 0.1.1\nConfig Stream: develop\nLvmpy stream: 1.1.2\n--------------------------------------------------\n') # noqa result = run_command(version, ['--json']) assert result.exit_code == 0 From 3f9de98784be3626c08f1a6a5a992a7cf36540d4 Mon Sep 17 00:00:00 2001 From: badrogger Date: Wed, 4 Dec 2024 19:01:55 +0000 Subject: [PATCH 38/56] Fix GA pipline --- .github/workflows/test.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 68ae7b26..27abe86d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -66,5 +66,4 @@ jobs: - name: Run nftables tests run: | - bash ./scripts/run_tests.sh - docker build . -t node-cli-tester && docker run --name tester --cap-add=NET_ADMIN --cap-add=NET_RAW --rm tester scripts/run_nftables_test.sh + scripts/run_nftables_test.sh From f25d4b647efeb549d2a2710531fc8902a74f47f7 Mon Sep 17 00:00:00 2001 From: badrogger Date: Wed, 4 Dec 2024 20:38:15 +0000 Subject: [PATCH 39/56] Optional monitoring ports --- node_cli/cli/node.py | 7 +- node_cli/core/checks.py | 2 +- node_cli/core/nftables.py | 8 +- node_cli/core/node.py | 177 ++++++++++-------------------- node_cli/operations/base.py | 8 +- node_cli/operations/skale_node.py | 7 +- node_cli/utils/docker_utils.py | 24 ++-- scripts/run_nftables_test.sh | 4 - 8 files changed, 83 insertions(+), 154 deletions(-) diff --git a/node_cli/cli/node.py b/node_cli/cli/node.py index ff781249..fe826a58 100644 --- a/node_cli/cli/node.py +++ b/node_cli/cli/node.py @@ -239,12 +239,13 @@ def check(network): run_checks(network) -@node.command(help='Reconfigure iptables rules') +@node.command(help='Reconfigure nftables rules') +@click.option('--monitoring', is_flag=True) @click.option('--yes', is_flag=True, callback=abort_if_false, expose_value=False, prompt='Are you sure you want to reconfigure firewall rules?') -def configure_firewall(): - configure_firewall_rules() +def configure_firewall(monitoring): + configure_firewall_rules(enable_monitoring=monitoring) @node.command(help='Show node version information') diff --git a/node_cli/core/checks.py b/node_cli/core/checks.py index b4902b57..a5e85c70 100644 --- a/node_cli/core/checks.py +++ b/node_cli/core/checks.py @@ -422,7 +422,7 @@ def docker_compose(self) -> CheckResult: return self._failed(name=name, info=info) v_cmd_result = run_cmd( - ['docker compose', 'version'], + ['docker', 'compose', 'version'], check_code=False, separate_stderr=True ) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index fe3dfc77..b7db5bb7 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -287,7 +287,7 @@ def add_loopback_rule(self, chain) -> None: else: logger.info('Loopback rule already exists in chain %s', chain) - def setup_firewall(self) -> None: + def setup_firewall(self, enable_monitoring: bool = False) -> None: """Setup firewall rules""" try: self.create_table_if_not_exists() @@ -306,6 +306,8 @@ def setup_firewall(self) -> None: self.add_connection_tracking_rule(self.chain) tcp_ports = [get_ssh_port(), 8080, 443, 53, 3009, 9100] + if enable_monitoring: + tcp_ports.extend([8080, 9100]) for port in tcp_ports: self.add_rule_if_not_exists(Rule(chain=self.chain, protocol='tcp', port=port)) @@ -330,7 +332,7 @@ def setup_firewall(self) -> None: raise NFTablesError(e) -def configure_nftables() -> None: +def configure_nftables(enable_monitoring: bool = False) -> None: nft_mgr = NFTablesManager() - nft_mgr.setup_firewall() + nft_mgr.setup_firewall(enable_monitoring=enable_monitoring) logger.info('Firewall setup completed successfully') diff --git a/node_cli/core/node.py b/node_cli/core/node.py index a805cb8d..b728b5dc 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -39,16 +39,14 @@ SCHAINS_MNT_DIR_SYNC, SKALE_DIR, SKALE_STATE_DIR, - TM_INIT_TIMEOUT + TM_INIT_TIMEOUT, ) from node_cli.cli import __version__ from node_cli.configs.env import get_env_config from node_cli.configs.cli_logger import LOG_DATA_PATH as CLI_LOG_DATA_PATH from node_cli.core.nftables import configure_nftables -from node_cli.core.host import ( - is_node_inited, save_env_params, get_flask_secret_key -) +from node_cli.core.host import is_node_inited, save_env_params, get_flask_secret_key from node_cli.core.checks import run_checks as run_host_checks from node_cli.core.resources import update_resource_allocation from node_cli.operations import ( @@ -59,21 +57,24 @@ restore_op, init_sync_op, repair_sync_op, - update_sync_op + update_sync_op, ) from node_cli.utils.print_formatters import ( - print_failed_requirements_checks, print_node_cmd_error, print_node_info + print_failed_requirements_checks, + print_node_cmd_error, + print_node_info, +) +from node_cli.utils.helper import ( + error_exit, + get_request, + post_request, + extract_env_params, + str_to_bool, ) -from node_cli.utils.helper import error_exit, get_request, post_request -from node_cli.utils.helper import extract_env_params from node_cli.utils.meta import get_meta_info from node_cli.utils.texts import Texts from node_cli.utils.exit_codes import CLIExitCodes -from node_cli.utils.decorators import ( - check_not_inited, - check_inited, - check_user -) +from node_cli.utils.decorators import check_not_inited, check_inited, check_user from node_cli.migrations.focal_to_jammy import migrate as migrate_2_6 @@ -87,6 +88,7 @@ class NodeStatuses(Enum): """This class contains possible node statuses""" + ACTIVE = 0 LEAVING = 1 FROZEN = 2 @@ -107,9 +109,7 @@ def is_update_safe() -> bool: @check_inited @check_user -def register_node(name, p2p_ip, - public_ip, port, domain_name): - +def register_node(name, p2p_ip, public_ip, port, domain_name): if not is_node_inited(): print(TEXTS['node']['not_inited']) return @@ -120,13 +120,9 @@ def register_node(name, p2p_ip, 'ip': p2p_ip, 'publicIP': public_ip, 'port': port, - 'domain_name': domain_name + 'domain_name': domain_name, } - status, payload = post_request( - blueprint=BLUEPRINT_NAME, - method='register', - json=json_data - ) + status, payload = post_request(blueprint=BLUEPRINT_NAME, method='register', json=json_data) if status == 'ok': msg = TEXTS['node']['registered'] logger.info(msg) @@ -142,20 +138,16 @@ def init(env_filepath): env = get_node_env(env_filepath) if env is None: return - configure_firewall_rules() + + enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) + configure_firewall_rules(enable_monitoring=enable_monitoring) inited_ok = init_op(env_filepath, env) if not inited_ok: - error_exit( - 'Init operation failed', - exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR - ) + error_exit('Init operation failed', exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR) logger.info('Waiting for containers initialization') time.sleep(TM_INIT_TIMEOUT) if not is_base_containers_alive(): - error_exit( - 'Containers are not running', - exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR - ) + error_exit('Containers are not running', exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR) logger.info('Generating resource allocation file ...') update_resource_allocation(env['ENV_TYPE']) logger.info('Init procedure finished') @@ -173,49 +165,31 @@ def restore(backup_path, env_filepath, no_snapshot=False, config_only=False): logger.info('Adding BACKUP_RUN to env ...') env['BACKUP_RUN'] = 'True' # should be str - configure_firewall_rules() + enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) + configure_firewall_rules(enable_monitoring=enable_monitoring) restored_ok = restore_op(env, backup_path, config_only=config_only) if not restored_ok: - error_exit( - 'Restore operation failed', - exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR - ) + error_exit('Restore operation failed', exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR) time.sleep(RESTORE_SLEEP_TIMEOUT) logger.info('Generating resource allocation file ...') update_resource_allocation(env['ENV_TYPE']) print('Node is restored from backup') -def init_sync( - env_filepath: str, - archive: bool, - historic_state: bool, - snapshot_from: str -) -> None: - configure_firewall_rules() +def init_sync(env_filepath: str, archive: bool, historic_state: bool, snapshot_from: str) -> None: env = get_node_env(env_filepath, sync_node=True) if env is None: return - inited_ok = init_sync_op( - env_filepath, - env, - archive, - historic_state, - snapshot_from - ) + enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) + configure_firewall_rules(enable_monitoring=enable_monitoring) + inited_ok = init_sync_op(env_filepath, env, archive, historic_state, snapshot_from) if not inited_ok: - error_exit( - 'Init operation failed', - exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR - ) + error_exit('Init operation failed', exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR) logger.info('Waiting for containers initialization') time.sleep(TM_INIT_TIMEOUT) if not is_base_containers_alive(sync_node=True): - error_exit( - 'Containers are not running', - exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR - ) + error_exit('Containers are not running', exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR) logger.info('Sync node initialized successfully') @@ -226,8 +200,9 @@ def update_sync(env_filepath: str, unsafe_ok: bool = False) -> None: prev_version = get_meta_info()['version'] if (__version__ == 'test' or __version__.startswith('2.6')) and prev_version == '2.5.0': migrate_2_6() - configure_firewall_rules() env = get_node_env(env_filepath, sync_node=True) + enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) + configure_firewall_rules(enable_monitoring=enable_monitoring) update_ok = update_sync_op(env_filepath, env) if update_ok: logger.info('Waiting for containers initialization') @@ -242,36 +217,23 @@ def update_sync(env_filepath: str, unsafe_ok: bool = False) -> None: @check_inited @check_user -def repair_sync( - archive: bool, - historic_state: bool, - snapshot_from: str -) -> None: - +def repair_sync(archive: bool, historic_state: bool, snapshot_from: str) -> None: env_params = extract_env_params(INIT_ENV_FILEPATH, sync_node=True) schain_name = env_params['SCHAIN_NAME'] repair_sync_op( schain_name=schain_name, archive=archive, historic_state=historic_state, - snapshot_from=snapshot_from + snapshot_from=snapshot_from, ) logger.info('Schain was started from scratch') def get_node_env( - env_filepath, - inited_node=False, - sync_schains=None, - pull_config_for_schain=None, - sync_node=False + env_filepath, inited_node=False, sync_schains=None, pull_config_for_schain=None, sync_node=False ): if env_filepath is not None: - env_params = extract_env_params( - env_filepath, - sync_node=sync_node, - raise_for_status=True - ) + env_params = extract_env_params(env_filepath, sync_node=sync_node, raise_for_status=True) save_env_params(env_filepath) else: env_params = extract_env_params(INIT_ENV_FILEPATH, sync_node=sync_node) @@ -282,7 +244,7 @@ def get_node_env( 'SCHAINS_MNT_DIR': mnt_dir, 'FILESTORAGE_MAPPING': FILESTORAGE_MAPPING, 'SKALE_LIB_PATH': SKALE_STATE_DIR, - **env_params + **env_params, } if inited_node and not sync_node: flask_secret_key = get_flask_secret_key() @@ -305,13 +267,14 @@ def update(env_filepath: str, pull_config_for_schain: str, unsafe_ok: bool = Fal if (__version__ == 'test' or __version__.startswith('2.6')) and prev_version == '2.5.0': migrate_2_6() logger.info('Node update started') - configure_firewall_rules() env = get_node_env( env_filepath, inited_node=True, sync_schains=False, - pull_config_for_schain=pull_config_for_schain + pull_config_for_schain=pull_config_for_schain, ) + enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) + configure_firewall_rules(enable_monitoring=enable_monitoring) update_ok = update_op(env_filepath, env) if update_ok: logger.info('Waiting for containers initialization') @@ -326,11 +289,7 @@ def update(env_filepath: str, pull_config_for_schain: str, unsafe_ok: bool = Fal def get_node_signature(validator_id): params = {'validator_id': validator_id} - status, payload = get_request( - blueprint=BLUEPRINT_NAME, - method='signature', - params=params - ) + status, payload = get_request(blueprint=BLUEPRINT_NAME, method='signature', params=params) if status == 'ok': return payload['signature'] else: @@ -343,7 +302,7 @@ def backup(path): def get_backup_filename(): - time = datetime.datetime.utcnow().strftime("%Y-%m-%d-%H-%M-%S") + time = datetime.datetime.utcnow().strftime('%Y-%m-%d-%H-%M-%S') return f'{BACKUP_ARCHIVE_NAME}-{time}.tar.gz' @@ -390,20 +349,13 @@ def create_backup_archive(backup_filepath): print('Creating backup archive...') cli_log_path = CLI_LOG_DATA_PATH container_log_path = LOG_PATH - pack_dir( - SKALE_DIR, - backup_filepath, - exclude=(cli_log_path, container_log_path) - ) + pack_dir(SKALE_DIR, backup_filepath, exclude=(cli_log_path, container_log_path)) print(f'Backup archive succesfully created {backup_filepath}') def set_maintenance_mode_on(): print('Setting maintenance mode on...') - status, payload = post_request( - blueprint=BLUEPRINT_NAME, - method='maintenance-on' - ) + status, payload = post_request(blueprint=BLUEPRINT_NAME, method='maintenance-on') if status == 'ok': msg = TEXTS['node']['maintenance_on'] logger.info(msg) @@ -416,10 +368,7 @@ def set_maintenance_mode_on(): def set_maintenance_mode_off(): print('Setting maintenance mode off...') - status, payload = post_request( - blueprint=BLUEPRINT_NAME, - method='maintenance-off' - ) + status, payload = post_request(blueprint=BLUEPRINT_NAME, method='maintenance-off') if status == 'ok': msg = TEXTS['node']['maintenance_off'] logger.info(msg) @@ -459,18 +408,13 @@ def turn_on(maintenance_off, sync_schains, env_file): def is_base_containers_alive(sync_node: bool = False): dclient = docker.from_env() containers = dclient.containers.list() - skale_containers = list(filter( - lambda c: c.name.startswith('skale_'), containers - )) + skale_containers = list(filter(lambda c: c.name.startswith('skale_'), containers)) containers_amount = SYNC_BASE_CONTAINERS_AMOUNT if sync_node else BASE_CONTAINERS_AMOUNT return len(skale_containers) >= containers_amount def get_node_info_plain(): - status, payload = get_request( - blueprint=BLUEPRINT_NAME, - method='info' - ) + status, payload = get_request(blueprint=BLUEPRINT_NAME, method='info') if status == 'ok': return payload['node_info'] else: @@ -484,10 +428,7 @@ def get_node_info(format): elif node_info['status'] == NodeStatuses.NOT_CREATED.value: print(TEXTS['service']['node_not_registered']) else: - print_node_info( - node_info, - get_node_status(int(node_info['status'])) - ) + print_node_info(node_info, get_node_status(int(node_info['status']))) def get_node_status(status): @@ -499,11 +440,7 @@ def get_node_status(status): def set_domain_name(domain_name): print(f'Setting new domain name: {domain_name}') status, payload = post_request( - blueprint=BLUEPRINT_NAME, - method='set-domain-name', - json={ - 'domain_name': domain_name - } + blueprint=BLUEPRINT_NAME, method='set-domain-name', json={'domain_name': domain_name} ) if status == 'ok': msg = TEXTS['node']['domain_name_changed'] @@ -516,7 +453,7 @@ def set_domain_name(domain_name): def run_checks( network: str = 'mainnet', container_config_path: str = CONTAINER_CONFIG_PATH, - disk: Optional[str] = None + disk: Optional[str] = None, ) -> None: if not is_node_inited(): print(TEXTS['node']['not_inited']) @@ -525,11 +462,7 @@ def run_checks( if disk is None: env = get_env_config() disk = env['DISK_MOUNTPOINT'] - failed_checks = run_host_checks( - disk, - network, - container_config_path - ) + failed_checks = run_host_checks(disk, network, container_config_path) if not failed_checks: print('Requirements checking succesfully finished!') else: @@ -537,7 +470,7 @@ def run_checks( print_failed_requirements_checks(failed_checks) -def configure_firewall_rules() -> None: +def configure_firewall_rules(enable_monitoring: bool = False) -> None: print('Configuring firewall ...') - configure_nftables() + configure_nftables(enable_monitoring=enable_monitoring) print('Done') diff --git a/node_cli/operations/base.py b/node_cli/operations/base.py index 2a27807b..2a2867a9 100644 --- a/node_cli/operations/base.py +++ b/node_cli/operations/base.py @@ -140,7 +140,7 @@ def update(env_filepath: str, env: Dict) -> None: distro.id(), distro.version() ) - update_images(env.get('CONTAINER_CONFIGS_DIR') != '') + update_images(env=env) compose_up(env) return True @@ -175,7 +175,7 @@ def init(env_filepath: str, env: dict) -> bool: distro.version() ) update_resource_allocation(env_type=env['ENV_TYPE']) - update_images(env.get('CONTAINER_CONFIGS_DIR') != '') + update_images(env=env) compose_up(env) return True @@ -231,7 +231,7 @@ def init_sync( if snapshot_from: update_node_cli_schain_status(schain_name, snapshot_from=snapshot_from) - update_images(env.get('CONTAINER_CONFIGS_DIR') != '', sync_node=True) + update_images(env=env, sync_node=True) compose_up(env, sync_node=True) return True @@ -273,7 +273,7 @@ def update_sync(env_filepath: str, env: Dict) -> bool: distro.id(), distro.version() ) - update_images(env.get('CONTAINER_CONFIGS_DIR') != '', sync_node=True) + update_images(env=env, sync_node=True) compose_up(env, sync_node=True) return True diff --git a/node_cli/operations/skale_node.py b/node_cli/operations/skale_node.py index b3745070..d91e4765 100644 --- a/node_cli/operations/skale_node.py +++ b/node_cli/operations/skale_node.py @@ -35,11 +35,12 @@ logger = logging.getLogger(__name__) -def update_images(local: bool = False, sync_node: bool = False) -> None: +def update_images(env: dict, sync_node: bool = False) -> None: + local = env.get('CONTAINER_CONFIGS_DIR') != '' if local: - compose_build(sync_node=sync_node) + compose_build(env=env, sync_node=sync_node) else: - compose_pull(sync_node=sync_node) + compose_pull(env=env, sync_node=sync_node) def download_skale_node(stream: Optional[str], src: Optional[str]) -> None: diff --git a/node_cli/utils/docker_utils.py b/node_cli/utils/docker_utils.py index f625e282..7a4957db 100644 --- a/node_cli/utils/docker_utils.py +++ b/node_cli/utils/docker_utils.py @@ -33,7 +33,6 @@ SYNC_COMPOSE_PATH, REMOVED_CONTAINERS_FOLDER_PATH, SGX_CERTIFICATES_DIR_NAME, - SKALE_DIR, NGINX_CONTAINER_NAME ) @@ -247,7 +246,7 @@ def compose_rm(env={}, sync_node: bool = False): compose_path = get_compose_path(sync_node) run_cmd( cmd=( - 'docker compose', + 'docker', 'compose', '-f', compose_path, 'down', '-t', str(COMPOSE_SHUTDOWN_TIMEOUT), @@ -257,34 +256,30 @@ def compose_rm(env={}, sync_node: bool = False): logger.info('Compose containers removed') -def compose_pull(sync_node: bool = False): +def compose_pull(env: dict, sync_node: bool = False): logger.info('Pulling compose containers') compose_path = get_compose_path(sync_node) run_cmd( - cmd=('docker compose', '-f', compose_path, 'pull'), - env={ - 'SKALE_DIR': SKALE_DIR - } + cmd=('docker', 'compose', '-f', compose_path, 'pull'), + env=env ) -def compose_build(sync_node: bool = False): +def compose_build(env: dict, sync_node: bool = False): logger.info('Building compose containers') compose_path = get_compose_path(sync_node) run_cmd( - cmd=('docker compose', '-f', compose_path, 'build'), - env={ - 'SKALE_DIR': SKALE_DIR - } + cmd=('docker', 'compose', '-f', compose_path, 'build'), + env=env ) def get_up_compose_cmd(services): - return ('docker compose', '-f', COMPOSE_PATH, 'up', '-d', *services) + return ('docker', 'compose', '-f', COMPOSE_PATH, 'up', '-d', *services) def get_up_compose_sync_cmd(): - return ('docker compose', '-f', SYNC_COMPOSE_PATH, 'up', '-d') + return ('docker', 'compose', '-f', SYNC_COMPOSE_PATH, 'up', '-d') def get_compose_path(sync_node: bool) -> str: @@ -302,6 +297,7 @@ def compose_up(env, sync_node=False): if 'SGX_CERTIFICATES_DIR_NAME' not in env: env['SGX_CERTIFICATES_DIR_NAME'] = SGX_CERTIFICATES_DIR_NAME + logger.debug('Launching containers with env %s', env) run_cmd(cmd=get_up_compose_cmd(BASE_COMPOSE_SERVICES), env=env) if str_to_bool(env.get('MONITORING_CONTAINERS', 'False')): logger.info('Running monitoring containers') diff --git a/scripts/run_nftables_test.sh b/scripts/run_nftables_test.sh index 98827e3c..e4bf8563 100755 --- a/scripts/run_nftables_test.sh +++ b/scripts/run_nftables_test.sh @@ -1,10 +1,6 @@ #!/usr/bin/env bash set -ea -# DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" -# PROJECT_DIR=$(dirname $DIR) -# export DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" - docker rm -f ncli-tester || true docker build . -t ncli-tester docker run \ From baff1737b0bd2cb3d1e91718ddc28b848a63edb9 Mon Sep 17 00:00:00 2001 From: badrogger Date: Thu, 5 Dec 2024 12:02:45 +0000 Subject: [PATCH 40/56] Fix tcp port condition --- node_cli/core/nftables.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index b7db5bb7..518f511c 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -305,7 +305,7 @@ def setup_firewall(self, enable_monitoring: bool = False) -> None: self.add_connection_tracking_rule(self.chain) - tcp_ports = [get_ssh_port(), 8080, 443, 53, 3009, 9100] + tcp_ports = [get_ssh_port(), 53, 443, 3009] if enable_monitoring: tcp_ports.extend([8080, 9100]) for port in tcp_ports: From d9d2410c0ce1d5606f24b6f2438acc48237fca34 Mon Sep 17 00:00:00 2001 From: badrogger Date: Thu, 5 Dec 2024 12:03:12 +0000 Subject: [PATCH 41/56] Remove iptables-persistant from required checks --- node_cli/core/checks.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/node_cli/core/checks.py b/node_cli/core/checks.py index a5e85c70..57a64454 100644 --- a/node_cli/core/checks.py +++ b/node_cli/core/checks.py @@ -321,10 +321,6 @@ def _check_apt_package(self, package_name: str, else: return self._ok(name=package_name, info=info) - @preinstall - def iptables_persistent(self) -> CheckResult: - return self._check_apt_package('iptables-persistent') - @preinstall def lvm2(self) -> CheckResult: return self._check_apt_package('lvm2') From 1925849a94cefe6922c31631395ea569ca155bd9 Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 9 Dec 2024 12:28:00 +0000 Subject: [PATCH 42/56] Save nftables rules at the end of node configuration --- node_cli/cli/node.py | 4 ++-- node_cli/configs/__init__.py | 2 ++ node_cli/core/nftables.py | 32 ++++++++++++++++++++++++++++++-- node_cli/core/node.py | 23 +++++++++++------------ 4 files changed, 45 insertions(+), 16 deletions(-) diff --git a/node_cli/cli/node.py b/node_cli/cli/node.py index fe826a58..1b491456 100644 --- a/node_cli/cli/node.py +++ b/node_cli/cli/node.py @@ -19,8 +19,8 @@ import click +from node_cli.core.nftables import configure_nftables from node_cli.core.node import ( - configure_firewall_rules, get_node_signature, init, restore, @@ -245,7 +245,7 @@ def check(network): expose_value=False, prompt='Are you sure you want to reconfigure firewall rules?') def configure_firewall(monitoring): - configure_firewall_rules(enable_monitoring=monitoring) + configure_nftables(enable_monitoring=monitoring) @node.command(help='Show node version information') diff --git a/node_cli/configs/__init__.py b/node_cli/configs/__init__.py index 7e742536..abd9399f 100644 --- a/node_cli/configs/__init__.py +++ b/node_cli/configs/__init__.py @@ -163,3 +163,5 @@ def _get_env(): TELEGRAF_TEMPLATE_PATH = os.path.join(CONTAINER_CONFIG_PATH, 'telegraf.conf.j2') TELEGRAF_CONFIG_PATH = os.path.join(CONTAINER_CONFIG_PATH, 'telegraf.conf') NODE_DOCKER_CONFIG_PATH = os.path.join(NODE_DATA_PATH, 'docker.json') + +NFTABLES_RULES_PATH = '/etc/nftables.conf' diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index 518f511c..846558a5 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -4,8 +4,8 @@ from typing import Optional from dataclasses import dataclass -from node_cli.configs import ENV -from node_cli.utils.helper import get_ssh_port +from node_cli.configs import ENV, NFTABLES_RULES_PATH +from node_cli.utils.helper import get_ssh_port, run_cmd logger = logging.getLogger(__name__) @@ -333,6 +333,34 @@ def setup_firewall(self, enable_monitoring: bool = False) -> None: def configure_nftables(enable_monitoring: bool = False) -> None: + logger.info('Enabling nftables services') + enable_nftables_service() + logger.info('Setting up firewall') nft_mgr = NFTablesManager() nft_mgr.setup_firewall(enable_monitoring=enable_monitoring) logger.info('Firewall setup completed successfully') + + +def enable_nftables_service() -> None: + run_cmd(['systemctl', 'enable', 'nftables']) + + +def get_plain_ruleset(nft: nftables.Nftables) -> str: + nft.set_json_output(False) + try: + rc, output, error = nft.cmd('list ruleset') + if rc != 0: + raise NFTablesError(f'Failed to get ruleset: {error}') + return output + finally: + nft.set_json_output(True) + + +def save_nftables_rules(nft: Optional[nftables.Nftables] = None) -> None: + logger.info('Saving nftables rules') + nft = nft or nftables.Nftables() + ruleset = get_plain_ruleset(nft) + content = '#!/usr/sbin/nft -f\n' + 'flush ruleset\n' + ruleset + with open(NFTABLES_RULES_PATH, 'w') as f: + f.write(content) + logger.info('Rules saved successfully to %s', NFTABLES_RULES_PATH) diff --git a/node_cli/core/node.py b/node_cli/core/node.py index b728b5dc..dfdee143 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -45,7 +45,7 @@ from node_cli.configs.env import get_env_config from node_cli.configs.cli_logger import LOG_DATA_PATH as CLI_LOG_DATA_PATH -from node_cli.core.nftables import configure_nftables +from node_cli.core.nftables import configure_nftables, save_nftables_rules from node_cli.core.host import is_node_inited, save_env_params, get_flask_secret_key from node_cli.core.checks import run_checks as run_host_checks from node_cli.core.resources import update_resource_allocation @@ -140,7 +140,7 @@ def init(env_filepath): return enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) - configure_firewall_rules(enable_monitoring=enable_monitoring) + configure_nftables(enable_monitoring=enable_monitoring) inited_ok = init_op(env_filepath, env) if not inited_ok: error_exit('Init operation failed', exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR) @@ -150,6 +150,7 @@ def init(env_filepath): error_exit('Containers are not running', exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR) logger.info('Generating resource allocation file ...') update_resource_allocation(env['ENV_TYPE']) + save_nftables_rules() logger.info('Init procedure finished') @@ -166,7 +167,7 @@ def restore(backup_path, env_filepath, no_snapshot=False, config_only=False): env['BACKUP_RUN'] = 'True' # should be str enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) - configure_firewall_rules(enable_monitoring=enable_monitoring) + configure_nftables(enable_monitoring=enable_monitoring) restored_ok = restore_op(env, backup_path, config_only=config_only) if not restored_ok: @@ -174,6 +175,7 @@ def restore(backup_path, env_filepath, no_snapshot=False, config_only=False): time.sleep(RESTORE_SLEEP_TIMEOUT) logger.info('Generating resource allocation file ...') update_resource_allocation(env['ENV_TYPE']) + save_nftables_rules() print('Node is restored from backup') @@ -182,10 +184,11 @@ def init_sync(env_filepath: str, archive: bool, historic_state: bool, snapshot_f if env is None: return enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) - configure_firewall_rules(enable_monitoring=enable_monitoring) + configure_nftables(enable_monitoring=enable_monitoring) inited_ok = init_sync_op(env_filepath, env, archive, historic_state, snapshot_from) if not inited_ok: error_exit('Init operation failed', exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR) + save_nftables_rules() logger.info('Waiting for containers initialization') time.sleep(TM_INIT_TIMEOUT) if not is_base_containers_alive(sync_node=True): @@ -202,9 +205,10 @@ def update_sync(env_filepath: str, unsafe_ok: bool = False) -> None: migrate_2_6() env = get_node_env(env_filepath, sync_node=True) enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) - configure_firewall_rules(enable_monitoring=enable_monitoring) + configure_nftables(enable_monitoring=enable_monitoring) update_ok = update_sync_op(env_filepath, env) if update_ok: + save_nftables_rules() logger.info('Waiting for containers initialization') time.sleep(TM_INIT_TIMEOUT) alive = is_base_containers_alive(sync_node=True) @@ -274,9 +278,10 @@ def update(env_filepath: str, pull_config_for_schain: str, unsafe_ok: bool = Fal pull_config_for_schain=pull_config_for_schain, ) enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) - configure_firewall_rules(enable_monitoring=enable_monitoring) + configure_nftables(enable_monitoring=enable_monitoring) update_ok = update_op(env_filepath, env) if update_ok: + save_nftables_rules() logger.info('Waiting for containers initialization') time.sleep(TM_INIT_TIMEOUT) alive = is_base_containers_alive() @@ -468,9 +473,3 @@ def run_checks( else: print('Node is not fully meet the requirements!') print_failed_requirements_checks(failed_checks) - - -def configure_firewall_rules(enable_monitoring: bool = False) -> None: - print('Configuring firewall ...') - configure_nftables(enable_monitoring=enable_monitoring) - print('Done') From b311103d633e30d7d7ed9877be167003e9c4ce0d Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 9 Dec 2024 15:08:55 +0000 Subject: [PATCH 43/56] Move configure_nftables to operations.base --- node_cli/cli/node.py | 4 ++-- node_cli/core/nftables.py | 30 +++++++++++++++--------------- node_cli/core/node.py | 23 +++++------------------ node_cli/operations/__init__.py | 3 ++- node_cli/operations/base.py | 23 ++++++++++++++++++++++- tests/cli/node_test.py | 4 ++-- tests/cli/sync_node_test.py | 8 ++++---- tests/core/core_checks_test.py | 4 ++-- tests/core/core_node_test.py | 4 ++-- 9 files changed, 56 insertions(+), 47 deletions(-) diff --git a/node_cli/cli/node.py b/node_cli/cli/node.py index 1b491456..8eee2d96 100644 --- a/node_cli/cli/node.py +++ b/node_cli/cli/node.py @@ -19,7 +19,7 @@ import click -from node_cli.core.nftables import configure_nftables +from node_cli.core.node import configure_firewall_rules from node_cli.core.node import ( get_node_signature, init, @@ -245,7 +245,7 @@ def check(network): expose_value=False, prompt='Are you sure you want to reconfigure firewall rules?') def configure_firewall(monitoring): - configure_nftables(enable_monitoring=monitoring) + configure_firewall_rules(enable_monitoring=monitoring) @node.command(help='Show node version information') diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index 846558a5..20aa7a17 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -287,6 +287,16 @@ def add_loopback_rule(self, chain) -> None: else: logger.info('Loopback rule already exists in chain %s', chain) + def get_plain_ruleset(self) -> str: + self.nft.set_json_output(False) + try: + rc, output, error = self.nft.cmd('list ruleset') + if rc != 0: + raise NFTablesError(f'Failed to get ruleset: {error}') + return output + finally: + self.nft.set_json_output(True) + def setup_firewall(self, enable_monitoring: bool = False) -> None: """Setup firewall rules""" try: @@ -335,9 +345,12 @@ def setup_firewall(self, enable_monitoring: bool = False) -> None: def configure_nftables(enable_monitoring: bool = False) -> None: logger.info('Enabling nftables services') enable_nftables_service() - logger.info('Setting up firewall') + logger.info('Configuring firewall rules') nft_mgr = NFTablesManager() nft_mgr.setup_firewall(enable_monitoring=enable_monitoring) + logger.info('Firewall rules are configured') + ruleset = nft_mgr.get_plain_ruleset() + save_nftables_rules(ruleset) logger.info('Firewall setup completed successfully') @@ -345,21 +358,8 @@ def enable_nftables_service() -> None: run_cmd(['systemctl', 'enable', 'nftables']) -def get_plain_ruleset(nft: nftables.Nftables) -> str: - nft.set_json_output(False) - try: - rc, output, error = nft.cmd('list ruleset') - if rc != 0: - raise NFTablesError(f'Failed to get ruleset: {error}') - return output - finally: - nft.set_json_output(True) - - -def save_nftables_rules(nft: Optional[nftables.Nftables] = None) -> None: +def save_nftables_rules(ruleset: str) -> None: logger.info('Saving nftables rules') - nft = nft or nftables.Nftables() - ruleset = get_plain_ruleset(nft) content = '#!/usr/sbin/nft -f\n' + 'flush ruleset\n' + ruleset with open(NFTABLES_RULES_PATH, 'w') as f: f.write(content) diff --git a/node_cli/core/node.py b/node_cli/core/node.py index dfdee143..71f0eeff 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -45,11 +45,11 @@ from node_cli.configs.env import get_env_config from node_cli.configs.cli_logger import LOG_DATA_PATH as CLI_LOG_DATA_PATH -from node_cli.core.nftables import configure_nftables, save_nftables_rules from node_cli.core.host import is_node_inited, save_env_params, get_flask_secret_key from node_cli.core.checks import run_checks as run_host_checks from node_cli.core.resources import update_resource_allocation from node_cli.operations import ( + configure_firewall, update_op, init_op, turn_off_op, @@ -69,7 +69,6 @@ get_request, post_request, extract_env_params, - str_to_bool, ) from node_cli.utils.meta import get_meta_info from node_cli.utils.texts import Texts @@ -139,8 +138,6 @@ def init(env_filepath): if env is None: return - enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) - configure_nftables(enable_monitoring=enable_monitoring) inited_ok = init_op(env_filepath, env) if not inited_ok: error_exit('Init operation failed', exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR) @@ -150,7 +147,6 @@ def init(env_filepath): error_exit('Containers are not running', exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR) logger.info('Generating resource allocation file ...') update_resource_allocation(env['ENV_TYPE']) - save_nftables_rules() logger.info('Init procedure finished') @@ -166,16 +162,12 @@ def restore(backup_path, env_filepath, no_snapshot=False, config_only=False): logger.info('Adding BACKUP_RUN to env ...') env['BACKUP_RUN'] = 'True' # should be str - enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) - configure_nftables(enable_monitoring=enable_monitoring) - restored_ok = restore_op(env, backup_path, config_only=config_only) if not restored_ok: error_exit('Restore operation failed', exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR) time.sleep(RESTORE_SLEEP_TIMEOUT) logger.info('Generating resource allocation file ...') update_resource_allocation(env['ENV_TYPE']) - save_nftables_rules() print('Node is restored from backup') @@ -183,12 +175,9 @@ def init_sync(env_filepath: str, archive: bool, historic_state: bool, snapshot_f env = get_node_env(env_filepath, sync_node=True) if env is None: return - enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) - configure_nftables(enable_monitoring=enable_monitoring) inited_ok = init_sync_op(env_filepath, env, archive, historic_state, snapshot_from) if not inited_ok: error_exit('Init operation failed', exit_code=CLIExitCodes.OPERATION_EXECUTION_ERROR) - save_nftables_rules() logger.info('Waiting for containers initialization') time.sleep(TM_INIT_TIMEOUT) if not is_base_containers_alive(sync_node=True): @@ -204,11 +193,8 @@ def update_sync(env_filepath: str, unsafe_ok: bool = False) -> None: if (__version__ == 'test' or __version__.startswith('2.6')) and prev_version == '2.5.0': migrate_2_6() env = get_node_env(env_filepath, sync_node=True) - enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) - configure_nftables(enable_monitoring=enable_monitoring) update_ok = update_sync_op(env_filepath, env) if update_ok: - save_nftables_rules() logger.info('Waiting for containers initialization') time.sleep(TM_INIT_TIMEOUT) alive = is_base_containers_alive(sync_node=True) @@ -277,11 +263,8 @@ def update(env_filepath: str, pull_config_for_schain: str, unsafe_ok: bool = Fal sync_schains=False, pull_config_for_schain=pull_config_for_schain, ) - enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) - configure_nftables(enable_monitoring=enable_monitoring) update_ok = update_op(env_filepath, env) if update_ok: - save_nftables_rules() logger.info('Waiting for containers initialization') time.sleep(TM_INIT_TIMEOUT) alive = is_base_containers_alive() @@ -473,3 +456,7 @@ def run_checks( else: print('Node is not fully meet the requirements!') print_failed_requirements_checks(failed_checks) + + +def configure_firewall_rules(enable_monitoring: bool = False) -> None: + configure_firewall(enable_monitoring=enable_monitoring) diff --git a/node_cli/operations/__init__.py b/node_cli/operations/__init__.py index ca1b076d..7037a5c5 100644 --- a/node_cli/operations/__init__.py +++ b/node_cli/operations/__init__.py @@ -25,5 +25,6 @@ turn_off as turn_off_op, turn_on as turn_on_op, restore as restore_op, - repair_sync as repair_sync_op + repair_sync as repair_sync_op, + configure_nftables as configure_firewall ) diff --git a/node_cli/operations/base.py b/node_cli/operations/base.py index 2a2867a9..60ae1744 100644 --- a/node_cli/operations/base.py +++ b/node_cli/operations/base.py @@ -28,6 +28,7 @@ from node_cli.core.docker_config import configure_docker from node_cli.core.nginx import generate_nginx_config +from node_cli.core.nftables import configure_nftables from node_cli.core.node_options import NodeOptions from node_cli.core.resources import update_resource_allocation, init_shared_space_volume @@ -58,6 +59,7 @@ ) from node_cli.utils.meta import get_meta_info, update_meta from node_cli.utils.print_formatters import print_failed_requirements_checks +from node_cli.utils.helper import str_to_bool logger = logging.getLogger(__name__) @@ -104,12 +106,14 @@ def update(env_filepath: str, env: Dict) -> None: remove_dynamic_containers() sync_skale_node() - ensure_btrfs_kernel_module_autoloaded() if env.get('SKIP_DOCKER_CONFIG') != 'True': configure_docker() + enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) + configure_nftables(enable_monitoring=enable_monitoring) + backup_old_contracts() download_contracts(env) @@ -153,6 +157,9 @@ def init(env_filepath: str, env: dict) -> bool: if env.get('SKIP_DOCKER_CONFIG') != 'True': configure_docker() + enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) + configure_nftables(enable_monitoring=enable_monitoring) + prepare_host( env_filepath, env_type=env['ENV_TYPE'] @@ -198,6 +205,9 @@ def init_sync( if env.get('SKIP_DOCKER_CONFIG') != 'True': configure_docker() + enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) + configure_nftables(enable_monitoring=enable_monitoring) + prepare_host( env_filepath, env_type=env['ENV_TYPE'], @@ -250,6 +260,9 @@ def update_sync(env_filepath: str, env: Dict) -> bool: if env.get('SKIP_DOCKER_CONFIG') != 'True': configure_docker() + enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) + configure_nftables(enable_monitoring=enable_monitoring) + ensure_filestorage_mapping() backup_old_contracts() download_contracts(env) @@ -297,6 +310,10 @@ def turn_on(env): ) if env.get('SKIP_DOCKER_CONFIG') != 'True': configure_docker() + + enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) + configure_nftables(enable_monitoring=enable_monitoring) + logger.info('Launching containers on the node...') compose_up(env) @@ -314,9 +331,13 @@ def restore(env, backup_path, config_only=False): return False ensure_btrfs_kernel_module_autoloaded() + if env.get('SKIP_DOCKER_CONFIG') != 'True': configure_docker() + enable_monitoring = str_to_bool(env.get('MONITORING_CONTAINERS', 'False')) + configure_nftables(enable_monitoring=enable_monitoring) + link_env_file() lvmpy_install(env) init_shared_space_volume(env['ENV_TYPE']) diff --git a/tests/cli/node_test.py b/tests/cli/node_test.py index 0f41aaed..e12b4329 100644 --- a/tests/cli/node_test.py +++ b/tests/cli/node_test.py @@ -309,7 +309,7 @@ def test_restore(mocked_g_config): ), patch( 'node_cli.core.node.get_meta_info', return_value=CliMeta(version='2.4.0', config_stream='3.0.2'), - ), patch('node_cli.core.node.configure_firewall_rules'): + ), patch('node_cli.operations.base.configure_nftables'): result = run_command(restore_node, [backup_path, './tests/test-env']) assert result.exit_code == 0 assert 'Node is restored from backup\n' in result.output # noqa @@ -331,7 +331,7 @@ def test_restore_no_snapshot(mocked_g_config): ), patch( 'node_cli.core.node.get_meta_info', return_value=CliMeta(version='2.4.0', config_stream='3.0.2'), - ), patch('node_cli.core.node.configure_firewall_rules'): + ), patch('node_cli.operations.base.configure_nftables'): result = run_command(restore_node, [backup_path, './tests/test-env', '--no-snapshot']) assert result.exit_code == 0 assert 'Node is restored from backup\n' in result.output # noqa diff --git a/tests/cli/sync_node_test.py b/tests/cli/sync_node_test.py index 5e98cd41..80548775 100644 --- a/tests/cli/sync_node_test.py +++ b/tests/cli/sync_node_test.py @@ -41,7 +41,7 @@ def test_init_sync(mocked_g_config): 'node_cli.core.node.init_sync_op' ), mock.patch('node_cli.core.node.is_base_containers_alive', return_value=True), mock.patch( 'node_cli.core.resources.get_disk_size', return_value=BIG_DISK_SIZE - ), mock.patch('node_cli.core.node.configure_firewall_rules'), mock.patch( + ), mock.patch('node_cli.operations.base.configure_nftables'), mock.patch( 'node_cli.utils.decorators.is_node_inited', return_value=False ): result = run_command(_init_sync, ['./tests/test-env']) @@ -74,7 +74,7 @@ def test_init_sync_archive(mocked_g_config, clean_node_options): 'node_cli.operations.base.update_images' ), mock.patch('node_cli.operations.base.compose_up'), mock.patch( 'node_cli.core.resources.get_disk_size', return_value=BIG_DISK_SIZE - ), mock.patch('node_cli.core.node.configure_firewall_rules'), mock.patch( + ), mock.patch('node_cli.operations.base.configure_nftables'), mock.patch( 'node_cli.utils.decorators.is_node_inited', return_value=False ): result = run_command( @@ -95,7 +95,7 @@ def test_init_sync_historic_state_fail(mocked_g_config, clean_node_options): 'node_cli.core.node.init_sync_op' ), mock.patch('node_cli.core.node.is_base_containers_alive', return_value=True), mock.patch( 'node_cli.core.resources.get_disk_size', return_value=BIG_DISK_SIZE - ), mock.patch('node_cli.core.node.configure_firewall_rules'), mock.patch( + ), mock.patch('node_cli.operations.base.configure_nftables'), mock.patch( 'node_cli.utils.decorators.is_node_inited', return_value=False ): result = run_command(_init_sync, ['./tests/test-env', '--historic-state']) @@ -110,7 +110,7 @@ def test_update_sync(mocked_g_config): 'node_cli.core.node.update_sync_op' ), mock.patch('node_cli.core.node.is_base_containers_alive', return_value=True), mock.patch( 'node_cli.core.resources.get_disk_size', return_value=BIG_DISK_SIZE - ), mock.patch('node_cli.core.node.configure_firewall_rules'), mock.patch( + ), mock.patch('node_cli.operations.base.configure_nftables'), mock.patch( 'node_cli.utils.decorators.is_node_inited', return_value=True ), mock.patch( 'node_cli.core.node.get_meta_info', return_value={'version': __version__} diff --git a/tests/core/core_checks_test.py b/tests/core/core_checks_test.py index 6e75bbdd..921a46ec 100644 --- a/tests/core/core_checks_test.py +++ b/tests/core/core_checks_test.py @@ -346,9 +346,9 @@ def test_get_checks(requirements_data): disk = 'test-disk' checkers = get_all_checkers(disk, requirements_data) checks = get_checks(checkers) - assert len(checks) == 16 + assert len(checks) == 15 checks = get_checks(checkers, check_type=CheckType.PREINSTALL) - assert len(checks) == 14 + assert len(checks) == 13 checks = get_checks(checkers, check_type=CheckType.POSTINSTALL) assert len(checks) == 2 diff --git a/tests/core/core_node_test.py b/tests/core/core_node_test.py index aeb35600..471062c6 100644 --- a/tests/core/core_node_test.py +++ b/tests/core/core_node_test.py @@ -146,7 +146,7 @@ def test_init_node(no_resource_file): # todo: write new init node test 'node_cli.core.resources.get_disk_size', return_value=BIG_DISK_SIZE ), mock.patch('node_cli.core.host.prepare_host'), mock.patch( 'node_cli.core.host.init_data_dir' - ), mock.patch('node_cli.core.node.configure_firewall_rules'), mock.patch( + ), mock.patch('node_cli.operations.base.configure_nftables'), mock.patch( 'node_cli.core.node.init_op' ), mock.patch('node_cli.core.node.is_base_containers_alive', return_value=True), mock.patch( 'node_cli.utils.helper.post_request', resp_mock @@ -163,7 +163,7 @@ def test_update_node(mocked_g_config, resource_file): 'node_cli.core.node.update_op' ), mock.patch('node_cli.core.node.get_flask_secret_key'), mock.patch( 'node_cli.core.node.save_env_params' - ), mock.patch('node_cli.core.node.configure_firewall_rules'), mock.patch( + ), mock.patch('node_cli.operations.base.configure_nftables'), mock.patch( 'node_cli.core.host.prepare_host' ), mock.patch('node_cli.core.node.is_base_containers_alive', return_value=True), mock.patch( 'node_cli.utils.helper.post_request', resp_mock From 2080b7457a93d5253e0205a0cb512f27eb964f3f Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 16 Dec 2024 19:34:21 +0000 Subject: [PATCH 44/56] Add include to the nftables conf file --- node_cli/configs/__init__.py | 1 + node_cli/core/nftables.py | 26 +++++++++++++++++--------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/node_cli/configs/__init__.py b/node_cli/configs/__init__.py index abd9399f..7d256f14 100644 --- a/node_cli/configs/__init__.py +++ b/node_cli/configs/__init__.py @@ -165,3 +165,4 @@ def _get_env(): NODE_DOCKER_CONFIG_PATH = os.path.join(NODE_DATA_PATH, 'docker.json') NFTABLES_RULES_PATH = '/etc/nftables.conf' +NFTABLES_CHAIN_FOLDER_PATH = '/etc/nft.conf.d/chains' diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index 20aa7a17..f7df5a47 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -1,10 +1,11 @@ import json import logging +import os import sys from typing import Optional from dataclasses import dataclass -from node_cli.configs import ENV, NFTABLES_RULES_PATH +from node_cli.configs import ENV, NFTABLES_RULES_PATH, NFTABLES_CHAIN_FOLDER_PATH from node_cli.utils.helper import get_ssh_port, run_cmd logger = logging.getLogger(__name__) @@ -34,7 +35,7 @@ class NFTablesError(Exception): class NFTablesManager: - def __init__(self, family: str = 'inet', table: str = 'firewall', chain: str = 'input') -> None: + def __init__(self, family: str = 'inet', table: str = 'firewall', chain: str = 'skale') -> None: self.nft = nftables.Nftables() self.nft.set_json_output(True) self.family = family @@ -72,7 +73,7 @@ def chain_exists(self, chain_name: str) -> bool: return chain_name in self.get_chains() def create_chain_if_not_exists( - self, chain: str, hook: str, priority: int = 0, policy: str = 'accept' + self, chain: str, hook: str, priority: int = 1, policy: str = 'accept' ) -> None: if not self.chain_exists(chain): cmd = { @@ -299,6 +300,8 @@ def get_plain_ruleset(self) -> str: def setup_firewall(self, enable_monitoring: bool = False) -> None: """Setup firewall rules""" + + logger.info('Configuring firewall rules') try: self.create_table_if_not_exists() @@ -306,6 +309,7 @@ def setup_firewall(self, enable_monitoring: bool = False) -> None: 'input': {'hook': 'input', 'policy': 'accept'}, 'forward': {'hook': 'forward', 'policy': 'drop'}, 'output': {'hook': 'output', 'policy': 'accept'}, + 'skale': {'hook': 'input', 'policy': 'accept'}, } for chain, config in base_chains_config.items(): @@ -334,33 +338,37 @@ def setup_firewall(self, enable_monitoring: bool = False) -> None: ) ) - self.add_drop_rule_if_node_exists(protocol='tcp') + # self.add_drop_rule_if_node_exists(protocol='tcp') self.add_drop_rule_if_node_exists(protocol='udp') except Exception as e: logger.error('Failed to setup firewall: %s', e) raise NFTablesError(e) + logger.info('Firewall rules are configured') + + +def prepare_directories() -> None: + logger.info('Prepare directories for nftables') + os.makedirs(NFTABLES_CHAIN_FOLDER_PATH, exist_ok=True) def configure_nftables(enable_monitoring: bool = False) -> None: - logger.info('Enabling nftables services') + prepare_directories() enable_nftables_service() - logger.info('Configuring firewall rules') nft_mgr = NFTablesManager() nft_mgr.setup_firewall(enable_monitoring=enable_monitoring) - logger.info('Firewall rules are configured') ruleset = nft_mgr.get_plain_ruleset() save_nftables_rules(ruleset) - logger.info('Firewall setup completed successfully') def enable_nftables_service() -> None: + logger.info('Enabling nftables services') run_cmd(['systemctl', 'enable', 'nftables']) def save_nftables_rules(ruleset: str) -> None: logger.info('Saving nftables rules') - content = '#!/usr/sbin/nft -f\n' + 'flush ruleset\n' + ruleset + content = f'#!/usr/sbin/nft -f\nflush ruleset\n{ruleset}\ninclude "{NFTABLES_CHAIN_FOLDER_PATH}/*"' # noqa with open(NFTABLES_RULES_PATH, 'w') as f: f.write(content) logger.info('Rules saved successfully to %s', NFTABLES_RULES_PATH) From ef5ac867085a3c7d42a0d21d588460bc99366fdf Mon Sep 17 00:00:00 2001 From: badrogger Date: Tue, 17 Dec 2024 19:52:07 +0000 Subject: [PATCH 45/56] Fix turn-off --- node_cli/core/nftables.py | 10 ++++++---- node_cli/core/node.py | 29 ++++++++++++++++++----------- node_cli/operations/base.py | 6 +++--- node_cli/utils/helper.py | 24 ++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 18 deletions(-) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index f7df5a47..fe5b975a 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -6,7 +6,7 @@ from dataclasses import dataclass from node_cli.configs import ENV, NFTABLES_RULES_PATH, NFTABLES_CHAIN_FOLDER_PATH -from node_cli.utils.helper import get_ssh_port, run_cmd +from node_cli.utils.helper import get_ssh_port, remove_between_brackets, run_cmd logger = logging.getLogger(__name__) @@ -288,16 +288,18 @@ def add_loopback_rule(self, chain) -> None: else: logger.info('Loopback rule already exists in chain %s', chain) - def get_plain_ruleset(self) -> str: + def get_base_ruleset(self) -> str: self.nft.set_json_output(False) + output = '' try: rc, output, error = self.nft.cmd('list ruleset') if rc != 0: raise NFTablesError(f'Failed to get ruleset: {error}') - return output finally: self.nft.set_json_output(True) + return remove_between_brackets(text=output, pattern='skale-') + def setup_firewall(self, enable_monitoring: bool = False) -> None: """Setup firewall rules""" @@ -357,7 +359,7 @@ def configure_nftables(enable_monitoring: bool = False) -> None: enable_nftables_service() nft_mgr = NFTablesManager() nft_mgr.setup_firewall(enable_monitoring=enable_monitoring) - ruleset = nft_mgr.get_plain_ruleset() + ruleset = nft_mgr.get_base_ruleset() save_nftables_rules(ruleset) diff --git a/node_cli/core/node.py b/node_cli/core/node.py index 71f0eeff..24f4ea00 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -42,7 +42,7 @@ TM_INIT_TIMEOUT, ) from node_cli.cli import __version__ -from node_cli.configs.env import get_env_config +from node_cli.configs.env import get_env_config, SKALE_DIR_ENV_FILEPATH from node_cli.configs.cli_logger import LOG_DATA_PATH as CLI_LOG_DATA_PATH from node_cli.core.host import is_node_inited, save_env_params, get_flask_secret_key @@ -134,7 +134,7 @@ def register_node(name, p2p_ip, public_ip, port, domain_name): @check_not_inited def init(env_filepath): - env = get_node_env(env_filepath) + env = compose_node_env(env_filepath) if env is None: return @@ -152,7 +152,7 @@ def init(env_filepath): @check_not_inited def restore(backup_path, env_filepath, no_snapshot=False, config_only=False): - env = get_node_env(env_filepath) + env = compose_node_env(env_filepath) if env is None: return save_env_params(env_filepath) @@ -172,7 +172,7 @@ def restore(backup_path, env_filepath, no_snapshot=False, config_only=False): def init_sync(env_filepath: str, archive: bool, historic_state: bool, snapshot_from: str) -> None: - env = get_node_env(env_filepath, sync_node=True) + env = compose_node_env(env_filepath, sync_node=True) if env is None: return inited_ok = init_sync_op(env_filepath, env, archive, historic_state, snapshot_from) @@ -192,7 +192,7 @@ def update_sync(env_filepath: str, unsafe_ok: bool = False) -> None: prev_version = get_meta_info()['version'] if (__version__ == 'test' or __version__.startswith('2.6')) and prev_version == '2.5.0': migrate_2_6() - env = get_node_env(env_filepath, sync_node=True) + env = compose_node_env(env_filepath, sync_node=True) update_ok = update_sync_op(env_filepath, env) if update_ok: logger.info('Waiting for containers initialization') @@ -219,12 +219,18 @@ def repair_sync(archive: bool, historic_state: bool, snapshot_from: str) -> None logger.info('Schain was started from scratch') -def get_node_env( - env_filepath, inited_node=False, sync_schains=None, pull_config_for_schain=None, sync_node=False +def compose_node_env( + env_filepath, + inited_node=False, + sync_schains=None, + pull_config_for_schain=None, + sync_node=False, + save: bool = True ): if env_filepath is not None: env_params = extract_env_params(env_filepath, sync_node=sync_node, raise_for_status=True) - save_env_params(env_filepath) + if save: + save_env_params(env_filepath) else: env_params = extract_env_params(INIT_ENV_FILEPATH, sync_node=sync_node) @@ -257,7 +263,7 @@ def update(env_filepath: str, pull_config_for_schain: str, unsafe_ok: bool = Fal if (__version__ == 'test' or __version__.startswith('2.6')) and prev_version == '2.5.0': migrate_2_6() logger.info('Node update started') - env = get_node_env( + env = compose_node_env( env_filepath, inited_node=True, sync_schains=False, @@ -375,13 +381,14 @@ def turn_off(maintenance_on: bool = False, unsafe_ok: bool = False) -> None: error_exit(error_msg, exit_code=CLIExitCodes.UNSAFE_UPDATE) if maintenance_on: set_maintenance_mode_on() - turn_off_op() + env = compose_node_env(SKALE_DIR_ENV_FILEPATH, save=False) + turn_off_op(env=env) @check_inited @check_user def turn_on(maintenance_off, sync_schains, env_file): - env = get_node_env(env_file, inited_node=True, sync_schains=sync_schains) + env = compose_node_env(env_file, inited_node=True, sync_schains=sync_schains) turn_on_op(env) logger.info('Waiting for containers initialization') time.sleep(TM_INIT_TIMEOUT) diff --git a/node_cli/operations/base.py b/node_cli/operations/base.py index 60ae1744..540f0188 100644 --- a/node_cli/operations/base.py +++ b/node_cli/operations/base.py @@ -292,14 +292,14 @@ def update_sync(env_filepath: str, env: Dict) -> bool: return True -def turn_off(): +def turn_off(env: dict) -> None: logger.info('Turning off the node...') - compose_rm() + compose_rm(env=env) remove_dynamic_containers() logger.info('Node was successfully turned off') -def turn_on(env): +def turn_on(env: dict) -> None: logger.info('Turning on the node...') update_meta( VERSION, diff --git a/node_cli/utils/helper.py b/node_cli/utils/helper.py index 39de440a..d6747cd3 100644 --- a/node_cli/utils/helper.py +++ b/node_cli/utils/helper.py @@ -422,3 +422,27 @@ def get_ssh_port(ssh_service_name='ssh'): except OSError: logger.exception('Cannot get ssh service port') return DEFAULT_SSH_PORT + + +def remove_between_brackets(text: str, pattern: str) -> str: + result = [] + skip = 0 + found_pattern = False + + lines = text.split('\n') + for line in lines: + if pattern in line: + found_pattern = True + + if found_pattern: + if '{' in line: + skip += line.count('{') + if '}' in line: + skip -= line.count('}') + if skip == 0: + found_pattern = False + continue + else: + result.append(line) + + return '\n'.join(result) From 1ceae64feaf4b6179d8afe97189d7078cff7409e Mon Sep 17 00:00:00 2001 From: badrogger Date: Wed, 18 Dec 2024 19:18:57 +0000 Subject: [PATCH 46/56] Fix migration --- node_cli/core/nftables.py | 29 ++++++++++++---- node_cli/core/node.py | 1 + node_cli/migrations/focal_to_jammy.py | 50 ++++++++++++++++----------- 3 files changed, 54 insertions(+), 26 deletions(-) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index fe5b975a..a752a847 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -307,12 +307,7 @@ def setup_firewall(self, enable_monitoring: bool = False) -> None: try: self.create_table_if_not_exists() - base_chains_config = { - 'input': {'hook': 'input', 'policy': 'accept'}, - 'forward': {'hook': 'forward', 'policy': 'drop'}, - 'output': {'hook': 'output', 'policy': 'accept'}, - 'skale': {'hook': 'input', 'policy': 'accept'}, - } + base_chains_config = {'skale': {'hook': 'input', 'policy': 'accept'}} for chain, config in base_chains_config.items(): self.create_chain_if_not_exists( @@ -348,6 +343,28 @@ def setup_firewall(self, enable_monitoring: bool = False) -> None: raise NFTablesError(e) logger.info('Firewall rules are configured') + def flush_chain(self, chain: str) -> None: + """Remove all rules from a specific chain""" + json_cmd = { + 'nftables': [{ + 'flush': { + 'chain': { + 'family': self.family, + 'table': self.table, + 'name': chain + } + } + }] + } + + try: + rc, output, error = self.nft.json_cmd(json_cmd) + if rc != 0: + raise NFTablesError(f'Failed to flush chain: {error}') + except Exception as e: + logger.error(f'Failed to flush chain: {str(e)}') + raise NFTablesError('Flushing chain errored') + def prepare_directories() -> None: logger.info('Prepare directories for nftables') diff --git a/node_cli/core/node.py b/node_cli/core/node.py index 24f4ea00..147a0a30 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -260,6 +260,7 @@ def update(env_filepath: str, pull_config_for_schain: str, unsafe_ok: bool = Fal error_exit(error_msg, exit_code=CLIExitCodes.UNSAFE_UPDATE) prev_version = get_meta_info().version + logger.info('HERE %s %s', __version__, prev_version) if (__version__ == 'test' or __version__.startswith('2.6')) and prev_version == '2.5.0': migrate_2_6() logger.info('Node update started') diff --git a/node_cli/migrations/focal_to_jammy.py b/node_cli/migrations/focal_to_jammy.py index 0f7be46c..7c0f0625 100644 --- a/node_cli/migrations/focal_to_jammy.py +++ b/node_cli/migrations/focal_to_jammy.py @@ -1,5 +1,6 @@ import logging +from node_cli.core.nftables import NFTablesManager from node_cli.utils.helper import get_ssh_port, run_cmd logger = logging.getLogger(__name__) @@ -19,67 +20,73 @@ '53' # dns ] +IPTABLES_CHAIN = 'INPUT' + + +class NFTablesCmdFailedError(Exception): + pass + def remove_tcp_rules(ssh_port: int) -> None: - tcp_rule_template = 'iptables -{} INPUT -p tcp -m tcp --dport {} -j ACCEPT' + tcp_rule_template = 'iptables -{} {} -p tcp -m tcp --dport {} -j ACCEPT' for tcp_port in [*ALLOWED_INCOMING_TCP_PORTS, ssh_port]: - check_cmd = tcp_rule_template.format('C', tcp_port).split(' ') - remove_cmd = tcp_rule_template.format('D', tcp_port).split(' ') + check_cmd = tcp_rule_template.format('C', IPTABLES_CHAIN, tcp_port).split(' ') + remove_cmd = tcp_rule_template.format('D', IPTABLES_CHAIN, tcp_port).split(' ') result = run_cmd(check_cmd, check_code=False) if result.returncode == 0: result = run_cmd(remove_cmd) def remove_udp_rules() -> None: - udp_rule_template = 'iptables -{} INPUT -p udp -m udp --dport {} -j ACCEPT' + udp_rule_template = 'iptables -{} {} -p udp -m udp --dport {} -j ACCEPT' for udp_port in [*ALLOWED_INCOMING_UDP_PORTS]: - check_cmd = udp_rule_template.format('C', udp_port).split(' ') - remove_cmd = udp_rule_template.format('D', udp_port).split(' ') + check_cmd = udp_rule_template.format('C', IPTABLES_CHAIN, udp_port).split(' ') + remove_cmd = udp_rule_template.format('D', IPTABLES_CHAIN, udp_port).split(' ') result = run_cmd(check_cmd, check_code=False) if result.returncode == 0: result = run_cmd(remove_cmd) def remove_loopback_rules() -> None: - loopback_rule_template = 'iptables -{} INPUT -i lo -j ACCEPT' - check_cmd = loopback_rule_template.format('C').split(' ') - remove_cmd = loopback_rule_template.format('D').split(' ') + loopback_rule_template = 'iptables -{} {} -i lo -j ACCEPT' + check_cmd = loopback_rule_template.format('C', IPTABLES_CHAIN).split(' ') + remove_cmd = loopback_rule_template.format('D', IPTABLES_CHAIN).split(' ') result = run_cmd(check_cmd, check_code=False) if result.returncode == 0: result = run_cmd(remove_cmd) def remove_icmp_rules() -> None: - icmp_rule_template = 'iptables -{} INPUT -p icmp -m icmp --icmp-type {} -j ACCEPT' + icmp_rule_template = 'iptables -{} {} -p icmp -m icmp --icmp-type {} -j ACCEPT' for icmp_type in [3, 4, 11]: - check_cmd = icmp_rule_template.format('C', icmp_type).split(' ') - remove_cmd = icmp_rule_template.format('D', icmp_type).split(' ') + check_cmd = icmp_rule_template.format('C', IPTABLES_CHAIN, icmp_type).split(' ') + remove_cmd = icmp_rule_template.format('D', IPTABLES_CHAIN, icmp_type).split(' ') result = run_cmd(check_cmd, check_code=False) if result.returncode == 0: result = run_cmd(remove_cmd) def remove_conntrack_rules() -> None: - track_rule_template = 'iptables -{} INPUT -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT' - check_cmd = track_rule_template.format('C').split(' ') - remove_cmd = track_rule_template.format('D').split(' ') + track_rule_template = 'iptables -{} {} -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT' + check_cmd = track_rule_template.format('C', IPTABLES_CHAIN).split(' ') + remove_cmd = track_rule_template.format('D', IPTABLES_CHAIN).split(' ') result = run_cmd(check_cmd, check_code=False) if result.returncode == 0: result = run_cmd(remove_cmd) def remove_drop_rules() -> None: - drop_rule_template = 'iptables -{} INPUT -p {} -j DROP' + drop_rule_template = 'iptables -{} {} -p {} -j DROP' protocols = ['tcp', 'udp'] for proto in protocols: - check_cmd = drop_rule_template.format('C', proto).split(' ') - remove_cmd = drop_rule_template.format('D', proto).split(' ') + check_cmd = drop_rule_template.format('C', IPTABLES_CHAIN, proto).split(' ') + remove_cmd = drop_rule_template.format('D', IPTABLES_CHAIN, proto).split(' ') result = run_cmd(check_cmd, check_code=False) if result.returncode == 0: result = run_cmd(remove_cmd) -def remove_old_firewall_rules(ssh_port: int) -> None: +def remove_old_iptables_rules(ssh_port: int) -> None: remove_drop_rules() remove_conntrack_rules() remove_loopback_rules() @@ -91,5 +98,8 @@ def remove_old_firewall_rules(ssh_port: int) -> None: def migrate() -> None: ssh_port = get_ssh_port() logger.info('Running migration from focal to jammy') - remove_old_firewall_rules(ssh_port) + remove_old_iptables_rules(ssh_port) + logger.info('Flushing nftables rules generated by release upgrade') + nft = NFTablesManager(family='ip', table='filter') + nft.flush_chain(IPTABLES_CHAIN) logger.info('Migration from focal to jammy completed') From fbc27e77f7fa55af29715b996b5d1adcfca7e240 Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 23 Dec 2024 13:50:21 +0000 Subject: [PATCH 47/56] Small improvements --- node_cli/core/nftables.py | 2 +- node_cli/core/node.py | 5 ++--- node_cli/migrations/focal_to_jammy.py | 19 +++++++++++++++++++ node_cli/operations/__init__.py | 2 +- node_cli/utils/helper.py | 1 + 5 files changed, 24 insertions(+), 5 deletions(-) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index a752a847..22442a8b 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -18,7 +18,7 @@ from collections import namedtuple # hotfix for tests iptc = namedtuple('nftables', ['Chain', 'Rule']) else: - logger.error(f'Unable to import iptc due to an error {err}') + logger.error(f'Unable to import nftables due to an error {err}') @dataclass diff --git a/node_cli/core/node.py b/node_cli/core/node.py index 147a0a30..e360d7dc 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -49,7 +49,7 @@ from node_cli.core.checks import run_checks as run_host_checks from node_cli.core.resources import update_resource_allocation from node_cli.operations import ( - configure_firewall, + configure_nftables, update_op, init_op, turn_off_op, @@ -260,7 +260,6 @@ def update(env_filepath: str, pull_config_for_schain: str, unsafe_ok: bool = Fal error_exit(error_msg, exit_code=CLIExitCodes.UNSAFE_UPDATE) prev_version = get_meta_info().version - logger.info('HERE %s %s', __version__, prev_version) if (__version__ == 'test' or __version__.startswith('2.6')) and prev_version == '2.5.0': migrate_2_6() logger.info('Node update started') @@ -467,4 +466,4 @@ def run_checks( def configure_firewall_rules(enable_monitoring: bool = False) -> None: - configure_firewall(enable_monitoring=enable_monitoring) + configure_nftables(enable_monitoring=enable_monitoring) diff --git a/node_cli/migrations/focal_to_jammy.py b/node_cli/migrations/focal_to_jammy.py index 7c0f0625..eb63b6e2 100644 --- a/node_cli/migrations/focal_to_jammy.py +++ b/node_cli/migrations/focal_to_jammy.py @@ -1,3 +1,22 @@ +# -*- coding: utf-8 -*- +# +# This file is part of node-cli +# +# Copyright (C) 2019 SKALE Labs +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + + import logging from node_cli.core.nftables import NFTablesManager diff --git a/node_cli/operations/__init__.py b/node_cli/operations/__init__.py index 7037a5c5..de13f14b 100644 --- a/node_cli/operations/__init__.py +++ b/node_cli/operations/__init__.py @@ -26,5 +26,5 @@ turn_on as turn_on_op, restore as restore_op, repair_sync as repair_sync_op, - configure_nftables as configure_firewall + configure_nftables as configure_nftables ) diff --git a/node_cli/utils/helper.py b/node_cli/utils/helper.py index d6747cd3..f479093e 100644 --- a/node_cli/utils/helper.py +++ b/node_cli/utils/helper.py @@ -425,6 +425,7 @@ def get_ssh_port(ssh_service_name='ssh'): def remove_between_brackets(text: str, pattern: str) -> str: + """ Remove all lines between brackets where the bracket line starts with the pattern """ result = [] skip = 0 found_pattern = False From cd0bce6db6dd10333a81ebda52e5ce058011cc33 Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 23 Dec 2024 15:45:55 +0000 Subject: [PATCH 48/56] Fix tests --- tests/core/core_node_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/core_node_test.py b/tests/core/core_node_test.py index 471062c6..d418ad15 100644 --- a/tests/core/core_node_test.py +++ b/tests/core/core_node_test.py @@ -172,7 +172,7 @@ def test_update_node(mocked_g_config, resource_file): ), mock.patch( 'node_cli.core.node.get_meta_info', return_value=CliMeta( - version='2.5.0', config_stream='3.0.2' + version='2.6.0', config_stream='3.0.2' ) ): with mock.patch( 'node_cli.utils.helper.requests.get', return_value=safe_update_api_response()): # noqa From 35e33aa6331366d18d5ca8a113e973c9b4865fc3 Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 23 Dec 2024 16:39:59 +0000 Subject: [PATCH 49/56] Fix migration tests --- tests/core/migration_test.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/core/migration_test.py b/tests/core/migration_test.py index 29a19680..ce5a47fb 100644 --- a/tests/core/migration_test.py +++ b/tests/core/migration_test.py @@ -4,8 +4,11 @@ from node_cli.utils.helper import run_cmd +CUSTOM_CHAIN_NAME = 'TEST' + def add_base_rules(): + run_cmd(f'iptables -N {CUSTOM_CHAIN_NAME}'.split(' ')) run_cmd('iptables -A INPUT -m conntrack --ctstate ESTABLISHED,RELATED -j ACCEPT'.split(' ')) run_cmd('iptables -A INPUT -i lo -j ACCEPT'.split(' ')) run_cmd('iptables -A INPUT -p tcp --dport 22 -j ACCEPT'.split(' ')) @@ -15,7 +18,7 @@ def add_base_rules(): run_cmd('iptables -A INPUT -p udp --dport 53 -j ACCEPT'.split(' ')) run_cmd('iptables -A INPUT -p tcp --dport 3009 -j ACCEPT'.split(' ')) # non skale related rule - run_cmd('iptables -A INPUT -p tcp --dport 2222 -j ACCEPT'.split(' ')) + run_cmd(f'iptables -A {CUSTOM_CHAIN_NAME} -p tcp --dport 2222 -j ACCEPT'.split(' ')) run_cmd('iptables -A INPUT -p tcp --dport 9100 -j ACCEPT'.split(' ')) run_cmd('iptables -A INPUT -p tcp -j DROP'.split(' ')) run_cmd('iptables -A INPUT -p udp -j DROP'.split(' ')) @@ -37,4 +40,5 @@ def test_migration(base_rules): migrate() res = run_cmd(['iptables', '-S']) output = res.stdout.decode('utf-8') - assert output == '-P INPUT ACCEPT\n-P FORWARD ACCEPT\n-P OUTPUT ACCEPT\n-A INPUT -p tcp -m tcp --dport 2222 -j ACCEPT\n' # noqa + print(output) + assert output == f'-P INPUT ACCEPT\n-P FORWARD ACCEPT\n-P OUTPUT ACCEPT\n-N {CUSTOM_CHAIN_NAME}\n-A {CUSTOM_CHAIN_NAME} -p tcp -m tcp --dport 2222 -j ACCEPT\n' # noqa From 20bff35f89f63cf71d321eba95af38805e615c3e Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 23 Dec 2024 17:15:00 +0000 Subject: [PATCH 50/56] Remove comments --- .github/workflows/test.yml | 1 - tests/core/migration_test.py | 1 - 2 files changed, 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 27abe86d..31625095 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -28,7 +28,6 @@ jobs: - name: Install python dependencies run: | python -m pip install --upgrade pip - # pip install setuptools==75.5.0 pip install -e .[dev] - name: Lint with flake8 diff --git a/tests/core/migration_test.py b/tests/core/migration_test.py index ce5a47fb..c3f20d57 100644 --- a/tests/core/migration_test.py +++ b/tests/core/migration_test.py @@ -40,5 +40,4 @@ def test_migration(base_rules): migrate() res = run_cmd(['iptables', '-S']) output = res.stdout.decode('utf-8') - print(output) assert output == f'-P INPUT ACCEPT\n-P FORWARD ACCEPT\n-P OUTPUT ACCEPT\n-N {CUSTOM_CHAIN_NAME}\n-A {CUSTOM_CHAIN_NAME} -p tcp -m tcp --dport 2222 -j ACCEPT\n' # noqa From eb71c193d82db71fd3fe1abf314624925701cd42 Mon Sep 17 00:00:00 2001 From: badrogger Date: Thu, 26 Dec 2024 17:22:51 +0000 Subject: [PATCH 51/56] Small improvements --- node_cli/operations/__init__.py | 2 +- tests/cli/sync_node_test.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/node_cli/operations/__init__.py b/node_cli/operations/__init__.py index de13f14b..28e46ff9 100644 --- a/node_cli/operations/__init__.py +++ b/node_cli/operations/__init__.py @@ -26,5 +26,5 @@ turn_on as turn_on_op, restore as restore_op, repair_sync as repair_sync_op, - configure_nftables as configure_nftables + configure_nftables ) diff --git a/tests/cli/sync_node_test.py b/tests/cli/sync_node_test.py index 80548775..360b1b5e 100644 --- a/tests/cli/sync_node_test.py +++ b/tests/cli/sync_node_test.py @@ -47,7 +47,6 @@ def test_init_sync(mocked_g_config): result = run_command(_init_sync, ['./tests/test-env']) node_options = NodeOptions() - print(node_options) assert not node_options.archive assert not node_options.catchup assert not node_options.historic_state From 37cf9baff6625a17e81d1772174e33759416ac6a Mon Sep 17 00:00:00 2001 From: badrogger Date: Thu, 26 Dec 2024 17:30:29 +0000 Subject: [PATCH 52/56] Remove unnecessary comment --- node_cli/core/nftables.py | 1 - 1 file changed, 1 deletion(-) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index 22442a8b..b9b05911 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -335,7 +335,6 @@ def setup_firewall(self, enable_monitoring: bool = False) -> None: ) ) - # self.add_drop_rule_if_node_exists(protocol='tcp') self.add_drop_rule_if_node_exists(protocol='udp') except Exception as e: From ab876fffa3ad067861b8cb543f8d78fc0791e54b Mon Sep 17 00:00:00 2001 From: badrogger Date: Thu, 26 Dec 2024 17:33:01 +0000 Subject: [PATCH 53/56] Add missing license header --- node_cli/core/nftables.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/node_cli/core/nftables.py b/node_cli/core/nftables.py index b9b05911..de411ddb 100644 --- a/node_cli/core/nftables.py +++ b/node_cli/core/nftables.py @@ -1,3 +1,22 @@ +# -*- coding: utf-8 -*- +# +# This file is part of node-cli +# +# Copyright (C) 2019 SKALE Labs +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + import json import logging import os From 151e57183e1e7056f2b40898273ccbd42353cfe5 Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 30 Dec 2024 13:54:24 +0000 Subject: [PATCH 54/56] Update lvmpy. Fix is update safe --- lvmpy | 2 +- node_cli/core/node.py | 7 ++++++- node_cli/utils/docker_utils.py | 21 +++++++++++++++++++++ tests/cli/node_test.py | 29 ++++++++++++++--------------- tests/core/core_node_test.py | 28 +++++++++++++++++++++++----- 5 files changed, 65 insertions(+), 22 deletions(-) diff --git a/lvmpy b/lvmpy index 0d796233..694e2533 160000 --- a/lvmpy +++ b/lvmpy @@ -1 +1 @@ -Subproject commit 0d7962335f1e60797fdd89d3c9d1e750e0355275 +Subproject commit 694e25337b51fe97aaadd3ea45e88759e838184e diff --git a/node_cli/core/node.py b/node_cli/core/node.py index e360d7dc..7330cfd9 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -74,6 +74,7 @@ from node_cli.utils.texts import Texts from node_cli.utils.exit_codes import CLIExitCodes from node_cli.utils.decorators import check_not_inited, check_inited, check_user +from node_cli.utils.docker_utils import is_admin_running, is_api_running, is_sync_admin_running from node_cli.migrations.focal_to_jammy import migrate as migrate_2_6 @@ -96,7 +97,11 @@ class NodeStatuses(Enum): NOT_CREATED = 5 -def is_update_safe() -> bool: +def is_update_safe(sync_node: bool = False) -> bool: + if not sync_node and (not is_admin_running() or not is_api_running()): + return True + if sync_node and not is_sync_admin_running(): + return True status, payload = get_request(BLUEPRINT_NAME, 'update-safe') if status == 'error': return False diff --git a/node_cli/utils/docker_utils.py b/node_cli/utils/docker_utils.py index 7a4957db..fe271d79 100644 --- a/node_cli/utils/docker_utils.py +++ b/node_cli/utils/docker_utils.py @@ -335,6 +335,27 @@ def cleanup_unused_images(dclient=None, ignore=None): ) +def is_container_running(name: str, dclient: Optional[DockerClient] = None) -> bool: + dc = dclient or docker_client() + try: + container = dc.containers.get(name) + return container.status == 'running' + except docker.errors.NotFound: + return False + + +def is_admin_running(dclient: Optional[DockerClient] = None) -> bool: + return is_container_running(name='skale_admin', dclient=dclient) + + +def is_api_running(dclient: Optional[DockerClient] = None) -> bool: + return is_container_running(name='skale_api', dclient=dclient) + + +def is_sync_admin_running(dclient: Optional[DockerClient] = None) -> bool: + return is_container_running(name='skale_sync_admin', dclient=dclient) + + def system_prune(): logger.info('Removing dangling docker artifacts') cmd = ['docker', 'system', 'prune', '-f'] diff --git a/tests/cli/node_test.py b/tests/cli/node_test.py index e12b4329..7e206db1 100644 --- a/tests/cli/node_test.py +++ b/tests/cli/node_test.py @@ -46,7 +46,6 @@ response_mock, run_command, run_command_mock, - safe_update_api_response, subprocess_run_mock, ) from tests.resources_test import BIG_DISK_SIZE @@ -368,8 +367,19 @@ def test_turn_off_maintenance_on(mocked_g_config): with mock.patch('subprocess.run', new=subprocess_run_mock), mock.patch( 'node_cli.core.node.turn_off_op' ), mock.patch('node_cli.utils.decorators.is_node_inited', return_value=True): + result = run_command_mock( + 'node_cli.utils.helper.requests.post', + resp_mock, + _turn_off, + ['--maintenance-on', '--yes'], + ) + assert ( + result.output + == 'Setting maintenance mode on...\nNode is successfully set in maintenance mode\n' + ) # noqa + assert result.exit_code == 0 with mock.patch( - 'node_cli.utils.helper.requests.get', return_value=safe_update_api_response() + 'node_cli.utils.docker_utils.is_container_running', return_value=True ): result = run_command_mock( 'node_cli.utils.helper.requests.post', @@ -377,19 +387,8 @@ def test_turn_off_maintenance_on(mocked_g_config): _turn_off, ['--maintenance-on', '--yes'], ) - assert ( - result.output - == 'Setting maintenance mode on...\nNode is successfully set in maintenance mode\n' - ) # noqa - assert result.exit_code == 0 - result = run_command_mock( - 'node_cli.utils.helper.requests.post', - resp_mock, - _turn_off, - ['--maintenance-on', '--yes'], - ) - assert 'Cannot turn off safely' in result.output - assert result.exit_code == CLIExitCodes.UNSAFE_UPDATE + assert 'Cannot turn off safely' in result.output + assert result.exit_code == CLIExitCodes.UNSAFE_UPDATE def test_turn_on_maintenance_off(mocked_g_config): diff --git a/tests/core/core_node_test.py b/tests/core/core_node_test.py index d418ad15..f79c6fa3 100644 --- a/tests/core/core_node_test.py +++ b/tests/core/core_node_test.py @@ -181,14 +181,32 @@ def test_update_node(mocked_g_config, resource_file): def test_is_update_safe(): - assert not is_update_safe() + assert is_update_safe() + assert is_update_safe(sync_node=True) + + with mock.patch('node_cli.core.node.is_admin_running', return_value=True): + with mock.patch('node_cli.core.node.is_api_running', return_value=True): + assert not is_update_safe() + assert is_update_safe(sync_node=True) + + with mock.patch('node_cli.core.node.is_sync_admin_running', return_value=True): + assert is_update_safe() + assert not is_update_safe(sync_node=True) + + with mock.patch('node_cli.utils.docker_utils.is_container_running', return_value=True): + with mock.patch( + 'node_cli.utils.helper.requests.get', return_value=safe_update_api_response() + ): + assert is_update_safe() + with mock.patch('node_cli.utils.helper.requests.get', return_value=safe_update_api_response()): assert is_update_safe() - with mock.patch( - 'node_cli.utils.helper.requests.get', return_value=safe_update_api_response(safe=False) - ): - assert not is_update_safe() + with mock.patch('node_cli.utils.docker_utils.is_container_running', return_value=True): + with mock.patch( + 'node_cli.utils.helper.requests.get', return_value=safe_update_api_response(safe=False) + ): + assert not is_update_safe() def test_repair_sync(tmp_sync_datadir, mocked_g_config, resource_file): From 0048a1e873ddd6b1a74075e5f060988d05e37768 Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 30 Dec 2024 15:30:41 +0000 Subject: [PATCH 55/56] Fix is_update_safe exception condition --- node_cli/core/node.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node_cli/core/node.py b/node_cli/core/node.py index 7330cfd9..dd950235 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -98,7 +98,7 @@ class NodeStatuses(Enum): def is_update_safe(sync_node: bool = False) -> bool: - if not sync_node and (not is_admin_running() or not is_api_running()): + if not sync_node and (not is_admin_running() and not is_api_running()): return True if sync_node and not is_sync_admin_running(): return True From 8cbdcff9a483da48e029a45204a8e73aeb5cf6cc Mon Sep 17 00:00:00 2001 From: badrogger Date: Mon, 30 Dec 2024 17:08:34 +0000 Subject: [PATCH 56/56] Remove redundant braces --- node_cli/core/node.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node_cli/core/node.py b/node_cli/core/node.py index dd950235..5fb53294 100644 --- a/node_cli/core/node.py +++ b/node_cli/core/node.py @@ -98,7 +98,7 @@ class NodeStatuses(Enum): def is_update_safe(sync_node: bool = False) -> bool: - if not sync_node and (not is_admin_running() and not is_api_running()): + if not sync_node and not is_admin_running() and not is_api_running(): return True if sync_node and not is_sync_admin_running(): return True