TPT-4277: Implement support for Reserved IP for IPv4#919
TPT-4277: Implement support for Reserved IP for IPv4#919mgwoj wants to merge 7 commits intolinode:proj/reserved-ipsfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Implements remaining Go SDK (linodego) support for the Reserved IPv4 feature to better align with the API spec, primarily by expanding reserved IP metadata support and adding unit coverage.
Changes:
- Add
reserved_ipv4_addressessupport to tag creation and decode tagged objects of typereserved_ipv4_address. - Add reserved IP tags support plus new endpoints for updating a reserved IP and listing reserved IP types (pricing).
- Add/extend unit tests and fixtures to validate request bodies and response decoding for reserved IP behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tags.go |
Adds tag create option for reserved IPv4 addresses and decodes tagged reserved IP objects. |
network_reserved_ips.go |
Adds tag support for reserving IPs, plus update + types-list endpoints and related models. |
instance_ips.go |
Extends InstanceIP with tags field to capture reserved IP tags. |
test/unit/tag_test.go |
Adds unit coverage for tag create with reserved IPv4 addresses and tagged-object decoding. |
test/unit/nodebalancer_test.go |
Adds request-body validation test for NodeBalancer create with ipv4. |
test/unit/network_reserved_ips_test.go |
Extends reserved IP tests to cover tags, update, and type listing. |
test/unit/network_ips_test.go |
Adds request-body validation for reserve allocation and combined RDNS+reserved update. |
test/unit/instance_test.go |
Adds request-body validation for instance create with explicit IPv4 list. |
test/unit/instance_ip_test.go |
Adds request-body validation for assigning a reserved IP to an instance. |
test/unit/fixtures/tagged_objects_reserved_ip_list.json |
Adds fixture for tagged reserved IPv4 address objects. |
test/unit/fixtures/network_reserved_ips_list.json |
Updates reserved IP list fixture with reserved and tags. |
test/unit/fixtures/network_reserved_ips_get.json |
Updates reserved IP get fixture with reserved and tags. |
test/unit/fixtures/network_reserved_ips.json |
Updates reserve-IP create fixture with reserved and tags. |
test/unit/fixtures/network_reserved_ip_update.json |
Adds fixture for reserved IP update response. |
test/unit/fixtures/network_reserved_ip_types_list.json |
Adds fixture for reserved IP types (pricing) list response. |
Comments suppressed due to low confidence (1)
test/unit/fixtures/network_reserved_ips_list.json:26
- This list fixture is missing the standard "page" field used by paginated list responses in this repo's fixtures. If this fixture is used with getPaginatedResults, pagination decoding may fail or behave inconsistently; add "page": 1 to match other list fixtures.
],
"pages": 1,
"results": 2
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 30 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // UpdateReservedIPOptions represents the options for updating a reserved IP address | ||
| // NOTE: Reserved IP feature may not currently be available to all users. | ||
| type UpdateReservedIPOptions struct { | ||
| Tags []string `json:"tags"` | ||
| } |
There was a problem hiding this comment.
UpdateReservedIPOptions.Tags is a non-pointer slice without omitempty, so the zero value (nil) will marshal as "tags":null. If the API expects an array (even when clearing), this can cause request validation errors or unintended semantics. Consider using Tags *[]string json:"tags,omitempty"`` (consistent with other update options in the SDK) or validate that Tags is non-nil before issuing the PUT.
There was a problem hiding this comment.
You can use omitzero instead of omitempty for any list to make sure empty slice are correctly marshalled.
There was a problem hiding this comment.
Agree, this is the way to go
yec-akamai
left a comment
There was a problem hiding this comment.
Overall looks good. Is the security vulnerability related to this change?
No, it is not related at all |
| defer teardown() | ||
|
|
||
| resIP, err := client.ReserveIPAddress(context.Background(), linodego.ReserveIPOptions{ | ||
| Region: "us-east", |
There was a problem hiding this comment.
Hardcoded region, it will fail on DevCloud as there is only pl-labkrk-2 available. Maybe it is worth to use getRegionsWithCaps instead?
|
|
||
| // Reserve without tags first | ||
| resIP, err := client.ReserveIPAddress(context.Background(), linodego.ReserveIPOptions{ | ||
| Region: "us-east", |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
e150bbf to
1012645
Compare
📝 Description
Complete the Go SDK (linodego) support for the Reserved IP feature. Several foundations are already in place; this story addresses the remaining gaps to bring the SDK fully in line with the API Specification.
✔️ How to Test
As the API is not ready yet, there are only mocked integration tests.