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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/testing: disallow special chars in machine names in network expressions #46806

Merged

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Sep 17, 2018

Motivation for this change

These names are referenced by Perl variables inside the testing
frameworks which don't allow - as character inside. An exemplary
expression may look like this:

{
  x11-vm = {
    services.xserver.enable = true;
  };
}

This expression evaluates, e.g. when running nixos-build-vms, but when
trying to run ./result/bin/nixos-run-vms, an error like this occurs:

starting VDE switch for network 1
running the VM test script
error: Can't modify subtraction (-) in scalar assignment at (eval 17) line 1, at EOF
Bareword "test" not allowed while "strict subs" in use at (eval 17) line 1.
Can't modify subtraction (-) in scalar assignment at (eval 17) line 1, at EOF
Bareword "test" not allowed while "strict subs" in use at (eval 17) line 1.
vde_switch: EOF on stdin, cleaning up and exiting
cleaning up

This can be very confusing for beginners, this change breaks evaluation
if such names are used for machines.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Fits CONTRIBUTING.md.

@Ma27 Ma27 changed the title nixos/testing: disallow - as machine names in network expressions nixos/testing: disallow - in machine names in network expressions Sep 17, 2018
@fpletz
Copy link
Member

fpletz commented Sep 21, 2018

Maybe we should do a more thorough validation for Perl variable names while we're at it? Attribute names can be arbitrary Nix strings after all. See https://perldoc.perl.org/perlvar.html.

@Ma27
Copy link
Member Author

Ma27 commented Sep 21, 2018

Thanks a lot for your feedback, sorry for missing this!

@Ma27 Ma27 force-pushed the disallow-dash-separators-in-machine-declarations branch from c9d45c8 to 72d7605 Compare September 21, 2018 22:00
@Ma27
Copy link
Member Author

Ma27 commented Sep 21, 2018

@fpletz fixed. I now use a more restrictive regex with builtins.match for this:)

@Ma27 Ma27 changed the title nixos/testing: disallow - in machine names in network expressions nixos/testing: disallow special chars in machine names in network expressions Sep 21, 2018
@Ma27
Copy link
Member Author

Ma27 commented Oct 19, 2018

@fpletz would you mind having a second look at this? :)

@Ma27
Copy link
Member Author

Ma27 commented Dec 4, 2018

@fpletz would you mind having a second look at this? This IMHO improves the test driver, but I'm hesitant merging such a change without an approval :)

…ressions

These names are referenced by Perl variables inside the testing
frameworks which don't allow chars like `-` as character inside. An exemplary
expression may look like this:

```
{
  x11-vm = {
    services.xserver.enable = true;
  };
}
```

This expression evaluates, e.g. when running `nixos-build-vms`, but when
trying to run `./result/bin/nixos-run-vms`, an error like this occurs:

```
starting VDE switch for network 1
running the VM test script
error: Can't modify subtraction (-) in scalar assignment at (eval 17) line 1, at EOF
Bareword "test" not allowed while "strict subs" in use at (eval 17) line 1.
Can't modify subtraction (-) in scalar assignment at (eval 17) line 1, at EOF
Bareword "test" not allowed while "strict subs" in use at (eval 17) line 1.
vde_switch: EOF on stdin, cleaning up and exiting
cleaning up
```

This can be very confusing for beginners, this change breaks evaluation
if such names are used for machines.
@Ma27 Ma27 force-pushed the disallow-dash-separators-in-machine-declarations branch from 72d7605 to 113a6b9 Compare December 18, 2018 00:59
@fpletz fpletz merged commit 670c5ac into NixOS:master Dec 18, 2018
@Ma27 Ma27 deleted the disallow-dash-separators-in-machine-declarations branch December 18, 2018 01:03
@c0bw3b c0bw3b added the 6.topic: testing Tooling for automated testing of packages and modules label Apr 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants