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.

Sign up for GitHub

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

provider/openstack: LoadBalancer Security Groups #11074

Merged
merged 2 commits into from
Jan 12, 2017
Merged

provider/openstack: LoadBalancer Security Groups #11074

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9933a6f
provider/openstack: LoadBalancer Security Groups
jtopjian Jan 6, 2017
78dec03
provider/openstack: LoadBalancer Security Groups Refactor
jtopjian Jan 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions builtin/providers/openstack/resource_openstack_lb_loadbalancer_v2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/gophercloud/gophercloud"
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/lbaas_v2/loadbalancers"
"github.com/gophercloud/gophercloud/openstack/networking/v2/ports"
)

func resourceLoadBalancerV2() *schema.Resource {
Expand Down Expand Up @@ -80,6 +81,13 @@ func resourceLoadBalancerV2() *schema.Resource {
Computed: true,
ForceNew: true,
},

"security_group_ids": &schema.Schema{
Type: schema.TypeSet,
Optional: true,
Elem: &schema.Schema{Type: schema.TypeString},
Set: schema.HashString,
},
},
}
}
Expand Down Expand Up @@ -126,6 +134,25 @@ func resourceLoadBalancerV2Create(d *schema.ResourceData, meta interface{}) erro
return err
}

// Once the loadbalancer has been created, apply any requested security groups
// to the port that was created behind the scenes.
if lb.VipPortID != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth letting the Create func call the Update func here? That way you wouldn't need to duplicate the code too much

Will that introduce any more risk?

P.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting idea! I'll try it out.

if _, ok := d.GetOk("security_group_ids"); ok {
updateOpts := ports.UpdateOpts{
SecurityGroups: resourcePortSecurityGroupsV2(d),
}

log.Printf("[DEBUG] Adding security groups to OpenStack LoadBalancer (%s) "+
"VIP Port (%s): %#v", lb.ID, lb.VipPortID, updateOpts)

_, err = ports.Update(networkingClient, lb.VipPortID, updateOpts).Extract()
if err != nil {
return err
}
}
}

// If all has been successful, set the ID on the resource
d.SetId(lb.ID)

return resourceLoadBalancerV2Read(d, meta)
Expand Down Expand Up @@ -155,6 +182,16 @@ func resourceLoadBalancerV2Read(d *schema.ResourceData, meta interface{}) error
d.Set("flavor", lb.Flavor)
d.Set("provider", lb.Provider)

// Get any security groups on the VIP Port
if lb.VipPortID != "" {
port, err := ports.Get(networkingClient, lb.VipPortID).Extract()
if err != nil {
return err
}

d.Set("security_group_ids", port.SecurityGroups)
}

return nil
}

Expand Down Expand Up @@ -184,6 +221,21 @@ func resourceLoadBalancerV2Update(d *schema.ResourceData, meta interface{}) erro
return fmt.Errorf("Error updating OpenStack LBaaSV2 LoadBalancer: %s", err)
}

// Security Groups get updated separately
if d.HasChange("security_group_ids") {
vipPortID := d.Get("vip_port_id").(string)
if vipPortID != "" {
updateOpts := ports.UpdateOpts{
SecurityGroups: resourcePortSecurityGroupsV2(d),
}

_, err := ports.Update(networkingClient, vipPortID, updateOpts).Extract()
if err != nil {
return err
}
}
}

return resourceLoadBalancerV2Read(d, meta)
}

Expand Down
191 changes: 185 additions & 6 deletions builtin/providers/openstack/resource_openstack_lb_loadbalancer_v2_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@ import (
"regexp"
"testing"

"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/lbaas_v2/loadbalancers"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"

"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/lbaas_v2/loadbalancers"
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups"
"github.com/gophercloud/gophercloud/openstack/networking/v2/ports"
)

func TestAccLBV2LoadBalancer_basic(t *testing.T) {
Expand All @@ -19,13 +22,13 @@ func TestAccLBV2LoadBalancer_basic(t *testing.T) {
CheckDestroy: testAccCheckLBV2LoadBalancerDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: TestAccLBV2LoadBalancerConfig_basic,
Config: testAccLBV2LoadBalancerConfig_basic,
Check: resource.ComposeTestCheckFunc(
testAccCheckLBV2LoadBalancerExists("openstack_lb_loadbalancer_v2.loadbalancer_1", &lb),
),
},
resource.TestStep{
Config: TestAccLBV2LoadBalancerConfig_update,
Config: testAccLBV2LoadBalancerConfig_update,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"openstack_lb_loadbalancer_v2.loadbalancer_1", "name", "loadbalancer_1_updated"),
Expand All @@ -38,6 +41,62 @@ func TestAccLBV2LoadBalancer_basic(t *testing.T) {
})
}

func TestAccLBV2LoadBalancer_secGroup(t *testing.T) {
var lb loadbalancers.LoadBalancer
var sg_1, sg_2 groups.SecGroup

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckLBV2LoadBalancerDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: testAccLBV2LoadBalancer_secGroup,
Check: resource.ComposeTestCheckFunc(
testAccCheckLBV2LoadBalancerExists(
"openstack_lb_loadbalancer_v2.loadbalancer_1", &lb),
testAccCheckNetworkingV2SecGroupExists(
"openstack_networking_secgroup_v2.secgroup_1", &sg_1),
testAccCheckNetworkingV2SecGroupExists(
"openstack_networking_secgroup_v2.secgroup_1", &sg_2),
resource.TestCheckResourceAttr(
"openstack_lb_loadbalancer_v2.loadbalancer_1", "security_group_ids.#", "1"),
testAccCheckLBV2LoadBalancerHasSecGroup(&lb, &sg_1),
),
},
resource.TestStep{
Config: testAccLBV2LoadBalancer_secGroup_update1,
Check: resource.ComposeTestCheckFunc(
testAccCheckLBV2LoadBalancerExists(
"openstack_lb_loadbalancer_v2.loadbalancer_1", &lb),
testAccCheckNetworkingV2SecGroupExists(
"openstack_networking_secgroup_v2.secgroup_2", &sg_1),
testAccCheckNetworkingV2SecGroupExists(
"openstack_networking_secgroup_v2.secgroup_2", &sg_2),
resource.TestCheckResourceAttr(
"openstack_lb_loadbalancer_v2.loadbalancer_1", "security_group_ids.#", "2"),
testAccCheckLBV2LoadBalancerHasSecGroup(&lb, &sg_1),
testAccCheckLBV2LoadBalancerHasSecGroup(&lb, &sg_2),
),
},
resource.TestStep{
Config: testAccLBV2LoadBalancer_secGroup_update2,
Check: resource.ComposeTestCheckFunc(
testAccCheckLBV2LoadBalancerExists(
"openstack_lb_loadbalancer_v2.loadbalancer_1", &lb),
testAccCheckNetworkingV2SecGroupExists(
"openstack_networking_secgroup_v2.secgroup_2", &sg_1),
testAccCheckNetworkingV2SecGroupExists(
"openstack_networking_secgroup_v2.secgroup_2", &sg_2),
resource.TestCheckResourceAttr(
"openstack_lb_loadbalancer_v2.loadbalancer_1", "security_group_ids.#", "1"),
testAccCheckLBV2LoadBalancerHasSecGroup(&lb, &sg_2),
),
},
},
})
}

func testAccCheckLBV2LoadBalancerDestroy(s *terraform.State) error {
config := testAccProvider.Meta().(*Config)
networkingClient, err := config.networkingV2Client(OS_REGION_NAME)
Expand All @@ -59,7 +118,8 @@ func testAccCheckLBV2LoadBalancerDestroy(s *terraform.State) error {
return nil
}

func testAccCheckLBV2LoadBalancerExists(n string, lb *loadbalancers.LoadBalancer) resource.TestCheckFunc {
func testAccCheckLBV2LoadBalancerExists(
n string, lb *loadbalancers.LoadBalancer) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
if !ok {
Expand Down Expand Up @@ -91,7 +151,31 @@ func testAccCheckLBV2LoadBalancerExists(n string, lb *loadbalancers.LoadBalancer
}
}

const TestAccLBV2LoadBalancerConfig_basic = `
func testAccCheckLBV2LoadBalancerHasSecGroup(
lb *loadbalancers.LoadBalancer, sg *groups.SecGroup) resource.TestCheckFunc {
return func(s *terraform.State) error {
config := testAccProvider.Meta().(*Config)
networkingClient, err := config.networkingV2Client(OS_REGION_NAME)
if err != nil {
return fmt.Errorf("Error creating OpenStack networking client: %s", err)
}

port, err := ports.Get(networkingClient, lb.VipPortID).Extract()
if err != nil {
return err
}

for _, p := range port.SecurityGroups {
if p == sg.ID {
return nil
}
}

return fmt.Errorf("LoadBalancer does not have the security group")
}
}

const testAccLBV2LoadBalancerConfig_basic = `
resource "openstack_networking_network_v2" "network_1" {
name = "network_1"
admin_state_up = "true"
Expand All @@ -110,7 +194,7 @@ resource "openstack_lb_loadbalancer_v2" "loadbalancer_1" {
}
`

const TestAccLBV2LoadBalancerConfig_update = `
const testAccLBV2LoadBalancerConfig_update = `
resource "openstack_networking_network_v2" "network_1" {
name = "network_1"
admin_state_up = "true"
Expand All @@ -129,3 +213,98 @@ resource "openstack_lb_loadbalancer_v2" "loadbalancer_1" {
vip_subnet_id = "${openstack_networking_subnet_v2.subnet_1.id}"
}
`

const testAccLBV2LoadBalancer_secGroup = `
resource "openstack_networking_secgroup_v2" "secgroup_1" {
name = "secgroup_1"
description = "secgroup_1"
}

resource "openstack_networking_secgroup_v2" "secgroup_2" {
name = "secgroup_2"
description = "secgroup_2"
}

resource "openstack_networking_network_v2" "network_1" {
name = "network_1"
admin_state_up = "true"
}

resource "openstack_networking_subnet_v2" "subnet_1" {
name = "subnet_1"
network_id = "${openstack_networking_network_v2.network_1.id}"
cidr = "192.168.199.0/24"
}

resource "openstack_lb_loadbalancer_v2" "loadbalancer_1" {
name = "loadbalancer_1"
vip_subnet_id = "${openstack_networking_subnet_v2.subnet_1.id}"
security_group_ids = [
"${openstack_networking_secgroup_v2.secgroup_1.id}"
]
}
`

const testAccLBV2LoadBalancer_secGroup_update1 = `
resource "openstack_networking_secgroup_v2" "secgroup_1" {
name = "secgroup_1"
description = "secgroup_1"
}

resource "openstack_networking_secgroup_v2" "secgroup_2" {
name = "secgroup_2"
description = "secgroup_2"
}

resource "openstack_networking_network_v2" "network_1" {
name = "network_1"
admin_state_up = "true"
}

resource "openstack_networking_subnet_v2" "subnet_1" {
name = "subnet_1"
network_id = "${openstack_networking_network_v2.network_1.id}"
cidr = "192.168.199.0/24"
}

resource "openstack_lb_loadbalancer_v2" "loadbalancer_1" {
name = "loadbalancer_1"
vip_subnet_id = "${openstack_networking_subnet_v2.subnet_1.id}"
security_group_ids = [
"${openstack_networking_secgroup_v2.secgroup_1.id}",
"${openstack_networking_secgroup_v2.secgroup_2.id}"
]
}
`

const testAccLBV2LoadBalancer_secGroup_update2 = `
resource "openstack_networking_secgroup_v2" "secgroup_1" {
name = "secgroup_1"
description = "secgroup_1"
}

resource "openstack_networking_secgroup_v2" "secgroup_2" {
name = "secgroup_2"
description = "secgroup_2"
}

resource "openstack_networking_network_v2" "network_1" {
name = "network_1"
admin_state_up = "true"
}

resource "openstack_networking_subnet_v2" "subnet_1" {
name = "subnet_1"
network_id = "${openstack_networking_network_v2.network_1.id}"
cidr = "192.168.199.0/24"
}

resource "openstack_lb_loadbalancer_v2" "loadbalancer_1" {
name = "loadbalancer_1"
vip_subnet_id = "${openstack_networking_subnet_v2.subnet_1.id}"
security_group_ids = [
"${openstack_networking_secgroup_v2.secgroup_2.id}"
]
depends_on = ["openstack_networking_secgroup_v2.secgroup_1"]
}
`
5 changes: 5 additions & 0 deletions website/source/docs/providers/openstack/r/lb_loadbalancer_v2.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ The following arguments are supported:
* `provider` - (Optional) The name of the provider. Changing this creates a new
loadbalancer.

* `security_group_ids` - (Optional) A list of security group IDs to apply to the
loadbalancer. The security groups must be specified by ID and not name (as
opposed to how they are configured with the Compute Instance).

## Attributes Reference

The following attributes are exported:
Expand All @@ -66,4 +70,5 @@ The following attributes are exported:
* `admin_state_up` - See Argument Reference above.
* `flavor` - See Argument Reference above.
* `provider` - See Argument Reference above.
* `security_group_ids` - See Argument Reference above.
* `vip_port_id` - The Port ID of the Load Balancer IP.