Skip to content

Specify working directory of imagebuilder to avoid executing in /tmp - #7515

Merged
hanwen-cluster merged 3 commits into
aws:developfrom
hanwen-cluster:proxy
Aug 7, 2026
Merged

Specify working directory of imagebuilder to avoid executing in /tmp#7515
hanwen-cluster merged 3 commits into
aws:developfrom
hanwen-cluster:proxy

Conversation

@hanwen-cluster

@hanwen-cluster hanwen-cluster commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Description of changes

  • Specify working directory of imagebuilder to avoid executing in /tmp
  • Add integration tests
  • Reuse ami-patching.yaml to create an AMI with noexec on /tmp
  • Fix ami-patching.yaml cleanup failure

See commits descriptions for details

Tests

The following tests have passed

  • test_essential_features
  • test_build_image
  • test_multiple_nics

References

  • Link to impacted open issues.
  • Link to related PRs in other packages (i.e. cookbook, node).
  • Link to documentation useful to understand the changes.

Checklist

  • Make sure you are pointing to the right branch.
  • If you're creating a patch for a branch other than develop add the branch name as prefix in the PR title (e.g. [release-3.6]).
  • Check all commits' messages are clear, describing what and why vs how.
  • Make sure to have added unit tests or integration tests to cover the new/modified code.
  • Check if documentation is impacted by this change.

Please review the guidelines for contributing and Pull Request Instructions.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@hanwen-cluster
hanwen-cluster requested review from a team as code owners July 30, 2026 18:50
@hanwen-cluster
hanwen-cluster force-pushed the proxy branch 2 times, most recently from 42106e3 to 699b311 Compare July 30, 2026 18:52
Cookbook:
ExtraChefAttributes: |
{"cluster": {"dcv": {"install_enabled": false}}}
{% endif %}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DCV knob is removed because the code is dead

Comment thread tests/integration-tests/tests/createami/test_createami.py
hgreebe
hgreebe previously approved these changes Jul 30, 2026
@hanwen-cluster
hanwen-cluster force-pushed the proxy branch 2 times, most recently from 81032f4 to 6bd5942 Compare August 4, 2026 19:50
@hanwen-cluster hanwen-cluster changed the title [integ-tests] Set noexec on /tmp for some tests Specify working directory of imagebuilder to avoid executing in /tmp Aug 4, 2026
@hanwen-cluster
hanwen-cluster force-pushed the proxy branch 3 times, most recently from aed8e9e to 9e68425 Compare August 4, 2026 20:22
@hanwen-cluster
hanwen-cluster force-pushed the proxy branch 4 times, most recently from 88046b1 to e2ea64e Compare August 5, 2026 18:42
@@ -0,0 +1,288 @@
AWSTemplateFormatVersion: 2010-09-09

@himani2411 himani2411 Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refrain from using words like hardened, STIG,etc which can cause confusion with the scope of this feature. So I suggest you remove the mention of these words from this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@hanwen-cluster
hanwen-cluster force-pushed the proxy branch 4 times, most recently from d946601 to 9f5e5d3 Compare August 5, 2026 22:08
Comment thread cloudformation/patching/ami-patching.yaml Outdated
himani2411
himani2411 previously approved these changes Aug 6, 2026
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
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`.
WorkingDirectory: /opt/parallelcluster/tmp
Components:
- ComponentArn: !Ref PatchComponent
- ComponentArn: !If [BuildNoExecTmpImage, !Ref NoExecTmpComponent, !Ref PatchComponent]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need a separate gate? Can;t we just go with a patch component that does not make use of exec in tmp?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This template is reused for two purposes. One is to create an patched AMI, the other one is to create an AMI with noexec on tmp. Depending on the input parameters, we choose one component

DevSettings:
EfaInterfaceType: efa
Cookbook:
ExtraChefAttributes: '{"cluster": {"tmp_noexec": "true"}}' No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

future improvement: we can make this a test flag thta is injected via test config rather than hard wiring

@hanwen-cluster
hanwen-cluster merged commit 1a04193 into aws:develop Aug 7, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants