Skip to content
This repository has been archived by the owner on Jun 21, 2024. It is now read-only.

IPReservationRequest: change type of attribute CustomData to interface{} #221

Merged
merged 3 commits into from
Dec 9, 2020
+37 −30
Merged

IPReservationRequest: change type of attribute CustomData to interface{} #221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
22c672f
IPReservationRequest: change CustomData from map[string]interface{} t…
t0mk Dec 3, 2020
f3f6c9e
IPReservationRequest: change CustomData type to interface{}
t0mk Dec 4, 2020
1404104
change type of testing customdata value from string to map[string]int…
t0mk Dec 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 23 additions & 23 deletions ip.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,23 +39,23 @@ type ProjectIPService interface {
}

type IpAddressCommon struct { //nolint:golint
ID string `json:"id"`
Address string `json:"address"`
Gateway string `json:"gateway"`
Network string `json:"network"`
AddressFamily int `json:"address_family"`
Netmask string `json:"netmask"`
Public bool `json:"public"`
CIDR int `json:"cidr"`
Created string `json:"created_at,omitempty"`
Updated string `json:"updated_at,omitempty"`
Href string `json:"href"`
Management bool `json:"management"`
Manageable bool `json:"manageable"`
Project Href `json:"project"`
Global *bool `json:"global_ip"`
Tags []string `json:"tags,omitempty"`
CustomData map[string]interface{} `json:"customdata,omitempty"`
ID string `json:"id"`
Address string `json:"address"`
Gateway string `json:"gateway"`
Network string `json:"network"`
AddressFamily int `json:"address_family"`
Netmask string `json:"netmask"`
Public bool `json:"public"`
CIDR int `json:"cidr"`
Created string `json:"created_at,omitempty"`
Updated string `json:"updated_at,omitempty"`
Href string `json:"href"`
Management bool `json:"management"`
Manageable bool `json:"manageable"`
Project Href `json:"project"`
Global *bool `json:"global_ip"`
Tags []string `json:"tags,omitempty"`
CustomData interface{} `json:"customdata,omitempty"`
}

// IPAddressReservation is created when user sends IP reservation request for a project (considering it's within quota).
Expand Down Expand Up @@ -87,12 +87,12 @@ type IPAddressAssignment struct {

// IPReservationRequest represents the body of a reservation request.
type IPReservationRequest struct {
Type string `json:"type"`
Quantity int `json:"quantity"`
Description string `json:"details,omitempty"`
Facility *string `json:"facility,omitempty"`
Tags []string `json:"tags,omitempty"`
CustomData map[string]interface{} `json:"customdata,omitempty"`
Type string `json:"type"`
Quantity int `json:"quantity"`
Description string `json:"details,omitempty"`
Facility *string `json:"facility,omitempty"`
Tags []string `json:"tags,omitempty"`
CustomData interface{} `json:"customdata,omitempty"`
// FailOnApprovalRequired if the IP request cannot be approved automatically, rather than sending to
// the longer Equinix Metal approval process, fail immediately with a 422 error
FailOnApprovalRequired bool `json:"fail_on_approval_required,omitempty"`
Expand Down
21 changes: 14 additions & 7 deletions ip_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,15 @@ func TestAccPublicIPReservation(t *testing.T) {
}

customData := map[string]interface{}{"custom1": "data", "custom2": map[string]interface{}{"nested": "data"}}
tags := []string{"Tag1", "Tag2"}

req := IPReservationRequest{
Type: PublicIPv4,
Quantity: quantity,
Facility: &testFac,
CustomData: customData,
Type: PublicIPv4,
Quantity: quantity,
Facility: &testFac,
CustomData: &customData,
Tags: tags,
FailOnApprovalRequired: true,
}

res, _, err := c.ProjectIPs.Request(projectID, &req)
Expand Down Expand Up @@ -61,8 +64,12 @@ func TestAccPublicIPReservation(t *testing.T) {
}

if !reflect.DeepEqual(customData, res.CustomData) {
t.Fatalf("CustomData of new reservation should be %+v, was %+v", customData, res.CustomData)
}

if !reflect.DeepEqual(tags, res.Tags) {
t.Fatalf(
"CustomData of new reservation should be %+v, was %+v", customData, res.CustomData)
"Tags of new reservation should be %+v, was %+v", tags, res.Tags)
}

ipList, _, err = c.ProjectIPs.List(projectID, &ListOptions{})
Expand Down Expand Up @@ -199,7 +206,7 @@ func TestAccGlobalIPReservation(t *testing.T) {
}
}

func TestPublicIPReservationFailFast(t *testing.T) {
func TestAccPublicIPReservationFailFast(t *testing.T) {
skipUnlessAcceptanceTestsAllowed(t)

c, projectID, teardown := setupWithProject(t)
Expand All @@ -215,7 +222,7 @@ func TestPublicIPReservationFailFast(t *testing.T) {
Type: PublicIPv4,
Quantity: quantity,
Facility: &testFac,
CustomData: customData,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The map[string]interface{} tests should still work, do they not? These would save JSON structures in the EM API, and would result in JSON structures in return.

The updated tests will be storing strings and I assume receiving strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we established that there can be non-map types coming from the EM API (strings, lists, numbers). Then, map[string]interface{} woulndn't work I think.

panic: interface conversion: interface {} is string, not map[string]interface {}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@t0mk you can ignore that comment, I believe it is from before the investigation and discovery of how customdata is stored and delivered.

Copy link
Contributor

@displague displague Dec 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I misunderstood myself :-)

I mean, we should still be able to test with: customData := map[string]interface{}{"custom1": "data", "custom2": map[string]interface{}{"nested": "data"}}. This would verify that arbitrary json structures are stored in their native format and returned the same.

By testing with:

customData := `{"custom1":"data","custom2":{"nested":"data"}}`

we are verifying that scalars (string) work, and we could test with a simpler string foo to prove that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both tests should be valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you mean the concrete testing value in the Public IP reservation test. You're right that nested map[string]interface{} is more fitting here, thanks. I overlooked that there was still a string. I fixed it.

I also changed the name of this PR.

CustomData: &customData,
FailOnApprovalRequired: true,
}

Expand Down