Bug 1190269 - Review Request: openstack-barbican - Secrets as a Service
Summary: Review Request: openstack-barbican - Secrets as a Service
Keywords:
Status: MODIFIED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Haïkel Guémar
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-02-06 20:30 UTC by Greg Swift
Modified: 2015-12-05 00:33 UTC (History)
10 users (show)

Fixed In Version: openstack-barbican-1.0.0-1.el7
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: ---
karlthered: fedora-review+


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Launchpad 1504291 None None None Never

Description Greg Swift 2015-02-06 20:30:27 UTC
Spec URL: http://nytefyre.net/rpms/openstack-barbican.spec
SRPM URL: http://nytefyre.net/rpms/openstack-barbican-2015.1-1.0b2.fc21.src.rpm
Description:

I'm an engineer on a team close to the Barbican dev group at Rackspace. I've been pushing to get it packaged and pushable into Fedora, and so here it is.  For all of its dependencies to be met it only builds in fc22. fc21 is missing python-keystonemiddleware iirc.

The kilo release and FC22 seem to be targetted around the same time for release, this is the kilo-2 release candidate.  K-3 (final rc) should be cut before the FC22 feature freeze.

Barbican is a ReST API designed for the secure storage, provisioning and management of secrets. It is aimed at being useful for all environments, including large ephemeral Clouds.

Fedora Account System Username: xaeth

Comment 1 Greg Swift 2015-02-09 18:30:24 UTC
Update:

To follow closer with the RDO openstack process I've rolled this back to juno stable:

Spec URL: http://nytefyre.net/rpms/openstack-barbican.spec
SRPM URL: http://nytefyre.net/rpms/openstack-barbican-2014.2-1.fc21.src.rpm

Comment 2 Alan Pevec (Fedora) 2015-02-09 21:20:08 UTC
> The kilo release and FC22 seem to be targetted around the same time for
> release, this is the kilo-2 release candidate.  K-3 (final rc) should be cut
> before the FC22 feature freeze.

> To follow closer with the RDO openstack process I've rolled this back to juno stable.

Thanks for the package!
Let me just clarify for those following along: OpenStack K-3 is final development milestone, RCs start Apr 9 [1] and Fedora 22 Code Complete is Mar 31 [2] so final Kilo misses it. But also we (RDO packaging team) are keeping OpenStack / Fedora mapping 1:1 and F22 is mapped [3] to Juno.
Kilo packing, until F22 is branched, is developed in separate package repositories[4].


[1] https://wiki.openstack.org/wiki/Kilo_Release_Schedule
[2] https://fedoraproject.org/wiki/Releases/22/Schedule
[3] https://fedoraproject.org/wiki/OpenStack#OpenStack
[4] https://openstack.redhat.com/packaging/rdo-packaging.html#master-pkg-guide

Comment 3 Haïkel Guémar 2015-04-01 11:36:21 UTC
Sorry for the delay:
1. python-barbican contains no python modules, that's due to missing BuildRequires. By adding oslo stuff, I was able to fix that issue.
https://github.com/openstack/barbican/blob/stable/juno/requirements.txt
=> blocker as it prevents barbican to be built properly in a chrooted env.

2. use python versioned macros %{__python2} and %{__python2_sitelib}
For EL6, here's a fallback macro
%if 0%{?rhel} && 0%{?rhel} <= 6
%{!?__python2:        %global __python2 /usr/bin/python2}
%{!?python2_sitelib:  %global python2_sitelib %(%{__python2} -c "from distutils.sysconfig import get_python_lib; print(get_python_lib())")}
%{!?python2_sitearch: %global python2_sitearch %(%{__python2} -c "from distutils.sysconfig import get_python_lib; print(get_python_lib(1))")}
%endif

3.we are dropping PBR patches starting kilo, so you may just drop it too and add Requires: python-pbr to python-barbican

4.drop the %clean section not required for EL6+ and supported Fedora

5. drop the %defattr, RPM has sensible defaults for EL6+ 

6.minor but I would prefer using an openstack url rather than the cloudkeep one


Please fix and set me in the needinfo flag when you're done.

Comment 4 Greg Swift 2015-04-01 15:04:46 UTC
1: I'm confused. I have python modules in my python-barbican rpms that was built from this.  While not BuildRequires, there are requires on oslo config and messaging attached to python-barbican. Adding them as BR didn't change the output of the build.

2: Done, and converted rest to use these macros

3: Added conditional to be removed when all the other juno specific bits are removed.  python-pbr is already required.

4, 5: done

6: That was an artifact from when this was first created. Updated.

URL for the item in my git repo:
https://github.com/gregswift/barbican-spec/blob/juno/openstack-barbican.spec

Updated info:
Spec URL: http://nytefyre.net/rpms/openstack-barbican.spec
SRPM URL: http://nytefyre.net/rpms/openstack-barbican-2014.2-2.fc21.src.rpm

Comment 5 Haïkel Guémar 2015-04-01 22:44:47 UTC
The main difference is likely to be that I used mock to rebuild the package so in a minimal chrooted environment.
You probably have these installed in build host and they were detected by setup.py

Remains few issues reported by rpmlint.
* Summary of python-barbican ends with a dot, drop it
* logrotate file should be marked as %config
* why barbican-api.conf is in openstack-barbican-worker subpackage ?
* I would create a barbican-api subpackage
* the pbr patch is still there but not needed since there's a requirement on pbr
* no version in changelog.

Comment 6 Greg Swift 2015-04-08 04:39:33 UTC
(In reply to Haïkel Guémar from comment #5)
> The main difference is likely to be that I used mock to rebuild the package
> so in a minimal chrooted environment.
I had built it in a mock environment (specifically via a local mockchain as well as in copr). I thought i had run installs off of it. Looking back now that package is not clean, so i'm not sure what happened with that testing. I should have realized this when asking the question before. Sorry.

> Remains few issues reported by rpmlint.
> * Summary of python-barbican ends with a dot, drop it
done
> * logrotate file should be marked as %config
done
> * why barbican-api.conf is in openstack-barbican-worker subpackage ?
the worker uses this config file right now.  refactoring to where it just requires the -api. which is annoying but less obtuse.
> * I would create a barbican-api subpackage
done, but still leaves barbican-api.conf in worker subpackage until they can separate that out.
> * the pbr patch is still there but not needed since there's a requirement on
> pbr
you said post-juno. Its still there, for juno. There are several juno specific bits in there.  When one part goes, it all does.
> * no version in changelog.
done

URL for the item in my git repo:
https://github.com/gregswift/barbican-spec/blob/juno/openstack-barbican.spec

Updated info:
Spec URL: http://nytefyre.net/rpms/openstack-barbican.spec
SRPM URL: http://nytefyre.net/rpms/openstack-barbican-2014.2-3.fc21.src.rpm

Comment 7 Haïkel Guémar 2015-05-18 12:52:07 UTC
Services cannot run, they require barbican.sh which is not installed.
I suggest that we find a better way to start barbican services in the future.

Comment 8 Greg Swift 2015-05-18 14:31:31 UTC
Ya, unfortunately that is a growth point issue for the project. barbican.sh is very much a development bootstrap. It does side loading via pip, etc.

In talking to a few of the developers I originally got 'well, they run it with whatever engine they want'.  I have since gotten more buy-in on the concept of making it run in apache with mod_uwsgi.  But they haven't gotten to that yet.

Comment 9 Haïkel Guémar 2015-05-18 19:38:26 UTC
By curiosity, I gave a shot to the paste-based scripts provided upstream (master) and they work perfectly fine, I was able to run service without glitches.
https://github.com/openstack/barbican/tree/master/bin

Could we ship them separately for Juno ?

Comment 10 Greg Swift 2015-05-20 03:39:07 UTC
hrm.. not sure why i didnt include those. dont see why we couldnt just use them.  i'm gonna be out for about a week then i'll get on this.

Comment 11 Michael McCune 2015-06-09 22:53:18 UTC
there has been some recent activity surrounding creating better scripts for barbican. here is a bug[1] that has been logged to track the effort.


there are also plans to create a better wsgi container app for barbican to ease integration into existing platforms, for example apache. but these are still in the planning stages.


i think it would be worthwhile to take barbican.sh and create very trimmed down versions of the start and stop commands found within. i have some experimental pocs for these scripts and would be happy to share if needed.


[1]: https://bugs.launchpad.net/barbican/+bug/1462458

Comment 12 Michael McCune 2015-06-24 15:18:25 UTC
update on the start/stop scripts.

at the last barbican upstream meeting user jkf shared with me a few systemd unit files for barbican that are in use on centos systems. these look like they could be adapted to suit this package as well.

the one caveat is that they require uwsgi/systemd integration, which i believe is already present in fedora. also, this would require including uwsgi as a dependency. i'm not sure if this is acceptable or if we are required to have a separate runner for the barbican controller.

Comment 13 Kevin Fox 2015-06-24 16:18:20 UTC
I've been using uwsgi and systemd on the rpm's I had to hand craft for our centos 7.1 system. It is working ok. I think its a good solution to make progress on this.

Comment 14 Greg Swift 2015-07-01 15:04:20 UTC
Michael did a bunch of work to get the spec updated. I've merged Michael's changes into the spec:

https://github.com/gregswift/barbican-spec/commit/43f5dd2100815365b638e172f9c2b1479b1c22f8#diff-97d28e1bc9d09fb06e5f6409dc9ad722

And updated the published spec and SRPM:
Spec URL: http://nytefyre.net/rpms/openstack-barbican.spec
SRPM URL: http://nytefyre.net/rpms/openstack-barbican-2014.2-4.fc21.src.rpm

Comment 15 Greg Swift 2015-07-01 15:05:07 UTC
(In reply to Kevin Fox from comment #13)
> I've been using uwsgi and systemd on the rpm's I had to hand craft for our
> centos 7.1 system. It is working ok. I think its a good solution to make
> progress on this.

Kevin, if this new RPM should work on your el7 system.

Comment 16 Kevin Fox 2015-07-01 15:29:23 UTC
Got it to build. Trying to install but python-cryptography is needed by python-barbican-2014.2-4.el7.centos.noarch but not in epel. I'll see if I can dig up a copy somewhere.

Thanks,
Kevin

Comment 17 Haïkel Guémar 2015-07-03 08:03:01 UTC
python-cryptography is branched but all EL7 builds but it's in RDO repositories.
Note that OpenStack services are not built in EPEL for EL but on CentOS buildsystem so it shouldn't matter.

Comment 18 Haïkel Guémar 2015-07-03 13:11:53 UTC
Few things:
* a typo
install -p -D -m 644 %{SOURCE1} %{buildroot}%{_unitdir}/openstack-barbican-worker.service
should be =>
install -p -D -m 644 %{SOURCE2} %{buildroot}%{_unitdir}/openstack-barbican-worker.service
* openstack-barbican-worker unit is not working as it's calling barbican-worker with the wrong name.
* missing logrotate configuration file
http://pkgs.fedoraproject.org/cgit/openstack-neutron.git/tree/neutron.logrotate

Comment 19 Greg Swift 2015-07-08 15:58:16 UTC
* Updated the spec to fix types
* Rename the bin scripts based on an upstream bug being worked on
* Everything uses the same log file, so moved the logrotate to the shared base package

Spec URL: http://nytefyre.net/rpms/openstack-barbican.spec
SRPM URL: http://nytefyre.net/rpms/openstack-barbican-2014.2-5.fc21.src.rpm

Comment 20 Haïkel Guémar 2015-07-09 11:28:03 UTC
Fails to build

RPM build errors:
error: Installed (but unpackaged) file(s) found:
   /usr/bin/barbican-db-manage.py
   /usr/bin/barbican-worker.py
    Installed (but unpackaged) file(s) found:
   /usr/bin/barbican-db-manage.py
   /usr/bin/barbican-worker.py


---

That's due to these lines:
install -m 755 bin/barbican-worker.py %{buildroot}%{_bindir}/barbican-worker
install -m 755 bin/barbican-db-manage.py %{buildroot}%{_bindir}/barbican-db-manage

You actually copied the files but originals were not deleted.
I recommend using this snippet instead but the point is to remove original files

for command in bin/*.py; do
  cp -p $command ${command%.py} && rm $command
done;

Comment 22 Haïkel Guémar 2015-09-01 22:53:30 UTC
Only small change before importing the package
%doc LICENSE => %license LICENSE
For El6, you'd need the following macros as fallback: %{!?_licensedir:%global license %%doc}

I hereby approve this package into Fedora Packages Collection. Please submit a SCM
request.

I also suggest that you add apevec as owner of your package.



Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Apache (v2.0)", "Unknown or generated", "*No copyright* Apache
     (v2.0)". 2 files have unknown license. Detailed output of licensecheck
     in /home/haikel/1190269-openstack-barbican/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: Package requires other packages for directories it uses.
     Note: No known owner of /etc/barbican/vassals
[x]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/lib/systemd,
     /usr/lib/systemd/system, /etc/logrotate.d, /etc/barbican,
     /etc/barbican/vassals
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[x]: Each %files section contains %defattr if rpm < 4.4
     Note: %defattr present but not needed
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[ ]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[x]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 61440 bytes in 3 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: %config files are marked noreplace or the reason is justified.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: No %config files under /usr.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[x]: Python eggs must not download any dependencies during the build
     process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[-]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in python-
     barbican , openstack-barbican-api , openstack-barbican-worker
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Scriptlets must be sane, if used.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Spec use %global instead of %define unless justified.
     Note: %define requiring justification: %{?release_number: %define
     milestone 0b%{release_number}}, %{?milestone: %define
     version_milestone .%{milestone}}, %{?release_number: %define
     release_version %{release_name}-%{release_number}},
     %{!?release_version: %define release_version %{release_name}}
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX is a working URL.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: openstack-barbican-2014.2-6.fc24.noarch.rpm
          python-barbican-2014.2-6.fc24.noarch.rpm
          openstack-barbican-api-2014.2-6.fc24.noarch.rpm
          openstack-barbican-worker-2014.2-6.fc24.noarch.rpm
          openstack-barbican-2014.2-6.fc24.src.rpm
openstack-barbican.noarch: W: spelling-error %description -l en_US passphrases -> pass phrases, pass-phrases, paraphrases
openstack-barbican.noarch: E: incoherent-logrotate-file /etc/logrotate.d/barbican-api
openstack-barbican.noarch: W: no-manual-page-for-binary barbican-db-manage

(dropped false-positive warnings as the output is too long for bugzilla)

openstack-barbican-worker.noarch: W: log-files-without-logrotate ['/var/log/barbican']
openstack-barbican-worker.noarch: W: no-manual-page-for-binary barbican-worker
openstack-barbican.noarch: E: incoherent-logrotate-file /etc/logrotate.d/barbican-api
openstack-barbican.noarch: W: no-manual-page-for-binary barbican-db-manage
openstack-barbican-api.noarch: W: no-documentation
4 packages and 0 specfiles checked; 1 errors, 806 warnings.



Requires
--------
python-barbican (rpmlib, GLIBC filtered):
    python(abi)
    python-alembic
    python-babel
    python-crypto
    python-cryptography
    python-eventlet
    python-iso8601
    python-jsonschema
    python-kombu
    python-netaddr
    python-oslo-config
    python-oslo-messaging
    python-paste
    python-paste-deploy
    python-pbr
    python-pecan
    python-six
    python-sqlalchemy
    python-stevedore
    python-webob

openstack-barbican-worker (rpmlib, GLIBC filtered):
    /bin/sh
    /usr/bin/python2
    openstack-barbican-api
    python-barbican

openstack-barbican (rpmlib, GLIBC filtered):
    /bin/sh
    /usr/bin/python2
    config(openstack-barbican)
    python-barbican
    shadow-utils
    systemd
    uwsgi
    uwsgi-plugin-python

openstack-barbican-api (rpmlib, GLIBC filtered):
    /bin/sh
    config(openstack-barbican-api)
    python-barbican



Provides
--------
python-barbican:
    python-barbican

openstack-barbican-worker:
    openstack-barbican-worker

openstack-barbican:
    config(openstack-barbican)
    openstack-barbican

openstack-barbican-api:
    config(openstack-barbican-api)
    openstack-barbican-api



Source checksums
----------------
https://launchpad.net/barbican/juno/2014.2/+download/barbican-2014.2.tar.gz :
  CHECKSUM(SHA256) this package     : 177c6fcb21f927afa50b578b9ebdc4579b7b396b378b123991a41920aae1bfa8
  CHECKSUM(SHA256) upstream package : 177c6fcb21f927afa50b578b9ebdc4579b7b396b378b123991a41920aae1bfa8


Generated by fedora-review 0.5.3 (bcf15e3) last change: 2015-05-04
Command line :/usr/bin/fedora-review -b 1190269 -m fedora-rawhide-x86_64
Buildroot used: fedora-rawhide-x86_64
Active plugins: Python, Generic, Shell-api
Disabled plugins: Java, C/C++, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

Comment 23 Chandan Kumar 2015-09-14 19:32:06 UTC
Hello Greg,

Please proceed with the next step.
[.1] Apply for a new package scm request: https://fedoraproject.org/wiki/Package_SCM_admin_requests#New_Packages

Thanks,

Chandan Kumar

Comment 24 Greg Swift 2015-09-30 21:12:16 UTC
New Package SCM Request
=======================
Package Name: openstack-barbican
Short Description: Openstack Barbican Key Manager
Upstream URL: https://github.com/openstack/barbican
Owners: xaeth alee elmiko 
Branches: f21 f22 f23 epel7
InitialCC:

Comment 25 Alan Pevec 2015-09-30 23:32:32 UTC
You probably do not want f21:
f21=Icehouse
f22=Juno
f23=Kilo

Comment 26 Gwyn Ciesla 2015-10-01 13:02:09 UTC
alee not in packager group.

Comment 27 Greg Swift 2015-10-05 14:56:20 UTC
Apparently alee's fas user is vakwetu.  I can just add him after

Comment 28 Alan Pevec 2015-10-09 09:41:33 UTC
LP 1504291 - Add support for Gunicorn (as reportedly used in production deployments)

uwsgi is a huge with lots of plugins for different languages, pure-python gunicorn is an easier to rebuild in CentOS CloudSIG.

Comment 29 Alan Pevec 2015-10-23 11:59:26 UTC
No movement on the upstream bug, I found gunicorn usage in openstack/akanda-appliance:
* init script https://github.com/openstack/akanda-appliance/blob/master/scripts/etc/init.d/akanda-router-api-server
* template for the gunicorn config file https://github.com/openstack/akanda-appliance/blob/master/ansible/templates/gunicorn.j2
* python service startup code https://github.com/openstack/akanda-appliance/blob/master/akanda/router/api/server.py

Only openstack-barbican-api.service uses uwsgi, keystone-listener and worker services are using oslo_service i.e. eventlet.

Comment 30 Alan Pevec 2015-10-23 12:04:05 UTC
Ignore akanda-router-api example, it's using Flask.
Barbican already has paste ini file so it should as simple as:
gunicorn --paste /etc/barbican/barbican-api-paste.ini ...
http://docs.gunicorn.org/en/19.3/run.html#paste

Comment 31 Alan Pevec 2015-12-05 00:33:19 UTC
Changes to replace uwsgi with gunicorn:
https://github.com/openstack-packages/barbican/commit/343314d29da79cc67d81a3221cc4c8596607962f


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