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

Use Rawxml format for global API policies string #9296

Merged
merged 1 commit into from
Nov 19, 2020
Merged

Use Rawxml format for global API policies string #9296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions azurerm/internal/services/apimanagement/api_management_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/apimanagement/parse"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/suppress"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
)
Expand Down Expand Up @@ -357,7 +356,7 @@ func resourceArmApiManagementService() *schema.Resource {
Optional: true,
Computed: true,
ConflictsWith: []string{"policy.0.xml_link"},
DiffSuppressFunc: suppress.XmlDiff,
DiffSuppressFunc: XmlWithDotNetInterpolationsDiffSuppress,
},

"xml_link": {
Expand Down Expand Up @@ -1343,7 +1342,7 @@ func expandApiManagementPolicies(input []interface{}) (*apimanagement.PolicyCont
if xmlContent != "" {
return &apimanagement.PolicyContract{
PolicyContractProperties: &apimanagement.PolicyContractProperties{
Format: apimanagement.XML,
Format: apimanagement.Rawxml,
Copy link
Member

Choose a reason for hiding this comment

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

Running the tests for this we get the following diff:

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

To fix that we'll also need to update this line: https://github.com/terraform-providers/terraform-provider-azurerm/blob/master/azurerm/internal/services/apimanagement/api_management_resource.go#L362

to ignore the differences when comparing the diff - since these get returned encoded from the API, we can do this using this function: https://github.com/terraform-providers/terraform-provider-azurerm/blob/841a9c9f7da454581b6fa761754909e90d3f8b7e/azurerm/internal/services/apimanagement/api_management_product_policy_resource.go#L47

As such could we update this so that this test passes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review and the guidance. I will update the code accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How come the acceptance test didn't fail in GitHub? Are only unit tests performed there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was unfortunately not able to get the acceptance test to run in my VS Code devcontainer. I think I would have to set a whole bunch of ARM_* environment variables somehow.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah Github Actions only runs the unit tests - the acceptance tests take longer (and can cost a lot) - which is why we run these manually using another tool

No worries - I'll kick off the tests again now but I think we should be good to go :)

Value: utils.String(xmlContent),
},
}, nil
Expand Down
1 change: 1 addition & 0 deletions azurerm/internal/services/apimanagement/tests/api_management_resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,7 @@ resource "azurerm_api_management" "test" {
xml_content = <<XML
<policies>
<inbound>
<set-variable name="abc" value="@(context.Request.Headers.GetValueOrDefault("X-Header-Name", ""))" />
<find-and-replace from="xyz" to="abc" />
</inbound>
</policies>
Expand Down