feat(servicecatalogappregistry): application-associator L2 Construct #22024
Conversation
…ssociate allStacks inside a cdk App via new construct Automatic Application.
|
|
| }; | ||
|
|
||
| const app = new App(); | ||
| const autoApp = new appreg.AutomaticApplication(app, 'AutoApplication', { |
There was a problem hiding this comment.
Per what I see in the code, AutomaticApplication is a Construct. Construct's scope should be a Stack, not App. I think it makes more sense to have AppRegistry application to be part of the component's toolchain. The toolchain contains anything related to software development life cycle of the component. Some examples: 1/ pull request validation in trunk-based development model 2/ tenant provisioning logical unit in multi-tenant SaaS applications using silo deployment model. Here is an example of a toolchain containing continuous deployment and pull request validation logical units: https://github.com/alexpulver/usermanagement-backend/blob/main/toolchain.py
There was a problem hiding this comment.
An approach similar to this AWS AppConfig blog post which is aligned with AWS mutli-account recommendations.
There was a problem hiding this comment.
Construct's scope should be a Stack, not App.
I don't agree with this necessarily. A construct's scope can be any other construct, including an App. Yes, CFN resources ultimately need to be created in the (transitive) scope of a Stack, but this thing can be an exception.
Having said that, I think you actually mean something else. I think you mean something along the lines of:
In the case of a Pipeline deployment, the Application should be created in the scope of the Pipeline Stack/Toolchain stack, not at the top level.
I tend to agree that's what most people would probably want. But wouldn't that just mean you don't use AutomaticApplication, but use a plain old Application instead?
| const autoApp = new appreg.AutomaticApplication(app, 'AutoApplication', { | ||
| applicationName: 'MyAutoApplication', | ||
| description: 'Testing auto application', | ||
| const registeredApp = new appreg.RegisterApplication(app, 'RegisterApplication', { |
There was a problem hiding this comment.
Hi, I am sure the team thought deeply about a name of this class. Still, I think having a class name expressing an action instead of an object looks unintuitive. Perhaps that should be a factory method on the application class? Something like appreg.Application.registerStacks(app)?
Related topics in AWS Construct Library Design Guidelines:
- Construct class - "The name of resource constructs must be identical to the name of the resource in the AWS API, which should be consistent with the resource name in the AWS CloudFormation spec"
- Factories
P.S.: Another reference can be cdk-nag project.
| }; | ||
|
|
||
| const app = new App(); | ||
| const registeredApp = new appreg.RegisterApplication(app, 'RegisterApplication', { |
There was a problem hiding this comment.
I also don't particularly like this new name. It should be a noun.
ApplicationRegisterer then, if you want to use that word. Though I kinda don't like that either.
mergify
bot
dismissed
rix0rrr’s stale review
Pull request has been modified.
rohitagg0807
changed the title
|
I will merge it to get the testing going. This is going into an alpha module so we can still change the API. Before we stabilize I would like to see the pipeline usage validated with users. |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…ws#22024) new application-associator L2 Construct : This construct is responsible for following: * Create a new AppRegistry Application * Associate all stacks inside a cdk app scope * share an app registry application upon determining cross account stack. [ This only works for non environment agnostic stack] ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* Co-authored by: Santanu Ghosh
…ws#22024) new application-associator L2 Construct : This construct is responsible for following: * Create a new AppRegistry Application * Associate all stacks inside a cdk app scope * share an app registry application upon determining cross account stack. [ This only works for non environment agnostic stack] ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* Co-authored by: Santanu Ghosh
new application-associator L2 Construct : This construct is responsible for following:
All Submissions:
Adding new Unconventional Dependencies:
New Features
yarn integto deploy the infrastructure and generate the snapshot (i.e.yarn integwithout--dry-run)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
Co-authored by: Santanu Ghosh