From 981c40fec1023fb0fe8440b93299cf6985ef5a8f Mon Sep 17 00:00:00 2001 From: Glen Mailer Date: Sun, 6 Sep 2015 23:43:37 +0100 Subject: [PATCH] Expand cloudstack_loadbalancer_rule to work without vpcs When using load balancer rules on an IP associated with a network instead of a vpc, the network field can be omitted and inferred from the IP. Filling this into state on read causes a spurious diff. The openfirewall flag defaults to true when used on a network IP. Implicit resource creation doesn't fit the terraform model, so we disable it. Also added a test which shows arguments that can be changed without creating a new resource. --- .../resource_cloudstack_loadbalancer.go | 19 +- .../resource_cloudstack_loadbalancer_test.go | 239 +++++++++++++++++- 2 files changed, 249 insertions(+), 9 deletions(-) diff --git a/builtin/providers/cloudstack/resource_cloudstack_loadbalancer.go b/builtin/providers/cloudstack/resource_cloudstack_loadbalancer.go index f0f905f06..73bf1d493 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_loadbalancer.go +++ b/builtin/providers/cloudstack/resource_cloudstack_loadbalancer.go @@ -79,6 +79,9 @@ func resourceCloudStackLoadBalancerRuleCreate(d *schema.ResourceData, meta inter d.Get("public_port").(int), ) + // Don't autocreate a firewall rule, use a resource if needed + p.SetOpenfirewall(false) + // Set the description if description, ok := d.GetOk("description"); ok { p.SetDescription(description.(string)) @@ -160,14 +163,16 @@ func resourceCloudStackLoadBalancerRuleRead(d *schema.ResourceData, meta interfa d.Set("public_port", lb.Publicport) d.Set("private_port", lb.Privateport) - // Get the network details - network, _, err := cs.Network.GetNetworkByID(lb.Networkid) - if err != nil { - return err - } - setValueOrUUID(d, "ipaddress", lb.Publicip, lb.Publicipid) - setValueOrUUID(d, "network", network.Name, lb.Networkid) + + // Only set network if user specified it to avoid spurious diffs + if _, ok := d.GetOk("network"); ok { + network, _, err := cs.Network.GetNetworkByID(lb.Networkid) + if err != nil { + return err + } + setValueOrUUID(d, "network", network.Name, lb.Networkid) + } return nil } diff --git a/builtin/providers/cloudstack/resource_cloudstack_loadbalancer_test.go b/builtin/providers/cloudstack/resource_cloudstack_loadbalancer_test.go index 5b07649bd..26e832daf 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_loadbalancer_test.go +++ b/builtin/providers/cloudstack/resource_cloudstack_loadbalancer_test.go @@ -35,6 +35,64 @@ func TestAccCloudStackLoadBalancerRule_basic(t *testing.T) { } func TestAccCloudStackLoadBalancerRule_update(t *testing.T) { + id := "" + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackLoadBalancerRuleDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccCloudStackLoadBalancerRule_basic, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackLoadBalancerRuleExist("cloudstack_loadbalancer_rule.foo"), + func(s *terraform.State) error { + rs, ok := s.RootModule().Resources["cloudstack_loadbalancer_rule.foo"] + if !ok { + return fmt.Errorf("Not found: cloudstack_loadbalancer_rule.foo") + } + id = rs.Primary.ID + return nil + }, + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "name", "terraform-lb"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "algorithm", "roundrobin"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "public_port", "80"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "private_port", "80"), + ), + }, + + resource.TestStep{ + Config: testAccCloudStackLoadBalancerRule_update, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackLoadBalancerRuleExist("cloudstack_loadbalancer_rule.foo"), + func(s *terraform.State) error { + rs, ok := s.RootModule().Resources["cloudstack_loadbalancer_rule.foo"] + if !ok { + return fmt.Errorf("Not found: cloudstack_loadbalancer_rule.foo") + } + if id != rs.Primary.ID { + return fmt.Errorf("Resource has changed!") + } + return nil + }, + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "name", "terraform-lb-update"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "algorithm", "leastconn"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "public_port", "80"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "private_port", "80"), + ), + }, + }, + }) +} + +func TestAccCloudStackLoadBalancerRule_forcenew(t *testing.T) { resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -56,7 +114,70 @@ func TestAccCloudStackLoadBalancerRule_update(t *testing.T) { }, resource.TestStep{ - Config: testAccCloudStackLoadBalancerRule_update, + Config: testAccCloudStackLoadBalancerRule_forcenew, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackLoadBalancerRuleExist("cloudstack_loadbalancer_rule.foo"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "name", "terraform-lb-update"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "algorithm", "leastconn"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "public_port", "443"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "private_port", "443"), + ), + }, + }, + }) +} + +func TestAccCloudStackLoadBalancerRule_vpc(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackLoadBalancerRuleDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccCloudStackLoadBalancerRule_vpc, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackLoadBalancerRuleExist("cloudstack_loadbalancer_rule.foo"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "name", "terraform-lb"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "algorithm", "roundrobin"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "public_port", "80"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "private_port", "80"), + ), + }, + }, + }) +} + +func TestAccCloudStackLoadBalancerRule_vpc_update(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackLoadBalancerRuleDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccCloudStackLoadBalancerRule_vpc, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackLoadBalancerRuleExist("cloudstack_loadbalancer_rule.foo"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "name", "terraform-lb"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "algorithm", "roundrobin"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "public_port", "80"), + resource.TestCheckResourceAttr( + "cloudstack_loadbalancer_rule.foo", "private_port", "80"), + ), + }, + + resource.TestStep{ + Config: testAccCloudStackLoadBalancerRule_vpc_update, Check: resource.ComposeTestCheckFunc( testAccCheckCloudStackLoadBalancerRuleExist("cloudstack_loadbalancer_rule.foo"), resource.TestCheckResourceAttr( @@ -133,6 +254,120 @@ func testAccCheckCloudStackLoadBalancerRuleDestroy(s *terraform.State) error { } var testAccCloudStackLoadBalancerRule_basic = fmt.Sprintf(` +resource "cloudstack_instance" "foobar1" { + name = "terraform-server1" + display_name = "terraform" + service_offering= "%s" + network = "%s" + template = "%s" + zone = "%s" + expunge = true +} + +resource "cloudstack_loadbalancer_rule" "foo" { + name = "terraform-lb" + ipaddress = "%s" + # network omitted, inferred from IP + algorithm = "roundrobin" + public_port = 80 + private_port = 80 + members = ["${cloudstack_instance.foobar1.id}"] +} + +# attempt to create dependent firewall rule +# this will clash if cloudstack creates the implicit rule as it does by default +resource "cloudstack_firewall" "foo" { + ipaddress = "${cloudstack_loadbalancer_rule.foo.ipaddress}" + rule { + source_cidr = "0.0.0.0/0" + protocol = "tcp" + ports = ["${cloudstack_loadbalancer_rule.foo.public_port}"] + } +} +`, + CLOUDSTACK_SERVICE_OFFERING_1, + CLOUDSTACK_NETWORK_1, + CLOUDSTACK_TEMPLATE, + CLOUDSTACK_ZONE, + CLOUDSTACK_PUBLIC_IPADDRESS) + +var testAccCloudStackLoadBalancerRule_update = fmt.Sprintf(` +resource "cloudstack_instance" "foobar1" { + name = "terraform-server1" + display_name = "terraform" + service_offering= "%s" + network = "%s" + template = "%s" + zone = "%s" + expunge = true +} + +resource "cloudstack_loadbalancer_rule" "foo" { + name = "terraform-lb-update" + ipaddress = "%s" + # network omitted, inferred from IP + algorithm = "leastconn" + public_port = 80 + private_port = 80 + members = ["${cloudstack_instance.foobar1.id}"] +} + +# attempt to create dependent firewall rule +# this will clash if cloudstack creates the implicit rule as it does by default +resource "cloudstack_firewall" "foo" { + ipaddress = "${cloudstack_loadbalancer_rule.foo.ipaddress}" + rule { + source_cidr = "0.0.0.0/0" + protocol = "tcp" + ports = ["${cloudstack_loadbalancer_rule.foo.public_port}"] + } +} +`, + CLOUDSTACK_SERVICE_OFFERING_1, + CLOUDSTACK_NETWORK_1, + CLOUDSTACK_TEMPLATE, + CLOUDSTACK_ZONE, + CLOUDSTACK_PUBLIC_IPADDRESS) + +var testAccCloudStackLoadBalancerRule_forcenew = fmt.Sprintf(` +resource "cloudstack_instance" "foobar1" { + name = "terraform-server1" + display_name = "terraform" + service_offering= "%s" + network = "%s" + template = "%s" + zone = "%s" + expunge = true +} + +resource "cloudstack_loadbalancer_rule" "foo" { + name = "terraform-lb-update" + ipaddress = "%s" + # network omitted, inferred from IP + algorithm = "leastconn" + public_port = 443 + private_port = 443 + members = ["${cloudstack_instance.foobar1.id}"] +} + +# attempt to create dependent firewall rule +# this will clash if cloudstack creates the implicit rule as it does by default +resource "cloudstack_firewall" "foo" { + ipaddress = "${cloudstack_loadbalancer_rule.foo.ipaddress}" + rule { + source_cidr = "0.0.0.0/0" + protocol = "tcp" + ports = ["${cloudstack_loadbalancer_rule.foo.public_port}"] + } +} +`, + CLOUDSTACK_SERVICE_OFFERING_1, + CLOUDSTACK_NETWORK_1, + CLOUDSTACK_TEMPLATE, + CLOUDSTACK_ZONE, + CLOUDSTACK_PUBLIC_IPADDRESS) + +var testAccCloudStackLoadBalancerRule_vpc = fmt.Sprintf(` resource "cloudstack_vpc" "foobar" { name = "terraform-vpc" cidr = "%s" @@ -180,7 +415,7 @@ resource "cloudstack_loadbalancer_rule" "foo" { CLOUDSTACK_SERVICE_OFFERING_1, CLOUDSTACK_TEMPLATE) -var testAccCloudStackLoadBalancerRule_update = fmt.Sprintf(` +var testAccCloudStackLoadBalancerRule_vpc_update = fmt.Sprintf(` resource "cloudstack_vpc" "foobar" { name = "terraform-vpc" cidr = "%s"