Skip to content

Implement net.cidr_contains builtin#471

Merged
anakrish merged 1 commit into
microsoft:mainfrom
tjons:tjons/net-cidr-contains
Sep 5, 2025
Merged

Implement net.cidr_contains builtin#471
anakrish merged 1 commit into
microsoft:mainfrom
tjons:tjons/net-cidr-contains

Conversation

@tjons

@tjons tjons commented Aug 31, 2025

Copy link
Copy Markdown
Contributor

Implement the net.cidr_contains builtin in Regorus. Added the IpNet crate to handle and standardize CIDR parsing, which will be useful for the remainder of the net.* built-ins.

Comment thread src/builtins/net.rs Outdated
Comment thread src/builtins/net.rs Outdated
Comment thread Cargo.toml Outdated
@tjons

tjons commented Sep 1, 2025

Copy link
Copy Markdown
Contributor Author

@anakrish these java bindings are killing me... I figured out how to update them based on the github action, but I have a couple questions:

  1. Is there a reason the Java bindings fail but all other languages succeed? All languages with a Cargo.lock file seem to build bindings with --frozen, and while the Java binding uses --locked as well, --frozen should be more restrictive than --locked.
  2. Could I contribute (in a future PR) some sort of tooling to check this locally before pushing to CI? Perhaps leveraging https://docs.rs/cargo-task/latest/cargo_task/ or adding something to the pre-push hook? Usually in Go projects we have Makefile targets to check code generation. Right now the only alternative I can think of is running the actions locally nektos/act which usually is a little more resource-intensive and harder to remember.

@anakrish

anakrish commented Sep 2, 2025

Copy link
Copy Markdown
Collaborator

Hey @tjons,

1.a I don't exactly know why Java bindings fail. In the past, I have seen it fail due to stricter clippy errors. I thought my recent PR that uses the same rust toolchain uniformly would fix it. But looks like it doesn't. cc @unexge for insights.
1.b Yes. It should also use --frozen.

  1. Yes, testing it locally is a great idea. The current pre commit hook approach is something I put together to prevent me from breaking stuff, while I was learning Rust. If there are better approaches (say cargo-task) we can totally use it.

@anakrish

anakrish commented Sep 2, 2025

Copy link
Copy Markdown
Collaborator

Hey @tjons, Looks like all the OPA tests are passing. Kindly squash the commits and use a conventional commit style changelist description.

I can then approve, rerun the test and merge.
Thanks for the contribution!

Major changes:
- Implement the `net.cidr_contains` builtin
- Enable the v0 and v1 test for `net.cidr_contains`
- Add the `netip` crate to standardize CIDR searching and other
  operations

Key Concept:
- Allow users to leverage the `net.cidr_contains` builtin to check
  whether an IPv4 or IPv6 CIDR contains a specified IP address or
  subnet.

Testing:
- All tests passing.

Signed-off-by: tjons <tylerschade99@gmail.com>
@tjons tjons force-pushed the tjons/net-cidr-contains branch from 8a21dda to 16ef477 Compare September 5, 2025 12:42
@tjons

tjons commented Sep 5, 2025

Copy link
Copy Markdown
Contributor Author

Created #475 to track language binding generation, and updated the commit message - thanks @anakrish!

@anakrish anakrish left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. Thanks for the contribution @tjons

@anakrish anakrish merged commit 1b0c2d4 into microsoft:main Sep 5, 2025
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants