Skip to content

Commit

Permalink
fix(cx-api): cannot detect CloudAssembly across different libraries (#…
Browse files Browse the repository at this point in the history
…32998)

### Reason for this change

We are publishing the `cx-api` package twice: Once as a standalone package `@aws-cdk/cx-api` and once as part of the construct library under `aws-cdk-lib/cx-api`. The code is copied during the release and the same versions of the packages will have the same code. 

However this makes it difficult for other packages to take a type dependency on types from this package. The most common class that's used from `cx-api` is `CloudAssembly` - the result of `app.synth()`. Previously a package had to take a dependency on the very large `aws-cdk-lib` just to use a single type. It would be better if other packages could instead depend on the smaller, much more focused `@aws-cdk/cx-api` package.

### Description of changes

This adds the same mechanism to `CloudAssembly` to detect cross-library compatibility, that we already use for constructs like `Stack` or `App`. In TypeScript, it's now possible for a consuming package to receive an object from either package and check at runtime if it satisfies the requirements.

We cannot get around type checking with this. Instead we introduce a new type `ICloudAssembly` into the Cloud Assembly Schema package (cdklabs/cloud-assembly-schema#133). This interface only declares a single property: `directory`. Consumers can use this type to indicate where they would like to receive a `CloudAssembly`. They can then use runtime code to either confirm a provided object already satisfies the requirements or fallback to creating a new `CloudAssembly` from the directory.

Because the `CloudAssembly` in `cxapi` implements the new interface, this approach will work in all jsii languages. In TypeScript it's even compatible with older version of `aws-cdk-lib`. Jsii language will only support this going forward.

#### Allowed breaking changes

```
weakened:aws-cdk-lib.cloud_assembly_schema.MetadataEntry
weakened:aws-cdk-lib.cx_api.MetadataEntryResult
```

This PR updates the version of `@aws-cdk/cloud-assembly-schema` to make new of the new interface.
However the update also includes a change to `MetadataEntry` which was introduced in cdklabs/cloud-assembly-schema#121. That change is weakening a type, because in #31041, the CDK started emitting booleans and numbers as metadata values.

But since these types weren't officially declared in the schema, jsii runtime type checking failed to load them.
The fix was to officially extend the type union to include `boolean` and `number` primitive values.
This is considered breaking, because when used as an output any consuming code will now need to account for the possibility of the value being a `boolean` or `number`. In static languages, the type would already have been treated as a generic Object with required runtime checks.

### Describe any new or updated permissions being added

n/a

### Description of how you validated changes

Unit tests.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
mrgrain authored Jan 21, 2025
1 parent 6d834c0 commit 94ba772
Show file tree
Hide file tree
Showing 17 changed files with 314 additions and 10,762 deletions.
10 changes: 9 additions & 1 deletion allowed-breaking-changes.txt
Original file line number Diff line number Diff line change
Expand Up @@ -942,4 +942,12 @@ change-return-type:aws-cdk-lib.aws_lambda.FilterRule.null
# output property was mistakenly marked as required even though it should have allowed
# for undefined, i.e optional
changed-type:@aws-cdk/cx-api.CloudFormationStackArtifact.notificationArns
changed-type:aws-cdk-lib.cx_api.CloudFormationStackArtifact.notificationArns
changed-type:aws-cdk-lib.cx_api.CloudFormationStackArtifact.notificationArns

# In aws/aws-cdk#31041, the CDK started emitting booleans and numbers as metadata values.
# Since these types weren't officially declared in the schema, jsii runtime type checking failed to load them.
# The type has now weakened, and when it's used as an output any consuming code will need to account for the possibility of the value being a boolean or number.
# In static languages, the type would already have been treated as a generic Object with required runtime checks.
# See: https://github.com/cdklabs/cloud-assembly-schema/pull/121
weakened:aws-cdk-lib.cloud_assembly_schema.MetadataEntry
weakened:aws-cdk-lib.cx_api.MetadataEntryResult
10,990 changes: 252 additions & 10,738 deletions packages/@aws-cdk/cli-lib-alpha/THIRD_PARTY_LICENSES

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions packages/@aws-cdk/cx-api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
"gen": "cdk-copy cx-api",
"watch": "cdk-watch",
"lint": "cdk-lint && madge --circular --extensions js lib",
"test": "cdk-test",
"test": "jest",
"pkglint": "pkglint -f",
"package": "cdk-package",
"awslint": "cdk-awslint",
Expand All @@ -82,12 +82,12 @@
"semver": "^7.6.3"
},
"peerDependencies": {
"@aws-cdk/cloud-assembly-schema": "^39.0.0"
"@aws-cdk/cloud-assembly-schema": "^39.2.0"
},
"license": "Apache-2.0",
"devDependencies": {
"@aws-cdk/cdk-build-tools": "0.0.0",
"@aws-cdk/cloud-assembly-schema": "^39.0.1",
"@aws-cdk/cloud-assembly-schema": "^39.2.0",
"@aws-cdk/pkglint": "0.0.0",
"@types/jest": "^29.5.14",
"@types/mock-fs": "^4.13.4",
Expand Down Expand Up @@ -120,4 +120,4 @@
"publishConfig": {
"tag": "latest"
}
}
}
4 changes: 2 additions & 2 deletions packages/@aws-cdk/integ-runner/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
},
"dependencies": {
"chokidar": "^3.6.0",
"@aws-cdk/cloud-assembly-schema": "^39.0.0",
"@aws-cdk/cloud-assembly-schema": "^39.2.0",
"@aws-cdk/cloudformation-diff": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"@aws-cdk/aws-service-spec": "^0.1.49",
Expand Down Expand Up @@ -109,4 +109,4 @@
"publishConfig": {
"tag": "latest"
}
}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import * as cxapi from '@aws-cdk/cx-api';
import * as fs from 'fs-extra';
import type { ICloudAssemblySource } from '../';
import { ContextAwareCloudAssembly, ContextAwareCloudAssemblyProps } from './context-aware-source';
Expand All @@ -10,7 +11,6 @@ import { debug } from '../../io/private';
import { AssemblyBuilder, CdkAppSourceProps } from '../source-builder';

export abstract class CloudAssemblySourceBuilder {

/**
* Helper to provide the CloudAssemblySourceBuilder with required toolkit services
* @deprecated this should move to the toolkit really.
Expand Down Expand Up @@ -40,13 +40,19 @@ export abstract class CloudAssemblySourceBuilder {
produce: async () => {
const outdir = determineOutputDirectory(props.outdir);
const env = await prepareDefaultEnvironment(services, { outdir });
return changeDir(async () =>
const assembly = await changeDir(async () =>
withContext(context.all, env, props.synthOptions ?? {}, async (envWithContext, ctx) =>
withEnv(envWithContext, () => builder({
outdir,
context: ctx,
})),
), props.workingDirectory);

if (cxapi.CloudAssembly.isCloudAssembly(assembly)) {
return assembly;
}

return new cxapi.CloudAssembly(assembly.directory);
},
},
contextAssemblyProps,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type * as cxapi from '@aws-cdk/cx-api';
import type * as cxschema from '@aws-cdk/cloud-assembly-schema';

export interface AppProps {
/**
Expand All @@ -12,7 +12,7 @@ export interface AppProps {
readonly context?: { [key: string]: any };
}

export type AssemblyBuilder = (props: AppProps) => Promise<cxapi.CloudAssembly>;
export type AssemblyBuilder = (props: AppProps) => Promise<cxschema.ICloudAssembly>;

/**
* Configuration for creating a CLI from an AWS CDK App directory
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/toolkit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"typescript": "~5.6.3"
},
"dependencies": {
"@aws-cdk/cloud-assembly-schema": "^39.0.1",
"@aws-cdk/cloud-assembly-schema": "^39.2.0",
"@aws-cdk/cloudformation-diff": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"@aws-cdk/region-info": "0.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ export default async () => {
new s3.Bucket(stack, 'MyBucket', {
bucketName: app.node.tryGetContext('externally-provided-bucket-name'),
});
return app.synth() as any;
return app.synth();
};

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ export default async () => {
const app = new core.App();
const stack = new core.Stack(app, 'Stack1');
new s3.Bucket(stack, 'MyBucket');
return app.synth() as any;
return app.synth();
};

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,5 @@ export default async () => {
new core.Stack(app, 'Stack1');
new core.Stack(app, 'Stack2');

// @todo fix api
return app.synth() as any;
return app.synth();
};
19 changes: 18 additions & 1 deletion packages/aws-cdk-lib/cx-api/lib/cloud-assembly.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
// This is deliberately importing the interface from the external package.
// We want this, so that jsii language packages can depend on @aws-cdk/cloud-assembly-schema
// instead of being forced to take a dependency on the much larger aws-cdk-lib.
import type { ICloudAssembly } from '@aws-cdk/cloud-assembly-schema';
import { CloudFormationStackArtifact } from './artifacts/cloudformation-artifact';
import { NestedCloudAssemblyArtifact } from './artifacts/nested-cloud-assembly-artifact';
import { TreeCloudArtifact } from './artifacts/tree-cloud-artifact';
import { CloudArtifact } from './cloud-artifact';
import { topologicalSort } from './toposort';
import * as cxschema from '../../cloud-assembly-schema';

const CLOUD_ASSEMBLY_SYMBOL = Symbol.for('@aws-cdk/cx-api.CloudAssembly');

/**
* The name of the root manifest file of the assembly.
*/
Expand All @@ -16,7 +22,16 @@ const MANIFEST_FILE = 'manifest.json';
/**
* Represents a deployable cloud application.
*/
export class CloudAssembly {
export class CloudAssembly implements ICloudAssembly {
/**
* Return whether the given object is a CloudAssembly.
*
* We do attribute detection since we can't reliably use 'instanceof'.
*/
public static isCloudAssembly(x: any): x is CloudAssembly {
return x !== null && typeof(x) === 'object' && CLOUD_ASSEMBLY_SYMBOL in x;
}

/**
* The root directory of the cloud assembly.
*/
Expand Down Expand Up @@ -54,6 +69,8 @@ export class CloudAssembly {
this.artifacts = this.renderArtifacts(loadOptions?.topoSort ?? true);
this.runtime = this.manifest.runtime || { libraries: { } };

Object.defineProperty(this, CLOUD_ASSEMBLY_SYMBOL, { value: true });

// force validation of deps by accessing 'depends' on all artifacts
this.validateDeps();
}
Expand Down
9 changes: 9 additions & 0 deletions packages/aws-cdk-lib/cx-api/test/cloud-assembly.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,12 @@ test('getStackArtifact retrieves a stack by artifact id from a nested assembly',
expect(assembly.getStackArtifact('stack1').id).toEqual('stack1');
expect(assembly.getStackArtifact('stack2').id).toEqual('stack2');
});

test('isCloudAssembly correctly detects Cloud Assemblies', () => {
const assembly = new CloudAssembly(path.join(FIXTURES, 'nested-assemblies'));
const inheritedAssembly = new (class extends CloudAssembly {})(path.join(FIXTURES, 'nested-assemblies'));

expect(CloudAssembly.isCloudAssembly(assembly)).toBe(true);
expect(CloudAssembly.isCloudAssembly(inheritedAssembly)).toBe(true);
expect(CloudAssembly.isCloudAssembly({})).toBe(false);
});
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@
"@aws-cdk/asset-awscli-v1": "^2.2.208",
"@aws-cdk/asset-kubectl-v20": "^2.1.3",
"@aws-cdk/asset-node-proxy-agent-v6": "^2.1.0",
"@aws-cdk/cloud-assembly-schema": "^39.0.1",
"@aws-cdk/cloud-assembly-schema": "^39.2.0",
"@balena/dockerignore": "^1.0.2",
"case": "1.6.3",
"fs-extra": "^11.2.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@
"xml-js": "^1.6.11"
},
"dependencies": {
"@aws-cdk/cloud-assembly-schema": "^39.0.0",
"@aws-cdk/cloud-assembly-schema": "^39.2.0",
"@aws-cdk/cloudformation-diff": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"@aws-cdk/region-info": "0.0.0",
Expand Down
4 changes: 2 additions & 2 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,15 @@
"@aws-cdk/service-spec-types" "^0.0.115"
"@cdklabs/tskb" "^0.0.3"

"@aws-cdk/cloud-assembly-schema@^39.0.0", "@aws-cdk/cloud-assembly-schema@^39.0.1", "@aws-cdk/cloud-assembly-schema@^39.1.34":
"@aws-cdk/cloud-assembly-schema@^39.1.34":
version "39.1.34"
resolved "https://registry.npmjs.org/@aws-cdk/cloud-assembly-schema/-/cloud-assembly-schema-39.1.34.tgz#06bfc07cd892bf431f97c8ff784c8b001480c134"
integrity sha512-/uDvlrOD67PAaMxEX/gbWA8tce9SdVkGBtxEXV/R+DN7z6T4BztX3aZhkjMP/mzWu0UbkgOkwbOzqD4terCGwg==
dependencies:
jsonschema "^1.4.1"
semver "^7.6.3"

"@aws-cdk/cloud-assembly-schema@^39.1.49":
"@aws-cdk/cloud-assembly-schema@^39.1.49", "@aws-cdk/cloud-assembly-schema@^39.2.0":
version "39.2.0"
resolved "https://registry.npmjs.org/@aws-cdk/cloud-assembly-schema/-/cloud-assembly-schema-39.2.0.tgz#2abf4afb91c873a0206b5b6467fe53e505f7a9b7"
integrity sha512-ymGG+ab4xN40iPx9O0zuuvu6qZi4RY+hr3YScSg5Ye0dkcchQ49RBINHrqqy7fZvcMbV7bkxf/Cxj9yxSF3BnA==
Expand Down

0 comments on commit 94ba772

Please sign in to comment.