diff --git a/cloudstack/resource_cloudstack_security_group_rule.go b/cloudstack/resource_cloudstack_security_group_rule.go index 074d6e75..c41887ca 100644 --- a/cloudstack/resource_cloudstack_security_group_rule.go +++ b/cloudstack/resource_cloudstack_security_group_rule.go @@ -33,9 +33,12 @@ import ( ) type authorizeSecurityGroupParams interface { + SetAccount(string) SetCidrlist([]string) + SetDomainid(string) SetIcmptype(int) SetIcmpcode(int) + SetProjectid(string) SetStartport(int) SetEndport(int) SetProtocol(string) @@ -155,6 +158,14 @@ func createSecurityGroupRules(d *schema.ResourceData, meta interface{}, rules *s cs := meta.(*cloudstack.CloudStackClient) var errs *multierror.Error + sg, _, err := cs.SecurityGroup.GetSecurityGroupByID( + d.Id(), + cloudstack.WithProject(d.Get("project").(string)), + ) + if err != nil { + return err + } + var wg sync.WaitGroup wg.Add(nrs.Len()) @@ -186,6 +197,7 @@ func createSecurityGroupRules(d *schema.ResourceData, meta interface{}, rules *s } p.SetSecuritygroupid(d.Id()) + setSecurityGroupRuleOwnership(p, sg) p.SetCidrlist([]string{cidr.(string)}) // Create a single rule @@ -198,10 +210,12 @@ func createSecurityGroupRules(d *schema.ResourceData, meta interface{}, rules *s if usgList, ok := rule["user_security_group_list"].(*schema.Set); ok && usgList.Len() > 0 { for _, usg := range usgList.List() { - sg, _, err := cs.SecurityGroup.GetSecurityGroupByName( - usg.(string), - cloudstack.WithProject(d.Get("project").(string)), - ) + sourceSG, err := getSecurityGroupRuleSourceGroup(cs, sg, usg.(string)) + if err != nil { + errs = multierror.Append(errs, err) + continue + } + sourceAccount, err := getSecurityGroupRuleSourceAccount(cs, sourceSG) if err != nil { errs = multierror.Append(errs, err) continue @@ -216,7 +230,8 @@ func createSecurityGroupRules(d *schema.ResourceData, meta interface{}, rules *s } p.SetSecuritygroupid(d.Id()) - p.SetUsersecuritygrouplist(map[string]string{sg.Account: usg.(string)}) + setSecurityGroupRuleOwnership(p, sg) + p.SetUsersecuritygrouplist(map[string]string{sourceAccount: usg.(string)}) // Create a single rule err = createSecurityGroupRule(d, meta, rule, p, usg.(string)) @@ -240,6 +255,64 @@ func createSecurityGroupRules(d *schema.ResourceData, meta interface{}, rules *s return errs.ErrorOrNil() } +func setSecurityGroupRuleOwnership(p authorizeSecurityGroupParams, sg *cloudstack.SecurityGroup) { + if sg.Projectid != "" { + p.SetProjectid(sg.Projectid) + return + } + + if sg.Domainid != "" { + p.SetDomainid(sg.Domainid) + + if sg.Account != "" { + p.SetAccount(sg.Account) + } + } +} + +func getSecurityGroupRuleSourceGroup(cs *cloudstack.CloudStackClient, targetSG *cloudstack.SecurityGroup, name string) (*cloudstack.SecurityGroup, error) { + p := cs.SecurityGroup.NewListSecurityGroupsParams() + p.SetSecuritygroupname(name) + + if targetSG.Projectid != "" { + p.SetProjectid(targetSG.Projectid) + } + + l, err := cs.SecurityGroup.ListSecurityGroups(p) + if err != nil { + return nil, err + } + + if l.Count == 0 { + return nil, fmt.Errorf("No match found for security group %q", name) + } + + if l.Count > 1 { + return nil, fmt.Errorf("There is more then one result for SecurityGroup name: %s", name) + } + + return l.SecurityGroups[0], nil +} + +func getSecurityGroupRuleSourceAccount(cs *cloudstack.CloudStackClient, sourceSG *cloudstack.SecurityGroup) (string, error) { + if sourceSG.Projectid == "" { + if sourceSG.Account == "" { + return "", fmt.Errorf("security group %q is missing account ownership", sourceSG.Name) + } + return sourceSG.Account, nil + } + + project, _, err := cs.Project.GetProjectByID(sourceSG.Projectid) + if err != nil { + return "", err + } + if project.Projectaccountname == "" { + return "", fmt.Errorf("project %q is missing project account ownership", sourceSG.Projectid) + } + + return project.Projectaccountname, nil +} + func createSecurityGroupRule(d *schema.ResourceData, meta interface{}, rule map[string]interface{}, p authorizeSecurityGroupParams, uuid string) error { cs := meta.(*cloudstack.CloudStackClient) uuids := rule["uuids"].(map[string]interface{}) diff --git a/cloudstack/resource_cloudstack_security_group_rule_test.go b/cloudstack/resource_cloudstack_security_group_rule_test.go index 9d44b55d..9ee8adc7 100644 --- a/cloudstack/resource_cloudstack_security_group_rule_test.go +++ b/cloudstack/resource_cloudstack_security_group_rule_test.go @@ -39,6 +39,7 @@ func TestAccCloudStackSecurityGroupRule_basic(t *testing.T) { Config: testAccCloudStackSecurityGroupRule_basic, Check: resource.ComposeTestCheckFunc( testAccCheckCloudStackSecurityGroupRulesExist("cloudstack_security_group.foo"), + testAccCheckCloudStackSecurityGroupRuleTargetScope("cloudstack_security_group.foo", false), resource.TestCheckResourceAttr( "cloudstack_security_group_rule.foo", "rule.#", "3"), resource.TestCheckTypeSetElemNestedAttrs( @@ -69,6 +70,47 @@ func TestAccCloudStackSecurityGroupRule_basic(t *testing.T) { }) } +func TestCloudStackSecurityGroupRuleOwnership_project(t *testing.T) { + p := new(cloudstack.SecurityGroupService).NewAuthorizeSecurityGroupIngressParams() + sg := &cloudstack.SecurityGroup{ + Account: "admin", + Domainid: "domain-id", + Projectid: "project-id", + } + + setSecurityGroupRuleOwnership(p, sg) + + if projectID, ok := p.GetProjectid(); !ok || projectID != "project-id" { + t.Fatalf("expected project-id ownership, got %q, %t", projectID, ok) + } + + if account, ok := p.GetAccount(); ok { + t.Fatalf("expected no account ownership for project rule, got %q", account) + } +} + +func TestCloudStackSecurityGroupRuleOwnership_account(t *testing.T) { + p := new(cloudstack.SecurityGroupService).NewAuthorizeSecurityGroupEgressParams() + sg := &cloudstack.SecurityGroup{ + Account: "admin", + Domainid: "domain-id", + } + + setSecurityGroupRuleOwnership(p, sg) + + if account, ok := p.GetAccount(); !ok || account != "admin" { + t.Fatalf("expected admin account ownership, got %q, %t", account, ok) + } + + if domainID, ok := p.GetDomainid(); !ok || domainID != "domain-id" { + t.Fatalf("expected domain-id ownership, got %q, %t", domainID, ok) + } + + if projectID, ok := p.GetProjectid(); ok { + t.Fatalf("expected no project ownership for account rule, got %q", projectID) + } +} + func TestAccCloudStackSecurityGroupRule_update(t *testing.T) { resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -79,6 +121,7 @@ func TestAccCloudStackSecurityGroupRule_update(t *testing.T) { Config: testAccCloudStackSecurityGroupRule_basic, Check: resource.ComposeTestCheckFunc( testAccCheckCloudStackSecurityGroupRulesExist("cloudstack_security_group.foo"), + testAccCheckCloudStackSecurityGroupRuleTargetScope("cloudstack_security_group.foo", false), resource.TestCheckResourceAttr( "cloudstack_security_group_rule.foo", "rule.#", "3"), resource.TestCheckTypeSetElemNestedAttrs( @@ -110,6 +153,7 @@ func TestAccCloudStackSecurityGroupRule_update(t *testing.T) { Config: testAccCloudStackSecurityGroupRule_update, Check: resource.ComposeTestCheckFunc( testAccCheckCloudStackSecurityGroupRulesExist("cloudstack_security_group.foo"), + testAccCheckCloudStackSecurityGroupRuleTargetScope("cloudstack_security_group.foo", false), resource.TestCheckResourceAttr( "cloudstack_security_group_rule.foo", "rule.#", "4"), resource.TestCheckTypeSetElemNestedAttrs( @@ -147,6 +191,39 @@ func TestAccCloudStackSecurityGroupRule_update(t *testing.T) { }) } +func TestAccCloudStackSecurityGroupRule_project(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackSecurityGroupRuleDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackSecurityGroupRule_project, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackSecurityGroupRulesExist("cloudstack_security_group.foo"), + testAccCheckCloudStackSecurityGroupRuleTargetScope("cloudstack_security_group.foo", true), + resource.TestCheckResourceAttr( + "cloudstack_security_group.foo", "project", "terraform"), + resource.TestCheckResourceAttr( + "cloudstack_security_group_rule.foo", "project", "terraform"), + resource.TestCheckResourceAttr( + "cloudstack_security_group_rule.foo", "rule.#", "1"), + resource.TestCheckResourceAttr( + "cloudstack_security_group_rule.foo", "rule.0.protocol", "tcp"), + resource.TestCheckResourceAttr( + "cloudstack_security_group_rule.foo", "rule.0.ports.#", "1"), + resource.TestCheckResourceAttr( + "cloudstack_security_group_rule.foo", "rule.0.ports.0", "80"), + resource.TestCheckResourceAttr( + "cloudstack_security_group_rule.foo", "rule.0.traffic_type", "ingress"), + resource.TestCheckResourceAttr( + "cloudstack_security_group_rule.foo", "rule.0.user_security_group_list.0", "terraform-project-security-group-bar"), + ), + }, + }, + }) +} + func testAccCheckCloudStackSecurityGroupRulesExist(n string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -159,7 +236,10 @@ func testAccCheckCloudStackSecurityGroupRulesExist(n string) resource.TestCheckF } cs := testAccProvider.Meta().(*cloudstack.CloudStackClient) - sg, count, err := cs.SecurityGroup.GetSecurityGroupByID(rs.Primary.ID) + sg, count, err := cs.SecurityGroup.GetSecurityGroupByID( + rs.Primary.ID, + cloudstack.WithProject(rs.Primary.Attributes["project"]), + ) if err != nil { if count == 0 { return fmt.Errorf("Security group %s not found", rs.Primary.ID) @@ -188,6 +268,40 @@ func testAccCheckCloudStackSecurityGroupRulesExist(n string) resource.TestCheckF } } +func testAccCheckCloudStackSecurityGroupRuleTargetScope(n string, wantProject bool) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) + } + + cs := testAccProvider.Meta().(*cloudstack.CloudStackClient) + sg, _, err := cs.SecurityGroup.GetSecurityGroupByID( + rs.Primary.ID, + cloudstack.WithProject(rs.Primary.Attributes["project"]), + ) + if err != nil { + return err + } + + if wantProject { + if sg.Projectid == "" { + return fmt.Errorf("security group %s is not assigned to a project", rs.Primary.ID) + } + return nil + } + + if sg.Projectid != "" { + return fmt.Errorf("security group %s is unexpectedly assigned to project %s", rs.Primary.ID, sg.Projectid) + } + if sg.Account == "" || sg.Domainid == "" { + return fmt.Errorf("security group %s is missing account/domain ownership", rs.Primary.ID) + } + + return nil + } +} + func testAccCheckCloudStackSecurityGroupRuleDestroy(s *terraform.State) error { cs := testAccProvider.Meta().(*cloudstack.CloudStackClient) @@ -200,7 +314,10 @@ func testAccCheckCloudStackSecurityGroupRuleDestroy(s *terraform.State) error { return fmt.Errorf("No security group rule ID is set") } - sg, count, err := cs.SecurityGroup.GetSecurityGroupByID(rs.Primary.ID) + sg, count, err := cs.SecurityGroup.GetSecurityGroupByID( + rs.Primary.ID, + cloudstack.WithProject(rs.Primary.Attributes["project"]), + ) if err != nil { if count == 0 { continue @@ -265,6 +382,33 @@ resource "cloudstack_security_group_rule" "foo" { depends_on = ["cloudstack_security_group.bar"] }` +const testAccCloudStackSecurityGroupRule_project = ` +resource "cloudstack_security_group" "foo" { + name = "terraform-project-security-group-foo" + description = "terraform-security-group-text" + project = "terraform" +} + +resource "cloudstack_security_group" "bar" { + name = "terraform-project-security-group-bar" + description = "terraform-security-group-text" + project = "terraform" +} + +resource "cloudstack_security_group_rule" "foo" { + security_group_id = cloudstack_security_group.foo.id + project = "terraform" + + rule { + protocol = "tcp" + ports = ["80"] + traffic_type = "ingress" + user_security_group_list = ["terraform-project-security-group-bar"] + } + + depends_on = ["cloudstack_security_group.bar"] +}` + const testAccCloudStackSecurityGroupRule_update = ` resource "cloudstack_security_group" "foo" { name = "terraform-security-group-foo" @@ -306,5 +450,5 @@ resource "cloudstack_security_group_rule" "foo" { user_security_group_list = ["terraform-security-group-bar"] } - depends_on = ["cloudstack_security_group.bar"] + depends_on = ["cloudstack_security_group.bar"] }`