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

Add a plugin for IPAddress (ipv4) #692

Merged
merged 10 commits into from
Apr 30, 2024
Merged

Conversation

perryzjc
Copy link
Contributor

  • Please check if the PR fulfills these requirements
  • Tests for the changes have been added
  • Docs have been added / updated
  • All CI checks are green
  • What kind of change does this PR introduce?

This PR introduces a new feature in the form of an IP address detector. This detector scans for public IP addresses (IPv4) and ignores certain non-public IPv4 addresses. The ignored addresses include 127.x.x.x, 10.x.x.x, 172.16-31.x.x and 192.168.x.x.

  • What is the current behavior?

Currently, there is no specific functionality in our system that allows for the detection of any IP addresses.

  • What is the new behavior (if this is a feature change)?
    With this feature change, the system can now detect public IPv4 addresses. It uses a regex-based detector to identify valid public IPv4 addresses and ignore certain non-public ones.
  • Does this PR introduce a breaking change?

This PR does not introduce a breaking change. The new detector class IPAddressDetector is a standalone addition and does not alter or interfere with the existing code base.

  • Other information:
    In addition to the implementation of the IP Address Detector, this PR also includes comprehensive tests to ensure the correct functionality of the detector. It tests the detector against valid public and non-public IPv4 addresses, as well as invalid IP addresses

The reference for the definition of public and non-public IPv4 addresses can be found in the following links:
IPv4 Address Space
Private Network - Wikipedia

The code documentation has been updated to reflect these changes. The new feature is expected to enhance the system's capabilities in handling IP address-related data.

@riverma
Copy link

riverma commented Oct 18, 2023

FYI @KevinHock - thoughts?

@lorenzodb1
Copy link
Member

Hi @perryzjc, thank you for opening this PR. I noticed our PR checks did not run, so I'll have to figure what happened there first. Rest assured I'll get to this as soon as possible.

@lorenzodb1
Copy link
Member

@perryzjc hi again 😄 could you please merge master into your branch?

@perryzjc
Copy link
Contributor Author

@perryzjc hi again 😄 could you please merge master into your branch?

@lorenzodb1 hi again, I've merged the latest updates from the master branch into my feature branch as requested. Please let me know if there's anything else needed for this pull request. Thanks!

@lorenzodb1
Copy link
Member

@perryzjc looks like there's some pre-commit checks failing, please take a look

@perryzjc
Copy link
Contributor Author

@perryzjc looks like there's some pre-commit checks failing, please take a look

@lorenzodb1 Thanks for letting me know. I've resolved the issues and all checks are now passing in the workflow.

tests/plugins/ip_public_test.py Outdated Show resolved Hide resolved
tests/plugins/ip_public_test.py Outdated Show resolved Hide resolved
@riverma
Copy link

riverma commented Mar 1, 2024

Hey @lorenzodb1 @jpdakran - just wanted to follow-up here. It would be great to have these plugins part of the main codebase when you get a chance! Great job @perryzjc.

@lorenzodb1
Copy link
Member

Hi @riverma, I have a bit of a backlog, but I promise you I'll get to this (and other great PRs) as soon as possible.

@riverma
Copy link

riverma commented Mar 12, 2024

Hi @riverma, I have a bit of a backlog, but I promise you I'll get to this (and other great PRs) as soon as possible.

Thank you @lorenzodb1 for your support!

Copy link
Member

@lorenzodb1 lorenzodb1 left a comment

Choose a reason for hiding this comment

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

@perryzjc thank you for your contribution! These changes will be included in the next release coming up at the end of this week 😄

@lorenzodb1 lorenzodb1 merged commit e98e137 into Yelp:master Apr 30, 2024
12 checks passed
@lorenzodb1 lorenzodb1 mentioned this pull request Apr 30, 2024
3 tasks
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.

None yet

3 participants