New drupal charm submission.

Greetings Darryl,

Thank you for your submission of the Drupal Charm! I've taken some time to review your work and have the following comments:

#Proof

There was no output from charm proof! Excellent! moving on to the next phase.

# Metadata

In metadata.yaml, you provide a default administrative password. While this is fine for personal charms, its store policy that passwords not be set to a default value, that persist in the deployment, as this opens an attack vector on services deployed with Juju. We recommend that you set the value to an empty string or None, and handle the action accordingly in the charm (in other use-case examples, the application refuses to deploy without a password, and returns 0)

hooks/install appears to have some idempotency issues. On a fresh deployment, the hook errored, and when attempting to resolve by attaching to debug-hooks and re-running juju resolved -r drupal/0 - i was prompted to overwrite a directory that was already created:

Install location /var/www/drupal already exists. Do you want to overwrite it? (y/n):

which then further rolled into Apache returning Drupal does not exist

Site default already disabled
ERROR: Site drupal does not exist!

Using Drush as a delivery mechanism is a great addition, and using it to power most of the site maintenance i see is an upstream accepted tool - however for inclusion to the charm store, all downloads need to be cryptographically verified. A simple fix for this would be to use Drush's git based checkout system instead of using the latest HEAD package that Drush fetches by default.

This is an excellent first round submission, and barring some modifications will be ready for further review for the charm store. Thank you for putting in the excellent work that has gone into this charm, and I look forward to the next iteration of the Drupal charm.

I'm going to place the status of this bug as incomplete. When you are ready for an additional review, please set the bug status to "Fix Comitted" or "new" and someone will be along shortly to review your work.

All the best!

I have implemented all the suggested changes.

Hi Darryl,

Thanks for your submission of this new charm! I am excited to have a Drupal charm in Juju.

I ran charm proof and saw an Error in the report. Here is the error I saw:

E: config.yaml: type of option password is specified as string, but the type of the default value is NoneType

It looks like you were responding to Chuck's previous review by not providing a default password. This looks like a YAML syntax error, the proper way to default to an empty string is default: "". It looks like your config-changed hook will properly handle the password values of an empty string.

I was able to deploy the drupal charm without any errors in Juju.

Not knowing much about Drupal before this review I was relying on the readme to give me information about the charm. The readme was in markdown format (that is great!) but looked a little sparse. I would highly suggest adding more information to that file. We have a tool to help you with that, run: juju charm create readme and a README.ex file will be created in that directory. That gives a template of what full readme should look like. Specifically include links for Drupal support, mailing lists (https://drupal.org/mailing-lists), bug tracker, and a link to Drupal documentation. Not all charms are available on port 80 of the IP address, so it would be a good idea to include some words about how to access Drupal after the deploy is complete. Being new to Drupal I didn't know the proper URL or the right username to log in with and they were not in the readme file.

Other than those things to add to the readme I believe this charm is outstanding! This drupal charm is an excellent example of writing clean hooks! The hooks are very easy to read and very few lines of code. I am very impressed.

Based on the charm proof error I am going to place the status of this bug as incomplete. When you are ready for an additional review, please set the bug status to "Fix Comitted" or "new" and it will get added back to the review queue.

Thanks again for your submission!