fix: override certain incorrect primary identifiers #2328
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There are a number of primary identifiers in the Registry Schema that
don't agree with what the `{ Ref }` intrinsic of CloudFormation is
returning.
This is not unexpected or wrong: the Registry Schema describes the
behavior of Cloud Control API, and the identifier uniquely identifies a
resource in an account.
However, certain resources in CloudFormation already had a certain
behavior, and perhaps the `{ Ref }` returned a value that wasn't unique
enough; like the name of a policy inside another resource: unique
*enough* in the context of a CloudFormation template, but not unique
enough for CCAPI.
Since the behavior of CloudFormation cannot be changed, this leads to
the inescapable conclusion that the primary identifiers of CCAPI and
CloudFormation can be different.
CDK needs to account for this difference, so we do, introducing two
kinds of primary identifiers: a CCAPI-level one and a CFN-level one.
We will do most of our codegen work off of the CFN identifier, but
we'll have the other one in our back pocket.
This list is not subject to maintenance (modulo things that Kiro and
myself have missed for whatever reason, I didn't do all of this work by
myself because that would have been crazy) because going forward for new
resources the schema must agree with the behavior of `{ Ref }`.
Contributor
|
@aws-cdk/aws-service-spec: Model database diff detected |
Contributor
|
@aws-cdk/aws-service-spec: Model database diff detected |
leonmk-aws
reviewed
Dec 23, 2025
Contributor
There was a problem hiding this comment.
Should not be committed right ?
leonmk-aws
reviewed
Dec 23, 2025
|
|
||
| export type UsesType = Relationship<Resource, TypeDefinition>; | ||
|
|
||
| export interface ResourceIdentifier extends Entity { |
Contributor
There was a problem hiding this comment.
We are removing this because we were not using it in the first place right ?
leonmk-aws
reviewed
Dec 23, 2025
| * value that other resources in the same service expect to see as the way to | ||
| * reference this resource (name or ARN or Id). | ||
| * | ||
| * Generally the same as the `ccApiPrimaryIdentifier`, but can be different |
Contributor
There was a problem hiding this comment.
Generally the same as primaryIdentifier or is ccApiPrimaryIdentifier something else ?
Contributor
Author
There was a problem hiding this comment.
Ah yes, I had this the other way around first.
Contributor
|
@aws-cdk/aws-service-spec: Model database diff detected |
Contributor
|
nit: even if we don't plan on updating the source, might be good to have it in the diff ? |
Contributor
|
@aws-cdk/aws-service-spec: Model database diff detected |
Contributor
|
Noticed there was no unit test, can you add a small unit test like the arn templates one and I am good to approve |
Contributor
Author
|
Added a test |
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
leonmk-aws
approved these changes
Dec 23, 2025
Contributor
|
@aws-cdk/aws-service-spec: Model database diff detected |
rix0rrr
added a commit
to aws/aws-cdk
that referenced
this pull request
Dec 24, 2025
This is the change in CDK that consumes the spec update cdklabs/awscdk-service-spec#2328. This accounts for `{ Ref }` not always returning what the schema says that the primary identifier of a resource is. This is not unexpected or wrong: the Registry Schema describes the behavior of Cloud Control API, and the identifier uniquely identifies a resource in an account. We do need to account for the differences though. What we will do is: - Retain the CC-API fields in the reference interface if we hvae the values. - Fall back to the (smaller) CFN identifier fields if we don't have the values. - Make the CFN identifier fields win if the identifiers are equal size (in which case we probably a Name vs ARN distinction).
mergify bot
pushed a commit
to aws/aws-cdk
that referenced
this pull request
Dec 29, 2025
…FN (#36493) This is the change in CDK that consumes the spec update cdklabs/awscdk-service-spec#2328. This accounts for `{ Ref }` not always returning what the schema says that the primary identifier of a resource is. This is not unexpected or wrong: the Registry Schema describes the behavior of Cloud Control API, and the identifier uniquely identifies a resource in an account. We do need to account for the differences though. What we will do is: - Retain the CC-API fields in the reference interface if we hvae the values. - Fall back to the (smaller) CFN identifier fields if we don't have the values. - Make the CFN identifier fields win if the identifiers are equal size (in which case we probably a Name vs ARN distinction). This brings in a new version of service-spec so it also adds a new service. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are a number of primary identifiers in the Registry Schema that don't agree with what the
{ Ref }intrinsic of CloudFormation is returning.This is not unexpected or wrong: the Registry Schema describes the behavior of Cloud Control API, and the identifier uniquely identifies a resource in an account.
However, certain resources in CloudFormation already had a certain behavior, and perhaps the
{ Ref }returned a value that wasn't unique enough; like the name of a policy inside another resource: unique enough in the context of a CloudFormation template, but not unique enough for CCAPI.Since the behavior of CloudFormation cannot be changed, this leads to the inescapable conclusion that the primary identifiers of CCAPI and CloudFormation can be different.
CDK needs to account for this difference, so we do, introducing two kinds of primary identifiers: a CCAPI-level one and a CFN-level one.
We will do most of our codegen work off of the CFN identifier, but we'll have the other one in our back pocket.
This list is not subject to maintenance (modulo things that Kiro and myself have missed for whatever reason, I didn't do all of this work by myself because that would have been crazy) because going forward for new resources the schema must agree with the behavior of
{ Ref }.