Comment 1 for bug 1273790

Matt Bruzek (mbruzek) wrote :

Greetings Saurabh,

Thank you very much for the submission of the wildfly charm! I am excited to have another Java charm in the charm store!

This is a +1 review from me, and someone else from ~charmers should be along to complete the review and promulgate this charm if there are no other issues. We appreciate your submission and look forward to the next round of review!

Charm Proof:

I saw no errors, warnings or information when running “juju charm proof”. That means your charm is high quality! Thank you for taking the time to address all the issues in charm proof before submitting.

README:

The README is in text file which is fine, but many charms use markdown format (README.md) because it allows more formatting options. Please consider releasing a markdown formatted version as it will look nicer in the charm store. If you have charm tools installed we have a command that saves you time by generating a README in markdown format. From the wildfly/ directory run the command: juju charm add readme and a README.ex template file is created with the charm store format.

The “Usage” section should include: juju expose wildfly command. The web console will not be accessible from juju otherwise.

Review comments:

I deployed this charm on local provider and hpcloud (OpenStack environment). I was not able to access the web console (after making wildfly exposed) on hpcloud. I saw no errors in “juju status” so I looked into this a little further. I was able to open the web console on the locally deployed wildfly but the hpcloud instance was still not visible using the http://<ip-address>:9990. I believe this is because you do not open the port through juju (open-port http-port) in your hooks. The local provider does not have a firewall but the other providers do.

According to the Charm Store Policy (https://juju.ubuntu.com/docs/authors-charm-policy.html) we should not use default passwords for charms. I would suggest making the administrator's password configurable by an option in config.yaml. Then users could set their own passwords with: juju set wildfly admin_password=”password” after deployment.

The start hook could be run multiple times, so it should be idempotent. I see the start hook is creating the admin user, and not sure what would happen if the admin user logs in to the web console and changes their password. If the start hook were to run again (for whatever reason). Would the admin user's password be reset in this case? One way to help with idempotency is to check if something (user) exists before creating it.

The install hook uses wget to download the wildfly tar file. Since you are not using apt-get the Charm Store Policy strongly suggests that you should verify the file contents cryptographically before installing it. I would recommend using sha1sum compared to a known value to prevent tampering or an incomplete download.

I moved this bug to incomplete as a result of the review. Once you have addressed the issues with code changes or comments and wish to have another review move the bug back to “New” or “Fix committed” to have it added back to the queue.

Again thanks for your work on this charm!