TPT-4432: Review and implement integration tests for ReservedIPs for IPv4#945
Conversation
There was a problem hiding this comment.
Pull request overview
Adds/updates Linodego support and test coverage around Reserved IPv4 functionality, while also refreshing several related networking/monitoring/firewall models and fixtures to match newer API responses.
Changes:
- Adds Reserved IPv4 tagging support (request/response fields + tag object handling) and expands unit/integration coverage around reserved IP flows.
- Updates Monitor Alert Definition/Channel models and adds entity-listing support + tests/fixtures.
- Updates firewall and interface-related request/response models and refreshes fixtures; also centralizes log header redaction behavior.
Reviewed changes
Copilot reviewed 44 out of 102 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| volumes.go | Formatting/comment adjustment for volume encryption field. |
| tags.go | Adds ReservedIPv4Addresses to tag create options; decodes reserved IP tagged objects. |
| network_reserved_ips.go | Adds tags/update/types support for reserved IPs. |
| monitor_alert_definitions.go | Adds scope/regions/entities modeling + entity listing endpoint support. |
| monitor_alert_channels.go | Removes deprecated content modeling; relies on details. |
| lke_node_pools.go | Adds disk_encryption to node pool create options and sets it in GetCreateOptions. |
| interfaces.go | Changes LinodeInterfaceCreateOptions.FirewallID pointer type. |
| instances.go | Reorders/expands InstanceCreateOptions (disk_encryption, placement_group) and formatting updates. |
| instance_ips.go | Adds Tags and AssignedEntity to InstanceIP. |
| instance_disks.go | Formatting update around disk_encryption field. |
| firewalls.go | Adds Entities to Firewall; renames devices field to LinodeInterfaces for create. |
| firewall_devices.go | Adds ParentEntity to FirewallDeviceEntity. |
| client.go | Adds reusable header redaction helpers; expands sanitization to response logs too. |
| client_test.go | Adds tests for header redaction and log sanitization. |
| CODEOWNERS | Adds dx-sdets as codeowner. |
| go.mod | Bumps golang.org/x deps. |
| go.sum | Updates checksums for bumped deps. |
| k8s/go.mod | Bumps golang.org/x deps (indirect). |
| k8s/go.sum | Updates checksums for bumped deps (indirect). |
| test/go.mod | Bumps golang.org/x deps for test module. |
| test/go.sum | Updates checksums for test module deps. |
| test/unit/tag_test.go | Adds unit tests for tag creation/listing with reserved IP objects and request body validation. |
| test/unit/nodebalancer_test.go | Adds request body validation for NodeBalancer IPv4 field. |
| test/unit/network_reserved_ips_test.go | Expands reserved IP tests to cover tags/assigned_entity/update/types. |
| test/unit/network_ips_test.go | Adds request body validation for IP update/allocate reserve and filter-by-reserved list. |
| test/unit/monitor_alert_definitions_test.go | Updates fixtures/assertions and adds unit test for listing alert definition entities. |
| test/unit/monitor_alert_channels_test.go | Adds unit test for listing monitor alert channels. |
| test/unit/interface_test.go | Updates firewall ID pointer usage in interface create test. |
| test/unit/instance_test.go | Adds request body validation for instance create with reserved IPv4. |
| test/unit/instance_ip_test.go | Adds assertions and request body validation for assigning reserved IP to instance. |
| test/unit/firewalls_test.go | Updates firewall create devices field name and validates entities/created/updated parsing. |
| test/unit/firewall_devices_test.go | Adds parent_entity assertions and a new test covering parent_entity responses. |
| test/unit/fixtures/tagged_objects_reserved_ip_list.json | Adds fixture for reserved IP tagged objects. |
| test/unit/fixtures/network_reserved_ips.json | Updates reserved IP fixture to include reserved/tags. |
| test/unit/fixtures/network_reserved_ips_list.json | Updates list fixture to include reserved/tags. |
| test/unit/fixtures/network_reserved_ips_get.json | Updates get fixture to include reserved/tags/assigned_entity. |
| 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 list. |
| test/unit/fixtures/instance_ip_reserved.json | Updates reserved instance IP fixture with reserved + assigned_entity. |
| test/unit/fixtures/firewall_create.json | Adds entities block to firewall create fixture. |
| test/integration/TestReservedIPAddresses_GetInstanceIPReservationStatus.yaml | Removes old reserved IP reservation status fixture. |
| test/integration/tags_test.go | Adds integration test for tagging a reserved IP. |
| test/integration/network_reserved_ips_test.go | Adds/updates integration tests for reserving with tags, updating tags, listing types, and region selection. |
| test/integration/network_ips_test.go | Adds tag presence assertions for reserved/unreserved lists. |
| test/integration/monitor_alert_definitions_test.go | Adds assertions for email details and entity listing test. |
| test/integration/lke_node_pools_test.go | Updates disk encryption expectations and create options. |
| test/integration/instance_reserved_ips_test.go | Updates reserved IP instance tests to be region-aware and adds robustness/cleanup behavior. |
| test/integration/instance_interfaces_test.go | Updates firewall ID pointer usage for interface create options. |
| test/integration/fixtures/TestReservedIPTypes_List.yaml | Adds recorded fixture for reserved IP types list. |
| test/integration/fixtures/TestMonitorAlertDefinitions_List.yaml | Updates recorded fixture for alert definitions list. |
| test/integration/fixtures/TestMonitorAlertDefinitionEntities_List.yaml | Adds recorded fixture for alert definition entities list. |
| test/integration/fixtures/TestMonitorAlertDefinition.yaml | Updates recorded fixture for alert definition CRUD flow. |
| test/integration/fixtures/TestMonitorAlertDefinition_CreateWithIdempotency.yaml | Updates recorded fixture for idempotent create flow. |
| test/integration/fixtures/TestMonitorAlertChannels_List.yaml | Updates recorded fixture for alert channels list. |
| test/integration/fixtures/TestLKENodePool_GetMissing.yaml | Updates recorded fixture headers/timestamps. |
| test/integration/fixtures/TestLKENodePool_GetFound_k8s.yaml | Updates recorded Kubernetes fixture (cluster/arch/metadata). |
| test/integration/fixtures/TestListMonitorAlertChannels.yaml | Removes old monitor alert channels fixture. |
| test/integration/fixtures/TestCreateMonitorAlertDefinitionWithIdempotency.yaml | Removes old idempotency fixture. |
| test/integration/fixtures/TestFirewall_Update.yaml | Updates recorded firewall update fixture (payload/ids/headers). |
| test/integration/fixtures/TestFirewall_Get.yaml | Updates recorded firewall get fixture (payload/ids/headers). |
| test/integration/firewalls_test.go | Adds entity-count assertions for firewall get/update. |
| test/integration/firewalls_devices_test.go | Adds entity/parent_entity assertions for firewall devices flows. |
| .github/workflows/release-notify-slack.yml | Updates Slack action major version. |
| .github/workflows/nightly_smoke_tests.yml | Updates Slack action major version. |
| .github/workflows/clean-release-notes.yml | Adds workflow to strip ticket prefixes from published release notes. |
| .github/workflows/ci.yml | Adds PR title validation and updates Slack action major version. |
Comments suppressed due to low confidence (2)
monitor_alert_channels.go:36
- Removing the exported
Contentfield fromAlertChannelis a breaking API change for SDK consumers who may still rely on it (even if the API no longer returns it). Consider keepingContentas a deprecated/optional field (or preserving the raw JSON) to maintain backwards compatibility while transitioning callers toDetails.
// AlertChannel represents a Monitor Channel object.
type AlertChannel struct {
Alerts AlertsInfo `json:"alerts"`
ChannelType AlertNotificationType `json:"channel_type"`
Details ChannelDetails `json:"details"`
Created *time.Time `json:"-"`
CreatedBy string `json:"created_by"`
Updated *time.Time `json:"-"`
UpdatedBy string `json:"updated_by"`
ID int `json:"id"`
Label string `json:"label"`
Type AlertChannelType `json:"type"`
}
interfaces.go:120
- Changing
LinodeInterfaceCreateOptions.FirewallIDfrom**intto*intremoves the ability to distinguish between an omitted field and an explicit JSON null. This is a breaking change for callers that usedDoublePointer/DoublePointerNullsemantics; consider preserving the previous behavior (e.g., via a new field/type or custom marshal logic) if null vs omitted is still meaningful to the API.
type LinodeInterfaceCreateOptions struct {
FirewallID *int `json:"firewall_id,omitempty"`
DefaultRoute *InterfaceDefaultRoute `json:"default_route,omitempty"`
Public *PublicInterfaceCreateOptions `json:"public,omitempty"`
VPC *VPCInterfaceCreateOptions `json:"vpc,omitempty"`
VLAN *VLANInterface `json:"vlan,omitempty"`
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| tagObjects, err := client.ListTaggedObjects(context.Background(), tag.Label, nil) | ||
| require.NoErrorf(t, err, "Failed to list tagged objects: %v", err) | ||
|
|
||
| if len(tagObjects) == 0 || tagObjects[0].Type != "reserved_ipv4_address" || !tagObjects[0].Data.(InstanceIP).Reserved || tagObjects[0].Data.(InstanceIP).Tags[0] != tag.Label { | ||
| t.Fatalf("Should have found Tag in tagged objects list, got %v", tagObjects) | ||
| } |
- replace hardcoded regions with getRegionsWithCaps - skip tests that will fail on DevCloud
86a65f7 to
5b7faed
Compare
| @@ -275,9 +303,16 @@ func TestInstance_AddReservedIPToInstanceVariants(t *testing.T) { | |||
| client, teardown := createTestClient(t, "fixtures/TestInstance_AddReservedIPToInstanceVariants") | |||
| defer teardown() | |||
|
|
|||
| env, _ := os.LookupEnv(linodego.APIHostVar) | |||
| if strings.Contains(env, "devcloud") { | |||
There was a problem hiding this comment.
I wonder if it would change in the future and it's worth to check it by another way, for example to verify region list.
There was a problem hiding this comment.
Probably it can stay as we run tests on PROD
|
Looks fine, good job. Tests are working for me. |
📝 Description
Refactor of existing and add new integration tests for ReservedIPs
Required tags:
can_reserve_ipreserved_ips_betareserved_ip_nodebalancer✔️ How to Test