Reorder simplestreams URLs

Reorder simplestreams URLs

This branch is a more complete fix for some work done late
last week when the 1.15 release was thought to be immiment.
It restores the order of simplestreams urls so that any that
are specified in the environments.yaml are used first. The
HP Cloud specific URL is inserted into the custom sources
instead of being processed at config validation time.

https://code.launchpad.net/~wallyworld/juju-core/reorder-simplestreams-urls/+merge/187439

(do not edit description out of merge proposal)

[wallyworld, 2013-09-25 02:28:40]

Please take a look.

[jameinel, 2013-09-25 09:30:32]

LGTM

https://codereview.appspot.com/13888043/diff/1/provider/openstack/export_test.go
File provider/openstack/export_test.go (right):

https://codereview.appspot.com/13888043/diff/1/provider/openstack/export_test...
provider/openstack/export_test.go:88: func PatchCertifiedURL(auth_url, fake_url
string) func() {
The fact that you are adding/removing a trailing slash here concerns me that our
matching is overly-strict internally.

Specifically, if I specify:
 auth-url: http://hp.foo/stuff

Then we wouldn't find the tools unless I did:
 auth-url: http://hp.foo/stuff/

Now while according to the URL spec, those *are* different places, they aren't
different to a User. So I'd rather we always-strip (or always append?).

This function doesn't check if you already have a '/' which means you might end
up with :
  http://localhost/foo//

Anyway, doesn't have to change, but does bring up a question about how we are
treating URLs and if we are being too strict with exact auth-url matching.

[wallyworld, 2013-09-25 10:48:44]

https://codereview.appspot.com/13888043/diff/1/provider/openstack/export_test.go
File provider/openstack/export_test.go (right):

https://codereview.appspot.com/13888043/diff/1/provider/openstack/export_test...
provider/openstack/export_test.go:88: func PatchCertifiedURL(auth_url, fake_url
string) func() {
On 2013/09/25 09:30:32, jameinel wrote:
> The fact that you are adding/removing a trailing slash here concerns me that
our
> matching is overly-strict internally.
> 
> Specifically, if I specify:
>  auth-url: http://hp.foo/stuff
> 
> Then we wouldn't find the tools unless I did:
>  auth-url: http://hp.foo/stuff/
> 
> Now while according to the URL spec, those *are* different places, they aren't
> different to a User. So I'd rather we always-strip (or always append?).
> 
> This function doesn't check if you already have a '/' which means you might
end
> up with :
>   http://localhost/foo//
> 
> Anyway, doesn't have to change, but does bring up a question about how we are
> treating URLs and if we are being too strict with exact auth-url matching.

The tools business logic does the right thing. Because this test helper is
poking the internals directly, it needs to be a bit fussy. But in practice, a
user can have an auth url with or without a trailing / and it will work.