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

DiffSuppressFunc to ignore azurerm_virtual_network.address_space reordering #23793

Merged
merged 3 commits into from Apr 23, 2024

Conversation

tiwood
Copy link
Contributor

@tiwood tiwood commented Nov 6, 2023

This changes azurerm_virtual_network.address_space from TypeList to TypeSet, because

  • The Azure API is ignoring ordering changes to this property (aka endless plan diffs)
  • The ordering as no technical effect on the network itself

Fixes #23328

…fect and is not updated by Azure if the element vals are not changing
Copy link
Member

@jackofallops jackofallops left a comment

Choose a reason for hiding this comment

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

Hi @tiwood
Thanks for this PR. Unfortunately a type change from a List to a Set is considered a breaking change in the schema, and given the importance and prevalence of use of this resource we're reluctant to do so to avoid breaking users configurations (for example, this would require changes to any interpolations of this field value). We understand the frustration of the diff, but this will need to be addressed in another way. I can suggest trying a CustomiseDiff which could normalise the order of this list for the old/new comparison and flag correct diffs appropriately. Is this something you can look into?

Alternatively, we can possibly reconsider this for a major version where we are better able to communicate and accept breaking changes

Thanks

@tiwood tiwood changed the title Type change: azurerm_virtual_network.address_space to TypeSet DiffSuppressFunc to ignore azurerm_virtual_network.address_space reordering Nov 10, 2023
@tiwood
Copy link
Contributor Author

tiwood commented Nov 11, 2023

Hi @jackofallops
Thanks for the review. I've implemented a custom diff function to mimic the set behaviour.
WDYT?

@tiwood
Copy link
Contributor Author

tiwood commented Nov 15, 2023

Hi @jackofallops
we're currently having huge diffs in one of our core automation stacks and merging this would really help us.
If we could get this on the way in the next days it would be much appreciated.

@tiwood
Copy link
Contributor Author

tiwood commented Nov 30, 2023

@jackofallops, @magodo, @tombuildsstuff

Sorry for the random mention, I was wondering if this can be reviewed soon?

@tiwood
Copy link
Contributor Author

tiwood commented Dec 7, 2023

@katbyte maybe you can help reviewing this?

@tiwood
Copy link
Contributor Author

tiwood commented Jan 24, 2024

@jackofallops friendly ping

@tombuildsstuff tombuildsstuff self-assigned this Feb 7, 2024
@stephybun stephybun added this to the v4.0.0 milestone Apr 3, 2024
@tiwood
Copy link
Contributor Author

tiwood commented Apr 8, 2024

@stephybun, merging this PR is no longer a breaking change, as I've changed the implementation according to the requested changes.

I really would appreciate a review. (ping @tombuildsstuff)

@stephybun stephybun removed this from the v4.0.0 milestone Apr 9, 2024
@jackofallops jackofallops merged commit 9060178 into hashicorp:main Apr 23, 2024
20 checks passed
@github-actions github-actions bot added this to the v3.101.0 milestone Apr 23, 2024
jackofallops added a commit that referenced this pull request Apr 23, 2024
Copy link
Member

@jackofallops jackofallops left a comment

Choose a reason for hiding this comment

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

post-merge approval due to browser error 🙈

dduportal pushed a commit to jenkins-infra/azure that referenced this pull request Apr 26, 2024
<Actions>
<action
id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8">
        <h3>Bump Terraform `azurerm` provider version</h3>
<details
id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24">
            <summary>Update Terraform lock file</summary>
<p>changes detected:&#xA;&#x9;&#34;hashicorp/azurerm&#34; updated from
&#34;3.100.0&#34; to &#34;3.101.0&#34; in file
&#34;.terraform.lock.hcl&#34;</p>
            <details>
                <summary>3.101.0</summary>
<pre>Changelog retrieved
from:&#xA;&#x9;https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.101.0&#xA;ENHANCEMENTS:&#xA;&#xA;*
dependencies: updating to `v0.20240424.1114424` of
`github.com/hashicorp/go-azure-sdk`
([#25749](https://github.com/hashicorp/terraform-provider-azurerm/issues/25749))&#xA;*
dependencies: updating to `v0.27.0` of
`github.com/tombuildsstuff/giovanni`
([#25702](https://github.com/hashicorp/terraform-provider-azurerm/issues/25702))&#xA;*
dependencies: updating `golang.org/x/net` to `0.23.0`&#xA;*
`azurerm_cognitive_account` - the `kind` property now supports
`ConversationalLanguageUnderstanding`
([#25735](https://github.com/hashicorp/terraform-provider-azurerm/issues/25735))&#xA;*
`azurerm_container_app_custom_domain` - support the ability to use Azure
Managed Certificates
([#25356](https://github.com/hashicorp/terraform-provider-azurerm/issues/25356))&#xA;&#xA;BUG
FIXES:&#xA;&#xA;* Data Source: `azurerm_application_insights` - set
correct AppID in data source
([#25687](https://github.com/hashicorp/terraform-provider-azurerm/issues/25687))&#xA;*
`azurerm_virtual_network` - suppress diff in ordering for
`address_space` due to inconsistent API response
([#23793](https://github.com/hashicorp/terraform-provider-azurerm/issues/23793))&#xA;*
`azurerm_storage_data_lake_gen2_filesystem` - add context deadline for
import
([#25712](https://github.com/hashicorp/terraform-provider-azurerm/issues/25712))&#xA;*
`azurerm_virtual_network_gateway` - preserve existing `nat_rules` on
updates
([#25690](https://github.com/hashicorp/terraform-provider-azurerm/issues/25690))&#xA;&#xA;&#xA;</pre>
            </details>
        </details>
<a
href="https://infra.ci.jenkins.io/job/updatecli/job/azure/job/main/135/">Jenkins
pipeline link</a>
    </action>
</Actions>

---

<table>
  <tr>
    <td width="77">
<img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli
logo" width="50" height="50">
    </td>
    <td>
      <p>
Created automatically by <a
href="https://www.updatecli.io/">Updatecli</a>
      </p>
      <details><summary>Options:</summary>
        <br />
<p>Most of Updatecli configuration is done via <a
href="https://www.updatecli.io/docs/prologue/quick-start/">its
manifest(s)</a>.</p>
        <ul>
<li>If you close this pull request, Updatecli will automatically reopen
it, the next time it runs.</li>
<li>If you close this pull request and delete the base branch, Updatecli
will automatically recreate it, erasing all previous commits made.</li>
        </ul>
        <p>
Feel free to report any issues at <a
href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br
/>
If you find this tool useful, do not hesitate to star <a
href="https://github.com/updatecli/updatecli/stargazers">our GitHub
repository</a> as a sign of appreciation, and/or to tell us directly on
our <a
href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>!
        </p>
      </details>
    </td>
  </tr>
</table>

Co-authored-by: Jenkins Infra Bot (updatecli) <60776566+jenkins-infra-bot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

azurerm_virtual_network see constant drift if address_space array order is updated
4 participants