Skip to content

Conversation

MaienM
Copy link
Contributor

@MaienM MaienM commented Feb 8, 2023

When going from a nonempty list of port overrides to an empty list omitting this list results in an error, while including the list as an empty list works as expected. (I ran into this issue while using the Terraform provider.)

@joshuaspence
Copy link
Collaborator

Could you provide some more context here? Is the provider producing an error, showing the incorrect diff or something else?

@MaienM
Copy link
Contributor Author

MaienM commented Feb 23, 2023

The PUT request to update the device doesn't actually error out, but it returns empty data and the change is not applied. This results in the Terraform provider giving the message Error: not found.

This is on UDMP firmware 2.4.27, though I believe that when I originally made this change I was still on the 1.x branch

Request/response without this change:

{
  "_id": "123456789012345678901234",
  "site_id": "default",
  "mac": "11:22:33:44:55:66",
  "adopted": false,
  "config_network": {},
  "name": "Dream Machine Pro",
  "rps_override": {},
  "state": 0
}
{
  "meta": {
    "rc": "ok",
    "msg": ""
  },
  "data": []
}

And with this change:

{
  "_id": "123456789012345678901234",
  "site_id": "default",
  "mac": "11:22:33:44:55:66",
  "adopted": false,
  "config_network": {},
  "name": "Dream Machine Pro",
  "port_overrides": [],
  "rps_override": {},
  "state": 0
}
{
  "meta": {
    "rc": "ok",
    "msg": ""
  },
  "data": [
    {
      "_id": "123456789012345678901234",
      "site_id": "123456789012345678901234",
      "mac": "11:22:33:44:55:66",
      "adopted": true,
      "config_network": {},
      "ethernet_overrides": [
        {
          "ifname": "eth8",
          "networkgroup": "WAN"
        },
        {
          "ifname": "eth9",
          "networkgroup": "WAN2"
        },
        {
          "ifname": "eth0",
          "networkgroup": "LAN"
        },
        {
          "ifname": "eth1",
          "networkgroup": "LAN"
        },
        {
          "ifname": "eth2",
          "networkgroup": "LAN"
        },
        {
          "ifname": "eth3",
          "networkgroup": "LAN"
        },
        {
          "ifname": "eth4",
          "networkgroup": "LAN"
        },
        {
          "ifname": "eth5",
          "networkgroup": "LAN"
        },
        {
          "ifname": "eth6",
          "networkgroup": "LAN"
        },
        {
          "ifname": "eth7",
          "networkgroup": "LAN"
        },
        {
          "ifname": "eth10",
          "networkgroup": "LAN"
        }
      ],
      "hostname": "Dream-Machine-Pro",
      "lcm_brightness": 80,
      "lcm_night_mode_begins": "22:00",
      "lcm_night_mode_enabled": true,
      "lcm_night_mode_ends": "08:00",
      "name": "Dream Machine Pro",
      "port_overrides": [],
      "rps_override": {},
      "state": 0,
      "stp_priority": "32768",
      "stp_version": "rstp"
    }
  ]
}

@joshuaspence
Copy link
Collaborator

Do you think you would be able to add a test case to https://github.com/paultyng/terraform-provider-unifi that demonstrates the issue and which is fixed by this PR? If you can't, I can try to do it. I've been improving the acceptance tests over the past few days, they should be in a better state now.

FYI the UDMP firmware version is irrelevant, only the Unifi Network version matters for this repo

@MaienM
Copy link
Contributor Author

MaienM commented Feb 27, 2023

Do you think you would be able to add a test case to paultyng/terraform-provider-unifi that demonstrates the issue and which is fixed by this PR?

Done. That was much easier than I had expected it to be.

FYI the UDMP firmware version is irrelevant, only the Unifi Network version matters for this repo

Right, duh. Looking at the results of the test it doesn't seem to matter much since all tested versions fail it (though the error changes between 7.1 and 7.2).

@joshuaspence
Copy link
Collaborator

Thanks for adding the acceptance test, it made it much easier to review this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants