Improve and unify the handling of regular expressions when searching in logs, files and command output in system tests.
- Use `Re()` for constructing regular expressions, which is an imported shorthand for `re.compile()` (imported as `from re import compile as Re`
- Add new `isctest.text.Text` interface which is a text wrapper that supports the `in` operator for line matching operation for both strings and regular expressions, e.g.:
- `assert "running" in ns1.log`
- `assert Re("a.example..*10.0.0.1") in response.out`
- Use the new `isctest.text.Text` for:
- `isctest.run.cmd()` output, where `.out` and `.err` can be used for stdout and stderr contents
- `NamedInstance.log` rather than the previous log interface (`.expect()` and `.prohibit()` is no longer available or needed. The `in` operator along with an `assert` statement can be used now instead.)
- `NamedInstance.rndc()` output, which returns identical output as `isctest.run.cmd()`
Merge branch 'nicki/pytest-grep' into 'main'
See merge request isc-projects/bind9!11054
To unify the command handling, utilize EnvCmd() to handle rndc commands:
1. Remove isctest.rndc abstractions. They were intended for an upcoming
python-only implementation. A couple of years later, it doesn't seem
to be coming any time soon, so let's stick with the interface that
makes sense today, i.e. use the same command handling interface
everywhere.
2. Remove the specialized rndc.log in favor of the generic logging
already implemented by isctest.run.cmd(). I believe the cause of the
many rndc(log=False) invocations was that nobody wanted this extra
file. Yet, logging everything by default makes sense for debugging,
unless there's a good reason not to. In almost all cases, logging was
switched to the default (enabled).
3. With the NamedInstance.rndc() call now returning CmdResult rather
than combined stdout+stderr string, adjust all the invocations to use
`.out` or `.err` as necessary.
4. Replace some manual rndc invocation and its base argument
construction with the standardized nsX.rndc() call.
5. In cases where rndc is expected to fail, utilize
raise_on_exception=False and check the `.rc` from the result, rather
than handling an exception.
6. In addzone/tests_rndc_deadlock.py, refactor the test slightly to
avoid using EnvCmd() entirely to avoid spamming the logs. This test
calls rndc in a loop from multiple threads and such test case is an
exception which doesn't warrant changing the `isctest.run.cmd()`
implementation.
A generic helper that calls the environment-specified binaries in a
developer-friendly manner, i.e. passing arguments as strings rather than
having to split them first.
The isctest.run.cmd() remains as the basis which provides a clean and
robust interface, while the isctest.run.EnvCmd() can be used as a
convenient wrapper for tests, or when there are some shared default
parameters.
The isctest.run.Dig() is superseded with the isctest.run.EnvCmd(). In
the future, we might revisit adding Dig() or command-specific helpers
again, but it probably only makes sense if they offer command-aware
attributes / methods, rather than just being shortcuts to
isctest.run.EnvCmd().
Refactor the file handling to write to a file directly when calling
isctest.run.cmd().
Refactor the existing code to use CmdResult rather than out and err
separately.
Add a new Grep-like interface which can be used for searching for
regular expressions in files. Replace the prior LogFile used for named
logs with the new TextFile interface.
Add a new module for working with text and keep the isctest.log.watchlog
module focused on its purpose. Move LogFile and LineReader into the new
module. Add compile_pattern() helper which will be useful in subsequent
commits.
It's a fairly common pattern to use regular expression in our tests.
Instead of using the fairly verbose re.compile(), import that function
as Re() instead to allow for more brevity in the test syntax.
When creating an NSEC3 opt-out chain, a node in the chain could be removed too soon, causing the previous NSEC3 being unable to be found, resulting in invalid NSEC3 records to be left in the zone. This has been fixed.
Closes#5671
Merge branch '5671-fix-dbiterator-prev' into 'main'
See merge request isc-projects/bind9!11328
dns_qpiter_{prev,next} requires the current iterator node to still be
valid which might not always the case after dereference_iter_node was
called. Currently, this is ensured via closeversion() mechanism, but it
is not guaranteed to be true in the future.
Move the call to dereference_iter_node to after the dns_qpiter_prev()
and dns_qpiter_next() to prevent a possible use-after-free of the
current iterator node.
'kasp->lock' was not released before returning. This could result in
named locking up if 'dns_keymgr_status' fails when 'rndc dnssec -status'
is called.
Closes#5675
Merge branch '5675-missing-unlock' into 'main'
See merge request isc-projects/bind9!11338
Follow-up of 38ce2906 as the size of the `cfg_obj_t` can actually goes
down to 40 bytes "for free", by using bitfields to only use 31 bits for
the `line` field, so the remaining bit can be use to hold the `cloned`
state without paying the extra 8 bytes padding.
Merge branch 'colin/cfgobj-40bytes' into 'main'
See merge request isc-projects/bind9!11334
Follow-up of 38ce2906 as the size of the `cfg_obj_t` can actually goes
down to 40 bytes "for free", by using bitfields to only use 31 bits for
the `line` field, so the remaining bit can be use to hold the `cloned`
state without paying the extra 8 bytes padding.
`padding` is incompatible with TSIG and SIG(0), not with "no" TSIG
and SIG(0).
Merge branch 'each-fix-padding-doc' into 'main'
See merge request isc-projects/bind9!11333
Extended DNS Error 13 (Cached Error) is now returned when the server
answers a message from a cached SERVFAIL.
See RFC 8914 section 4.14.
See #1836
Merge branch '1836-sfcache-ede' into 'main'
See merge request isc-projects/bind9!11322
A manual rollover when the zone is in an invalid DNSSEC state causes predecessor keys to be removed too quickly. Additional safeguards to prevent this have been added. DNSSEC records will not be removed from the zone until the underlying state machine has moved back into a valid DNSSEC state.
Closes#5458
Merge branch '5458-safeguard-against-key-rollovers-when-in-invalid-state' into 'main'
See merge request isc-projects/bind9!10813
The manykeys test case relies on keys being removed. Make sure the
zone is fully signed with the keys that will stay, so the other keys
may be removed safely.
This means the expected number of signatures generated and refreshed
will change. The CDS and CDNSKEY RRset also need to be signed now.
Configure the test case with sig-signing-signatures 100, large enough
that the entire zone is processed in a single step.
The nsec3 system test has a couple of cases where the configured policy
changes the algorithm, effectively triggering an algorithm rollover. Fix
those cases to start in a valid DNSSEC state. Then fix the expected key
states, no longer should the old algorithm be removed immediately.
When creating keys, set Publish and Activate times so that keys will
be initialized as omnipresent. This way we start with a safe DNSSEC
state. In most cases at least, because some tests depend on special
key timings.
The ttl[1-4].example cases have become incorrect. With dnssec-policy
we require the TTL to match the dnskey-ttl from the policy.
The delzsk.example will have a ZSK removed from the zone. It also
requires that the DNSKEY RRset is already published. This means
that for the existing keys the, no longer "is now published"
messages will be logged.
The nsec-only.example and reconf.example zones are fixed to have a
correct matching policy.
This all means the expected count of log messages changes slightly.
This test case enables DNSSEC and has a mismatch in policy. Fix the
policy so that it matches the existing key set, and adjust the
expected answer count because no longer a new key is generated.
If the keymgr state machine is in an invalid state, it tries to move
it self to a valid state. But when you do key rollovers during an
invalid state, and the next state is also an invalid state, the keymgr
will happily do the transition.
It would be good to not do key rollovers if there is not a KSK and ZSK
fully omnipresent. But also it would be good to safeguard against
unexpected transitions.
This commit does that by not moving things to unretentive (which is
the state where we would remove the corresponding record from the zone)
if the state machine is currently in an invalid state.
Make all non-scalar properties of `cfg_obj_t` allocated values, which
ensures the union size is the width of one pointer. Also reorder the
fields inside `cfg_obj_t` to avoid alignment padding that would increase
the size. As a result, a `cfg_obj_t` instance is now 48 bytes on a
64-bit platform.
Add a static assertion to avoid increasing the size of the struct by
mistake.
The function `parse_sockaddrsub` was taking advantage of the fact that
both sockaddr and sockaddrtls were in the same position, and used to
initialize the sockaddr field independently if this was a -tls one or
not. This doesn't work anymore now that all fields are allocated,
so it has been slightly rewritten to take both cases into account
separately.
Merge branch 'colin/cfgobj-48bytes' into 'main'
See merge request isc-projects/bind9!11239
Make all non-scalar properties of `cfg_obj_t` allocated values, which
ensures the union size is the width of one pointer. Also reorder the
fields inside `cfg_obj_t` to avoid alignment padding that would increase
the size. As a result, a `cfg_obj_t` instance is now 48 bytes on a
64-bit platform.
Add a static assertion to avoid increasing the size of the struct by
mistake.
The function `parse_sockaddrsub` was taking advantage of the fact that
both sockaddr and sockaddrtls were in the same position, and used to
initialize the sockaddr field independently if this was a -tls one or
not. This doesn't work anymore now that all fields are allocated,
so it has been slightly rewritten to take both cases into account
separately.
Removes the `cfg_obj_t` memory context pointer, as the parser always uses `isc_g_mctx`. This simplifies the parser API/configuration tree API (no need to pass the memory context); and the `cfg_obj_t` size goes down from 80 bytes to 72 bytes.
While not directly related to the changes, also remove the `cfg_parser_t` `references` field as it is not used anymore (since the `cfg_obj_t` types doesn't reference it anymore).
Merge branch 'colin/remove-memctx-cfgobj' into 'main'
See merge request isc-projects/bind9!11199
Document the way `__attribute__((__constructor__))` and
`__attribute__((__destructor__))` must be used in BIND9 libraries in
order to avoid unexpected behaviors with other third-party libraries.
The `--memstats` option from cfg_test is unused, and even if used, does
nothing because `--memstats` relies on `isc_mem_stats` which dump memory
pools statistics, which are not used at all for configuration.
Also, dropping the option avoid to add a parser API to get the memory
stats (as the parser now uses the global memory context).
As the isccfg library now uses the global memory context, it is now
used directly instead of passing the parser context around to grab its
memory context.
Also remove the memory context from the parser, as well as from
`cfg_obj_t`, as it's now useless.
The parser has a static function `create_string()` used
internally. But there was duplicate code to create a string node
in `namedconf.c`. Instead of implementing the same logic twice,
`create_string()` is now publicly exposed as `cfg_string_create()`.
Previously, there were over 40 separate definitions of `CHECK` macros, of
which most used `goto cleanup`, and the rest `goto failure` or `goto out`.
There were another 10 definitions of `RETERR`, of which most were identical
to `CHECK`, but some simply returned a result code instead
of jumping to a cleanup label.
This has now been standardized throughout the code base: `RETERR` is for
returning an error code in the case of an error, and `CHECK` is for jumping
to a cleanup tag, which is now always called `cleanup`. Both macros are
defined in `isc/util.h`.
Merge branch 'each-check-and-cleanup' into 'main'
See merge request isc-projects/bind9!10472
CLEANUP is a macro similar to CHECK but unconditional, jumping
to cleanup even if the result is ISC_R_SUCCESS. It is now used
in place of DST_RET, CLEANUP_WITH, and CHECK(<non-success constant>).
