-
Notifications
You must be signed in to change notification settings - Fork 25
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
Apply gofmt -s on the files #14
Open
bltavares
wants to merge
8
commits into
cormacrelf:master
Choose a base branch
from
bltavares:lint-cleanup
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Since Go 1.11 there is a new way to structure packages, which don't require the project to live under `$GOPATH`, and provides version management. `go mod` could superseed `dep`, given it is builtin on the core tooling of the language and requires less dependencies to contribute. This commit follows the [migration instructions] to adopt `go mod`. The `README` has been updated, as well as `Makefile` and Travis configuration. The `main.go` has been moved into the root directory, so we are able to `go build` and produce the `terraform-provider-zerotier` binary, simplifying a lot the development. [migration instructions]: https://github.com/golang/go/wiki/Modules#migrating-to-modules
Terraform [is able to import](https://www.terraform.io/docs/import/index.html) existing infrastructure. This allows you take resources you've created by some other means and bring it under Terraform management. This commit implements the `Importer` attribute for each resource, as documented by Terrafom ImporterPassthrough. Usage is documented on `README` as well. Closes cormacrelf#6 Built on top of: cormacrelf#7
Zerotier provides a central controller on https://my.zerotier.com and it is possible to run our own DIY controller if we desire as well. Currently, this provider uses a hardcoded string pointing to the central controller, which don't allow changes by downstream users. This changes allow overriding the controller url on the provider itself, with default falling back over to the central controller, including documentation changes on `README`. Built on top of cormacrelf#8
ZeroTier has a couple of settings related to IPv6 address assignment options which distribute IPv6 addresses computed based on the network id and the node id. RFC4139 assigns an single IPv6 address for each of the nodes. 6PLANE assigns a whole /80 prefix for each node, which could be redistributed by the member, such as a router or used for Docker containers. Both of them are deterministic values calculated based on the network id and node id, and they are not returned on the response of the controller, given that the client is capable of calculating it itself. IPv6 Assignment distribute IPv6 from the the assignment pool for each member. If there is no IPv6 assignment pool configured, no route will be distributed. It is important to also include the route configuration for that network, so there is traffic through ZeroTier. This commit exposes this information on the member resource, as a computed property, so we could reference this information on other Terraform resources (such as DNS settings or provisioner scripts). Given this is a calculated property that is always present, downstream modules should check if the network has it configured before using it. No errors will be thrown, it would only not route properly if the network has not enabled it. The commit also exposes the settings in the network to enable each of the IPv6 address distribution toggles on the network resource. There is also a bug fix, where the IPv4 configuration toggle was hardcoded and not reading the value from the resource definition.
The network and member resource use a couple of settings which could be defined multiple times. For example: the assigned ips on a member could change the order, while the semantic means the same. The provider was using a `schema.ListType`, which according to the documentation is an *ordered* list of elements, where the order matter. On those resource seetings, the order is not relevant, as they are sematically equivalent. There is a `schema.SetType` which provides an *unordered* collection of settings, and grants the same order given the same settings. This commit changes the schema of those resources to use `schema.SetType` instead of `schema.ListType` to avoid diff loops depending on the sorting of the response from the API.
There was a few settings on the network that are exposed on the ZeroTier Central UI which were not available on the provider. This commit includes those extra fields, where one of them are not documented but available on the UI.
Currently, the `ip_assignments` has a list of the IPs that ZeroTier provided to the member from its assignment pool. It contains a mix of IPv4 and IPv6 addresses. There is some scenarios where having the distiction of wheter it is an IPv4 or IPv6 changes which resource to create. For example using DNS records we are only allowed to create `AAAA` records in the presence of IPv6 addresses, and `A` records to IPv4. Filtering this information on Terraform is cumbersome, using the filter on list operations, while it is much easier to provide this information through the provider. This commit create two extra computed properties, `ipv6_assignments` and `ipv4_assignments`, which separates each address assignment as expected. The `ipv6_assignments` does not include RFC4139 nor 6PLANE addresses as they are always computed on the `member` resource level, even if the `network` is configured to not use those addresses, and their information is not returned by the controller API as an assigned address either.
This commit applies `gofmt -s` which changes the code with a simplified version if the formatter finds it is safe to apply. The `make dev` command will apply the simplfied rewriting automatically as well. An extra change added was to move the `assignment_pool` resource schema definition to a function, similar to how the `route` schema was defined, to keep the consistency on the file.
I will not be able to properly rebase this one until the others have landed, as this PR cleanup things created on the other PRs. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit applies
gofmt -s
which changes the code with a simplifiedversion if the formatter finds it is safe to apply.
The
make dev
command will apply the simplfied rewriting automaticallyas well.
An extra change added was to move the
assignment_pool
resource schemadefinition to a function, similar to how the
route
schema was defined,to keep the consistency on the file.
Built on top of #13 (I could rebase when it lands)
Diff with only the import changes