From 218345a29a3d2ebfb63858381f9643c6c69ae5dc Mon Sep 17 00:00:00 2001 From: Ujjwal Kumar Date: Wed, 19 Apr 2023 09:32:20 +0530 Subject: [PATCH] fix(bm): patch of security groups on bare metal server nics --- .../vpc/resource_ibm_is_bare_metal_server.go | 74 ++++++++++++--- .../resource_ibm_is_bare_metal_server_test.go | 94 +++++++++++++++++++ 2 files changed, 155 insertions(+), 13 deletions(-) diff --git a/ibm/service/vpc/resource_ibm_is_bare_metal_server.go b/ibm/service/vpc/resource_ibm_is_bare_metal_server.go index f8b109f42c..94c1ede67f 100644 --- a/ibm/service/vpc/resource_ibm_is_bare_metal_server.go +++ b/ibm/service/vpc/resource_ibm_is_bare_metal_server.go @@ -2339,7 +2339,7 @@ func bareMetalServerUpdate(context context.Context, d *schema.ResourceData, meta if d.HasChange(isBareMetalServerPrimaryNetworkInterface) { nicId := d.Get("primary_network_interface.0.id").(string) - + nicflag := false if d.HasChange("primary_network_interface.0.primary_ip.0.name") || d.HasChange("primary_network_interface.0.primary_ip.0.auto_delete") { subnetId := d.Get("primary_network_interface.0.subnet").(string) ripId := d.Get("primary_network_interface.0.primary_ip.0.reserved_ip").(string) @@ -2380,6 +2380,7 @@ func bareMetalServerUpdate(context context.Context, d *schema.ResourceData, meta } bmsNicPatchModel.AllowedVlans = allowedVlans } + nicflag = true } if d.HasChange("primary_network_interface.0.allow_ip_spoofing") { @@ -2388,32 +2389,79 @@ func bareMetalServerUpdate(context context.Context, d *schema.ResourceData, meta if allowIpSpoofing { bmsNicPatchModel.AllowIPSpoofing = &allowIpSpoofing } + nicflag = true } } if d.HasChange("primary_network_interface.0.enable_infrastructure_nat") { if enableNatOk, ok := d.GetOk("primary_network_interface.0.enable_infrastructure_nat"); ok { enableNat := enableNatOk.(bool) bmsNicPatchModel.EnableInfrastructureNat = &enableNat + nicflag = true + } + } + if d.HasChange("primary_network_interface.0.security_groups") && !d.IsNewResource() { + ovs, nvs := d.GetChange("primary_network_interface.0.security_groups") + ov := ovs.(*schema.Set) + nv := nvs.(*schema.Set) + remove := flex.ExpandStringList(ov.Difference(nv).List()) + add := flex.ExpandStringList(nv.Difference(ov).List()) + if len(add) > 0 { + networkID := d.Get("primary_network_interface.0.id").(string) + for i := range add { + createsgnicoptions := &vpcv1.CreateSecurityGroupTargetBindingOptions{ + SecurityGroupID: &add[i], + ID: &networkID, + } + _, response, err := sess.CreateSecurityGroupTargetBinding(createsgnicoptions) + if err != nil { + return fmt.Errorf("[ERROR] Error while creating security group %q for primary network interface of bare metal server %s\n%s: %q", add[i], d.Id(), err, response) + } + _, err = isWaitForBareMetalServerAvailable(sess, id, d.Timeout(schema.TimeoutUpdate), d) + if err != nil { + return err + } + } + + } + if len(remove) > 0 { + networkID := d.Get("primary_network_interface.0.id").(string) + for i := range remove { + deletesgnicoptions := &vpcv1.DeleteSecurityGroupTargetBindingOptions{ + SecurityGroupID: &remove[i], + ID: &networkID, + } + response, err := sess.DeleteSecurityGroupTargetBinding(deletesgnicoptions) + if err != nil { + return fmt.Errorf("[ERROR] Error while removing security group %q for primary network interface of bare metal server %s\n%s: %q", remove[i], d.Id(), err, response) + } + _, err = isWaitForBareMetalServerAvailable(sess, id, d.Timeout(schema.TimeoutUpdate), d) + if err != nil { + return err + } + } } } if d.HasChange("primary_network_interface.0.name") { if nameOk, ok := d.GetOk("primary_network_interface.0.name"); ok { name := nameOk.(string) bmsNicPatchModel.Name = &name + nicflag = true } } - bmsNicPatch, err := bmsNicPatchModel.AsPatch() - if err != nil { - return err - } - bmsNicUpdateOptions.BareMetalServerNetworkInterfacePatch = bmsNicPatch - _, _, err = sess.UpdateBareMetalServerNetworkInterfaceWithContext(context, bmsNicUpdateOptions) - if err != nil { - return err - } - _, err = isWaitForBareMetalServerAvailable(sess, id, d.Timeout(schema.TimeoutUpdate), d) - if err != nil { - return err + if nicflag { + bmsNicPatch, err := bmsNicPatchModel.AsPatch() + if err != nil { + return err + } + bmsNicUpdateOptions.BareMetalServerNetworkInterfacePatch = bmsNicPatch + _, _, err = sess.UpdateBareMetalServerNetworkInterfaceWithContext(context, bmsNicUpdateOptions) + if err != nil { + return err + } + _, err = isWaitForBareMetalServerAvailable(sess, id, d.Timeout(schema.TimeoutUpdate), d) + if err != nil { + return err + } } } if d.HasChange(isBareMetalServerName) { diff --git a/ibm/service/vpc/resource_ibm_is_bare_metal_server_test.go b/ibm/service/vpc/resource_ibm_is_bare_metal_server_test.go index 70e47c6719..3558a5c8ce 100644 --- a/ibm/service/vpc/resource_ibm_is_bare_metal_server_test.go +++ b/ibm/service/vpc/resource_ibm_is_bare_metal_server_test.go @@ -46,6 +46,54 @@ ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCKVmnMOlHKcZK8tpt3MP1lqOLAcqcJzhsvJcjscgVE }, }) } +func TestAccIBMISBareMetalServer_sg_update(t *testing.T) { + var server string + vpcname := fmt.Sprintf("tf-vpc-%d", acctest.RandIntRange(10, 100)) + name := fmt.Sprintf("tf-server-%d", acctest.RandIntRange(10, 100)) + subnetname := fmt.Sprintf("tfip-subnet-%d", acctest.RandIntRange(10, 100)) + publicKey := strings.TrimSpace(` +ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCKVmnMOlHKcZK8tpt3MP1lqOLAcqcJzhsvJcjscgVERRN7/9484SOBJ3HSKxxNG5JN8owAjy5f9yYwcUg+JaUVuytn5Pv3aeYROHGGg+5G346xaq3DAwX6Y5ykr2fvjObgncQBnuU5KHWCECO/4h8uWuwh/kfniXPVjFToc+gnkqA+3RKpAecZhFXwfalQ9mMuYGFxn+fwn8cYEApsJbsEmb0iJwPiZ5hjFC8wREuiTlhPHDgkBLOiycd20op2nXzDbHfCHInquEe/gYxEitALONxm0swBOwJZwlTDOB7C6y2dzlrtxr1L59m7pCkWI4EtTRLvleehBoj3u7jB4usR +`) + sshname := fmt.Sprintf("tf-sshname-%d", acctest.RandIntRange(10, 100)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acc.TestAccPreCheck(t) }, + Providers: acc.TestAccProviders, + CheckDestroy: testAccCheckIBMISBareMetalServerDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCheckIBMISBareMetalServerConfig(vpcname, subnetname, sshname, publicKey, name), + Check: resource.ComposeTestCheckFunc( + testAccCheckIBMISBareMetalServerExists("ibm_is_bare_metal_server.testacc_bms", server), + resource.TestCheckResourceAttr( + "ibm_is_bare_metal_server.testacc_bms", "name", name), + resource.TestCheckResourceAttr( + "ibm_is_bare_metal_server.testacc_bms", "zone", acc.ISZoneName), + resource.TestCheckResourceAttr( + "ibm_is_bare_metal_server.testacc_bms", "primary_network_interface.0.security_groups.#", "1"), + ), + }, + { + Config: testAccCheckIBMISBareMetalServerSgUpdateConfig(vpcname, subnetname, sshname, publicKey, name), + Check: resource.ComposeTestCheckFunc( + testAccCheckIBMISBareMetalServerExists("ibm_is_bare_metal_server.testacc_bms", server), + resource.TestCheckResourceAttr( + "ibm_is_bare_metal_server.testacc_bms", "name", name), + resource.TestCheckResourceAttr( + "ibm_is_bare_metal_server.testacc_bms", "zone", acc.ISZoneName), + resource.TestCheckResourceAttr( + "ibm_is_security_group.testacc_sg1", "name", "test-security-group1"), + resource.TestCheckResourceAttr( + "ibm_is_security_group.testacc_sg2", "name", "test-security-group2"), + resource.TestCheckResourceAttr( + "ibm_is_security_group.testacc_sg3", "name", "test-security-group3"), + resource.TestCheckResourceAttr( + "ibm_is_bare_metal_server.testacc_bms", "primary_network_interface.0.security_groups.#", "3"), + ), + }, + }, + }) +} func TestAccIBMISBareMetalServer_SecureBoot_tpm(t *testing.T) { var server string vpcname := fmt.Sprintf("tf-vpc-%d", acctest.RandIntRange(10, 100)) @@ -357,6 +405,52 @@ func testAccCheckIBMISBareMetalServerConfig(vpcname, subnetname, sshname, public } `, vpcname, subnetname, acc.ISZoneName, sshname, publicKey, acc.IsBareMetalServerProfileName, name, acc.IsBareMetalServerImage, acc.ISZoneName) } +func testAccCheckIBMISBareMetalServerSgUpdateConfig(vpcname, subnetname, sshname, publicKey, name string) string { + return fmt.Sprintf(` + resource "ibm_is_vpc" "testacc_vpc" { + name = "%s" + } + + resource "ibm_is_subnet" "testacc_subnet" { + name = "%s" + vpc = ibm_is_vpc.testacc_vpc.id + zone = "%s" + total_ipv4_address_count = 16 + } + + resource "ibm_is_ssh_key" "testacc_sshkey" { + name = "%s" + public_key = "%s" + } + + + resource "ibm_is_security_group" "testacc_sg1" { + name = "test-security-group1" + vpc = ibm_is_vpc.testacc_vpc.id + } + resource "ibm_is_security_group" "testacc_sg2" { + name = "test-security-group2" + vpc = ibm_is_vpc.testacc_vpc.id + } + resource "ibm_is_security_group" "testacc_sg3" { + name = "test-security-group3" + vpc = ibm_is_vpc.testacc_vpc.id + } + + resource "ibm_is_bare_metal_server" "testacc_bms" { + profile = "%s" + name = "%s" + image = "%s" + zone = "%s" + keys = [ibm_is_ssh_key.testacc_sshkey.id] + primary_network_interface { + subnet = ibm_is_subnet.testacc_subnet.id + security_groups = [ibm_is_security_group.testacc_sg1.id, ibm_is_security_group.testacc_sg2.id, ibm_is_security_group.testacc_sg3.id] + } + vpc = ibm_is_vpc.testacc_vpc.id + } +`, vpcname, subnetname, acc.ISZoneName, sshname, publicKey, acc.IsBareMetalServerProfileName, name, acc.IsBareMetalServerImage, acc.ISZoneName) +} func testAccCheckIBMISBareMetalServerSecureBootTpmConfig(vpcname, subnetname, sshname, publicKey, tpm, name string, secureBoot bool) string { return fmt.Sprintf(` resource "ibm_is_vpc" "testacc_vpc" {