From e6f6ea44326192fb9e557801fadf0d8a33a9c8af Mon Sep 17 00:00:00 2001 From: hanwenli Date: Wed, 5 Aug 2026 11:41:04 -0700 Subject: [PATCH 1/3] Fix ami-patching.yaml cleanup failure 1. The ARN of EBS snapshot should't contain account id. 2. The tagging of EBS snapshots inside the Lambda function is not necessary because imagebuilder propagates the tags to snapshots --- cloudformation/patching/ami-patching.yaml | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/cloudformation/patching/ami-patching.yaml b/cloudformation/patching/ami-patching.yaml index 443feceb8d..74d8b04275 100644 --- a/cloudformation/patching/ami-patching.yaml +++ b/cloudformation/patching/ami-patching.yaml @@ -85,9 +85,7 @@ Resources: urllib.request.urlopen(req) def cleanup(stack_name): - # Deregister the patched AMI(s) built by this stack and delete their - # snapshots. The snapshots are tagged first so DeleteSnapshot is allowed - # by the (tag-scoped) IAM policy. + # Deregister the patched AMI(s) built by this stack and delete their snapshots. if not stack_name: return images = ec2.describe_images(Owners=["self"], Filters=[ @@ -95,9 +93,6 @@ Resources: for img in images: snaps = [m["Ebs"]["SnapshotId"] for m in img.get("BlockDeviceMappings", []) if m.get("Ebs", {}).get("SnapshotId")] - if snaps: - ec2.create_tags(Resources=snaps, Tags=[ - {"Key": "parallelcluster:ami-patching-stack", "Value": stack_name}]) ec2.deregister_image(ImageId=img["ImageId"]) for snap in snaps: ec2.delete_snapshot(SnapshotId=snap) @@ -142,12 +137,6 @@ Resources: - Effect: Allow Action: ec2:DescribeImages Resource: "*" - - Effect: Allow - Action: ec2:CreateTags - Resource: !Sub arn:${AWS::Partition}:ec2:${AWS::Region}:${AWS::AccountId}:snapshot/* - Condition: - StringEquals: - aws:RequestTag/parallelcluster:ami-patching-stack: !Ref AWS::StackName - Effect: Allow Action: ec2:DeregisterImage Resource: !Sub arn:${AWS::Partition}:ec2:${AWS::Region}::image/* @@ -156,7 +145,7 @@ Resources: aws:ResourceTag/parallelcluster:ami-patching-stack: !Ref AWS::StackName - Effect: Allow Action: ec2:DeleteSnapshot - Resource: !Sub arn:${AWS::Partition}:ec2:${AWS::Region}:${AWS::AccountId}:snapshot/* + Resource: !Sub arn:${AWS::Partition}:ec2:${AWS::Region}::snapshot/* Condition: StringEquals: aws:ResourceTag/parallelcluster:ami-patching-stack: !Ref AWS::StackName From 0f958cd1dc8b6d309fe70a93b2d8c02fb617bc7e Mon Sep 17 00:00:00 2001 From: hanwenli Date: Wed, 5 Aug 2026 11:41:15 -0700 Subject: [PATCH 2/3] Specify working directory of imagebuilder to avoid executing in /tmp This commit also set `noexec` on `/tmp` for some tests to monitor ParallelCluster compliance of `noexec` on `/tmp`: 1. test_essential_features: now `noexec` is set on `/tmp` by default. This commit fixes the `gpu_job.sh` to not execute on `/tmp` 2. test_multiple_nics: now `noexec` is set on `/tmp` by default. This test is added because multi-nics configuration script was executed in `/tmp` before pcluster 3.16.0 3. test_build_image: If the test is run on a non-GPU instance, where Nvidia is not installed, create a custom AMI with `noexec` persisted on `/tmp` and run `build-image` on top of this custom AMI. Using the custom AMI has better coverage than (1) and (2), because the `noexec` persisted on `/tmp` in the custom AMI covers the whole workflow of `build-image` and `create-cluster`. --- CHANGELOG.md | 2 +- .../pcluster/templates/imagebuilder_stack.py | 2 + cloudformation/patching/ami-patching.yaml | 117 +++++++++++++++--- tests/integration-tests/configs/develop.yaml | 4 + tests/integration-tests/conftest.py | 41 ++++++ .../pcluster.config.yaml | 3 + .../tests/common/data/gpu_job.sh | 6 +- .../tests/createami/test_createami.py | 13 +- .../test_build_image/image.config.yaml | 5 - .../test_multiple_nics/pcluster.config.yaml | 2 + 10 files changed, 164 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4085c55187..b3e9381f85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ CHANGELOG - Improve cluster update resiliency on login nodes by reusing the head-node-driven orchestration already in place on compute nodes, removing the dependency on cfn-hup and cfn-init. - Move all ParallelCluster-managed bootstrap files off `/tmp` into a dedicated `/opt/parallelcluster/tmp` - directory. Therefore, Image builds, cluster creations and updates work on custom AMIs that mount `/tmp` with `noexec`. + directory. Therefore, cluster creations and updates work on custom AMIs that mount `/tmp` with `noexec`. Image builds work on the custom AMIs only if GDRcopy installation is skipped. **CHANGES** - The validator `ClusterNameValidator` now enforces cluster names to be limited to 40 characters when using `ExternalSlurmdbd`, diff --git a/cli/src/pcluster/templates/imagebuilder_stack.py b/cli/src/pcluster/templates/imagebuilder_stack.py index 69edd490ee..e14b7ca6ec 100644 --- a/cli/src/pcluster/templates/imagebuilder_stack.py +++ b/cli/src/pcluster/templates/imagebuilder_stack.py @@ -41,6 +41,7 @@ PCLUSTER_IMAGE_NAME_TAG, PCLUSTER_S3_BUCKET_TAG, PCLUSTER_S3_IMAGE_DIR_TAG, + PCLUSTER_TMP_DIR, PCLUSTER_VERSION_TAG, ) from pcluster.imagebuilder_utils import ( @@ -347,6 +348,7 @@ def _add_imagebuilder_image_recipe(self, build_tags, components): version=utils.get_installed_version(base_version_only=True), tags=build_tags, parent_image=self.config.build.parent_image, + working_directory=PCLUSTER_TMP_DIR, components=components, block_device_mappings=[ imagebuilder.CfnImageRecipe.InstanceBlockDeviceMappingProperty( diff --git a/cloudformation/patching/ami-patching.yaml b/cloudformation/patching/ami-patching.yaml index 74d8b04275..4a8833c0b8 100644 --- a/cloudformation/patching/ami-patching.yaml +++ b/cloudformation/patching/ami-patching.yaml @@ -4,7 +4,7 @@ Description: >- Parameters: ParentImage: - Description: The ParallelCluster AMI to patch. + Description: The AMI to patch. Type: String InstanceType: Description: Instance type used by Image Builder to build the patched AMI. @@ -18,6 +18,7 @@ Parameters: PatchScriptS3Uri: Description: S3 URI (s3://bucket/key) of the patching script to run on the build instance. Type: String + Default: "" PatchFlavour: Description: >- Patching flavour passed to the patch script. 'minimal' applies only security @@ -31,6 +32,17 @@ Parameters: - full - minimal-capped - full-capped + EnableNoExecTmp: + Description: Build an AMI with noexec, nosuid, and nodev mount options on /tmp instead of applying patches. + Type: String + Default: "false" + AllowedValues: + - "true" + - "false" + +Conditions: + BuildNoExecTmpImage: !Equals [!Ref EnableNoExecTmp, "true"] + BuildPatchedImage: !Equals [!Ref EnableNoExecTmp, "false"] Resources: @@ -153,8 +165,10 @@ Resources: # =========================================================================== # Image Builder # - # Builds the patched AMI: the build instance downloads and runs the patching - # script, reboots, executes the AMI cleanup and create the new AMI. + # Builds an AMI using the selected component: by default the build instance + # downloads and runs the patching script; when EnableNoExecTmp is true, it + # configures restrictive /tmp mount options instead. Both paths reboot, run + # the AMI cleanup, and create a new AMI. # =========================================================================== PatchedImage: @@ -173,11 +187,13 @@ Resources: Name: !Sub pcluster-ami-patching-recipe-${AWS::StackName} Version: 1.0.0 ParentImage: !Ref ParentImage + WorkingDirectory: /opt/parallelcluster/tmp Components: - - ComponentArn: !Ref PatchComponent + - ComponentArn: !If [BuildNoExecTmpImage, !Ref NoExecTmpComponent, !Ref PatchComponent] PatchComponent: Type: AWS::ImageBuilder::Component + Condition: BuildPatchedImage Properties: Name: !Sub pcluster-ami-patching-${AWS::StackName} Platform: Linux @@ -222,6 +238,66 @@ Resources: commands: - /usr/local/sbin/ami_cleanup.sh + NoExecTmpComponent: + Type: AWS::ImageBuilder::Component + Condition: BuildNoExecTmpImage + Properties: + Name: !Sub pcluster-tmp-noexec-${AWS::StackName} + Platform: Linux + Version: 1.0.0 + Description: Persist and validate noexec, nosuid, and nodev mount options on /tmp. + Data: | + name: ConfigureNoExecTmpMount + description: Persist and validate restrictive mount options on /tmp. + schemaVersion: 1.0 + phases: + - name: build + steps: + - name: ConfigureTmpMount + action: ExecuteBash + inputs: + commands: + - | + set -euxo pipefail + awk ' + function add_option(options_list, option, item_count, i, items) { + item_count = split(options_list, items, ",") + for (i = 1; i <= item_count; i++) { + if (items[i] == option) return options_list + } + return options_list "," option + } + BEGIN { found = 0 } + $1 !~ /^#/ && $2 == "/tmp" { + $4 = add_option($4, "noexec") + $4 = add_option($4, "nosuid") + $4 = add_option($4, "nodev") + found = 1 + } + { print } + END { + if (!found) print "/tmp", "/tmp", "none", "bind,rw,nodev,nosuid,noexec", "0", "0" + } + ' OFS='\t' /etc/fstab > /etc/fstab.pcluster-tmp-noexec + install -o root -g root -m 0644 /etc/fstab.pcluster-tmp-noexec /etc/fstab + rm -f /etc/fstab.pcluster-tmp-noexec + findmnt --verify --tab-file /etc/fstab + - name: Reboot + action: Reboot + - name: VerifyTmpMountAfterReboot + action: ExecuteBash + inputs: + commands: + - | + set -euxo pipefail + options_list=$(findmnt -no OPTIONS --target /tmp) + for option in noexec nosuid nodev; do + echo "$options_list" | tr ',' '\n' | grep -Fx "$option" + done + if [[ -x /usr/local/sbin/ami_cleanup.sh ]]; then + /usr/local/sbin/ami_cleanup.sh + fi + RecipeLogGroup: Type: AWS::Logs::LogGroup DeletionPolicy: Retain @@ -251,13 +327,16 @@ Resources: Distributions: - Region: !Ref AWS::Region AmiDistributionConfiguration: - # The distributed AMI name is "patched--". The + # Patched AMIs use "patched--". The # "patched-" prefix keeps the name from matching the pcluster AMI name # filter used to discover base AMIs, so the patched AMI is only consumed # explicitly by id and never picked up as the "latest" AMI on updates. - Name: !Sub - - "patched-${SourceName}-{{ imagebuilder:buildDate }}" - - SourceName: !GetAtt AmiHelper.SourceName + Name: !If + - BuildNoExecTmpImage + - !Sub "tmp-noexec-${AWS::StackName}-{{ imagebuilder:buildDate }}" + - !Sub + - "patched-${SourceName}-{{ imagebuilder:buildDate }}" + - SourceName: !GetAtt AmiHelper.SourceName AmiTags: parallelcluster:ami-patching-stack: !Ref AWS::StackName parallelcluster:source-ami: !Ref ParentImage @@ -281,16 +360,18 @@ Resources: ManagedPolicyArns: - !Sub arn:${AWS::Partition}:iam::aws:policy/EC2InstanceProfileForImageBuilder - !Sub arn:${AWS::Partition}:iam::aws:policy/AmazonSSMManagedInstanceCore - Policies: - - PolicyName: read-patch-script - PolicyDocument: - Version: 2012-10-17 - Statement: - - Effect: Allow - Action: s3:GetObject - Resource: !Sub - - arn:${AWS::Partition}:s3:::${BucketAndKey} - - BucketAndKey: !Select [1, !Split ["s3://", !Ref PatchScriptS3Uri]] + Policies: !If + - BuildNoExecTmpImage + - !Ref AWS::NoValue + - - PolicyName: read-patch-script + PolicyDocument: + Version: 2012-10-17 + Statement: + - Effect: Allow + Action: s3:GetObject + Resource: !Sub + - arn:${AWS::Partition}:s3:::${BucketAndKey} + - BucketAndKey: !Select [1, !Split ["s3://", !Ref PatchScriptS3Uri]] BuildSecurityGroup: Type: AWS::EC2::SecurityGroup diff --git a/tests/integration-tests/configs/develop.yaml b/tests/integration-tests/configs/develop.yaml index 8f51def088..467c94fc42 100644 --- a/tests/integration-tests/configs/develop.yaml +++ b/tests/integration-tests/configs/develop.yaml @@ -147,6 +147,10 @@ test-suites: instances: ["g4dn.2xlarge"] oss: {{ NO_RHEL_OS_X86 }} schedulers: ["slurm"] + - regions: ["us-east-1"] + instances: ["c5.4xlarge"] + oss: [{{ OS_X86_2 }}] + schedulers: ["slurm"] - regions: ["cnn1-az1"] instances: ["g4dn.2xlarge"] schedulers: ["slurm"] diff --git a/tests/integration-tests/conftest.py b/tests/integration-tests/conftest.py index 8b9ebb70b0..f26544c954 100644 --- a/tests/integration-tests/conftest.py +++ b/tests/integration-tests/conftest.py @@ -22,6 +22,7 @@ import time from functools import partial from itertools import product +from pathlib import Path from shutil import copyfile from traceback import format_tb from typing import Any, Dict, List, Optional, Union @@ -2474,3 +2475,43 @@ def _store_secret(region, secret_string=None, secret_binary=None): for secret_arn in secrets: logging.info("Deleting secret %s", secret_arn) secrets_manager_client.delete_secret(SecretId=secret_arn) + + +@pytest.fixture() +def noexec_tmp_ami_factory(region, vpc_stack, request, cfn_stacks_factory): + """Build temporary AMIs whose /tmp mount is noexec from early boot.""" + template_path = Path(__file__).resolve().parents[2] / "cloudformation" / "patching" / "ami-patching.yaml" + template_body = template_path.read_text(encoding="utf-8") + built = [] + + def _build(parent_image, builder_instance): + stack_name = generate_stack_name("integ-tests-tmp-noexec-ami", request.config.getoption("stackname_suffix")) + stack = CfnStack( + name=stack_name, + region=region, + template=template_body, + parameters=[ + {"ParameterKey": "ParentImage", "ParameterValue": parent_image}, + {"ParameterKey": "InstanceType", "ParameterValue": builder_instance}, + {"ParameterKey": "SubnetId", "ParameterValue": vpc_stack.get_public_subnet()}, + {"ParameterKey": "VpcId", "ParameterValue": vpc_stack.cfn_outputs["VpcId"]}, + {"ParameterKey": "EnableNoExecTmp", "ParameterValue": "true"}, + ], + capabilities=["CAPABILITY_IAM"], + ) + logging.info("Building a temporary /tmp-noexec AMI from %s", parent_image) + cfn_stacks_factory.create_stack(stack) + ami_id = stack.cfn_outputs["AmiId"] + boto3.client("ec2", region_name=region).get_waiter("image_available").wait(ImageIds=[ami_id]) + built.append((ami_id, stack_name)) + logging.info("Temporary /tmp-noexec AMI %s is available", ami_id) + return ami_id + + yield _build + if request.config.getoption("no_delete"): + logging.info("--no-delete specified: retaining temporary /tmp-noexec AMI(s) and stack(s): %s", built) + return + + for ami_id, stack_name in reversed(built): + logging.info("Deleting stack %s and temporary /tmp-noexec AMI %s", stack_name, ami_id) + cfn_stacks_factory.delete_stack(stack_name, region) diff --git a/tests/integration-tests/tests/basic/test_essential_features/test_essential_features/pcluster.config.yaml b/tests/integration-tests/tests/basic/test_essential_features/test_essential_features/pcluster.config.yaml index 4a5fe443ac..87fb306cd1 100644 --- a/tests/integration-tests/tests/basic/test_essential_features/test_essential_features/pcluster.config.yaml +++ b/tests/integration-tests/tests/basic/test_essential_features/test_essential_features/pcluster.config.yaml @@ -146,3 +146,6 @@ SharedStorage: - MountDir: /shared Name: name1 StorageType: Ebs +DevSettings: + Cookbook: + ExtraChefAttributes: '{"cluster": {"tmp_noexec": "true"}}' \ No newline at end of file diff --git a/tests/integration-tests/tests/common/data/gpu_job.sh b/tests/integration-tests/tests/common/data/gpu_job.sh index d744d841a1..529e3a152b 100644 --- a/tests/integration-tests/tests/common/data/gpu_job.sh +++ b/tests/integration-tests/tests/common/data/gpu_job.sh @@ -44,8 +44,10 @@ else exit 2 fi -WORKDIR=$(mktemp -d) -trap 'rm -rf "$WORKDIR"' EXIT +WORKDIR=$(sudo -n mktemp -d "/opt/parallelcluster/tmp/pcluster-cuda-samples.XXXXXX") +sudo -n chown "$(id -u):$(id -g)" "$WORKDIR" +export TMPDIR="$WORKDIR" +trap 'sudo rm -rf "$WORKDIR"' EXIT # Shared scaffolding required by every sample (Common/, top-level cmake/) cp -r "$SAMPLES_SRC"/{Common,cmake,CMakeLists.txt} "$WORKDIR"/ diff --git a/tests/integration-tests/tests/createami/test_createami.py b/tests/integration-tests/tests/createami/test_createami.py index 9dead474c9..b3bb840325 100644 --- a/tests/integration-tests/tests/createami/test_createami.py +++ b/tests/integration-tests/tests/createami/test_createami.py @@ -60,12 +60,11 @@ def _get_base_ami(region, os, architecture): Uses first-stage AMIs for RHEL/Rocky/Ubuntu (kernel version requirements), remarkable (Deep Learning) AMIs for ubuntu2204, and official AMIs for everything else. Returns (base_ami, feature_flags) where feature_flags is a dict with keys: - enable_nvidia, update_os_packages, enable_lustre_client, enable_dcv. + enable_nvidia, update_os_packages, enable_lustre_client. """ enable_nvidia = os not in ["ubuntu2404"] update_os_packages = os in ["alinux2023", "rocky9"] enable_lustre_client = True - enable_dcv = True if os in ["ubuntu2204"]: # Test Deep Learning AMIs @@ -87,7 +86,6 @@ def _get_base_ami(region, os, architecture): "enable_nvidia": enable_nvidia, "update_os_packages": update_os_packages, "enable_lustre_client": enable_lustre_client, - "enable_dcv": enable_dcv, } return base_ami, feature_flags @@ -189,6 +187,7 @@ def test_build_image( s3_bucket_factory, build_image_custom_resource, images_factory, + noexec_tmp_ami_factory, request, clusters_factory, scheduler_commands_factory, @@ -213,15 +212,19 @@ def test_build_image( # Get base AMI and feature flags base_ami, flags = _get_base_ami(region, os, architecture) + enable_nvidia = flags["enable_nvidia"] and get_gpu_count(instance) > 0 + if not enable_nvidia: + # Build from an AMI where /tmp is already noexec at boot. + base_ami = noexec_tmp_ami_factory(base_ami, instance) + image_config = pcluster_config_reader( config_file="image.config.yaml", parent_image=base_ami, instance_role=instance_role, bucket_name=bucket_name, - enable_nvidia=str(flags["enable_nvidia"] and get_gpu_count(instance) > 0).lower(), + enable_nvidia=str(enable_nvidia).lower(), update_os_packages=str(flags["update_os_packages"]).lower(), enable_lustre_client=str(flags["enable_lustre_client"]).lower(), - enable_dcv=str(flags["enable_dcv"]).lower(), ) image = images_factory(image_id, image_config, region) diff --git a/tests/integration-tests/tests/createami/test_createami/test_build_image/image.config.yaml b/tests/integration-tests/tests/createami/test_createami/test_build_image/image.config.yaml index 2a3a8bd9b1..db15a8ece3 100644 --- a/tests/integration-tests/tests/createami/test_createami/test_build_image/image.config.yaml +++ b/tests/integration-tests/tests/createami/test_createami/test_build_image/image.config.yaml @@ -36,8 +36,3 @@ DeploymentSettings: - {{ default_vpc_security_group_id }} DevSettings: TerminateInstanceOnFailure: True -{% if enable_dcv == "false" %} - Cookbook: - ExtraChefAttributes: | - {"cluster": {"dcv": {"install_enabled": false}}} -{% endif %} diff --git a/tests/integration-tests/tests/multiple_nics/test_multiple_nics/test_multiple_nics/pcluster.config.yaml b/tests/integration-tests/tests/multiple_nics/test_multiple_nics/test_multiple_nics/pcluster.config.yaml index cd67c2a59c..ce482a9313 100644 --- a/tests/integration-tests/tests/multiple_nics/test_multiple_nics/test_multiple_nics/pcluster.config.yaml +++ b/tests/integration-tests/tests/multiple_nics/test_multiple_nics/test_multiple_nics/pcluster.config.yaml @@ -41,3 +41,5 @@ SharedStorage: DeletionPolicy: Delete DevSettings: EfaInterfaceType: efa + Cookbook: + ExtraChefAttributes: '{"cluster": {"tmp_noexec": "true"}}' \ No newline at end of file From c06ad4cc91305317a66d9f2169ff098fbcf24d8a Mon Sep 17 00:00:00 2001 From: hanwenli Date: Wed, 5 Aug 2026 11:46:13 -0700 Subject: [PATCH 3/3] Delete unnecessary validate steps of ParallelCluster --- .../pcluster/resources/imagebuilder/parallelcluster.yaml | 9 --------- .../resources/imagebuilder/parallelcluster_validate.yaml | 0 2 files changed, 9 deletions(-) delete mode 100644 cli/src/pcluster/resources/imagebuilder/parallelcluster_validate.yaml diff --git a/cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml b/cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml index 890660e26c..5b67b3c883 100644 --- a/cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml +++ b/cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml @@ -296,12 +296,3 @@ phases: # Final cleanup rm -f /opt/parallelcluster/system_info /usr/local/sbin/ami_cleanup.sh "${CfnParamIsOfficialAmiBuild}" - - - name: validate - steps: - - name: PClusterValidate - action: ExecuteBash - inputs: - commands: - - | - echo "Check ParallelCluster software stack has been installed" diff --git a/cli/src/pcluster/resources/imagebuilder/parallelcluster_validate.yaml b/cli/src/pcluster/resources/imagebuilder/parallelcluster_validate.yaml deleted file mode 100644 index e69de29bb2..0000000000