Bug 1287193 - Review Request: letsencrypt - A free, automated certificate authority
Summary: Review Request: letsencrypt - A free, automated certificate authority
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Zbigniew Jędrzejewski-Szmek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 1215478 (view as bug list)
Depends On: 1286324
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-12-01 17:48 UTC by James Hogarth
Modified: 2016-06-29 04:49 UTC (History)
31 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-12-05 19:26:38 UTC
Type: ---
Embargoed:
zbyszek: fedora-review+


Attachments (Terms of Use)

Description James Hogarth 2015-12-01 17:48:31 UTC
Spec URL: https://jhogarth.fedorapeople.org/letsencrypt/letsencrypt.spec
SRPM URL: https://jhogarth.fedorapeople.org/letsencrypt/letsencrypt-0.0.0-2.dev20151123.fc23.src.rpm

Description: Let's Encrypt is a free, automated certificate authority that aims to lower the barriers to entry for encrypting all HTTP traffic on the internet.

This package is the core CLI client used to request a certificate.

The initial submission is just the letsencrypt client.

It can be used by:
letsencrypt -d mydomain.example.com certonly --manual

There are other options to authenticate owning the domain than manual (such as webroot or standalone) - see the upstream documentation at https://letsencrypt.readthedocs.org/en/latest/ for details.

At this time the apache and nginx plugins are not included (in order to reduce time for this initial package to be put in place) however the plan is to add these either as subpackages of this rpm or seperate review requests - however there is no dependency on them for the CLI client to be packaged.

Note that until python-acme is in rawhide fedora-review against this bug will not work.

Fedora Account System Username: jhogarth

Comment 1 James Hogarth 2015-12-01 17:50:31 UTC
*** Bug 1215478 has been marked as a duplicate of this bug. ***

Comment 2 James Hogarth 2015-12-01 17:54:01 UTC
Following from the discussion in the other bug wouldn't this be better named python-letsencrypt to follow the general python naming guidelines?

The spec is named letsencrypt for the time being to match the upstream project name and to follow on from previous discussions.

It could still have a provides of letsencrypt so that dnf/tum locates it easily ...

Comment 3 Till Maas 2015-12-01 18:02:09 UTC
(In reply to James Hogarth from comment #2)
> Following from the discussion in the other bug wouldn't this be better named
> python-letsencrypt to follow the general python naming guidelines?

AFAIK the python-* naming is meant for modules not tools. For example dnf is also packaged in dnf but it also builds a sub-package with only the python modules in it. This would be an option if letsencrypt supports both python3 and python2, then there could be a python2 subpackage with only the python2 module files for letsencrypt.

Btw. as far as I understood there were also plans to make letsencrypt available on EPEL but the spec does not yet support EPEL. Do you not want to maintain letsencrypt on EPEL?

Comment 4 Nick 2015-12-01 18:25:55 UTC
I can maintain letsencrypt on EPEL

Comment 5 Zbigniew Jędrzejewski-Szmek 2015-12-01 18:57:54 UTC
letsencrypt seem to be the best option. It matches the upstream URL, pypi package name, executable name, and the python module name.

It seems only python2 is supported, although python3 is available [1]. So it seems that python2 should be used for the binaries... Nevertheless, python2-letsencrypt and python3-letsencrypt packages should be split out. This will allow early adopters to easily test with python3 and easy the eventual transition. Please follow the template [2]. The main package should Require: python2-letsencrypt = %{version}.

[1] https://github.com/letsencrypt/letsencrypt/issues/962
[2] https://fedoraproject.org/wiki/Packaging:Python#Example_common_spec_file

Requires: python2-dialog >= 3.3.0

The plan to support only manual mode in the beggining sounds reasonable, although it would be really nice to integrate with Fedora apache config so that the apache automatic mode works ootb.

Till, do you want to do the formal review, or should I?

Comment 6 Till Maas 2015-12-01 19:48:29 UTC
Zbigniew, please go ahead with the review.

Comment 7 James Hogarth 2015-12-02 11:31:45 UTC
(In reply to Till Maas from comment #3)
> (In reply to James Hogarth from comment #2)
> > Following from the discussion in the other bug wouldn't this be better named
> > python-letsencrypt to follow the general python naming guidelines?
> 
> AFAIK the python-* naming is meant for modules not tools. For example dnf is
> also packaged in dnf but it also builds a sub-package with only the python
> modules in it. This would be an option if letsencrypt supports both python3
> and python2, then there could be a python2 subpackage with only the python2
> module files for letsencrypt.
> 
> Btw. as far as I understood there were also plans to make letsencrypt
> available on EPEL but the spec does not yet support EPEL. Do you not want to
> maintain letsencrypt on EPEL?

To simplify this somewhat and cut down effort and time to get it into Fedora as the primary concern the plan is to get this reviewed for rawhide and tested in there.

Once we are happy with it working properly there (to clarify standalone, manual and webroot will be supported initially as I've had reports of the apache plugin not playing nicely with Fedora as it was written with Debian in mind and would like to take a better look at that in due course) Nick and I will work together on a separated spec to avoid messing this up with conditionals and will file bugs as required to get the dependencies in EPEL7 (and after that EL6 if it's feasible).

As it stands right now the SRPM cannot be built for EPEL7 (or even f23) due to various dependencies either outright missing or needing an update.

I've split the spec up to letsencrypt and python2-letsencrypt with the docs and %{_bindir} files in the former and the libraries in the latter with the idea of starting it separated this way to make a clean transition to python3-letsencrypt when upstream support that in due course.

That new spec will be uploaded a little later on today when I've had a chance to test a bit more.

I've requested the rawhide git for python-acme so give it 24 hours or so and we should have that in rawhide and fedora-review against this will be possible.

Comment 8 Zbigniew Jędrzejewski-Szmek 2015-12-02 15:19:46 UTC
(In reply to James Hogarth from comment #7)
> That new spec will be uploaded a little later on today when I've had a
> chance to test a bit more.
Great, I'll review it than.

> I've requested the rawhide git for python-acme so give it 24 hours or so and
> we should have that in rawhide and fedora-review against this will be
> possible.
Yeah. No need to mention this though, putting the bug in "Depends on" field means pretty much the same thing. There's no problem with running fedora-review against unreleased packages, it is easy enough to install something in the mock chroot beforehand.

Comment 9 James Hogarth 2015-12-02 18:35:26 UTC
Spec URL: https://jhogarth.fedorapeople.org/letsencrypt/letsencrypt.spec
SRPM URL: https://jhogarth.fedorapeople.org/letsencrypt/letsencrypt-0.0.0-3.dev20151123.fc23.src.rpm

I've used this within the mock chroot to obtain a valid certificate for my domain.

The docs are poor at the moment at best (readme, changes and contributing) but that's due to upstream being pretty bad ... I'll write up something decent for man pages to pass back upstream during the F24 release cycle - but in the meanwhile the docs (and upcoming Fedora article) will be sufficient to get early adopters using rawhide to start testing.

As I suspected letsencrypt-apache is a no-go for now (calls debianisms such as a2enmod) but that can come later as well ...

This build has tests enabled in %check and has the python libraries themselves separated out from the the files in %{_bindir} as previously discussed to facilitate testing and switching to python3 when upstreams supports in in coming months.

This build also adds a %ghost for /etc/letsencrypt as after first run certificates and configurations for the domains end up in here. Optionally we could create this directly in the RPM I guess ... up to personal preference... I've argued myself both directions in having the etc skeleton in the RPM or letting letsencrypt create it itself over the course of the day ...

Comment 10 James Hogarth 2015-12-02 18:44:54 UTC
Spec URL: https://jhogarth.fedorapeople.org/letsencrypt/letsencrypt.spec
SRPM URL: https://jhogarth.fedorapeople.org/letsencrypt/letsencrypt-0.0.0-4.dev20151123.fc23.src.rpm

One quick change - the python2 libraries should have had the requires and not the bindir one ... else if someone just installs the libraries to use on their own client it won't work... 

End of day tired brain fart - apologies...

Comment 11 James Hogarth 2015-12-03 09:59:29 UTC
With python-acme being inserted in my mock chroot whilst building and testing this it polluted the environment a bit ...

There's python libraries missing from BuildRequires to get tests in %check to pass ...

Just working through them to get the minimal set in place and then release 5 will be up.

I'll probably bump this to dev20151202 today as well as that's required for the open beta (has a change in CA URL).

Comment 12 James Hogarth 2015-12-03 11:45:25 UTC
Right package updated for the open beta ...

Spec URL: https://jhogarth.fedorapeople.org/letsencrypt/letsencrypt.spec
SRPM URL: https://jhogarth.fedorapeople.org/letsencrypt/letsencrypt-0.1.0-1.fc23.src.rpm

This does have a dependency on python2-acme-0.1.0 which has been updated for the open beta and is in the process of being built in koji for rawhide.

This has been tested in a mock chroot for my domain and acquired and revoked certificates using --manual as expected:

https://crt.sh/?id=10989140

Until I've written man pages I'm happy (pending any alterations to spec to comply with review) for this to end up in rawhide I think now.

Comment 13 Robert Buchholz 2015-12-03 18:51:23 UTC
James and everyone else, thank you for your effort.

I've been wading through the steps necessary to get this running on EPEL. I'm not sure this may be a discussion to be had in a new bug report specific to EPEL?
I have all changed files in git and SRPMs on COPR at https://copr.fedoraproject.org/coprs/rbu/letsencrypt/

The following packages are either not yet in EPEL or they are in EPEL and can be built on COPR:
python-cryptography-vectors
python-configargparse
python-dialog
python-ipaddress
python-pyrfc3339
python-sphinxcontrib-programoutput

The following packages are either not yet in EPEL, but fail to build:
letsencrypt  # due to failed dependencies
python-acme  # due to failed dependencies
python-cryptography  # due to failing tests and missing dependencies, but the tests fail on rawhide as well
python-parsedatetime  # due to failing tests, but the tests fail on rawhide as well

I also tried to rebuild python-six-1.9.0-2.el7.src.rpm from Centos 7.2 "CR" (a dependency of python-cryptography), but it fails on COPR. It should only be a question of time until the build slaves have the latest "six".

Most problematic are these two packages that are in EL7 (cffi moved to EL7 recently in 7.2, but it was never in EPEL):
python-cffi
python-pyasn1

Comment 14 James Hogarth 2015-12-03 19:02:22 UTC
(In reply to Robert Buchholz from comment #13)
> James and everyone else, thank you for your effort.
> 
> I've been wading through the steps necessary to get this running on EPEL.
> I'm not sure this may be a discussion to be had in a new bug report specific
> to EPEL?
> I have all changed files in git and SRPMs on COPR at
> https://copr.fedoraproject.org/coprs/rbu/letsencrypt/
> 

Let's keep the focus on Fedora 24 in this review and once that's sorted we can track the EPEL packaging in an EPEL specific bug or the epel-devel mailing list.

Comment 15 Zbigniew Jędrzejewski-Szmek 2015-12-03 21:40:01 UTC
- Please also build the python3 subpackage

- What about the docs? Shouldn't they be built?
  (python-repoze-sphinx-autointerface.noarch seems to be required, and
   $PATH has to be set to include the executables, but it seems to work fine.)

Comment 16 James Hogarth 2015-12-03 22:41:21 UTC
(In reply to Zbigniew Jędrzejewski-Szmek from comment #15)
> - Please also build the python3 subpackage
>

On my last tests %py3_build failed to build as is and seeing as upstream doesn't support anything but python2 right now I have no idea how much work there is to do so ... if py2to3 will be enough or if there are more fundamental issues.

Right now I don't see this feasible, as soon as it is it'll be added.
 
> - What about the docs? Shouldn't they be built?
>   (python-repoze-sphinx-autointerface.noarch seems to be required, and
>    $PATH has to be set to include the executables, but it seems to work
> fine.)

The man page that gets built is just the output of --help all .. it's not actually very useful.

I can do it but I just didn't think it added much.

I am in the process of writing something *useful* for that to submit back upstream which can be included at that time.

Happy to modify if you feel that is a MUST though...

Comment 17 Robert Buchholz 2015-12-04 01:26:58 UTC
(In reply to James Hogarth from comment #14)
> Let's keep the focus on Fedora 24 in this review and once that's sorted we
> can track the EPEL packaging in an EPEL specific bug or the epel-devel
> mailing list.

Agreed. There's going to be a lot to sort out to get this into EL7 proper. James and Nick proposed to open a tracker bug once this passes the Fedora review. Until then, I'll be working in the COPR linked above. I did get all packages to build on EL7 in the repository, which should allow for some testing. If need be, please send feedback to me off-bug.

Comment 18 James Hogarth 2015-12-04 17:19:58 UTC
Spec URL: https://jhogarth.fedorapeople.org/letsencrypt/letsencrypt.spec
SRPM URL: https://jhogarth.fedorapeople.org/letsencrypt/letsencrypt-0.1.0-2.fc23.src.rpm


As discussed no python3 as it's not supported by upstream and fails to build. This will be added once upstream supports it (it is on there roadmap post GA).

A docs subpackage has been added (recommended by python2-letsencrypt) as well with the API docs and the man pages added (even though they are just --help output) to the packages.

Comment 19 Zbigniew Jędrzejewski-Szmek 2015-12-05 00:37:52 UTC
- license is OK
- license file is present, %license is used
- latest version
- uses the new python packaging template
- requires and provides look ok
- no scriptlets
- builds, install, runs fine
- rpmlint:

letsencrypt.noarch: W: spurious-executable-perm /usr/share/man/man1/letsencrypt-renewer.1.gz
letsencrypt.noarch: W: spurious-executable-perm /usr/share/man/man1/letsencrypt.1.gz
Please remove executable bit.

letsencrypt-doc.noarch: W: spurious-executable-perm /usr/share/man/man7/letsencrypt.7.gz
There's no need for this file to be in doc subpackage too.

letsencrypt.noarch: E: non-readable /etc/letsencrypt 0
This dir is %ghosted, seems to be a bug in rpmlint.

letsencrypt.src:60: W: unversioned-explicit-provides bundled(jquery)
letsencrypt.src:61: W: unversioned-explicit-provides bundled(underscore)
letsencrypt.src:62: W: unversioned-explicit-provides bundled(inconsolata-fonts)
letsencrypt.src:63: W: unversioned-explicit-provides bundled(lato-fonts)
letsencrypt.src:64: W: unversioned-explicit-provides bundled(robotoslab-fonts)
You should add versions... Probably matters the most for jquery and underscore, the rest can be ignored.

letsencrypt-doc.noarch: W: dangling-symlink /usr/share/doc/letsencrypt-doc/html/_static/fonts/fontawesome-webfont.eot /usr/share/fonts/fontawesome/fontawesome-webfont.eot
letsencrypt-doc.noarch: W: dangling-symlink /usr/share/doc/letsencrypt-doc/html/_static/fonts/fontawesome-webfont.svg /usr/share/fonts/fontawesome/fontawesome-webfont.svg
letsencrypt-doc.noarch: W: dangling-symlink /usr/share/doc/letsencrypt-doc/html/_static/fonts/fontawesome-webfont.ttf /usr/share/fonts/fontawesome/fontawesome-webfont.ttf
letsencrypt-doc.noarch: W: dangling-symlink /usr/share/doc/letsencrypt-doc/html/_static/fonts/fontawesome-webfont.woff /usr/share/fonts/fontawesome/fontawesome-webfont.woff
Those are actually fine when installed.

python2-letsencrypt.noarch: W: no-documentation
python2-letsencrypt.noarch: W: pem-certificate /usr/lib/python2.7/site-packages/letsencrypt/tests/testdata/cert-san.pem
python2-letsencrypt.noarch: W: pem-certificate /usr/lib/python2.7/site-packages/letsencrypt/tests/testdata/cert.pem
python2-letsencrypt.noarch: W: pem-certificate /usr/lib/python2.7/site-packages/letsencrypt/tests/testdata/dsa_cert.pem
python2-letsencrypt.noarch: W: pem-certificate /usr/lib/python2.7/site-packages/letsencrypt/tests/testdata/matching_cert.pem
4 packages and 0 specfiles checked; 1 errors, 17 warnings.

Those are obviously used for testing, so that's all fine.

Please fix up the small issues pointed out above when uploading. Package is APPROVED.

Comment 20 Till Maas 2015-12-05 10:37:00 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/letsencrypt

Comment 21 James Hogarth 2015-12-05 19:26:38 UTC
This has now been built in rawhide (I do need to dig into the versions of stuff bundled ... I'll do this later next week though as it is just docs).

Tracker bugs opened for F23, EPEL6 and EPEL7 which will have appropriate blockers added in due course for those following interested in those platforms.


Note You need to log in before you can comment on or make changes to this bug.