fix: override certain incorrect primary identifiers

.projenrc.ts

@@ -136,6 +136,7 @@ const serviceSpecSchemaTask = serviceSpecImporters.addTask('gen-schemas', {
     'SamTemplateSchema',
     'CloudWatchConsoleServiceDirectory',
     'GetAttAllowList',
+    'CfnPrimaryIdentifierOverrides',
     'OobRelationshipData',
   ].map((typeName: string) => ({
     exec: [

packages/@aws-cdk/aws-service-spec/build/full-database.ts

@@ -30,7 +30,8 @@ export class FullDatabase extends DatabaseBuilder {
       .importLogSources(path.join(SOURCES, 'LogSources/log-source-resource.json'))
       .importScrutinies()
       .importAugmentations()
-      .importEventBridgeSchema(path.join(SOURCES, 'EventBridgeSchema'));
+      .importEventBridgeSchema(path.join(SOURCES, 'EventBridgeSchema'))
+      .importCfnPrimaryIdentifierOverrides(path.join(SOURCES, 'CloudFormationRefOverrides/ref-overrides.json'));
   }
 
   /**

packages/@aws-cdk/service-spec-importers/schemas/CfnPrimaryIdentifierOverrides.schema.json

@@ -0,0 +1,16 @@
+{
+  "$ref": "#/definitions/CfnPrimaryIdentifierOverrides",
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "definitions": {
+    "CfnPrimaryIdentifierOverrides": {
+      "additionalProperties": {
+        "items": {
+          "type": "string"
+        },
+        "type": "array"
+      },
+      "description": "Maps a resource type to a list of properties that are overrides of the CCAPI schema primary identifiers",
+      "type": "object"
+    }
+  }
+}

packages/@aws-cdk/service-spec-importers/src/db-builder.ts

@@ -3,6 +3,7 @@ import { emptyDatabase, SpecDatabase } from '@aws-cdk/service-spec-types';
 import { assertSuccess, Result } from '@cdklabs/tskb';
 import { importArnTemplates } from './importers/import-arn-templates';
 import { importCannedMetrics } from './importers/import-canned-metrics';
+import { importCfnPrimaryIdentifierOverrides } from './importers/import-cfn-primaryidentifier-overrides';
 import { importCloudFormationDocumentation } from './importers/import-cloudformation-docs';
 import { importCloudFormationRegistryResource } from './importers/import-cloudformation-registry';
 import { importEventBridgeSchema } from './importers/import-eventbridge-schema';
@@ -23,6 +24,7 @@ import {
   loadSamSchema,
   loadSamSpec,
   loadOobRelationships,
+  loadCfnPrimaryIdentifierOverrides,
 } from './loaders';
 import { loadDefaultEventBridgeSchema } from './loaders/load-eventbridge-schema';
 import { JsonLensPatcher } from './patching';
@@ -198,6 +200,20 @@ export class DatabaseBuilder {
     });
   }
 
+  /**
+   * Import patches to primary identifiers
+   */
+  public importCfnPrimaryIdentifierOverrides(specFilePath: string) {
+    return this.addSourceImporter(async (db, report) => {
+      const cfnIdentifiers = this.loadResult(
+        await loadCfnPrimaryIdentifierOverrides(specFilePath, this.options),
+        report,
+      );
+
+      importCfnPrimaryIdentifierOverrides(db, cfnIdentifiers);
+    });
+  }
+
   public importArnTemplates(filePath: string) {
     return this.addSourceImporter(async (db, report) => {
       const arnFormatIndex = JSON.parse(await fs.readFile(filePath, { encoding: 'utf-8' }));

packages/@aws-cdk/service-spec-importers/src/db-diff.ts

@@ -117,9 +117,8 @@ export class DbDiff {
       vendedLogs: diffField(a, b, 'vendedLogs', jsonEq),
       vendedLogsConfig: diffField(a, b, 'vendedLogsConfig', jsonEq),
       tagInformation: diffField(a, b, 'tagInformation', jsonEq),
-      primaryIdentifier: collapseEmptyDiff(
-        diffList(a.primaryIdentifier ?? [], b.primaryIdentifier ?? [], (x, y) => x === y),
-      ),
+      primaryIdentifier: diffField(a, b, 'primaryIdentifier', jsonEq),
+      cfnRefIdentifier: diffField(a, b, 'cfnRefIdentifier', jsonEq),
       attributes: collapseEmptyDiff(diffMap(a.attributes, b.attributes, (x, y) => this.diffAttribute(x, y))),
       properties: collapseEmptyDiff(diffMap(a.properties, b.properties, (x, y) => this.diffProperty(x, y))),
       typeDefinitionDiff: this.diffResourceTypeDefinitions(a, b),

packages/@aws-cdk/service-spec-importers/src/diff-fmt.ts

@@ -115,6 +115,8 @@ export class DiffFormatter {
           'tagInformation',
           'arnTemplate',
           'vendedLogs',
+          'primaryIdentifier',
+          'cfnRefIdentifier',
         ]),
       ).indent(META_INDENT),
       listWithCaption('properties', this.renderProperties(r.properties, db)),
@@ -150,6 +152,9 @@ export class DiffFormatter {
       'scrutinizable',
       'tagInformation',
       'arnTemplate',
+      'vendedLogs',
+      'primaryIdentifier',
+      'cfnRefIdentifier',
     ]);
 
     return new PrintableTree(`resource ${key}`).addBullets([

packages/@aws-cdk/service-spec-importers/src/importers/eventbridge/event-resource-matcher.ts

@@ -67,8 +67,6 @@ function eventDecider({
   const resourceMatches = matchTypeFieldsToResources(resources, typeInfos);
 
   if (resourceMatches.length > 0) {
-    // TODO: remove this
-    console.log(`Resources Matches = ${resourceMatches.length}`);
     return { resource: resourceMatches[0].resource, matches: resourceMatches[0].matches };
   } else if (resourceMatches.length == 0) {
     // TODO: change this to report
@@ -112,7 +110,7 @@ function matchTypeFieldsToResources(resources: Resource[], typeInfos: EventTypeD
     if (matches.length > 0) {
       if (matches.length > 1) {
         //TODO: 17 events affected by this, some of them has resourceId & resourceName, some has in multiple levels the resource
-        console.log('here we are', { resource, matches: JSON.stringify(matches, null, 2) });
+        // console.log('here we are', { resource, matches: JSON.stringify(matches, null, 2) });
       }
       resourceMatches.push({
         resource,

packages/@aws-cdk/service-spec-importers/src/importers/import-cfn-primaryidentifier-overrides.ts

@@ -0,0 +1,29 @@
+import { SpecDatabase } from '@aws-cdk/service-spec-types';
+import { CfnPrimaryIdentifierOverrides } from '../types';
+
+/**
+ * For the given resources, update the primary identifiers of these resources
+ */
+export function importCfnPrimaryIdentifierOverrides(db: SpecDatabase, allowList: CfnPrimaryIdentifierOverrides) {
+  const errors = new Array<string>();
+
+  for (const [resourceType, propNames] of Object.entries(allowList)) {
+    try {
+      const resource = db.lookup('resource', 'cloudFormationType', 'equals', resourceType).only();
+
+      for (const propName of propNames) {
+        if (!resource.attributes[propName] && !resource.properties[propName]) {
+          errors.push(`No such property in CFN Primary Identifiers Override file: ${resourceType}.${propName}`);
+        }
+
+        resource.cfnRefIdentifier = propNames;
+      }
+    } catch (e: any) {
+      errors.push(e.message);
+    }
+  }
+
+  if (errors.length > 0) {
+    throw new Error(errors.join('\n'));
+  }
+}

packages/@aws-cdk/service-spec-importers/src/loaders/index.ts

@@ -8,3 +8,4 @@ export * from './load-sam-spec';
 export * from './load-cloudwatch-console-service-directory';
 export * from './load-getatt-allowlist';
 export * from './load-oob-relationships';
+export * from './load-cfn-primary-identifier-overrides';

packages/@aws-cdk/service-spec-importers/src/loaders/load-cfn-primary-identifier-overrides.ts

@@ -0,0 +1,19 @@
+import { assertSuccess } from '@cdklabs/tskb';
+import { Loader, LoadResult, LoadSourceOptions } from './loader';
+import { CfnPrimaryIdentifierOverrides } from '../types';
+
+export async function loadCfnPrimaryIdentifierOverrides(
+  filePath: string,
+  options: LoadSourceOptions = {},
+): Promise<LoadResult<CfnPrimaryIdentifierOverrides>> {
+  const loader = await Loader.fromSchemaFile<CfnPrimaryIdentifierOverrides>(
+    'CfnPrimaryIdentifierOverrides.schema.json',
+    {
+      mustValidate: options.validate,
+    },
+  );
+
+  const result = await loader.loadFile(filePath);
+  assertSuccess(result);
+  return result;
+}

packages/@aws-cdk/service-spec-importers/src/types/cfn-primaryidentifier-overrides/cfn-primaryidentifier-overrides.ts

@@ -0,0 +1,6 @@
+/**
+ * Maps a resource type to a list of properties that are also attributes
+ */
+export interface GetAttAllowList {
+  [resourceType: string]: string[];
+}

packages/@aws-cdk/service-spec-importers/src/types/getatt-allowlist/getatt-allowlist.ts

@@ -1,6 +1,6 @@
 /**
- * Maps a resource type to a list of properties that are also attributes
+ * Maps a resource type to a list of properties that are overrides of the CCAPI schema primary identifiers
  */
-export interface GetAttAllowList {
+export interface CfnPrimaryIdentifierOverrides {
   [resourceType: string]: string[];
 }

packages/@aws-cdk/service-spec-importers/src/types/index.ts

@@ -8,3 +8,4 @@ export * from './cloudwatch-console-service-directory/CloudWatchConsoleServiceDi
 export * from './getatt-allowlist/getatt-allowlist';
 export * from './oob-relationships/OobRelationships';
 export * from './eventbridge/EventBridgeSchema';
+export * from './cfn-primaryidentifier-overrides/cfn-primaryidentifier-overrides';

packages/@aws-cdk/service-spec-importers/test/import-cfn-identifiers.test.ts

@@ -0,0 +1,31 @@
+import { emptyDatabase, Resource } from '@aws-cdk/service-spec-types';
+import { importCfnPrimaryIdentifierOverrides } from '../src/importers/import-cfn-primaryidentifier-overrides';
+
+let db: ReturnType<typeof emptyDatabase>;
+beforeEach(() => {
+  db = emptyDatabase();
+});
+
+test('exercise the CFN identifier import flow', () => {
+  db.allocate('resource', {
+    cloudFormationType: 'AWS::S3::Bucket',
+    attributes: {},
+    name: 'Type',
+    primaryIdentifier: ['BucketName'],
+    properties: {
+      MyProp: { type: { type: 'string' } },
+    },
+  });
+
+  const overridesDocument = {
+    'AWS::S3::Bucket': ['MyProp'],
+  };
+
+  importCfnPrimaryIdentifierOverrides(db, overridesDocument);
+
+  const res = db.lookup('resource', 'cloudFormationType', 'equals', 'AWS::S3::Bucket').only();
+
+  expect(res).toMatchObject({
+    cfnRefIdentifier: ['MyProp'],
+  } satisfies Partial<Resource>);
+});

packages/@aws-cdk/service-spec-types/src/types/diff.ts

@@ -45,7 +45,8 @@ export interface UpdatedResource {
   readonly vendedLogs?: ScalarDiff<Resource['vendedLogs']>;
   readonly vendedLogsConfig?: ScalarDiff<Resource['vendedLogsConfig']>;
   readonly typeDefinitionDiff?: MapDiff<TypeDefinition, UpdatedTypeDefinition>;
-  readonly primaryIdentifier?: ListDiff<string, void>;
+  readonly primaryIdentifier?: ScalarDiff<string[]>;
+  readonly cfnRefIdentifier?: ScalarDiff<string[]>;
   readonly metrics?: MapDiff<Metric, ChangedMetric>;
   readonly events?: MapDiff<Event, UpdatedEvent>;
 }

packages/@aws-cdk/service-spec-types/src/types/resource.ts

@@ -61,7 +61,28 @@ export interface Resource extends Entity {
    */
   cloudFormationTransform?: string;
   documentation?: string;
+
+  /**
+   * The primary identifier, or identifiers, in Cloud Control API
+   *
+   * Uniquely identifies a resource in an account.
+   *
+   * This is read from the schema, which always pertains to the CCAPI identifier.
+   * Typically the same as the CloudFormation identifier, but not necessarily.
+   */
   primaryIdentifier?: string[];
+
+  /**
+   * The primary identifier, or identifiers, in CloudFormation
+   *
+   * Whatever gets returned when you call `{ Ref }` a resource. Typically the
+   * value that other resources in the same service expect to see as the way to
+   * reference this resource (name or ARN or Id).
+   *
+   * If missing, the `cfnRefIdentifier` is the same as the (CC-API) `primaryIdentifier`.
+   */
+  cfnRefIdentifier?: string[];
+
   readonly properties: ResourceProperties;
   readonly attributes: Record<string, Attribute>;
   readonly validations?: unknown;
@@ -303,11 +324,6 @@ export type ResourceInRegion = Relationship<Region, Resource>;
 
 export type UsesType = Relationship<Resource, TypeDefinition>;
 
-export interface ResourceIdentifier extends Entity {
-  readonly arnTemplate?: string;
-  readonly primaryIdentifier?: string[];
-}
-
 /**
  * Mark a resource as a resource that needs additional scrutiy when added, removed or changed
  *

sources/CloudFormationRefOverrides/README.md

@@ -0,0 +1,13 @@
+# Summary
+
+This directory contains overrides for what the `{ Ref }` function returns in CloudFormation, if that disagrees
+with the primary identifier from the schema.
+
+The schema describes the behavior of CloudControl API, which is the same as the
+behavior of CloudFormation, except when it isn't.
+
+These are all validated by hand because there is no other automated source of truth for this information.
+
+# Motivation
+
+Since CDK is primarily built on top of CloudFormation, we need to know what CloudFormation actually does.