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

Update and extend support for Service Catalog #18074

Closed
wants to merge 1 commit into from

Conversation

ahgittin
Copy link

This consists of the following:

  • Minor fixes to existing resource_aws_servicecatalog_portfolio support
  • Addition of resource_aws_servicecatalog_product building on work done by @trung and @bw-intuit in New Resource: aws_servicecatalog_product (compliment PR 2064) #4980
  • Addition of several other resources:
    • To provision products:
      • resource_aws_servicecatalog_portfolio_principal_association
      • resource_aws_servicecatalog_portfolio_product_association
      • resource_aws_servicecatalog_provisioned_product
    • Constraints:
      • resource_aws_servicecatalog_constraint (any constraint)
      • resource_aws_servicecatalog_launch_notification_constraint
      • resource_aws_servicecatalog_launch_role_constraint
      • aws/resource_aws_servicecatalog_launch_template_constraint

The long commit history is in #14281 but has been squashed here to simplify history when merged upstream.
As can be seen there much of the code here is due to @kemitix as well.
`

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #0000

Closes #4980 #13797 #13834 #13864 #13866 #13837 #14281

Release note for CHANGELOG:

New Resource: `aws_servicecatalog_product`
New Resource: `aws_servicecatalog_portfolio_product_association`
New Resource: `aws_servicecatalog_portfolio_principal_association`
New Resource: `aws_servicecatalog_provisioned_product`
New Resource: `aws_servicecatalog_constraint`
New Resource: `aws_servicecatalog_launch_notification_constraint`
New Resource: `aws_servicecatalog_launch_role_constraint`
New Resource: `aws_servicecatalog_launch_template_constraint`

Output from acceptance testing will be posted when the run completes. (The PRs referenced above do already show the acceptance tests being run on individual features.)

This consists of the following:

* Minor fixes to existing `resource_aws_servicecatalog_portfolio` support
* Addition of `resource_aws_servicecatalog_product` building on work done by @trung and @bw-intuit in hashicorp#4980
* Addition of several other resources:
  * To provision products:
    * `resource_aws_servicecatalog_portfolio_principal_association`
    * `resource_aws_servicecatalog_portfolio_product_association`
    * `resource_aws_servicecatalog_provisioned_product`
  * Constraints:
    * `resource_aws_servicecatalog_constraint` (any constraint)
    * `resource_aws_servicecatalog_launch_notification_constraint`
    * `resource_aws_servicecatalog_launch_role_constraint`
    * `aws/resource_aws_servicecatalog_launch_template_constraint`

The long commit history is in hashicorp#14281 but has been squashed here to simplify history when merged upstream.
As can be seen there much of the code here is due to @kemitix as well.
`
@ahgittin ahgittin requested a review from a team as a code owner March 12, 2021 17:19
@ghost ghost added size/XXL Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS. service/servicecatalog Issues and PRs that pertain to the servicecatalog service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Mar 12, 2021
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Mar 12, 2021
@ahgittin
Copy link
Author

@YakDriver I can see the format linter has new rules which this now fails. Probably worth merging master in to this as well.

Any review or input is totally welcome ahead of this but otherwise leave this with us. Happy to mark the PR [WIP] or close it until we've fixed it.

@YakDriver YakDriver removed the needs-triage Waiting for first response or review from a maintainer. label Mar 23, 2021
@YakDriver YakDriver self-assigned this Mar 23, 2021
@YakDriver
Copy link
Member

YakDriver commented Mar 23, 2021

Overall, good job! This is looking much more manageable.

To finish this off, this is my proposal. For a first phase, I've done a very quick review and noted some general items that need to be fixed below. Are you able and willing to fix these in the relatively near future?

After the first phase, in the second phase, I'll fix the final items after going through with a fine-tooth comb. Make sure you've checked "Allow edits from maintainers." You will receive all credit for your work - I will only add commits.

This approach should help us get this done relatively quickly.

Phase 1, general areas for you to fix:

@YakDriver YakDriver added the waiting-response Maintainers are waiting on response from community or contributor. label Mar 23, 2021
@ahgittin
Copy link
Author

Thanks @YakDriver . We based this original work on pre-existing files but I guess we picked not great exemplars, and the linter passed at the original PR, but these are all good new requirements and make sense.

We'll let you know when we can address them -- hopefully soon.

@ghost ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Mar 25, 2021
@YakDriver
Copy link
Member

@ahgittin Can you have your work up to where you're at currently pushed by Friday (April 9)? I understand completely that you are likely to have other priorities. Based on our commitments, I will take over where ever you leave off to get this off the ground. Thank you for all your help! I may cherry pick your commits to another branch but you will still be credited for them.

@TheRealKim
Copy link

Is there anything I can do to help move this PR forward? I am not really an expert in GO but can probably figure out how to fix linting and/or documentation issues if any. I have build the code in this commit locally and the terraform resource resource_aws_servicecatalog_provisioned_product is exactly what I am looking for to be able to continue my project. So would it be a possibility to cherry pick the code for this resource as well and have it in a separate PR?
Thanks a lot ahgittin and Yakdriver for already adding more support for AWS service catalog for terraform!

@ghost ghost removed waiting-response Maintainers are waiting on response from community or contributor. labels May 26, 2021
@YakDriver
Copy link
Member

Since these resource have been added, I'm going to close this PR.

@YakDriver YakDriver closed this Jul 13, 2021
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS. service/servicecatalog Issues and PRs that pertain to the servicecatalog service. size/XXL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants