Skip to content

Commit 63e3079

Browse files
lentzi90lentzi90
committed
Fix value_specs implementation
The current implementation does not do anything useful. The key-pairs must be extracted and added directly in the body instead of under "value_specs". This commit fixes the implementation and also adds validation of the value_specs based on what is done in Heat. There are some banned keys that are not allowed in value_specs, and values are not allowed to overwrite existing fields either. Signed-off-by: Lennart Jern <lennart.jern@est.tech>
1 parent 9f29754 commit 63e3079

4 files changed

Lines changed: 91 additions & 23 deletions

File tree

‎openstack/networking/v2/ports/requests.go‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package ports
33
import (
44
"fmt"
55
"net/url"
6+
"slices"
67

78
"github.com/gophercloud/gophercloud"
89
"github.com/gophercloud/gophercloud/pagination"
@@ -130,7 +131,37 @@ type CreateOpts struct {
130131

131132
// ToPortCreateMap builds a request body from CreateOpts.
132133
func (opts CreateOpts) ToPortCreateMap() (map[string]interface{}, error) {
133-
return gophercloud.BuildRequestBody(opts, "port")
134+
body, err := gophercloud.BuildRequestBody(opts, "port")
135+
if err != nil {
136+
return nil, err
137+
}
138+
139+
return AddValueSpecs(body)
140+
}
141+
142+
// AddValueSpecs expands the 'value_specs' object and removes 'value_specs'
143+
// from the request body. It will return error if the value specs would overwrite
144+
// an existing field or contains forbidden keys.
145+
func AddValueSpecs(body map[string]interface{}) (map[string]interface{}, error) {
146+
// Banned the same as in heat. See https://github.com/openstack/heat/blob/dd7319e373b88812cb18897f742b5196a07227ea/heat/engine/resources/openstack/neutron/neutron.py#L59
147+
bannedKeys := []string{"shared", "tenant_id"}
148+
port := body["port"].(map[string]interface{})
149+
150+
if port["value_specs"] != nil {
151+
for k, v := range port["value_specs"].(map[string]interface{}) {
152+
if slices.Contains(bannedKeys, k) {
153+
return nil, fmt.Errorf("forbidden key in value_specs: %s", k)
154+
}
155+
if _, ok := port[k]; ok {
156+
return nil, fmt.Errorf("value_specs would overwrite key: %s", k)
157+
}
158+
port[k] = v
159+
}
160+
delete(port, "value_specs")
161+
}
162+
body["port"] = port
163+
164+
return body, nil
134165
}
135166

136167
// Create accepts a CreateOpts struct and creates a new network using the values
@@ -173,7 +204,11 @@ type UpdateOpts struct {
173204

174205
// ToPortUpdateMap builds a request body from UpdateOpts.
175206
func (opts UpdateOpts) ToPortUpdateMap() (map[string]interface{}, error) {
176-
return gophercloud.BuildRequestBody(opts, "port")
207+
body, err := gophercloud.BuildRequestBody(opts, "port")
208+
if err != nil {
209+
return nil, err
210+
}
211+
return AddValueSpecs(body)
177212
}
178213

179214
// Update accepts a UpdateOpts struct and updates an existing port using the

‎openstack/networking/v2/ports/results.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,6 @@ type Port struct {
114114
// PropagateUplinkStatus enables/disables propagate uplink status on the port.
115115
PropagateUplinkStatus bool `json:"propagate_uplink_status"`
116116

117-
// Extra parameters to include in the request.
118-
ValueSpecs map[string]string `json:"value_specs"`
119-
120117
// RevisionNumber optionally set via extensions/standard-attr-revisions
121118
RevisionNumber int `json:"revision_number"`
122119

‎openstack/networking/v2/ports/testing/fixtures.go‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -299,9 +299,7 @@ const CreateValueSpecRequest = `
299299
"mac_address": "fa:16:3e:c9:cb:f0"
300300
}
301301
],
302-
"value_specs": {
303-
"key": "value"
304-
}
302+
"test": "value"
305303
}
306304
}
307305
`
@@ -332,9 +330,6 @@ const CreateValueSpecResponse = `
332330
"mac_address": "fa:16:3e:c9:cb:f0"
333331
}
334332
],
335-
"value_specs": {
336-
"key": "value"
337-
},
338333
"device_id": ""
339334
}
340335
}
@@ -544,9 +539,7 @@ const UpdatePropagateUplinkStatusResponse = `
544539
const UpdateValueSpecsRequest = `
545540
{
546541
"port": {
547-
"value_specs": {
548-
"key": "value"
549-
}
542+
"test": "update"
550543
}
551544
}
552545
`
@@ -577,9 +570,6 @@ const UpdateValueSpecsResponse = `
577570
"security_groups": [
578571
"f0ac4394-7e4a-4409-9701-ba8be283dbc3"
579572
],
580-
"value_specs": {
581-
"key": "value"
582-
},
583573
"device_id": ""
584574
}
585575
}

‎openstack/networking/v2/ports/testing/requests_test.go‎

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ func TestCreateWithValueSpecs(t *testing.T) {
389389
{IPAddress: "10.0.0.4", MACAddress: "fa:16:3e:c9:cb:f0"},
390390
},
391391
ValueSpecs: &map[string]string{
392-
"key": "value",
392+
"test": "value",
393393
},
394394
}
395395
n, err := ports.Create(fake.ServiceClient(), options).Extract()
@@ -410,7 +410,55 @@ func TestCreateWithValueSpecs(t *testing.T) {
410410
th.AssertDeepEquals(t, n.AllowedAddressPairs, []ports.AddressPair{
411411
{IPAddress: "10.0.0.4", MACAddress: "fa:16:3e:c9:cb:f0"},
412412
})
413-
th.AssertDeepEquals(t, n.ValueSpecs, map[string]string{"key": "value"})
413+
}
414+
415+
func TestCreateWithInvalidValueSpecs(t *testing.T) {
416+
th.SetupHTTP()
417+
defer th.TeardownHTTP()
418+
419+
th.Mux.HandleFunc("/v2.0/ports", func(w http.ResponseWriter, r *http.Request) {
420+
th.TestMethod(t, r, "POST")
421+
th.TestHeader(t, r, "X-Auth-Token", fake.TokenID)
422+
th.TestHeader(t, r, "Content-Type", "application/json")
423+
th.TestHeader(t, r, "Accept", "application/json")
424+
th.TestJSONRequest(t, r, CreateValueSpecRequest)
425+
426+
w.Header().Add("Content-Type", "application/json")
427+
w.WriteHeader(http.StatusCreated)
428+
429+
fmt.Fprintf(w, CreateValueSpecResponse)
430+
})
431+
432+
asu := true
433+
options := ports.CreateOpts{
434+
Name: "private-port",
435+
AdminStateUp: &asu,
436+
NetworkID: "a87cc70a-3e15-4acf-8205-9b711a3531b7",
437+
FixedIPs: []ports.IP{
438+
{SubnetID: "a0304c3a-4f08-4c43-88af-d796509c97d2", IPAddress: "10.0.0.2"},
439+
},
440+
SecurityGroups: &[]string{"foo"},
441+
AllowedAddressPairs: []ports.AddressPair{
442+
{IPAddress: "10.0.0.4", MACAddress: "fa:16:3e:c9:cb:f0"},
443+
},
444+
ValueSpecs: &map[string]string{
445+
// This is a forbidden key
446+
"shared": "value",
447+
},
448+
}
449+
450+
// We expect an error here since we used a fobidden key in the value specs.
451+
_, err := ports.Create(fake.ServiceClient(), options).Extract()
452+
th.AssertErr(t, err)
453+
454+
options.ValueSpecs = &map[string]string{
455+
// Try to overwrite an existing field
456+
"name": "overwrite",
457+
}
458+
459+
// We expect an error here since the value specs would overwrite an existing field.
460+
_, err = ports.Create(fake.ServiceClient(), options).Extract()
461+
th.AssertErr(t, err)
414462
}
415463

416464
func TestRequiredCreateOpts(t *testing.T) {
@@ -598,14 +646,12 @@ func TestUpdateValueSpecs(t *testing.T) {
598646

599647
options := ports.UpdateOpts{
600648
ValueSpecs: &map[string]string{
601-
"key": "value",
649+
"test": "update",
602650
},
603651
}
604652

605-
s, err := ports.Update(fake.ServiceClient(), "65c0ee9f-d634-4522-8954-51021b570b0d", options).Extract()
653+
_, err := ports.Update(fake.ServiceClient(), "65c0ee9f-d634-4522-8954-51021b570b0d", options).Extract()
606654
th.AssertNoErr(t, err)
607-
608-
th.AssertDeepEquals(t, s.ValueSpecs, map[string]string{"key": "value"})
609655
}
610656

611657
func TestUpdatePortSecurity(t *testing.T) {

0 commit comments

Comments
 (0)