Details
- Reviewers
olivier - Group Reviewers
tests - Commits
- rGea1e4a7dd9bc: tests.7: Announce allow_network_access test suite conf variable
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
While ideally tests should not depend on external things as much as possible to have more reproducible nature, some of them may require to reach external resources, let's say to fetch something or to talk to the world services like DNS, etc.
The proposal is to discuss and agree on a new FreeBSD test suite configuration variable (Kyua's terminology) which could be used to "tag" such tests. "Tagging" means that a test would require such configuration variable, e.g. in terms of atf-sh it would say atf_set "require.config" "allow_network_access". And, having such conf var disabled by default as all other vars we have, the test suite would skip such tests until the conf var is defined explicitly. For example, the CI intentionally enables such conf vars like allow_sysctl_side_effects.
All name options I have in mind currently:
- allow_network_access
- allow_network_use_side_effects
- allow_network_utilization_side_effects
P.S. It seems that some of the recently added lib.libc.net.getaddrinfo.* tests could be the first candidates to use it.
I like this idea (the name allow_network_access ), here are other tests that need to depends of this great new condition:
- /usr/tests/lib/libc/resolv/resolv_test (from /usr/src/lib/libc/tests/resolv/resolv_test.c)
- /usr/tests/lib/libc/nss/gethostby_test (from /usr/src/lib/libc/tests/nss/gethostby_test.c)
We can do it later, but if you mark these variables up as It Va instead of It, they become searchable; e.g. apropos Va=allow_network.
Great spot. I guess all the options mentioned here could be improved with a separate commit. Please, feel free to do that anytime, this patch can easily be rebased with the style updated.
Sorry for being late to this (well, the window is quite short and I have been busy with other things recently...)
Indeed and that's also why I am still hesitated to enable internet access on the jails in builder (when doing builds with the patches come from external) and when running the tests in VM. It's not only the security reason but also the matter of tests stability and reliability.
The proposal is to discuss and agree on a new FreeBSD test suite configuration variable (Kyua's terminology) which could be used to "tag" such tests. "Tagging" means that a test would require such configuration variable, e.g. in terms of atf-sh it would say atf_set "require.config" "allow_network_access". And, having such conf var disabled by default as all other vars we have, the test suite would skip such tests until the conf var is defined explicitly. For example, the CI intentionally enables such conf vars like allow_sysctl_side_effects.
All name options I have in mind currently:
- allow_network_access
- allow_network_use_side_effects
- allow_network_utilization_side_effects
P.S. It seems that some of the recently added lib.libc.net.getaddrinfo.* tests could be the first candidates to use it.
I am happy to see allow_network_access feature has been added to the test framework. I believe it would be useful in some cases that network access is mandatory. However, in the case of DNS resolving, I suggest we just using the built-in local unbound for a simple local resolver to serve the test needs.
For the longer term goals, I am thinking we can have more dummy/stub/mock services in the test infra (e.g., add to the test VM) to server the different test needs. I hope we can have it self-contained as much as possible.
P.S. Don't forget to bump .Dd field when modifying the content of a man page. :-)
Thank you very much for the facts. I guess some of us were looking more for the answer whether the test vm has Internet access instead of why it is configured specific way :). Just to make sure that all the reasoning around those newly added tests is accurate. And about "why", yes, it seems to have a very good reason not to provide such access.
The proposal is to discuss and agree on a new FreeBSD test suite configuration variable (Kyua's terminology) which could be used to "tag" such tests. "Tagging" means that a test would require such configuration variable, e.g. in terms of atf-sh it would say atf_set "require.config" "allow_network_access". And, having such conf var disabled by default as all other vars we have, the test suite would skip such tests until the conf var is defined explicitly. For example, the CI intentionally enables such conf vars like allow_sysctl_side_effects.
All name options I have in mind currently:
- allow_network_access
- allow_network_use_side_effects
- allow_network_utilization_side_effects
P.S. It seems that some of the recently added lib.libc.net.getaddrinfo.* tests could be the first candidates to use it.
I am happy to see allow_network_access feature has been added to the test framework. I believe it would be useful in some cases that network access is mandatory. However, in the case of DNS resolving, I suggest we just using the built-in local unbound for a simple local resolver to serve the test needs.
For the longer term goals, I am thinking we can have more dummy/stub/mock services in the test infra (e.g., add to the test VM) to server the different test needs. I hope we can have it self-contained as much as possible.
Indeed, it is an opportunity for future improvements.
P.S. Don't forget to bump .Dd field when modifying the content of a man page. :-)
Thanks! I forget it, kind of, always.
Fortunately, it does not need any specific implementation, we just name it and use it. This patch simply makes it officially documented. I guess the goal was to remove those CI test fails as soon as possible. Yes, there is an opportunity to revise the tests.
Exactly. Also reducing the skipped tests as many as we can.
As we have a tracking ticket: https://bugs.freebsd.org/285826 , we can mark these tests as skipped (only in CI) first. To avoid the continuing failing builds reduce people's awareness.