fix(core): allow override with cross-stack references

[tmokmss]

closes #18882

Below is what I found as the root cause and how it can be fixed. I confirmed all the unit tests are passing.

---

Previously when we used a cross-stack reference in override, it was not resolved as an Fn::ImportValue.

To make it an Fn::ImportValue, we need to get every token in an app and find references (tokens that references resources outside of its stack) from them. The related code is here:

aws-cdk/packages/@aws-cdk/core/lib/private/refs.ts

Lines 139 to 140 in 810d736

	function findAllReferences(root: IConstruct) {
	const result = new Array<{ source: CfnElement, value: CfnReference }>();

To get all the tokens in an app, we use RememberingTokenResolver, which remembers every token it has found during resolution. So basically this resolver must be used on every resolution to find all the tokens.

aws-cdk/packages/@aws-cdk/core/lib/private/resolve.ts

Lines 270 to 276 in 810d736

	export class RememberingTokenResolver extends DefaultTokenResolver {
	private readonly tokensSeen = new Set<IResolvable>();
	public resolveToken(t: IResolvable, context: IResolveContext, postProcessor: IPostProcessor) {
	this.tokensSeen.add(t);
	return super.resolveToken(t, context, postProcessor);
	}

However, the resolver is not used specifically when we resolve tokens in raw overrides. Actually the current interface of postProcess function of PostResolveToken class makes It difficult to use an external resolver.

aws-cdk/packages/@aws-cdk/core/lib/cfn-resource.ts

Lines 374 to 380 in 810d736

	const resolvedRawOverrides = Tokenization.resolve(this.rawOverrides, {
	scope: this,
	resolver: CLOUDFORMATION_TOKEN_RESOLVER,
	// we need to preserve the empty elements here,
	// as that's how removing overrides are represented as
	removeEmpty: false,
	});

That is why, in this PR, we move the resolution process outside of the postProcess, allowing to resolve tokens in raw overrides with the RememberingTokenResolver resolver.

This change also simplifies the current implementation of deepMerge.

Related PRs: #15018 #20608 #22294

Note: Now the removeEmpty flag introduced by #15018 is not used by any internal code and can be removed. However, we cannot remove it immediately since it is a public API. Maybe we can just deprecate and remove it in a future version.

---

All Submissions:

• Have you followed the guidelines in our Contributing guide?

Adding new Construct Runtime Dependencies:

• This PR adds new construct runtime dependencies following the process described here

New Features

• Have you added the new feature to an integration test?
  • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

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

[gitpod-io]

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

[tmokmss]

I don't think we need to change or add an integration test for this fix. It's ready for review :)

[aws-cdk-automation]

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 11a8ac0
• Result: SUCCEEDED
• Build Logs (available for 30 days)

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

[aws-cdk-automation]

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

[tmokmss]

Hi @corymhall, could you take a look at this when you get a chance? I saw you recently worked on some related PRs. Or at least It'd be great if you could reopen and add exempt-integ-test label to this :)

[aws-cdk-automation]

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

[tmokmss]

Hi @comcalvi, thank you for the attention! Could you check it again? I think it needs an exempt-integ-test label.

[aws-cdk-automation]

The pull request linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.

PRs must pass status checks before we can provide a meaningful review.

If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing Exemption Request and/or Clarification Request.