| Message ID | 20231117171948.897229-1-thaller@redhat.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [nft,1/1] tests/shell: sanitize "handle" in JSON output | expand |
On Fri, Nov 17, 2023 at 06:18:45PM +0100, Thomas Haller wrote: > The "handle" in JSON output is not stable. Sanitize/normalizeit to 1216. > > The number is chosen arbitrarily, but it's somewhat unique in the code > base. So when you see it, you may guess it originates from sanitization. Valid handles are monotonic starting at 1. Using 0 as a replacement is too simple? > Signed-off-by: Thomas Haller <thaller@redhat.com> > --- > Note that only a few .json-nft files are adjusted, because otherwise the > patch is too large. Before applying, you need to adjust them all, by > running `./tests/shell/run-tests.sh -g`. Just put the bulk change into a second patch? [...] > diff --git a/tests/shell/helpers/json-sanitize-ruleset.sh b/tests/shell/helpers/json-sanitize-ruleset.sh > index 270a6107e0aa..3b66adabf055 100755 > --- a/tests/shell/helpers/json-sanitize-ruleset.sh > +++ b/tests/shell/helpers/json-sanitize-ruleset.sh > @@ -6,7 +6,14 @@ die() { > } > > do_sed() { > - sed '1s/\({"nftables": \[{"metainfo": {"version": "\)[0-9.]\+\(", "release_name": "\)[^"]\+\(", "\)/\1VERSION\2RELEASE_NAME\3/' "$@" > + # Normalize the "version"/"release_name", otherwise we have to regenerate the > + # JSON output upon new release. > + # > + # Also, "handle" are not stable. Normalize them to 1216 (arbitrarily chosen). > + sed \ > + -e '1s/\({"nftables": \[{"metainfo": {"version": "\)[0-9.]\+\(", "release_name": "\)[^"]\+\(", "\)/\1VERSION\2RELEASE_NAME\3/' \ > + -e '1s/"handle": [0-9]\+\>/"handle": 1216/g' \ > + "$@" > } Why not just drop the whole metainfo object? A dedicated test could still ensure its existence. Also, scoping these replacements to line 1 is funny with single line input. Worse is identifying the change in the resulting diff. Maybe write a helper in python which lets you more comfortably sanitize input, sort attributes by key and output pretty-printed? In general, the long lines in your scripts make them quite hard to read. Any particular reason why you don't stick to the 80 columns maxim? Cheers, Phil
Hi, On Sat, 2023-11-18 at 03:36 +0100, Phil Sutter wrote: > On Fri, Nov 17, 2023 at 06:18:45PM +0100, Thomas Haller wrote: > > The "handle" in JSON output is not stable. Sanitize/normalizeit to > > 1216. > > > > The number is chosen arbitrarily, but it's somewhat unique in the > > code > > base. So when you see it, you may guess it originates from > > sanitization. > > Valid handles are monotonic starting at 1. Using 0 as a replacement > is > too simple? Changed. > > > Signed-off-by: Thomas Haller <thaller@redhat.com> > > --- > > Note that only a few .json-nft files are adjusted, because > > otherwise the > > patch is too large. Before applying, you need to adjust them all, > > by > > running `./tests/shell/run-tests.sh -g`. > > Just put the bulk change into a second patch? it would require 3 patches to stay below the limit. Also, it blows up the inbox by everybody on the list by 850K (57k gzipped). The rest of the patch is generated. Just generate it. Alternatively, git fetch https://gitlab.freedesktop.org/thaller/nftables df984038a33c6da5b159e6f6458351c4fa673bf1 git merge FETCH_HEAD > > [...] > > diff --git a/tests/shell/helpers/json-sanitize-ruleset.sh > > b/tests/shell/helpers/json-sanitize-ruleset.sh > > index 270a6107e0aa..3b66adabf055 100755 > > --- a/tests/shell/helpers/json-sanitize-ruleset.sh > > +++ b/tests/shell/helpers/json-sanitize-ruleset.sh > > @@ -6,7 +6,14 @@ die() { > > } > > > > do_sed() { > > - sed '1s/\({"nftables": \[{"metainfo": {"version": "\)[0- > > 9.]\+\(", "release_name": "\)[^"]\+\(", > > "\)/\1VERSION\2RELEASE_NAME\3/' "$@" > > + # Normalize the "version"/"release_name", otherwise we > > have to regenerate the > > + # JSON output upon new release. > > + # > > + # Also, "handle" are not stable. Normalize them to 1216 > > (arbitrarily chosen). > > + sed \ > > + -e '1s/\({"nftables": \[{"metainfo": {"version": > > "\)[0-9.]\+\(", "release_name": "\)[^"]\+\(", > > "\)/\1VERSION\2RELEASE_NAME\3/' \ > > + -e '1s/"handle": [0-9]\+\>/"handle": 1216/g' \ > > + "$@" > > } > > Why not just drop the whole metainfo object? A dedicated test could > still ensure its existence. Normalization should only perform the absolute minimal of tampering. > Also, scoping these replacements to line 1 is funny with single line > input. Worse is identifying the change in the resulting diff. Maybe > write a helper in python which lets you more comfortably sanitize > input, > sort attributes by key and output pretty-printed? You mean, to parse and re-encode the JSON? That introduces additional changes, which seems undesirable. That's why the regex is limited to the first line (even if we only expect to ever see one line there). Also, normalization via 2 regex seems simpler than writing some python. Well, pretty-printing the output with `jq` would have the advantage, that future diffs might be smaller (changing individual lines, vs. replace one large line). Still, I think it's better to keep the amount of post-processing minimal. > > In general, the long lines in your scripts make them quite hard to > read. > Any particular reason why you don't stick to the 80 columns maxim? I wrapped two lines in the patch. Thomas
On Tue, Nov 21, 2023 at 01:10:11PM +0100, Thomas Haller wrote: > On Sat, 2023-11-18 at 03:36 +0100, Phil Sutter wrote: [...] > > Also, scoping these replacements to line 1 is funny with single line > > input. Worse is identifying the change in the resulting diff. Maybe > > write a helper in python which lets you more comfortably sanitize > > input, > > sort attributes by key and output pretty-printed? > > You mean, to parse and re-encode the JSON? That introduces additional > changes, which seems undesirable. That's why the regex is limited to > the first line (even if we only expect to ever see one line there). > > Also, normalization via 2 regex seems simpler than writing some python. > > Well, pretty-printing the output with `jq` would have the advantage, > that future diffs might be smaller (changing individual lines, vs. > replace one large line). Still, I think it's better to keep the amount > of post-processing minimal. The testsuite relies upon Python and respective modules already, using jq introduces a new dependency. Hence why I suggested to write a script. JSON object attributes are not bound to any ordering, the code may change it. When analyzing testsuite failures, a diff of two overlong lines is inconvenient to the point that one may pipe both through json_pp and then diff again. The testsuite may do just that in case of offending output, but the problem of reordered attributes remains. I'd really appreciate if testsuite changes prioritized usability. I rather focus on fixing bugs instead of parsing the testsuite results. Cheers, Phil
Hi Thomas, On Tue, Nov 21, 2023 at 01:10:11PM +0100, Thomas Haller wrote: > On Sat, 2023-11-18 at 03:36 +0100, Phil Sutter wrote: > > On Fri, Nov 17, 2023 at 06:18:45PM +0100, Thomas Haller wrote: [...] > > > Note that only a few .json-nft files are adjusted, because > > > otherwise the > > > patch is too large. Before applying, you need to adjust them all, > > > by > > > running `./tests/shell/run-tests.sh -g`. > > > > Just put the bulk change into a second patch? > > it would require 3 patches to stay below the limit. > > Also, it blows up the inbox by everybody on the list by 850K (57k > gzipped). The rest of the patch is generated. Just generate it. Excuse for jumping through this, just a submission style notice: Please, Cc maintainers with big patches that don't fit in into the mailing list in the future. > Alternatively, > > git fetch https://gitlab.freedesktop.org/thaller/nftables df984038a33c6da5b159e6f6458351c4fa673bf1 > git merge FETCH_HEAD I'd suggest you place this also in the cover letter, so at least there is a record on the mailing list archive that some patches could not get through but there was an alternative way to take them for others. As for maintainers, they can collect the large patch as attachment from the email. Thanks.
On Tue, 2023-11-21 at 13:39 +0100, Phil Sutter wrote: > On Tue, Nov 21, 2023 at 01:10:11PM +0100, Thomas Haller wrote: > > On Sat, 2023-11-18 at 03:36 +0100, Phil Sutter wrote: > [...] > > > Also, scoping these replacements to line 1 is funny with single > > > line > > > input. Worse is identifying the change in the resulting diff. > > > Maybe > > > write a helper in python which lets you more comfortably sanitize > > > input, > > > sort attributes by key and output pretty-printed? > > > > You mean, to parse and re-encode the JSON? That introduces > > additional > > changes, which seems undesirable. That's why the regex is limited > > to > > the first line (even if we only expect to ever see one line there). > > > > Also, normalization via 2 regex seems simpler than writing some > > python. > > > > Well, pretty-printing the output with `jq` would have the > > advantage, > > that future diffs might be smaller (changing individual lines, vs. > > replace one large line). Still, I think it's better to keep the > > amount > > of post-processing minimal. > > The testsuite relies upon Python and respective modules already, > using > jq introduces a new dependency. Hence why I suggested to write a > script. > > JSON object attributes are not bound to any ordering, the code may > change it. Don't have .nft dumps the same concern? In JSON the order of things certainly matters. libjansson has JSON_PRESERVE_ORDER, which is used by libnftables. Also, JSON_PRESERVE_ORDER is deprecated since 2016 and order is always preserved. If the order changes, that should be visible (in form of a test failure). > > When analyzing testsuite failures, a diff of two overlong lines is > inconvenient to the point that one may pipe both through json_pp and > then diff again. The testsuite may do just that in case of offending > output, but the problem of reordered attributes remains. > > I'd really appreciate if testsuite changes prioritized usability. I > rather focus on fixing bugs instead of parsing the testsuite results. The test suite prioritizes usability. No need to suggest otherwise. To make debugging easier, the test suite can additionally show a prettified diff. It does not determine how the .json-nft file is stored in git. Thomas
On Tue, Nov 21, 2023 at 01:58:41PM +0100, Thomas Haller wrote: > On Tue, 2023-11-21 at 13:39 +0100, Phil Sutter wrote: > > On Tue, Nov 21, 2023 at 01:10:11PM +0100, Thomas Haller wrote: > > > On Sat, 2023-11-18 at 03:36 +0100, Phil Sutter wrote: > > [...] > > > > Also, scoping these replacements to line 1 is funny with single > > > > line > > > > input. Worse is identifying the change in the resulting diff. > > > > Maybe > > > > write a helper in python which lets you more comfortably sanitize > > > > input, > > > > sort attributes by key and output pretty-printed? > > > > > > You mean, to parse and re-encode the JSON? That introduces > > > additional > > > changes, which seems undesirable. That's why the regex is limited > > > to > > > the first line (even if we only expect to ever see one line there). > > > > > > Also, normalization via 2 regex seems simpler than writing some > > > python. > > > > > > Well, pretty-printing the output with `jq` would have the > > > advantage, > > > that future diffs might be smaller (changing individual lines, vs. > > > replace one large line). Still, I think it's better to keep the > > > amount > > > of post-processing minimal. > > > > The testsuite relies upon Python and respective modules already, > > using > > jq introduces a new dependency. Hence why I suggested to write a > > script. > > > > JSON object attributes are not bound to any ordering, the code may > > change it. > > Don't have .nft dumps the same concern? Not as far as I can tell: Objects are sorted by name, rule ordering is inherently relevant. > In JSON the order of things certainly matters. libjansson has > JSON_PRESERVE_ORDER, which is used by libnftables. Also, > JSON_PRESERVE_ORDER is deprecated since 2016 and order is always > preserved. The reason why JSON_PRESERVE_ORDER exists is just because ordering does not matter per se. For a proper JSON parser, | {"a": 1, "b": 2} and | {"b": 2, "a": 1} are semantically identical. > If the order changes, that should be visible (in form of a test > failure). Why? If we change e.g. the ordering of array elements by adding them in reverse, isn't this a legal change and any testsuite complaints about it just noise? > > When analyzing testsuite failures, a diff of two overlong lines is > > inconvenient to the point that one may pipe both through json_pp and > > then diff again. The testsuite may do just that in case of offending > > output, but the problem of reordered attributes remains. > > > > I'd really appreciate if testsuite changes prioritized usability. I > > rather focus on fixing bugs instead of parsing the testsuite results. > > The test suite prioritizes usability. No need to suggest otherwise. Then why not store JSON dumps pretty printed to make diffs more readable? > To make debugging easier, the test suite can additionally show a > prettified diff. It does not determine how the .json-nft file is stored > in git. Is this "can" in a pending patch? Because I don't see that "prettified diff" option in tests/shell/helpers/test-wrapper.sh. Cheers, Phil
On Tue, 2023-11-21 at 14:40 +0100, Phil Sutter wrote: > On Tue, Nov 21, 2023 at 01:58:41PM +0100, Thomas Haller wrote: > > On Tue, 2023-11-21 at 13:39 +0100, Phil Sutter wrote: > > > On Tue, Nov 21, 2023 at 01:10:11PM +0100, Thomas Haller wrote: > > > > On Sat, 2023-11-18 at 03:36 +0100, Phil Sutter wrote: > > > [...] > > > > > Also, scoping these replacements to line 1 is funny with > > > > > single > > > > > line > > > > > input. Worse is identifying the change in the resulting diff. > > > > > Maybe > > > > > write a helper in python which lets you more comfortably > > > > > sanitize > > > > > input, > > > > > sort attributes by key and output pretty-printed? > > > > > > > > You mean, to parse and re-encode the JSON? That introduces > > > > additional > > > > changes, which seems undesirable. That's why the regex is > > > > limited > > > > to > > > > the first line (even if we only expect to ever see one line > > > > there). > > > > > > > > Also, normalization via 2 regex seems simpler than writing some > > > > python. > > > > > > > > Well, pretty-printing the output with `jq` would have the > > > > advantage, > > > > that future diffs might be smaller (changing individual lines, > > > > vs. > > > > replace one large line). Still, I think it's better to keep the > > > > amount > > > > of post-processing minimal. > > > > > > The testsuite relies upon Python and respective modules already, > > > using > > > jq introduces a new dependency. Hence why I suggested to write a > > > script. > > > > > > JSON object attributes are not bound to any ordering, the code > > > may > > > change it. > > > > Don't have .nft dumps the same concern? > > Not as far as I can tell: Objects are sorted by name, rule ordering > is > inherently relevant. If sorting is necessary to get stable output, then JSON handling should do the same. It is a desirable property, that the output of a command is stable. > > > In JSON the order of things certainly matters. libjansson has > > JSON_PRESERVE_ORDER, which is used by libnftables. Also, > > JSON_PRESERVE_ORDER is deprecated since 2016 and order is always > > preserved. > > The reason why JSON_PRESERVE_ORDER exists is just because ordering > does > not matter per se. > For a proper JSON parser, > > {"a": 1, "b": 2} > and > > {"b": 2, "a": 1} > are semantically identical. Whitespace in JSON is even more irrelevant for "semantically identical". From that, it doesn't follow that `nft -j list ruleset` should change the output (regarding order or whitespace) arbitrarily. The tool should make an effort to not change the output. > > If the order changes, that should be visible (in form of a test > > failure). > > Why? If we change e.g. the ordering of array elements by adding them > in > reverse, isn't this a legal change and any testsuite complaints about > it > just noise? If there are good reasons to change something, it can be done. It is a "legal" change, but not accidental or inconsequential. Adjusting tests int that case is a good (and easy) thing. > > > > When analyzing testsuite failures, a diff of two overlong lines > > > is > > > inconvenient to the point that one may pipe both through json_pp > > > and > > > then diff again. The testsuite may do just that in case of > > > offending > > > output, but the problem of reordered attributes remains. > > > > > > I'd really appreciate if testsuite changes prioritized usability. > > > I > > > rather focus on fixing bugs instead of parsing the testsuite > > > results. > > > > The test suite prioritizes usability. No need to suggest otherwise. > > Then why not store JSON dumps pretty printed to make diffs more > readable? That's still on the table. Though, I would much rather do an absolute minimum of post-processing ("json-sanitize-ruleset.sh") to not accidentally hiding a bug. Yes, that may be more inconvenient. But IMO only negligibly so. > > > To make debugging easier, the test suite can additionally show a > > prettified diff. It does not determine how the .json-nft file is > > stored > > in git. > > Is this "can" in a pending patch? Because I don't see that > "prettified > diff" option in tests/shell/helpers/test-wrapper.sh. No. I said "can". You just brought this (good) idea up. Could be something like: fi if [ "$NFT_TEST_HAVE_json" != n -a -f "$JDUMPFILE" ] ; then if ! $DIFF -u "$JDUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after.json" &> "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" ; then + "$NFT_TEST_BASEDIR/helpers/json-diff-pretty.sh" \ + "$JDUMPFILE" \ + "$NFT_TEST_TESTTMPDIR/ruleset-after.json" \ + > "$NFT_TEST_TESTTMPDIR/ruleset-diff-json-pretty" show_file "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" "Failed \`$DIFF -u \"$JDUMPFILE\" \"$NFT_TEST_TESTTMPDIR/ruleset-after.json\"\`" >> "$NFT_TEST_TESTTMPDIR/rc-failed-dump" rc_dump=1 else Having such a "json-diff-pretty" script in the toolbox might be handy for debugging anyway. I guess, it's somewhere under tests/py already? Thomas
On Tue, Nov 21, 2023 at 03:35:43PM +0100, Thomas Haller wrote: > On Tue, 2023-11-21 at 14:40 +0100, Phil Sutter wrote: > > On Tue, Nov 21, 2023 at 01:58:41PM +0100, Thomas Haller wrote: > > > On Tue, 2023-11-21 at 13:39 +0100, Phil Sutter wrote: > > > > On Tue, Nov 21, 2023 at 01:10:11PM +0100, Thomas Haller wrote: > > > > > On Sat, 2023-11-18 at 03:36 +0100, Phil Sutter wrote: > > > > [...] > > > > > > Also, scoping these replacements to line 1 is funny with > > > > > > single > > > > > > line > > > > > > input. Worse is identifying the change in the resulting diff. > > > > > > Maybe > > > > > > write a helper in python which lets you more comfortably > > > > > > sanitize > > > > > > input, > > > > > > sort attributes by key and output pretty-printed? > > > > > > > > > > You mean, to parse and re-encode the JSON? That introduces > > > > > additional > > > > > changes, which seems undesirable. That's why the regex is > > > > > limited > > > > > to > > > > > the first line (even if we only expect to ever see one line > > > > > there). > > > > > > > > > > Also, normalization via 2 regex seems simpler than writing some > > > > > python. > > > > > > > > > > Well, pretty-printing the output with `jq` would have the > > > > > advantage, > > > > > that future diffs might be smaller (changing individual lines, > > > > > vs. > > > > > replace one large line). Still, I think it's better to keep the > > > > > amount > > > > > of post-processing minimal. > > > > > > > > The testsuite relies upon Python and respective modules already, > > > > using > > > > jq introduces a new dependency. Hence why I suggested to write a > > > > script. > > > > > > > > JSON object attributes are not bound to any ordering, the code > > > > may > > > > change it. > > > > > > Don't have .nft dumps the same concern? > > > > Not as far as I can tell: Objects are sorted by name, rule ordering > > is > > inherently relevant. > > If sorting is necessary to get stable output, then JSON handling should > do the same. You mean src/json.c? If so, I'm against that. We'd introduce overhead just for an internal use-case. > It is a desirable property, that the output of a command is stable. Define "stable" in the context of ECMA-404 (the JSON standard). > > > In JSON the order of things certainly matters. libjansson has > > > JSON_PRESERVE_ORDER, which is used by libnftables. Also, > > > JSON_PRESERVE_ORDER is deprecated since 2016 and order is always > > > preserved. > > > > The reason why JSON_PRESERVE_ORDER exists is just because ordering > > does > > not matter per se. > > > > For a proper JSON parser, > > > {"a": 1, "b": 2} > > and > > > {"b": 2, "a": 1} > > are semantically identical. > > > Whitespace in JSON is even more irrelevant for "semantically > identical". > > From that, it doesn't follow that `nft -j list ruleset` should change > the output (regarding order or whitespace) arbitrarily. The tool should > make an effort to not change the output. Not arbitrarily. But I don't see a point in having to adjust the dumps if it does. If the testsuite is tolerant to these changes, it even makes it easier to assert their correctness. Adjusting dumps to actually incorrect output is a not uncommon human error, especially if it's a bulk change. > > > If the order changes, that should be visible (in form of a test > > > failure). > > > > Why? If we change e.g. the ordering of array elements by adding them > > in > > reverse, isn't this a legal change and any testsuite complaints about > > it > > just noise? > > > If there are good reasons to change something, it can be done. > > It is a "legal" change, but not accidental or inconsequential. > Adjusting tests int that case is a good (and easy) thing. Why is it a good thing? Also, in the context of resulting patches exceeding limits, is it easy? > > > > When analyzing testsuite failures, a diff of two overlong lines > > > > is > > > > inconvenient to the point that one may pipe both through json_pp > > > > and > > > > then diff again. The testsuite may do just that in case of > > > > offending > > > > output, but the problem of reordered attributes remains. > > > > > > > > I'd really appreciate if testsuite changes prioritized usability. > > > > I > > > > rather focus on fixing bugs instead of parsing the testsuite > > > > results. > > > > > > The test suite prioritizes usability. No need to suggest otherwise. > > > > Then why not store JSON dumps pretty printed to make diffs more > > readable? > > That's still on the table. > > Though, I would much rather do an absolute minimum of post-processing > ("json-sanitize-ruleset.sh") to not accidentally hiding a bug. That's an argument for a proper JSON parser and against your minimalistic replacements using sed. > Yes, that may be more inconvenient. But IMO only negligibly so. Just look at the diffs in this patch - for me it's impossible to assert only the handle value changes in the dumps and nothing else. Can you really work with unified diffs of 600 character lines? > > > To make debugging easier, the test suite can additionally show a > > > prettified diff. It does not determine how the .json-nft file is > > > stored > > > in git. > > > > Is this "can" in a pending patch? Because I don't see that > > "prettified > > diff" option in tests/shell/helpers/test-wrapper.sh. > > No. I said "can". You just brought this (good) idea up. It's bad because the diffs in patches remain unreadable. It's just an alternative if the dumps have to remain the same, but I don't see a reason for that. > Could be something like: > > fi > if [ "$NFT_TEST_HAVE_json" != n -a -f "$JDUMPFILE" ] ; then > if ! $DIFF -u "$JDUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after.json" &> "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" ; then > + "$NFT_TEST_BASEDIR/helpers/json-diff-pretty.sh" \ > + "$JDUMPFILE" \ > + "$NFT_TEST_TESTTMPDIR/ruleset-after.json" \ > + > "$NFT_TEST_TESTTMPDIR/ruleset-diff-json-pretty" > show_file "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" "Failed \`$DIFF -u \"$JDUMPFILE\" \"$NFT_TEST_TESTTMPDIR/ruleset-after.json\"\`" >> "$NFT_TEST_TESTTMPDIR/rc-failed-dump" > rc_dump=1 > else > > Having such a "json-diff-pretty" script in the toolbox might be handy > for debugging anyway. I guess, it's somewhere under tests/py already? Please have a look at nft-tests.py and how it creates *.json.got files. Cheers, Phil
On Tue, Nov 21, 2023 at 02:22:54PM +0100, Thomas Haller wrote: > The "handle" in JSON output is not stable. Sanitize/normalize to zero. > > Adjust the sanitize code, and regenerate the .json-nft files. Applied, thanks. I had to adjust a json dump, this diff is not so difficult: --- testcases/sets/dumps/0062set_connlimit_0.json-nft 2023-11-22 10:34:55.767232540 +0100 +++ /tmp/nft-test.20231122-103617.664.6hTWZt/test-testcases-sets-0062set_connlimit_0.1/ruleset-after.json 2023-11-22 10:36:19.338350215 +0100 @@ -1 +1 @@ -{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "x", "handle": 0}}, {"set": {"family": "ip", "name": "est-connlimit", "table": "x", "type": "ipv4_addr", "handle": 0, "size": 65535, "flags": ["dynamic"], "elem": [{"elem": {"val": "84.245.120.167", "ct count": {"val": 20, "inv": true}}}]}}, {"set": {"family": "ip", "name": "new-connlimit", "table": "x", "type": "ipv4_addr", "handle": 0, "size": 65535, "flags": ["dynamic"], "elem": [{"elem": {"val": "84.245.120.167", "ct count": {"val": 20, "inv": true}}}], "stmt": [{"ct count": {"val": 20, "inv": true}}]}}]} +{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "x", "handle": 0}}, {"set": {"family": "ip", "name": "est-connlimit", "table": "x", "type": "ipv4_addr", "handle": 0, "size": 65535, "flags": ["dynamic"]}}, {"set": {"family": "ip", "name": "new-connlimit", "table": "x", "type": "ipv4_addr", "handle": 0, "size": 65535, "flags": ["dynamic"], "stmt": [{"ct count": {"val": 20, "inv": true}}]}}]} I had to adjust a different much larger dump though, and it feels a bit like searching the needle in the stack in the diff (if a bug ever shows up in this path). Usability improvement via python script similar to what tests/py does would be really great to have. Thanks.
On Wed, 2023-11-22 at 11:36 +0100, Pablo Neira Ayuso wrote: > On Tue, Nov 21, 2023 at 02:22:54PM +0100, Thomas Haller wrote: > > The "handle" in JSON output is not stable. Sanitize/normalize to > > zero. > > > > Adjust the sanitize code, and regenerate the .json-nft files. > > Applied, thanks. > > I had to adjust a json dump, this diff is not so difficult: Hi, Hm. The json dump of the patch was generated. If you had to "adjust" a dump, does that mean that the output is not stable? In that case, the .json-nft file should be removed instead (and the cause for the difference investigated, fixed, and the dump-re-added). Thomas
On Wed, Nov 22, 2023 at 11:44:54AM +0100, Thomas Haller wrote: > On Wed, 2023-11-22 at 11:36 +0100, Pablo Neira Ayuso wrote: > > On Tue, Nov 21, 2023 at 02:22:54PM +0100, Thomas Haller wrote: > > > The "handle" in JSON output is not stable. Sanitize/normalize to > > > zero. > > > > > > Adjust the sanitize code, and regenerate the .json-nft files. > > > > Applied, thanks. > > > > I had to adjust a json dump, this diff is not so difficult: > > Hi, > > Hm. The json dump of the patch was generated. > > If you had to "adjust" a dump, does that mean that the output is not > stable? I had to adjust json output after my recent series for 4.19 -stable kernels: https://patchwork.ozlabs.org/project/netfilter-devel/list/?series=383354 (note I splitted a few tests there) with your patch output looks stable now here after this patch with different kernel versions, so all good, thanks! > In that case, the .json-nft file should be removed instead (and the > cause for the difference investigated, fixed, and the dump-re-added). Agreed, but this different case as explained above. BTW, I am intentionally missing .json-nft files in my series because I am focusing on making progress on 4.19 backports, if you can help me with with missing .json-nft that I am living on my way, I'd appreciate. I promise to make a more careful look on missing .json-nft in the next series. Thanks.
On Wed, 2023-11-22 at 12:22 +0100, Pablo Neira Ayuso wrote: > On Wed, Nov 22, 2023 at 11:44:54AM +0100, Thomas Haller wrote: > > On Wed, 2023-11-22 at 11:36 +0100, Pablo Neira Ayuso wrote: > > > On Tue, Nov 21, 2023 at 02:22:54PM +0100, Thomas Haller wrote: > > > > The "handle" in JSON output is not stable. Sanitize/normalize > > > > to > > > > zero. > > > > > > > > Adjust the sanitize code, and regenerate the .json-nft files. > > > > > > Applied, thanks. > > > > > > I had to adjust a json dump, this diff is not so difficult: > > > > Hi, > > > > Hm. The json dump of the patch was generated. > > > > If you had to "adjust" a dump, does that mean that the output is > > not > > stable? > > I had to adjust json output after my recent series for 4.19 -stable > kernels: > > https://patchwork.ozlabs.org/project/netfilter-devel/list/?series=383354 > > (note I splitted a few tests there) > > with your patch output looks stable now here after this patch with > different kernel versions, so all good, thanks! > > > In that case, the .json-nft file should be removed instead (and the > > cause for the difference investigated, fixed, and the dump-re- > > added). > > Agreed, but this different case as explained above. ACK. Thanks. > BTW, I am intentionally missing .json-nft files in my series because > I > am focusing on making progress on 4.19 backports, if you can help me > with with missing .json-nft that I am living on my way, I'd > appreciate. > I promise to make a more careful look on missing .json-nft in the > next > series. Note that on `master` there are - 386 shell tests - 14 tests with .nodump files - 372 tests with .nft dump files (386-14) - 339 tests with .json-nft files - 33 tests that lack .json-nft files (372-339) Meaning, that they are IMO useful already, and there is no immediate hurry to address the missing 33 files. Thomas PS: Reminder: you can identify missing .json-nft files by running `./tools/check-tree.sh`. You can generate missing files either via $ DUMPGEN=all ./tests/shell/run-tests.sh tests/shell/testcases/cache/0010_implicit_chain_0 or just $ touch tests/shell/testcases/cache/dumps/0010_implicit_chain_0.json-nft $ ./tests/shell/run-tests.sh tests/shell/testcases/cache/0010_implicit_chain_0 -g
On Wed, Nov 22, 2023 at 01:16:38PM +0100, Thomas Haller wrote: [...] > > Note that on `master` there are > > - 386 shell tests > - 14 tests with .nodump files > - 372 tests with .nft dump files (386-14) > - 339 tests with .json-nft files > - 33 tests that lack .json-nft files (372-339) > > Meaning, that they are IMO useful already, and there is no immediate > hurry to address the missing 33 files. This is useful, almost complete coverage is better than no coverage :) > PS: Reminder: you can identify missing .json-nft files by running > `./tools/check-tree.sh`. Thanks. Maybe update tests/shell/README to add a few lines, I like to go there when I forget things and... $ git grep check-tree README $ shows nothing. > You can generate missing files either via > > $ DUMPGEN=all ./tests/shell/run-tests.sh tests/shell/testcases/cache/0010_implicit_chain_0 > > or just > > $ touch tests/shell/testcases/cache/dumps/0010_implicit_chain_0.json-nft > $ ./tests/shell/run-tests.sh tests/shell/testcases/cache/0010_implicit_chain_0 -g Yes, that is missing.
diff --git a/tests/shell/helpers/json-sanitize-ruleset.sh b/tests/shell/helpers/json-sanitize-ruleset.sh index 270a6107e0aa..3b66adabf055 100755 --- a/tests/shell/helpers/json-sanitize-ruleset.sh +++ b/tests/shell/helpers/json-sanitize-ruleset.sh @@ -6,7 +6,14 @@ die() { } do_sed() { - sed '1s/\({"nftables": \[{"metainfo": {"version": "\)[0-9.]\+\(", "release_name": "\)[^"]\+\(", "\)/\1VERSION\2RELEASE_NAME\3/' "$@" + # Normalize the "version"/"release_name", otherwise we have to regenerate the + # JSON output upon new release. + # + # Also, "handle" are not stable. Normalize them to 1216 (arbitrarily chosen). + sed \ + -e '1s/\({"nftables": \[{"metainfo": {"version": "\)[0-9.]\+\(", "release_name": "\)[^"]\+\(", "\)/\1VERSION\2RELEASE_NAME\3/' \ + -e '1s/"handle": [0-9]\+\>/"handle": 1216/g' \ + "$@" } if [ "$#" = 0 ] ; then diff --git a/tests/shell/helpers/test-wrapper.sh b/tests/shell/helpers/test-wrapper.sh index 62414d0db074..9e8e60581890 100755 --- a/tests/shell/helpers/test-wrapper.sh +++ b/tests/shell/helpers/test-wrapper.sh @@ -138,8 +138,7 @@ if [ "$NFT_TEST_HAVE_json" != n ] ; then show_file "$NFT_TEST_TESTTMPDIR/chkdump" "Command \`$NFT -j list ruleset\` failed" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump" rc_chkdump=1 fi - # Normalize the version number from the JSON output. Otherwise, we'd - # have to regenerate the .json-nft files upon release. + # JSON output needs normalization/sanitization, otherwise it's not stable. "$NFT_TEST_BASEDIR/helpers/json-sanitize-ruleset.sh" "$NFT_TEST_TESTTMPDIR/ruleset-after.json" fi diff --git a/tests/shell/testcases/bitwise/dumps/0040mark_binop_0.json-nft b/tests/shell/testcases/bitwise/dumps/0040mark_binop_0.json-nft index 782cde4225ff..83f7a3445244 100644 --- a/tests/shell/testcases/bitwise/dumps/0040mark_binop_0.json-nft +++ b/tests/shell/testcases/bitwise/dumps/0040mark_binop_0.json-nft @@ -1 +1 @@ -{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "t", "handle": 1}}, {"chain": {"family": "ip", "table": "t", "name": "c", "handle": 1, "type": "filter", "hook": "output", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip", "table": "t", "chain": "c", "handle": 2, "expr": [{"match": {"op": "==", "left": {"meta": {"key": "oif"}}, "right": "lo"}}, {"mangle": {"key": {"ct": {"key": "mark"}}, "value": {"<<": [{"|": [{"meta": {"key": "mark"}}, 16]}, 8]}}}]}}]} +{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "t", "handle": 1216}}, {"chain": {"family": "ip", "table": "t", "name": "c", "handle": 1216, "type": "filter", "hook": "output", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip", "table": "t", "chain": "c", "handle": 1216, "expr": [{"match": {"op": "==", "left": {"meta": {"key": "oif"}}, "right": "lo"}}, {"mangle": {"key": {"ct": {"key": "mark"}}, "value": {"<<": [{"|": [{"meta": {"key": "mark"}}, 16]}, 8]}}}]}}]} diff --git a/tests/shell/testcases/bitwise/dumps/0040mark_binop_1.json-nft b/tests/shell/testcases/bitwise/dumps/0040mark_binop_1.json-nft index b887fb5befa4..365e13388673 100644 --- a/tests/shell/testcases/bitwise/dumps/0040mark_binop_1.json-nft +++ b/tests/shell/testcases/bitwise/dumps/0040mark_binop_1.json-nft @@ -1 +1 @@ -{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "t", "handle": 1}}, {"chain": {"family": "ip", "table": "t", "name": "c", "handle": 1, "type": "filter", "hook": "input", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip", "table": "t", "chain": "c", "handle": 2, "expr": [{"match": {"op": "==", "left": {"meta": {"key": "iif"}}, "right": "lo"}}, {"match": {"op": "==", "left": {"&": [{"ct": {"key": "mark"}}, 255]}, "right": 16}}, {"mangle": {"key": {"meta": {"key": "mark"}}, "value": {">>": [{"ct": {"key": "mark"}}, 8]}}}]}}]} +{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "t", "handle": 1216}}, {"chain": {"family": "ip", "table": "t", "name": "c", "handle": 1216, "type": "filter", "hook": "input", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip", "table": "t", "chain": "c", "handle": 1216, "expr": [{"match": {"op": "==", "left": {"meta": {"key": "iif"}}, "right": "lo"}}, {"match": {"op": "==", "left": {"&": [{"ct": {"key": "mark"}}, 255]}, "right": 16}}, {"mangle": {"key": {"meta": {"key": "mark"}}, "value": {">>": [{"ct": {"key": "mark"}}, 8]}}}]}}]} diff --git a/tests/shell/testcases/bitwise/dumps/0040mark_binop_2.json-nft b/tests/shell/testcases/bitwise/dumps/0040mark_binop_2.json-nft index 4ebe509c1cf6..cad1ea57e30d 100644 --- a/tests/shell/testcases/bitwise/dumps/0040mark_binop_2.json-nft +++ b/tests/shell/testcases/bitwise/dumps/0040mark_binop_2.json-nft @@ -1 +1 @@ -{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "t", "handle": 1}}, {"chain": {"family": "ip", "table": "t", "name": "c", "handle": 1, "type": "filter", "hook": "output", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip", "table": "t", "chain": "c", "handle": 2, "expr": [{"mangle": {"key": {"ct": {"key": "mark"}}, "value": {"|": [{"<<": [{"payload": {"protocol": "ip", "field": "dscp"}}, 2]}, 16]}}}]}}]} +{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "t", "handle": 1216}}, {"chain": {"family": "ip", "table": "t", "name": "c", "handle": 1216, "type": "filter", "hook": "output", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip", "table": "t", "chain": "c", "handle": 1216, "expr": [{"mangle": {"key": {"ct": {"key": "mark"}}, "value": {"|": [{"<<": [{"payload": {"protocol": "ip", "field": "dscp"}}, 2]}, 16]}}}]}}]} diff --git a/tests/shell/testcases/bitwise/dumps/0040mark_binop_3.json-nft b/tests/shell/testcases/bitwise/dumps/0040mark_binop_3.json-nft index df64f4e1ba84..d92d62dfe56f 100644 --- a/tests/shell/testcases/bitwise/dumps/0040mark_binop_3.json-nft +++ b/tests/shell/testcases/bitwise/dumps/0040mark_binop_3.json-nft @@ -1 +1 @@ -{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "t", "handle": 1}}, {"chain": {"family": "ip", "table": "t", "name": "c", "handle": 1, "type": "filter", "hook": "input", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip", "table": "t", "chain": "c", "handle": 2, "expr": [{"mangle": {"key": {"meta": {"key": "mark"}}, "value": {"|": [{"<<": [{"payload": {"protocol": "ip", "field": "dscp"}}, 2]}, 16]}}}]}}]} +{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "t", "handle": 1216}}, {"chain": {"family": "ip", "table": "t", "name": "c", "handle": 1216, "type": "filter", "hook": "input", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip", "table": "t", "chain": "c", "handle": 1216, "expr": [{"mangle": {"key": {"meta": {"key": "mark"}}, "value": {"|": [{"<<": [{"payload": {"protocol": "ip", "field": "dscp"}}, 2]}, 16]}}}]}}]} diff --git a/tests/shell/testcases/bitwise/dumps/0040mark_binop_4.json-nft b/tests/shell/testcases/bitwise/dumps/0040mark_binop_4.json-nft index 76bb83cff96f..d56adbbcf34c 100644 --- a/tests/shell/testcases/bitwise/dumps/0040mark_binop_4.json-nft +++ b/tests/shell/testcases/bitwise/dumps/0040mark_binop_4.json-nft @@ -1 +1 @@ -{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "t", "handle": 1}}, {"chain": {"family": "ip", "table": "t", "name": "c", "handle": 1, "type": "filter", "hook": "output", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip", "table": "t", "chain": "c", "handle": 2, "expr": [{"mangle": {"key": {"ct": {"key": "mark"}}, "value": {"|": [{"<<": [{"payload": {"protocol": "ip", "field": "dscp"}}, 26]}, 16]}}}]}}]} +{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "t", "handle": 1216}}, {"chain": {"family": "ip", "table": "t", "name": "c", "handle": 1216, "type": "filter", "hook": "output", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip", "table": "t", "chain": "c", "handle": 1216, "expr": [{"mangle": {"key": {"ct": {"key": "mark"}}, "value": {"|": [{"<<": [{"payload": {"protocol": "ip", "field": "dscp"}}, 26]}, 16]}}}]}}]} diff --git a/tests/shell/testcases/bitwise/dumps/0040mark_binop_5.json-nft b/tests/shell/testcases/bitwise/dumps/0040mark_binop_5.json-nft index eaa9df04fa3c..8cc9fecd2ec6 100644 --- a/tests/shell/testcases/bitwise/dumps/0040mark_binop_5.json-nft +++ b/tests/shell/testcases/bitwise/dumps/0040mark_binop_5.json-nft @@ -1 +1 @@ -{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "t", "handle": 1}}, {"chain": {"family": "ip", "table": "t", "name": "c", "handle": 1, "type": "filter", "hook": "input", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip", "table": "t", "chain": "c", "handle": 2, "expr": [{"mangle": {"key": {"meta": {"key": "mark"}}, "value": {"|": [{"<<": [{"payload": {"protocol": "ip", "field": "dscp"}}, 26]}, 16]}}}]}}]} +{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "t", "handle": 1216}}, {"chain": {"family": "ip", "table": "t", "name": "c", "handle": 1216, "type": "filter", "hook": "input", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip", "table": "t", "chain": "c", "handle": 1216, "expr": [{"mangle": {"key": {"meta": {"key": "mark"}}, "value": {"|": [{"<<": [{"payload": {"protocol": "ip", "field": "dscp"}}, 26]}, 16]}}}]}}]} diff --git a/tests/shell/testcases/bitwise/dumps/0040mark_binop_6.json-nft b/tests/shell/testcases/bitwise/dumps/0040mark_binop_6.json-nft index 100c604ba5c5..bc439fa67db8 100644 --- a/tests/shell/testcases/bitwise/dumps/0040mark_binop_6.json-nft +++ b/tests/shell/testcases/bitwise/dumps/0040mark_binop_6.json-nft @@ -1 +1 @@ -{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip6", "name": "t", "handle": 1}}, {"chain": {"family": "ip6", "table": "t", "name": "c", "handle": 1, "type": "filter", "hook": "output", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip6", "table": "t", "chain": "c", "handle": 2, "expr": [{"mangle": {"key": {"ct": {"key": "mark"}}, "value": {"|": [{"<<": [{"payload": {"protocol": "ip6", "field": "dscp"}}, 2]}, 16]}}}]}}]} +{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip6", "name": "t", "handle": 1216}}, {"chain": {"family": "ip6", "table": "t", "name": "c", "handle": 1216, "type": "filter", "hook": "output", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip6", "table": "t", "chain": "c", "handle": 1216, "expr": [{"mangle": {"key": {"ct": {"key": "mark"}}, "value": {"|": [{"<<": [{"payload": {"protocol": "ip6", "field": "dscp"}}, 2]}, 16]}}}]}}]} diff --git a/tests/shell/testcases/bitwise/dumps/0040mark_binop_7.json-nft b/tests/shell/testcases/bitwise/dumps/0040mark_binop_7.json-nft index 0e61a15eee2a..7eb6712254d6 100644 --- a/tests/shell/testcases/bitwise/dumps/0040mark_binop_7.json-nft +++ b/tests/shell/testcases/bitwise/dumps/0040mark_binop_7.json-nft @@ -1 +1 @@ -{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip6", "name": "t", "handle": 1}}, {"chain": {"family": "ip6", "table": "t", "name": "c", "handle": 1, "type": "filter", "hook": "input", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip6", "table": "t", "chain": "c", "handle": 2, "expr": [{"mangle": {"key": {"meta": {"key": "mark"}}, "value": {"|": [{"<<": [{"payload": {"protocol": "ip6", "field": "dscp"}}, 2]}, 16]}}}]}}]} +{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip6", "name": "t", "handle": 1216}}, {"chain": {"family": "ip6", "table": "t", "name": "c", "handle": 1216, "type": "filter", "hook": "input", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip6", "table": "t", "chain": "c", "handle": 1216, "expr": [{"mangle": {"key": {"meta": {"key": "mark"}}, "value": {"|": [{"<<": [{"payload": {"protocol": "ip6", "field": "dscp"}}, 2]}, 16]}}}]}}]} diff --git a/tests/shell/testcases/bitwise/dumps/0040mark_binop_8.json-nft b/tests/shell/testcases/bitwise/dumps/0040mark_binop_8.json-nft index f077295c7b42..d41a6f29386d 100644 --- a/tests/shell/testcases/bitwise/dumps/0040mark_binop_8.json-nft +++ b/tests/shell/testcases/bitwise/dumps/0040mark_binop_8.json-nft @@ -1 +1 @@ -{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip6", "name": "t", "handle": 1}}, {"chain": {"family": "ip6", "table": "t", "name": "c", "handle": 1, "type": "filter", "hook": "output", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip6", "table": "t", "chain": "c", "handle": 2, "expr": [{"mangle": {"key": {"ct": {"key": "mark"}}, "value": {"|": [{"<<": [{"payload": {"protocol": "ip6", "field": "dscp"}}, 26]}, 16]}}}]}}]} +{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip6", "name": "t", "handle": 1216}}, {"chain": {"family": "ip6", "table": "t", "name": "c", "handle": 1216, "type": "filter", "hook": "output", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip6", "table": "t", "chain": "c", "handle": 1216, "expr": [{"mangle": {"key": {"ct": {"key": "mark"}}, "value": {"|": [{"<<": [{"payload": {"protocol": "ip6", "field": "dscp"}}, 26]}, 16]}}}]}}]} diff --git a/tests/shell/testcases/bitwise/dumps/0040mark_binop_9.json-nft b/tests/shell/testcases/bitwise/dumps/0040mark_binop_9.json-nft index a71eebaea7f4..554153980427 100644 --- a/tests/shell/testcases/bitwise/dumps/0040mark_binop_9.json-nft +++ b/tests/shell/testcases/bitwise/dumps/0040mark_binop_9.json-nft @@ -1 +1 @@ -{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip6", "name": "t", "handle": 1}}, {"chain": {"family": "ip6", "table": "t", "name": "c", "handle": 1, "type": "filter", "hook": "input", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip6", "table": "t", "chain": "c", "handle": 2, "expr": [{"mangle": {"key": {"meta": {"key": "mark"}}, "value": {"|": [{"<<": [{"payload": {"protocol": "ip6", "field": "dscp"}}, 26]}, 16]}}}]}}]} +{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip6", "name": "t", "handle": 1216}}, {"chain": {"family": "ip6", "table": "t", "name": "c", "handle": 1216, "type": "filter", "hook": "input", "prio": 0, "policy": "accept"}}, {"rule": {"family": "ip6", "table": "t", "chain": "c", "handle": 1216, "expr": [{"mangle": {"key": {"meta": {"key": "mark"}}, "value": {"|": [{"<<": [{"payload": {"protocol": "ip6", "field": "dscp"}}, 26]}, 16]}}}]}}]} diff --git a/tests/shell/testcases/sets/dumps/0043concatenated_ranges_0.json-nft b/tests/shell/testcases/sets/dumps/0043concatenated_ranges_0.json-nft index 9d5ef47dfd7c..a4e4100446e1 100644 --- a/tests/shell/testcases/sets/dumps/0043concatenated_ranges_0.json-nft +++ b/tests/shell/testcases/sets/dumps/0043concatenated_ranges_0.json-nft @@ -1 +1 @@ -{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "inet", "name": "filter", "handle": 192}}, {"map": {"family": "inet", "name": "test", "table": "filter", "type": ["mark", "inet_service", "inet_proto"], "handle": 2, "map": "mark", "flags": ["interval", "timeout"]}}, {"chain": {"family": "inet", "table": "filter", "name": "output", "handle": 1, "type": "filter", "hook": "output", "prio": 0, "policy": "accept"}}, {"rule": {"family": "inet", "table": "filter", "chain": "output", "handle": 3, "expr": [{"mangle": {"key": {"meta": {"key": "mark"}}, "value": {"map": {"key": {"concat": [{"meta": {"key": "mark"}}, {"payload": {"protocol": "tcp", "field": "dport"}}, {"meta": {"key": "l4proto"}}]}, "data": "@test"}}}}, {"counter": {"packets": 0, "bytes": 0}}]}}]} +{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "inet", "name": "filter", "handle": 1216}}, {"map": {"family": "inet", "name": "test", "table": "filter", "type": ["mark", "inet_service", "inet_proto"], "handle": 1216, "map": "mark", "flags": ["interval", "timeout"]}}, {"chain": {"family": "inet", "table": "filter", "name": "output", "handle": 1216, "type": "filter", "hook": "output", "prio": 0, "policy": "accept"}}, {"rule": {"family": "inet", "table": "filter", "chain": "output", "handle": 1216, "expr": [{"mangle": {"key": {"meta": {"key": "mark"}}, "value": {"map": {"key": {"concat": [{"meta": {"key": "mark"}}, {"payload": {"protocol": "tcp", "field": "dport"}}, {"meta": {"key": "l4proto"}}]}, "data": "@test"}}}}, {"counter": {"packets": 0, "bytes": 0}}]}}]} diff --git a/tests/shell/testcases/sets/dumps/0044interval_overlap_1.json-nft b/tests/shell/testcases/sets/dumps/0044interval_overlap_1.json-nft index db9340bcd1a1..af738f15bdb6 100644 --- a/tests/shell/testcases/sets/dumps/0044interval_overlap_1.json-nft +++ b/tests/shell/testcases/sets/dumps/0044interval_overlap_1.json-nft @@ -1 +1 @@ -{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "t", "handle": 1}}, {"set": {"family": "ip", "name": "s", "table": "t", "type": "inet_service", "handle": 1, "flags": ["interval"], "elem": [25, 30, 82, 119, 349, 745, 748, 1165, 1233, 1476, 1550, 1562, 1743, 1745, 1882, 2070, 2194, 2238, 2450, 2455, 2642, 2671, 2906, 3093, 3203, 3287, 3348, 3411, 3540, 3892, 3943, 4133, 4205, 4317, 4733, 5095, 5156, 5223, 5230, 5432, 5826, 5828, 6044, 6377, 6388, 6491, 6952, 6986, 7012, 7187, 7300, 7305, 7549, 7664, 8111, 8206, 8396, 8782, 8920, 8981, 9067, 9216, 9245, 9315, 9432, 9587, 9689, 9844, 9991, 10045, 10252, 10328, 10670, 10907, 11021, 11337, 11427, 11497, 11502, 11523, 11552, 11577, 11721, 11943, 12474, 12718, 12764, 12794, 12922, 13186, 13232, 13383, 13431, 13551, 13676, 13685, 13747, 13925, 13935, 14015, 14090, 14320, 14392, 14515, 14647, 14911, 15096, 15105, 15154, 15440, 15583, 15623, 15677, 15710, 15926, 15934, 15960, 16068, 16166, 16486, 16489, 16528, 16646, 16650, 16770, 16882, 17052, 17237, 17387, 17431, 17886, 17939, 17999, 18092, 18123, 18238, 18562, 18698, 19004, 19229, 19237, 19585, 19879, 19938, 19950, 19958, 20031, 20138, 20157, 20205, 20368, 20682, 20687, 20873, 20910, 20919, 21019, 21068, 21115, 21188, 21236, 21319, 21563, 21734, 21806, 21810, 21959, 21982, 22078, 22181, 22308, 22480, 22643, 22854, 22879, 22961, 23397, 23534, 23845, 23893, 24130, 24406, 24794, 24997, 25019, 25143, 25179, 25439, 25603, 25718, 25859, 25949, 26006, 26022, 26047, 26170, 26193, 26725, 26747, 26924, 27023, 27040, 27233, 27344, 27478, 27593, 27600, 27664, 27678, 27818, 27822, 28003, 28038, 28709, 28808, 29010, 29057, 29228, 29485, 30132, 30160, 30415, 30469, 30673, 30736, 30776, 30780, 31450, 31537, 31669, 31839, 31873, 32019, 32229, 32685, 32879, 33318, 33337, 33404, 33517, 33906, 34214, 34346, 34416, 34727, 34848, 35325, 35400, 35451, 35501, 35637, 35653, 35710, 35761, 35767, 36238, 36258, 36279, 36464, 36586, 36603, 36770, 36774, 36805, 36851, 37079, 37189, 37209, 37565, 37570, 37585, 37832, 37931, 37954, 38006, 38015, 38045, 38109, 38114, 38200, 38209, 38214, 38277, 38306, 38402, 38606, 38697, 38960, 39004, 39006, 39197, 39217, 39265, 39319, 39460, 39550, 39615, 39871, 39886, 40088, 40135, 40244, 40323, 40339, 40355, 40385, 40428, 40538, 40791, 40848, 40959, 41003, 41131, 41349, 41643, 41710, 41826, 41904, 42027, 42148, 42235, 42255, 42498, 42680, 42973, 43118, 43135, 43233, 43349, 43411, 43487, 43840, 43843, 43870, 44040, 44204, 44817, 44883, 44894, 44958, 45201, 45259, 45283, 45357, 45423, 45473, 45498, 45519, 45561, 45611, 45627, 45831, 46043, 46105, 46116, 46147, 46169, 46349, 47147, 47252, 47314, 47335, 47360, 47546, 47617, 47648, 47772, 47793, 47846, 47913, 47952, 48095, 48325, 48334, 48412, 48419, 48540, 48569, 48628, 48751, 48944, 48971, 49008, 49025, 49503, 49505, 49613, 49767, 49839, 49925, 50022, 50028, 50238, 51057, 51477, 51617, 51910, 52044, 52482, 52550, 52643, 52832, 53382, 53690, 53809, 53858, 54001, 54198, 54280, 54327, 54376, 54609, 54776, 54983, 54984, 55019, 55038, 55094, 55368, 55737, 55793, 55904, 55941, 55960, 55978, 56063, 56121, 56314, 56505, 56548, 56568, 56696, 56798, 56855, 57102, 57236, 57333, 57334, 57441, 57574, 57659, 57987, 58325, 58404, 58509, 58782, 58876, 59116, 59544, 59685, 59700, 59750, 59799, 59866, 59870, 59894, 59984, 60343, 60481, 60564, 60731, 61075, 61087, 61148, 61174, 61655, 61679, 61691, 61723, 61730, 61758, 61824, 62035, 62056, 62661, 62768, 62946, 63059, 63116, 63338, 63387, 63672, 63719, 63881, 63995, 64197, 64374, 64377, 64472, 64606, 64662, 64777, 64795, 64906, 65049, 65122, 65318]}}]} +{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "t", "handle": 1216}}, {"set": {"family": "ip", "name": "s", "table": "t", "type": "inet_service", "handle": 1216, "flags": ["interval"], "elem": [25, 30, 82, 119, 349, 745, 748, 1165, 1233, 1476, 1550, 1562, 1743, 1745, 1882, 2070, 2194, 2238, 2450, 2455, 2642, 2671, 2906, 3093, 3203, 3287, 3348, 3411, 3540, 3892, 3943, 4133, 4205, 4317, 4733, 5095, 5156, 5223, 5230, 5432, 5826, 5828, 6044, 6377, 6388, 6491, 6952, 6986, 7012, 7187, 7300, 7305, 7549, 7664, 8111, 8206, 8396, 8782, 8920, 8981, 9067, 9216, 9245, 9315, 9432, 9587, 9689, 9844, 9991, 10045, 10252, 10328, 10670, 10907, 11021, 11337, 11427, 11497, 11502, 11523, 11552, 11577, 11721, 11943, 12474, 12718, 12764, 12794, 12922, 13186, 13232, 13383, 13431, 13551, 13676, 13685, 13747, 13925, 13935, 14015, 14090, 14320, 14392, 14515, 14647, 14911, 15096, 15105, 15154, 15440, 15583, 15623, 15677, 15710, 15926, 15934, 15960, 16068, 16166, 16486, 16489, 16528, 16646, 16650, 16770, 16882, 17052, 17237, 17387, 17431, 17886, 17939, 17999, 18092, 18123, 18238, 18562, 18698, 19004, 19229, 19237, 19585, 19879, 19938, 19950, 19958, 20031, 20138, 20157, 20205, 20368, 20682, 20687, 20873, 20910, 20919, 21019, 21068, 21115, 21188, 21236, 21319, 21563, 21734, 21806, 21810, 21959, 21982, 22078, 22181, 22308, 22480, 22643, 22854, 22879, 22961, 23397, 23534, 23845, 23893, 24130, 24406, 24794, 24997, 25019, 25143, 25179, 25439, 25603, 25718, 25859, 25949, 26006, 26022, 26047, 26170, 26193, 26725, 26747, 26924, 27023, 27040, 27233, 27344, 27478, 27593, 27600, 27664, 27678, 27818, 27822, 28003, 28038, 28709, 28808, 29010, 29057, 29228, 29485, 30132, 30160, 30415, 30469, 30673, 30736, 30776, 30780, 31450, 31537, 31669, 31839, 31873, 32019, 32229, 32685, 32879, 33318, 33337, 33404, 33517, 33906, 34214, 34346, 34416, 34727, 34848, 35325, 35400, 35451, 35501, 35637, 35653, 35710, 35761, 35767, 36238, 36258, 36279, 36464, 36586, 36603, 36770, 36774, 36805, 36851, 37079, 37189, 37209, 37565, 37570, 37585, 37832, 37931, 37954, 38006, 38015, 38045, 38109, 38114, 38200, 38209, 38214, 38277, 38306, 38402, 38606, 38697, 38960, 39004, 39006, 39197, 39217, 39265, 39319, 39460, 39550, 39615, 39871, 39886, 40088, 40135, 40244, 40323, 40339, 40355, 40385, 40428, 40538, 40791, 40848, 40959, 41003, 41131, 41349, 41643, 41710, 41826, 41904, 42027, 42148, 42235, 42255, 42498, 42680, 42973, 43118, 43135, 43233, 43349, 43411, 43487, 43840, 43843, 43870, 44040, 44204, 44817, 44883, 44894, 44958, 45201, 45259, 45283, 45357, 45423, 45473, 45498, 45519, 45561, 45611, 45627, 45831, 46043, 46105, 46116, 46147, 46169, 46349, 47147, 47252, 47314, 47335, 47360, 47546, 47617, 47648, 47772, 47793, 47846, 47913, 47952, 48095, 48325, 48334, 48412, 48419, 48540, 48569, 48628, 48751, 48944, 48971, 49008, 49025, 49503, 49505, 49613, 49767, 49839, 49925, 50022, 50028, 50238, 51057, 51477, 51617, 51910, 52044, 52482, 52550, 52643, 52832, 53382, 53690, 53809, 53858, 54001, 54198, 54280, 54327, 54376, 54609, 54776, 54983, 54984, 55019, 55038, 55094, 55368, 55737, 55793, 55904, 55941, 55960, 55978, 56063, 56121, 56314, 56505, 56548, 56568, 56696, 56798, 56855, 57102, 57236, 57333, 57334, 57441, 57574, 57659, 57987, 58325, 58404, 58509, 58782, 58876, 59116, 59544, 59685, 59700, 59750, 59799, 59866, 59870, 59894, 59984, 60343, 60481, 60564, 60731, 61075, 61087, 61148, 61174, 61655, 61679, 61691, 61723, 61730, 61758, 61824, 62035, 62056, 62661, 62768, 62946, 63059, 63116, 63338, 63387, 63672, 63719, 63881, 63995, 64197, 64374, 64377, 64472, 64606, 64662, 64777, 64795, 64906, 65049, 65122, 65318]}}]}
The "handle" in JSON output is not stable. Sanitize/normalizeit to 1216. The number is chosen arbitrarily, but it's somewhat unique in the code base. So when you see it, you may guess it originates from sanitization. Signed-off-by: Thomas Haller <thaller@redhat.com> --- Note that only a few .json-nft files are adjusted, because otherwise the patch is too large. Before applying, you need to adjust them all, by running `./tests/shell/run-tests.sh -g`. tests/shell/helpers/json-sanitize-ruleset.sh | 9 ++++++++- tests/shell/helpers/test-wrapper.sh | 3 +-- .../testcases/bitwise/dumps/0040mark_binop_0.json-nft | 2 +- .../testcases/bitwise/dumps/0040mark_binop_1.json-nft | 2 +- .../testcases/bitwise/dumps/0040mark_binop_2.json-nft | 2 +- .../testcases/bitwise/dumps/0040mark_binop_3.json-nft | 2 +- .../testcases/bitwise/dumps/0040mark_binop_4.json-nft | 2 +- .../testcases/bitwise/dumps/0040mark_binop_5.json-nft | 2 +- .../testcases/bitwise/dumps/0040mark_binop_6.json-nft | 2 +- .../testcases/bitwise/dumps/0040mark_binop_7.json-nft | 2 +- .../testcases/bitwise/dumps/0040mark_binop_8.json-nft | 2 +- .../testcases/bitwise/dumps/0040mark_binop_9.json-nft | 2 +- .../sets/dumps/0043concatenated_ranges_0.json-nft | 2 +- .../testcases/sets/dumps/0044interval_overlap_1.json-nft | 2 +- 14 files changed, 21 insertions(+), 15 deletions(-)