In v3.0.1-rc6 release network block macaddr value converts from upper to lower case

[maksimsamt]

Network Block

Seems it is related with:
#1139

Adding resources:

      + network {
          + bridge    = "vmbr0"
          + firewall  = false
          + id        = 0
          + link_down = false
          + macaddr   = "AC:15:11:99:D5:11"
          + model     = "virtio"
          + tag       = 1111
        }

Removing resources:

      - network {
          - bridge    = "vmbr0" -> null
          - firewall  = false -> null
          - id        = 0 -> null
          - link_down = false -> null
          - macaddr   = "ac:15:11:99:d5:11" -> null
          - model     = "virtio" -> null
          - mtu       = 0 -> null
          - queues    = 0 -> null
          - rate      = 0 -> null
          - tag       = 1111 -> null
        }

I have some macaddr manipulations after VM provisioning (with provisioner "local-exec") and due to this change this action fails:

│ Error: Provider produced inconsistent final plan
│ 
│ When expanding the plan for terraform_data.* to include new values learned so far during apply, provider "terraform.io/builtin/terraform" produced an invalid new value for .triggers_replace[0]: was
│ cty.StringVal("AC:15:11:99:D5:11"), but now cty.StringVal("ac:15:11:99:d5:11").
│ 
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.

[Tinyblargon]

@maksimsamt this was done on purpose to enforce the same case pve uses when it generates a mac address from the ui. As switching the case of the mac address counts as disconnecting and reconnecting the network interface.

[Tinyblargon]

Also, all mac address formats are valid, even octal as we use the binary representation of the mac address.

Would using lower-case mac addresses in your be a solution for you?

Personally prefer upper-case mac address, so could make it default to uppercase.

[maksimsamt]

@Tinyblargon,

Would using lower-case mac addresses in your be a solution for you?

Yes, already changed in my solution logic to use lower case values.

Personally prefer upper-case mac address, so could make it default to uppercase.

The same to me, upper-case mac address is more readable.

[Tinyblargon]

@maksimsamt gonna try to figure out if i can preserve the format when returning it.

I'll add it to RC7.

[github-actions]

This issue is stale because it has been open for 60 days with no activity. Please update the provider to the latest version and, in the issue persist, provide full configuration and debug logs

[Tinyblargon]

@maksimsamt would you be able to test #1244? It should preserve the case/syntax of the mac-address.

[maksimsamt]

@maksimsamt would you be able to test #1244? It should preserve the case/syntax of the mac-address.

@Tinyblargon ,
Unfortunately with new changes (newer than rc6) I can't test, there is permission issue:

Error: 403 Permission check failed

In rc6 this workaround worked:
#784 (comment)

Modify minimumPermissions as per below example (keep only Sys.Audit permission)...

This version is no longer working with this workaround:
https://github.com/Tinyblargon/terraform-provider-proxmox/tree/1182

[Tinyblargon]

@maksimsamt not sure what is causing the permission issue as we haven't added any functionality that requires the interaction with PVE to change.

[Tinyblargon]

@maksimsamt could you check if the permission issue already exists in 7e66709 as this would really narrow down the search.

[maksimsamt]

@maksimsamt could you check if the permission issue already exists in 7e66709 as this would really narrow down the search.

for what? when 3.0.1-rc6 works fine

[Tinyblargon]

@maksimsamt I'm trying to figure out why your workaround doesn't work anymore. The commit i mentioned adds a lot of code that functionality doesn't change anything. It would help a lot in trying to figure out if your workaround broke before or after that commit.

[maksimsamt]

@Tinyblargon,
Sorry, I thought this commit 7e66709 was before rc6.
I just checked and this commit doesn't have this permission issue.

[Tinyblargon]

@maksimsamt thanks for testing. :)

[maksimsamt]

@Tinyblargon ,
This latest commit 47d9e55 introduces this issue.

This previous 364b3ba works fine.

[Tinyblargon]

@maksimsamt the only meaningful change is on line 788 in the qemu code. We used to set pool through the vmref, but now through the vm config. As far as I'm aware should that do the exact same. Gonna check upstream what is causing this.

[maksimsamt]

@Tinyblargon ,
So, did test for:

364b3ba
+
#1244

The test was successful.
Now the macaddr value remains unchanged.

[Tinyblargon]

@maksimsamt, thank you for verifying and all the help with the permission issue. I know what to revert, just nor sure yet why.