-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat(spec2cdk): support for auto-generated grants in alpha modules #36206
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
base: main
Are you sure you want to change the base?
Conversation
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.
(This review is outdated)
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
| import writeCloudFormationIncludeMapping from './submodules/cloudformation-include'; | ||
|
|
||
| const awsCdkLibDir = path.join(__dirname, '..'); | ||
| const atAwsCdkDir = path.join(__dirname, '../../@aws-cdk'); |
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.
Is this tool, which lives in aws-cdk-lib, going to search outside its own directory for things to generate?
I don't like that very much.
I think we can't get around making the generator a standalone tool, perhaps move all this code into tools/@aws-cdk/spec2cdk, or perhaps one next to it... and then it gets invoked as part of yarn gen in each package separately and confines its search and generation to that package.
| 'elasticache:DescribeServerlessCaches', | ||
| ], | ||
| Resource: { 'Fn::GetAtt': ['Cache18F6EE16', 'ARN'] }, | ||
| Resource: { |
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've confirmed that this is the correct ARN format for a serverless cache.
| public write(module: Module, filePath: string): string { | ||
| log.debug(module.name, filePath, 'render'); | ||
| const fullPath = path.join(this.rootDir, filePath); | ||
| public write(module: Module, fullPath: string): string { |
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.
The behavior here has changed: the second parameter is an absolute path, instead of relative to some root directory passed to the constructor.
Grants classes are only generated in the
aws-cdk-libmodule (if agrants.jsonfile is present). However, for alpha modules, we want the auto-generated files to be treated as alpha.This PR adds support for generating Grants classes in alpha modules, if a
grants.jsonfile is present.Also added a new
grants.jsoninaws-elasticache-alphato demonstrate that the change works.Main changes:
generateAll()now takes an additional output directory: the root of the alpha modules.GrantsPropsstruct is being passed around, that contains the config itself plus a flag indicating whether the module is stable.import * as <service> from "./<service>.generated";, for stable modules, orimport * as <service> from "aws-cdk-lib/aws-<service>";, for unstable modules.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license