From b7e32cb10a9d74e0532a4d41dbe36af6ff78d9f4 Mon Sep 17 00:00:00 2001 From: Christopher Boumenot Date: Thu, 8 Jun 2017 17:57:59 -0700 Subject: [PATCH] azure: best error message possible --- builder/azure/arm/azure_client.go | 37 ++++++--- builder/azure/arm/azure_error_response.go | 67 ++++++++++++++++ ...tAzureErrorNestedShouldFormat.approved.txt | 4 + ...tAzureErrorSimpleShouldFormat.approved.txt | 1 + .../azure/arm/azure_error_response_test.go | 76 +++++++++++++++++++ builder/azure/arm/step_capture_image.go | 11 +++ .../azure/arm/step_create_resource_group.go | 3 + builder/azure/arm/step_delete_os_disk.go | 7 +- .../azure/arm/step_delete_resource_group.go | 3 + builder/azure/arm/step_deploy_template.go | 3 + builder/azure/arm/step_get_certificate.go | 1 + builder/azure/arm/step_get_ip_address.go | 1 + builder/azure/arm/step_get_os_disk.go | 6 +- builder/azure/arm/step_power_off_compute.go | 5 +- builder/azure/arm/step_validate_template.go | 3 + 15 files changed, 213 insertions(+), 15 deletions(-) create mode 100644 builder/azure/arm/azure_error_response.go create mode 100644 builder/azure/arm/azure_error_response_test.TestAzureErrorNestedShouldFormat.approved.txt create mode 100644 builder/azure/arm/azure_error_response_test.TestAzureErrorSimpleShouldFormat.approved.txt create mode 100644 builder/azure/arm/azure_error_response_test.go diff --git a/builder/azure/arm/azure_client.go b/builder/azure/arm/azure_client.go index 88a6990af..10c8d4bbe 100644 --- a/builder/azure/arm/azure_client.go +++ b/builder/azure/arm/azure_client.go @@ -47,6 +47,7 @@ type AzureClient struct { InspectorMaxLength int Template *CaptureTemplate + LastError azureErrorResponse } func getCaptureResponse(body string) *CaptureTemplate { @@ -98,6 +99,22 @@ func templateCapture(client *AzureClient) autorest.RespondDecorator { } } +func errorCapture(client *AzureClient) autorest.RespondDecorator { + return func(r autorest.Responder) autorest.Responder { + return autorest.ResponderFunc(func(resp *http.Response) error { + body, bodyString := handleBody(resp.Body, math.MaxInt64) + resp.Body = body + + errorResponse := newAzureErrorResponse(bodyString) + if errorResponse != nil { + client.LastError = *errorResponse + } + + return r.Respond(resp) + }) + } +} + // WAITING(chrboum): I have logged https://github.com/Azure/azure-sdk-for-go/issues/311 to get this // method included in the SDK. It has been accepted, and I'll cut over to the official way // once it ships. @@ -118,55 +135,55 @@ func NewAzureClient(subscriptionID, resourceGroupName, storageAccountName string azureClient.DeploymentsClient = resources.NewDeploymentsClientWithBaseURI(cloud.ResourceManagerEndpoint, subscriptionID) azureClient.DeploymentsClient.Authorizer = autorest.NewBearerAuthorizer(servicePrincipalToken) azureClient.DeploymentsClient.RequestInspector = withInspection(maxlen) - azureClient.DeploymentsClient.ResponseInspector = byInspecting(maxlen) + azureClient.DeploymentsClient.ResponseInspector = byConcatDecorators(byInspecting(maxlen), errorCapture(azureClient)) azureClient.DeploymentsClient.UserAgent += packerUserAgent azureClient.GroupsClient = resources.NewGroupsClientWithBaseURI(cloud.ResourceManagerEndpoint, subscriptionID) azureClient.GroupsClient.Authorizer = autorest.NewBearerAuthorizer(servicePrincipalToken) azureClient.GroupsClient.RequestInspector = withInspection(maxlen) - azureClient.GroupsClient.ResponseInspector = byInspecting(maxlen) + azureClient.GroupsClient.ResponseInspector = byConcatDecorators(byInspecting(maxlen), errorCapture(azureClient)) azureClient.GroupsClient.UserAgent += packerUserAgent azureClient.ImagesClient = compute.NewImagesClientWithBaseURI(cloud.ResourceManagerEndpoint, subscriptionID) azureClient.ImagesClient.Authorizer = autorest.NewBearerAuthorizer(servicePrincipalToken) azureClient.ImagesClient.RequestInspector = withInspection(maxlen) - azureClient.ImagesClient.ResponseInspector = byInspecting(maxlen) + azureClient.ImagesClient.ResponseInspector = byConcatDecorators(byInspecting(maxlen), errorCapture(azureClient)) azureClient.ImagesClient.UserAgent += packerUserAgent azureClient.InterfacesClient = network.NewInterfacesClientWithBaseURI(cloud.ResourceManagerEndpoint, subscriptionID) azureClient.InterfacesClient.Authorizer = autorest.NewBearerAuthorizer(servicePrincipalToken) azureClient.InterfacesClient.RequestInspector = withInspection(maxlen) - azureClient.InterfacesClient.ResponseInspector = byInspecting(maxlen) + azureClient.InterfacesClient.ResponseInspector = byConcatDecorators(byInspecting(maxlen), errorCapture(azureClient)) azureClient.InterfacesClient.UserAgent += packerUserAgent azureClient.SubnetsClient = network.NewSubnetsClientWithBaseURI(cloud.ResourceManagerEndpoint, subscriptionID) azureClient.SubnetsClient.Authorizer = autorest.NewBearerAuthorizer(servicePrincipalToken) azureClient.SubnetsClient.RequestInspector = withInspection(maxlen) - azureClient.SubnetsClient.ResponseInspector = byInspecting(maxlen) + azureClient.SubnetsClient.ResponseInspector = byConcatDecorators(byInspecting(maxlen), errorCapture(azureClient)) azureClient.SubnetsClient.UserAgent += packerUserAgent azureClient.VirtualNetworksClient = network.NewVirtualNetworksClientWithBaseURI(cloud.ResourceManagerEndpoint, subscriptionID) azureClient.VirtualNetworksClient.Authorizer = autorest.NewBearerAuthorizer(servicePrincipalToken) azureClient.VirtualNetworksClient.RequestInspector = withInspection(maxlen) - azureClient.VirtualNetworksClient.ResponseInspector = byInspecting(maxlen) + azureClient.VirtualNetworksClient.ResponseInspector = byConcatDecorators(byInspecting(maxlen), errorCapture(azureClient)) azureClient.VirtualNetworksClient.UserAgent += packerUserAgent azureClient.PublicIPAddressesClient = network.NewPublicIPAddressesClientWithBaseURI(cloud.ResourceManagerEndpoint, subscriptionID) azureClient.PublicIPAddressesClient.Authorizer = autorest.NewBearerAuthorizer(servicePrincipalToken) azureClient.PublicIPAddressesClient.RequestInspector = withInspection(maxlen) - azureClient.PublicIPAddressesClient.ResponseInspector = byInspecting(maxlen) + azureClient.PublicIPAddressesClient.ResponseInspector = byConcatDecorators(byInspecting(maxlen), errorCapture(azureClient)) azureClient.PublicIPAddressesClient.UserAgent += packerUserAgent azureClient.VirtualMachinesClient = compute.NewVirtualMachinesClientWithBaseURI(cloud.ResourceManagerEndpoint, subscriptionID) azureClient.VirtualMachinesClient.Authorizer = autorest.NewBearerAuthorizer(servicePrincipalToken) azureClient.VirtualMachinesClient.RequestInspector = withInspection(maxlen) - azureClient.VirtualMachinesClient.ResponseInspector = byConcatDecorators(byInspecting(maxlen), templateCapture(azureClient)) + azureClient.VirtualMachinesClient.ResponseInspector = byConcatDecorators(byInspecting(maxlen), templateCapture(azureClient), errorCapture(azureClient)) azureClient.VirtualMachinesClient.UserAgent += packerUserAgent azureClient.AccountsClient = armStorage.NewAccountsClientWithBaseURI(cloud.ResourceManagerEndpoint, subscriptionID) azureClient.AccountsClient.Authorizer = autorest.NewBearerAuthorizer(servicePrincipalToken) azureClient.AccountsClient.RequestInspector = withInspection(maxlen) - azureClient.AccountsClient.ResponseInspector = byInspecting(maxlen) + azureClient.AccountsClient.ResponseInspector = byConcatDecorators(byInspecting(maxlen), errorCapture(azureClient)) azureClient.AccountsClient.UserAgent += packerUserAgent keyVaultURL, err := url.Parse(cloud.KeyVaultEndpoint) @@ -177,7 +194,7 @@ func NewAzureClient(subscriptionID, resourceGroupName, storageAccountName string azureClient.VaultClient = common.NewVaultClient(*keyVaultURL) azureClient.VaultClient.Authorizer = autorest.NewBearerAuthorizer(servicePrincipalTokenVault) azureClient.VaultClient.RequestInspector = withInspection(maxlen) - azureClient.VaultClient.ResponseInspector = byInspecting(maxlen) + azureClient.VaultClient.ResponseInspector = byConcatDecorators(byInspecting(maxlen), errorCapture(azureClient)) azureClient.VaultClient.UserAgent += packerUserAgent // If this is a managed disk build, this should be ignored. diff --git a/builder/azure/arm/azure_error_response.go b/builder/azure/arm/azure_error_response.go new file mode 100644 index 000000000..d0b1cfacc --- /dev/null +++ b/builder/azure/arm/azure_error_response.go @@ -0,0 +1,67 @@ +package arm + +import ( + "bytes" + "encoding/json" + "fmt" +) + +type azureErrorDetails struct { + Code string `json:"code""` + Message string `json:"message"` + Details []azureErrorDetails `json:"details"` +} + +type azureErrorResponse struct { + ErrorDetails azureErrorDetails `json:"error"` +} + +func newAzureErrorResponse(s string) *azureErrorResponse { + var errorResponse azureErrorResponse + err := json.Unmarshal([]byte(s), &errorResponse) + if err == nil { + return &errorResponse + } + + return nil +} + +func (e *azureErrorDetails) isEmpty() bool { + return e.Code == "" +} + +func (e *azureErrorResponse) isEmpty() bool { + return e.ErrorDetails.isEmpty() +} + +func (e *azureErrorResponse) Error() string { + var buf bytes.Buffer + //buf.WriteString("-=-=- ERROR -=-=-") + formatAzureErrorResponse(e.ErrorDetails, &buf, "") + //buf.WriteString("-=-=- ERROR -=-=-") + return buf.String() +} + +// format a Azure Error Response by recursing through the JSON structure. +// +// Errors may contain nested errors, which are JSON documents that have been +// serialized and escaped. Keep following this nesting all the way down... +func formatAzureErrorResponse(error azureErrorDetails, buf *bytes.Buffer, indent string) { + if error.isEmpty() { + return + } + + buf.WriteString(fmt.Sprintf("ERROR: %s-> %s : %s\n", indent, error.Code, error.Message)) + for _, x := range error.Details { + newIndent := fmt.Sprintf("%s ", indent) + + var aer azureErrorResponse + err := json.Unmarshal([]byte(x.Message), &aer) + if err == nil { + buf.WriteString(fmt.Sprintf("ERROR: %s-> %s\n", newIndent, x.Code)) + formatAzureErrorResponse(aer.ErrorDetails, buf, newIndent) + } else { + buf.WriteString(fmt.Sprintf("ERROR: %s-> %s : %s\n", newIndent, x.Code, x.Message)) + } + } +} diff --git a/builder/azure/arm/azure_error_response_test.TestAzureErrorNestedShouldFormat.approved.txt b/builder/azure/arm/azure_error_response_test.TestAzureErrorNestedShouldFormat.approved.txt new file mode 100644 index 000000000..9160d5bc3 --- /dev/null +++ b/builder/azure/arm/azure_error_response_test.TestAzureErrorNestedShouldFormat.approved.txt @@ -0,0 +1,4 @@ +ERROR: -> DeploymentFailed : At least one resource deployment operation failed. Please list deployment operations for details. Please see https://aka.ms/arm-debug for usage details. +ERROR: -> BadRequest +ERROR: -> InvalidRequestFormat : Cannot parse the request. +ERROR: -> InvalidJson : Error converting value "playground" to type 'Microsoft.WindowsAzure.Networking.Nrp.Frontend.Contract.Csm.Public.IpAllocationMethod'. Path 'properties.publicIPAllocationMethod', line 1, position 130. diff --git a/builder/azure/arm/azure_error_response_test.TestAzureErrorSimpleShouldFormat.approved.txt b/builder/azure/arm/azure_error_response_test.TestAzureErrorSimpleShouldFormat.approved.txt new file mode 100644 index 000000000..d50bdc052 --- /dev/null +++ b/builder/azure/arm/azure_error_response_test.TestAzureErrorSimpleShouldFormat.approved.txt @@ -0,0 +1 @@ +ERROR: -> ResourceNotFound : The Resource 'Microsoft.Compute/images/PackerUbuntuImage' under resource group 'packer-test00' was not found. diff --git a/builder/azure/arm/azure_error_response_test.go b/builder/azure/arm/azure_error_response_test.go new file mode 100644 index 000000000..f077fa6fe --- /dev/null +++ b/builder/azure/arm/azure_error_response_test.go @@ -0,0 +1,76 @@ +package arm + +import ( + "github.com/approvals/go-approval-tests" + "github.com/hashicorp/packer/common/json" + "strings" + "testing" +) + +const AzureErrorSimple = `{"error":{"code":"ResourceNotFound","message":"The Resource 'Microsoft.Compute/images/PackerUbuntuImage' under resource group 'packer-test00' was not found."}}` +const AzureErrorNested = `{"status":"Failed","error":{"code":"DeploymentFailed","message":"At least one resource deployment operation failed. Please list deployment operations for details. Please see https://aka.ms/arm-debug for usage details.","details":[{"code":"BadRequest","message":"{\r\n \"error\": {\r\n \"code\": \"InvalidRequestFormat\",\r\n \"message\": \"Cannot parse the request.\",\r\n \"details\": [\r\n {\r\n \"code\": \"InvalidJson\",\r\n \"message\": \"Error converting value \\\"playground\\\" to type 'Microsoft.WindowsAzure.Networking.Nrp.Frontend.Contract.Csm.Public.IpAllocationMethod'. Path 'properties.publicIPAllocationMethod', line 1, position 130.\"\r\n }\r\n ]\r\n }\r\n}"}]}}` + +func TestAzureErrorSimpleShouldUnmarshal(t *testing.T) { + var azureErrorReponse azureErrorResponse + err := json.Unmarshal([]byte(AzureErrorSimple), &azureErrorReponse) + if err != nil { + t.Fatal(err) + } + + if azureErrorReponse.ErrorDetails.Code != "ResourceNotFound" { + t.Errorf("Error.Code") + } + if azureErrorReponse.ErrorDetails.Message != "The Resource 'Microsoft.Compute/images/PackerUbuntuImage' under resource group 'packer-test00' was not found." { + t.Errorf("Error.Message") + } +} + +func TestAzureErrorNestedShouldUnmarshal(t *testing.T) { + var azureError azureErrorResponse + err := json.Unmarshal([]byte(AzureErrorNested), &azureError) + if err != nil { + t.Fatal(err) + } + + if azureError.ErrorDetails.Code != "DeploymentFailed" { + t.Errorf("Error.Code") + } + if !strings.HasPrefix(azureError.ErrorDetails.Message, "At least one resource deployment operation failed") { + t.Errorf("Error.Message") + } +} + +func TestAzureErrorEmptyShouldFormat(t *testing.T) { + var aer azureErrorResponse + s := aer.Error() + + if s != "" { + t.Fatalf("Expected \"\", but got %s", aer.Error()) + } +} + +func TestAzureErrorSimpleShouldFormat(t *testing.T) { + var azureErrorReponse azureErrorResponse + err := json.Unmarshal([]byte(AzureErrorSimple), &azureErrorReponse) + if err != nil { + t.Fatal(err) + } + + err = approvaltests.VerifyString(t, azureErrorReponse.Error()) + if err != nil { + t.Fatal(err) + } +} + +func TestAzureErrorNestedShouldFormat(t *testing.T) { + var azureErrorReponse azureErrorResponse + err := json.Unmarshal([]byte(AzureErrorNested), &azureErrorReponse) + if err != nil { + t.Fatal(err) + } + + err = approvaltests.VerifyString(t, azureErrorReponse.Error()) + if err != nil { + t.Fatal(err) + } +} diff --git a/builder/azure/arm/step_capture_image.go b/builder/azure/arm/step_capture_image.go index d3f26c5de..eac4b5c63 100644 --- a/builder/azure/arm/step_capture_image.go +++ b/builder/azure/arm/step_capture_image.go @@ -46,16 +46,27 @@ func NewStepCaptureImage(client *AzureClient, ui packer.Ui) *StepCaptureImage { func (s *StepCaptureImage) generalize(resourceGroupName string, computeName string) error { _, err := s.client.Generalize(resourceGroupName, computeName) + if err != nil { + s.say(s.client.LastError.Error()) + } return err } func (s *StepCaptureImage) captureImageFromVM(resourceGroupName string, imageName string, image *compute.Image, cancelCh <-chan struct{}) error { _, errChan := s.client.ImagesClient.CreateOrUpdate(resourceGroupName, imageName, *image, cancelCh) + err := <-errChan + if err != nil { + s.say(s.client.LastError.Error()) + } return <-errChan } func (s *StepCaptureImage) captureImage(resourceGroupName string, computeName string, parameters *compute.VirtualMachineCaptureParameters, cancelCh <-chan struct{}) error { _, errChan := s.client.Capture(resourceGroupName, computeName, *parameters, cancelCh) + err := <-errChan + if err != nil { + s.say(s.client.LastError.Error()) + } return <-errChan } diff --git a/builder/azure/arm/step_create_resource_group.go b/builder/azure/arm/step_create_resource_group.go index 13a09e0e7..b8e24149a 100644 --- a/builder/azure/arm/step_create_resource_group.go +++ b/builder/azure/arm/step_create_resource_group.go @@ -36,6 +36,9 @@ func (s *StepCreateResourceGroup) createResourceGroup(resourceGroupName string, Tags: tags, }) + if err != nil { + s.say(s.client.LastError.Error()) + } return err } diff --git a/builder/azure/arm/step_delete_os_disk.go b/builder/azure/arm/step_delete_os_disk.go index 4cedd513d..0041a8ec6 100644 --- a/builder/azure/arm/step_delete_os_disk.go +++ b/builder/azure/arm/step_delete_os_disk.go @@ -34,7 +34,12 @@ func NewStepDeleteOSDisk(client *AzureClient, ui packer.Ui) *StepDeleteOSDisk { func (s *StepDeleteOSDisk) deleteBlob(storageContainerName string, blobName string) error { blob := s.client.BlobStorageClient.GetContainerReference(storageContainerName).GetBlobReference(blobName) - return blob.Delete(nil) + err := blob.Delete(nil) + + if err != nil { + s.say(s.client.LastError.Error()) + } + return err } func (s *StepDeleteOSDisk) Run(state multistep.StateBag) multistep.StepAction { diff --git a/builder/azure/arm/step_delete_resource_group.go b/builder/azure/arm/step_delete_resource_group.go index d5412ff3c..5a1683287 100644 --- a/builder/azure/arm/step_delete_resource_group.go +++ b/builder/azure/arm/step_delete_resource_group.go @@ -34,6 +34,9 @@ func (s *StepDeleteResourceGroup) deleteResourceGroup(resourceGroupName string, _, errChan := s.client.GroupsClient.Delete(resourceGroupName, cancelCh) err := <-errChan + if err != nil { + s.say(s.client.LastError.Error()) + } return err } diff --git a/builder/azure/arm/step_deploy_template.go b/builder/azure/arm/step_deploy_template.go index 25a16a573..962486846 100644 --- a/builder/azure/arm/step_deploy_template.go +++ b/builder/azure/arm/step_deploy_template.go @@ -43,6 +43,9 @@ func (s *StepDeployTemplate) deployTemplate(resourceGroupName string, deployment _, errChan := s.client.DeploymentsClient.CreateOrUpdate(resourceGroupName, deploymentName, *deployment, cancelCh) err = <-errChan + if err != nil { + s.say(s.client.LastError.Error()) + } return err } diff --git a/builder/azure/arm/step_get_certificate.go b/builder/azure/arm/step_get_certificate.go index 8253ff8ed..ffc27b141 100644 --- a/builder/azure/arm/step_get_certificate.go +++ b/builder/azure/arm/step_get_certificate.go @@ -35,6 +35,7 @@ func NewStepGetCertificate(client *AzureClient, ui packer.Ui) *StepGetCertificat func (s *StepGetCertificate) getCertificateUrl(keyVaultName string, secretName string) (string, error) { secret, err := s.client.GetSecret(keyVaultName, secretName) if err != nil { + s.say(s.client.LastError.Error()) return "", err } diff --git a/builder/azure/arm/step_get_ip_address.go b/builder/azure/arm/step_get_ip_address.go index ed22fed00..fed78afbb 100644 --- a/builder/azure/arm/step_get_ip_address.go +++ b/builder/azure/arm/step_get_ip_address.go @@ -54,6 +54,7 @@ func NewStepGetIPAddress(client *AzureClient, ui packer.Ui, endpoint EndpointTyp func (s *StepGetIPAddress) getPrivateIP(resourceGroupName string, ipAddressName string, interfaceName string) (string, error) { resp, err := s.client.InterfacesClient.Get(resourceGroupName, interfaceName, "") if err != nil { + s.say(s.client.LastError.Error()) return "", err } diff --git a/builder/azure/arm/step_get_os_disk.go b/builder/azure/arm/step_get_os_disk.go index 566348e3a..6b21d57de 100644 --- a/builder/azure/arm/step_get_os_disk.go +++ b/builder/azure/arm/step_get_os_disk.go @@ -33,7 +33,11 @@ func NewStepGetOSDisk(client *AzureClient, ui packer.Ui) *StepGetOSDisk { } func (s *StepGetOSDisk) queryCompute(resourceGroupName string, computeName string) (compute.VirtualMachine, error) { - return s.client.VirtualMachinesClient.Get(resourceGroupName, computeName, "") + vm, err := s.client.VirtualMachinesClient.Get(resourceGroupName, computeName, "") + if err != nil { + s.say(s.client.LastError.Error()) + } + return vm, err } func (s *StepGetOSDisk) Run(state multistep.StateBag) multistep.StepAction { diff --git a/builder/azure/arm/step_power_off_compute.go b/builder/azure/arm/step_power_off_compute.go index 1a19c3b2a..a1d38f349 100644 --- a/builder/azure/arm/step_power_off_compute.go +++ b/builder/azure/arm/step_power_off_compute.go @@ -35,10 +35,9 @@ func (s *StepPowerOffCompute) powerOffCompute(resourceGroupName string, computeN err := <-errChan if err != nil { - return err + s.say(s.client.LastError.Error()) } - - return nil + return err } func (s *StepPowerOffCompute) Run(state multistep.StateBag) multistep.StepAction { diff --git a/builder/azure/arm/step_validate_template.go b/builder/azure/arm/step_validate_template.go index f2e286b0b..5a041f912 100644 --- a/builder/azure/arm/step_validate_template.go +++ b/builder/azure/arm/step_validate_template.go @@ -40,6 +40,9 @@ func (s *StepValidateTemplate) validateTemplate(resourceGroupName string, deploy } _, err = s.client.Validate(resourceGroupName, deploymentName, *deployment) + if err != nil { + s.say(s.client.LastError.Error()) + } return err }