feat: add tags when updating a security group (#101)

* feat: add tags when updating a security group

* refactor: function signature

* chore: apply pr feedback

Author: adisazhar123

neutron/live_test.go

@@ -218,7 +218,7 @@ func (s *LiveTests) TestSecurityGroupsV2(c *gc.C) {
 		c.Errorf("expected to find added security group %s", newSecGrp)
 	}
 	// Change the created SecurityGroup's name
-	updatedSecGroup, err := s.neutron.UpdateSecurityGroupV2(newSecGrp.Id, "NameChanged", "")
+	updatedSecGroup, err := s.neutron.UpdateSecurityGroupV2(newSecGrp.Id, "NameChanged", "", []string{})
 	c.Assert(err, gc.IsNil)
 	// Verify the name change
 	foundSecGrps, err := s.neutron.SecurityGroupByNameV2(updatedSecGroup.Name)
@@ -238,6 +238,96 @@ func (s *LiveTests) TestSecurityGroupsV2(c *gc.C) {
 	c.Assert(err, gc.Not(gc.IsNil))
 }
 
+func (s *LiveTests) TestUpdateSecurityGroupsWithTagsV2(c *gc.C) {
+	newSecGrp, err := s.neutron.CreateSecurityGroupV2("SecurityGroupTest", "Testing create security group", []string{})
+	c.Assert(err, gc.IsNil)
+	c.Assert(newSecGrp, gc.Not(gc.IsNil))
+	c.Assert(newSecGrp.Tags, gc.HasLen, 0)
+	defer s.deleteSecurityGroup(newSecGrp.Id, c)
+	query := neutron.ListSecurityGroupsV2Query{}
+	secGrps, err := s.neutron.ListSecurityGroupsV2(query)
+	c.Assert(err, gc.IsNil)
+	c.Assert(secGrps, gc.Not(gc.HasLen), 0)
+	var found bool
+	for _, secGrp := range secGrps {
+		c.Check(secGrp.Id, gc.Not(gc.Equals), "")
+		c.Check(secGrp.Name, gc.Not(gc.Equals), "")
+		c.Check(secGrp.Description, gc.Not(gc.Equals), "")
+		c.Check(secGrp.TenantId, gc.Not(gc.Equals), "")
+		// Is this the SecurityGroup we just created?
+		if secGrp.Id == newSecGrp.Id {
+			found = true
+		}
+	}
+	if !found {
+		c.Errorf("expected to find added security group %s", newSecGrp)
+	}
+	// Change the created SecurityGroup's name
+	updatedSecGroup, err := s.neutron.UpdateSecurityGroupV2(newSecGrp.Id, "NameChanged", "", []string{"new-tag-here"})
+	c.Assert(err, gc.IsNil)
+	// Verify the name change
+	foundSecGrps, err := s.neutron.SecurityGroupByNameV2(updatedSecGroup.Name)
+	c.Assert(err, gc.IsNil)
+	c.Assert(foundSecGrps, gc.Not(gc.HasLen), 0)
+	c.Assert(updatedSecGroup.Tags, gc.HasLen, 1)
+	c.Assert(updatedSecGroup.Tags[0], gc.Equals, "new-tag-here")
+	found = false
+	for _, secGrp := range foundSecGrps {
+		if secGrp.Id == updatedSecGroup.Id {
+			found = true
+			break
+		}
+	}
+	if !found {
+		c.Errorf("expected to find added security group %s, when requested by name", updatedSecGroup.Name)
+	}
+	_, err = s.neutron.SecurityGroupByNameV2(newSecGrp.Name)
+	c.Assert(err, gc.Not(gc.IsNil))
+}
+
+func (s *LiveTests) TestUpdateSecurityGroupsWithTagsV2Rollback(c *gc.C) {
+	newSecGrp, err := s.neutron.CreateSecurityGroupV2("SecurityGroupTest", "Testing create security group", []string{"awesome-group"})
+	c.Assert(err, gc.IsNil)
+	c.Assert(newSecGrp, gc.Not(gc.IsNil))
+	c.Assert(newSecGrp.Tags, gc.HasLen, 1)
+	defer s.deleteSecurityGroup(newSecGrp.Id, c)
+	query := neutron.ListSecurityGroupsV2Query{
+		Tags: []string{"awesome-group"},
+	}
+	secGrps, err := s.neutron.ListSecurityGroupsV2(query)
+	c.Assert(err, gc.IsNil)
+	c.Assert(secGrps, gc.Not(gc.HasLen), 0)
+	var found bool
+	for _, secGrp := range secGrps {
+		c.Check(secGrp.Id, gc.Not(gc.Equals), "")
+		c.Check(secGrp.Name, gc.Not(gc.Equals), "")
+		c.Check(secGrp.Description, gc.Not(gc.Equals), "")
+		c.Check(secGrp.TenantId, gc.Not(gc.Equals), "")
+		// Is this the SecurityGroup we just created?
+		if secGrp.Id == newSecGrp.Id {
+			found = true
+		}
+	}
+	if !found {
+		c.Errorf("expected to find added security group %s", newSecGrp)
+	}
+	// The given tag forces it to rollback
+	updatedSecGroup, err := s.neutron.UpdateSecurityGroupV2(newSecGrp.Id, "NameChanged", "", []string{"unit-test-rollback"})
+	c.Assert(updatedSecGroup, gc.IsNil)
+	c.Assert(err, gc.NotNil)
+	c.Assert(strings.Contains(err.Error(), "updating tags failed, rolled back security group"), gc.Equals, true)
+	// Verify that the name is still the same
+	foundSecGrps, err := s.neutron.SecurityGroupByNameV2(newSecGrp.Name)
+	c.Assert(foundSecGrps, gc.HasLen, 1)
+	c.Assert(err, gc.IsNil)
+	c.Assert(foundSecGrps, gc.NotNil)
+	c.Assert(foundSecGrps[0].Name, gc.Equals, "SecurityGroupTest")
+	c.Assert(foundSecGrps[0].Tags, gc.HasLen, 1)
+
+	nonExistingGroups, _ := s.neutron.SecurityGroupByNameV2("NameChanged")
+	c.Assert(nonExistingGroups, gc.HasLen, 0)
+}
+
 func (s *LiveTests) TestSecurityGroupsV2WithTags(c *gc.C) {
 	newSecGrp, err := s.neutron.CreateSecurityGroupV2("SecurityGroupTest", "Testing create security group", []string{"awesome-group"})
 	c.Assert(err, gc.IsNil)

neutron/neutron.go

@@ -466,8 +466,23 @@ func (c *Client) DeleteSecurityGroupV2(groupId string) error {
 	return err
 }
 
+// GetSecurityGroupV2 finds a security group by ID.
+func (c *Client) GetSecurityGroupV2(groupId string) (*SecurityGroupV2, error) {
+	var resp struct {
+		SecurityGroup SecurityGroupV2 `json:"security_group"`
+	}
+	url := fmt.Sprintf("%s/%s", ApiSecurityGroupsV2, groupId)
+	requestData := goosehttp.RequestData{ExpectedStatus: []int{http.StatusOK}, RespValue: &resp}
+	err := c.client.SendRequest(client.GET, "network", "v2.0", url, &requestData)
+	if err != nil {
+		return nil, errors.Newf(err, "failed to show security group with ID: %s", groupId)
+	}
+
+	return &resp.SecurityGroup, nil
+}
+
 // UpdateSecurityGroupV2 updates the name and description of the given group.
-func (c *Client) UpdateSecurityGroupV2(groupId, name, description string) (*SecurityGroupV2, error) {
+func (c *Client) updateSecurityGroupV2(groupId, name, description string) (*SecurityGroupV2, error) {
 	var req struct {
 		SecurityGroupV2 struct {
 			Name        string `json:"name"`
@@ -488,6 +503,41 @@ func (c *Client) UpdateSecurityGroupV2(groupId, name, description string) (*Secu
 	return &resp.SecurityGroup, nil
 }
 
+// UpdateSecurityGroupV2 updates the name, description, and tags (if specified) of the given group.
+// When tags are passed, it will overwrite the existing tags that are attached to the group.
+func (c *Client) UpdateSecurityGroupV2(groupId, name, description string, tags []string) (*SecurityGroupV2, error) {
+	existingGroup, err := c.GetSecurityGroupV2(groupId)
+	if err != nil {
+		return nil, errors.Newf(err, "fetching an existing security group failed with id: %s", groupId)
+	}
+	oldName := existingGroup.Name
+	oldDescription := existingGroup.Description
+
+	securityGroup, err := c.updateSecurityGroupV2(groupId, name, description)
+	if err != nil {
+		return nil, errors.Newf(err, "updating a security group failed with id: %s", groupId)
+	}
+
+	if len(tags) == 0 {
+		return securityGroup, nil
+	}
+
+	// Updates the tags for the group.
+	err = c.ReplaceAllTags("security-groups", securityGroup.Id, tags)
+	if err != nil {
+		// Rollback to old name and description if updating tags failed.
+		if _, updateErr := c.updateSecurityGroupV2(groupId, oldName, oldDescription); updateErr != nil {
+			return nil, errors.Newf(updateErr, "updating tags and attempt to roll back failed for security group with id: %s", securityGroup.Id)
+		}
+
+		return nil, errors.Newf(err, "updating tags failed, rolled back security group with id: %s", securityGroup.Id)
+	}
+
+	securityGroup.Tags = tags
+
+	return securityGroup, nil
+}
+
 // RuleInfoV2 allows the callers of CreateSecurityGroupRuleV2() to
 // create 2 types of security group rules: ingress rules and egress
 // rules. Security Groups are applied on neutron ports.

testservices/neutronmodel/neutronmodel.go

@@ -177,7 +177,22 @@ func (n *NeutronModel) UpdateSecurityGroup(group neutron.SecurityGroupV2) error
 	}
 	existingGroup.Name = group.Name
 	existingGroup.Description = group.Description
-	existingGroup.Tags = group.Tags
+	n.groups[group.Id] = *existingGroup
+	return nil
+}
+
+// UpdateSecurityGroupWithTags updates an existing security group with tags given a
+// neutron.SecurityGroupRuleV2.
+func (n *NeutronModel) UpdateSecurityGroupWithTags(group neutron.SecurityGroupV2, tags []string) error {
+	n.rwMu.Lock()
+	defer n.rwMu.Unlock()
+	existingGroup, err := n.SecurityGroup(group.Id)
+	if err != nil {
+		return testservices.NewSecurityGroupByIDNotFoundError(group.Id)
+	}
+	existingGroup.Name = group.Name
+	existingGroup.Description = group.Description
+	existingGroup.Tags = tags
 	n.groups[group.Id] = *existingGroup
 	return nil
 }
@@ -232,8 +247,7 @@ func (n *NeutronModel) AddTagsToSecurityGroup(groupId string, tags []string) err
 	if group == nil {
 		return testservices.NewNotFoundError(fmt.Sprintf("Resource security_groups %s could not be found.", groupId))
 	}
-	group.Tags = tags
-	return n.UpdateSecurityGroup(*group)
+	return n.UpdateSecurityGroupWithTags(*group, tags)
 }
 
 // AddNovaSecurityGroup creates a new security group given a nova.SecurityGroup.