feat(gen2-migration): detecting service type to distinguish between cdk and cfn custom resources#14410
Closed
sanjanaravikumar-az wants to merge 9 commits intogen2-migrationfrom
Closed
feat(gen2-migration): detecting service type to distinguish between cdk and cfn custom resources#14410sanjanaravikumar-az wants to merge 9 commits intogen2-migrationfrom
sanjanaravikumar-az wants to merge 9 commits intogen2-migrationfrom
Conversation
added 8 commits
December 10, 2025 10:22
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| type CustomResourceServiceType = 'customCDK' | 'customCloudformation'; | ||
|
|
||
| const getCustomResourcesWithType = (): Map<string, CustomResourceServiceType> => { |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the problem, remove the unused function getCustomResourcesWithType. This eliminates the unused binding and any dead logic inside it, without affecting existing functionality, because nothing currently depends on it.
Concretely:
- In
packages/amplify-cli/src/commands/gen2-migration/codegen-generate/src/codegen-head/command-handlers.ts, delete the entiregetCustomResourcesWithTypedeclaration (lines 327–338 in the snippet), keeping the surroundingtype CustomResourceServiceTypeandgetCustomResourcesdefinitions intact. - No imports, exports, or other definitions need to be added or changed, since the function is local and not referenced.
Suggested changeset
1
packages/amplify-cli/src/commands/gen2-migration/codegen-generate/src/codegen-head/command-handlers.ts
| @@ -324,19 +324,6 @@ | ||
|
|
||
| type CustomResourceServiceType = 'customCDK' | 'customCloudformation'; | ||
|
|
||
| const getCustomResourcesWithType = (): Map<string, CustomResourceServiceType> => { | ||
| const meta = stateManager.getMeta(); | ||
| const customCategory = meta?.custom; | ||
| const resourceMap = new Map<string, CustomResourceServiceType>(); | ||
|
|
||
| if (customCategory) { | ||
| Object.entries(customCategory).forEach(([name, config]: [string, { service: CustomResourceServiceType }]) => { | ||
| resourceMap.set(name, config.service); | ||
| }); | ||
| } | ||
| return resourceMap; | ||
| }; | ||
|
|
||
| const getCustomResources = (): string[] => { | ||
| const meta = stateManager.getMeta(); | ||
| const customCategory = meta?.custom; |
Copilot is powered by AI and may make mistakes. Always verify output.
Contributor
There was a problem hiding this comment.
Can you address this as either true positive or false positive?
iankhou
requested changes
Dec 24, 2025
Contributor
iankhou
left a comment
iankhou
left a comment
There was a problem hiding this comment.
Looks good, but difficult for me to verify functionality by looking at it. How was it tested?
We are making a lot of changes to transformations here. I think it would be good to have unit tests that cover functionality introduced by the new parameter resourceDependencies.
|
|
||
| type CustomResourceServiceType = 'customCDK' | 'customCloudformation'; | ||
|
|
||
| const getCustomResourcesWithType = (): Map<string, CustomResourceServiceType> => { |
Contributor
There was a problem hiding this comment.
Can you address this as either true positive or false positive?
Comment on lines
+514
to
+537
| for (const resource of customResources) { | ||
| const cdkStackFilePath = path.join(destinationCustomResourcePath, resource, 'cdk-stack.ts'); | ||
|
|
||
| try { | ||
| const cdkStackContent = await fs.readFile(cdkStackFilePath, { encoding: 'utf-8' }); | ||
| const dependencies: string[] = []; | ||
|
|
||
| // Parse for AmplifyHelpers.addResourceDependency calls | ||
| const dependencyRegex = /AmplifyHelpers\.addResourceDependency\s*\([^,]+,[^,]+,[^,]+,\s*\[([^\]]+)\]/g; | ||
| let match; | ||
|
|
||
| while ((match = dependencyRegex.exec(cdkStackContent)) !== null) { | ||
| const dependenciesArray = match[1]; | ||
|
|
||
| // Extract category values from dependency objects | ||
| const categoryRegex = /category:\s*['"]([^'"]+)['"]/g; | ||
| let categoryMatch; | ||
|
|
||
| while ((categoryMatch = categoryRegex.exec(dependenciesArray)) !== null) { | ||
| const category = categoryMatch[1]; | ||
| if (!dependencies.includes(category)) { | ||
| dependencies.push(category); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
For this kind of logic I would generally like to see some kind of test, since regex is not verifiable by reading it (at least not by me). A unit test may be sufficient for now.
| `throw new Error('Follow https://docs.amplify.aws/react/start/migrate-to-gen2/ to update the resource dependency');\n\nexport class`, | ||
| ); | ||
| } | ||
| // AmplifyHelpers.addResourceDependency is now supported by the transformer |
iliapolo
changed the title
2 tasks
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
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.
Overview
This PR adds service type detection for custom resources during Gen1 to Gen2 migration. It introduces the ability to distinguish between CDK-based (customCDK) and CloudFormation-based (customCloudformation) custom resources.
Key Changes
CustomResourceServiceTypetype to distinguish between customCDK and customCloudformation resourcesgetCustomResourcesWithType()function that reads from amplify-meta.json and returns a map of resource names to their service typesGraphQLAPIKeyOutputmapping back tocfnResources.cfnGraphqlApi.attrApiKeyBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.