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

New Resource: api_management_api #2073

Closed
wants to merge 33 commits into from
Closed

New Resource: api_management_api #2073

wants to merge 33 commits into from

Conversation

torresdal
Copy link
Contributor

Added next resource in the API Management family as requested in #1177

For now this resource is quite basic with main focus on importing API's through swagger/wsdl. Remaining future options for this resource would typically depend on other not yet developed resources which is in the pipeline (like Authorization and Product).

Note: In API Management API the concepts of Revisions and Versions are supported, but I'm not sure those concepts belong in this Terraform resource, since they are more deployment concepts and not infrastructure. Because of this I've omitted them for now, except as computed values. If you agree with this we should probably say something about that in the docs.

@ghost ghost added the size/XXL label Oct 13, 2018
tombuildsstuff
tombuildsstuff suggested changes Oct 23, 2018
View reviewed changes
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @torresdal

Thanks for this PR :)

I've taken a look through and this is off to a good start - I've left some questions/comments inline, particularly around exposing soap_pass_through vs the api_type - but most of the comments I've left are fairly minor and this is looking pretty good 👍

Thanks!

azurerm/config.go Show resolved Hide resolved
azurerm/data_source_api_management_api.go Outdated Show resolved Hide resolved
azurerm/data_source_api_management_api.go Outdated Show resolved Hide resolved
azurerm/data_source_api_management_api.go Outdated Show resolved Hide resolved
azurerm/data_source_api_management_api.go Show resolved Hide resolved
website/docs/r/api_management_api.html.markdown Outdated Show resolved Hide resolved
website/docs/r/api_management_api.html.markdown Outdated Show resolved Hide resolved
website/docs/r/api_management_api.html.markdown Outdated Show resolved Hide resolved
website/docs/r/api_management_api.html.markdown Outdated Show resolved Hide resolved
website/docs/r/api_management_api.html.markdown Outdated Show resolved Hide resolved
@torresdal
Copy link
Contributor Author

@tombuildsstuff I've resolved the ones that I've made code changes to, but left the ones I've commented on un-resolved. Could you please have a look at my comments, so we can find a solution to those, and then I'll push my changes?

@torresdal
Copy link
Contributor Author

torresdal commented Oct 25, 2018 •
edited
Loading

Just pushed the changes we did so far as a result of the review. Only thing missing now is to have a look at the unresolved items in the review and agree on a solution for each.

Basically agree on what to do with:

  • api_type
  • to use IsNewResource / ForceNew for Import or not

@katbyte katbyte added this to the 1.19.0 milestone Oct 25, 2018
@tombuildsstuff tombuildsstuff modified the milestones: 1.19.0, 1.20.0 Nov 14, 2018
torresdal and others added 16 commits November 22, 2018 13:28
@torresdal
Added basic support for API Management API
97cde54
@torresdal
Cleaned up unused schema elements of API Management API
19d3895
@torresdal
Added docs for api_management_api resource
ad9c3b1
@torresdal
Added missing fields to schema
091bed3
@torresdal
Cleaned up, tested and added data to schema fields
b7be791
@torresdal
Cleanup:
9ada8fe 
* Replaced soap_api_type with bool soap_pass_through
* Added validation for api path and name
* Added validation tests for api path and name
@torresdal
Updated docs for API Management API resource and data to reflect rece…
dfe6909 
…nt changes
@torresdal
Fixed sidebar issues in doc
bf8f2d7
@torresdal
Removed display_name option - name works for both
469322b
@torresdal
Add simple test for data source
74f443f
@torresdal
Add simple test for API Management API
f2b49b6
@torresdal
Testing:
b5e03aa 
* Fixed basic data test and include more props to check
* Replaced test data with files instead of pointing to urls
* Add complete test for API Management API
@torresdal
Fixed error found in integration tests:
c523a99 
* Ignore import, since state is only persisted in Terraform
* Removed location from schema
* Add seperate get after creating resource to get ID
@torresdal
Check for generated name in test. Use local swagger file instead of url.
e4ac886
@tombuildsstuff @torresdal
Output APIM Service name on error
22fef3c 
Co-Authored-By: torresdal <[email protected]>
@tombuildsstuff @torresdal
Output APIM Service name on error
9726bb1 
Co-Authored-By: torresdal <[email protected]>
tombuildsstuff and others added 10 commits November 22, 2018 13:28
@torresdal
Copy link
Contributor Author

@tombuildsstuff Anything else I should do or we should agree on before moving this on?

@ghost ghost removed the waiting-response label Nov 27, 2018
@tombuildsstuff tombuildsstuff modified the milestones: 1.20.0, 1.21.0 Dec 7, 2018
@torresdal torresdal closed this Dec 13, 2018
@torresdal torresdal mentioned this pull request Dec 23, 2018
Closed
@katbyte katbyte removed this from the 1.21.0 milestone Jan 15, 2019
@AndyMoore
Copy link
Contributor

can this be reopened and added to a future release?

@tombuildsstuff @katbyte @torresdal

@ghost
Copy link

ghost commented Mar 5, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tombuildsstuff tombuildsstuff tombuildsstuff requested changes

Assignees

@tombuildsstuff tombuildsstuff

Labels
new-resource service/api-management size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@torresdal @AndyMoore @tombuildsstuff @katbyte