certificate install from remote API fails with exception

[Allain Legacy]

Brief Description
-----------------
Using the API to install a certificate results in an exception in the sysinv-api process and a 500 response code to the user. The certificate install handler assumes that the incoming request will contain a "certificate_file" attribute which points to the absolute path of the certificate file being installed. This is in addition to the actual file contents that are included in the multipart message. When this "certificate_file" attribute is not provided the API throws an exception.

This code block assumes that the certificate_file is always present, but it is not required so the code should be changed to first check for None.

        if os.path.isabs(certificate_file):
            if not os.path.isfile(certificate_file):
                msg = "'certificate_file' is not a valid file path"
                LOG.info(msg)
                return dict(success="", error=msg)

The "certificate_file" attribute is only valid/meaningful when the cgts-client installs the certificate from a local filesystem. Under this usecase scenario, the file path is passed to the API and the API deletes the file for security reasons once the command completes. If the cgts-client is run remotely then this no longer makes sense. This should be refactored so that the certificate_file is no longer passed to the API and no longer expected by the API. If the file needs to be deleted then it should be deleted by the client; not by the API.

Severity
--------
Critical

Steps to Reproduce
------------------
This can be tested with a custom API client or complex curl commands but the easiest way is to simply manually modify the cgts-client code to not pass the certificate_file attribute.

Remove 'certificate_file' from this location at: cgtsclient/v1/certificate_shell.py:86

    data = {'passphrase': args.passphrase,
            'mode': args.mode,
            'certificate_file': os.path.abspath(args.certificate_file)}

Expected Behavior
------------------
No exceptions are expected.

Actual Behavior
----------------
This exception is thrown:

2019-04-27 07:04:03.817 5783 ERROR /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc [-] 'NoneType' object has no attribute 'startswith'
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc Traceback (most recent call last):
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc File "/usr/lib/python2.7/site-packages/pecan/middleware/debug.py", line 78, in __call__
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc return self.app(environ, start_response)
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc File "/usr/lib/python2.7/site-packages/pecan/middleware/recursive.py", line 56, in __call__
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc return self.application(environ, start_response)
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc File "/usr/lib64/python2.7/site-packages/sysinv/api/middleware/parsable_error.py", line 70, in __call__
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc app_iter = self.app(environ, replacement_start_response)
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc File "/usr/lib/python2.7/site-packages/pecan/core.py", line 835, in __call__
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc return super(Pecan, self).__call__(environ, start_response)
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc File "/usr/lib/python2.7/site-packages/pecan/core.py", line 678, in __call__
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc self.invoke_controller(controller, args, kwargs, state)
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc File "/usr/lib/python2.7/site-packages/pecan/core.py", line 569, in invoke_controller
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc result = controller(*args, **kwargs)
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc File "/usr/lib/python2.7/site-packages/oslo_concurrency/lockutils.py", line 274, in inner
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc return f(*args, **kwargs)
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc File "/usr/lib64/python2.7/site-packages/sysinv/api/controllers/v1/certificate.py", line 284, in certificate_install
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc if os.path.isabs(certificate_file):
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc File "/usr/lib64/python2.7/posixpath.py", line 61, in isabs
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc return s.startswith('/')
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc AttributeError: 'NoneType' object has no attribute 'startswith'
2019-04-27 07:04:03.817 5783 TRACE /usr/lib/python2.7/site-packages/pecan/middleware/debug.pyc

Reproducibility
---------------
100%

System Configuration
--------------------
Any

Branch/Pull Time/Commit
-----------------------
2019-04-24

Last Pass
---------
Unknown

Timestamp/Logs
--------------
See above

Test Activity
-------------
Developer testing

[Frank Miller (sensfan22)]

Marking stx.2.0 release gating as https functionality is not working via APIs.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/664607
Committed: https://git.openstack.org/cgit/starlingx/config/commit/?id=f68e183cb0e47b0dea449ad9474314f80e9c26e4
Submitter: Zuul
Branch: master

commit f68e183cb0e47b0dea449ad9474314f80e9c26e4
Author: Allain Legacy <email address hidden>
Date: Tue Jun 11 07:51:48 2019 -0500

    sysinv: avoid exception when checking for certificate file presence

    The "certificate_file" attribute is optional but the API controller for
    certificates assumes that it is present and uses it to check the
    validity of the absolute path expected. This leads to an exception when
    installing a certificate from a remote location without specifying a
    local absolute path.

    As per the comments in the bug report this is only a partial fix. The
    "certificate_file" attribute should be deprecated and the ability to
    check and delete the file when done should be moved into the client and
    not be included as part of the tasks done by the API (server-side).

    Change-Id: I9ed5082c4118014b11062539410bacea9a28577d
    Partial-Bug: #1827208
    Signed-off-by: Allain Legacy <email address hidden>

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/666431

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/666431
Committed: https://git.openstack.org/cgit/starlingx/config/commit/?id=7bb0b606bb1de7f97d146555b811252f60decc0d
Submitter: Zuul
Branch: master

commit 7bb0b606bb1de7f97d146555b811252f60decc0d
Author: Teresa Ho <email address hidden>
Date: Fri Jun 14 10:42:55 2019 -0400

    Fix certificate install exception

    The removal of the certificate file should be done in the client
    instead of API. The certificate_file attribute is removed from the
    certificate_install API. The certificate_install CLI removes the
    certificate file when the installation is complete.

    Closes-Bug: 1827208

    Change-Id: Ie23186f6515e3bc250e22aa1bd1d3076928ad0fe
    Signed-off-by: Teresa Ho <email address hidden>

[Ghada Khalil (gkhalil)]

LP was fixed a long time ago; removing the stx.retestneeded tag