Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests/wireguard: init wg-quick #74504

Merged
merged 1 commit into from Mar 28, 2020
Merged

tests/wireguard: init wg-quick #74504

merged 1 commit into from Mar 28, 2020

Conversation

d-xo
Copy link
Contributor

@d-xo d-xo commented Nov 28, 2019

Motivation for this change

While updating wireguard-tools in #74347 I missed a new dependency in wg-quick, and my brief testing of wg-quick (executing the wg-quick binary with no arguments) failed to detect the oversight. Once merged this broke the wg-quick module (also on stable! 馃槩).

This change adds a simple test for the wg-quick module to the tests/wireguard folder, that should hopefully avoid similar situations in the future.

Changes:

  • Duplicate and adapt the existing default wireguard test for wg-quick
  • Pull common setup code out into a separate file
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @elseym @ericsagnes @Mic92 @globin @Ma27 @iclanzan

@Ma27
Copy link
Member

Ma27 commented Dec 19, 2019

@GrahamcOfBorg test wg-quick

ipv6.addresses = lib.singleton {
address = "fd00::1";
prefixLength = 64;
peer0 = { lib, ... }: lib.attrsets.recursiveUpdate (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When updating nixos configurations, it's generally safer to use lib.mkMerge.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I'm curious what the difference is though? I couldn't find any documentation for lib.mkMerge in the nixpkgs manual...

@d-xo
Copy link
Contributor Author

d-xo commented Dec 19, 2019

  • rebased on master
  • replaced lib.attrsets.recursiveUpdate with lib.mkMerge
  • pulled config merging logic into common.nix

Thanks @Ma27 for the review :)

@Ma27
Copy link
Member

Ma27 commented Dec 19, 2019

@GrahamcOfBorg test wg-quick

@d-xo
Copy link
Contributor Author

d-xo commented Dec 30, 2019

@Ma27 Is there anything more to do here?

- Duplicate and adapt the existing default wireguard test for wg-quick
- Pull common setup code out into a seperate file
@d-xo
Copy link
Contributor Author

d-xo commented Dec 30, 2019

spotted some issues after I asked 馃榿

  • fixed typo in all-tests.nix (missing .nix in path to wg-quick test file)
  • clearer naming for shared code (common.nix -> make-peer.nix)

rebased on master and made the above changes. I think this should be good to go now 馃檪

@Ma27 Ma27 merged commit 63e8105 into NixOS:master Mar 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants