Updates based on code review
diff --git a/acceptance/openstack/compute/v2/secgroup_test.go b/acceptance/openstack/compute/v2/secgroup_test.go
index 7094031..3163d88 100644
--- a/acceptance/openstack/compute/v2/secgroup_test.go
+++ b/acceptance/openstack/compute/v2/secgroup_test.go
@@ -87,7 +87,7 @@
}
func addRemoveRules(t *testing.T, client *gophercloud.ServiceClient, id string) {
- opts := secgroups.AddRuleOpts{
+ opts := secgroups.CreateRuleOpts{
ParentGroupID: id,
FromPort: 22,
ToPort: 22,
@@ -95,7 +95,7 @@
CIDR: "0.0.0.0/0",
}
- rule, err := secgroups.AddRule(client, opts).Extract()
+ rule, err := secgroups.CreateRule(client, opts).Extract()
th.AssertNoErr(t, err)
t.Logf("Adding rule %s to group %s", rule.ID, id)
diff --git a/openstack/compute/v2/extensions/secgroups/requests.go b/openstack/compute/v2/extensions/secgroups/requests.go
index 58ec6a8..3147310 100644
--- a/openstack/compute/v2/extensions/secgroups/requests.go
+++ b/openstack/compute/v2/extensions/secgroups/requests.go
@@ -45,13 +45,34 @@
// CreateOpts is the struct responsible for creating a security group.
type CreateOpts GroupOpts
+// CreateOptsBuilder builds the create options into a serializable format.
+type CreateOptsBuilder interface {
+ ToSecGroupCreateMap() (map[string]interface{}, error)
+}
+
+// ToSecGroupCreateMap builds the create options into a serializable format.
+func (opts CreateOpts) ToSecGroupCreateMap() (map[string]interface{}, error) {
+ sg := make(map[string]interface{})
+
+ if opts.Name != "" {
+ sg["name"] = opts.Name
+ }
+ if opts.Description != "" {
+ sg["description"] = opts.Description
+ }
+
+ return map[string]interface{}{"security_group": sg}, nil
+}
+
// Create will create a new security group.
-func Create(client *gophercloud.ServiceClient, opts CreateOpts) CreateResult {
+func Create(client *gophercloud.ServiceClient, opts CreateOptsBuilder) CreateResult {
var result CreateResult
- reqBody := struct {
- CreateOpts `json:"security_group"`
- }{opts}
+ reqBody, err := opts.ToSecGroupCreateMap()
+ if err != nil {
+ result.Err = err
+ return result
+ }
_, result.Err = perigee.Request("POST", rootURL(client), perigee.Options{
Results: &result.Body,
@@ -66,14 +87,35 @@
// UpdateOpts is the struct responsible for updating an existing security group.
type UpdateOpts GroupOpts
+// UpdateOptsBuilder builds the update options into a serializable format.
+type UpdateOptsBuilder interface {
+ ToSecGroupUpdateMap() (map[string]interface{}, error)
+}
+
+// ToSecGroupUpdateMap builds the update options into a serializable format.
+func (opts UpdateOpts) ToSecGroupUpdateMap() (map[string]interface{}, error) {
+ sg := make(map[string]interface{})
+
+ if opts.Name != "" {
+ sg["name"] = opts.Name
+ }
+ if opts.Description != "" {
+ sg["description"] = opts.Description
+ }
+
+ return map[string]interface{}{"security_group": sg}, nil
+}
+
// Update will modify the mutable properties of a security group, notably its
// name and description.
-func Update(client *gophercloud.ServiceClient, id string, opts UpdateOpts) UpdateResult {
+func Update(client *gophercloud.ServiceClient, id string, opts UpdateOptsBuilder) UpdateResult {
var result UpdateResult
- reqBody := struct {
- UpdateOpts `json:"security_group"`
- }{opts}
+ reqBody, err := opts.ToSecGroupUpdateMap()
+ if err != nil {
+ result.Err = err
+ return result
+ }
_, result.Err = perigee.Request("PUT", resourceURL(client, id), perigee.Options{
Results: &result.Body,
@@ -110,9 +152,9 @@
return result
}
-// AddRuleOpts represents the configuration for adding a new rule to an
+// CreateRuleOpts represents the configuration for adding a new rule to an
// existing security group.
-type AddRuleOpts struct {
+type CreateRuleOpts struct {
// Required - the ID of the group that this rule will be added to.
ParentGroupID string `json:"parent_group_id"`
@@ -137,36 +179,57 @@
FromGroupID string `json:"group_id,omitempty"`
}
-// AddRule will add a new rule to an existing security group (whose ID is
-// specified in AddRuleOpts). You have the option of controlling inbound
-// traffic from both an IP range (CIDR) or from another security group.
-func AddRule(client *gophercloud.ServiceClient, opts AddRuleOpts) AddRuleResult {
- var result AddRuleResult
+// CreateRuleOptsBuilder builds the create rule options into a serializable format.
+type CreateRuleOptsBuilder interface {
+ ToRuleCreateMap() (map[string]interface{}, error)
+}
+
+// ToRuleCreateMap builds the create rule options into a serializable format.
+func (opts CreateRuleOpts) ToRuleCreateMap() (map[string]interface{}, error) {
+ rule := make(map[string]interface{})
if opts.ParentGroupID == "" {
- result.Err = errors.New("A ParentGroupID must be set")
- return result
+ return rule, errors.New("A ParentGroupID must be set")
}
if opts.FromPort == 0 {
- result.Err = errors.New("A FromPort must be set")
- return result
+ return rule, errors.New("A FromPort must be set")
}
if opts.ToPort == 0 {
- result.Err = errors.New("A ToPort must be set")
- return result
+ return rule, errors.New("A ToPort must be set")
}
if opts.IPProtocol == "" {
- result.Err = errors.New("A IPProtocol must be set")
- return result
+ return rule, errors.New("A IPProtocol must be set")
}
if opts.CIDR == "" && opts.FromGroupID == "" {
- result.Err = errors.New("A CIDR or FromGroupID must be set")
- return result
+ return rule, errors.New("A CIDR or FromGroupID must be set")
}
- reqBody := struct {
- AddRuleOpts `json:"security_group_rule"`
- }{opts}
+ rule["parent_group_id"] = opts.ParentGroupID
+ rule["from_port"] = opts.FromPort
+ rule["to_port"] = opts.ToPort
+ rule["ip_protocol"] = opts.IPProtocol
+
+ if opts.CIDR != "" {
+ rule["cidr"] = opts.CIDR
+ }
+ if opts.FromGroupID != "" {
+ rule["from_group_id"] = opts.FromGroupID
+ }
+
+ return map[string]interface{}{"security_group_rule": rule}, nil
+}
+
+// CreateRule will add a new rule to an existing security group (whose ID is
+// specified in CreateRuleOpts). You have the option of controlling inbound
+// traffic from either an IP range (CIDR) or from another security group.
+func CreateRule(client *gophercloud.ServiceClient, opts CreateRuleOptsBuilder) CreateRuleResult {
+ var result CreateRuleResult
+
+ reqBody, err := opts.ToRuleCreateMap()
+ if err != nil {
+ result.Err = err
+ return result
+ }
_, result.Err = perigee.Request("POST", rootRuleURL(client), perigee.Options{
Results: &result.Body,
diff --git a/openstack/compute/v2/extensions/secgroups/requests_test.go b/openstack/compute/v2/extensions/secgroups/requests_test.go
index 37acd36..f2f6eb4 100644
--- a/openstack/compute/v2/extensions/secgroups/requests_test.go
+++ b/openstack/compute/v2/extensions/secgroups/requests_test.go
@@ -174,7 +174,7 @@
mockAddRuleResponse(t)
- opts := AddRuleOpts{
+ opts := CreateRuleOpts{
ParentGroupID: "b0e0d7dd-2ca4-49a9-ba82-c44a148b66a5",
FromPort: 22,
ToPort: 22,
@@ -182,7 +182,7 @@
CIDR: "0.0.0.0/0",
}
- rule, err := AddRule(client.ServiceClient(), opts).Extract()
+ rule, err := CreateRule(client.ServiceClient(), opts).Extract()
th.AssertNoErr(t, err)
expected := &Rule{
diff --git a/openstack/compute/v2/extensions/secgroups/results.go b/openstack/compute/v2/extensions/secgroups/results.go
index 8610bd5..aaf5052 100644
--- a/openstack/compute/v2/extensions/secgroups/results.go
+++ b/openstack/compute/v2/extensions/secgroups/results.go
@@ -21,7 +21,7 @@
// The rules which determine how this security group operates.
Rules []Rule
- // The ID of the tenant to which this security group belongs to.
+ // The ID of the tenant to which this security group belongs.
TenantID string `mapstructure:"tenant_id"`
}
@@ -43,7 +43,7 @@
// The CIDR IP range whose traffic can be received
IPRange IPRange `mapstructure:"ip_range"`
- // The security group ID which this rule belongs to
+ // The security group ID to which this rule belongs
ParentGroupID string `mapstructure:"parent_group_id"`
// Not documented.
@@ -106,6 +106,7 @@
commonResult
}
+// Extract will extract a SecurityGroup struct from most responses.
func (r commonResult) Extract() (*SecurityGroup, error) {
if r.Err != nil {
return nil, r.Err
@@ -120,13 +121,13 @@
return &response.SecurityGroup, err
}
-// AddRuleResult represents the result when adding rules to a security group.
-type AddRuleResult struct {
+// CreateRuleResult represents the result when adding rules to a security group.
+type CreateRuleResult struct {
gophercloud.Result
}
-// Extract will extract a Rule struct from an AddResultRule.
-func (r AddRuleResult) Extract() (*Rule, error) {
+// Extract will extract a Rule struct from a CreateRuleResult.
+func (r CreateRuleResult) Extract() (*Rule, error) {
if r.Err != nil {
return nil, r.Err
}