diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js index 24c6f0635..d84b5bdfb 100755 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js @@ -786,17 +786,7 @@ class CloudControlHotswapStack extends cdk.Stack { removalPolicy: cdk.RemovalPolicy.DESTROY, }); cdk.Tags.of(queue).add('DynamoTableArn', table.tableArn); - // TEMPORARILY DISABLED — do not re-enable without the CCAPI tag fix. - // Changing this tag makes `Tags` the Queue's only changed property, so the CCAPI - // hotswap emits `replace /Tags` with just the template-defined tags. Since 2026-09-01 - // that fails against a CloudFormation-created queue with: - // ValidationException: aws: prefixed tag key names are not allowed for external use - // because reconciling to a tag set that omits the queue's reserved - // `aws:cloudformation:*` tags implies removing them, which SQS forbids - // (https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/quotas-queues.html). - // With this tag static, the Queue has no hotswappable change and the Dashboard and - // Rule still exercise the CCAPI path. This drops Queue/`Tags` hotswap coverage. - // cdk.Tags.of(queue).add('DynamicTag', process.env.DYNAMIC_CC_PROPERTY_VALUE ?? 'original'); + cdk.Tags.of(queue).add('DynamicTag', process.env.DYNAMIC_CC_PROPERTY_VALUE ?? 'original'); // CloudWatch Dashboard — hotswapped via CCAPI, references the DynamoDB table name. // (This used to be an AWS::Bedrock::Agent, but Bedrock Agents Classic went into diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/hotswap/cloud-control-resource.ts b/packages/@aws-cdk/toolkit-lib/lib/api/hotswap/cloud-control-resource.ts index 9e375cd24..8cf69a5fa 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/hotswap/cloud-control-resource.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/hotswap/cloud-control-resource.ts @@ -83,6 +83,8 @@ export async function isHotswappableCloudControlChange( patchOps.push({ op: 'remove', path: `/${propName}` }); } else if (diff.isAddition) { patchOps.push({ op: 'add', path: `/${propName}`, value: newValue }); + } else if (propName === 'Tags' && newValue && typeof newValue === 'object') { + patchOps.push(...await buildTagPatchOps(cloudControl, resourceType, identifier, newValue)); } else { patchOps.push({ op: 'replace', path: `/${propName}`, value: newValue }); } @@ -108,6 +110,186 @@ export async function isHotswappableCloudControlChange( return ret; } +/** + * Tag keys beginning with `aws:` are reserved for AWS-managed tags. + * They are read-only: a service rejects any external attempt to + * create, update or delete them. + */ +function isReservedTagKey(key: unknown): boolean { + return typeof key === 'string' && key.toLowerCase().startsWith('aws:'); +} + +/** + * Escape a single JSON Pointer reference token (RFC 6901): `~` becomes `~0` and `/` becomes + * `~1`. Required because AWS tag keys may legally contain `/`, which would otherwise be read + * as a path separator. + */ +function escapeJsonPointerToken(key: string): string { + return key.replace(/~/g, '~0').replace(/\//g, '~1'); +} + +/** + * Build the patch operations for a changed `Tags` property. + * + * Address tags individually so reserved tags are never named by the patch and the + * service sees no change to them. That needs the current state, which is read via Cloud + * Control `GetResource`. + * + * CloudFormation models `Tags` either as a list of `{ Key, Value }` (addressed by index) or as + * a `{ key: value }` map (addressed by key). Both are handled. + */ +async function buildTagPatchOps( + cloudControl: ReturnType, + resourceType: string, + identifier: string, + desiredTags: any, +): Promise> { + const readTags = await currentResourceTags(cloudControl, resourceType, identifier); + const desiredIsList = Array.isArray(desiredTags); + const currentTags = readTags ?? (desiredIsList ? [] : {}); + + if (desiredIsList && Array.isArray(currentTags)) { + return buildListTagPatchOps(currentTags, desiredTags); + } + if (!desiredIsList && !Array.isArray(currentTags) && typeof currentTags === 'object') { + return buildMapTagPatchOps(currentTags as Record, desiredTags); + } + + // The live resource reports `Tags` in a shape the template does not declare. A + // resource type does not change its tag shape, so this is not expected to happen. + throw new ToolkitError( + 'HotswapTagReadFailed', + `could not interpret the current tags of ${identifier} (${resourceType}): the resource reports Tags as ${describeTagsShape(currentTags)} but the template declares ${describeTagsShape(desiredTags)}`, + ); +} + +/** + * Describe the shape of a `Tags` value, for error messages. + */ +function describeTagsShape(tags: unknown): string { + if (Array.isArray(tags)) { + return 'a list'; + } + if (tags === null) { + return 'null'; + } + if (typeof tags === 'object') { + return 'a map'; + } + return typeof tags; // "string", "number", "undefined", etc. +} +/** + * `Tags` as a `{ key: value }` map: address each tag by its key. Object members have no + * position, so unlike the list form there is no ordering constraint between operations. + */ +function buildMapTagPatchOps( + currentTags: Record, + desiredTags: Record, +): Array<{ op: string; path: string; value?: any }> { + const ops: Array<{ op: string; path: string; value?: any }> = []; + + for (const [key, value] of Object.entries(desiredTags)) { + const path = `/Tags/${escapeJsonPointerToken(key)}`; + if (!Object.hasOwn(currentTags, key)) { + ops.push({ op: 'add', path, value }); + } else if (JSON.stringify(currentTags[key]) !== JSON.stringify(value)) { + ops.push({ op: 'replace', path, value }); + } + } + + // Drop tags the template no longer defines — but never the reserved ones. + for (const key of Object.keys(currentTags)) { + if (!Object.hasOwn(desiredTags, key) && !isReservedTagKey(key)) { + ops.push({ op: 'remove', path: `/Tags/${escapeJsonPointerToken(key)}` }); + } + } + + return ops; +} + +/** + * `Tags` as a list of `{ Key, Value }`: address each tag by its index in the resource's + * current list. + */ +function buildListTagPatchOps( + currentTags: any[], + desiredTags: any[], +): Array<{ op: string; path: string; value?: any }> { + const isTag = (tag: any): boolean => tag && typeof tag === 'object' && typeof tag.Key === 'string'; + + const indexByKey = new Map(); + currentTags.forEach((tag, i) => { + if (isTag(tag) && !indexByKey.has(tag.Key)) { + indexByKey.set(tag.Key, i); + } + }); + + // Update tags that already exist (addressed by index); append the ones that don't. + const replacements: Array<{ op: string; path: string; value?: any }> = []; + const additions: Array<{ op: string; path: string; value?: any }> = []; + for (const tag of desiredTags) { + if (!isTag(tag)) { + continue; + } + const index = indexByKey.get(tag.Key); + if (index === undefined) { + additions.push({ op: 'add', path: '/Tags/-', value: tag }); + } else if (JSON.stringify(currentTags[index]) !== JSON.stringify(tag)) { + replacements.push({ op: 'replace', path: `/Tags/${index}`, value: tag }); + } + } + + // Drop tags the template no longer defines — but never the reserved ones. + const desiredKeys = new Set(desiredTags.filter(isTag).map((tag) => tag.Key)); + const removals = currentTags + .map((tag, i) => ({ tag, i })) + .filter(({ tag }) => isTag(tag) && !desiredKeys.has(tag.Key) && !isReservedTagKey(tag.Key)) + // Descending, so removing one does not shift the indices of the others. + .sort((a, b) => b.i - a.i) + .map(({ i }) => ({ op: 'remove', path: `/Tags/${i}` })); + + // Replacements first (their indices refer to the unmodified list), then removals, then + // appends, which only ever touch the end of the list. + return [...replacements, ...removals, ...additions]; +} + +/** + * Read the current `Tags` value of a resource via Cloud Control `GetResource`, which returns + * the resource model as a JSON string in `ResourceDescription.Properties`. + * + * Returns `undefined` when the resource simply has no tags. Throws when the current tags + * cannot be determined at all + */ +async function currentResourceTags( + cloudControl: ReturnType, + resourceType: string, + identifier: string, +): Promise { + const unreadable = (cause: unknown) => ToolkitError.withCause( + 'HotswapTagReadFailed', + `could not read the current tags of ${identifier} (${resourceType}), which are needed to update tags without removing the reserved aws: tags that AWS manages - ensure the deployment role is allowed to call cloudcontrolapi:GetResource for this resource type`, + cause, + ); + + let current; + try { + current = await cloudControl.getResource({ TypeName: resourceType, Identifier: identifier }); + } catch (e) { + throw unreadable(e); + } + + const properties = current.ResourceDescription?.Properties; + if (!properties) { + throw unreadable(new Error('GetResource returned no resource properties')); + } + + try { + return JSON.parse(properties).Tags; + } catch (e) { + throw unreadable(e); + } +} + /** * Resolves the Cloud Control API identifier for a resource. * diff --git a/packages/@aws-cdk/toolkit-lib/test/api/hotswap/cloud-control-hotswap-deployments.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/hotswap/cloud-control-hotswap-deployments.test.ts index 5b998c530..f59a903ad 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/hotswap/cloud-control-hotswap-deployments.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/hotswap/cloud-control-hotswap-deployments.test.ts @@ -1,4 +1,4 @@ -import { UpdateResourceCommand } from '@aws-sdk/client-cloudcontrol'; +import { UpdateResourceCommand, GetResourceCommand } from '@aws-sdk/client-cloudcontrol'; import { DescribeTypeCommand } from '@aws-sdk/client-cloudformation'; import { HotswapMode } from '../../../lib/api/hotswap'; import { mockCloudControlClient, mockCloudFormationClient } from '../../_helpers/mock-sdk'; @@ -740,3 +740,281 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('Property remov }); }); }); + +describe('Tags hotswap does not disturb reserved aws:-prefixed tags', () => { + // Mirrors the live tag layout observed on a CloudFormation-created SQS queue: the reserved + // aws:cloudformation:* tags are interleaved with the template's own tags, and the live + // ordering is NOT the template ordering. + const liveTags = [ + { Key: 'aws:cloudformation:logical-id', Value: 'Queue' }, + { Key: 'DynamoTableArn', Value: 'arn:aws:dynamodb:us-east-1:1111:table/T' }, + { Key: 'aws:cloudformation:stack-id', Value: 'arn:aws:cloudformation:us-east-1:1111:stack/s/1' }, + { Key: 'aws:cloudformation:stack-name', Value: 'my-stack' }, + { Key: 'DynamicTag', Value: 'original value' }, + ]; + + function givenQueue(currentTags: any, templateTags: any, newTemplateTags: any) { + mockCloudControlClient.on(GetResourceCommand).resolves({ + TypeName: 'AWS::SQS::Queue', + ResourceDescription: { + Identifier: 'q-123', + Properties: JSON.stringify({ Id: 'q-123', Tags: currentTags }), + }, + }); + setup.setCurrentCfnStackTemplate({ + Resources: { + Queue: { Type: 'AWS::SQS::Queue', Properties: { Id: 'q-123', Tags: templateTags } }, + }, + }); + setup.pushStackResourceSummaries(setup.stackSummaryOf('Queue', 'AWS::SQS::Queue', 'q-123')); + return setup.cdkStackArtifactOf({ + template: { + Resources: { + Queue: { Type: 'AWS::SQS::Queue', Properties: { Id: 'q-123', Tags: newTemplateTags } }, + }, + }, + }); + } + + const patchOf = () => { + const call = mockCloudControlClient.commandCalls(UpdateResourceCommand)[0]; + return JSON.parse((call.args[0].input as any).PatchDocument); + }; + + beforeEach(() => { + hotswapMockSdkProvider = setup.setupHotswapTests(); + mockCloudFormationClient.on(DescribeTypeCommand).resolves({ + Schema: JSON.stringify({ primaryIdentifier: ['/properties/Id'] }), + }); + mockCloudControlClient.on(UpdateResourceCommand).resolves({}); + }); + + test('addresses the changed tag by its index in the live list, naming no reserved tag', async () => { + // GIVEN - only DynamicTag changes; it sits at index 4 on the live resource + const artifact = givenQueue( + liveTags, + [{ Key: 'DynamicTag', Value: 'original value' }, { Key: 'DynamoTableArn', Value: 'arn:aws:dynamodb:us-east-1:1111:table/T' }], + [{ Key: 'DynamicTag', Value: 'new value' }, { Key: 'DynamoTableArn', Value: 'arn:aws:dynamodb:us-east-1:1111:table/T' }], + ); + + // WHEN + await hotswapMockSdkProvider.tryHotswapDeployment(HotswapMode.HOTSWAP_ONLY, artifact); + + // THEN - a single index-addressed replace, matching the patch shape proven to be accepted + expect(patchOf()).toEqual([ + { op: 'replace', path: '/Tags/4', value: { Key: 'DynamicTag', Value: 'new value' } }, + ]); + // and crucially: no wholesale /Tags replace, and no aws: key anywhere in the patch + const patchText = JSON.stringify(patchOf()); + expect(patchText).not.toContain('"path":"/Tags"'); + expect(patchText.toLowerCase()).not.toContain('aws:'); + }); + + test('appends a tag the resource does not have yet', async () => { + // GIVEN + const unchanged = { Key: 'DynamoTableArn', Value: 'arn:aws:dynamodb:us-east-1:1111:table/T' }; + const artifact = givenQueue( + liveTags, + [{ Key: 'DynamicTag', Value: 'original value' }, unchanged], + [{ Key: 'DynamicTag', Value: 'original value' }, unchanged, { Key: 'BrandNew', Value: 'x' }], + ); + + // WHEN + await hotswapMockSdkProvider.tryHotswapDeployment(HotswapMode.HOTSWAP_ONLY, artifact); + + // THEN + expect(patchOf()).toEqual([{ op: 'add', path: '/Tags/-', value: { Key: 'BrandNew', Value: 'x' } }]); + }); + + test('removes a tag dropped from the template but never a reserved one', async () => { + // GIVEN - the template no longer defines DynamoTableArn (live index 1) + const artifact = givenQueue( + liveTags, + [{ Key: 'DynamicTag', Value: 'original value' }, { Key: 'DynamoTableArn', Value: 'arn:aws:dynamodb:us-east-1:1111:table/T' }], + [{ Key: 'DynamicTag', Value: 'original value' }], + ); + + // WHEN + await hotswapMockSdkProvider.tryHotswapDeployment(HotswapMode.HOTSWAP_ONLY, artifact); + + // THEN - only the user tag is removed; the three reserved tags are untouched + expect(patchOf()).toEqual([{ op: 'remove', path: '/Tags/1' }]); + expect(JSON.stringify(patchOf()).toLowerCase()).not.toContain('aws:'); + }); + + test('fails loudly when the live tags cannot be read, instead of sending a wholesale replace', async () => { + // GIVEN - the deploy role may not read the resource, so GetResource fails + const artifact = givenQueue( + liveTags, + [{ Key: 'DynamicTag', Value: 'original value' }], + [{ Key: 'DynamicTag', Value: 'new value' }], + ); + mockCloudControlClient.on(GetResourceCommand).rejects(new Error('AccessDenied')); + + // WHEN / THEN - the hotswap fails with an actionable error... + await expect( + hotswapMockSdkProvider.tryHotswapDeployment(HotswapMode.HOTSWAP_ONLY, artifact), + ).rejects.toThrow(/could not read the current tags of q-123 \(AWS::SQS::Queue\)/); + + // ...and we never send the request that would have removed the reserved tags + expect(mockCloudControlClient).not.toHaveReceivedCommand(UpdateResourceCommand); + }); + + test('treats a resource with no tags as all additions', async () => { + // GIVEN - the live resource carries no Tags at all + const artifact = givenQueue( + undefined, + [{ Key: 'DynamicTag', Value: 'original value' }], + [{ Key: 'DynamicTag', Value: 'new value' }], + ); + + // WHEN + await hotswapMockSdkProvider.tryHotswapDeployment(HotswapMode.HOTSWAP_ONLY, artifact); + + // THEN + expect(patchOf()).toEqual([ + { op: 'add', path: '/Tags/-', value: { Key: 'DynamicTag', Value: 'new value' } }, + ]); + }); +}); + +describe('Tags modelled as a { key: value } map', () => { + // Some resource types model Tags as a map rather than a list of { Key, Value }. + const liveTags = { + 'aws:cloudformation:stack-name': 'my-stack', + 'aws:cloudformation:logical-id': 'Api', + 'DynamicTag': 'original value', + }; + + function givenApi(currentTags: any, templateTags: any, newTemplateTags: any) { + mockCloudControlClient.on(GetResourceCommand).resolves({ + TypeName: 'AWS::ApiGatewayV2::Api', + ResourceDescription: { + Identifier: 'api-123', + Properties: JSON.stringify({ Id: 'api-123', Tags: currentTags }), + }, + }); + setup.setCurrentCfnStackTemplate({ + Resources: { + Api: { Type: 'AWS::ApiGatewayV2::Api', Properties: { Id: 'api-123', Tags: templateTags } }, + }, + }); + setup.pushStackResourceSummaries(setup.stackSummaryOf('Api', 'AWS::ApiGatewayV2::Api', 'api-123')); + return setup.cdkStackArtifactOf({ + template: { + Resources: { + Api: { Type: 'AWS::ApiGatewayV2::Api', Properties: { Id: 'api-123', Tags: newTemplateTags } }, + }, + }, + }); + } + + const patchOf = () => { + const call = mockCloudControlClient.commandCalls(UpdateResourceCommand)[0]; + return JSON.parse((call.args[0].input as any).PatchDocument); + }; + + beforeEach(() => { + hotswapMockSdkProvider = setup.setupHotswapTests(); + mockCloudFormationClient.on(DescribeTypeCommand).resolves({ + Schema: JSON.stringify({ primaryIdentifier: ['/properties/Id'] }), + }); + mockCloudControlClient.on(UpdateResourceCommand).resolves({}); + }); + + test('addresses the changed tag by key, naming no reserved tag', async () => { + // GIVEN + const artifact = givenApi( + liveTags, + { DynamicTag: 'original value' }, + { DynamicTag: 'new value' }, + ); + + // WHEN + await hotswapMockSdkProvider.tryHotswapDeployment(HotswapMode.HOTSWAP_ONLY, artifact); + + // THEN + expect(patchOf()).toEqual([ + { op: 'replace', path: '/Tags/DynamicTag', value: 'new value' }, + ]); + const patchText = JSON.stringify(patchOf()); + expect(patchText).not.toContain('"path":"/Tags"'); + expect(patchText.toLowerCase()).not.toContain('aws:'); + }); + + test('escapes / and ~ in tag keys per RFC 6901', async () => { + // GIVEN - AWS tag keys may legally contain '/' + const artifact = givenApi( + { 'cost/center': 'old', 'a~b': 'old' }, + { 'cost/center': 'old', 'a~b': 'old' }, + { 'cost/center': 'new', 'a~b': 'new' }, + ); + + // WHEN + await hotswapMockSdkProvider.tryHotswapDeployment(HotswapMode.HOTSWAP_ONLY, artifact); + + // THEN - '/' -> '~1' and '~' -> '~0', so the key is not read as a path separator + expect(patchOf()).toEqual([ + { op: 'replace', path: '/Tags/cost~1center', value: 'new' }, + { op: 'replace', path: '/Tags/a~0b', value: 'new' }, + ]); + }); + + test('removes a tag dropped from the template but never a reserved one', async () => { + // GIVEN + const artifact = givenApi( + { ...liveTags, Obsolete: 'x' }, + { DynamicTag: 'original value', Obsolete: 'x' }, + { DynamicTag: 'original value' }, + ); + + // WHEN + await hotswapMockSdkProvider.tryHotswapDeployment(HotswapMode.HOTSWAP_ONLY, artifact); + + // THEN + expect(patchOf()).toEqual([{ op: 'remove', path: '/Tags/Obsolete' }]); + expect(JSON.stringify(patchOf()).toLowerCase()).not.toContain('aws:'); + }); +}); + +describe('Tags in an unexpected shape', () => { + beforeEach(() => { + hotswapMockSdkProvider = setup.setupHotswapTests(); + mockCloudFormationClient.on(DescribeTypeCommand).resolves({ + Schema: JSON.stringify({ primaryIdentifier: ['/properties/Id'] }), + }); + mockCloudControlClient.on(UpdateResourceCommand).resolves({}); + }); + + test('fails rather than sending a wholesale replace when the live Tags shape is not the template shape', async () => { + // GIVEN - the template declares a list, but the resource reports a scalar + mockCloudControlClient.on(GetResourceCommand).resolves({ + TypeName: 'AWS::SQS::Queue', + ResourceDescription: { + Identifier: 'q-123', + Properties: JSON.stringify({ Id: 'q-123', Tags: 'not-a-tag-collection' }), + }, + }); + setup.setCurrentCfnStackTemplate({ + Resources: { + Queue: { Type: 'AWS::SQS::Queue', Properties: { Id: 'q-123', Tags: [{ Key: 'DynamicTag', Value: 'a' }] } }, + }, + }); + setup.pushStackResourceSummaries(setup.stackSummaryOf('Queue', 'AWS::SQS::Queue', 'q-123')); + const artifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Queue: { Type: 'AWS::SQS::Queue', Properties: { Id: 'q-123', Tags: [{ Key: 'DynamicTag', Value: 'b' }] } }, + }, + }, + }); + + // WHEN / THEN + await expect( + hotswapMockSdkProvider.tryHotswapDeployment(HotswapMode.HOTSWAP_ONLY, artifact), + ).rejects.toThrow(/could not interpret the current tags of q-123 \(AWS::SQS::Queue\): the resource reports Tags as string but the template declares a list/); + + // and we never send the request that would have removed the reserved tags + expect(mockCloudControlClient).not.toHaveReceivedCommand(UpdateResourceCommand); + }); +});