Merge branch 'main' into sumughan/aspects-applied-undefined-fix

packages/@aws-cdk-testing/cli-integ/bin/query-github

@@ -1,2 +1,2 @@
 #!/usr/bin/env node
-require('./query-github.js');
+require('../lib/cli/query-github.js');

packages/@aws-cdk-testing/cli-integ/bin/run-suite

@@ -1,2 +1,2 @@
 #!/usr/bin/env node
-require('./run-suite.js');
+require('../lib/cli/run-suite.js');

packages/@aws-cdk-testing/cli-integ/bin/stage-distribution

@@ -1,2 +1,2 @@
 #!/usr/bin/env node
-require('./stage-distribution.js');
+require('../lib/cli/stage-distribution.js');

packages/@aws-cdk-testing/cli-integ/bin/test-root

@@ -1,2 +1,2 @@
 #!/usr/bin/env node
-require('./test-root.js');
+require('../lib/cli/test-root.js');

packages/@aws-cdk-testing/cli-integ/bin/query-github.ts → packages/@aws-cdk-testing/cli-integ/lib/cli/query-github.ts

@@ -1,5 +1,5 @@
 import * as yargs from 'yargs';
-import { fetchPreviousVersion } from '../lib/github';
+import { fetchPreviousVersion } from '../github';
 
 async function main() {
   const args = await yargs

packages/@aws-cdk-testing/cli-integ/bin/run-suite.ts → packages/@aws-cdk-testing/cli-integ/lib/cli/run-suite.ts

@@ -2,10 +2,10 @@
 import * as path from 'path';
 import * as jest from 'jest';
 import * as yargs from 'yargs';
-import { ReleasePackageSourceSetup } from '../lib/package-sources/release-source';
-import { RepoPackageSourceSetup, autoFindRoot } from '../lib/package-sources/repo-source';
-import { IPackageSourceSetup } from '../lib/package-sources/source';
-import { serializeForSubprocess } from '../lib/package-sources/subprocess';
+import { ReleasePackageSourceSetup } from '../package-sources/release-source';
+import { RepoPackageSourceSetup, autoFindRoot } from '../package-sources/repo-source';
+import { IPackageSourceSetup } from '../package-sources/source';
+import { serializeForSubprocess } from '../package-sources/subprocess';
 
 async function main() {
   const args = await yargs
@@ -126,7 +126,7 @@ async function main() {
       ...args.verbose ? ['--verbose'] : [],
       ...passWithNoTests ? ['--passWithNoTests'] : [],
       ...args['test-file'] ? [args['test-file']] : [],
-    ], path.resolve(__dirname, '..', 'resources', 'integ.jest.config.js'));
+    ], path.resolve(__dirname, '..', '..', 'resources', 'integ.jest.config.js'));
 
   } finally {
     await packageSource.cleanup();

packages/@aws-cdk-testing/cli-integ/bin/stage-distribution.ts → packages/@aws-cdk-testing/cli-integ/lib/cli/stage-distribution.ts

@@ -3,13 +3,13 @@ import * as path from 'path';
 import * as fs from 'fs-extra';
 import * as glob from 'glob';
 import * as yargs from 'yargs';
-import { shell } from '../lib';
-import { TestRepository } from '../lib/staging/codeartifact';
-import { uploadJavaPackages, mavenLogin } from '../lib/staging/maven';
-import { uploadNpmPackages, npmLogin } from '../lib/staging/npm';
-import { uploadDotnetPackages, nugetLogin } from '../lib/staging/nuget';
-import { uploadPythonPackages, pypiLogin } from '../lib/staging/pypi';
-import { UsageDir } from '../lib/staging/usage-dir';
+import { shell } from '..';
+import { TestRepository } from '../staging/codeartifact';
+import { uploadJavaPackages, mavenLogin } from '../staging/maven';
+import { uploadNpmPackages, npmLogin } from '../staging/npm';
+import { uploadDotnetPackages, nugetLogin } from '../staging/nuget';
+import { uploadPythonPackages, pypiLogin } from '../staging/pypi';
+import { UsageDir } from '../staging/usage-dir';
 
 async function main() {
   await yargs

packages/@aws-cdk-testing/cli-integ/bin/test-root.ts → packages/@aws-cdk-testing/cli-integ/lib/cli/test-root.ts

@@ -1,3 +1,3 @@
 import * as path from 'path';
 // eslint-disable-next-line no-console
-console.log(path.resolve(__dirname, '..'));
+console.log(path.resolve(__dirname, '..', '..'));

packages/aws-cdk-lib/core/test/aspect.prop.test.ts

@@ -32,7 +32,7 @@ describe('every aspect gets invoked exactly once', () => {
           for (const aspectApplication of originalAspectApplications) {
             // Check that each original AspectApplication also visited all child nodes of its original scope:
             for (const construct of originalConstructsOnApp) {
-              if (ancestorOf(aspectApplication.construct, construct)) {
+              if (isAncestorOf(aspectApplication.construct, construct)) {
                 if (!visitsMap.get(construct)!.includes(aspectApplication.aspect)) {
                   throw new Error(`Aspect ${aspectApplication.aspect.constructor.name} applied on ${aspectApplication.construct.node.path} did not visit construct ${construct.node.path} in its original scope.`);
                 }
@@ -56,7 +56,7 @@ describe('every aspect gets invoked exactly once', () => {
           for (const aspectApplication of allAspectApplications) {
             // Check that each AspectApplication also visited all child nodes of its scope:
             for (const construct of allConstructsOnApp) {
-              if (ancestorOf(aspectApplication.construct, construct)) {
+              if (isAncestorOf(aspectApplication.construct, construct)) {
                 if (!visitsMap.get(construct)!.includes(aspectApplication.aspect)) {
                   throw new Error(`Aspect ${aspectApplication.aspect.constructor.name} applied on ${aspectApplication.construct.node.path} did not visit construct ${construct.node.path} in its scope.`);
                 }
@@ -107,7 +107,11 @@ test('for every construct, lower priorities go before higher priorities', () =>
           const aPrio = lowestAspectPrioFor(a.aspect, a.construct);
           const bPrio = lowestAspectPrioFor(b.aspect, b.construct);
 
-          if (!implies(aPrio < bPrio, a.index < b.index)) {
+          // But only if the application of aspect A exists at least as long as the application of aspect B
+          const aAppliedT = aspectAppliedT(testApp, a.aspect, a.construct);
+          const bAppliedT = aspectAppliedT(testApp, b.aspect, b.construct);
+
+          if (!implies(aPrio < bPrio && aAppliedT <= bAppliedT, a.index < b.index)) {
             throw new Error(
               `Aspect ${a.aspect}@${aPrio} at ${a.index} should have been before ${b.aspect}@${bPrio} at ${b.index}, but was after`,
             );
@@ -250,7 +254,7 @@ function sameAspect(a: AspectVisit, b: AspectVisit) {
 /**
  * Returns whether `a` is an ancestor of `b` (or if they are the same construct)
  */
-function ancestorOf(a: Construct, b: Construct) {
+function isAncestorOf(a: Construct, b: Construct) {
   return b.node.path === a.node.path || b.node.path.startsWith(a.node.path + '/');
 }
 
@@ -282,6 +286,19 @@ function allAspectApplicationsInScope(a: Construct): AspectApplication[] {
   return ancestors(a).flatMap((c) => Aspects.of(c).applied);
 }
 
+/**
+ * Find the lowest timestamp that could lead to the execution of the given aspect on the given construct
+ *
+ * Take the minimum of all added applications that could lead to this execution.
+ */
+function aspectAppliedT(prettyApp: PrettyApp, a: IAspect, c: Construct): number {
+  const potentialTimes = [...prettyApp.initialAspects, ...prettyApp.addedAspects].filter((appl) => {
+    return appl.aspect === a && isAncestorOf(appl.construct, c);
+  }).map((appl) => appl.t);
+
+  return Math.min(...potentialTimes);
+}
+
 /**
  * Return the lowest priority of Aspect `a` inside the given list of applications
  */
@@ -350,12 +367,20 @@ function arbConstructTreeFactory(): fc.Arbitrary<ConstructFactory> {
  */
 class PrettyApp {
   private readonly initialTree: Set<string>;
-  private readonly initialAspects: Map<string, Set<string>>;
+  private readonly _initialAspects: Map<string, Set<string>>;
+  public readonly initialAspects: RecordedAspectApplication[];
 
   constructor(public readonly cdkApp: App, public readonly executionState: ExecutionState) {
     const constructs = cdkApp.node.findAll();
     this.initialTree = new Set(constructs.map(c => c.node.path));
-    this.initialAspects = new Map(constructs.map(c => [c.node.path, new Set(renderAspects(c))]));
+    this._initialAspects = new Map(constructs.map(c => [c.node.path, new Set(renderAspects(c))]));
+
+    this.initialAspects = constructs.flatMap(c => Aspects.of(c).applied.map(a => ({
+      aspect: a.aspect,
+      construct: a.construct,
+      priority: a.priority,
+      t: 0,
+    } as RecordedAspectApplication)));
   }
 
   /**
@@ -408,7 +433,7 @@ class PrettyApp {
       const aspects = renderAspects(construct);
 
       for (let i = 0; i < aspects.length; i++) {
-        if (!(self.initialAspects.get(construct.node.path) ?? new Set()).has(aspects[i])) {
+        if (!(self._initialAspects.get(construct.node.path) ?? new Set()).has(aspects[i])) {
           aspects[i] += ' (added)';
         }
       }
@@ -693,6 +718,7 @@ interface TestAspectApplication extends PartialTestAspectApplication {
  * An aspect application added by another aspect, during execution
  */
 interface RecordedAspectApplication extends PartialTestAspectApplication {
+  t: number;
   construct: Construct;
 }
 
@@ -715,7 +741,9 @@ class NodeAddingAspect extends TracingAspect {
     const newConstruct = new ArbConstruct(scope, this.loc.id);
     for (const { aspect, priority } of this.newAspects) {
       Aspects.of(newConstruct).add(aspect, { priority });
-      this.executionState(node).addedAspects.push({
+      const executionState = this.executionState(node);
+      executionState.addedAspects.push({
+        t: executionState.visitLog.length,
         construct: newConstruct,
         aspect,
         priority,
@@ -744,7 +772,9 @@ class AspectAddingAspect extends TracingAspect {
     const constructs = this.newAspect.constructPaths.map((p) => findConstructDeep(node.node.root, p));
     for (const construct of constructs) {
       Aspects.of(construct).add(this.newAspect.aspect, { priority: this.newAspect.priority });
-      this.executionState(node).addedAspects.push({
+      const executionState = this.executionState(node);
+      executionState.addedAspects.push({
+        t: executionState.visitLog.length,
         construct,
         aspect: this.newAspect.aspect,
         priority: this.newAspect.priority,

tools/@aws-cdk/spec2cdk/lib/cdk/tagging.ts

@@ -32,6 +32,18 @@ export function resourceTaggabilityStyle(resource: Resource): TaggabilityStyle |
   return undefined;
 }
 
+/**
+ * Mapping of legacy taggable resources to their tag property name and variant
+ *
+ * Before the introduction of iTaggablev2 the CDK defined a `tags` property on constructs
+ * which contained the tagManager, however in cases where the resource itself contained a
+ * property named 'tags', the CDK would create a `tagsRaw` property to represent that CFN tags property.
+ *
+ * Upon the introduction of iTaggablev2, the CDK now uses the `cdkTagManager` property to manage tags.
+ * This mapping of legacy resources is used to preserve the legacy behavior of applying tags so customers
+ * who previously were tagging these constructs using `myConstruct.tags.setTag('key', 'value')` will
+ * continue to be able to do so, without breaking changes.
+ */
 const LEGACY_TAGGABLES: Record<string, [string, TagInformation['variant']]> = {
   'AWS::ACMPCA::CertificateAuthority': ['Tags', 'standard'],
   'AWS::AccessAnalyzer::Analyzer': ['Tags', 'standard'],