get-sp-metadata action hardcodes sp-metadata file location without relying on the application name

Bug #1833202 reported by Dmitrii Shcherbakov
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Keystone SAML Mellon Charm
Fix Released
Undecided
Dmitrii Shcherbakov

Bug Description

get-sp-metadata action hardcodes the location of an sp metadata file which is incorrect - it should use the application name to allow custom application names and usage of multiple keystone-saml-mellon applications (to have multiple IdPs).

actions/actions.py:
SP_METADATA_FILE = "/etc/apache2/mellon/sp-meta.keystone-saml-mellon.xml"

src/lib/charm/openstack/keystone_saml_mellon.py:
               os.path.join('/etc/apache2/mellon',
                            f.format(hookenv.service_name())) for f in [
                                'idp-meta.{}.xml',
                                'sp-meta.{}.xml',
                                'sp-pk.{}.pem',

Tags: cpe-onsite
tags: added: cpe-onsite
Revision history for this message
Ryan Beisner (1chb1n) wrote :

Please provide more detail and context as to the use case, back-end, and sanitized reproducer bundle/info. Also, what version of OpenStack?

Changed in charm-keystone-saml-mellon:
status: New → Incomplete
Revision history for this message
Ryan Beisner (1chb1n) wrote :

Please also provide a sample/example xml data file for the use cases affected by this bug/feature request. Thank you.

Revision history for this message
Dmitrii Shcherbakov (dmitriis) wrote :

Ryan, it's a one-liner as no charm should ever hard-code an application name because it is inherently variable and comes from a hook environment.

It does not depend on a cloud version or any resources

https://review.opendev.org/#/c/665946/2/src/actions/actions.py

I have already created a review and manually tested the fix. There is a unit test modification which I have not yet gotten to.

Changed in charm-keystone-saml-mellon:
status: Incomplete → New
Revision history for this message
Dmitrii Shcherbakov (dmitriis) wrote :

An updated review with unit test modifications: https://review.opendev.org/#/c/665946/3

Changed in charm-keystone-saml-mellon:
status: New → In Progress
assignee: nobody → Dmitrii Shcherbakov (dmitriis)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to charm-keystone-saml-mellon (master)

Reviewed: https://review.opendev.org/665946
Committed: https://git.openstack.org/cgit/openstack/charm-keystone-saml-mellon/commit/?id=8a4b2cb7ccc311070e518201f0b762d8113eb328
Submitter: Zuul
Branch: master

commit 8a4b2cb7ccc311070e518201f0b762d8113eb328
Author: Dmitrii Shcherbakov <email address hidden>
Date: Tue Jun 18 13:07:45 2019 +0300

    Fix get-sp-metadata sp-metadata file location

    get-sp-metadata action hardcodes the location of an sp metadata file
    which is incorrect - it should use the application name to allow custom
    application names and usage of multiple keystone-saml-mellon
    applications (to have multiple IdPs).

    In order to make unit testing easier the global variable for the sp
    metadata file in actions.py is moved into a function so that it is
    evaluated at the function call time, not module import time.

    Change-Id: Ia73cd633f2948080e9a892869074ed2775ecc33c
    Closes-Bug: #1833202

Changed in charm-keystone-saml-mellon:
status: In Progress → Fix Committed
James Page (james-page)
Changed in charm-keystone-saml-mellon:
milestone: none → 19.07
David Ames (thedac)
Changed in charm-keystone-saml-mellon:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.