upgrade apimanagement from 2018-01-01 to 2019-01-01 #5096

tnicolaysen · 2019-12-06T12:49:56Z

added support for OpenAPI import types in API resource
2018-01-01 api has been removed from /vendor
2019-01-01 api has been added to /vendor
documentation has been updated
all breaking compiler changes have been fixed
API, Policy, Subscription and User resources have been tested

TODOs are still left and awaiting further detailing after discussions and input from the community

- added support for OpenAPI import types in API resource - 2018-01-01 api has been removed from /vendor - 2019-01-01 api has been added to /vendor - documentation has been updated - all breaking compiler changes have been fixed - API, Policy, Subscription and User resources have been tested TODOs are still left and awaiting further detailing after discussions and input from the community

tnicolaysen · 2019-12-06T12:52:11Z

azurerm/resource_arm_api_management.go

@@ -553,14 +554,14 @@ func resourceArmApiManagementServiceCreateUpdate(d *schema.ResourceData, meta in
 	signInSettingsRaw := d.Get("sign_in").([]interface{})
 	signInSettings := expandApiManagementSignInSettings(signInSettingsRaw)
 	signInClient := meta.(*ArmClient).ApiManagement.SignInClient
-	if _, err := signInClient.CreateOrUpdate(ctx, resourceGroup, name, signInSettings); err != nil {
+	if _, err := signInClient.CreateOrUpdate(ctx, resourceGroup, name, signInSettings, ""); err != nil { // TODO: Check how to deal with etag


Need help verifying how to work with the If-Match header / Etags in this plugin.

tnicolaysen · 2019-12-06T12:52:19Z

azurerm/resource_arm_api_management.go

 		return fmt.Errorf("Error setting Sign In settings for API Management Service %q (Resource Group %q): %+v", name, resourceGroup, err)
 	}

 	signUpSettingsRaw := d.Get("sign_up").([]interface{})
 	signUpSettings := expandApiManagementSignUpSettings(signUpSettingsRaw)
 	signUpClient := meta.(*ArmClient).ApiManagement.SignUpClient
-	if _, err := signUpClient.CreateOrUpdate(ctx, resourceGroup, name, signUpSettings); err != nil {
+	if _, err := signUpClient.CreateOrUpdate(ctx, resourceGroup, name, signUpSettings, ""); err != nil { // TODO: Check how to deal with etag


Need help verifying how to work with the If-Match header / Etags in this plugin.

tnicolaysen · 2019-12-06T12:52:23Z

azurerm/resource_arm_api_management.go

@@ -581,7 +582,7 @@ func resourceArmApiManagementServiceCreateUpdate(d *schema.ResourceData, meta in

 		// then add the new one, if it exists
 		if policy != nil {
-			if _, err := policyClient.CreateOrUpdate(ctx, resourceGroup, name, *policy); err != nil {
+			if _, err := policyClient.CreateOrUpdate(ctx, resourceGroup, name, *policy, ""); err != nil { // TODO: Check how to deal with etag


Need help verifying how to work with the If-Match header / Etags in this plugin.

azurerm/resource_arm_api_management.go

tnicolaysen · 2019-12-06T12:54:26Z

azurerm/resource_arm_api_management.go

@@ -711,7 +712,7 @@ func resourceArmApiManagementServiceDelete(d *schema.ResourceData, meta interfac
 	log.Printf("[DEBUG] Deleting API Management Service %q (Resource Grouo %q)", name, resourceGroup)
 	resp, err := client.Delete(ctx, resourceGroup, name)
 	if err != nil {
-		if !utils.ResponseWasNotFound(resp) {
+		if !(resp.Response() != nil && (*resp.Response()).StatusCode == http.StatusNotFound) { // TODO: VERIFY!!!


The new API returns a Future type. The helper did not have support for that.
I think this codes does the same logical check, i.e. check that the response is not 404.

I don't get it.
The 2019 api already excludes 404 error
https://github.com/Azure/azure-sdk-for-go/blob/1a2940b1f9d783e439ac76230b3ecc983a8cc0b3/services/apimanagement/mgmt/2019-01-01/apimanagement/service.go#L404-L413

You referenced the code for the create function, this is the delete function.

https://github.com/Azure/azure-sdk-for-go/blob/1a2940b1f9d783e439ac76230b3ecc983a8cc0b3/services/apimanagement/mgmt/2019-01-01/apimanagement/service.go#L419-L451

It is different than in the 2018 API
https://github.com/Azure/azure-sdk-for-go/blob/1a2940b1f9d783e439ac76230b3ecc983a8cc0b3/services/apimanagement/mgmt/2018-01-01/apimanagement/service.go#L419-L456

azurerm/resource_arm_api_management_api_operation_policy.go

azurerm/resource_arm_api_management_api_policy.go

tnicolaysen · 2019-12-06T12:57:57Z

azurerm/resource_arm_api_management_api_schema.go

@@ -159,8 +167,9 @@ func resourceArmApiManagementApiSchemaDelete(d *schema.ResourceData, meta interf
 	serviceName := id.Path["service"]
 	apiName := id.Path["apis"]
 	schemaID := id.Path["schemas"]
+	force := false // TODO: Verify that this is correct


Not sure if the plugin has a concept of force. Is false the correct value?

tnicolaysen · 2019-12-06T12:58:03Z

azurerm/resource_arm_api_management_logger.go

@@ -241,8 +241,9 @@ func resourceArmApiManagementLoggerDelete(d *schema.ResourceData, meta interface
 	resourceGroup := id.ResourceGroup
 	serviceName := id.Path["service"]
 	name := id.Path["loggers"]
+	force := false // TODO: validate that this is correct


Not sure if the plugin has a concept of force. Is false the correct value?

tnicolaysen · 2019-12-06T13:01:20Z

azurerm/resource_arm_api_management_subscription.go

-			ProductID:   utils.String(productId),
+			Scope:       utils.String(scope),
 			State:       apimanagement.SubscriptionState(state),
-			UserID:      utils.String(userId),
+			OwnerID:     utils.String(ownerId),


Major breaking change in the Azure SDK.
ProductID -> Scope
UserID -> OwnerID
The values have different formats. Example from the docs:

{ "properties": { "ownerId": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.ApiManagement/service/apimService1/users/57127d485157a511ace86ae7", "scope": "/subscriptions/subid/resourceGroups/rg1/providers/Microsoft.ApiManagement/service/apimService1/products/5600b59475ff190048060002", "displayName": "testsub" } }

The Go SDK looked like it supported relative paths, i.e. /users/USER-ID
The scope property supports other things than the product, but I used product since it was used in the current implementation.

https://docs.microsoft.com/en-us/rest/api/apimanagement/2019-01-01/subscription/createorupdate

tnicolaysen · 2019-12-06T13:01:47Z

azurerm/resource_arm_api_management_subscription.go

@@ -184,8 +187,8 @@ func resourceArmApiManagementSubscriptionRead(d *schema.ResourceData, meta inter
 		d.Set("primary_key", props.PrimaryKey)
 		d.Set("secondary_key", props.SecondaryKey)
 		d.Set("state", string(props.State))
-		d.Set("product_id", props.ProductID)
-		d.Set("user_id", props.UserID)
+		d.Set("scope", props.Scope)      // TODO: check if this is a breaking change. was product_id


Will this mess up diffing (i.e. plan) after updating?

tnicolaysen · 2019-12-06T13:02:52Z

@tombuildsstuff Do you want a issue to go along with this, or is a PR enough?

- tested and looks like it behaves as expected. inspected state-file.

tombuildsstuff · 2019-12-18T10:50:13Z

hey @tnicolaysen

Thanks for this PR - apologies for the delayed review here!

Previous versions of the API Management API only supported using JSON for the API Management Schema - however API version 2019-01-01 introduces support for using SOAP/XML in addition to JSON. This is problematic for us due to the way the API Response is structured differently - where the properties object is different for XML and JSON - meaning that in it's current form this isn't deserialized correctly in the Azure SDK for Go.

We've opened several issues/PR's to fix this in the Azure SDK for Go (or rather, the Azure/azure-rest-api-specs repository, which it's generated from) - however the API team has instead opted to fix this in a new API version, which is being tracked in this Github issue.

Ultimately this means that we're currently blocked from upgrading the version of the Azure API being used for API Management until the new API version is available (which we're aware is blocking multiple PR's/bits of new functionality). However since we're currently blocked from upgrading to this API version (and I'd like to thank you for this contribution) - unfortunately I'm going to close this PR for the moment until the upstream issue has been resolved/the new API version has become available.

Thanks!

ghost · 2020-03-29T14:56:28Z

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 👉 hashibot-feedback@hashicorp.com. Thanks!

ghost added size/XXL dependencies labels Dec 6, 2019