Nova and Glance contain a near-identical signature_utils module

[Mark McLoughlin]

It appears that https://review.openstack.org/256069 took the signature_utils modules from Glance and modified it in fairly superficial ways based on review feedback:

  $ diff -u nova/nova/signature_utils.py glance/glance/common/signature_utils.py | diffstat
  signature_utils.py | 182 ++++++++++++++++++++++++-----------------------------
  1 file changed, 83 insertions(+), 99 deletions(-)

The Oslo project was created to avoid this sort of short-sighted cut-and-pasting. This code should really be in a python library that both Glance and Nova could use directly.

Perhaps the code could be moved to a new library in the Glance project, or a new library in the Oslo project, or into the cryptography library itself?

[Louis Taylor (kragniz)]

oslo.signatures, or perhaps just in oslo.utils?

[Mark McLoughlin (markmc)]

oslo.utils might be ok, but making crytography a dependency for all oslo.utils users is probably undesirable

[Davanum Srinivas (DIMS) (dims-v)]

glance-store?

[Daniel Berrange (berrange)]

FYI, the code in Nova may look the same right now, but that's only because it was not following nova best practice. That method needs converting to use Nova's nova.objects.ImageMeta model instead of using anonymous dicts, since Nova's intends to 100% eliminate passing & use of dicts for image properties in favour of an explicit model. So if there is a desire to share the code, it wouldl have to be re-factored so that all the neccessary signature relatsed parameters are explicitly passed in, rather than passing in the image meta dict directly.

[Dane Fichter (dane-fichter)]

@Daniel Berrange: If we are moving to an explicit model for image properties, then perhaps the best way to meet that requirement and make this code not specific to Nova is to directly pass all the required image properties to the signature_utils get_verifier method.

I should also add that the Glance and Nova implementations will continue to diverge during the Mitaka cycle, since the Glance implementation has to deprecate an old approach, and the Nova spec will require us to add certificate validation functionality to the Nova implementation.

As compromise to all of the above consideration and with the goal of eliminating duplicate code I propose the following path forward:

1. Amend Nova implementation to take in image properties as individual parameters, not bundled in a dict or ImageMeta object.
2. Remove other Nova-specific aspects of the signature_utils class, but continue implementing the spec and keep the code in Nova for Mitaka.
3. At the beginning of the N development cycle, after Glance no longer has to give deprecation warnings for the outdated approach, merge the signature_utils module into oslo as oslo.signature_utils and remove the duplicate code from Nova and Glance.

Thoughts?

[Daniel Berrange (berrange)]

That sounds like a reasonable plan

[Dane Fichter (dane-fichter)]

Alright, I'll move forward with this unless there are objections.

[Andrew Laski (alaski)]

Dane if you are still working on this please assign it back to yourself. But there's been no progress for over 4 months so I'm setting this so others feel free to pick it up.

[Dane Fichter (dane-fichter)]

Done. Thanks for reminding me. Resolving this is one of my goals for this cycle.

[John Garbutt (johngarbutt)]

We did make creating this library a requirement for the feature. Although we didn't want to block on its creation. Seems we probably should have.

[Dane Fichter (dane-fichter)]

Hi John, we've created the cursive library and will definitely be able to do a release before Nova's code freeze. However, we'd like to add certificate validation (with the approval of Nova core reviewers) before we remove the signature_utils code from Nova. Could you please take a look at:

https://review.openstack.org/#/c/331823/

[Matt Riedemann (mriedem)]

There was no spec proposed in Newton for https://review.openstack.org/#/c/331823/ so that's going to have to wait for Ocata.

[Daniel Berrange (berrange)]

This isn't a bug - its a feature request to switch to using a new library from Nova. I'm fine with that as a suggestion, but it should be file as a blueprint - probably can be a specless blueprint.

[Matt Riedemann (mriedem)]

This is not really a bug. Create a specless blueprint for Ocata to integrate the library into Nova in that release.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/351232

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/352391

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/352391
Committed: https://git.openstack.org/cgit/openstack/glance/commit/?id=5afb5d33ed7714aa2684ee9209d05f30655e2c41
Submitter: Jenkins
Branch: master

commit 5afb5d33ed7714aa2684ee9209d05f30655e2c41
Author: Dane Fichter <email address hidden>
Date: Tue Aug 2 23:13:46 2016 -0400

    Use cursive for signature verification

    This change removes the signature_utils module
    from Glance and uses the cursive library, which
    contains an identical module.

    Change-Id: I80fcafa528b87a83b90ed7c0e4c0db9228852bc2
    Depends-On: Ic3ffb6b318dc2ac6c9d3a60bed5198fd4d40e318
    Partial-Bug: #1528349

[OpenStack Infra (hudson-openstack)]

Change abandoned by Sean Dague (<email address hidden>) on branch: master
Review: https://review.openstack.org/351232
Reason: This review is > 6 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/351232
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=d17e701ddbbb6ebcc2a27c4d27053d6e1f87989d
Submitter: Jenkins
Branch: master

commit d17e701ddbbb6ebcc2a27c4d27053d6e1f87989d
Author: dane-fichter <email address hidden>
Date: Tue Mar 14 09:23:44 2017 -0700

    Use cursive for signature verification

    This change removes the signature_utils module
    from Nova and uses the cursive library, which
    contains an identical module.

    Change-Id: I8179282a9d19f829aca0b5bd2775d855b3364c86
    Depends-On: I7e5797661fee258bc0270b5f109704b591633519
    Implements: blueprint signature-code-cleanup
    Partial-Bug: #1528349

[Brian Rosmaita (brian-rosmaita)]

This was fixed in glance by Change-Id: I80fcafa528b87a83b90ed7c0e4c0db9228852bc2