Bug 221043 - security/py-certbot: Add periodic script for renewing certificates
Summary: security/py-certbot: Add periodic script for renewing certificates
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Kubilay Kocak
URL:
Keywords: feature
Depends on:
Blocks:
 
Reported: 2017-07-27 10:08 UTC by Dmitry Marakasov
Modified: 2019-08-13 15:18 UTC (History)
4 users (show)

See Also:
asomers: maintainer-feedback? (koobs)


Attachments
Patch (2.71 KB, patch)
2017-07-27 10:08 UTC, Dmitry Marakasov
no flags Details | Diff
Patch v2 (2.71 KB, patch)
2017-08-03 15:06 UTC, Dmitry Marakasov
no flags Details | Diff
Add periodic script for security/py-certbot (2.71 KB, patch)
2017-09-02 16:41 UTC, Alan Somers
no flags Details | Diff
Modified version of Alan's patch (2.12 KB, patch)
2019-02-15 21:24 UTC, Yasuhiro KIMURA
no flags Details | Diff
Modified version of Alan's patch (2.12 KB, patch)
2019-02-15 21:32 UTC, Yasuhiro KIMURA
no flags Details | Diff
Modified version of Alan's patch (2.15 KB, patch)
2019-03-01 01:54 UTC, Yasuhiro KIMURA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Marakasov freebsd_committer 2017-07-27 10:08:28 UTC
Created attachment 184762 [details]
Patch

- Fix run-depend on acme, == version depends it breaks on PORTREVISION
- Remove python version limitation, it works fine with python 3.6
- Add periodic script for automatic certificate renewal
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2017-07-31 03:09:45 UTC
Thanks Dmitry,

- For allowing Python 3.x builds, see bug 220469
- For fixing acme RUN_DEPENDS version spec, it should have always been PORTVERSION not PKGVERSION (which includes PORTREVISION). *_DEPENDS for Python ports should reflect the upstream *_requires versions as accurately as possible (without being incorrect or causing errors), so >0 is not quite right here.

Could you please:

- Update to patch to only include the periodic additions/changes please? I'll make each of the changes separately (referencing the relevant issue ID's)

- Change the word 'Updating' to 'Renewing' in the script (obvious/consistent terminology)
Comment 2 Dmitry Marakasov freebsd_committer 2017-08-03 15:06:34 UTC
Created attachment 184997 [details]
Patch v2

> - For fixing acme RUN_DEPENDS version spec, it should have always been PORTVERSION not PKGVERSION (which includes PORTREVISION).

This won't work either, as it still compares versions including PORTEPOCH and PORTREVISION, so any PORTREVISION change in either py-acme or py-certpot will break the latter. == should never be used in *_DEPENDS.

> Python ports should reflect the upstream *_requires versions as accurately as possible

Upstream build system should take care of it, there's no excuse for condition duplication.

> - Update to patch to only include the periodic additions/changes please? I'll make each of the changes separately (referencing the relevant issue ID's)
> 
> - Change the word 'Updating' to 'Renewing' in the script (obvious/consistent terminology)

Done
Comment 3 Kubilay Kocak freebsd_committer freebsd_triage 2017-08-06 10:50:13 UTC
Testing indicates the following (with dummy/testing PORTREVISION=1 entry created in Makefile):

[security/py-certbot] make -V PORTREVISION
1
[security/py-certbot] make -V PORTEPOCH
1
[security/py-certbot] make -V PORTVERSION
0.16.0
[security/py-certbot] make -V PKGVERSION
0.16.0_1,1

Further:

1) unless setup.py:*_requires dependencies are also patched, software will fail at runtime with (setuptools:pkg_resources) dependency mismatch errors.

2) Even when patching *_requires, arbitrarily setting relaxed *_DEPENDS can produce incompatible and broken environments unless API compatibility is thoroughly checked and verified. In these cases it is instead recommended and preferable to ask upstream to use >= dependencies so that their CI systems test with the latest versions. 

3) The version-spec (==) provides a bonus early warning system to ensure that mismatched versions aren't committed which would then impact users, and that updates are committed together where they need to be. This has worked well for Carlos and I so far.

I'll be landing the other two changes shortly so they can be merged, then will land the rc script feature from this issue
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2017-08-06 11:09:33 UTC
(In reply to Dmitry Marakasov from comment #2)

Ah, I think I see what you meant: the version checked is always against the installed port/package, which includes PORTREVISION/PORTEPOCH.
Comment 5 Kubilay Kocak freebsd_committer freebsd_triage 2017-08-06 11:12:57 UTC
(In reply to Kubilay Kocak from comment #4)

>=${PORTVERSION} will have to do for now then
Comment 6 commit-hook freebsd_committer 2017-08-06 11:29:13 UTC
A commit references this bug:

Author: koobs
Date: Sun Aug  6 11:28:15 UTC 2017
New revision: 447458
URL: https://svnweb.freebsd.org/changeset/ports/447458

Log:
  security/py-certbot: Relax Python version-spec, support Python 3.x

  certbot has supported Python 3 (3.3+) since the 0.14.0 release [1]. Update
  the USES=python:<version-spec> to match, allowing Python 3 builds. [2]

  Update acme (security/py-acme) RUN_DEPENDS entry to use PORTVERSION not
  PKGVERSION (that includes PORTEPOCH/PORTREVISION) which caused the
  dependency to be incorrectly reported as unsatisfied if either was
  defined. Further, since *_DEPENDS version-specifiers compare against
  installed package versions, only a version that includes PORTEPOCH
  and/or PORTREVISION is available, so use >= not ==. [3]

  While I'm here, enable 'concurrent' to automatically produce
  version-suffixed files.

  [1] https://github.com/certbot/certbot/issues/4507

  PR:		220469 221043 [3]
  Submitted by:	Kamigishi Rei <spambox haruhiism net> [2]
  Reported by:	amdmi3 [3]
  MFH:		2017Q3

Changes:
  head/security/py-certbot/Makefile
Comment 7 commit-hook freebsd_committer 2017-08-17 13:05:06 UTC
A commit references this bug:

Author: feld
Date: Thu Aug 17 13:04:12 UTC 2017
New revision: 448093
URL: https://svnweb.freebsd.org/changeset/ports/448093

Log:
  MFH: r445423 r447458

  security/py-{acme,certbot}: Update to 0.16.0

  Common:

  - Update PORTVERSION and distinfo checksum (0.16.0)

  Changelog: https://github.com/certbot/certbot/blob/master/CHANGELOG.md#0160---2017-07-05

  Reviewed by:	koobs (maintainer, py-certbot)
  Approved by:	koobs (maintainer, py-certbot)
  Differential Revision:	https://reviews.freebsd.org/D11517

  security/py-certbot: Relax Python version-spec, support Python 3.x

  certbot has supported Python 3 (3.3+) since the 0.14.0 release [1]. Update
  the USES=python:<version-spec> to match, allowing Python 3 builds. [2]

  Update acme (security/py-acme) RUN_DEPENDS entry to use PORTVERSION not
  PKGVERSION (that includes PORTEPOCH/PORTREVISION) which caused the
  dependency to be incorrectly reported as unsatisfied if either was
  defined. Further, since *_DEPENDS version-specifiers compare against
  installed package versions, only a version that includes PORTEPOCH
  and/or PORTREVISION is available, so use >= not ==. [3]

  While I'm here, enable 'concurrent' to automatically produce
  version-suffixed files.

  [1] https://github.com/certbot/certbot/issues/4507

  PR:		220469 221043 [3]
  Submitted by:	Kamigishi Rei <spambox haruhiism net> [2]
  Reported by:	amdmi3 [3]

  Approved by:	ports-secteam (with hat)

Changes:
_U  branches/2017Q3/
  branches/2017Q3/security/py-acme/Makefile
  branches/2017Q3/security/py-acme/distinfo
  branches/2017Q3/security/py-certbot/Makefile
  branches/2017Q3/security/py-certbot/distinfo
Comment 8 Alan Somers freebsd_committer 2017-09-02 16:39:32 UTC
Comment on attachment 184997 [details]
Patch v2

A few comments on the periodic script:
1) Periodic scripts are sorted by the leading number.  000 will run before any other script.  So you probably shouldn't use that without good reason.  Pick a number in the middle instead.
2) Your web server won't use the new certificate until it gets restarted.  You can take advantage of certbot's hook scripts to restart the web server if and only if it's necessary.
Comment 9 Alan Somers freebsd_committer 2017-09-02 16:41:47 UTC
Created attachment 185994 [details]
Add periodic script for security/py-certbot

Here is an alternate patch that I've been using.  It differs from the OP in three ways:
1) Monthly instead of weekly
2) Runs in the middle of the periodic sequence instead of at the beginning
3) Can optionally restart your web server to use the new certificate.
Comment 10 Walter Schwarzenfeld freebsd_triage 2018-01-16 15:39:17 UTC
Is this still relevant or is it fixed?
Comment 11 Alan Somers freebsd_committer 2018-01-16 15:44:43 UTC
Hm, experience shows that monthly runs aren't frequent enough.  My last run on January-1 didn't renew the certificate because it wasn't up for renewal yet.  And today I got an expiration notice email.  I'll update the patch to make it weekly again, as in Dmitry's original patch.
Comment 12 Walter Schwarzenfeld freebsd_triage 2018-01-16 15:50:55 UTC
Ok, thanks for reply.
Comment 13 Yasuhiro KIMURA 2019-02-15 21:24:23 UTC
Created attachment 202051 [details]
Modified version of Alan's patch

Attached file is modified version of Alan's patch. Changes are,

* Make it fit to latest ports tree.
* Switch back from monthly to weekly.
* Use anticongestion instead of random sleep feature of certbot.
Comment 14 Yasuhiro KIMURA 2019-02-15 21:32:22 UTC
Created attachment 202052 [details]
Modified version of Alan's patch

Oops, I forgot to update comment in periodic script. Please commit attached patch instead of my original one.
Comment 15 Yasuhiro KIMURA 2019-03-01 01:54:51 UTC
Created attachment 202464 [details]
Modified version of Alan's patch

Update patch to make message compatible with other periodic scripts.
Comment 16 Walter Schwarzenfeld freebsd_triage 2019-08-13 11:23:58 UTC
ping!
Comment 17 Alan Somers freebsd_committer 2019-08-13 15:17:42 UTC
Requesting feedback for Yasuhiro's patch.