r/api_management_api_policy: sending policy as Raw XML

[riordanp]

Should fix #3918

I have updated the existing test case for policy links and string policies. I will try and figure out how to run the full test suite tomorrow. The issue might affect the other policy resources as well so I'll check that at the same time.

[tombuildsstuff]

hey @riordanp

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

Taking a look through this mostly LGTM however I think needs one fix to work for downloaded XML files (which I've left a comment inline for) - I've kicked off the test suite anyway and I'll update once they're complete :)

Thanks!

[riordanp]

I have run the acceptance tests and updated test cases for the other policy resources, which also seem to fail.

The fix has sort of worked, rather than failing during TestAccAzureRMApiManagementAPIPolicy_basic with a 400 error, it now fails during TestAccAzureRMApiManagementAPIPolicy_update with the message

--- FAIL: TestAccAzureRMApiManagementAPIPolicy_update (2298.50s)
    testing.go:568: Step 1 error: After applying this step, the plan was not empty:

with xml_content in the diff changing like:

xml_content:         "<policies>\r\n\t<inbound>\r\n\t\t<set-variable name=\"abc\" value=\"@(context.Request.Headers.GetValueOrDefault(&quot;X-Header-Name&quot;, &quot;&quot;))\" />\r\n\t\t<find-and-replace from=\"xyz\" to=\"abc\" />\r\n\t</inbound>\r\n</policies>" => "<policies>\n  <inbound>\n    <set-variable name=\"abc\" value=\"@(context.Request.Headers.GetValueOrDefault(\"X-Header-Name\", \"\"))\" />\n    <find-and-replace from=\"xyz\" to=\"abc\" />\n  </inbound>\n</policies>\n"

It looks like API-M is adding Windows style line endings (I'm running from git bash on Windows) and encoding spaces as tabs, but the DiffSuppressFunc on the property should prevent this type of thing? Any suggestions @tombuildsstuff?

[tombuildsstuff]

hey @riordanp

Sorry for the delayed response - I'd forgotten to post the test results here 🤦‍♂

It looks like API-M is adding Windows style line endings (I'm running from git bash on Windows) and encoding spaces as tabs, but the DiffSuppressFunc on the property should prevent this type of thing?

That was my guess - digging into this it appears this is because Golang's XML parser doesn't consider the line:

<set-variable name="abc" value="@(context.Request.Headers.GetValueOrDefault("X-Header-Name", ""))" />

to be valid XML, since parsing it via the DiffSuppressFunc returns the error: Error parsing XML: XML syntax error on line 3: attribute name without = in element - where any error returned means the Diff won't be suppressed.

That said - it appears that the quotes within the quotes returned from Azure here are encoded, as such we should be able to update the tests to account for this. I'm taking a look at this at the moment and I'm hoping that with ignoring tabs/newlines should allow this to work as expected; will update in a bit :)

[jeanpaulsmit]

For me this issue is really blocking, as it can't be easily done using ARM templates just for API policies.
I see this is part of v1.36, when is this scheduled for release?

[tombuildsstuff]

hey @riordanp

Apologies for the delayed response here!

I spent some time yesterday digging into this - there's a couple of problems in tandem:

Since the XML Content can contain expressions, the Azure API returns this XML Encoded, which varies from the state locally. Whilst it's possible to XML Encode the XML Content from the Terraform Configuration - unfortunately this applies the encoding to the entire file (e.g. <policies>.. becomes >policies<) which makes this invalid XML when it hits Azure. I've also spent some time trying the inverse of this - however whilst Golang itself has an EscapeString method, there's no UnescapeString method to normalise this.

In order to work around this I've had to do a combination of things which definitely isn't ideal, but appears to solve the issue. We now HTML Unencode the Policy from the API (since, this is close enough to XML Unencoding for our purposes) - however this also required a custom diff function to first try parsing this as XML (if there's no expressions) and then fall back to string comparison, which whilst isn't ideal seems to work for these purposes.

I've run the tests for this PR (and added another for the policy content on it's own) - as such I hope you don't mind but so that we can get this merged I've pushed those changes (and rebased this on top of master).

Thanks!

[tombuildsstuff]

Tests pass:

[jeanpaulsmit]

Awesome, although I managed to work around this by encoding the specific expression in the policy, I'm very happy to see this solved. Thanks.

[tombuildsstuff]

@jeanpaulsmit once this is merged you may need to alter the local config so that it's decoded - but I believe that scenario should be covered by the tests 👍

[jeanpaulsmit]

@tombuildsstuff is this also fixed for links to externally stored policies? I'm still getting "Expecting white space" when I use a policy containing this in the inbound section:
<set-variable name="token" value="@(context.Request.Headers.GetValueOrDefault("field"))" />

while using this script:
resource "azurerm_api_management_api_policy" "apiPolicy" {
api_name = "${azurerm_api_management_api.api.name}"
api_management_name = "${data.azurerm_api_management.apim.name}"
resource_group_name = "${data.azurerm_resource_group.rg.name}"

xml_link = "https://some-url/policy.xml"
}

Or does that still need to be decoded somehow?

[tombuildsstuff]

@jeanpaulsmit this hasn't shipped yet - but it's also fixed for external/URI downloaded policies when it ships :)

[jeanpaulsmit]

@tombuildsstuff aha, so that will be available in v1.37! Cool and thanks again.

[ghost]

This has been released in version 1.37.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 = "~> 1.37.0"
}
# ... other configuration ...

[ghost]

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!