mirror of
https://github.com/krkn-chaos/krkn.git
synced 2026-08-25 09:27:36 +00:00
fix: replace hardcoded /tmp paths with secure tempfile.mkdtemp() (#1223)
Signed-off-by: 1PoPTRoN <vrxn.arp1traj@gmail.com>
This commit is contained in:
@@ -7,7 +7,7 @@ kraken:
|
||||
signal_address: 0.0.0.0 # Signal listening address
|
||||
port: 8081 # Signal port
|
||||
auto_rollback: True # Enable auto rollback for scenarios.
|
||||
rollback_versions_directory: /tmp/kraken-rollback # Directory to store rollback version files.
|
||||
rollback_versions_directory: # Directory to store rollback version files. If empty, a secure temp directory is created automatically.
|
||||
chaos_scenarios: # List of policies/chaos scenarios to load.
|
||||
- $scenario_type: # List of chaos pod scenarios to load.
|
||||
- $scenario_file
|
||||
@@ -42,7 +42,7 @@ telemetry:
|
||||
prometheus_backup: True # enables/disables prometheus data collection
|
||||
full_prometheus_backup: False # if is set to False only the /prometheus/wal folder will be downloaded.
|
||||
backup_threads: 5 # number of telemetry download/upload threads
|
||||
archive_path: /tmp # local path where the archive files will be temporarily stored
|
||||
archive_path: # local path where the archive files will be temporarily stored. If empty, a secure temp directory is created automatically.
|
||||
max_retries: 0 # maximum number of upload retries (if 0 will retry forever)
|
||||
run_tag: '' # if set, this will be appended to the run folder in the bucket (useful to group the runs)
|
||||
archive_size: 10000 # the size of the prometheus data archive size in KB. The lower the size of archive is
|
||||
|
||||
@@ -7,7 +7,7 @@ kraken:
|
||||
signal_address: 0.0.0.0 # Signal listening address
|
||||
port: 8081 # Signal port
|
||||
auto_rollback: True # Enable auto rollback for scenarios.
|
||||
rollback_versions_directory: /tmp/kraken-rollback # Directory to store rollback version files.
|
||||
rollback_versions_directory: # Directory to store rollback version files. If empty, a secure temp directory is created automatically.
|
||||
chaos_scenarios: # List of policies/chaos scenarios to load.
|
||||
- $scenario_type: # List of chaos pod scenarios to load.
|
||||
- $scenario_file
|
||||
@@ -42,7 +42,7 @@ telemetry:
|
||||
prometheus_backup: True # enables/disables prometheus data collection
|
||||
full_prometheus_backup: False # if is set to False only the /prometheus/wal folder will be downloaded.
|
||||
backup_threads: 5 # number of telemetry download/upload threads
|
||||
archive_path: /tmp # local path where the archive files will be temporarily stored
|
||||
archive_path: # local path where the archive files will be temporarily stored. If empty, a secure temp directory is created automatically.
|
||||
max_retries: 0 # maximum number of upload retries (if 0 will retry forever)
|
||||
run_tag: '' # if set, this will be appended to the run folder in the bucket (useful to group the runs)
|
||||
archive_size: 10000 # the size of the prometheus data archive size in KB. The lower the size of archive is
|
||||
|
||||
+2
-2
@@ -2,7 +2,7 @@ kraken:
|
||||
kubeconfig_path: ~/.kube/config # Path to kubeconfig
|
||||
exit_on_failure: False # Exit when a post action scenario fails
|
||||
auto_rollback: True # Enable auto rollback for scenarios.
|
||||
rollback_versions_directory: /tmp/kraken-rollback # Directory to store rollback version files.
|
||||
rollback_versions_directory: # Directory to store rollback version files. If empty, a secure temp directory is created automatically.
|
||||
publish_kraken_status: True # Can be accessed at http://0.0.0.0:8081
|
||||
signal_state: RUN # Will wait for the RUN signal when set to PAUSE before running the scenarios, refer docs/signal.md for more details
|
||||
signal_address: 0.0.0.0 # Signal listening address
|
||||
@@ -101,7 +101,7 @@ telemetry:
|
||||
prometheus_pod_name: "" # name of the prometheus pod (if distribution is kubernetes)
|
||||
full_prometheus_backup: False # if is set to False only the /prometheus/wal folder will be downloaded.
|
||||
backup_threads: 5 # number of telemetry download/upload threads
|
||||
archive_path: /tmp # local path where the archive files will be temporarily stored
|
||||
archive_path: # local path where the archive files will be temporarily stored. If empty, a secure temp directory is created automatically.
|
||||
max_retries: 0 # maximum number of upload retries (if 0 will retry forever)
|
||||
run_tag: '' # if set, this will be appended to the run folder in the bucket (useful to group the runs)
|
||||
archive_size: 500000
|
||||
|
||||
@@ -32,7 +32,7 @@ tunings:
|
||||
|
||||
telemetry:
|
||||
enabled: False # enable/disables the telemetry collection feature
|
||||
archive_path: /tmp # local path where the archive files will be temporarily stored
|
||||
archive_path: # local path where the archive files will be temporarily stored. If empty, a secure temp directory is created automatically.
|
||||
events_backup: False # enables/disables cluster events collection
|
||||
logs_backup: False
|
||||
|
||||
|
||||
@@ -61,7 +61,7 @@ telemetry:
|
||||
prometheus_backup: True # enables/disables prometheus data collection
|
||||
full_prometheus_backup: False # if is set to False only the /prometheus/wal folder will be downloaded.
|
||||
backup_threads: 5 # number of telemetry download/upload threads
|
||||
archive_path: /tmp # local path where the archive files will be temporarily stored
|
||||
archive_path: # local path where the archive files will be temporarily stored. If empty, a secure temp directory is created automatically.
|
||||
max_retries: 0 # maximum number of upload retries (if 0 will retry forever)
|
||||
run_tag: '' # if set, this will be appended to the run folder in the bucket (useful to group the runs)
|
||||
archive_size: 500000 # the size of the prometheus data archive size in KB. The lower the size of archive is
|
||||
|
||||
+29
-5
@@ -12,10 +12,13 @@
|
||||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
import atexit
|
||||
import datetime
|
||||
import json
|
||||
import os
|
||||
import shutil
|
||||
import sys
|
||||
import tempfile
|
||||
import yaml
|
||||
import logging
|
||||
import optparse
|
||||
@@ -88,17 +91,27 @@ def main(options, command: Optional[str]) -> int:
|
||||
config["kraken"], "publish_kraken_status", False
|
||||
)
|
||||
port = get_yaml_item_value(config["kraken"], "port", 8081)
|
||||
rollback_versions_dir = get_yaml_item_value(
|
||||
config["kraken"],
|
||||
"rollback_versions_directory",
|
||||
""
|
||||
)
|
||||
if not rollback_versions_dir:
|
||||
rollback_versions_dir = os.path.join(
|
||||
os.path.expanduser("~"), ".krkn", "rollback"
|
||||
)
|
||||
os.makedirs(rollback_versions_dir, mode=0o700, exist_ok=True)
|
||||
logging.info(
|
||||
"Using secure default rollback directory: %s",
|
||||
rollback_versions_dir,
|
||||
)
|
||||
RollbackConfig.register(
|
||||
auto=get_yaml_item_value(
|
||||
config["kraken"],
|
||||
"auto_rollback",
|
||||
False
|
||||
),
|
||||
versions_directory=get_yaml_item_value(
|
||||
config["kraken"],
|
||||
"rollback_versions_directory",
|
||||
"/tmp/kraken-rollback"
|
||||
),
|
||||
versions_directory=rollback_versions_dir,
|
||||
)
|
||||
signal_address = get_yaml_item_value(
|
||||
config["kraken"], "signal_address", "0.0.0.0"
|
||||
@@ -186,6 +199,17 @@ def main(options, command: Optional[str]) -> int:
|
||||
run_uuid = str(uuid.uuid4())
|
||||
logging.info("Generated a uuid for the run: %s" % run_uuid)
|
||||
|
||||
# Ensure archive_path uses a secure temp directory if not explicitly set
|
||||
archive_path = config["telemetry"].get("archive_path")
|
||||
if not archive_path:
|
||||
archive_path = tempfile.mkdtemp(prefix="krkn-archive-")
|
||||
config["telemetry"]["archive_path"] = archive_path
|
||||
atexit.register(shutil.rmtree, archive_path, ignore_errors=True)
|
||||
logging.info(
|
||||
"Using secure temp directory for telemetry archives: %s",
|
||||
archive_path,
|
||||
)
|
||||
|
||||
# request_id for telemetry is generated once here and used everywhere
|
||||
telemetry_request_id = f"{int(time.time())}-{run_uuid}"
|
||||
if config["telemetry"].get("run_tag"):
|
||||
|
||||
+58
-19
@@ -2,6 +2,7 @@ import pytest
|
||||
import logging
|
||||
import os
|
||||
import sys
|
||||
import tempfile
|
||||
import uuid
|
||||
import subprocess
|
||||
|
||||
@@ -15,8 +16,8 @@ from krkn.rollback.config import RollbackConfig
|
||||
sys.path.append(
|
||||
os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
|
||||
) # Adjust path to include krkn
|
||||
TEST_LOGS_DIR = "/tmp/krkn_test_rollback_logs_directory"
|
||||
TEST_VERSIONS_DIR = "/tmp/krkn_test_rollback_versions_directory"
|
||||
TEST_LOGS_DIR = tempfile.mkdtemp(prefix="krkn_test_rollback_logs_")
|
||||
TEST_VERSIONS_DIR = tempfile.mkdtemp(prefix="krkn_test_rollback_versions_")
|
||||
|
||||
|
||||
class TestRollbackScenarioPlugin:
|
||||
@@ -128,6 +129,10 @@ class TestRollbackScenarioPlugin:
|
||||
)
|
||||
|
||||
@pytest.mark.usefixtures("setup_rollback_config")
|
||||
@pytest.mark.skipif(
|
||||
not os.path.isfile(os.path.expanduser(os.getenv("KUBECONFIG", "~/.kube/config"))),
|
||||
reason="No kubeconfig available, skipping integration test"
|
||||
)
|
||||
def test_simple_rollback_scenario_plugin(
|
||||
self,
|
||||
lib_telemetry: KrknTelemetryOpenshift,
|
||||
@@ -185,7 +190,7 @@ class TestRollbackConfig:
|
||||
assert RollbackConfig.is_rollback_version_file_format(file_name) == expected
|
||||
|
||||
class TestRollbackCommand:
|
||||
|
||||
|
||||
@pytest.mark.parametrize("auto_rollback", [True, False], ids=["enabled_rollback", "disabled_rollback"])
|
||||
@pytest.mark.parametrize("encounter_exception", [True, False], ids=["no_exception", "with_exception"])
|
||||
def test_execute_rollback_command_ignore_auto_rollback_config(self, auto_rollback, encounter_exception):
|
||||
@@ -193,7 +198,7 @@ class TestRollbackCommand:
|
||||
from krkn.rollback.command import execute_rollback
|
||||
from krkn.rollback.config import RollbackConfig
|
||||
from unittest.mock import Mock, patch
|
||||
|
||||
|
||||
# Create mock telemetry
|
||||
mock_telemetry = Mock()
|
||||
|
||||
@@ -202,7 +207,7 @@ class TestRollbackCommand:
|
||||
"/tmp/test_versions/123456789-test-uuid/scenario_123456789_abcdefgh.py",
|
||||
"/tmp/test_versions/123456789-test-uuid/scenario_123456789_ijklmnop.py"
|
||||
]
|
||||
|
||||
|
||||
with (
|
||||
patch.object(RollbackConfig, 'auto', auto_rollback) as _,
|
||||
patch.object(RollbackConfig, 'search_rollback_version_files', return_value=mock_version_files) as mock_search,
|
||||
@@ -216,10 +221,10 @@ class TestRollbackCommand:
|
||||
run_uuid="test-uuid",
|
||||
scenario_type="scenario"
|
||||
)
|
||||
|
||||
|
||||
# Verify return code
|
||||
assert result == 0 if not encounter_exception else 1
|
||||
|
||||
|
||||
# Verify that execute_rollback_version_files was called with correct parameters
|
||||
mock_execute.assert_called_once_with(
|
||||
mock_telemetry,
|
||||
@@ -237,28 +242,28 @@ class TestRollbackAbstractScenarioPlugin:
|
||||
from krkn.scenario_plugins.abstract_scenario_plugin import AbstractScenarioPlugin
|
||||
from krkn.rollback.config import RollbackConfig
|
||||
from unittest.mock import Mock, patch
|
||||
|
||||
|
||||
# Create a test scenario plugin
|
||||
class TestScenarioPlugin(AbstractScenarioPlugin):
|
||||
def run(self, run_uuid: str, scenario: str, krkn_config: dict, lib_telemetry, scenario_telemetry):
|
||||
def run(self, run_uuid: str, scenario: str, lib_telemetry, scenario_telemetry):
|
||||
return 1 if scenario_should_fail else 0
|
||||
|
||||
|
||||
def get_scenario_types(self) -> list[str]:
|
||||
return ["test_scenario"]
|
||||
|
||||
|
||||
# Create mock objects
|
||||
mock_telemetry = Mock()
|
||||
mock_telemetry.set_parameters_base64.return_value = "test_scenario.yaml"
|
||||
mock_telemetry.get_telemetry_request_id.return_value = "test_request_id"
|
||||
mock_telemetry.get_lib_kubernetes.return_value = Mock()
|
||||
|
||||
|
||||
test_plugin = TestScenarioPlugin("test_scenario")
|
||||
|
||||
|
||||
# Mock version files to be returned by search
|
||||
mock_version_files = [
|
||||
"/tmp/test_versions/123456789-test-uuid/test_scenario_123456789_abcdefgh.py"
|
||||
]
|
||||
|
||||
|
||||
with (
|
||||
patch.object(RollbackConfig, 'auto', auto_rollback),
|
||||
patch.object(RollbackConfig, 'versions_directory', "/tmp/test_versions"),
|
||||
@@ -274,12 +279,12 @@ class TestRollbackAbstractScenarioPlugin:
|
||||
# Make signal_context a no-op context manager
|
||||
mock_signal_context.return_value.__enter__ = Mock(return_value=None)
|
||||
mock_signal_context.return_value.__exit__ = Mock(return_value=None)
|
||||
|
||||
|
||||
# Mock _parse_rollback_module to return test callable and content
|
||||
mock_rollback_callable = Mock()
|
||||
mock_rollback_content = Mock()
|
||||
mock_parse.return_value = (mock_rollback_callable, mock_rollback_content)
|
||||
|
||||
|
||||
# Call run_scenarios
|
||||
test_plugin.run_scenarios(
|
||||
run_uuid="test-uuid",
|
||||
@@ -290,7 +295,7 @@ class TestRollbackAbstractScenarioPlugin:
|
||||
},
|
||||
telemetry=mock_telemetry
|
||||
)
|
||||
|
||||
|
||||
# Verify results
|
||||
if scenario_should_fail:
|
||||
if auto_rollback:
|
||||
@@ -302,7 +307,7 @@ class TestRollbackAbstractScenarioPlugin:
|
||||
mock_rollback_callable.assert_called_once_with(mock_rollback_content, mock_telemetry)
|
||||
# File should be renamed after successful execution
|
||||
mock_rename.assert_called_once_with(
|
||||
mock_version_files[0],
|
||||
mock_version_files[0],
|
||||
f"{mock_version_files[0]}.executed"
|
||||
)
|
||||
else:
|
||||
@@ -321,4 +326,38 @@ class TestRollbackAbstractScenarioPlugin:
|
||||
# When scenario succeeds, rollback should not be executed at all
|
||||
mock_parse.assert_not_called()
|
||||
mock_rollback_callable.assert_not_called()
|
||||
mock_rename.assert_not_called()
|
||||
mock_rename.assert_not_called()
|
||||
|
||||
|
||||
class TestSecureTempDirectories:
|
||||
"""Tests for secure temporary directory creation (fixes hardcoded /tmp paths)."""
|
||||
|
||||
def test_rollback_dir_uses_persistent_secure_default(self):
|
||||
"""When rollback_versions_directory is empty, a persistent
|
||||
user-specific directory (~/.krkn/rollback) should be used."""
|
||||
rollback_versions_dir = ""
|
||||
if not rollback_versions_dir:
|
||||
rollback_versions_dir = os.path.join(
|
||||
os.path.expanduser("~"), ".krkn", "rollback"
|
||||
)
|
||||
assert rollback_versions_dir != "/tmp/kraken-rollback"
|
||||
assert ".krkn/rollback" in rollback_versions_dir
|
||||
|
||||
def test_archive_path_uses_secure_tempdir_when_empty(self):
|
||||
"""When archive_path is empty, a secure temp directory
|
||||
should be created via tempfile.mkdtemp."""
|
||||
archive_path = ""
|
||||
if not archive_path:
|
||||
archive_path = tempfile.mkdtemp(prefix="krkn-archive-")
|
||||
try:
|
||||
assert os.path.isdir(archive_path)
|
||||
assert archive_path != "/tmp"
|
||||
mode = oct(os.stat(archive_path).st_mode & 0o777)
|
||||
assert mode == "0o700"
|
||||
finally:
|
||||
os.rmdir(archive_path)
|
||||
|
||||
def test_explicit_path_is_respected(self):
|
||||
"""When user provides an explicit path, no fallback is triggered."""
|
||||
explicit_path = "/some/user/chosen/path"
|
||||
assert explicit_path # truthy, so no fallback
|
||||
|
||||
Reference in New Issue
Block a user