Charm needed: chive mysql admin panel

[Jorge Castro]

http://www.chive-project.com/

Chive looks way cool, looks pretty straightforward to install

[Marco Ceppi (marcoceppi)]

Reviewing this now

[Marco Ceppi (marcoceppi)]

Hi Stefan! Thanks for your work on this charm. It's looking great so far, just a few blockers that will need to be addressed before this can be considered ready for the charm store:

# Blockers

## Install hook is not "safe"

Currently you wget over http and extract the tar - which is what the upstream recommends. The problem with this is there is no cryptographic verification and the communication isn't encrypted. The requirement is that either or both payload verification or encrypted and validated communication must take place when pulling remove sources[1]. Their website doesn't have a valid SSL certificate, but it looks like they mirror their releases on Launchpad[2] which has a valid SSL cert (and MD5 hashes). Seeing as their release schedule is about one release every 3 months it might be worth simply using Launchpad and the MD5 hashes with the "ch_get_file" charm helper tool to get and verify the payload directly from Launchpad. If you need some help with the last part feel free to join us on freenode in the #irc room as the documentation on these helpers are pretty light.

## Empty MySQL Relation

Currently you define that the charm requires the db: mysql relation, and you have the hook for it. But the hook is empty and instead the service installs MySQL on the unit the charm is deployed to. Ideally, for this to work you'd want to do something similar to the phpMyAdmin charm[3] which uses the db-admin relation to get a user with elevated privileges to the deployed MySQL service.

## No ports get opened

In order for Apache to be accessible outside of the juju deployment, you'll need to make a call to open-port to let Juju know which ports it should expose once the user runs `juju expose`[4]. A good example of this can be seen in the "Writing a charm" section of the Juju docs[5].

# Small stuff, nitpicks

These are more or less "minor" things to consider

## Stop hook is empty

Ideally charms should have _something_ in their start and stop hooks (typically just turning on and off Apache

## "all charms should provide at least one thing"

When running charm proof, this warning comes up:

> W: all charms should provide at least one thing

In this case you can make an argument that it provides a website on the http interface, much like other charms of this time.

Overall this is a great start to an awesome charm that will help bring more value to Juju. Chive definitely looks cool and I can't wait to see what the next round of reviews bring! When you're ready again for a review move the status to Fix Committed to add your charm back to the review queue.

[1]: https://juju.ubuntu.com/docs/policy.html
[2]: https://launchpad.net/chive/+download
[3]: http://jujucharms.com/charms/precise/phpmyadmin/hooks/db-admin-relation-changed
[4]: https://juju.ubuntu.com/docs/expose-services.html
[5]: https://juju.ubuntu.com/docs/write-charm.html#write-hooks

[Marco Ceppi (marcoceppi)]

Hi Stefan,

I just wanted to clarify a point I made previously:

>The requirement is that either or both payload verification or encrypted and validated communication must take place when pulling remove sources[1]. Their website doesn't have a valid SSL certificate, but it looks like they mirror their releases on Launchpad[2] which has a valid SSL cert (and MD5 hashes).

With regards to this, grabbing sources from non-encrypted sources is fine so long as you have a valid checksum you can verify against the file the charm grabs. The https only works if make sure the certificate is valid. In doing so you guarentee the transmission of the file is coming from the intended source. Using https communication with a valid and verified SSL certificate can be used in lieu of having to do checksum matches against the file.

I kind of ran those idea together above, wanted to make sure I conveyed exactly what is needed to satisfy the 4th bullet of https://juju.ubuntu.com/docs/policy.html

[shartmann (stefan-hartmann)]

Hey,

thx for the review! I will upload the new version as fast as i can ;)

Stefan

On Mon, Oct 1, 2012 at 11:00 PM, Marco Ceppi <email address hidden> wrote:

> Hi Stefan,
>
> I just wanted to clarify a point I made previously:
>
> >The requirement is that either or both payload verification or
> encrypted and validated communication must take place when pulling
> remove sources[1]. Their website doesn't have a valid SSL certificate,
> but it looks like they mirror their releases on Launchpad[2] which has a
> valid SSL cert (and MD5 hashes).
>
> With regards to this, grabbing sources from non-encrypted sources is
> fine so long as you have a valid checksum you can verify against the
> file the charm grabs. The https only works if make sure the certificate
> is valid. In doing so you guarentee the transmission of the file is
> coming from the intended source. Using https communication with a valid
> and verified SSL certificate can be used in lieu of having to do
> checksum matches against the file.
>
> I kind of ran those idea together above, wanted to make sure I conveyed
> exactly what is needed to satisfy the 4th bullet of
> https://juju.ubuntu.com/docs/policy.html
>
> --
> You received this bug notification because you are a bug assignee.
> https://bugs.launchpad.net/bugs/1047480
>
> Title:
> Charm needed: chive mysql admin panel
>
> Status in Juju Charms:
> Incomplete
>
> Bug description:
> http://www.chive-project.com/
>
> Chive looks way cool, looks pretty straightforward to install
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/charms/+bug/1047480/+subscriptions
>

[shartmann (stefan-hartmann)]

Just started with fixing the charm, one question:

You suggested to load chive with "ch_get_file" and the md5-hash from
launchpad.

Does this mean that I have to update the chive-charm with each new
chive-release because the url to the package and the md5 changes?

Is there a way to always get the latest version from the same url?
Example:
load md5 from "h <https://launchpad.net/chive/+download>ttps://
lauchpad.net/chive/latest/md5"
ch_get_file from "https://launchpad.net/chive/latest" and check it with the
md5
tar gz and so on

On Tue, Oct 2, 2012 at 7:44 AM, Stefan Hartmann <<email address hidden>
> wrote:

> Hey,
>
> thx for the review! I will upload the new version as fast as i can ;)
>
> Stefan
>
>
> On Mon, Oct 1, 2012 at 11:00 PM, Marco Ceppi <email address hidden> wrote:
>
>> Hi Stefan,
>>
>> I just wanted to clarify a point I made previously:
>>
>> >The requirement is that either or both payload verification or
>> encrypted and validated communication must take place when pulling
>> remove sources[1]. Their website doesn't have a valid SSL certificate,
>> but it looks like they mirror their releases on Launchpad[2] which has a
>> valid SSL cert (and MD5 hashes).
>>
>> With regards to this, grabbing sources from non-encrypted sources is
>> fine so long as you have a valid checksum you can verify against the
>> file the charm grabs. The https only works if make sure the certificate
>> is valid. In doing so you guarentee the transmission of the file is
>> coming from the intended source. Using https communication with a valid
>> and verified SSL certificate can be used in lieu of having to do
>> checksum matches against the file.
>>
>> I kind of ran those idea together above, wanted to make sure I conveyed
>> exactly what is needed to satisfy the 4th bullet of
>> https://juju.ubuntu.com/docs/policy.html
>>
>> --
>> You received this bug notification because you are a bug assignee.
>> https://bugs.launchpad.net/bugs/1047480
>>
>> Title:
>> Charm needed: chive mysql admin panel
>>
>> Status in Juju Charms:
>> Incomplete
>>
>> Bug description:
>> http://www.chive-project.com/
>>
>> Chive looks way cool, looks pretty straightforward to install
>>
>> To manage notifications about this bug go to:
>> https://bugs.launchpad.net/charms/+bug/1047480/+subscriptions
>>
>
>

[shartmann (stefan-hartmann)]

Just started with fixing the charm, one question:

You suggested to load chive with "ch_get_file" and the md5-hash from launchpad.

Does this mean that I have to update the chive-charm with each new chive-release because the url to the package and the md5 changes?

Is there a way to always get the latest version from the same url?
Example:
load md5 from "https://lauchpad.net/chive/latest/md5"
ch_get_file from "https://launchpad.net/chive/latest" and check it with the md5
tar gz and so on

[Jorge Castro (jorge)]

There doesn't seem to be a way to do this in Launchpad as far as I can tell, I'll ask around.

I looked to see if they had a PPA which would make it easier but all I found was an old version.

[Marco Ceppi (marcoceppi)]

Launchpad really doesn't give you a way to pull the "latest" download or md5 hash. There's nothing wrong with using a "hard-coded" version of the release. It means the charm will always work even if upstream changes. Given they release about every 3-4 months it wouldn't be _too_ tedious to update every release, or not. For now, until a more clear release pattern emerges (and development settles) I would just hard-code "17213009a326a730a9de40b70ce93978" for the md5 hash of the tar.gz download from launchpad (https://launchpad.net/chive/1.1/1.1/+download/chive_1.1.tar.gz) So it would look something like this:

chive_file=`ch_get_file "https://launchpad.net/chive/1.1/1.1/+download/chive_1.1.tar.gz" "17213009a326a730a9de40b70ce93978"`

I would be great if they updated their PPA, you could consider contacting the author and asking them to update it.

[Clint Byrum (clint-fewbar)]

You don't need to get the MD5 if you are fetching from launchpad via https. The https itself is verification that you are talking to launchpad, so the MD5 is redundant.

You can use wget's more advanced features to grab files:

wget --timestamping --no-parent --no-directories -A '*.php' -Pdownloads -r https://launchpad.net/chive/+download

This results in a dir called 'downloads' files in it, robots.txt berand chive_1.1.phar_beta.php. You could then get the 'latest' with:

find downloads -name '*.php' | sort -r | head -1

Hang on though, lets take a step back...

I'd actually recommend that you just pick an https download URL, put it in a config.yaml option, and let users change that url in config.yaml if it doesn't work for them. This also means that yandou'd need to do all of this download in hooks/config-changed, not install. Anyway, that would still be very automatic, but it will give your users flexibility if they need it. That approach also means that you can test new versions and only bump the default url when a new version has tested good.

[shartmann (stefan-hartmann)]

Bugfixing-Summary:

# Blockers

## Install hook is not "safe"

Added config-yaml with a download url, wget from launchpad over https

## Empty MySQL Relation

removed mysql-relation

## No ports get opened

opening port 80 during installation

# Small stuff, nitpicks

## Stop hook is empty

turning off apache in the hook now

## "all charms should provide at least one thing"

provides a website on the http interface now. I'm getting a "relation website has no hook" - information on charm proof, is that ok?

[Marco Ceppi (marcoceppi)]

Hi Stefan!

Just going over your latest changes, thanks for taking the time to review and implement fixes so far! For the warning in charm proof about "relation website has no hook" you'll want to create a hook for `website-relation-joined` with something simple like: http://jujucharms.com/charms/precise/mediawiki/hooks/website-relation-joined that way the information about the unit can be passed along to services which consume the website relation.

The two major things remaining, from what I can see, is the MySQL relation and configuration changes.

I apologize for not making this more clear previously when I was referring to the MySQL hook being empty. What I meant by that statement is MySQL shouldn't be installed at all on this machine. Instead the service should be related to the MySQL charm. So I meant for the reverse to happen: remove the mysql-server line from the install hook (which doesn't seem to be there anymore) and instead flesh out the db-admin:mysql-root relation. The end goal would be that chive could be used interchangeably with the phpMyAdmin charm as having chive run on an empty server defeats the point ;) The end result would be something like this:

    juju deploy mysql
    juju deploy chive
    juju add-relation chive mysql

As for the config hook, this is a slightly smaller blocker, but currently if someone changes the "download-url" config option, nothing will happen to the node. Ideally configuration changes should be respected in the service. In this charm it would probably be as simple as removing the chive directory and fetching the new download URL thereby allowing an "in-place" upgrade without needing to upgrade the actual charm.

Otherwise great work so far, there are a few other nitpick stuff but they're rather minor in comparison to the above and mostly consist of semantics.

The only one thing I could find to complain about is the order of the open-port. Open-port should technically be run only when the service is ready to expose itself. So after chive is downloaded and installed. Sliding it down a few lines would satisfy this :)

If you've got any other questions feel free to reply or join the #juju room on freenode, great work so far!

[Clint Byrum (clint-fewbar)]

Excerpts from Marco Ceppi's message of 2012-10-25 14:25:41 UTC:
> Hi Stefan!
>
> Just going over your latest changes, thanks for taking the time to
> review and implement fixes so far! For the warning in charm proof about
> "relation website has no hook" you'll want to create a hook for
> `website-relation-joined` with something simple like:
> http://jujucharms.com/charms/precise/mediawiki/hooks/website-relation-
> joined that way the information about the unit can be passed along to
> services which consume the website relation.
>
> The two major things remaining, from what I can see, is the MySQL
> relation and configuration changes.
>
> I apologize for not making this more clear previously when I was
> referring to the MySQL hook being empty. What I meant by that statement
> is MySQL shouldn't be installed at all on this machine. Instead the
> service should be related to the MySQL charm. So I meant for the reverse
> to happen: remove the mysql-server line from the install hook (which
> doesn't seem to be there anymore) and instead flesh out the db-admin
> :mysql-root relation. The end goal would be that chive could be used
> interchangeably with the phpMyAdmin charm as having chive run on an
> empty server defeats the point ;) The end result would be something like
> this:
>
> juju deploy mysql
> juju deploy chive
> juju add-relation chive mysql
>

Note that you can run it on one box still with jitsu deploy-to. So

juju bootstrap
jitsu deploy-to 0 mysql
jitsu deploy-to 0 chive
juju add-relation mysql chive

This use case will most likely be supported directly in juju-core, so
I feel confident that we can start recommending use of jitsu's version
of it.

[Jorge Castro (jorge)]

Hi Stefan, this bug fell out from under my radar, should we go ahead and see what needs to be done to finish this and get it in the store?