TPT-4432: Review and implement integration tests for ReservedIPs for IPv4#945
Open
mawilk90 wants to merge 10 commits into
Conversation
Contributor
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.
Comment on lines
+107
to
+112
| 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Description
Refactor of existing and new integration tests for ReservedIPs
✔️ How to Test