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

tool: rewrite suricatasc and suricatactl in rust - v10 #12540

Closed
wants to merge 6 commits into from

Conversation

jasonish
Copy link
Member

@jasonish jasonish commented Feb 6, 2025

Previous PR: #12504

Changes:

  • Fixup documentation about suricatasc being an example python script.

Replaces suricatasc and suricatactl with Rust variants removing python from our
distributed code (except suricata-update).

These now pass CI as some CI tests now use suricatasc where they didn't in the
previous version.

These should also be pure drop-in replacements.

Usually rewrites are a bad idea, but these are small, and in our core language
set, and may inspire us to write new more interesting tooling in the future,
like perhaps a plugin manager?

Ticket: https://redmine.openinfosecfoundation.org/issues/6287

This is a re-implementation of suricatasc program in Rust that
attempts to be a 100% drop-in replacement.
As we have 2 Windows builds, do one using the release-style
distribution file.
These should probably be removed even without the rewrite, and
suricatasc has been installed as a proper program for many releases.
@jasonish jasonish requested review from inashivb, jufajardini and a team as code owners February 6, 2025 22:37
Copy link

codecov bot commented Feb 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.71%. Comparing base (d4330ef) to head (d5194ee).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12540      +/-   ##
==========================================
+ Coverage   80.68%   80.71%   +0.02%     
==========================================
  Files         925      926       +1     
  Lines      258914   258917       +3     
==========================================
+ Hits       208914   208977      +63     
+ Misses      50000    49940      -60     
Flag Coverage Δ
fuzzcorpus 56.88% <ø> (+0.05%) ⬆️
livemode 19.41% <ø> (-0.01%) ⬇️
pcap 44.24% <ø> (+0.05%) ⬆️
suricata-verify 63.39% <ø> (-0.01%) ⬇️
unittests 58.38% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24662

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.

Crom the docs, it looks like this will also make it easier for less experienced folks to try the unix-socket. Maybe now I'll be able to use it :P

@victorjulien
Copy link
Member

Breaks make distcheck on the usual suspect in my CI: FreeBSD 14.

Making all in rust
gmake[2]: Entering directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust'
Making all in suricatasc
gmake[3]: Entering directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust/suricatasc'
gmake[3]: Nothing to be done for 'all'.
gmake[3]: Leaving directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust/suricatasc'
Making all in suricatactl
gmake[3]: Entering directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust/suricatactl'
gmake[3]: Nothing to be done for 'all'.
gmake[3]: Leaving directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust/suricatactl'
gmake[3]: Entering directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust'
mkdir -p /var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust/gen
cd /var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/../../rust && \
	CARGO_HOME="/var/tmp/gitlab_runner/.cargo" CARGO_TARGET_DIR="/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust/target" SURICATA_LUA_SYS_HEADER_DST="/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust/gen" LOCALSTATEDIR=/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_inst/var/run/suricata \
	/usr/local/bin/cargo build --release  \
		--features "ja3 ja4  " 
    Updating crates.io index
error: failed to write /var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/rust/Cargo.lock
Caused by:
  failed to open: /var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/rust/Cargo.lock
Caused by:
  Permission denied (os error 13)
gmake[3]: Leaving directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust'
gmake[3]: *** [Makefile:737: all-local] Error 101
gmake[2]: *** [Makefile:464: all-recursive] Error 1
gmake[2]: Leaving directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust'
gmake[1]: *** [Makefile:504: all-recursive] Error 1
gmake[1]: Leaving directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub'
gmake: *** [Makefile:718: distcheck] Error 1

@victorjulien
Copy link
Member

I guess expected, but this does break my CI checks for unix socket that don't install anything and call the python script directly. Not sure if this is worth considering as part of a upgrade doc/process, but it doesn't make it transparent.
(also, thus untested here so far)

@jasonish
Copy link
Member Author

I guess expected, but this does break my CI checks for unix socket that don't install anything and call the python script directly. Not sure if this is worth considering as part of a upgrade doc/process, but it doesn't make it transparent. (also, thus untested here so far)

Yeah, I guess its transparent for end-users, but not for those running from the source directory. There'll be a path change like rust/target/{release,debug}/suricatactl depending on the type of build I guess.

@jasonish
Copy link
Member Author

Breaks make distcheck on the usual suspect in my CI: FreeBSD 14.

Making all in rust
gmake[2]: Entering directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust'
Making all in suricatasc
gmake[3]: Entering directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust/suricatasc'
gmake[3]: Nothing to be done for 'all'.
gmake[3]: Leaving directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust/suricatasc'
Making all in suricatactl
gmake[3]: Entering directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust/suricatactl'
gmake[3]: Nothing to be done for 'all'.
gmake[3]: Leaving directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust/suricatactl'
gmake[3]: Entering directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust'
mkdir -p /var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust/gen
cd /var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/../../rust && \
	CARGO_HOME="/var/tmp/gitlab_runner/.cargo" CARGO_TARGET_DIR="/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust/target" SURICATA_LUA_SYS_HEADER_DST="/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust/gen" LOCALSTATEDIR=/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_inst/var/run/suricata \
	/usr/local/bin/cargo build --release  \
		--features "ja3 ja4  " 
    Updating crates.io index
error: failed to write /var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/rust/Cargo.lock
Caused by:
  failed to open: /var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/rust/Cargo.lock
Caused by:
  Permission denied (os error 13)
gmake[3]: Leaving directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust'
gmake[3]: *** [Makefile:737: all-local] Error 101
gmake[2]: *** [Makefile:464: all-recursive] Error 1
gmake[2]: Leaving directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust'
gmake[1]: *** [Makefile:504: all-recursive] Error 1
gmake[1]: Leaving directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub'
gmake: *** [Makefile:718: distcheck] Error 1

Out of date Cargo.lock, I forgot to update it. Need to add a CI check for that!

@jasonish
Copy link
Member Author

Continued at #12552:

  • Includes CI check for out of date Cargo.lock.in.

@jasonish jasonish closed this Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants