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

Merging features with bltavares/terraform-provider-zerotier #17

Open
1 of 4 tasks
bltavares opened this issue Sep 1, 2021 · 4 comments
Open
1 of 4 tasks

Merging features with bltavares/terraform-provider-zerotier #17

bltavares opened this issue Sep 1, 2021 · 4 comments

Comments

@bltavares
Copy link

bltavares commented Sep 1, 2021

Hello there folks,

I'm really happy to see updates on zerotier. I've been using and maintaining my own version of the terraform provider for about 2-3 years already and I would like to merge some features so I can deprecate my version.

Here are the list of missing features:

Those were missing features required for me to migrate my current network definition to Terraform, and better integrate with other providers. Having separate computed properties on network and member for 6plane and RFC4193 attributes allows easier integration with a DNS provider or with file outputs for scripting.

I won't have time the following weeks to make this change as PRs, but all suggested commits should be easily ported to new versions but I can take a look at it on the next month.

@someara
Copy link
Contributor

someara commented Sep 2, 2021

Hi Bruno!

Thanks for this.
I'll get these to you soon!

-s

@erikh
Copy link
Contributor

erikh commented Sep 2, 2021

Hi, I wrote most of the code here. I'm fine with these suggestions myself.

I'm a little hesitant on computed addresses as I was playing with some stuff around CIDR computation (if it's not still in tree, it's in the log) for these situations that I gutted because it wasn't going so hot. :) Mostly a lack of understanding on my part about the state lifecycles in terraform, would mostly just like to make sure a new implementation doesn't re-introduce that. The CIDR code was fine however and you may wish to use it, it's fairly efficient.

anyways I'll try to dip a toe in and help. I am working on a few other projects that require a lot of attention so I may not be very fast. Feel free to reach me @ my github profile's email if you'd prefer to coordinate off github.

@bltavares
Copy link
Author

Hey @erikh, I'm glad the suggestions are well received. No worries on taking a look at this, as neither do I have time to work on it also on the short term.

I mostly wanted to share it early so the differences are mapped and we can work together to unify the features.

Thanks for mentioning the issues you've found with computed properties. Would you be able to share more details of what happened? I've been using it for a while already, and as long as I used the schema.TypeSet I didn't have any issues with Terraform.

@erikh
Copy link
Contributor

erikh commented Sep 3, 2021

I'm trying to remember; it was almost certainly something I was doing wrong. I'll make a note to bisect it later; sean and I hit it during release prep and there was a version with it it in there.

laduke added a commit that referenced this issue Oct 14, 2022
re #17 and #29

I'm not sure how to add a test for `import`ing yet.
The cool tf harness doesn't support `import`.

The terraform file I used for testing.

```
terraform {
  required_providers {
    zerotier = {
      source  = "zerotier/zerotier"
    }
  }
}
resource "zerotier_member" "bob" {}
```

`terraform import zerotier_member.bob 8286ac0e4786b2aa-1122334455` The member needs to already exist

`terraform state show zerotier_member.bob`

implementation borrowed from bltavares  branch
someara pushed a commit that referenced this issue Oct 18, 2022
re #17 and #29

I'm not sure how to add a test for `import`ing yet.
The cool tf harness doesn't support `import`.

The terraform file I used for testing.

```
terraform {
  required_providers {
    zerotier = {
      source  = "zerotier/zerotier"
    }
  }
}
resource "zerotier_member" "bob" {}
```

`terraform import zerotier_member.bob 8286ac0e4786b2aa-1122334455` The member needs to already exist

`terraform state show zerotier_member.bob`

implementation borrowed from bltavares  branch
laduke added a commit that referenced this issue Oct 18, 2022
See issue #17

Turn Lists into Sets, so the order doesn't matter
laduke added a commit that referenced this issue Oct 18, 2022
See issue #17

Turn Lists into Sets, so the order doesn't matter
laduke added a commit that referenced this issue Oct 19, 2022
See issue #17

Turn Lists into Sets, so the order doesn't matter
laduke added a commit that referenced this issue Oct 19, 2022
See issue #17

Turn Lists into Sets, so the order doesn't matter
someara pushed a commit that referenced this issue Oct 25, 2022
* Add some notes re: tests, docker, and cleanup

* create quick unit test of toMember

added testify because can't find a non
annoying way to compare sets of
caps, tags, ipaddresses, ...

* avoid unnecessary diffs in members

See issue #17

Turn Lists into Sets, so the order doesn't matter

* Expose rfc4193 and sixplane computed properties

* Expose ipv4/6 managed addresses in separate lists

They are a pain to separate in terraform.
Example use-case: make AAAA or A records for a node.

* Bump version
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

No branches or pull requests

3 participants