From 88f2b7ba99c3b3be01b5938fe69f02740a09ebaf Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 29 Jan 2025 16:16:28 -0800 Subject: [PATCH] Protect from non-bool --- solarwinds_apm/apm_config.py | 9 ++--- .../test_apm_config_is_legacy.py | 35 +++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/solarwinds_apm/apm_config.py b/solarwinds_apm/apm_config.py index 8e36eebd..bdc9fe14 100644 --- a/solarwinds_apm/apm_config.py +++ b/solarwinds_apm/apm_config.py @@ -248,15 +248,16 @@ def _get_extension_components( @classmethod def calculate_is_legacy(cls) -> bool: """Checks if agent is running in a legacy environment. + Invalid boolean values are ignored. Order of precedence: Environment Variable > config file > default False """ is_legacy = False cnf_dict = cls.get_cnf_dict() if cnf_dict: - is_legacy = cls.convert_to_bool(cnf_dict.get("legacy", is_legacy)) - is_legacy = cls.convert_to_bool( - os.environ.get("SW_APM_LEGACY", is_legacy) - ) + cnf_legacy = cls.convert_to_bool(cnf_dict.get("legacy")) + is_legacy = cnf_legacy if cnf_legacy is not None else is_legacy + env_legacy = cls.convert_to_bool(os.environ.get("SW_APM_LEGACY")) + is_legacy = env_legacy if env_legacy is not None else is_legacy return is_legacy @classmethod diff --git a/tests/unit/test_apm_config/test_apm_config_is_legacy.py b/tests/unit/test_apm_config/test_apm_config_is_legacy.py index ac718fc8..e2ba4ade 100644 --- a/tests/unit/test_apm_config/test_apm_config_is_legacy.py +++ b/tests/unit/test_apm_config/test_apm_config_is_legacy.py @@ -17,6 +17,10 @@ def test_calculate_is_legacy_default(self, mocker): ) assert SolarWindsApmConfig.calculate_is_legacy() is False + def test_calculate_is_legacy_with_env_var_not_boolean(self, mocker): + mocker.patch.dict(os.environ, {"SW_APM_LEGACY": "foo-bar"}) + assert SolarWindsApmConfig.calculate_is_legacy() is False + def test_calculate_is_legacy_with_env_var_false(self, mocker): mocker.patch.dict(os.environ, {"SW_APM_LEGACY": "false"}) assert SolarWindsApmConfig.calculate_is_legacy() is False @@ -25,6 +29,13 @@ def test_calculate_is_legacy_with_env_var_true(self, mocker): mocker.patch.dict(os.environ, {"SW_APM_LEGACY": "true"}) assert SolarWindsApmConfig.calculate_is_legacy() is True + def test_calculate_is_legacy_with_config_not_boolean(self, mocker): + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.get_cnf_dict", + return_value={"legacy": "foo-bar"}, + ) + assert SolarWindsApmConfig.calculate_is_legacy() is False + def test_calculate_is_legacy_with_config_false(self, mocker): mocker.patch( "solarwinds_apm.apm_config.SolarWindsApmConfig.get_cnf_dict", @@ -39,6 +50,14 @@ def test_calculate_is_legacy_with_config_true(self, mocker): ) assert SolarWindsApmConfig.calculate_is_legacy() is True + def test_calculate_is_legacy_with_config_not_bool_env_var_not_bool(self, mocker): + mocker.patch.dict(os.environ, {"SW_APM_LEGACY": "foo-bar"}) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.get_cnf_dict", + return_value={"legacy": "foo-bar"}, + ) + assert SolarWindsApmConfig.calculate_is_legacy() is False + def test_calculate_is_legacy_with_config_false_env_var_false(self, mocker): mocker.patch.dict(os.environ, {"SW_APM_LEGACY": "false"}) mocker.patch( @@ -55,6 +74,14 @@ def test_calculate_is_legacy_with_config_false_env_var_true(self, mocker): ) assert SolarWindsApmConfig.calculate_is_legacy() is True + def test_calculate_is_legacy_with_config_not_bool_env_var_true(self, mocker): + mocker.patch.dict(os.environ, {"SW_APM_LEGACY": "true"}) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.get_cnf_dict", + return_value={"legacy": "foo-bar"}, + ) + assert SolarWindsApmConfig.calculate_is_legacy() is True + def test_calculate_is_legacy_with_config_true_env_var_false(self, mocker): mocker.patch.dict(os.environ, {"SW_APM_LEGACY": "false"}) mocker.patch( @@ -63,6 +90,14 @@ def test_calculate_is_legacy_with_config_true_env_var_false(self, mocker): ) assert SolarWindsApmConfig.calculate_is_legacy() is False + def test_calculate_is_legacy_with_config_true_env_var_not_bool(self, mocker): + mocker.patch.dict(os.environ, {"SW_APM_LEGACY": "foo-bar"}) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.get_cnf_dict", + return_value={"legacy": "true"}, + ) + assert SolarWindsApmConfig.calculate_is_legacy() is True + def test_calculate_is_legacy_with_config_true_env_var_true(self, mocker): mocker.patch.dict(os.environ, {"SW_APM_LEGACY": "true"}) mocker.patch(