fix(ecr-assets): incorrect handling of nested excludes in .dockerignore #34810

Abogical · 2025-06-25T12:57:39Z

Issue #13636

Reason for this change

Fixes a bug where asset copying does not correctly process .dockerignore files. CDK did not copy files excluded by a nested exclude pattern in .dockerignore.

Description of changes

Added a new method in DockerIgnoreStrategy that determines whether a directory is fully or partially ignored. The method is DockerIgnoreStrategy.completelyIgnores and if it returns true, it means that the directory is ignored AND that all of its children are ignored as well. For directories that are ignored but not "completely" ignored, the copyDirectory function will still walk through them.

As this changes the behaviour of asset copying, this can potientally be a breaking change. Therefore, a new feature flag is added which enables this fix. The feature flag is DOCKER_IGNORE_NESTED_EXCLUDE_FIX.

Describe any new or updated permissions being added

No new permissions are added.

Description of how you validated changes

Unit tests and integration tests are added.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

aws-cdk-automation · 2025-06-25T13:01:12Z

➡️ PR build request submitted to test-main-pipeline ⬅️

A maintainer must now check the pipeline and add the pr-linter/cli-integ-tested label once the pipeline succeeds.

rix0rrr · 2025-06-25T13:21:46Z

Please make sure that your PR title confirms to the conventional commit standard (fix, feat, chore) and that it is written in a style that will reflect correctly in the change log. In particular, if this is a bug fix, please describe the bug that was fixed, not the change you made. (See Contributing Guide, Pull Requests)

packages/aws-cdk-lib/core/lib/fs/options.ts

rix0rrr · 2025-06-25T13:30:23Z

packages/aws-cdk-lib/core/lib/fs/options.ts

+   * @see {@link https://github.com/aws/aws-cdk/issues/13636} for more info on the bug this option fixes.
+   * @default true if the feature flag DOCKER_IGNORE_NESTED_EXCLUDE_FIX is set to true, otherwise false
+   */
+  readonly copyNestedExcludes?: boolean;


Why would you copy "excludes" ?

Does this mean respectedNestedIgnoreFiles ?

"respect" seems to be a better description. I've renamed it to dockerIgnoreRespectNestedExcludes

I think this is a bit verbose. Can you shorten it? For example, do both ignore and exclude not refer to the same thing? So do we need both?

respectNestedDockerIgnores?

"exclude" means something that is excluded from the .dockerignore, so an exclude and ignore are not the same thing, they're the opposite.

** # Ignore !build/*.js # Exclude

It is verbose, but I don't think there's a good way to shorten it.

Wait, are you saying that:

In nested .dockerignore files, we do respect positive (ignore) declarations, but we didn't use to respect negative (dont-ignore) declarations?

So we were ignoring more files than necessary?

In that case, I'm leaning towards this not needing a flag. Because we will only ever include more files into the bundle, right? We will never accidentally include too few files.

Which means the chances if this breaking someone are vanishingly small?

Yes @rix0rrr , we were ignoring more files than necessary.

The behavior of including more files in the docker image is undefined. Chance of breaking change is still small regardless. If you think it's safe to remove the feature flag, I can do so.

Fixes #13636

aws-cdk-automation · 2025-07-03T09:14:03Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 23d512a
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2025-07-03T09:14:26Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

github-actions · 2025-07-03T09:14:43Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

Abogical requested a review from a team as a code owner June 25, 2025 12:57

aws-cdk-automation requested a review from a team June 25, 2025 12:57

github-actions bot added repeat-contributor [Pilot] contributed between 3-5 PRs to the CDK bug This issue is a bug. effort/small Small work item – less than a day of effort p1 labels Jun 25, 2025

mergify bot added the contribution/core This is a PR that came from AWS. label Jun 25, 2025

Abogical temporarily deployed to test-pipeline June 25, 2025 12:58 — with GitHub Actions Inactive

Abogical force-pushed the fix-dockerignore-nested-exclude branch from 9e88188 to 3913643 Compare June 25, 2025 13:10

Abogical had a problem deploying to test-pipeline June 25, 2025 13:11 — with GitHub Actions Failure

Abogical changed the title ~~fix(ecr-assets): process nested excludes in .dockerignore correctly~~ fix(ecr-assets): incorrect processing of nested excludes in .dockerignore Jun 25, 2025

Abogical changed the title ~~fix(ecr-assets): incorrect processing of nested excludes in .dockerignore~~ fix(ecr-assets): incorrect handling of nested excludes in .dockerignore Jun 25, 2025

rix0rrr reviewed Jun 25, 2025

View reviewed changes

Abogical force-pushed the fix-dockerignore-nested-exclude branch from 3913643 to 06574ad Compare June 25, 2025 14:00

Abogical had a problem deploying to test-pipeline June 25, 2025 14:01 — with GitHub Actions Failure

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jun 25, 2025

Abogical force-pushed the fix-dockerignore-nested-exclude branch from 06574ad to 315d2ff Compare June 25, 2025 16:12

Abogical had a problem deploying to test-pipeline June 25, 2025 16:13 — with GitHub Actions Failure

github-actions bot mentioned this pull request Jul 1, 2025

Monthly PR metrics report #34865

Closed

rix0rrr self-assigned this Jul 2, 2025

Abogical force-pushed the fix-dockerignore-nested-exclude branch 5 times, most recently from af3d07b to 4e93672 Compare July 2, 2025 15:56

fix(ecr-assets): process nested excludes in .dockerignore correctly

23d512a

Fixes #13636

Abogical force-pushed the fix-dockerignore-nested-exclude branch from 4e93672 to 23d512a Compare July 3, 2025 08:42

rix0rrr approved these changes Jul 3, 2025

View reviewed changes

mergify bot merged commit 8328c79 into main Jul 3, 2025
19 checks passed

mergify bot deleted the fix-dockerignore-nested-exclude branch July 3, 2025 09:14

github-actions bot locked as resolved and limited conversation to collaborators Jul 3, 2025

fix(ecr-assets): incorrect handling of nested excludes in .dockerignore #34810

fix(ecr-assets): incorrect handling of nested excludes in .dockerignore #34810

Conversation

Abogical commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue #13636

Reason for this change

Description of changes

Describe any new or updated permissions being added

Description of how you validated changes

Checklist

Uh oh!

aws-cdk-automation commented Jun 25, 2025

Uh oh!

rix0rrr commented Jun 25, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Abogical Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aws-cdk-automation commented Jul 3, 2025

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Jul 3, 2025

Uh oh!

Uh oh!

github-actions bot commented Jul 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Abogical commented Jun 25, 2025 •

edited

Loading

Abogical Jun 26, 2025 •

edited

Loading