paultyng / go-unifi

Unifi Controller API SDK for Go
Mozilla Public License 2.0
168 stars 50 forks source link

Supporting 6.X.X versions of Unifi Network Controller #13

Closed kurtmc closed 3 years ago

kurtmc commented 4 years ago

I am currently looking into updating https://github.com/paultyng/terraform-provider-unifi to support all the features of Unifi Network that I require, and I am also using Unifi Network 6.0.23 and I have noticed that the API is different to the 5.X.X versions.

I am keen to add support for 6.X.X to this library but I am unsure what the best strategy for that would be. Should I add some sort of version field to the client and use that to decide how to interact with the API?

The different versions of the controller have different fields for the API calls, so it might become a bit of a mess to add two definitions of some of the models depending on the version? E.g. having type WLAN_v5 struct and type WLAN_v6 struct

Please let me know how you would like to see it done

paultyng commented 4 years ago

I'm not sure honestly. I think for best usage in the Terraform provider, a single API client just handles all the potential upstream weirdness, but that obviously makes the SDK implementation a lot harder. On the other hand though it would also be a lot of maintenance though to maintain a 5.x SDK and Terraform Provider and a separate 6.x SDK and Terraform Provider.

Depending on the differences maybe we could just expose a single WLAN struct, which maybe represents v6, and then internally we downgrade it to a v5 struct if necessary?

paultyng commented 4 years ago

I've been poking around v6, looks like it will kind of be a pain with some of the breaking changes (network ids vs vlans, ap groups, etc). It may be worth just going with a different version, I'm not sure. In an ideal world maybe the SDK or provider can support both, but just not sure how much work we are talking about you know.

The code generation also doesn't work for the v2 API paths, so will have to code those manually for now I guess.

kurtmc commented 4 years ago

I am keen to help out. Perhaps a good initial step would be to setup tests like you have in terraform-provider-unifi and we could have a suite of tests that run against v5 and v6 so we can ensure we all the changes stay backwards compatible?

paultyng commented 4 years ago

Yeah I switched this to use a v5 and v6 tag: https://github.com/paultyng/terraform-provider-unifi/pull/73

paultyng commented 4 years ago

We may have to add some tests that only work in each version due to breaking API changes, I wonder if you can get the controller version from the API

kurtmc commented 4 years ago

I think you should be able to. Playing around in the network tab in the browser I found this: https://localhost:8443/api/s/default/stat/sysinfo

{
    "meta": {
        "rc": "ok"
    },
    "data": [
        {
            "timezone": "America/New_York",
            "autobackup": false,
            "build": "atag_6.0.23_14253",
            "version": "6.0.23",
            "previous_version": "5.12.35",
            "debug_mgmt": "warn",
            "debug_system": "warn",
            "debug_device": "warn",
            "debug_sdn": "warn",
            "data_retention_days": 90,
            "data_retention_time_in_hours_for_5minutes_scale": 24,
            "data_retention_time_in_hours_for_hourly_scale": 720,
            "data_retention_time_in_hours_for_daily_scale": 2160,
            "data_retention_time_in_hours_for_monthly_scale": 8760,
            "data_retention_time_in_hours_for_others": 2160,
            "update_available": false,
            "update_downloaded": false,
            "live_chat": "super-only",
            "store_enabled": "super-only",
            "hostname": "example-domain.ui.com",
            "name": "TF Acceptance Test Controller",
            "ip_addrs": [
                "172.17.0.2"
            ],
            "inform_port": 8080,
            "https_port": 8443,
            "override_inform_host": false,
            "image_maps_use_google_engine": false,
            "radius_disconnect_running": false,
            "facebook_wifi_registered": false,
            "sso_app_id": "90waTUDb0XgRdfyYlA7sM5xEGS90S0N3zyFbyo8v",
            "sso_app_sec": "8IVW8RyPBsqYLsPX9LPMmWMQxHU8E6wV8cX6C6iIlRnLT4bvuuTdLqchzvsq24TnR3nMW37mrNaOT0Z6SEapGoAExfu9hTD7MZHPx8Mew0xWK7ozecSJm5VSYBWOPX9f",
            "uptime": 148,
            "unsupported_device_count": 0,
            "unsupported_device_list": [],
            "unifi_go_enabled": false,
            "default_site_device_auth_password_alert": false
        }
    ]
}
paultyng commented 4 years ago

This is what Unifi poller was hitting: https://localhost:8443/status

Should give it to us without needing a site ID.

{
    "meta": {
        "rc": "ok",
        "up": true,
        "server_version": "6.0.23",
        "uuid": "37049884-1c2d-4003-8bf6-2a0cf9e454d7"
    },
    "data": []
}
paultyng commented 4 years ago

Added some code to this module and the provider in https://github.com/paultyng/terraform-provider-unifi/pull/73 that is passing for both, let me know your thoughts.

paultyng commented 3 years ago

Fixed in abc676f62b5a663ffa87c2502b598c801682f123