| 24 Nov 2021 |
 m1cr0man | yeah, me too. I'm just looking at the conditionals surrounding it | 22:16:37 |
 hexa | the old domain is not part of the ExecStart script | 22:17:18 |
| m1cr0man | Ok I think I see what's up | 22:17:27 |
| m1cr0man | extraDomains is not part of the hashData, which is what certDir is set based upon | 22:17:42 |
| m1cr0man | I need to get that vs code plugin that lets me get links to github files.. | 22:18:11 |
| m1cr0man | tada https://github.com/m1cr0man/nixpkgs/blob/e5f9c5215ae48e0f0373787bb56c5deddbe2d9fb/nixos/modules/security/acme.nix#L156 | 22:20:10 |
| hexa | oh no! | 22:20:38 |
| m1cr0man | so yeah, the problem is it checks if the certificate files exist before checking domainhash. In retrospect - I should have probably just got rid of domainhash and used certdir alone to determine if renewal was needed. I think I thought at the time lego wasn't this dumb and would simply exclude the unspecified extra domains... | 22:21:33 |
| m1cr0man | if the files exist and domainhash is different, it goes to line 385 (forced renew) | 22:21:56 |
| m1cr0man | oh look echo 1>&2 "certificate domain(s) have changed; will renew now" | 22:22:04 |
| m1cr0man | https://github.com/m1cr0man/nixpkgs/commit/34b5c5c1a408d105beb9b92b9ed5b1565135e75e "Allow for key reuse when domains are the only thing that changed!" Aha. | 22:23:01 |
| hexa | ah, we lost extraDomains /o\ | 22:23:34 |
| m1cr0man | ok so the question is, can I delete the cert files and not the key and will lego do a renew | 22:23:47 |
| hexa | I can give that a try | 22:24:22 |
| hexa | the certificate is likely the only place that has the info about the old san | 22:24:50 |
| hexa | do you need me to test something or can I just go ahead and purge this mess? | 22:38:14 |
| hexa | m1cr0man: ^ | 22:44:13 |
| m1cr0man | sorry - go ahead and purge | 22:53:27 |
| 25 Nov 2021 |
| m1cr0man | I think I simply overlooked testing removing a domain from existing domains. I'm going to add this to the test suite and then work on fixing it plus working on another ticket I saw (allowing setting more cert options at the sercurity.acme level) | 00:14:59 |
| m1cr0man | it's an easy enough thing to fix - but it's hard to fix without triggering mass renewals which is always a concern with ACME stuff | 00:15:29 |
| m1cr0man | https://github.com/NixOS/nixpkgs/issues/108237 also this one is interesting. The easy solution here is to add a message explaining that "If you are reading this after a nixos-rebuild - don't panic! This is just a certificate renewal failure and self-signed certs will be in place" (or something along those lines), but that would have to appear for all failures not just on rebuild. If anyone has suggestions feel free to shout them out. I'm off for now but will be working towards a PR by the weekend | 00:20:59 |
| 26 Nov 2021 |
| m1cr0man | If I'm doing some work to fix multiple issues, should I make a PR for each issue or do what I normally do and make one PR for all of them? I always make separate commits for each fix regardless. | 21:50:07 |
| hexa | the latter should be fine given they're separate commits | 22:01:35 |
| hexa | saves rebase headaches between pr's possibly | 22:01:45 |
| m1cr0man | yeah, cool. | 22:06:27 |
| m1cr0man | I also just found that It would be silly (and require a rebase) to fix one of these issues and not the other in one commit :P | 22:06:45 |
| m1cr0man | Btw turns out separating domain hash from the certsDir hash was a great idea - because now i can safely remove domainhash entirely and fix your issue. Btw, can you open a ticket for that, just so that we can track the change reasoning against it? | 22:07:27 |
| hexa | https://github.com/NixOS/nixpkgs/issues/147540 | 22:12:30 |
| hexa | here we go | 22:12:31 |
| m1cr0man | ty# | 22:12:39 |
