Various interpolation functions for CIDR range manipulation.

[apparentlymart]

These new functions allow Terraform to be used for network address space planning tasks, and make it easier to produce reusable modules that contain or depend on network infrastructure.

For example:

• cidrsubnet allows an aws_subnet to derive its CIDR prefix from its parent aws_vpc.
• cidrhost allows a fixed IP address for a resource to be assigned within an address range defined elsewhere, for when dynamic allocation is not desired. Computing a host address allows reusable modules to derive addresses from input variables.
• cidrfirstaddr and cidrlastaddr respectively provide the IP addresses that bound the range, which is what many DHCP servers accept as configuration of a dynamic allocation range.
• cidrmask provides the dotted-decimal form of a prefix length that is accepted by some systems such as routers and OS-level static network interface configuration files. (e.g. 255.255.0.0)

The bulk of the work here is done by an external library I authored called go-cidr. It is MIT licensed and was implemented primarily for the purpose of using it within Terraform. It has its own unit tests and so the unit tests within this change focus on simple success cases and on the correct handling of the various error cases.

• Implementation
• Tests (unit tests run as part of make test)
• Documentation

[apparentlymart]

The Travis-CI tests are failing on go vet with the following output:

builtin/providers/google/resource_compute_instance_template_test.go:139: arg item.Value for printf verb %s of wrong type: *string
builtin/providers/google/resource_compute_instance_test.go:339: arg item.Value for printf verb %s of wrong type: *string

I don't believe that's related to my change but if I'm mistaken please let me know and I'll fix it!

[catsby]

Thanks @apparentlymart – the Travis CI failures are indeed unrelated; I've patched them up in #3130

[apparentlymart]

I rebased this on top of the fix in #3130, so the tests are now passing.

[radeksimko]

👍 for the idea! This will make the DSL even more readable and clear.

[apparentlymart]

@phinze maintaining this changeset in a mergable state is quite bothersome for its size, because gofmt wants the map of names to functions to be all lined up and so this goes into conflict each time we add a new interpolation function.

Could I get a quick review on this so I don't have to keep manually rebasing it every few weeks? 😀

[phinze]

@apparentlymart +1 cutting a small v0.6.5 right now so i'd like to keep master quiet for a bit, will add this to my list for today once that is out

[apparentlymart]

Oh okay cool. Wish I'd known that before I just merged a bunch of stuff 😀, but I'll stop.

[phinze]

The behavior of this doesn't match what I'm used to seeing in CIDR manipulation tooling. Here's the output of ipcalc on that example CIDR:

❯ ipcalc 10.0.0.0/8
Address:   10.0.0.0             00001010. 00000000.00000000.00000000
Netmask:   255.0.0.0 = 8        11111111. 00000000.00000000.00000000
Wildcard:  0.255.255.255        00000000. 11111111.11111111.11111111
=>
Network:   10.0.0.0/8           00001010. 00000000.00000000.00000000
HostMin:   10.0.0.1             00001010. 00000000.00000000.00000001
HostMax:   10.255.255.254       00001010. 11111111.11111111.11111110
Broadcast: 10.255.255.255       00001010. 11111111.11111111.11111111

I'd expect cidrfirstaddr to match HostMin there, but it does not. Is there a use case I'm missing?

[phinze]

Oh okay cool. Wish I'd known that before I just merged a bunch of stuff 😀, but I'll stop.

No worries! I should have broadcast that I was starting the release wheels beforehand.

Initial review Qs inline. 👌

[apparentlymart]

I've never used ipcalc before so I wasn't aware of how it represented things. That's a useful example to copy, though.

For firstaddr and lastaddr I think the subtlety here is that my naming is talking about "first address" and "last address" while ipcalc is talking about "first host address" and "last host address", with the network and broadcast addresses being dealt with separately. I agree that it makes more sense that way, since network/broadcast addresses are generally used for different purposes so it makes sense for them to be first-class in this model.

I'll write up a new version of this that makes the same distinctions ipcalc does and uses similar terminology:

• cidrhost as before
• cidrhostmin and cidrhostmax, as in ipcalc, skipping the network and broadcast addresses
• cidrhostcount returning the number of hosts in the prefix, excluding the network/broadcast addresses
• cidrnetwork doing what cidrfirstaddr used to do
• cidrbroadcast doing what cidrlastaddr used to do
• cidrmask renamed to cidrnetmask for ipcalc-consistency
• cidrsubnet unchanged

[apparentlymart]

Having looked at this a bit more deeply, I think there is more complexity here than I first realized. IPv6 doesn't have the concept of broadcast addresses since it lacks broadcast, and so I don't want to run into this and implicitly impose IPv4-oriented concepts onto IPv6.

So instead I'm planning to strip this down a bit to the core stuff that definitely won't differ between IPv4 and IPv6:

• cidrhost
• cidrnetmask
• cidrsubnet

...and then I'll let the other use-cases soak for a while.

[apparentlymart]

@phinze as noted above, I reduced this down to just three of the functions to start with since I think these are the core things needed to do basic address calculations and I want to research a little more how the other functions should behave for IPv6 addresses.

If you're cool with it I'd like to merge in these three to start with and then follow up with the others later once I have a bit more understanding of the usual conventions for IPv6 addressing schemes. (Sadly there's no IPv6 in my world right now because we're on AWS and there's no IPv6 there...)

[phinze]

@apparentlymart Sounds good to me! These look good. 👍

[thegedge]

I think you want to get rid of the , 2 in your example here.

[apparentlymart]

You're right. Thanks for catching that!

[apparentlymart]

Thanks! I corrected the doc bug that @thegedge spotted, along with another one I spotted while fixing it (the function was still documented as cidrmask rather than cidrnetmask)... and then I merged it.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.