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

dns: add missing dns keywords to schema.json v2 #10401

Closed
wants to merge 1 commit into from

Conversation

hadiqaalamdar
Copy link
Contributor

Feature #5642

Link to redmine ticket: https://redmine.openinfosecfoundation.org/issues/5642

Previous PR: #10193

Describe changes:

  • Added tc boolean field to dns object in schema.json file
  • Added sshfp field in dns.answers and dns.grouped in schema.json
  • SV tests are passing:
===> dns/dns-sshfp: OK

PASSED:  1
FAILED:  0
SKIPPED: 0

SV_BRANCH=OISF/suricata-verify#1588

Found and added missing dns fields in schema.json found through manual code review
Feature OISF#5642
Copy link

NOTE: This PR may contain new authors.

@jufajardini
Copy link
Contributor

NOTE: This PR may contain new authors.

Interesting, why would this be triggered, considering Hadiqa has contributions merged to master, and her author info seems unchanged? 🤔

@jasonish
Copy link
Member

NOTE: This PR may contain new authors.

Interesting, why would this be triggered, considering Hadiqa has contributions merged to master, and her author info seems unchanged? 🤔

This is dependabot getting flagged. So please ignore. We'll have to suppress dependabot since it uses some job id in its email. Still curious as to why it got flagged as it wasn't part of this PR, maybe a rebase would clear it out as well.

@catenacyber
Copy link
Contributor

You can rebase on latest master and the authors check should be green.

#10408 about it

@catenacyber
Copy link
Contributor

And I rebased my SV PR to get CI greener on next run

@jufajardini jufajardini added the needs rebase Needs rebase to master label Feb 14, 2024
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.

The additions look good, thanks for removing the added duplicated fields from last PR.

We are trying to add descriptions to the fields in the JSON schema, so this can also later be used for documentation. Could you please add that to the fields you have added? (one example PR where this is done: https://github.com/OISF/suricata/pull/10348/files)

I have added the needs rebase label based on the existing comments here.

@hadiqaalamdar
Copy link
Contributor Author

New PR: #10429

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs rebase Needs rebase to master
4 participants