Skip to content

✨ ip.cidr, ip.address and raw IP storage#5292

Merged
tas50 merged 10 commits intomainfrom
dom/ip-internal
Mar 8, 2025
Merged

✨ ip.cidr, ip.address and raw IP storage#5292
tas50 merged 10 commits intomainfrom
dom/ip-internal

Conversation

@arlimus
Copy link
Copy Markdown
Member

@arlimus arlimus commented Mar 7, 2025

Adds the address field, which only returns the address itself:

# obscure IPv4, printed nicely
> ip("192.168.0.0/24").address
"192.168.0.0"

> ip("2001:db8:3c4d:15::1a2f:1a2b/48").address
"2001:db8:3c4d:15::1a2f:1a2b"

Adds the cidr field, which returns the CIDR notation of the IP address. We fill in the prefix length for IPv4 class A/B/C networks and IPv6 automatically:

# obscure IPv4, printed nicely
> ip(2885681153).cidr
"172.0.0.1/16"

> ip("2001:db8:3c4d:15::1a2f:1a2b").cidr
"2001:db8:3c4d:15::1a2f:1a2b/64"

internal:

The bigger thing this PR does is overhaul the internal data structure for IP so we don't have to constantly parse it again. This increases efficiency but also means we have to take care of serialization/deserialization.

  • Mask was renamed to PrefixLength because that's what it actually is
  • NewIP is really a ParseIP which also separates it from ParseIntIP (which is demonstrated above)

Why proto-based marshaling?
Not an easy decision, but one that is more maintainable while the interface still shakes out. A byte-optimized marshaling approach will be added later as an alternative.

arlimus added 2 commits March 6, 2025 16:10
Signed-off-by: Dominik Richter <dominik.richter@gmail.com>
Signed-off-by: Dominik Richter <dominik.richter@gmail.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 7, 2025

Test Results

3 506 tests  +26   3 502 ✅ +26   1m 51s ⏱️ +9s
  392 suites ± 0       4 💤 ± 0 
   30 files   ± 0       0 ❌ ± 0 

Results for commit b87f9db. ± Comparison against base commit ae0f3a3.

This pull request removes 1 and adds 27 tests. Note that renamed tests count towards both.
go.mondoo.com/cnquery/v11/llx ‑ TestRawData_Version_Result
go.mondoo.com/cnquery/v11/llx ‑ TestRawData_JSON/0.0.0.0/8
go.mondoo.com/cnquery/v11/llx ‑ TestRawData_JSON/1.2.3
go.mondoo.com/cnquery/v11/llx ‑ TestRawData_JSON/192.168.0.1/13
go.mondoo.com/cnquery/v11/llx ‑ TestRawData_JSON/2001:db8:3c4d:15::1a2f:1a2b/64
go.mondoo.com/cnquery/v11/llx ‑ TestRawData_JSON/255.255.255.255/24
go.mondoo.com/cnquery/v11/llx ‑ TestResultRawConversions
go.mondoo.com/cnquery/v11/llx ‑ TestResultRawConversions/1.2.3
go.mondoo.com/cnquery/v11/llx ‑ TestResultRawConversions/192.168.0.1/27
go.mondoo.com/cnquery/v11/llx ‑ TestSuccess/1.2.3
go.mondoo.com/cnquery/v11/llx ‑ TestSuccess/192.168.0.1/24
…

♻️ This comment has been updated with latest results.

Signed-off-by: Dominik Richter <dominik.richter@gmail.com>
@arlimus arlimus requested a review from afiune March 7, 2025 01:56
arlimus added 3 commits March 6, 2025 18:00
Signed-off-by: Dominik Richter <dominik.richter@gmail.com>
Signed-off-by: Dominik Richter <dominik.richter@gmail.com>
Signed-off-by: Dominik Richter <dominik.richter@gmail.com>
@arlimus arlimus marked this pull request as ready for review March 7, 2025 02:17
Signed-off-by: Dominik Richter <dominik.richter@gmail.com>
@arlimus arlimus changed the title ✨ ip.cidr and raw IP storage ✨ ip.cidr, ip.address and raw IP storage Mar 7, 2025
@arlimus arlimus marked this pull request as draft March 7, 2025 03:55
Signed-off-by: Dominik Richter <dominik.richter@gmail.com>
@arlimus arlimus marked this pull request as ready for review March 7, 2025 20:47
Comment thread llx/llx.proto
bytes address = 1;
bool has_prefix = 2;
int32 prefix_length = 3;
} No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

Comment thread llx/builtin_ip.go Outdated
var bitmasks = []byte{0x00, 0x80, 0xc0, 0xe0, 0xf0, 0xf8, 0xfc, 0xfe, 0xff}

func int2ip(i int64) string {
func int2ip[T int | int64 | int32 | uint | uint64 | uint32](i T) net.IP {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

beautiful ✨

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a question since I see in MQL we use int64 everywhere. It looks awesome but I'm curious if we are even using all of those types? 🤔

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah tbh I just went over-eager here 😁 If I'm honest, we only need int and int64, just like the other method

Comment thread llx/builtin_ip.go
return t, true
case string:
return ParseIP(t), true
case int64:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

are we missing some types here? like int and others? 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm guessing no, but if so, we won't ever use the other types, I think.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not at this position, because the internal values can only be these 3, ie for int it can only be an int64. Much of the backend is built around this.

Copy link
Copy Markdown
Contributor

@afiune afiune left a comment

Choose a reason for hiding this comment

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

Pretty nice, just a question about the int2ip() func.

Comment thread llx/builtin_ip.go Outdated
var bitmasks = []byte{0x00, 0x80, 0xc0, 0xe0, 0xf0, 0xf8, 0xfc, 0xfe, 0xff}

func int2ip(i int64) string {
func int2ip[T int | int64 | int32 | uint | uint64 | uint32](i T) net.IP {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a question since I see in MQL we use int64 everywhere. It looks awesome but I'm curious if we are even using all of those types? 🤔

arlimus added 2 commits March 7, 2025 13:35
Signed-off-by: Dominik Richter <dominik.richter@gmail.com>
Signed-off-by: Dominik Richter <dominik.richter@gmail.com>
@tas50 tas50 merged commit 0e01b21 into main Mar 8, 2025
@tas50 tas50 deleted the dom/ip-internal branch March 8, 2025 03:55
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants