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

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

Merged
merged 2 commits into from
Jun 11, 2020
Merged

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

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b4ee9f
update
May 21, 2020
72355bf
r1
May 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions azurerm/internal/services/apimanagement/api_management_api_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

//`wsdl_selector` is necessary under format `wsdl`
if len(wsdlSelectorVs) == 0 && contentFormat == string(apimanagement.Wsdl) {
return fmt.Errorf("`wsdl_selector` is required when content format is `wsdl` in API Management API %q", name)
}

if len(wsdlSelectorVs) > 0 {
wsdlSelectorV := wsdlSelectorVs[0].(map[string]interface{})
wSvcName := wsdlSelectorV["service_name"].(string)
Expand Down