Skip to content
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

servicecatalogappregistry: Use cdk.App as scope for TargetApplication application #22973

Closed
1 of 2 tasks
alexpulver opened this issue Nov 18, 2022 · 4 comments · Fixed by #22977
Closed
1 of 2 tasks

servicecatalogappregistry: Use cdk.App as scope for TargetApplication application #22973

alexpulver opened this issue Nov 18, 2022 · 4 comments · Fixed by #22977
Assignees
Labels
@aws-cdk/aws-servicecatalogappregistry effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@alexpulver
Copy link
Contributor

Describe the feature

Use stackId prop value verbatim in TargetApplication class, with no prefix.

Update the stackId prop description to align with the definition in Identifiers documentation:
“The id of a stack is also the identifier that you use to refer to it in the AWS CDK Toolkit (cdk command).”

Use Case

As a user, I want to have full control over the stack ID, to maintain a naming convention when using AWS CDK Toolkit. AWS CDK Construct Library v2.51.0 allows to set the stack ID for TargetApplication, but prefixes the name with the containing construct ID, which I think it shouldn’t.

For an example, the following example code snippet:

application = appregistry_alpha.TargetApplication.create_application_stack(
    application_name=constants.APP_NAME,
    stack_id=constants.APP_NAME + "AppRegistryApplication",
    env=cdk.Environment(
        account=APPREGISTRY_APPLICATION_ACCOUNT,
        region=APPREGISTRY_APPLICATION_REGION,
    ),
)
appregistry_application_associator = appregistry_alpha.ApplicationAssociator(
    app, "AppRegistryApplicationAssociator", applications=[application]
)

Produces the following stack ID for the AppRegistry application (I define the other stacks elsewhere):

Supply a stack id (AppRegistryApplicationAssociator/UserManagementBackendAppRegistryApplication, UserManagementBackendSandbox, UserManagementBackendToolchain) to display its template. 

I expected the behavior to be:

Supply a stack id (UserManagementBackendAppRegistryApplication, UserManagementBackendSandbox, UserManagementBackendToolchain) to display its template. 

Proposed Solution

Update this to scope in

this.application = targetApplication.bind(this).application;
(thanks @rohitagg0807!)

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.51.0

Environment details (OS name and version, etc.)

macOS Monterey 12.6.1

@alexpulver alexpulver added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Nov 18, 2022
@peterwoodworth peterwoodworth added p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Nov 18, 2022
@peterwoodworth
Copy link
Contributor

Thanks @rohitagg0807 for the PR.

I'm not certain if this is a direction we'll want to take, but I can see the reasoning. If we do want to take this direction, we'll be able to make this change even though it's breaking as the module not yet considered stable

@rohitagg0807
Copy link
Contributor

Yes this is the direction we want to take and move forward.

@mergify mergify bot closed this as completed in #22977 Nov 28, 2022
mergify bot pushed a commit that referenced this issue Nov 28, 2022
… to give user more control over the passed stackId (#22977)

As a user, I want to have full control over the stack ID, to maintain a naming convention when using AWS CDK Toolkit. AWS CDK Construct Library v2.51.0 allows to set the stack ID for TargetApplication, but prefixes the name with the containing construct ID. Fixes #22973 

BREAKING CHANGE: Stack inside ApplicationAssociator is no longer is created inside ApplicationAssociator Construct scope. The stack will now get created inside cdk.App scope.
* ** servicecatalogappregistry:** stackId  will no longer have ApplicationAssociator Construct scope.

### 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

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] 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*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@alexpulver
Copy link
Contributor Author

@rohitagg0807 thank you! It looks much better now from user experience perspective 😄

brennanho pushed a commit to brennanho/aws-cdk that referenced this issue Dec 9, 2022
… to give user more control over the passed stackId (aws#22977)

As a user, I want to have full control over the stack ID, to maintain a naming convention when using AWS CDK Toolkit. AWS CDK Construct Library v2.51.0 allows to set the stack ID for TargetApplication, but prefixes the name with the containing construct ID. Fixes aws#22973 

BREAKING CHANGE: Stack inside ApplicationAssociator is no longer is created inside ApplicationAssociator Construct scope. The stack will now get created inside cdk.App scope.
* ** servicecatalogappregistry:** stackId  will no longer have ApplicationAssociator Construct scope.

### 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

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] 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*
brennanho pushed a commit to brennanho/aws-cdk that referenced this issue Jan 20, 2023
… to give user more control over the passed stackId (aws#22977)

As a user, I want to have full control over the stack ID, to maintain a naming convention when using AWS CDK Toolkit. AWS CDK Construct Library v2.51.0 allows to set the stack ID for TargetApplication, but prefixes the name with the containing construct ID. Fixes aws#22973 

BREAKING CHANGE: Stack inside ApplicationAssociator is no longer is created inside ApplicationAssociator Construct scope. The stack will now get created inside cdk.App scope.
* ** servicecatalogappregistry:** stackId  will no longer have ApplicationAssociator Construct scope.

### 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

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] 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*
brennanho pushed a commit to brennanho/aws-cdk that referenced this issue Feb 22, 2023
… to give user more control over the passed stackId (aws#22977)

As a user, I want to have full control over the stack ID, to maintain a naming convention when using AWS CDK Toolkit. AWS CDK Construct Library v2.51.0 allows to set the stack ID for TargetApplication, but prefixes the name with the containing construct ID. Fixes aws#22973 

BREAKING CHANGE: Stack inside ApplicationAssociator is no longer is created inside ApplicationAssociator Construct scope. The stack will now get created inside cdk.App scope.
* ** servicecatalogappregistry:** stackId  will no longer have ApplicationAssociator Construct scope.

### 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

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] 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*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-servicecatalogappregistry effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
4 participants