fix: Added IPAssignments type

[codinja1188]

No description provided.

[displague]

Does it make sense to introduce an IPAddress.type enum with value IPAssignment if that field doesn't really exist in the API?

We've discussed alternative approaches elsewhere. Some of the fields that would be involved are not included in the spec today. We can also see these fields were included in packngo: https://github.com/packethost/packngo/blob/master/ip.go#L56-L130

We can see the complete result with PACKNGO_DEBUG=1 metal ip list --project-id $PROJECT_ID --include assignments -o yaml 2>&1 > /dev/null. What we see are a list of IPs (reservations and dynamically provisioned) with embedded "assignments". These assignments share some of the structure that IPs report, but have distinct fields and omissions when compared to IPs.

field	evidences	apparent purpose
addon	IPReservation	(not documented) appears to be true for all approved elastic IPs, false otherwise
bill	IPReservation	not obvious if only or all reservations would have this as true, but also appears true for approved elastic IPs, false otherwise
state		reserved and non-reserved IPs will say "created" in their success state
requested_by	IPReservation	this field seems to have a User reference (href, id, short_id) with reservations, and is null (but present) otherwise
assignments		no bearing on reserved or not, and appears to be a list of IPAssignments present in the root objects in the project IPs list
assigned_to	IPAssignment	a required field for an IPAssignment, not present in the the root objects in the project IPs list
type		Will not be present in an IPAssignment, will be present in a reservation or non-reserved IP

Based on this, expecting addon to be present (required) may be our best way to distinguish IPReservations, and assigned_to (required) when checking for IPAssignments.

[ctreatma]

In terms of identifying that a particular JSON object is an IP reservation or a VRF IP reservation, that is already in place using the type enum as specified in the upstream spec, and I don't think we should change that; the case we are covering with this patch is how to identify that a particular JSON object that is an IP reservation (or a VRF IP reservation) is not also an IP assignment.

The invalid type workaround was initially chosen because it lines up with how IPReservation and VrfIPReservation are differentiated. By specifying an invalid value for type, we provide a way for the SDK to know that an IPReservation or VrfIPReservation is not an IPAssignment, without making assumptions about the other properties that are defined for IPAssignment. Since the type is optional, the JSON for an IP assignment will still match the patched spec; since the type value specified for IPAssignment is not a value the API will return, the JSON for an IP reservation or VRF IP reservation will not match the patched spec.

Requiring the assigned_to property in the IPAssignment schema instead would be more direct, so I'm on board with that.

[codinja1188]

Here are the metal-cli test logs. check and approve the PR

Vasubabus-MacBook-Pro:metal-cli vasubabu$ make test
go test -v ./... -timeout 1000s
=== RUN   TestCli_RegisterCommands
=== RUN   TestCli_RegisterCommands/test
--- PASS: TestCli_RegisterCommands (0.00s)
    --- PASS: TestCli_RegisterCommands/test (0.00s)
PASS
ok  	github.com/equinix/metal-cli/cmd	(cached)
?   	github.com/equinix/metal-cli/cmd/metal	[no test files]
?   	github.com/equinix/metal-cli/internal/capacity	[no test files]
?   	github.com/equinix/metal-cli/internal/cli	[no test files]
?   	github.com/equinix/metal-cli/internal/completion	[no test files]
?   	github.com/equinix/metal-cli/internal/devices	[no test files]
?   	github.com/equinix/metal-cli/internal/docs	[no test files]
?   	github.com/equinix/metal-cli/internal/emdocs	[no test files]
?   	github.com/equinix/metal-cli/internal/env	[no test files]
?   	github.com/equinix/metal-cli/internal/events	[no test files]
?   	github.com/equinix/metal-cli/internal/facilities	[no test files]
?   	github.com/equinix/metal-cli/internal/gateway	[no test files]
?   	github.com/equinix/metal-cli/internal/hardware	[no test files]
?   	github.com/equinix/metal-cli/internal/init	[no test files]
?   	github.com/equinix/metal-cli/internal/ips	[no test files]
?   	github.com/equinix/metal-cli/internal/metros	[no test files]
?   	github.com/equinix/metal-cli/internal/organizations	[no test files]
?   	github.com/equinix/metal-cli/internal/os	[no test files]
?   	github.com/equinix/metal-cli/internal/outputs	[no test files]
?   	github.com/equinix/metal-cli/internal/pagination	[no test files]
?   	github.com/equinix/metal-cli/internal/plans	[no test files]
?   	github.com/equinix/metal-cli/internal/ports	[no test files]
?   	github.com/equinix/metal-cli/internal/projects	[no test files]
?   	github.com/equinix/metal-cli/internal/ssh	[no test files]
?   	github.com/equinix/metal-cli/internal/twofa	[no test files]
?   	github.com/equinix/metal-cli/internal/users	[no test files]
?   	github.com/equinix/metal-cli/internal/vlan	[no test files]
=== RUN   TestCli_Capacity
=== RUN   TestCli_Capacity/get
=== RUN   TestCli_Capacity/get_by_plan_1
=== RUN   TestCli_Capacity/get_by_plan_2
=== RUN   TestCli_Capacity/check_by_multi_metro
=== RUN   TestCli_Capacity/check_by_multi_plan
=== RUN   TestCli_Capacity/check_by_multi_metro_and_plan
--- PASS: TestCli_Capacity (8.92s)
    --- PASS: TestCli_Capacity/get (1.72s)
    --- PASS: TestCli_Capacity/get_by_plan_1 (2.64s)
    --- PASS: TestCli_Capacity/get_by_plan_2 (3.01s)
    --- PASS: TestCli_Capacity/check_by_multi_metro (0.47s)
    --- PASS: TestCli_Capacity/check_by_multi_plan (0.48s)
    --- PASS: TestCli_Capacity/check_by_multi_metro_and_plan (0.59s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/capacitytest	(cached)
=== RUN   TestCli_Devices_Create_Flags
=== RUN   TestCli_Devices_Create_Flags/create_device
--- PASS: TestCli_Devices_Create_Flags (121.10s)
    --- PASS: TestCli_Devices_Create_Flags/create_device (121.10s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/devices/devicecreateflagstest	122.105s
=== RUN   TestCli_Devices_Create
=== RUN   TestCli_Devices_Create/create_device
--- PASS: TestCli_Devices_Create (225.97s)
    --- PASS: TestCli_Devices_Create/create_device (225.97s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/devices/devicecreatetest	226.697s
=== RUN   TestCli_Devices_Get
=== RUN   TestCli_Devices_Get/get_by_device_id
--- PASS: TestCli_Devices_Get (184.92s)
    --- PASS: TestCli_Devices_Get/get_by_device_id (184.92s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/devices/devicegettest	186.207s
=== RUN   TestCli_Devices_Update
=== RUN   TestCli_Devices_Update/reinstall_device
--- PASS: TestCli_Devices_Update (775.33s)
    --- PASS: TestCli_Devices_Update/reinstall_device (775.33s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/devices/devicereinstalltest	777.436s
=== RUN   TestCli_Devices_Update
=== RUN   TestCli_Devices_Update/start_device
--- PASS: TestCli_Devices_Update (192.14s)
    --- PASS: TestCli_Devices_Update/start_device (192.14s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/devices/devicestarttest	193.709s
=== RUN   TestCli_Devices_Update
=== RUN   TestCli_Devices_Update/stop_device
--- PASS: TestCli_Devices_Update (299.97s)
    --- PASS: TestCli_Devices_Update/stop_device (299.97s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/devices/devicestoptest	302.922s
=== RUN   TestCli_Devices_Update
=== RUN   TestCli_Devices_Update/update_device
--- PASS: TestCli_Devices_Update (209.89s)
    --- PASS: TestCli_Devices_Update/update_device (209.89s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/devices/deviceupdatetest	212.551s
=== RUN   TestCli_Events_Get
=== RUN   TestCli_Events_Get/get_events_by_dev_id
--- PASS: TestCli_Events_Get (207.06s)
    --- PASS: TestCli_Events_Get/get_events_by_dev_id (207.06s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/events/deviceeventstest	207.497s
=== RUN   TestCli_Events_Get
=== RUN   TestCli_Events_Get/get_events_by_proj_id
--- PASS: TestCli_Events_Get (2.36s)
    --- PASS: TestCli_Events_Get/get_events_by_proj_id (2.36s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/events/projecteventstest	4.735s
=== RUN   TestCli_Ips_Get
=== RUN   TestCli_Ips_Get/get_ip_reservations
--- PASS: TestCli_Ips_Get (3.46s)
    --- PASS: TestCli_Ips_Get/get_ip_reservations (3.46s)
=== RUN   TestCli_Vlan_Create
=== RUN   TestCli_Vlan_Create/Request_NewIP
--- PASS: TestCli_Vlan_Create (12.19s)
    --- PASS: TestCli_Vlan_Create/Request_NewIP (12.19s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/ipstest	17.471s
=== RUN   TestCli_Metros
=== RUN   TestCli_Metros/get
--- PASS: TestCli_Metros (0.53s)
    --- PASS: TestCli_Metros/get (0.53s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/metrotest	(cached)
=== RUN   TestCli_Organization
=== RUN   TestCli_Organization/get
+--------------------------------------+---------------------------------+-------------------------------+
|                  ID                  |              NAME               |            CREATED            |
+--------------------------------------+---------------------------------+-------------------------------+
| 4d12f460-8c5e-43ea-986d-529d328815ee | DevRel Engineering (Playground) | 2023-08-03 20:50:33 +0000 UTC |
+--------------------------------------+---------------------------------+-------------------------------+
--- PASS: TestCli_Organization (0.54s)
    --- PASS: TestCli_Organization/get (0.54s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/organizationtest	(cached)
=== RUN   TestCli_OperatingSystem
=== RUN   TestCli_OperatingSystem/get
--- PASS: TestCli_OperatingSystem (0.83s)
    --- PASS: TestCli_OperatingSystem/get (0.83s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/ostest	(cached)
=== RUN   TestCli_Plans
=== RUN   TestCli_Plans/get
=== RUN   TestCli_Plans/get_by_slug
=== RUN   TestCli_Plans/get_by_type
--- PASS: TestCli_Plans (3.25s)
    --- PASS: TestCli_Plans/get (1.90s)
    --- PASS: TestCli_Plans/get_by_slug (0.35s)
    --- PASS: TestCli_Plans/get_by_type (1.00s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/plantest	(cached)
=== RUN   TestCli_SshKey
=== RUN   TestCli_SshKey/get
--- PASS: TestCli_SshKey (0.44s)
    --- PASS: TestCli_SshKey/get (0.44s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/sshtest	(cached)
=== RUN   TestCli_Users
=== RUN   TestCli_Users/get
--- PASS: TestCli_Users (0.59s)
    --- PASS: TestCli_Users/get (0.59s)
PASS
ok  	github.com/equinix/metal-cli/test/e2e/usertest	(cached)
?   	github.com/equinix/metal-cli/test/helper	[no test files]

[ctreatma]

@Vasubabu I was going to ask if you had a chance to test with @displague's suggestion to mark assigned_to as required, but then I remembered that required field validation doesn't work in this SDK.

I created #159 to better track the required field issue (there was already a PR to experiment with implementing it, but having an issue for it will make it more visible). Given that issue, I'm OK with keeping the type workaround you've implemented, but I'd like @displague to give a 👍 or 👎 before this gets merged.

[ctreatma]

I'll go ahead and merge this; it's already been tested in metal-cli and we don't have a viable alternative. We can revisit after #159 is addressed.

[github-actions]

This PR is included in version 0.23.1 🎉