The future path for the Unifi provider

[evenh]

Hi all 👋🏻

I've tested this provider through the last few years and got it 80% working every time. As a irregular contributor to various open source projects myself I know that maintenance of and interest in these projects can vary when jobs, roles or life itself changes. With that in mind I reached out to @paultyng via email this summer and got a reply that drowned in my mailbox until I found it today (I'm really sorry). The reply was this:

I started the provider when I worked on providers at HashiCorp, I no longer do that, I'm not even in the DevOps space anymore and haven't been for the past 3 years or so. […]. I do use the provider occasionally on my home network, but I haven't had to change anything in some while, so its grown pretty stale as you see.

I'd be happy to have some one take it over, the issue for me is mostly that its under my name, and without a lot of vetting, its hard to bring folks in I don't really know, so I think a fork is possibly the best option, for the SDK as well. I'm happy to archive it and provide a pointer to a new fork assuming it has forward momentum but I don't really have the time to interview new maintainers on my repo.

I thought I'd share it with the community in case somebody wants to step up and maintain a fork. At least this is a starting point for a discussion. I found a few recent forks which may or may not be candidates going forward:

• https://github.com/ubiquiti-community/terraform-provider-unifi (@appkins)
• https://github.com/sayedh/terraform-provider-unifi (@sayedh)

Related issues:

• Maintained? go-unifi#213
• Multiple network resource and data source improvements #454

cc @joshuaspence

[joshuaspence]

Thanks for the write up. I have been meaning to get better at maintaining this project but have young kids at home, which has made it difficult.

I am happy to bring in additional maintainers to the project. With some additional maintainers I thought no it would help to renew my interest in the project

The last time I worked on this project I was trying to think about how to handle a change in the underlying API in which Unifi removed / significantly changed functionality in a minor version and I wasn't sure how to deal with that. I can't remember if that is still an issue

[elacy]

Only registered provider other than this one is https://registry.terraform.io/providers/akerl/unifi/latest
not sure if @akerl is still maintaining theirs

[akerl]

I'm not not maintaining it 😄

When I forked, my original goal was to apply my patch for #389 , so that I could have a public statefile w/o leaking passphrases.

From there, I've been pulling in updates and patches (some of mine, some from other folks opening PRs on this repo) to keep it working as Unifi changes their services.

I maybe run the provider once every couple of months, when I add some new DHCP reservations to my network or decom some old services, so I'm not actively using it on a daily basis, but I do have interest in keeping it working.

[sayedh]

Hi everyone,

Thank you @evenh for sharing Paul's message and opening the discussion.

I've forked the original terraform-provider-unifi and created an updated version:

• Terraform Provider: sayedh/unifi
• GitHub Repository

This fork supports UniFi Controller v8.4.59. Currently, the following resources are fully tested and confirmed working:

• Resource: Network
• Resource: Port Profile

I'm actively working to expand the feature set and improve stability. For more information, please refer to the README and the Terraform documentation.

I should note that this provider is under active development and should be considered experimental at this stage. While some features are thoroughly tested, others are still a work in progress. Testing and feedback are highly appreciated.

I’d also like to acknowledge the efforts of those who have been working on forks of this provider, such as @appkins, @akerl and others. It would be fantastic if we could coordinate our efforts to create a unified, actively maintained version.

I've also forked and updated the go-unifi SDK to support UniFi Controller v8.4.59.

If Paul agrees, it would be great if the original repository could point to this fork as the actively maintained version for those looking for continued development and compatibility with newer UniFi versions.

Thanks again for sharing this and for everyone's interest in keeping this project moving forward. I'm looking forward to any contributions, feedback, or discussions from the community.

Best,
Sayed (@sayedh)

[joshuaspence]

Thanks @sayedh for your input. I am a co-maintainer of this project although haven't been able to spend any time of it lately for personal reasons. Having said that, I am experiencing a renewed interest for this project and would like to continue development. I would rather continue working on this project than point to a fork but, of course, this is open source and the community can choose to use this repository or any of the forks.

I will work to close off the open PRs this week

[DevOpsFu]

I noticed my PR is mentioned here. I'm happy to help with any clean up needed to get the changes merged in. As it happens I've been revisiting this stuff today because I need to automate some configuration on my USG.

[joshuaspence]

I am currently working through the PR backlog. One thing that would help us making sure your PRs have acceptance test coverage. There are a few flakey acceptance tests that I need to chase down but mostly the acceptance tests are in a good place.

The acceptance tests work by spinning up a demo install of Unifi Network. The demo install has a bunch of demo devices that can be used for testing devices, everything else can be created/modified as needed within the tests.

I'll try to close some of the PRs off without tests (or add the tests myself) but I'd like to maintain a high level of test coverage for this provider as there are a lot of edge cases in using an unofficial, undocumented API

[britthouser]

@joshuaspence - Thanks for taking this up again and working through the backlog. I'd like to pitch in somehow. I've been using terraform at work for a couple years, and just recently got some ubiquiti gear. I have programming background, but haven't learned go yet. I suspect there are several people here with similar skill sets. Is there something I can do to help out?

[joshuaspence]

@britthouser The main thing that needs to be done to make it easier to contribute to this repository is to fix the flakey acceptance tests, which I would definitely welcome contributions on. Once this is done it will be significantly easier to review PRs with confidence that we won't break existing functionality.

Historically, running the acceptance tests was tricky as many of the UniFi features require physical hardware to configure. I fixed this in #150, #155, #185 and #188 by utilising an undocumented demo mode, which creates a bunch of fake UniFi devices which behave like real devices. So as of today we should be able to test most of the functionality provided by the provider. There may be some gaps because I don't think the demo mode creates all of the available device types, but I suspect 90-95% of functionality can and should have acceptance test coverage. Note that since this demo mode is undocumented it is possible that UniFi removes it in the future, which will complicate things significantly.

Basically, in demo mode we have 20 fake UniFi devices (this is configurable). When an acceptance test needs to use a device it calls allocateDevice, which allocates a device from the devicePool. In the future we will likely want to be able to allocate different device types, currently we only do this for switches. There is some additional logic to ignore certain devices for various reasons (see #316 for one such examples).

Test flakiness is sometimes caused when multiple tests attempt to use the same name for a resource. The easiest fix for this is to randomise the resource names, see #156 for one such examples.

Another common failure I have seen but haven't had a chance to investigate is this:

=== NAME  TestAccSite_basic
    testing_new.go:88: Error running post-test destroy, there may be dangling resources: site not destroyed

To aid with debugging, the Docker containers will dump out the UniFi logs after test execution (see #335). In theory this should aid with debugging although in practice it generates a lot of noise. It might be reasonable to log to a file instead and/or tweaking log levels.

There is also some logic for allocating MAC addresses and VLANs. This logic is not as robust as the device allocation code and will likely need work at some stage, but I don't think it is causing issues right now. Specifically the getTestVlan function doesn't have a way to unallocate VLANs so it can only be used ~4096 times.

[svengreb]

@joshuaspence Thanks too for picking the project up!

I'd also like to help to get this provider up-to-date and in a status where every functionality is supported.
One, at least in my opinion, important point is that the provider should be migrated from the Terraform SDK v2 to the "new" Plugin Framework. This will eliminate a lot of bugs and problems that might only be caused by the way ho the SDK works. The plugin framework is really a large step forward for stability, maintainability and helps a lot to get things done without the fear of breaking something else. I've developed some features and improvements for the bpg/proxmox provider and it is definitely easier to use. The bpg/proxmox provider also runs in a "mixed" mode where the previous SDK-based implementations are used together with implementations that use the plugin framework so one provider can smoothly migrate without breaking users states or without the need to migrate everything all at once.
My guess is that it would also be way easier to get new contributors on board since it is also easier to understand even when someone does not have a lot of Go experience but other programming languages.

[joshuaspence]

Thanks, I agree that we should migrate to the new plugin framework. I think that the acceptance tests should be fixed first though to give a greater confidence in the migration.

[joshuaspence]

Another consideration here is whether we should continue supporting all of the Unifi versions that are currently supported. I think it's probably reasonable to drop support for 6.x and 7.x now

[joshuaspence]

The bpg/proxmox provider also runs in a "mixed" mode where the previous SDK-based implementations are used together with implementations that use the plugin framework so one provider can smoothly migrate without breaking users states or without the need to migrate everything all at once.

Ah neat, I wasn't aware that was possible

[filipowm]

Hi @joshuaspence , great work so far! I hope to have one provider and to bring this repo to life.

I tried to track down problems with acceptance tests you mentioned, and there seems to be few things at first glance:

1. Most common one I saw appearing over and over is issue with TestAccSite_basic which ends up with

   testing_new.go:88: Error running post-test destroy, there may be dangling resources: site not destroyed"
   

   So far I only found this reference in terraform-plugin-sdk: ExpectError is ignored for test case where destroy action expected to have error hashicorp/terraform-plugin-sdk#609 .

2. The other common one is with TestAccAccount_mac:

   resource_account_test.go:29: Step 1/2 error: Error running apply: exit status 1
     
     Error: api.err.DuplicateAccountName (400 ) for POST https://localhost:8443/api/s/default/rest/account
        
       with unifi_account.test,
       on terraform_plugin_test.tf line 2, in resource "unifi_account" "test":
        2: resource "unifi_account" "test" {
   

   I didn't yet trace down the root cause, but seems to be also an issue with destroying resources.

3. A lot of resources report invalid key exists error, but it does not fail test, while in my opinion it should when running test for specific UniFi controller version. I think this comes also with a topic of backward compatibility of the provider with older versions of controller. My propose is in long run to just support latest major version of controller, and temporarily use deprecation messages. Then in newer versions we could just wipe them out.

To deal with #1 and #2, I'd recommend to either temporarily disable those or mark as allowed to fail until they are fixed. This would allow for bringing parity with other providers quickly, while solving test flakiness just after that.

I can spend some time to move over changes from other providers mentioned here, to have parity with those. I would additionally work on deprecations as I mentioned.

If you're fine I can drop few PRs in upcoming weeks with those.

[joshuaspence]

To deal with #1 and #2, I'd recommend to either temporarily disable those or mark as allowed to fail until they are fixed. This would allow for bringing parity with other providers quickly, while solving test flakiness just after that.

Yeah, agree.

A lot of resources report invalid key exists error, but it does not fail test, while in my opinion it should when running test for specific UniFi controller version.

These issues should be minor and shouldn't fail the tests. If you find any real issues then we can address those but I found some in #338 which seems to be because some fields can be read from the API but not written (e.g. device state). Should be easy to fix in go-unifi.

My propose is in long run to just support latest major version of controller, and temporarily use deprecation messages. Then in newer versions we could just wipe them out.

I'd like to maintain support for the latest major version. Possibly we should maintain support for the previous version as well, although only if it is easy to do so.

If you're fine I can drop few PRs in upcoming weeks with those

Yeah absolutely