nixos-generate-config: Add system.copySystemConfiguration = true #67475

infinisil · 2019-08-26T03:45:36Z

Motivation for this change

This will be useful for the many people who accidentally delete their
configuration.nix file.

The alternative of changing the default of this option to true won't
work well because for some NixOS build, we can't assume for the
configuration file to be at the standard location. E.g. in nixops,
will point to something completely different than what
the nixops evaluation thinks, therefore leaking the nixops deploy users
config into the deployment machines. See also #16922

This continues #24707

In the future we might want to add an option like system.copySystemConfigurationDir = ./. which would copy the whole directory the configuration is in.

Ping @danbst @techhazard @bobvanderlinden @ryantrinkle @vcunat

Things done

Tested that it works with

diff --git a/nixos/tests/installer.nix b/nixos/tests/installer.nix
index a136678c6ef..4599d01e1ef 100644
--- a/nixos/tests/installer.nix
+++ b/nixos/tests/installer.nix
@@ -754,4 +754,13 @@ in {
     '';
   };
 
+  copySystemConfig = makeInstallerTest "copySystemConfig" (simple-test-config // {
+    extraConfig = ''
+      system.copySystemConfiguration = true;
+    '';
+    preBootCommands = ''
+      $machine->succeed("test -e /run/current-system/configuration.nix");
+    '';
+  });
+
 }

I don't think it's worth including this as another NixOS test though

This will be useful for the many people who accidentally delete their configuration.nix file. The alternative of changing the default of this option to true won't work well because for some NixOS build, we can't assume for the configuration file to be at the standard location. E.g. in nixops, <nixos-config> will point to something completely different than what the nixops evaluation thinks, therefore leaking the nixops deploy users config into the deployment machines.

danbst · 2019-08-26T06:49:55Z

nixos/modules/installer/tools/tools.nix

+        # still recover it from
+        # - /run/current-system/configuration.nix (last one)
+        # - /nix/var/nix/profiles/system-*/configuration.nix (old versions)
+        system.copySystemConfiguration = true;


Should this be false by default?

The option is false by default, but I think the point of this pull request is have it enabled for NixOS.

I'm saying this, because originally I've suggested "false" as default.

#24707 (comment)

My reasoning is like this:

default config should be close to real NixOS defaults.

ideally, we should take an idea from postgresql default config - list an option, but leave it commented, together with default value. If you uncomment - it is still default, you have both to uncomment and change to make thing happen.

configuration can have secrets embedded, in which case it shouldn't be copied to store by default. Or at least warned about that. We care about security.

configuration can have secrets embedded, in which case it shouldn't be copied to store by default. Or at least warned about that. We care about security.

If the secret is embedded and used, it ends up in the nix store anyway, no?

@mmahut 🤔 indeed. Okay, then I'd be fine with commented-by-default, as are other options.

I'd like for this to be true and uncommented because there's no harm in doing so, the worst thing it can do is increase the closure size a bit, but the benefit it provides will be appreciated by a lot. If problems like the one with nixops wouldn't exist I'd make this option's default = true;; changing the default here is the next best thing we can do.

I won't take a hard stance and counter this. I just want to keep consistency in the sample config file. The only uncommented options are vital ones - hardware config, boot loader config and state version. Everything else is "recommendation" and thus, commented out.

From pragmatic viewpoint, yes, it would be great to make it enabled for new users.

vcunat · 2019-08-26T18:31:18Z

Well, hopefully this comment makes it clear to most people that only this single file is copied. Otherwise I'd fear that we'd exchange small early loss for a larger later loss (due to not noticing soon, as it's natural to assume that all configuration is copied). I personally still think it might be better to instead somehow encourage version-controlling the config, but I'm not against this change.

grahamc · 2019-08-31T16:34:30Z

My objection to this PR is the name of the option is a bit of a lie: It does not copy the system configuration, it copies maybe-some-of-it. Enabling a sort-of-broken feature by default turns on a footgun people might think they can rely on.

mmahut · 2019-08-31T17:19:41Z

@grahamc that is a good point. I guess would not be very easy to eval the imports and copy them as well? Hmm.

infinisil · 2019-08-31T17:29:44Z

Well I think the message in this PR is clear enough that only that file is backed up. In addition I think a lot of people, especially beginners, only have a single configuration.nix anyways.

But yeah, maybe an option like system.copySystemConfigurationDir = ./. would be good to implement (as already suggested in earlier issues), which would copy all of /etc/nixos with that value, covering a much wider range of config setups.

disassembler · 2019-09-01T02:21:39Z

I'm not a fan of protecting users from themselves. We have rollbacks, people can use version control for their nixos configs. I don't see much benefit in adding this option, and as @grahamc said above, it creates a false sense of security that their configs are backed up when really it's just configuration.nix, which I try to encourage people to keep as minimal as possible (ideally just imports) e.g. https://github.com/disassembler/network/blob/master/machines/irkutsk.nix is the configuration.nix I symlink for my main laptop.

infinisil · 2019-09-05T00:12:31Z

@disassembler I think we should protect users from themselves! Users (including me) do stupid things, and unless we have protective measures against this, bad stuff happens too easily. A good example is how rm received the --no-preserve-root flag.

While people can rollback, that doesn't allow recovering a Nix config. And while people can use version control, many do not, I presume many aren't even familiar with git.

The "false sense of security" argument is a good reason though, people with only a single configuration.nix might become accustomed to being able to recover it, but once they split it into multiple files they could get surprised that the others haven't been backed up the same way.

So I think a system.copySystemConfigurationDir = ./. would be a good way to solve this.

danbst · 2019-09-05T17:37:36Z

@disassembler in my beginner times, I've lost configuration.nix once.

TBH I'd better prefer that /etc/nixos would be git-controlled (git add . && git commit on rebuild switch). It actually isn't something complicated to do - just a custom activationScript.

infinisil · 2020-01-07T03:53:29Z

Not interested in pursuing this anymore

ofborg bot added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Aug 26, 2019

danbst reviewed Aug 26, 2019

View reviewed changes

mmahut approved these changes Aug 26, 2019

View reviewed changes

Lassulus mentioned this pull request Oct 2, 2019

[Discussion] Lxc image should contain configuration used for generation as /etc/nixos/configuration.nix nix-community/nixos-generators#42

Open

infinisil closed this Jan 7, 2020

d4g mentioned this pull request Jul 15, 2020

Allow us to define files to be copied into the image nix-community/nixos-generators#60

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos-generate-config: Add system.copySystemConfiguration = true #67475

nixos-generate-config: Add system.copySystemConfiguration = true #67475

infinisil commented Aug 26, 2019

danbst Aug 26, 2019

mmahut Aug 26, 2019

danbst Aug 26, 2019

mmahut Aug 26, 2019

danbst Aug 26, 2019

infinisil Aug 26, 2019

danbst Aug 26, 2019

vcunat commented Aug 26, 2019

grahamc commented Aug 31, 2019

mmahut commented Aug 31, 2019

infinisil commented Aug 31, 2019

disassembler commented Sep 1, 2019

infinisil commented Sep 5, 2019

danbst commented Sep 5, 2019

infinisil commented Jan 7, 2020

nixos-generate-config: Add system.copySystemConfiguration = true #67475

nixos-generate-config: Add system.copySystemConfiguration = true #67475

Conversation

infinisil commented Aug 26, 2019

Motivation for this change

Things done

danbst Aug 26, 2019

Choose a reason for hiding this comment

mmahut Aug 26, 2019

Choose a reason for hiding this comment

danbst Aug 26, 2019

Choose a reason for hiding this comment

mmahut Aug 26, 2019

Choose a reason for hiding this comment

danbst Aug 26, 2019

Choose a reason for hiding this comment

infinisil Aug 26, 2019

Choose a reason for hiding this comment

danbst Aug 26, 2019

Choose a reason for hiding this comment

vcunat commented Aug 26, 2019

grahamc commented Aug 31, 2019

mmahut commented Aug 31, 2019

infinisil commented Aug 31, 2019

disassembler commented Sep 1, 2019

infinisil commented Sep 5, 2019

danbst commented Sep 5, 2019

infinisil commented Jan 7, 2020