Skip to content

config: test deterministic glob include ordering (v2) - #3250

Closed
ssam18 wants to merge 1 commit into
OISF:masterfrom
ssam18:config-includes-glob-order-v2
Closed

config: test deterministic glob include ordering (v2)#3250
ssam18 wants to merge 1 commit into
OISF:masterfrom
ssam18:config-includes-glob-order-v2

Conversation

@ssam18

@ssam18 ssam18 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Depends on OISF/suricata#15937 (the include: glob feature, Redmine #8427), which supersedes #15574. This test exercises that feature, so CI here will stay red until it is merged into OISF/suricata: suricata-verify CI builds suricata from the official branches (master/main-7.0.x/main-8.0.x), none of which have the feature yet. The pair is validated in the suricata PR's CI (which builds the feature and runs this exact branch via SV_BRANCH) and locally.

Previous PR: #3151

Adds config-includes-glob-order, a test for the include: glob-pattern support added in OISF/suricata (Redmine #8427).

glob.yaml includes *-order.yaml, which matches three drop-in files (01-order.yaml, 50-order.yaml, 99-order.yaml). Each defines its own unique marker key (glob-order-seen-NN). Because glob(3) returns matches in sorted (lexicographic) order, the files are merged in ascending numeric-prefix order. The common 01..99 drop-in directory convention.

Using --dump-config, the test asserts:

  • all three matched files were included (one unique marker key per file), and
  • the marker keys appear in sorted-filename order, observable because the config tree preserves insertion order.

No key is defined twice, so the test does not depend on duplicate-key override behaviour. What it pins is the sort order, which is what the numbered drop-in convention relies on in practice.

The glob config is kept in glob.yaml and run via command: rather than a shipped suricata.yaml, because suricata-verify runs suricata -c suricata.yaml --dump-config during test setup, before the min-version check.

This addresses review feedback on the suricata PR (victorjulien/jasonish) requesting the include tests live in suricata-verify, with an explicit check that include ordering is deterministic.

Changes since #3151

  • Added a second requires: script: probe, grep '^#define HAVE_GLOB_H 1' src/autoconf.h, so the test is skipped on builds without glob(3).

    Glob expansion needs glob(3), which Windows builds do not have. Per review on the suricata side (jasonish), that platform now skips an include pattern with a warning and loads none of the matched files, so the ordering checks cannot hold there. Probing the build rather than the platform keeps the test running everywhere glob(3) is available, including macOS and the BSDs, unlike a requires: os: linux gate.

    Previously the test avoided failing on Windows only incidentally: requires: script: runs under cmd.exe there, where the > /dev/null redirect fails and makes any probe return false. That is fragile and would break the moment suricata-verify fixed the redirect.

  • Documented the second probe in README.md.

No change to the test's configs, checks, or what it proves.

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

Verified locally against a 9.0.0-dev build with the glob feature: ===> config-includes-glob-order: OK. Verified the skip path by undefining HAVE_GLOB_H in a local build: ===> config-includes-glob-order: SKIPPED: requires script returned false.

@ssam18

ssam18 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

This is v2 of #3151, which @jasonish approved there ("Test looks OK"). A new PR was needed because the branch of an open PR should not be updated.

The only delta from the approved version is your own follow-up suggestion:

Depending on OISF/suricata#15574 (review), this might need a not runs on Windows requirement.

Rather than requires: os: linux, it probes the build: grep '^#define HAVE_GLOB_H 1' src/autoconf.h. Windows is what actually lacks glob(3), so probing the build keeps the test running on macOS and the BSDs, where it works. Plus a README paragraph explaining it.

The drop-in configs, glob.yaml, and every check are unchanged from the version approved in #3151, the ordering assertions @catenacyber and @jasonish discussed there are untouched.

Verified locally: ===> config-includes-glob-order: OK against a build of OISF/suricata#15937, and SKIPPED: requires script returned false against the same build with HAVE_GLOB_H undefined.

Add config-includes-glob-order, which checks that a glob pattern in an include directive expands to all matching files and includes them in sorted order. glob.yaml includes *-order.yaml, matching three drop-in files (01/50/99) that each define their own unique marker key. The test asserts that all three keys are present in --dump-config output and that they appear in sorted-filename order, observable because the config tree preserves insertion order. No key is defined twice, so the test does not rely on duplicate-key override behavior.

Include order matters because settings from later includes override earlier ones, which is what makes the numbered 01..99 drop-in convention predictable for users.

The glob config is kept in glob.yaml and run via command: rather than a shipped suricata.yaml, because suricata-verify runs "suricata -c suricata.yaml --dump-config" during test setup before the min-version check. A glob include in suricata.yaml would break that setup step on Suricata versions without glob support.

Until the Suricata side lands, a requires script greps src/conf-yaml-loader.c for the glob expansion code and skips the test on builds without it, the same way the eve-alert-metadata tests probe for METADATA_DEFAULTS. This keeps CI green against main and the release branches.

A second requires script greps src/autoconf.h for HAVE_GLOB_H. Glob expansion needs glob(3), which Windows builds do not have; there an include pattern is skipped with a warning and none of the matched files are loaded, so the ordering checks could not hold. Probing the build rather than the platform keeps the test running everywhere glob(3) is available, including macOS and the BSDs.

Exercises the include glob support added in OISF/suricata for this ticket. Ticket: #8427
@ssam18

ssam18 commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #3281: same test rebased onto current master (no changes to the test itself), resubmitted as a new PR per the workflow since this branch had to be updated. The companion suricata PR is now OISF/suricata#16031.

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.

2 participants