Merge lp:~rvb/gwacl/delete-deployment into lp:gwacl

[Raphaël Badin]

Add method to delete a deployment.

[Gavin Panella (allenap)]

Looks good, but [1] is something we need to address before this
library is ready for production.

[1]

+    URI := "services/hostedservices/" + serviceName + "/deployments/" + deploymentName

serviceName and deploymentName ought to be passed through
url.QueryEscape... but then that will cause double escaping if set as
Path on a url.URL instance. Perhaps what needs to be done is to
replace / with %2F, *then* set as the Path. Something like:

    uri := url.Parse(base_url)
    uri.Path = (
        "services/hostedservices/" +
        strings.Replace(serviceName, "/", "%2F") +
        "/deployments/" +
        strings.Replace(deploymentName, "/", "%2F"))
    return api.session.delete(uri.String())

(Fwiw, I gleaned this from reading url.go alongside §3.3 of RFC 3986.)

This is not a problem unique to this branch, so I don't really expect
you to fix it, but we really ought to before stamping this with a 1.0
version.

Fwiw, a poor-man's fix would be to ensure that we (a) don't allow
forward-slash in user-supplied parts of URLs, and (b) ensure that URLs
are put together using url.URL so that proper escaping takes place.

review: Approve

[Raphaël Badin (rvb)]

Thanks for the review. As discussed on IRC, I completely agree that we should fix it. But we should fix it with a utility method used everywhere because this escaping problem is all over the place.

I'll look into it and see if we can create a utility like func Urlf(format string, components ...string) that would url-escape the components given to it.

=== modified file 'example/live_example_managementapi.go'
--- example/live_example_managementapi.go	2013-03-27 09:01:34 +0000
+++ example/live_example_managementapi.go	2013-03-28 12:23:20 +0000
@@ -143,6 +143,10 @@
     api.PerformRoleOperation(hostServiceName, deployment.Name, role.RoleName, operation)
     fmt.Println("Done starting VM\n")
 
+    fmt.Println("Deleting deployment...")
+    api.DeleteDeployment(hostServiceName, deployment.Name)
+    fmt.Println("Done deleting deployment\n")
+
     fmt.Println("Deleting hosted service...")
     api.DeleteHostedService(hostServiceName)
     fmt.Println("Done deleting hosted service\n")
 
=== modified file 'managementapi.go'
--- managementapi.go	2013-03-28 06:34:47 +0000
+++ managementapi.go	2013-03-28 12:23:20 +0000
@@ -83,6 +83,11 @@
     return nil
 }
 
+func (api *ManagementAPI) DeleteDeployment(serviceName string, deploymentName string) error {
+    URI := "services/hostedservices/" + serviceName + "/deployments/" + deploymentName
+    return api.session.delete(URI)
+}
+
 // AddStorageAccount starts the creation of a storage account.  This is
 // called a storage service in the Azure API, but nomenclature seems to
 // have changed.
 
=== modified file 'managementapi_test.go'
--- managementapi_test.go	2013-03-28 06:34:47 +0000
+++ managementapi_test.go	2013-03-28 12:23:20 +0000
@@ -145,6 +145,18 @@
     checkOneRequest(c, recordedRequests, expectedURL, expectedPayload, "POST")
 }
 
+func (suite *managementAPISuite) TestDeleteDeployment(c *C) {
+    api := suite.makeAPI(c)
+    recordedRequests := setUpDispatcher("operationID")
+    hostedServiceName := "testHosterServiceName"
+    deploymentName := "testDeploymentName"
+    err := api.DeleteDeployment(hostedServiceName, deploymentName)
+
+    c.Assert(err, IsNil)
+    expectedURL := AZURE_URL + api.session.subscriptionId + "/services/hostedservices/" + hostedServiceName + "/deployments/" + deploymentName
+    checkOneRequest(c, recordedRequests, expectedURL, []byte{}, "DELETE")
+}
+
 func (suite *managementAPISuite) TestAddStorageAccount(c *C) {
     api := suite.makeAPI(c)
     rigFixedResponseDispatcher(&x509Response{StatusCode: http.StatusAccepted})