diff --git a/cli/src/pcluster/config/imagebuilder_config.py b/cli/src/pcluster/config/imagebuilder_config.py index 074e5c24c6..f57be84626 100644 --- a/cli/src/pcluster/config/imagebuilder_config.py +++ b/cli/src/pcluster/config/imagebuilder_config.py @@ -13,6 +13,7 @@ # These objects are obtained from the configuration file through a conversion based on the Schema classes. # +import json from typing import List from pcluster.aws.common import get_region @@ -27,6 +28,7 @@ ) from pcluster.imagebuilder_utils import ROOT_VOLUME_TYPE from pcluster.validators.common import ValidatorContext +from pcluster.validators.dev_settings_validators import CliAttributeOverridesValidator from pcluster.validators.ebs_validators import EbsVolumeTypeSizeValidator from pcluster.validators.ec2_validators import InstanceTypeBaseAMICompatibleValidator from pcluster.validators.iam_validators import IamPolicyValidator, InstanceProfileValidator, RoleValidator @@ -240,8 +242,7 @@ def __init__( distribution_configuration: DistributionConfiguration = None, terminate_instance_on_failure: bool = None, disable_validate_and_test: bool = None, - cinc_installer_url: str = None, - cinc_version: str = None, + cli_attribute_overrides: str = None, disable_kernel_update: bool = None, slurm_patches_s3_archive: str = None, **kwargs @@ -251,14 +252,36 @@ def __init__( self.distribution_configuration = distribution_configuration self.terminate_instance_on_failure = Resource.init_param(terminate_instance_on_failure, default=True) self.disable_validate_and_test = Resource.init_param(disable_validate_and_test, default=True) - self.cinc_installer_url = Resource.init_param(cinc_installer_url, default="") - # Optional override for the CINC client version baked into the AMI - # (CfnParamCincVersion -> ChefVersion in parallelcluster.yaml). When - # left empty the YAML-rendered default ChefVersion is used. - self.cinc_version = Resource.init_param(cinc_version, default="") + # JSON string of CLI-side build-image overrides (e.g. cinc_version, + # cinc_installer_url), unfurled by the accessors below. Mirrors + # Cookbook.ExtraChefAttributes: freeform JSON, well-formedness only. + self.cli_attribute_overrides = Resource.init_param(cli_attribute_overrides, default="") self.disable_kernel_update = Resource.init_param(disable_kernel_update, default=False) self.slurm_patches_s3_archive = Resource.init_param(slurm_patches_s3_archive, default="") + def _cli_attribute_override(self, key): + """Unfurl a single value from the cli_attribute_overrides JSON blob.""" + if not self.cli_attribute_overrides: + return "" + return json.loads(self.cli_attribute_overrides).get(key, "") + + @property + def cinc_installer_url(self): + """CINC installer URL override (from cli_attribute_overrides).""" + return self._cli_attribute_override("cinc_installer_url") + + @property + def cinc_version(self): + """CINC client version override (from cli_attribute_overrides).""" + return self._cli_attribute_override("cinc_version") + + def _register_validators(self, context: ValidatorContext = None): + super()._register_validators(context) + if self.cli_attribute_overrides: + self._register_validator( + CliAttributeOverridesValidator, cli_attribute_overrides=self.cli_attribute_overrides + ) + class ImagebuilderDeploymentSettings(BaseDeploymentSettings): """Represent the deployment settings for the ImageBuilder.""" diff --git a/cli/src/pcluster/schemas/imagebuilder_schema.py b/cli/src/pcluster/schemas/imagebuilder_schema.py index e6e1061c2d..a11ee147f4 100644 --- a/cli/src/pcluster/schemas/imagebuilder_schema.py +++ b/cli/src/pcluster/schemas/imagebuilder_schema.py @@ -242,8 +242,7 @@ class ImagebuilderDevSettingsSchema(BaseDevSettingsSchema): distribution_configuration = fields.Nested(DistributionConfigurationSchema) terminate_instance_on_failure = fields.Bool() disable_validate_and_test = fields.Bool() - cinc_installer_url = fields.Str() - cinc_version = fields.Str() + cli_attribute_overrides = fields.Str() disable_kernel_update = fields.Bool() @post_load diff --git a/cli/src/pcluster/validators/dev_settings_validators.py b/cli/src/pcluster/validators/dev_settings_validators.py index 73e6e66984..72a9b61841 100644 --- a/cli/src/pcluster/validators/dev_settings_validators.py +++ b/cli/src/pcluster/validators/dev_settings_validators.py @@ -19,6 +19,33 @@ ATTR_CLUSTER_READINESS_CHECK_IGNORE_FAILURE = "cluster.cluster_readiness_check_ignore_failure" MIN_SLURM_RECONFIGURE_TIMEOUT = 300 +CLI_ATTRIBUTE_OVERRIDES_PATH = "DevSettings/CliAttributeOverrides" +# Keys the CLI currently unfurls from the blob. Kept for an optional future +# strict check; today validation is well-formedness only (like ExtraChefAttributes). +KNOWN_CLI_ATTRIBUTE_OVERRIDES = {"cinc_version", "cinc_installer_url"} + + +class CliAttributeOverridesValidator(Validator): + """Validate DevSettings/CliAttributeOverrides is a well-formed JSON object.""" + + def _validate(self, cli_attribute_overrides: str = None): + if not cli_attribute_overrides: + return + try: + attrs = json.loads(cli_attribute_overrides) + except ValueError: + self._add_failure( + f"Invalid value in {CLI_ATTRIBUTE_OVERRIDES_PATH}: must be a valid JSON string.", + FailureLevel.ERROR, + ) + return + if not isinstance(attrs, dict): + self._add_failure( + f"Invalid value in {CLI_ATTRIBUTE_OVERRIDES_PATH}: must be a JSON object.", + FailureLevel.ERROR, + ) + # TODO: optionally validate keys against KNOWN_CLI_ATTRIBUTE_OVERRIDES here. + class ExtraChefAttributesValidator(Validator): """Validate DevSettings/Cookbook/ExtraChefAttributes.""" diff --git a/cli/tests/pcluster/config/test_imagebuilder_config.py b/cli/tests/pcluster/config/test_imagebuilder_config.py new file mode 100644 index 0000000000..639db8d927 --- /dev/null +++ b/cli/tests/pcluster/config/test_imagebuilder_config.py @@ -0,0 +1,43 @@ +# Copyright 2026 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 assertpy import assert_that + +from pcluster.config.imagebuilder_config import ImagebuilderDevSettings + + +@pytest.mark.parametrize( + "cli_attribute_overrides, expected_cinc_version, expected_cinc_installer_url", + [ + pytest.param(None, "", "", id="unset blob -> empty overrides"), + pytest.param("", "", "", id="empty blob -> empty overrides"), + pytest.param( + '{"cinc_version": "18.8.54", "cinc_installer_url": "https://omnitruck.cinc.sh/install.sh"}', + "18.8.54", + "https://omnitruck.cinc.sh/install.sh", + id="both keys unfurled", + ), + pytest.param('{"cinc_version": "18.8.54"}', "18.8.54", "", id="only cinc_version present"), + pytest.param( + '{"cinc_installer_url": "https://omnitruck.cinc.sh/install.sh"}', + "", + "https://omnitruck.cinc.sh/install.sh", + id="only cinc_installer_url present", + ), + pytest.param('{"unrelated": "value"}', "", "", id="unknown keys ignored"), + ], +) +def test_dev_settings_cli_attribute_overrides_unfurl( + cli_attribute_overrides, expected_cinc_version, expected_cinc_installer_url +): + dev_settings = ImagebuilderDevSettings(cli_attribute_overrides=cli_attribute_overrides) + assert_that(dev_settings.cinc_version).is_equal_to(expected_cinc_version) + assert_that(dev_settings.cinc_installer_url).is_equal_to(expected_cinc_installer_url) diff --git a/cli/tests/pcluster/validators/test_all_validators.py b/cli/tests/pcluster/validators/test_all_validators.py index 252415a518..f1042602d0 100644 --- a/cli/tests/pcluster/validators/test_all_validators.py +++ b/cli/tests/pcluster/validators/test_all_validators.py @@ -166,6 +166,9 @@ def test_slurm_all_validators_are_called(test_datadir, mocker): if m["name"] in ["TagKeyValidator", "ClusterNameValidator", "InstanceProfileValidator", "RoleValidator"]: # ToDo: Reserved tag keys to be aligned between cluster and image builder continue + if m["name"] in ["CliAttributeOverridesValidator"]: + # Image-builder-only validator (DevSettings/CliAttributeOverrides); not run on cluster config. + continue print(f"Checking validator of class \"{m['name']}\" is called") m["mocker"].assert_called() diff --git a/cli/tests/pcluster/validators/test_dev_settings_validators.py b/cli/tests/pcluster/validators/test_dev_settings_validators.py index 82ee6b632f..184ba01304 100644 --- a/cli/tests/pcluster/validators/test_dev_settings_validators.py +++ b/cli/tests/pcluster/validators/test_dev_settings_validators.py @@ -11,7 +11,10 @@ import pytest from pcluster.validators.common import FailureLevel -from pcluster.validators.dev_settings_validators import ExtraChefAttributesValidator +from pcluster.validators.dev_settings_validators import ( + CliAttributeOverridesValidator, + ExtraChefAttributesValidator, +) from tests.pcluster.validators.utils import assert_failure_level, assert_failure_messages @@ -219,3 +222,42 @@ def test_extra_chef_attributes_validator_cluster_readiness_check_ignore_failure( assert_failure_messages(actual_failures, expected_message) if expected_failure_level: assert_failure_level(actual_failures, expected_failure_level) + + +@pytest.mark.parametrize( + "cli_attribute_overrides, expected_message, expected_failure_level", + [ + pytest.param("", None, None, id="empty string is valid"), + pytest.param(None, None, None, id="None is valid"), + pytest.param( + '{"cinc_version": "18.8.54", "cinc_installer_url": "https://omnitruck.cinc.sh/install.sh"}', + None, + None, + id="valid JSON object", + ), + pytest.param("{}", None, None, id="empty JSON object is valid"), + pytest.param( + "{not valid json", + "Invalid value in DevSettings/CliAttributeOverrides: must be a valid JSON string.", + FailureLevel.ERROR, + id="malformed JSON throws error", + ), + pytest.param( + '["cinc_version"]', + "Invalid value in DevSettings/CliAttributeOverrides: must be a JSON object.", + FailureLevel.ERROR, + id="JSON array (not object) throws error", + ), + pytest.param( + '"just a string"', + "Invalid value in DevSettings/CliAttributeOverrides: must be a JSON object.", + FailureLevel.ERROR, + id="JSON scalar (not object) throws error", + ), + ], +) +def test_cli_attribute_overrides_validator(cli_attribute_overrides, expected_message, expected_failure_level): + actual_failures = CliAttributeOverridesValidator().execute(cli_attribute_overrides=cli_attribute_overrides) + assert_failure_messages(actual_failures, expected_message) + if expected_failure_level: + assert_failure_level(actual_failures, expected_failure_level)