Skip to content

tests: add geoip-enrichment tests#2838

Open
jayhardee9 wants to merge 1 commit intoOISF:masterfrom
jayhardee9:geoip-enrichment-tests
Open

tests: add geoip-enrichment tests#2838
jayhardee9 wants to merge 1 commit intoOISF:masterfrom
jayhardee9:geoip-enrichment-tests

Conversation

@jayhardee9
Copy link

Add tests for the geoip-enrichment output feature

  • geoip-enrichment: Tests that geoip fields are added to alert and flow events when
    geoip-enrichment is enabled

  • geoip-enrichment-disabled: Tests that NO events have geoip fields when geoip-enrichment is disabled

Ticket

Redmine ticket: https://redmine.openinfosecfoundation.org/issues/6999

Add tests for the geoip-enrichment output feature.

- geoip-enrichment: Tests that geoip_dst fields are
added to alert and flow events when
geoip-enrichment is enabled

- geoip-enrichment-disabled: Tests that NO events
have geoip fields when geoip-enrichment is disabled

Ticket: 6999
Copy link
Contributor

@jufajardini jufajardini left a comment

Choose a reason for hiding this comment

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

Thanks for adding tests :)

We must also require a min-version: 9.0 check, to ensure they're only tested against main.

In addition to the inline comments, could we adjust the test so that it would show all of the GeoIP enrichment that is being added? When I ran it locally, I only got:

  "geoip_dst": {
    "ip": "82.165.177.154",
    "geo": {
      "country_iso_code": "DE"
    }
  },

count: 1
match:
event_type: flow
has-key: geoip_dst
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: since we have the check below, this has-key isn't needed

Copy link
Contributor

Choose a reason for hiding this comment

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

(same for the previous check)

count: 0
match:
event_type: alert
has-key: geoip_dst
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use not-has-key, instead.

@jayhardee9
Copy link
Author

Thank you for the quick feedback @jufajardini! When I submit my changes, do I open a new PR or can I push them to this one?

@catenacyber catenacyber added the requires suricata pr Depends on a PR in Suricata label Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

requires suricata pr Depends on a PR in Suricata

Development

Successfully merging this pull request may close these issues.

3 participants