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

certbot: add certbot.withPlugins #92786

Merged
merged 4 commits into from Jul 22, 2020
Merged

Conversation

flokli
Copy link
Contributor

@flokli flokli commented Jul 9, 2020

Simplifies making use of certbot plugins introduced in #92696.

This can be used to wrap certbot to include some plugins.

certbot.withPlugins has a similar calling convention as python*.withPackages:

certbot.withPlugins (cp: [ cp.certbot-dns-foo ])
Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

pkgs/tools/admin/certbot/default.nix Show resolved Hide resolved
pkgs/tools/admin/certbot/default.nix Outdated Show resolved Hide resolved
@flokli
Copy link
Contributor Author

flokli commented Jul 21, 2020

@FRidh I pushed the version that tries to use certbot.overridePythonAttrs, but doesn't work. Can you take a look? Otherwise, I'd propose to just drop the last commit and merge as-is.

@FRidh
Copy link
Member

FRidh commented Jul 21, 2020

See https://github.com/FRidh/nixpkgs/tree/certbot for what I would say is the preferred solution.

@flokli
Copy link
Contributor Author

flokli commented Jul 21, 2020

Agreed. Thanks for the commits! I pushed them to this PR (slightly modifying HEAD~1) while doing so. PTAL.

Copy link
Member

@FRidh FRidh left a comment

Choose a reason for hiding this comment

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

IMO the commits can be squashed into a single one. Do try to get rid of the whitespace changes in pkgs/top-level/python-packages.nix. Other than that it looks good to me.

@flokli
Copy link
Contributor Author

flokli commented Jul 22, 2020

I'll squash the commits adding the withPlugins feature, but I think having individual commits of the refactor/move itself should make things more understandable.

The trailing whitespace cleanups are harder to get rid of, my editor is really persistent on cleaning these up :-D I'll see if I can move them into a individual commit, so git blame is happy - we should really just have a github check preventing them from occuring in first place IMHO.

This can be used to wrap certbot to include some plugins.

certbot.withPlugins has a similar calling convention as python*.withPackages:

```
certbot.withPlugins (cp: [ cp.certbot-dns-foo ])
```
@flokli flokli merged commit 3d50d64 into NixOS:master Jul 22, 2020
@flokli flokli deleted the certbot-with-plugins branch July 22, 2020 10:53
@onny
Copy link
Contributor

onny commented Sep 10, 2022

Can I use this inside my system configuration? Not sure how to configure it

environment.systemPackages = with pkgs; [
  (certbot.withPackages (ps: with ps; [ certbot-dns-inwx ] ) )
];

@flokli
Copy link
Contributor Author

flokli commented Sep 10, 2022

You need to use certbot.withPlugins, not certbot.withPackages, so the following would work:

certbot.withPlugins (ps: with ps; [ certbot-dns-cloudflare ])

Note certbot-dns-inwx isn't packaged yet, but you can take a look at the other existing certbox-dns-* plugins, happy to review a PR adding inwx :-)

@onny
Copy link
Contributor

onny commented Sep 10, 2022

Thanks that worked for me, at least importing the plugin where I have the PR here #190677

Unfortunately it doesn't get recognized yet

 environment.systemPackages = with pkgs; [
   ( certbot.withPlugins (ps: with ps; [ python310Packages.certbot-dns-inwx ]) )
 ];
[root@piproxy:~]# certbot certonly -a dns-inwx -d example.org
Saving debug log to /var/log/letsencrypt/letsencrypt.log
The requested dns-inwx plugin does not appear to be installed
Ask for help or search for solutions at https://community.letsencrypt.org. See the logfile /var/log/letsencrypt/letsencrypt.log or re-run Certbot with -v for more details.

@flokli
Copy link
Contributor Author

flokli commented Sep 10, 2022

Let's take this over to the PR, this is unrelated to the introduction of certbot.withPlugins itself.

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

Successfully merging this pull request may close these issues.

None yet

3 participants