fix: Prevent duplicated cdk-defined resource ids

[rafael747]

Limiting aws:cdk:path metadata partitions length in order to prevent duplicated resource ids. Logical id will be used if it is not possible to ensure that the resource id extracted from aws:cdk:path is unique.

Which issue(s) does this change fix?

#4064
#4034
aws/aws-cdk#21134

Why is this change necessary?

SAM CLI extracts the resource id from aws:cdk:path metadata value when it is present.
This value is always unique when generated by CDK. However, only the second last partition is used, which might not be unique cross all resources in the stack.

• Stack/aaa/Fn/Resource -> SamResouceId: Fn
• Stack/bbb/Fn/Resource -> SamResouceId: Fn
• Stack/ccc/Fn/Resource -> SamResouceId: Fn

How does it address the issue?

By limiting the usage of aws:cdk:path metadata when the number of partitions is bigger than 3.
In these situations, it is not possible to ensure that the second last partition will be unique, and the logical id should be used instead.

What side effects does this change have?

Mandatory Checklist

PRs will only be reviewed after checklist is complete

• Add input/output type hints to new functions/methods
• Write design document if needed (Do I need to write a design document?)
• Write/update unit tests
• Write/update integration tests
• Write/update functional tests if needed
• make pr passes
• make update-reproducible-reqs if dependencies were changed
• Write documentation

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

[XaaXaaX]

Hi everyone ,
just try to see if this PR has any news?

i recongnize no reviewer niether active comments ;)

[XaaXaaX]

For anyone waiting before the PR being merged
in a meantime i resolved the problem using an aspect as below

import { Annotations, IAspect } from "aws-cdk-lib";
import { CfnFunction } from "aws-cdk-lib/aws-lambda";
import { PATH_METADATA_KEY } from "aws-cdk-lib/cx-api";
import { IConstruct } from "constructs";

export class ApplyLocalFunctionAspect implements IAspect {
  public visit(node: IConstruct): void {
    if (process.env["LOCAL"] !== "true") return;
    if (node instanceof CfnFunction) {
      this.updateCdkMetadata(node);
      this.UpdateFunctionProperties(node);
    }
  }

  protected error(node: IConstruct, message: string): void {
    Annotations.of(node).addError(message);
  }

  private updateCdkMetadata = (node: CfnFunction) => {
    const metadata = node.cfnOptions.metadata;
    const currentPath = metadata?.[PATH_METADATA_KEY];

    if (currentPath) {
      const partitions = currentPath.split('/');

      // Get first and last part of currentpath
      const firstPart = partitions[0];
      const lastPart = partitions[partitions.length - 1];

      // Ge a join of the rest
      const rest = partitions.slice(1, partitions.length - 1).join('');
      const newPath = `${firstPart}/${rest}/${lastPart}`;

      node.addMetadata(PATH_METADATA_KEY, newPath);
      Annotations.of(node).addWarningV2(currentPath, `Modified aws:cdk:path to ${newPath}`);
    }
  };

  private UpdateFunctionProperties = (node: CfnFunction) => {
    node.addPropertyOverride("Layers", []);
    Annotations.of(node).addWarningV2("Layers", "Remove Lambda Layers for local");
  };
}

and simply pass it to the stack you need

  const myAppStack = new MyApplicationStack(....);
  Aspects.of(myAppStack).add(new ApplyLocalFunctionAspect());

[rafael747]

Another workaround is to disable path metadata during synth: cdk synth --path-metadata false

[XaaXaaX]

Another workaround is to disable path metadata during synth: cdk synth --path-metadata false

I was not aware of that, but what about the layers? Sam will fail if layers exists in template using refs, but works for direct layer Arns, but sur this one can be handy and simplifies things ,
Will give it a try 💜