diff --git a/cli/src/pcluster/config/common.py b/cli/src/pcluster/config/common.py index 06e20c303f..4633e1bdcb 100644 --- a/cli/src/pcluster/config/common.py +++ b/cli/src/pcluster/config/common.py @@ -21,6 +21,7 @@ from typing import List, Set from pcluster.validators.common import AsyncValidator, FailureLevel, ValidationResult, Validator, ValidatorContext +from pcluster.validators.dev_settings_validators import ExtraChefAttributesValidator from pcluster.validators.iam_validators import AdditionalIamPolicyValidator from pcluster.validators.networking_validators import LambdaFunctionsVpcConfigValidator from pcluster.validators.s3_validators import UrlValidator @@ -333,6 +334,8 @@ def __init__(self, chef_cookbook: str = None, extra_chef_attributes: str = None) def _register_validators(self, context: ValidatorContext = None): if self.chef_cookbook is not None: self._register_validator(UrlValidator, url=self.chef_cookbook) + if self.extra_chef_attributes: + self._register_validator(ExtraChefAttributesValidator, extra_chef_attributes=self.extra_chef_attributes) class LambdaFunctionsVpcConfig(Resource): diff --git a/cli/src/pcluster/validators/dev_settings_validators.py b/cli/src/pcluster/validators/dev_settings_validators.py new file mode 100644 index 0000000000..abf3aa7d0b --- /dev/null +++ b/cli/src/pcluster/validators/dev_settings_validators.py @@ -0,0 +1,62 @@ +# Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance +# with the License. A copy of the License is located at +# +# http://aws.amazon.com/apache2.0/ +# +# or in the "LICENSE.txt" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES +# OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and +# limitations under the License. +import json + +from pcluster.validators.common import FailureLevel, Validator +from pcluster.validators.utils import dig, is_boolean_string, str_to_bool + +EXTRA_CHEF_ATTRIBUTES_PATH = "DevSettings/Cookbook/ExtraChefAttributes" +ATTR_IN_PLACE_UPDATE_ON_FLEET_ENABLED = "in_place_update_on_fleet_enabled" + + +class ExtraChefAttributesValidator(Validator): + """Validate DevSettings/Cookbook/ExtraChefAttributes.""" + + def _validate(self, extra_chef_attributes: str = None): + """Validate extra Chef attributes. + + Args: + extra_chef_attributes: JSON string containing Chef attributes. + Schema validation ensures this is valid JSON. + """ + if not extra_chef_attributes: + return + else: + self._validate_in_place_update_on_fleet_enabled(json.loads(extra_chef_attributes)) + + def _validate_in_place_update_on_fleet_enabled(self, extra_chef_attributes: dict = None): + """Validate attribute cluster.in_place_update_on_fleet_enabled. + + It returns an error if the attribute is set to a non-boolean value. + It returns a warning if the in-place update is disabled. + + Args: + extra_chef_attributes: Dictionary of Chef attributes to validate. + """ + in_place_update_on_fleet_enabled = dig(extra_chef_attributes, "cluster", ATTR_IN_PLACE_UPDATE_ON_FLEET_ENABLED) + + if in_place_update_on_fleet_enabled is None: + return + + if not is_boolean_string(str(in_place_update_on_fleet_enabled)): + self._add_failure( + f"Invalid value in {EXTRA_CHEF_ATTRIBUTES_PATH}: " + f"attribute '{ATTR_IN_PLACE_UPDATE_ON_FLEET_ENABLED}' must be a boolean value.", + FailureLevel.ERROR, + ) + return + + if str_to_bool(str(in_place_update_on_fleet_enabled)) is False: + self._add_failure( + "When in-place updates are disabled, cluster updates are applied " + "by replacing compute and login nodes according to the selected QueueUpdateStrategy.", + FailureLevel.WARNING, + ) diff --git a/cli/src/pcluster/validators/utils.py b/cli/src/pcluster/validators/utils.py index 83fc0cbe09..67dc388737 100644 --- a/cli/src/pcluster/validators/utils.py +++ b/cli/src/pcluster/validators/utils.py @@ -12,7 +12,54 @@ # This module contains all the classes representing the Resources objects. # These objects are obtained from the configuration file through a conversion based on the Schema classes. # +from typing import Any, Union + +BOOLEAN_VALUES = ["true", "false"] def get_bucket_name_from_s3_url(import_path): return import_path.split("/")[2] + + +def str_to_bool(string: str = None) -> bool: + """Convert string to boolean value. + + Args: + string: String to convert. Defaults to None. + + Returns: + True if string is "true" (case-insensitive), False otherwise. + """ + return str(string).lower() == "true" + + +def is_boolean_string(value: str) -> bool: + """Check if value is a valid boolean string. + + Args: + value: String value to check. + + Returns: + True if value is "true" or "false" (case-insensitive), False otherwise. + """ + return str(value).lower() in BOOLEAN_VALUES + + +def dig(dictionary: dict, *keys: str) -> Union[dict, None, Any]: + """Navigate nested dictionary using key path. + + Args: + dictionary: Dictionary to navigate. + *keys: Sequence of keys to traverse. + + Returns: + Value at the specified key path, or None if path doesn't exist. + """ + if dictionary is None: + return None + value = dictionary + for key in keys: + if value is None or not isinstance(value, dict): + return None + value = value.get(key) + return value diff --git a/cli/tests/pcluster/validators/test_all_validators.py b/cli/tests/pcluster/validators/test_all_validators.py index 4a5eec7988..34dc425ba6 100644 --- a/cli/tests/pcluster/validators/test_all_validators.py +++ b/cli/tests/pcluster/validators/test_all_validators.py @@ -19,6 +19,7 @@ from pcluster.validators import ( cluster_validators, database_validators, + dev_settings_validators, ebs_validators, ec2_validators, feature_validators, @@ -67,6 +68,7 @@ async def _validate_async(*args, **kwargs): modules = [ cluster_validators, database_validators, + dev_settings_validators, ebs_validators, ec2_validators, fsx_validators, @@ -282,6 +284,10 @@ def test_slurm_validators_are_called_with_correct_argument(test_datadir, mocker) compute_resource_tags_validator = mocker.patch( tags_validators + ".ComputeResourceTagsValidator._validate", return_value=[] ) + dev_settings_validators = validators_path + ".dev_settings_validators" + extra_chef_attributes_validator = mocker.patch( + dev_settings_validators + ".ExtraChefAttributesValidator._validate", return_value=[] + ) mocker.patch( "pcluster.config.cluster_config.HeadNode.architecture", new_callable=PropertyMock(return_value="x86_64") ) @@ -451,6 +457,8 @@ def test_slurm_validators_are_called_with_correct_argument(test_datadir, mocker) any_order=True, ) + extra_chef_attributes_validator.assert_has_calls([call(extra_chef_attributes='{"attr1": {"attr2": "value"}}')]) + # No assertion on the argument for minor validators name_validator.assert_called() feature_region_validator.assert_called() diff --git a/cli/tests/pcluster/validators/test_all_validators/test_slurm_validators_are_called_with_correct_argument/slurm.yaml b/cli/tests/pcluster/validators/test_all_validators/test_slurm_validators_are_called_with_correct_argument/slurm.yaml index 4b83e8a1af..5ca5cbfde5 100644 --- a/cli/tests/pcluster/validators/test_all_validators/test_slurm_validators_are_called_with_correct_argument/slurm.yaml +++ b/cli/tests/pcluster/validators/test_all_validators/test_slurm_validators_are_called_with_correct_argument/slurm.yaml @@ -101,3 +101,7 @@ Tags: Value: String - Key: cluster_tag2 Value: String +DevSettings: + Cookbook: + ExtraChefAttributes: | + {"attr1": {"attr2": "value"}} \ No newline at end of file diff --git a/cli/tests/pcluster/validators/test_dev_settings_validators.py b/cli/tests/pcluster/validators/test_dev_settings_validators.py new file mode 100644 index 0000000000..8767688328 --- /dev/null +++ b/cli/tests/pcluster/validators/test_dev_settings_validators.py @@ -0,0 +1,69 @@ +# Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance +# with the License. A copy of the License is located at +# +# http://aws.amazon.com/apache2.0/ +# +# or in the "LICENSE.txt" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES +# OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and +# limitations under the License. +import pytest + +from pcluster.validators.common import FailureLevel +from pcluster.validators.dev_settings_validators import ExtraChefAttributesValidator +from tests.pcluster.validators.utils import assert_failure_level, assert_failure_messages + + +@pytest.mark.parametrize( + "extra_chef_attributes, expected_message, expected_failure_level", + [ + pytest.param(None, None, None, id="No extra chef attributes"), + pytest.param('{"other_attribute": "value"}', None, None, id="in_place_update_on_fleet_enabled not set"), + pytest.param( + '{"cluster": {"in_place_update_on_fleet_enabled": "true"}}', + None, + None, + id="in_place_update_true_string 'true' passes", + ), + pytest.param( + '{"cluster": {"in_place_update_on_fleet_enabled": true}}', + None, + None, + id="in_place_update_true_string true passes", + ), + pytest.param( + '{"cluster": {"in_place_update_on_fleet_enabled": "false"}}', + "When in-place updates are disabled, cluster updates are applied " + "by replacing compute and login nodes according to the selected QueueUpdateStrategy.", + FailureLevel.WARNING, + id="in_place_update_true_string 'false' throws a warning", + ), + pytest.param( + '{"cluster": {"in_place_update_on_fleet_enabled": false}}', + "When in-place updates are disabled, cluster updates are applied " + "by replacing compute and login nodes according to the selected QueueUpdateStrategy.", + FailureLevel.WARNING, + id="in_place_update_true_string false throws a warning", + ), + pytest.param( + '{"cluster": {"in_place_update_on_fleet_enabled": "invalid"}}', + "Invalid value in DevSettings/Cookbook/ExtraChefAttributes: " + "attribute 'in_place_update_on_fleet_enabled' must be a boolean value.", + FailureLevel.ERROR, + id="in_place_update_true_string invalid (string) throws an error", + ), + pytest.param( + '{"cluster": {"in_place_update_on_fleet_enabled": 123}}', + "Invalid value in DevSettings/Cookbook/ExtraChefAttributes: " + "attribute 'in_place_update_on_fleet_enabled' must be a boolean value.", + FailureLevel.ERROR, + id="n_place_update_true_string invalid (number) throws an error", + ), + ], +) +def test_extra_chef_attributes_validator(extra_chef_attributes, expected_message, expected_failure_level): + actual_failures = ExtraChefAttributesValidator().execute(extra_chef_attributes=extra_chef_attributes) + assert_failure_messages(actual_failures, expected_message) + if expected_failure_level: + assert_failure_level(actual_failures, expected_failure_level) diff --git a/cli/tests/pcluster/validators/test_utils.py b/cli/tests/pcluster/validators/test_utils.py new file mode 100644 index 0000000000..1d0ea151c2 --- /dev/null +++ b/cli/tests/pcluster/validators/test_utils.py @@ -0,0 +1,86 @@ +# Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance +# with the License. A copy of the License is located at +# +# http://aws.amazon.com/apache2.0/ +# +# or in the "LICENSE.txt" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES +# OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and +# limitations under the License. +import pytest + +from pcluster.validators.utils import dig, is_boolean_string, str_to_bool + + +@pytest.mark.parametrize( + "string_value, expected_result", + [ + # Valid cases + ("true", True), + ("True", True), + ("TRUE", True), + ("false", False), + ("False", False), + ("FALSE", False), + # Invalid cases + ("yes", False), + ("no", False), + ("1", False), + ("0", False), + ("", False), + (None, False), + ], +) +def test_str_to_bool(string_value, expected_result): + assert str_to_bool(string_value) == expected_result + + +@pytest.mark.parametrize( + "value, expected_result", + [ + # Valid cases + ("true", True), + ("True", True), + ("TRUE", True), + ("false", True), + ("False", True), + ("FALSE", True), + (True, True), + (False, True), + # Invalid cases + (None, False), + ("", False), + ("yes", False), + ("no", False), + ("1", False), + ("0", False), + (1, False), + (0, False), + ], +) +def test_is_boolean_string(value, expected_result): + assert is_boolean_string(value) == expected_result + + +@pytest.mark.parametrize( + "dictionary, keys, expected_result", + [ + # Cases where value is found + ({"a": {"b": {"c": "value"}}}, ("a", "b", "c"), "value"), + ({"a": {"b": "value"}}, ("a", "b"), "value"), + ({"a": "value"}, ("a",), "value"), + ({"a": {"b": {"c": "value"}}}, ("a", "b"), {"c": "value"}), + # Cases where value is not found + ({"a": {"b": {"c": "value"}}}, ("a", "nonexistent"), None), + ({"a": {"b": {"c": "value"}}}, ("nonexistent",), None), + ({"a": {"b": {"c": "value"}}}, ("a", "b", "c", "d"), None), + ({}, ("a",), None), + (None, ("a",), None), + ({"a": None}, ("a", "b"), None), + ({"a": "not_dict"}, ("a", "b"), None), + ({"a": {"b": None}}, ("a", "b", "c"), None), + ], +) +def test_dig(dictionary, keys, expected_result): + assert dig(dictionary, *keys) == expected_result