-
Notifications
You must be signed in to change notification settings - Fork 280
Networking extension conformance tests #507
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Networking extension conformance tests #507
Conversation
| test { | ||
| name: "cidr_get_ip_ipv6" | ||
| expr: "cidr('2001:db8::/32').ip() == ip('2001:db8::')" | ||
| value: { bool_value: "true" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| value: { bool_value: "true" } | |
| value: { bool_value: true } |
| name: "cidr_masked_ipv4" | ||
| expr: "cidr('192.168.0.1/24').masked()" | ||
| value: { string_value: "192.168.0.0/24" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this correct? I think the signature for masked should be <cidr>.masked() -> cidr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's definitely wrong, moved the masked() check into a comparison
| } | ||
| test { | ||
| name: "cidr_contains_ip_ipv4_object" | ||
| expr: "cidr('192.168.0.0/24').containsIP(ip('192.168.0.1'))" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cross-family tests are likely worth introducing
ex: cidr('192.168.0.0/24').containsIP(ip('2001:db8::1')). Should this be false or an error?
| value: { bool_value: false } | ||
| } | ||
| } | ||
| section { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add error cases for zones and ipv4 mapped ipv6 addresses:
ip('::ffff:192.168.0.1')ip('fe80::1%eth0')
|
|
||
| name: "network_ext" | ||
| description: "Conformance tests for IP address extensions in CEL" | ||
| section { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Equality is tested implicitly in some places, but imo, worth adding an explicit section for this (especially the mixed case strings and cross family cases):
ip('127.0.0.1') == ip('127.0.0.1') -> True
ip('127.0.0.1') == ip('1.1.1.1') -> False
ip('0.0.0.0') == ip('::') -> False
ip('2001:db8::1') == ip('2001:DB8::1') -> True
| expr: "cidr('192.168.0.0/24').containsIP('192.168.1.1')" | ||
| value: { bool_value: false } | ||
| } | ||
| test { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add a test case for an exact match: cidr('10.0.0.0/8').containsCIDR(cidr('10.0.0.0/8')) -> True
Conformance tests for IP and CIDR extensions.
The tests are derived from the extensions currently supported in Kubernetes and are
being introduced to validate changes prior to upstreaming the K8s library into the CEL
stacks.