-
Notifications
You must be signed in to change notification settings - Fork 693
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
[xenial] Ensures config test coverage for Xenial #4099
Conversation
f94c462
to
66ac605
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
make build-debs
passes without error (caveat: a single failing test for unapplied updates is OK) -
make build-debs-xenial
passes without error (caveat: a single failing test for unapplied updates is OK) -
make staging && molecule verify -s libvirt-staging
passes without error 🔥
The third test is giving the following error.
-- generated xml file: /home/kdas/code/securedrop/junit/testinfra-results.xml --
=================================== FAILURES ===================================
________________ test_mon_iptables_rules[ansible://mon-staging] ________________
[gw3] linux2 -- Python 2.7.13 /home/kdas/code/sd/bin/python2
host = <testinfra.host.Host object at 0x7f15d41b1450>
def test_mon_iptables_rules(host):
# Build a dict of variables to pass to jinja for iptables comparison
kwargs = dict(
app_ip=os.environ.get('APP_IP', securedrop_test_vars.app_ip),
> default_interface=Command.check_output(
"ip r | head -n 1 | awk '{ print $5 }'"),
tor_user_id=Command.check_output("id -u debian-tor"),
ssh_group_gid=Command.check_output("getent group ssh | cut -d: -f3"),
postfix_user_id=Command.check_output("id -u postfix"),
dns_server=securedrop_test_vars.dns_server)
E NameError: global name 'Command' is not defined
../testinfra/staging/mon/test_mon_network.py:17: NameError
________________ test_app_iptables_rules[ansible://app-staging] ________________
[gw3] linux2 -- Python 2.7.13 /home/kdas/code/sd/bin/python2
host = <testinfra.host.Host object at 0x7f15d6da6f90>
def test_app_iptables_rules(host):
# Build a dict of variables to pass to jinja for iptables comparison
kwargs = dict(
mon_ip=os.environ.get('MON_IP', securedrop_test_vars.mon_ip),
> default_interface=Command.check_output("ip r | head -n 1 | "
"awk '{ print $5 }'"),
tor_user_id=Command.check_output("id -u debian-tor"),
securedrop_user_id=Command.check_output("id -u www-data"),
ssh_group_gid=Command.check_output("getent group ssh | cut -d: -f3"),
dns_server=securedrop_test_vars.dns_server)
E NameError: global name 'Command' is not defined
../testinfra/staging/app/test_app_network.py:17: NameError
=== 2 failed, 444 passed, 13 skipped, 8 xfailed, 2 xpassed in 227.74 seconds ===
@kushaldas Changes addressed, please have another look! |
I suddenly got the following for
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
make build-debs
passes without error (caveat: a single failing test for unapplied updates is OK) -
make build-debs-xenial
passes without error (caveat: a single failing test for unapplied updates is OK) -
make staging && molecule verify -s libvirt-staging
passes without error - 🔥 🔥 🔥 🔥 🔥
make staging-xenial && molecule verify -s libvirt-staging-xenial
fails with the following errors:
=================================== FAILURES ===================================
__________ test_pax_flags[ansible://app-staging-/usr/sbin/grub-probe] __________
[gw1] linux2 -- Python 2.7.13 /home/kdas/code/sd/bin/python2
[XPASS(strict)] PaX flags unset at install time, see issue #3916
_______ test_pax_flags[ansible://app-staging-/usr/sbin/grub-mkdevicemap] _______
[gw1] linux2 -- Python 2.7.13 /home/kdas/code/sd/bin/python2
[XPASS(strict)] PaX flags unset at install time, see issue #3916
_______ test_pax_flags[ansible://app-staging-/usr/bin/grub-script-check] _______
[gw1] linux2 -- Python 2.7.13 /home/kdas/code/sd/bin/python2
[XPASS(strict)] PaX flags unset at install time, see issue #3916
__________ test_pax_flags[ansible://mon-staging-/usr/sbin/grub-probe] __________
[gw3] linux2 -- Python 2.7.13 /home/kdas/code/sd/bin/python2
[XPASS(strict)] PaX flags unset at install time, see issue #3916
_______ test_pax_flags[ansible://mon-staging-/usr/sbin/grub-mkdevicemap] _______
[gw1] linux2 -- Python 2.7.13 /home/kdas/code/sd/bin/python2
[XPASS(strict)] PaX flags unset at install time, see issue #3916
_______ test_pax_flags[ansible://mon-staging-/usr/bin/grub-script-check] _______
[gw2] linux2 -- Python 2.7.13 /home/kdas/code/sd/bin/python2
[XPASS(strict)] PaX flags unset at install time, see issue #3916
=== 6 failed, 446 passed, 13 skipped, 2 xfailed, 2 xpassed in 328.45 seconds ===
Had some trouble reproducing @kushaldas's findings, but finally narrowed it down to base box version. In short, newer base boxes show the problem, whereas older don't. That kind of variation isn't useful in the config tests, so I've removed the |
Looks like there is a linting failure - @conorsch want to fix and repush when you get a chance? then @kushaldas can a final spin and approve if all else looks good |
Using wrapper functions to call discrete checks depending on platform, which is always either Trusty or Xenial. Most of the branching logic was already present, just cleaning it up a bit in order to make the functions more readable. Added a systemd-style check for xvfb under Xenial, since we weren't inspecting that config on disk. Accordingly, updates the dev docs to state that we've handled the config tests.
The old sytanx of automatic module imports as part of the test function args has been deprecated since testinfra v1.6.0 (2017-04-21). It's still working, but planned to be dropped as of testinfra v2.0. Took the opportunity to convert our checks throughout.
The "strict=true" pragma on pytest xfail means that the config test suite will exit non-zero if the tests pass. We added strict=True in order to understand better the problem of PaX flags not matching, and it appears we've confirmed that the age of installed packages is sufficient determinant. That is, if apt such as grub are upgraded as part of the install, then PaX flags set by the grsecurity role will be clobbered very quickly.
cbd810d
to
948d7e9
Compare
As I reported the issue, there is now an upstream PR opened python/typeshed#2787 We can remove these two lines after a new typeshed release.
Just now pushed a commit in this branch to fix the |
Kicking the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
make build-debs
passes without error (caveat: a single failing test for unapplied updates is OK) -
make build-debs-xenial
passes without error (caveat: a single failing test for unapplied updates is OK) -
make staging && molecule verify -s libvirt-staging
passes without error -
make staging-xenial && molecule verify -s libvirt-staging-xenial
without error
This is now good. Approved. 👍
Status
Ready for review.
Description of Changes
Fixes #3964.
Changes proposed in this pull request:
host
pragma, due to deprecation warningsTesting
make build-debs
passes without error (caveat: a single failing test for unapplied updates is OK)make build-debs-xenial
passes without error (caveat: a single failing test for unapplied updates is OK)make staging && molecule verify -s libvirt-staging
passes without errormake staging-xenial && molecule verify -s libvirt-staging-xenial
passes without errorThere's still a bit of branching logic for Trusty/Xenial support, which can't be avoided, but tried to keep it clear and maintainable. Most of the diff is the new-ish
host
syntax for testinfra, which was easy enough to slot in here given that I was reading through all the files anyway.The divergence for unconfined AppArmor processes isn't as clearly documented as it should be—see #4098 for tracking follow-up.
Deployment
No, these changes are dev-env & CI only.
Checklist
If you made changes to the server application code:
make ci-lint
) and tests (make -C securedrop test
) pass in the development containerIf you made changes to
securedrop-admin
:make -C admin test
) pass in the admin development containerIf you made changes to the system configuration:
If you made non-trivial code changes:
If you made changes to documentation:
make docs-lint
) passed locally