add age.secrets.*.{action,service}

[Radvendii]

I don't have the brainspace to test this right now, but I figured I would put it up in case other people were inspired to test before I did.

Things that need testing:

• [x] Does this even evaluate in current form?
• [x] Does it correctly restart services when secrets change
• [x] Does it not restart services when secrets don't change (or when some secrets change but other don't
• [x] If you change a secret and it's action atomically, which action gets performed? The old one or the new one? (I think this should be the new one)

Implements #84

[winterqt]

Does it not restart services when secrets don't change (or when some secrets change but other don't

I believe this is false because of #89.

If you change a secret and it's action atomically, which action gets performed? The old one or the new one? (I think this should be the new one)

As far as I can tell, it would be the old action, since the path condition fires before the units get reloaded.

[Radvendii]

@winterqt thank you! After looking at your comment and thinking about it for a bit, I've completely reworked this PR.

It now hijacks NixOS's mechanism for restarting services. If you specify a service, this is simple. it just adds the information about the secret to the service file so that if any of it changes, the service gets reloaded.

If you specify an action, it creates a dummy service that just executes that action, and similarly causes it to be reloaded when the secret changes.

A few design decisions I'm not sure about:

• Do we even need action? Do people use this for anything besides restarting services? Someone might want to reload a service. In theory they can set systemd.services.<service>.reloadIfChanged, but maybe they want it to restart if they actually change the service itself?
• Should we change service to services and make it a list? There are sometimes multiple services that need to be restarted. This would be needed if we remove action, but maybe not if we don't.
• Is there a better name for the service than agenix-${name}-action?

[winterqt]

This is really nicely done, congrats. Do you mind if I PR additions to the tests to account for this functionality to your branch, so this all lands in one PR?

[Radvendii]

Not at all, go for it.

[Radvendii]

Well, I thought of yet another rewrite. Advantages of this one include:

• Simpler (not mucking with nix systemd magic)
• compares the decrypted secrets, so if you rekey this won't run the action

We could just add something like this to the activation script for each secret:

    if ! [ -e ${secretType.path} ] || \
       ! ${pkgs.diffutils}/bin/diff -q $TMP_FILE ${secretType.path} >/dev/null || \
       [ "$(stat -c '%a' "$TMP_FILE")" -eq "$(stat -c '%a' "${secretType.path}")" ]
    then
      echo "Updated secret ${secretType.name} performing action..."
      ${secretType.action};
    fi

This isn't working on my machine though, because the file at secretType.path doesn't exist while this is running for some reason. Something I've noticed on my machine is that /run/agenix.d doesn't actually store any older generations of secrets, only the current one. Maybe that's the problem? It's getting deleted somewhere? Can't figure out where though.

[winterqt]

@ryantm The changes to the integration test to include this functionality changes some reformatting using nixpkgs-fmt, plus some changes to make it work on recent Nixpkgs versions.

Would you prefer that I split the fixes and formatting changes from the additions, or don't format at all, or are you fine with all of this (implementation, fixing the existing test, and testing of this functionality) being in one commit?

[Radvendii]

Oh, and another advantage of that last rewrite is that it would perform the action the first time the secret is created / the action was added, not just when it gets changed. The disadvantage, of course, is that I still don't know how to get it to work at all.

[ryantm]

The changes to the integration test

Maybe a bunch of things just changed because I don't see any changes to the integration tests now? Generally, I'd prefer to keep formatting changes in a separate PR, so the individual ones are easier to review.

[Radvendii]

I don't think @winterqt committed the integration test yet.

[Radvendii]

Oh, and for that you should @winterqt you should know I changed it to have services which takes a list rather than just a single service. I found that it would be helpful in practice.

[Radvendii]

@winterqt any progress on this? I'm happy to work on the integration test but I don't want to duplicate effort in case you just forgot to push.

[winterqt]

@Radvendii I opened https://github.com/Radvendii/agenix/pull/1 a few days ago. I assumed you would be notified, but I just noticed that GitHub doesn't enable notifications on forks for their creators by default, like they do for normal repos.

[Radvendii]

Whoops! Also my notification settings are a bit scuffed right now, which might be why.

[Radvendii]

Okay, this is no longer WIP. I think it's good to go

[Radvendii]

Bump? @ryantm

[infinisil]

Check out https://github.com/ryantm/agenix/issues/84#issuecomment-1099884559 for a potential alternative

[Radvendii]

I've overhauled this to use /run/activation-{restart,reload}-list and to check whether the actual decrypted secrets have changed.

h/t @EHfive

[Radvendii]

ping @ryantm I think this PR is in an even better state and is ready to merge again (from my end)

[Radvendii]

This is very bizzare. nix flake check works on my machine and it's on the same commit. TFW flakes not reproducible.

[Radvendii]

I fixed the behaviour in some edge cases (including the one we discussed), and added a bunch more tests to make sure it's behaving right in those edge cases. There are even more cases that could be added (e.g. changing more than one thing at the same time), but that becomes very big very fast. let me know if you'd like me to add those tests.

[Radvendii]

ping @ryantm hopefully it's ready now.

[Radvendii]

@ryantm

[winterqt]

ping @ryantm

[winterqt]

also ping @cole-h :)