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 arg --outfile #272

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

add arg --outfile #272

wants to merge 5 commits into from

Conversation

btwe
Copy link

@btwe btwe commented Oct 23, 2021

I have acme-tiny integrated as a systemd service + a timer. I find it handy to also specify an outfile, where the signed certificate + chain should be written to.

Of course, the default behaviour remains, which prints the signed cert to STDOUT.

Output with --outfile /etc/pki/acme/domainA.cert:

acme_tiny.py[55382]: Certificate signed!
acme_tiny.py[55382]: Writing signed certificate to /etc/pki/acme/domainA.cert
systemd[1]: acme.service: Succeeded.

Output without --outfile:

acme_tiny.py[55468]: Signing certificate...
acme_tiny.py[55468]: Certificate signed!
acme_tiny.py[55468]: Writing signed certificate to STDOUT
acme_tiny.py[55468]: -----BEGIN CERTIFICATE-----
acme_tiny.py[55468]: MIIHPTCCBiWgAwIBAgITAPpExoEu268QebqRtct2tMvL2jANBgkqhkiG9w0BAQsF
...
acme_tiny.py[55468]: oZ2lS38fL18Aon458fbc0BPHtenfhKj5
acme_tiny.py[55468]: -----END CERTIFICATE-----
systemd[1]: acme.service: Succeeded.

Bastian Tweddell added 5 commits October 23, 2021 22:58
- default behaviour: if --outfile is not given or equals "STDOUT" then
  the signed cert is printed to STDOUT as legacy
- otherwise the file given with --outfile is opened for write and sigend
  cert is written to it (no handling of mode bits, because the cert is
  no secret)
this should fix the test errors
@diafygi
Copy link
Owner

diafygi commented Oct 26, 2021

@btwe It looks like this pull request puts acme-tiny over the 200 line limit, so I'm not going to merge it for now. However, I'll keep this pull request open if I decide to increase the line limit, I can merge this.

@btwe
Copy link
Author

btwe commented Oct 26, 2021

I understand, this would change the selling point. But let me try to show another aspect: The line count limit itself does not increase readability. I think you could somehow squeeze the output conditional into a single line. But readability of the source is also a selling point for acme-tiny. My little experience is that trying to stick to pep8 might also increase readability with the human eye, but of course this would blow the 200 lines by a lot (~70)

Don't get me wrong, I do not intend to convince you of changing anything. I know how to patch this PR in on my side; also thanks to the small code base, it is very easy to do this.

So, thanks a lot for your work on acme-tiny and publishing it. I have this PR on watch so I'll get updated 👍

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.

2 participants