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

Add home-manager module #180

Merged
merged 7 commits into from
May 12, 2023
Merged

Add home-manager module #180

merged 7 commits into from
May 12, 2023

Conversation

ambroisie
Copy link
Contributor

This is to update and fix the issues I saw in 1 and 2.

Using a service definition instead of an activation script should resolve the issue about the secrets disappearing after rebooting.

Removed the user and group option as they do not make sense to me for a home-manager module, which should target a single user. They can always be added back if somebody comes screaming.

This is somewhat modeled after sops-nix's own module 3.

@ambroisie
Copy link
Contributor Author

ambroisie commented Apr 22, 2023

I have yet to write a NixOS/Darwin test for this module, this should shake out any bug.

EDIT: if anybody feels like doing that before I eventually get around to it, please do so!

EDIT2: I added a home-manager section to the NixOS integration test.

modules/home-age.nix Outdated Show resolved Hide resolved
modules/home-age.nix Outdated Show resolved Hide resolved
modules/home-age.nix Outdated Show resolved Hide resolved
modules/home-age.nix Outdated Show resolved Hide resolved
modules/home-age.nix Outdated Show resolved Hide resolved
@ambroisie
Copy link
Contributor Author

ambroisie commented Apr 24, 2023

Added fixups to address this round of reviews.

Kept the mkIf for the systemd service, since it doesn't hurt and accomodates people that would want to use the module on older home-manager versions for some reason(?).

EDIT: since those commits are fixups, please rebase --autosquash before merging if a rogue maintainer passes by.

EDIT2: a rebase would also allow to reword the commit which adds tests, as it does not follow the same title formatting.

EDIT3: rebased to remove the fixups and re-word the test commit.

Copy link
Owner

@ryantm ryantm left a comment

Choose a reason for hiding this comment

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

Thanks. This is looking pretty good. Left a few comments. Also, lets get CI passing.

modules/home-age.nix Outdated Show resolved Hide resolved
modules/home-age.nix Outdated Show resolved Hide resolved
modules/home-age.nix Outdated Show resolved Hide resolved
modules/home-age.nix Outdated Show resolved Hide resolved
@ambroisie
Copy link
Contributor Author

Force pushed to address the review comments, as well as:

  • Running nix fmt to use alejandra's formatting instead of nixpkgs-fmt.
  • Renaming the secret directories to agenix and agenix.d for consistency with the log directory.

@ambroisie
Copy link
Contributor Author

Force pushed once again to fix a place where I had missed the home-age.nix -> age-home.nix rename.

@ambroisie ambroisie requested a review from ryantm April 28, 2023 17:01
@happysalada
Copy link

hey, this is absolutely amazing, thank you for this contribution!

I'm currently testing this branch on darwin in the following way.

      age = {
        identityPaths = [ "/Users/raphael/.ssh/id_ed25519" ];
        secrets =  {
          NU_ENV = {
            file = ../secrets/env.nu.age;
          };
        };
      };

and then trying to use the secret like

  envFile.source = config.age.secrets.NU_ENV.path;

however when I try to switch I get the following error

error: A definition for option `home-manager.users.raphael.programs.nushell.envFile.source' is not of type `null or path'. Definition values:
       - In `<unknown-file>': "$(getconf DARWIN_USER_TEMP_DIR)/agenix/NU_ENV"
(use '--show-trace' to show detailed location information)

Did I misundertand how to use this ?

@ambroisie
Copy link
Contributor Author

ambroisie commented Apr 29, 2023

@happysalada unfortunately, since the .path attribute relies on shell expansion, you can't use it for module options that won't go through a shell (i.e: shellVariables or being used in a script wrapper)

This is a limitation of the default value for secretsMountPoint and secretsDir. You can define an explicit path for those options and use it for e.g: home.files.<name>.path.

So try to do the following :

  • In a shell, echo "$(getconf DARWIN_USER_TEMP_DIR)" (replace by echo "$XDG_RUNTIME_DIR" on Linux)
  • Use that value to define the two options I mentioned earlier
  • It should Just Work ™

@ambroisie
Copy link
Contributor Author

ambroisie commented Apr 29, 2023

The other solution is to adapt the way you do things to accommodate the shell expansion.

You could write something like (I'm assuming some things about nushell, the syntax might be wrong)

  envFile.text = '' 
    source "${config.age.secrets.NU_ENV.path}"
  '';

In this case you're letting the shell do the expansion to get the path to the decrypted file.

@happysalada
Copy link

Thanks a lot for your answer.
I had originally thought to add an envVars attribute set for nushell so that env vars could be defined from a path or from text.
nix-community/home-manager#3930
Now I realise that this is not going to work.
I think now that the best design would be to have a simple attribute set that enables shell expansion so it would be something like this

nushell.envVars = {
   # the equivalent in traditional posix style is "$(getconf DARWIN_USER_TEMP_DIR)agenix/OPENAI_API_KEY"
   OPENAI_API_KEY = $"(getconf DARWIN_USER_TEMP_DIR)agenix/OPENAI_API_KEY
}

With the rest being

age = {
        identityPaths = [ "/Users/raphael/.ssh/id_ed25519" ];
        secrets =  {
          OPENAI_API_KEY = {
            file = ../secrets/openai.key.age;
          };
        };
      };

Having said that, trying to switch to my new system and going into the (getconf DARWIN_USER_TEMP_DIR) directory, I can't find any agenix directory there.
I also don't see anything in "Library/Logs/agenix" (there is not even an agenix directory in there.
Maybe I need to restart my system once ?

@ambroisie
Copy link
Contributor Author

I don't have a Darwin system to test unfortunately, this is based on sops-nix's module.

If you find errors in the service definition, don't hesitate to suggest patches.

@happysalada
Copy link

All good, i was wondering if i had missed something.
Now that i know how to test this , i will try to have a look at it this week.
Im going to have a look at some modules in home-manager, im sure the problem is a tiny adjustment.

@happysalada
Copy link

For the record here is what I tried so far
launchd.agents.activate-agenix.enable = true (apparently you have to enable an agent)
launchd.agents.activate-agenix.config.RunAtLoad = true (you need to add this trigger to run the agents)

But even with those, this still fails. I'm going to give this a bit more thought.
By it fails, I mean, there isn't any agent config corresponding to activate-agenix. so it's likely something hasn't been enabled somewhere.

@happysalada
Copy link

@ryantm Would you know someone with darwin experience that could be interested to just give hypotheses as what would work, I would be happy to test any ideas ?

@happysalada
Copy link

Also my personal opinion (I'm on darwin and I've been waiting for this feature) is that this PR should not be delayed because of darwin support. We could just say it isn't working for darwin and someone with better knowledge will fix this later. (just my opinion considering the fact that this feature will make a few people happy).

@n8henrie
Copy link
Collaborator

n8henrie commented May 1, 2023

I'm on Darwin and should be able to investigate a little today.

@n8henrie
Copy link
Collaborator

n8henrie commented May 1, 2023

There certainly seems to be a lot of interest in a HM module; I'm a happy HM user but haven't quite figured out the use case people have in mind. At the end of the day, if this module was working as desired, are people just hoping to have a /run/agenix/myFile that is in effect chmod 0400 $USER? Is the goal so people can use home-manager switch on non-NixOS / non-nix-darwin systems?

Just hoping to make sure I know how to test that it is working as people hope.

Also, I'm surprised that so much had to be duplicated for the new module, I had expected there to be more opportunity for code reuse.

@n8henrie
Copy link
Collaborator

n8henrie commented May 1, 2023

Currently not building on Darwin, complains about config.systemd. Once I hide that behind mkMerge and optionalAttrs (like the current module) I get infinite recursion with regards to e.g. pkgs.stdenv.isDarwin.

If I override that (just for testing) with let isDarwin = true; ..., then I get

error: The option `home-manager.users.n8henrie.age' does not exist.

or

error: The option `home-manager.age' does not exist.

depending on if I put age.secrets.hm-darwin-test.file = ./secrets/hm-darwin-test.age; in my home.nix for users.n8henrie or as a top-level attribute on home-manager.

I'm importing inputs.agenix.homeManagerModules.default alongside the home-manager nix-darwin module.

Am I supposed to put my age config somewhere other than home.nix?

Does anyone have this PR working on Linux and a config they'd like to share so I can compare?

@ambroisie
Copy link
Contributor Author

At the end of the day, if this module was working as desired, are people just hoping to have a /run/agenix/myFile that is in effect chmod 0400 $USER? Is the goal so people can use home-manager switch on non-NixOS / non-nix-darwin systems?

The point to me is that some of my secrets are not system-wide, and instead belong specifically to my user.

I could use a NixOS/Darwin scoped agenix module to manage those secrets, but that prevents me from being able to use my configuration as a stand-alone home-manager installation (which I do want to do).

Adding an home-manager module takes care of both:

  • scoping, to reduce the scope of module definition as tightly as possible.
  • portability, as explained above.

Also, I'm surprised that so much had to be duplicated for the new module, I had expected there to be more opportunity for code reuse.

I think that can come in a second wave.

Currently not building on Darwin, complains about config.systemd. Once I hide that behind mkMerge and optionalAttrs (like the current module) I get infinite recursion with regards to e.g. pkgs.stdenv.isDarwin.

Do you use a recent version of home-manager? This module has only been tested on unstable (read: master branch), and it should not complain about the service definition on that version (see this commit).

error: The option `home-manager.users.n8henrie.age' does not exist.

Just making sure, are you importing the module in your home-manager configuration? See how it is done in the test.

Does anyone have this PR working on Linux and a config they'd like to share so I can compare?

Same as above, feel free to look at the test.

Don't hesitate to ask more questions, I'll try to answer as I can. If you want, we can also take this to Matrix.

@n8henrie
Copy link
Collaborator

n8henrie commented May 1, 2023

The point to me is that some of my secrets are not system-wide, and instead belong specifically to my user
use my configuration as a stand-alone home-manager installation (which I do want to do)

Just making sure I understand -- in concrete terms, what this would look like:

  1. You want to configure age under e.g. home-manager.users.ambroisie.age as opposed to a top-level config.age
  2. When you home-manager switch, you want your secrets to show up under /run/user/$UID/agenix-home/ instead of /run/agenix

Does that sound like a fair summary?

I think that can come in a second wave.

I'd hope for it to come sooner rather than later, as this seems like it would roughly double the maintenance burden for the module until reconciled.

Do you use a recent version of home-manager?

No, all of my systems track the most recent stable nixpkgs release (22.11 as of now). It looked like you were aiming for compatibility with "older" HM releases -- is that no longer the case?

Just making sure, are you importing the module in your home-manager configuration?

Yes, I was following the example in the test, the issue is likely my HM version as you pointed out above.

Let me see if I can get HM unstable working.

@n8henrie
Copy link
Collaborator

n8henrie commented May 1, 2023

Gah, not that easy: nix-community/home-manager#3928

@n8henrie
Copy link
Collaborator

n8henrie commented May 1, 2023

Just making sure, are you importing the module in your home-manager configuration?

I'm having trouble getting it to import to my flake. My agenix input is passed via specialArgs.inputs.agenix, but trying to import in home.nix says it's not there:

{
  pkgs,
  unstable,
  config,
  ...
} @ attrs: let
  inherit (pkgs.stdenv) isDarwin isLinux;
in {
  imports = [attrs.inputs.agenix.homeManagerModules.default];
...

result:

error: attribute 'inputs' missing

       at /nix/store/8cij1zg3bk0q30sghac9ka9qgiqsa9p3-source/home.nix:9:14:

            8| in {
            9|   imports = [attrs.inputs.agenix.homeManagerModules.default];
             |              ^
           10|   age.secrets.hm-darwin-test.file = ./secrets/hm-darwin-test.age;
(use '--show-trace' to show detailed location information)

Trying to get at it more directly results in infinite recursion:

{
  pkgs,
  unstable,
  config,
  agenix,
  ...
} @ attrs: let
  inherit (pkgs.stdenv) isDarwin isLinux;
in {
  imports = [agenix.homeManagerModules.default];

Unfortunately using the test as an example doesn't help me sort this out, since it is passing the module as a path instead of a flake input.

@ambroisie
Copy link
Contributor Author

Does that sound like a fair summary?

Yup, sounds right to me.

It looked like like you were aiming for compatibility with "older" HM releases -- is that no longer the case?

It is, but I meant older unstable releases. I don't think it is advisable to try and support older HM given that the next stable release is imminent.

I think the best way to go about testing this would be if someone with Darwin build access could modify the test in a way that is similar to the NixOS one and debug its potential issues.

As for your import problems, I'm not sure what is going wrong there unfortunately.

@n8henrie
Copy link
Collaborator

n8henrie commented May 5, 2023

Does it work if you switch your system to it ?

I'm trying not to switch into it because then my whole environment gets screwed up! But yes.

@n8henrie
Copy link
Collaborator

n8henrie commented May 5, 2023

Hmmm, well the good news is I got the tests running and passing.

The bad news is that some git wires got crossed when I went to rebase and squash, as I'm sure you all noticed.

EDIT: That looks better.

@happysalada
Copy link

Im adding this on my list of todos to test tomorrow.
We will know for sure if jts the darwin module that has a problem or not

@n8henrie
Copy link
Collaborator

n8henrie commented May 5, 2023

Darwin module is passing. I added a standalone test using home-manager switch that relies on having a nixpkgs channel; it was getting rate limited fetching nixpkgs.

@happysalada
Copy link

@n8henrie just tested and it's working beautifully!
Great work to both of you, this is really nice!

@ambroisie
Copy link
Contributor Author

ambroisie commented May 6, 2023

@n8henrie I kind of disagree with setting a default value for the identityPaths option, since unlike the NixOS module we can't rely on a safe value.

Do we need to explicitly home-manager switch in the CI?

EDIT: I force pushed a history clean-up.

EDIT2: And removed the default value for identityPaths.

@n8henrie
Copy link
Collaborator

n8henrie commented May 6, 2023

I kind of disagree with setting a default value for the identityPaths option, since unlike the NixOS module we can't rely on a safe value.

I figured it should be analogous to the existing module. Unreadable (or nonexistent) paths are skipped, so I don't see what is "unsafe" about this, and it sets a useful example for users to follow, and will be correct in many cases (uses the default filenames that ssh-keygen suggests, and the customary path). No strong feelings, but it doesn't seem like a problem to me.

Do we need to explicitly home-manager switch in the CI?

Trying to test the "standalone" usage in a way that gives non-Darwin contributors like yourself the best representation of how it will work for Darwin users. Also a bit more work to test the launchd scripts (with no benefit I can see) otherwise. What problem is it presenting?

@n8henrie
Copy link
Collaborator

n8henrie commented May 6, 2023

As additional context, at least some of the paths added by the existing module don't exist in Darwin either, and don't seem to present a big problem.

ambroisie and others added 7 commits May 6, 2023 14:18
This is to update and fix the issues I saw in [1] and [2].

Using a service definition instead of an activation script should
resolve the issue about the secrets disappearing after rebooting.

Removed the `user` and `group` option as they do not make sense to me
for a home-manager module, which should target a single user. They can
always be added back if somebody comes screaming.

This is somewhat modeled after sops-nix's own module [3].

[1]: #58
[2]: #109
[3]: https://github.com/Mic92/sops-nix/blob/master/modules/home-manager/sops.nix
@happysalada
Copy link

@ryantm unless I missed something, I think this is ready for review.

Copy link
Owner

@ryantm ryantm left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me! Just one question.

modules/age-home.nix Show resolved Hide resolved
@ryantm
Copy link
Owner

ryantm commented May 12, 2023

It's clear a bunch of you are getting value out of this already. Great! I hope someone writes some docs in the Readme too!

@ryantm ryantm merged commit 9219727 into ryantm:main May 12, 2023
@andrevmatos
Copy link

This broke my system flake using flake-utils-plus. Reverting to the commit before makes it work again.

❯ nix build -L ".#nixosConfigurations.$(hostname).config.system.build.toplevel" --show-trace
warning: Git tree '/etc/nixos' is dirty
error:
       … while calling anonymous lambda

         at /nix/store/i5ipw8rvj1fvdaj01bqzc0dpw9irrb4h-source/flake.nix:37:20:

           36|           lhs // mapAttrs
           37|             (name: value:
             |                    ^
           38|               if isAttrs value then lhs.${name} or { } // value

       … from call site

         at /nix/store/i5ipw8rvj1fvdaj01bqzc0dpw9irrb4h-source/lib/mkFlake.nix:222:40:

          221|
          222|         pkgs = mapAttrs importChannel (mergeAny channelsFromFlakes channels);
             |                                        ^
          223|

       … while calling 'mergeAny'

         at /nix/store/i5ipw8rvj1fvdaj01bqzc0dpw9irrb4h-source/flake.nix:35:25:

           34|         # merge nested attribute sets and lists
           35|         mergeAny = lhs: rhs:
             |                         ^
           36|           lhs // mapAttrs

       … from call site

         at /nix/store/i5ipw8rvj1fvdaj01bqzc0dpw9irrb4h-source/lib/mkFlake.nix:208:25:

          207|         # For some odd reason `devshell` contains `legacyPackages` out put as well
          208|         channelFlakes = filterAttrs (_: value: value ? legacyPackages && value.legacyPackages.x86_64-linux ? nix) inputs;
             |                         ^
          209|         channelsFromFlakes = mapAttrs (name: input: { inherit input; }) channelFlakes;

       … while calling 'filterAttrs'

         at /nix/store/i5ipw8rvj1fvdaj01bqzc0dpw9irrb4h-source/lib/mkFlake.nix:203:29:

          202|       let
          203|         filterAttrs = pred: set:
             |                             ^
          204|           listToAttrs (concatMap (name: let value = set.${name}; in if pred name value then [ ({ inherit name value; }) ] else [ ]) (attrNames set));

       … while calling anonymous lambda

         at /nix/store/i5ipw8rvj1fvdaj01bqzc0dpw9irrb4h-source/lib/mkFlake.nix:204:35:

          203|         filterAttrs = pred: set:
          204|           listToAttrs (concatMap (name: let value = set.${name}; in if pred name value then [ ({ inherit name value; }) ] else [ ]) (attrNames set));
             |                                   ^
          205|

       … from call site

         at /nix/store/i5ipw8rvj1fvdaj01bqzc0dpw9irrb4h-source/lib/mkFlake.nix:204:72:

          203|         filterAttrs = pred: set:
          204|           listToAttrs (concatMap (name: let value = set.${name}; in if pred name value then [ ({ inherit name value; }) ] else [ ]) (attrNames set));
             |                                                                        ^
          205|

       … while calling anonymous lambda

         at /nix/store/i5ipw8rvj1fvdaj01bqzc0dpw9irrb4h-source/lib/mkFlake.nix:208:41:

          207|         # For some odd reason `devshell` contains `legacyPackages` out put as well
          208|         channelFlakes = filterAttrs (_: value: value ? legacyPackages && value.legacyPackages.x86_64-linux ? nix) inputs;
             |                                         ^
          209|         channelsFromFlakes = mapAttrs (name: input: { inherit input; }) channelFlakes;

       error: attribute 'x86_64-linux' missing

       at /nix/store/i5ipw8rvj1fvdaj01bqzc0dpw9irrb4h-source/lib/mkFlake.nix:208:74:

          207|         # For some odd reason `devshell` contains `legacyPackages` out put as well
          208|         channelFlakes = filterAttrs (_: value: value ? legacyPackages && value.legacyPackages.x86_64-linux ? nix) inputs;
             |                                                                          ^
          209|         channelsFromFlakes = mapAttrs (name: input: { inherit input; }) channelFlakes;

darwinConfigurations.integration.system = self.checks."x86_64-darwin".integration;
# Work-around for https://github.com/nix-community/home-manager/issues/3075
legacyPackages = nixpkgs.lib.genAttrs ["aarch64-darwin" "x86_64-darwin"] (system: {
homeConfigurations.integration-darwin = home-manager.lib.homeManagerConfiguration {

Choose a reason for hiding this comment

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

Probably because of this, usually legacyPackages is followed by the system, so maybe this should be:

${system}.homeConfigurations.integration-darwin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

genAttrs already prefixes it with the system.

Choose a reason for hiding this comment

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

Hmm, right, so it should be because flake-utils-plus assume anything containing legacyPackages will also contain x86-64-linux inside. I'll create an issue there.

@happysalada
Copy link

Just reporting what I've found to be happening after a bit of usage.
Upon updating my system, I get the following warning

Existing file '/Users/raphael/Library/LaunchAgents/org.nix-community.home.activate-agenix.plist' is in the way of '/nix/store/20knk37h6xpg262v1iif7j69j41rnxg9-home-manager-agents/org.nix-community.home.activate-agenix.plist'

removing the faulty file produces another warning on the next update

Bootstrap failed: 5: Input/output error
Try re-running the command as root for richer errors.

trying an update once more succeeds, however the agent is not run.
My solution so far is just to re-run manually the script inside the agent file.

@happysalada
Copy link

I'm curious if anyone has tried this on a machine where they have both system wide secrets and user level secrets. I can't make agenix.nixosModules.age and agenix.homeManagerModules.age work together. I'm not sure why.

@ambroisie
Copy link
Contributor Author

@happysalada I believe this is done in the tests IIRC? I haven't checked though.

@happysalada
Copy link

nixos tests only test for one user on the home-manager tests it seems
https://github.com/ryantm/agenix/blob/main/test/integration.nix#L49

@ambroisie
Copy link
Contributor Author

Well yes, but that is testing systemd-wide secrets and user-level secrets on the same host.

Anecdotally, it works on my machine ™️.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants