-
Notifications
You must be signed in to change notification settings - Fork 4.4k
fix(core): account for { Ref } incompatibility between schema and CFN
#36493
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c0ca0f7
e1b4803
15fa656
b25ed7c
ddfb1a0
79e140e
a071b05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export * as mixins from './mixins'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export * from './cfn-props-mixins.generated'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export * from './events.generated'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,2 @@ | ||
| export * as events from './events'; | ||
| export * as mixins from './mixins'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,2 @@ | ||
| export * from './cfn-props-mixins.generated'; | ||
| export * from './logs-delivery-mixins.generated'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| { | ||
| "targets": { | ||
| "java": { | ||
| "package": "software.amazon.awscdk.services.cases" | ||
| }, | ||
| "dotnet": { | ||
| "namespace": "Amazon.CDK.AWS.Cases" | ||
| }, | ||
| "python": { | ||
| "module": "aws_cdk.aws_cases" | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| # AWS::Cases Construct Library | ||
| <!--BEGIN STABILITY BANNER--> | ||
|
|
||
| --- | ||
|
|
||
|  | ||
|
|
||
| > All classes with the `Cfn` prefix in this module ([CFN Resources]) are always stable and safe to use. | ||
| > | ||
| > [CFN Resources]: https://docs.aws.amazon.com/cdk/latest/guide/constructs.html#constructs_lib | ||
|
|
||
| --- | ||
|
|
||
| <!--END STABILITY BANNER--> | ||
|
|
||
| This module is part of the [AWS Cloud Development Kit](https://github.com/aws/aws-cdk) project. | ||
|
|
||
| ```ts nofixture | ||
| import * as cases from 'aws-cdk-lib/aws-cases'; | ||
| ``` | ||
|
|
||
| <!--BEGIN CFNONLY DISCLAIMER--> | ||
|
|
||
| There are no official hand-written ([L2](https://docs.aws.amazon.com/cdk/latest/guide/constructs.html#constructs_lib)) constructs for this service yet. Here are some suggestions on how to proceed: | ||
|
|
||
| - Search [Construct Hub for Cases construct libraries](https://constructs.dev/search?q=cases) | ||
| - Use the automatically generated [L1](https://docs.aws.amazon.com/cdk/latest/guide/constructs.html#constructs_l1_using) constructs, in the same way you would use [the CloudFormation AWS::Cases resources](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/AWS_Cases.html) directly. | ||
|
|
||
|
|
||
| <!--BEGIN CFNONLY DISCLAIMER--> | ||
|
|
||
| There are no hand-written ([L2](https://docs.aws.amazon.com/cdk/latest/guide/constructs.html#constructs_lib)) constructs for this service yet. | ||
| However, you can still use the automatically generated [L1](https://docs.aws.amazon.com/cdk/latest/guide/constructs.html#constructs_l1_using) constructs, and use this service exactly as you would using CloudFormation directly. | ||
|
|
||
| For more information on the resources and properties available for this service, see the [CloudFormation documentation for AWS::Cases](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/AWS_Cases.html). | ||
|
|
||
| (Read the [CDK Contributing Guide](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and submit an RFC if you are interested in contributing to this construct library.) | ||
|
|
||
| <!--END CFNONLY DISCLAIMER--> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export * from './lib'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| // AWS::Cases Cloudformation Resources | ||
| export * from './cases.generated'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import { Resource } from '@aws-cdk/service-spec-types'; | ||
| import { $this, expr, Expression, PropertySpec, Type } from '@cdklabs/typewriter'; | ||
| import { attributePropertyName, referencePropertyName } from '../naming'; | ||
| import { attributePropertyName, propertyNameFromCloudFormation, referencePropertyName } from '../naming'; | ||
| import { extractResourceVariablesFromArnFormat, findArnProperty, findNonIdentifierArnProperty } from './arn'; | ||
| import { CDK_CORE } from './cdk'; | ||
|
|
||
|
|
@@ -9,6 +9,37 @@ export interface ReferenceProp { | |
| readonly cfnValue: Expression; | ||
| } | ||
|
|
||
| /** | ||
| * Calculations for to the resource reference interface. | ||
| * | ||
| * This class is slightly complicated because it needs to account for the differences between CloudFormation | ||
| * and CC-API identifiers. | ||
| * | ||
| * - In principle, the CC-API identifier is leading: it uniquely identifies a resource inside an environment. | ||
| * - For backwards compatibility reasons, the CFN identifier (the value returned by `{ Ref }`) isn't necessarily | ||
| * always the same. If it isn't the same, there can be two reasons for them to diverge: | ||
| * | ||
| * - SPECIFICITY: the CC-API identifier is more specific than the CFN | ||
| * identifier. For example, for `ApiGateway::Stage`, the unique identifier is | ||
| * `[ApiId, StageName]` but the value that `{ Ref }` returns is just | ||
| * `StageName`. | ||
| * | ||
| * This distinction happens for subresources, and in those cases the | ||
| * primary resource will be a required input property. For maximum | ||
| * flexibility we generate the interface according to the CC-API | ||
| * identifier, and get values from the CFN identifier, attributes and input | ||
| * properties as necessary. | ||
| * | ||
| * - ALIASING: the CC-API uses a different form of identifying the resource than the | ||
| * CFN identifier. For example, for `Batch::JobDefinition` the spec says the primary | ||
| * identifier is the `Name` but the actual value that `{ Ref }` returns is the | ||
| * the `Arn`. We will just use the CFN value as leading. | ||
| * | ||
| * We will identify the difference between these 2 cases by the length of the primary | ||
| * identifier: equal length = aliasing, different length = specificity. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. by length you mean length of the identifier array and not length of their string value, correct? i was slightly confused here at how we are identifying the difference between the two cases. this also means that we are certain that there is no case where the CC-API identifier is simply different than the CFN identifier (not aliased). we are certain this is the case? also, are we using 'unique identifier' and 'primary identifier' interchangably?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes.
Well, "aliased" is probably a wrong name here but I couldn't think of a better one. I'm thinking of
I guess so. I'm trying to put the distinction on CCAPI identifier vs CFN identifier, the other words are not different on purpose. |
||
| * | ||
| * If available, we also add an ARN field into the reference interface. | ||
| */ | ||
| export class ResourceReference { | ||
| public readonly resource: Resource; | ||
| public readonly arnPropertyName?: string; | ||
|
|
@@ -44,36 +75,20 @@ export class ResourceReference { | |
| } | ||
|
|
||
| private collectReferencesProps() { | ||
| // Primary identifier. We assume all parts are strings. | ||
| const primaryIdentifier = this.resource.primaryIdentifier ?? []; | ||
| if (primaryIdentifier.length === 1) { | ||
| const name = referencePropertyName(primaryIdentifier[0], this.resource.name); | ||
| // Reference fields | ||
| for (const cfnName of this.referenceFields) { | ||
| const name = referencePropertyName(cfnName, this.resource.name); | ||
| this._referenceProps.setIfAbsent(name, { | ||
| declaration: { | ||
| name, | ||
| type: Type.STRING, | ||
| immutable: true, | ||
| docs: { | ||
| summary: `The ${primaryIdentifier[0]} of the ${this.resource.name} resource.`, | ||
| summary: `The ${cfnName} of the ${this.resource.name} resource.`, | ||
| }, | ||
| }, | ||
| cfnValue: $this.ref, | ||
| cfnValue: this.getStringValue(cfnName), | ||
| }); | ||
| } else if (primaryIdentifier.length > 1) { | ||
| for (const [i, cfnName] of primaryIdentifier.entries()) { | ||
| const name = referencePropertyName(cfnName, this.resource.name); | ||
| this._referenceProps.setIfAbsent(name, { | ||
| declaration: { | ||
| name, | ||
| type: Type.STRING, | ||
| immutable: true, | ||
| docs: { | ||
| summary: `The ${cfnName} of the ${this.resource.name} resource.`, | ||
| }, | ||
| }, | ||
| cfnValue: splitSelect('|', i, $this.ref), | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| // Arn identifier | ||
|
|
@@ -137,6 +152,71 @@ export class ResourceReference { | |
| // we don't have a matching value | ||
| return undefined; | ||
| } | ||
|
|
||
| /** | ||
| * The actual reference fields | ||
| * | ||
| * The CFN values if present and the same length as the CC-API values, otherwise the CC-API values. | ||
| * | ||
| * For a CC-API identifier we filter out optional properties, such as for `ECS::Cluster`: the real | ||
| * unique identifier includes `Cluster` but that is an optional property because the Service will fall | ||
| * back to some implicit default Cluster that we can never replicate. | ||
| */ | ||
| public get referenceFields(): string[] { | ||
| if (this.resource.cfnRefIdentifier && this.resource.cfnRefIdentifier.length === this.resource.primaryIdentifier?.length) { | ||
| return this.resource.cfnRefIdentifier; | ||
| } | ||
|
|
||
| // Filter out properties we can't find a value for (will only be optional properties) | ||
| return (this.resource.primaryIdentifier ?? []) | ||
| .filter(p => this.tryGetStringValue(p) !== undefined); | ||
| } | ||
|
|
||
| /** | ||
| * What `{ Ref }` returns in CloudFormation | ||
| */ | ||
| public get cfnRefComponents(): string[] { | ||
| return this.resource.cfnRefIdentifier ?? this.resource.primaryIdentifier ?? []; | ||
| } | ||
|
|
||
| /** | ||
| * Return an expression to return the given value from the { Ref } or any of the attributes or properties | ||
| */ | ||
| private tryGetStringValue(name: string): Expression | undefined { | ||
| for (const [i, field] of this.cfnRefComponents.entries()) { | ||
| if (field === name) { | ||
| // Return entire field or Split expression, depending on whether we need to split at all | ||
| return this.cfnRefComponents.length > 1 ? splitSelect('|', i, $this.ref) : $this.ref; | ||
| } | ||
| } | ||
|
|
||
| // Is it an attr? | ||
| if (this.resource.attributes[name]) { | ||
| return $this[attributePropertyName(name)]; | ||
| } | ||
|
|
||
| // A required prop? | ||
| if (this.resource.properties[name]?.required) { | ||
| return $this[propertyNameFromCloudFormation(name)]; | ||
| } | ||
|
|
||
| return undefined; | ||
| } | ||
|
|
||
| /** | ||
| * Return a value, failing if it doesn't exist. | ||
| */ | ||
| private getStringValue(name: string): Expression { | ||
| const ret = this.tryGetStringValue(name); | ||
| if (ret) { | ||
| return ret; | ||
| } | ||
|
|
||
| const attributeNames = Object.keys(this.resource.attributes); | ||
| const requiredPropertyNames = Object.entries(this.resource.properties).filter(([_, p]) => p.required).map(([n, _]) => n); | ||
|
|
||
| throw new Error(`Cannot find reference interface value name ${name} for resource ${this.resource.cloudFormationType} (Ref components: ${this.cfnRefComponents}, attributes: ${attributeNames}, requiredProps: ${requiredPropertyNames})`); | ||
| } | ||
| } | ||
|
|
||
| class FirstOccurrenceMap<K, V> extends Map<K, V> { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i know you didn't add in the
!, but if we're confidentprimaryIdentifieralways exists, then why is the property typed as optional in the first place?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're confident that it exists here, not that it exists in general?
The property is probably optional because we added it later.