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.

Sign up for GitHub

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

Jump to bottom

servicecatalogappregistry: Application Manager URL as string property #23779

Closed
2 tasks
alexpulver opened this issue Jan 22, 2023 · 3 comments · Fixed by #24483
Closed
2 tasks

servicecatalogappregistry: Application Manager URL as string property #23779

alexpulver opened this issue Jan 22, 2023 · 3 comments · Fixed by #24483
Assignees
@santanugho
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

Application Manager URL as string property.

P.S.: Full service name and capitalization in the applicationManagerUrl description: "Application manager url for the application created" => "Systems Manager Application Manager URL for the application created"

Use Case

I see the library defines Application Manager URL as CfnOutput. I am not sure it would be a good practice, because CfnOutput is part of customer-defined stack outwards facing interface. Customers usually define outputs they need, and don’t want outputs they don’t need. They might want to define output with exactly this name, and it will be an error (output name is unique in a template). Making this value accessible through construct interface will let the customer decide whether and where to make a CfnOutput out of this.

Proposed Solution

Make ApplicationManagerUrl a string instead of CfnOutput.

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.61.1

Environment details (OS name and version, etc.)

macOS Monterey 12.6.2
@alexpulver alexpulver added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jan 22, 2023
@santanugho
Copy link
Contributor

Thanks for your feedback, we will get this triaged and will address accordingly.

@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 Feb 8, 2023
@peterwoodworth
Copy link
Contributor

Thank you for the feature request!

@santanugho is this something you're interested in picking up still?

@zorrofox zorrofox mentioned this issue Feb 17, 2023
Closed
This was referenced Feb 28, 2023
Closed
Closed
Merged
@mergify mergify bot closed this as completed in #24483 Mar 7, 2023
mergify bot pushed a commit that referenced this issue Mar 7, 2023
@liwewang-amazon
fix(servicecatalogappregistry): allow disabling automatic CfnOutput (#…
3db1a0d 
…24483)

When an application is created using `Application` construct, an output is automatically created in the customer defined stack without customer's intention to show related Application Manager URL for the application created. This can increase customer's CFN output usage without customer acknowledge and control. 

This commit:
- emits the CFN Output in the AppRegistry managed stack where the application is created to allow all the stacks deployed in the cdk project to be associated to the application. Customers can control whether to emit the URL as CFN output by setting `emitApplicationManagerUrlAsOutput`.
- changes `applicationManagerUrl` to a string type. Customers can create further create CFN output from this property.

Closes #23779 

BREAKING CHANGE: This commit contains destructive changes to the RAM Share.
Since the application RAM share name is calculated by the application construct, where one property is removed. Integration test detects a breaking change where RAM share will be created. Integration test snapshot is updated to cater this destructive change.

----

*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

github-actions bot commented Mar 7, 2023

⚠️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.

homakk pushed a commit to homakk/aws-cdk that referenced this issue Mar 28, 2023
@liwewang-amazon @homakk
fix(servicecatalogappregistry): allow disabling automatic CfnOutput (a…
0d8608b 
…ws#24483)

When an application is created using `Application` construct, an output is automatically created in the customer defined stack without customer's intention to show related Application Manager URL for the application created. This can increase customer's CFN output usage without customer acknowledge and control. 

This commit:
- emits the CFN Output in the AppRegistry managed stack where the application is created to allow all the stacks deployed in the cdk project to be associated to the application. Customers can control whether to emit the URL as CFN output by setting `emitApplicationManagerUrlAsOutput`.
- changes `applicationManagerUrl` to a string type. Customers can create further create CFN output from this property.

Closes aws#23779 

BREAKING CHANGE: This commit contains destructive changes to the RAM Share.
Since the application RAM share name is calculated by the application construct, where one property is removed. Integration test detects a breaking change where RAM share will be created. Integration test snapshot is updated to cater this destructive change.

----

*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
Assignees

@santanugho santanugho

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
Milestone
No milestone
3 participants
@alexpulver @santanugho @peterwoodworth