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

azurerm_api_management_api wsdl_selector` is required when content_format = "wsdl" #7076

Merged
merged 2 commits into from Jun 11, 2020

Conversation

yupwei68
Copy link
Contributor

Fix #6795

yupwei68 added 2 commits May 21, 2020 10:37
@ghost ghost added the size/XS label May 26, 2020
@@ -258,6 +258,12 @@ func resourceArmApiManagementApiCreateUpdate(d *schema.ResourceData, meta interf
},
}
wsdlSelectorVs := importV["wsdl_selector"].([]interface{})

Copy link
Contributor

Choose a reason for hiding this comment

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

I saw you linked this to #6795, but I don't think that it's the cause for what I reported. It also affected swagger_json formats.

I think it might have something to do with the whole if-statement that this code is in. The comment on line 241-242 is:

// If import is used, we need to send properties to Azure API in two operations.
// First we execute import and then updated the other props.

This was added when the underlying API was 2018-XX-XX. and I'm guessing that it was a workaround for a bug back then. I could not find anything in the REST API doc. that stated it has to be two requests.

Could it be an idea to do this in one request instead of two? Should not be very difficult to merge the request into one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @tnicolaysen ,for issue #6795, I have tried to add wsdl_selector in the code, and it's created with success. Do I miss anything?

Copy link
Contributor

Choose a reason for hiding this comment

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

I can try to make a build from this branch and see if it solves my problem. But, it is still weird that the code performs the update in two steps.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have tested this branch with my setup.

  • Creation of new OpenAPI APIs work now.
  • Update of existing APIs work.
  • Switch from swagger_json to openapi on an existing API worked.
  • The primary/secondary key issue is confirmed fixed for me. (Not relevant to this PR).

I also got this expected error:

Error: `wsdl_selector` is required when content format is `wsdl` in API Management API "xxx-api"

  on main.tf line 163, in resource "azurerm_api_management_api" "xxx":
 163: resource "azurerm_api_management_api" "xxx" {

I didn't know that it was required. It's not required when importing a WSDL through Azure Portal. Maybe they just default to the first or something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hi @tnicolaysen , yes, I have split the two problems in two PRs. Would you like to try if the below code works for you? And on portal, wsdl_selector is required as the screenshot below.

resource "azurerm_api_management_api" "example" {
  api_management_name = data.azurerm_api_management.apim.name
  resource_group_name = data.azurerm_api_management.apim.resource_group_name

  display_name      = "Example API"
  name              = "example-api"
  path              = "example-api"
  protocols         = ["https"]
  revision          = "1"
  service_url       = "http://example.org"
  soap_pass_through = true

  // NOTICE: It partially works when not having an import, but it's just one of the issues
  import {
    content_format = "wsdl"
    content_value = file("example.wsdl")

 wsdl_selector {
      service_name  = "Calculator"
      endpoint_name = "CalculatorHttpsSoap11Endpoint"
    }
  }
}

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we're seeing different UIs. Might have something to do with the underlying API-M version and when it was provisioned.

image

I tested with code that was similar to what you showed, and it worked for me.

@tnicolaysen
Copy link
Contributor

Any chance to get this one merged for the next release? :)

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Thanks @yupwei68 - LGTM 👍

@katbyte katbyte added this to the v2.14.0 milestone Jun 11, 2020
@katbyte katbyte merged commit 62c16c1 into hashicorp:master Jun 11, 2020
@yupwei68 yupwei68 deleted the wyp-apim-api branch June 11, 2020 03:11
katbyte added a commit that referenced this pull request Jun 11, 2020
@ghost
Copy link

ghost commented Jun 11, 2020

This has been released in version 2.14.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.14.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Jul 11, 2020

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!

@hashicorp hashicorp locked and limited conversation to collaborators Jul 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

issues with subscription key and import in API-M since 2.7.0
3 participants