Comment 4 for bug 1510216

Revision history for this message
Kevin W Monroe (kwmonroe) wrote :

Hi Prabakaran / Rajith. Thanks for the submission and request to make IBM Platform RTM a recommended application in the charm store. Unfortunately, we cannot recommend this charm in its current state. Outstanding issues:

- config.yaml: values are never used. Since this charm uses terms and resources, these config options are no longer needed. If RTM has no other configuration available, remove the config.yaml.

- layer.yaml: again since this charm uses terms and resources, there is no need to use the ibm base layer here (the primary purpose of the ibm base layer was to offer license acceptance and curl helpers for charms that could not leverage terms and resources). I recommend removing 'layer:ibm-base' from layer.yaml.

- README.md: the process for deploying mariadb-5.5 has changed recently. I committed an update to the readme with current instructions and a few formatting changes:

http://bazaar.launchpad.net/~ibmcharmers/charms/trusty/layer-ibm-platform-rtm/trunk/revision/2#README.md

- tests/01-deploy.py: lots of simple lint errors here need to be fixed (incorrect whitespace, unused imports, etc). Run 'flake8 ./tests/*.py to see the errors. More importantly, mariadb is never added to the deployment. You'll need to include something like this to deploy mariadb-5.5 since it is required by this charm:

self.d.add('mariadb')
self.d.configure('mariadb', {'series': '5.5'})

- reactive/ibm-platform-rtm.sh
  - This code is very difficult to follow because of inconsistent formatting. Please be consistent with function open/closing braces (ie: '{' on the function line or by itself) as well as indentation throughout the code (ie: indent the body of a function consistently).

  - I could not determine a valid reason for disabling the firewall. Instead, you could call 'open-port 80' and instruct the user run 'juju expose ibm-platform-rtm' in the readme. Security guidelines in the charm author policy say the following (note: disabling the firewall is not a best practice):

https://jujucharms.com/docs/stable/authors-charm-policy
...
Must provide a means to protect users from known security vulnerabilities in a way consistent with best practices
...

  - The charm does not need to apt install mariadb-client-core because the mariadb charm will handle that for you.

  - The 'check_mariadb_inst()' function must not 'exit 0'. This will cause the running hook to always succeed which means this charm would never handle the case where mariadb was unavailable.

  - In 'ibm_prtm_INSTALL()', the charm should not copy resources to a new location, and it should not have the resource tarball name hard coded. The charm could instead do something like this:

cd /var/lib/juju/agents/*rtm*/resources
res_name=`resource-get 'ibm_prtm_installer'
tar -xvf $res_name

  - The charm must not 'wget' files from the internet without some verification of the payload integrity. I recommend defining resources for each of the plugins that are currently fetched with wget and push those to the charm store to live alongside this charm. The charm would then use 'resource-get' for each plugin just as it does for the main installers. FYI, the relevant guideline from the charm author policy says:

https://jujucharms.com/docs/stable/authors-charm-policy
...
Must verify that any software installed or utilized is verified as coming from the intended source.
...

Please let me know if you have any questions/concerns with my findings. I'll move this into 'In Progress' for now. Thanks!