Merge oscap-daemon-api into openscap-daemon

[mvazquezc]

Some discussion topics:

• Should Flask be a requirement to run oscap-daemon if no rest-api is being used?
• Does it make sense to allow users to disable dbus_daemon and use rest-api only?

My opinion:

• The Flask thing I'd like to know what your thought are. Either option works for me.
• Since rest-api is not as feature complete as dbus_daemon, I'd say no.

[openscap-ci]

Can one of the admins verify this patch?

[pep8speaks]

Hello @mvazquezc! Thanks for updating the PR.

• There are no PEP8 issues in the file openscap_daemon/config.py !

• There are no PEP8 issues in the file openscap_daemon/dbus_daemon.py !

• In the file openscap_daemon/oscap_helpers.py, following are the PEP8 issues :

  Line 101:21: E128 continuation line under-indented for visual indent Line 349:5: E722 do not use bare 'except' Line 559:1: E305 expected 2 blank lines after class or function definition, found 1

• In the file openscap_daemon/rest_api.py, following are the PEP8 issues :

  Line 126:100: E501 line too long (108 > 99 characters) Line 128:100: E501 line too long (146 > 99 characters) Line 199:100: E501 line too long (125 > 99 characters) Line 208:100: E501 line too long (111 > 99 characters) Line 240:100: E501 line too long (109 > 99 characters) Line 257:100: E501 line too long (107 > 99 characters) Line 302:100: E501 line too long (113 > 99 characters) Line 307:100: E501 line too long (103 > 99 characters) Line 313:100: E501 line too long (101 > 99 characters) Line 352:100: E501 line too long (140 > 99 characters) Line 353:100: E501 line too long (153 > 99 characters)

Comment last updated on November 06, 2018 at 11:53 Hours UTC

[mpreisler]

@openscap-ci add to whitelist

[jan-cerny]

@mvazquezc

IMO Flask should be an optional dependency and the Daemon should be able to start without that, because I can imagine there can be people who do not plan to use REST API, and they would like to install Daemon without dependencies they will not necesarily need.

Regarding the second question, I would personally add 2 CLI or config file options:

1. to enable or disable REST API
2. to enable or disable DBus API

But I have no idea if that's possible or how difficult would it be to implement.

[mvazquezc]

@mvazquezc

IMO Flask should be an optional dependency and the Daemon should be able to start without that, because I can imagine there can be people who do not plan to use REST API, and they would like to install Daemon without dependencies they will not necesarily need.

Regarding the second question, I would personally add 2 CLI or config file options:

1. to enable or disable REST API
2. to enable or disable DBus API

But I have no idea if that's possible or how difficult would it be to implement.

I don't know the implications of starting oscapd without dbus daemon enabled, as a first move, I will implement the logic to use the flask dependency only if rest api is enabled in config.

[jan-cerny]

I don't know the implications of starting oscapd without dbus daemon enabled, as a first move, I will implement the logic to use the flask dependency only if rest api is enabled in config.

Great! Thanks. That will be fine.

[mpreisler]

Hi @mvazquezc , this code looks very good. I added a few comments with suggestions.

In the longer term future it would be very nice to refactor the code to instantiate the system, then instantiate the dbus and REST "interfaces" if it's enabled in the config file. These interfaces would reference the system and would be instantiated if and only if all dependencies are met and it's enabled in the config file.

[mvazquezc]

@mpreisler In this PR I would like to handle only things related to the rest api implementation. Would you mind if we open an issue in order to handle the dbus daemon changes?

[mpreisler]

@mpreisler In this PR I would like to handle only things related to the rest api implementation. Would you mind if we open an issue in order to handle the dbus daemon changes?

That works for me.

[mvazquezc]

@mpreisler I've added your suggestions. Please, review the changes and let me know if further changes are needed.

[mpreisler]

@matejak @jan-cerny Could you please take over the review here?

[jan-cerny]

@mpreisler Yes, I can.

[jan-cerny]

@mvazquezc Apart from the comment regarding ssgs endpoint it looks good to me.

[jan-cerny]

I have checked the code once again, I have tried to run the daemon on my F29 VM and contacting the endpoints. Everything works OK. @mvazquezc Thank you very much. Great job!