Skip to content
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

feat: Use CIDR format for connection-manager allow/deny lists #2783

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

acul71
Copy link

@acul71 acul71 commented Oct 24, 2024

@SgtPooki @wemeetagain @dhuseby

Description

This PR updates the connection manager to treat multiaddrs in the allow/deny lists using the standard IP CIDR format (e.g. /ip4/52.55.0.0/ipcidr/16) rather than string prefixes (e.g. /ip4/52.55). This allows us to validate multiaddrs accurately and ensures better control over IP address matching.

Changes:

  • Converted the .allow and .deny properties in the connection manager to use IpNet[] for IP range validation.
  • Updated tests to use the IpNet format for both allow and deny lists.
  • This will improve validation, making it more aligned with IP network management best practices by using CIDR blocks.

Relevant issues:

Notes & open questions

  • How developer will know that they have to use IpNet format in allow and deny list?
  • This PR focuses on using the IpNet format to ensure the connection manager handles IPs in CIDR notation.
  • Any future changes related to this feature would involve expanding IP management logic.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

@acul71 acul71 requested a review from a team as a code owner October 24, 2024 03:27
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