Poor validation of pool members / idempotency

sbrinkerhoff commented 7 years ago

When attempting to add nodes to a bigip_ltm_pool there appears to be limited validation and error checking which results in incorrect output, and subsequent runs attempt to correct and silently fail.

This code will suggest that it added two nodes..

Invalid Names

No errors are thrown for either node below, the first one is invalid for no domain, neither are real nodes. Additionally they are missing service nodes which makes them ineligible to be added.

variable "pool_name" { default="test-pool" }
variable "pool_nodes" { type="list" default=["no-domain","/Common/not-a-real-node"]}
variable "allow_snat" { default = true }
variable "allow_nat" { default = true }
variable "monitors" { type = "list", default = []}

resource "bigip_ltm_pool" "pool" {
  name = "${var.pool_name}"
  nodes = "${var.pool_nodes}"
  allow_snat = "${var.allow_snat}"
  allow_nat = "${var.allow_nat}"
  monitors = "${var.monitors}"
}

  nodes.#:             "1" => "2"
  nodes.2161144047:    "" => "no-domain"
  nodes.596047430:     "" => "/Common/not-a-real-node"

Service Port Missing

Nodes can be defined without a service port, and the bigip_ltm_pool provider does not return a failure in adding them, similar to the error above.

Idempotency

Even when valid node names are used, the ID seems to change each run and causes Terraform to state that the nodes were updated (and likely were disruptively added/removed from the pool.

Note the F5 seemingly is returning node names without the domain below, which Terraform seemingly removes and replaces with the names containing a domain prefix.

  monitors.2541227442: "" => "http"
  monitors.576276785:  "/Common/http" => ""
  nodes.1615781422:    "dc4plat-xxxxxxx:80" => ""
  nodes.2161144047:    "" => "/Common/dc4plat-xxxxxxx:80"
  nodes.3273152647:    "dc4plat-xxxxxxx:80" => ""
  nodes.596047430:     "" => "/Common/dc4plat-xxxxxxx:80"

jakauppila commented 7 years ago

@sbrinkerhoff What do you mean by the following?

first one is invalid for no domain

Just that it doesn't include the partition on it?

In terms of idempotency, I have a PR on the upstream library to address this, https://github.com/scottdware/go-bigip/pull/31. I'll get the required changes in here once it's accepted there.

mattcanty commented 7 years ago

@jakauppila do you have a binary available of the bigip provider with this idempotency fix? I've been struggling to build the solution.

DealerDotCom / terraform-provider-bigip

Poor validation of pool members / idempotency #42