Code review comment for lp:~frankban/juju-core/get-charm-api

Revision history for this message
Francesco Banconi (frankban) wrote :

Reviewers: mp+207994_code.launchpad.net,

Message:
Please take a look.

Description:
Implement the get charm file API.

Add to the HTTPS API server the ability
to serve local charm files.

As discussed with Rick and Dimiter if the query includes the file,
then the file contents are served. If file points to a directory
an error is returned. If the query does not include a file, then
a JSON response is sent including a list of charm files.

QA:
- Download the Ghost charm from https://github.com/hatched/ghost-charm
   (the zip archive can be downloaded from the sidebar on the right).
- Bootstrap a local environment using this branch.
- Upload the ghost local charm, e.g.:
   `curl -ikL --data-binary @/path/to/ghost-charm-master.zip -H
"Content-Type: application/zip"
https://user-admin:MYPASSWD@10.0.3.1:17070/charms?series=precise`.
   The response should be something like:
{"CharmURL":"local:precise/ghost-4"}.
- Download ghost charm files, e.g.:
   `curl -ikLG -d "url=local:precise/ghost-4&file=icon.svg"
https://user-admin:MYPASSWD@10.0.3.1:17070/charms`
   or
   `curl -ikLG -d "url=local:precise/ghost-4&file=hooks/install"
https://user-admin:MYPASSWD@10.0.3.1:17070/charms`:
   you should see the file contents.
- Ensure directories are not allowed, e.g.:
   `curl -ikLG -d "url=local:precise/ghost-4&file=hooks"
https://user-admin:MYPASSWD@10.0.3.1:17070/charms`: you should see a 403
forbidden.
- Retrieve a list of the charm files:
   `curl -ikLG -d "url=local:precise/ghost-4"
https://user-admin:MYPASSWD@10.0.3.1:17070/charms`.
- Done, destroy the environment, thank you!

https://code.launchpad.net/~frankban/juju-core/get-charm-api/+merge/207994

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/67750045/

Affected files (+352, -30 lines):
   A [revision details]
   M state/api/params/internal.go
   M state/apiserver/apiserver.go
   M state/apiserver/charms.go
   M state/apiserver/charms_test.go

« Back to merge proposal