Merge pull request #8905 from hashicorp/fix_8493

Fix azure key vault cleanup failure

builder/azure/arm/azure_client.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute"
 	newCompute "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-03-01/compute"
+	"github.com/Azure/azure-sdk-for-go/services/keyvault/mgmt/2018-02-14/keyvault"
 	"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-01-01/network"
 	"github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources"
 	armStorage "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage"
@@ -50,7 +51,7 @@ type AzureClient struct {
 	InspectorMaxLength int
 	Template           *CaptureTemplate
 	LastError          azureErrorResponse
-	VaultClientDelete  common.VaultClient
+	VaultClientDelete  keyvault.VaultsClient
 }
 
 func getCaptureResponse(body string) *CaptureTemplate {
@@ -251,15 +252,9 @@ func NewAzureClient(subscriptionID, resourceGroupName, storageAccountName string
 	azureClient.VaultClient.UserAgent = fmt.Sprintf("%s %s", useragent.String(), azureClient.VaultClient.UserAgent)
 	azureClient.VaultClient.Client.PollingDuration = PollingDuration
 
-	// TODO(boumenot) - SDK still does not have a full KeyVault client.
+	// This client is different than the above because it manages the vault
-	// There are two ways that KeyVault has to be accessed, and each one has their own SPN.  An authenticated SPN
+	// itself rather than the contents of the vault.
-	// is tied to the URL, and the URL associated with getting the secret is different than the URL
+	azureClient.VaultClientDelete = keyvault.NewVaultsClient(subscriptionID)
-	// associated with deleting the KeyVault.  As a result, I need to have *two* different clients to
-	// access KeyVault.  I did not want to split it into two separate files, so I am starting with this.
-	//
-	// I do not like this implementation.  It is getting long in the tooth, and should be re-examined now
-	// that we have a "working" solution.
-	azureClient.VaultClientDelete = common.NewVaultClientWithBaseURI(cloud.ResourceManagerEndpoint, subscriptionID)
 	azureClient.VaultClientDelete.Authorizer = autorest.NewBearerAuthorizer(servicePrincipalToken)
 	azureClient.VaultClientDelete.RequestInspector = withInspection(maxlen)
 	azureClient.VaultClientDelete.ResponseInspector = byConcatDecorators(byInspecting(maxlen), errorCapture(azureClient))

builder/azure/arm/builder.go

@@ -228,11 +228,11 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 			NewStepDeleteAdditionalDisks(azureClient, ui),
 		}
 	} else if b.config.OSType == constants.Target_Windows {
-		keyVaultDeploymentName := b.stateBag.Get(constants.ArmKeyVaultDeploymentName).(string)
 		steps = []multistep.Step{
 			NewStepCreateResourceGroup(azureClient, ui),
 		}
 		if b.config.BuildKeyVaultName == "" {
+			keyVaultDeploymentName := b.stateBag.Get(constants.ArmKeyVaultDeploymentName).(string)
 			steps = append(steps,
 				NewStepValidateTemplate(azureClient, ui, &b.config, GetKeyVaultDeployment),
 				NewStepDeployTemplate(azureClient, ui, &b.config, keyVaultDeploymentName, GetKeyVaultDeployment),
@@ -399,7 +399,7 @@ func (b *Builder) configureStateBag(stateBag multistep.StateBag) {
 	stateBag.Put(constants.ArmComputeName, b.config.tmpComputeName)
 	stateBag.Put(constants.ArmDeploymentName, b.config.tmpDeploymentName)
 
-	if b.config.OSType == constants.Target_Windows {
+	if b.config.OSType == constants.Target_Windows && b.config.BuildKeyVaultName == "" {
 		stateBag.Put(constants.ArmKeyVaultDeploymentName, fmt.Sprintf("kv%s", b.config.tmpDeploymentName))
 	}
 

builder/azure/arm/step_delete_resource_group.go

@@ -45,7 +45,8 @@ func (s *StepDeleteResourceGroup) deleteResourceGroup(ctx context.Context, state
 
 		if keyVaultDeploymentName, ok := state.GetOk(constants.ArmKeyVaultDeploymentName); ok {
 			// Only delete if custom keyvault was not provided.
-			if exists := state.Get(constants.ArmIsExistingKeyVault).(bool); exists {
+			if exists := state.Get(constants.ArmIsExistingKeyVault).(bool); !exists {
+				s.say("\n Deleting the keyvault deployment because it was created by Packer...")
 				err = s.deleteDeploymentResources(ctx, keyVaultDeploymentName.(string), resourceGroupName)
 				if err != nil {
 					return err

builder/azure/arm/step_deploy_template.go

@@ -117,8 +117,7 @@ func deleteResource(ctx context.Context, client *AzureClient, resourceType strin
 		}
 		return err
 	case "Microsoft.KeyVault/vaults":
-		// TODO(paulmey): not sure why VaultClient doesn't do cancellation
+		_, err := client.VaultClientDelete.Delete(ctx, resourceGroupName, resourceName)
-		_, err := client.VaultClientDelete.Delete(resourceGroupName, resourceName)
 		return err
 	case "Microsoft.Network/networkInterfaces":
 		f, err := client.InterfacesClient.Delete(ctx, resourceGroupName, resourceName)

builder/azure/common/vault.go

@@ -9,7 +9,6 @@ import (
 	"net/url"
 
 	"github.com/Azure/go-autorest/autorest"
-	"github.com/Azure/go-autorest/autorest/azure"
 )
 
 const (
@@ -20,9 +19,6 @@ const (
 type AZVaultClientIface interface {
 	GetSecret(string, string) (*Secret, error)
 	SetSecret(string, string, string) error
-	DeletePreparer(string, string) (*http.Request, error)
-	DeleteResponder(*http.Response) (autorest.Response, error)
-	DeleteSender(*http.Request) (*http.Response, error)
 }
 
 type VaultClient struct {
@@ -137,72 +133,6 @@ func (client *VaultClient) SetSecret(vaultName, secretName string, secretValue s
 	return nil
 }
 
-// Delete deletes the specified Azure key vault.
-//
-// resourceGroupName is the name of the Resource Group to which the vault belongs. vaultName is the name of the vault
-// to delete
-func (client *VaultClient) Delete(resourceGroupName string, vaultName string) (result autorest.Response, err error) {
-	req, err := client.DeletePreparer(resourceGroupName, vaultName)
-	if err != nil {
-		err = autorest.NewErrorWithError(err, "keyvault.VaultsClient", "Delete", nil, "Failure preparing request")
-		return
-	}
-
-	resp, err := client.DeleteSender(req)
-	if err != nil {
-		result.Response = resp
-		err = autorest.NewErrorWithError(err, "keyvault.VaultsClient", "Delete", resp, "Failure sending request")
-		return
-	}
-
-	result, err = client.DeleteResponder(resp)
-	if err != nil {
-		err = autorest.NewErrorWithError(err, "keyvault.VaultsClient", "Delete", resp, "Failure responding to request")
-	}
-
-	return
-}
-
-// DeletePreparer prepares the Delete request.
-func (client *VaultClient) DeletePreparer(resourceGroupName string, vaultName string) (*http.Request, error) {
-	pathParameters := map[string]interface{}{
-		"resourceGroupName": autorest.Encode("path", resourceGroupName),
-		"SubscriptionID":    autorest.Encode("path", client.SubscriptionID),
-		"vaultName":         autorest.Encode("path", vaultName),
-	}
-
-	queryParameters := map[string]interface{}{
-		"api-version": AzureVaultApiVersion,
-	}
-
-	preparer := autorest.CreatePreparer(
-		autorest.AsDelete(),
-		autorest.WithBaseURL(client.baseURI),
-		autorest.WithPathParameters("/subscriptions/{SubscriptionID}/resourceGroups/{resourceGroupName}/providers/Microsoft.KeyVault/vaults/{vaultName}", pathParameters),
-		autorest.WithQueryParameters(queryParameters))
-	return preparer.Prepare(&http.Request{})
-}
-
-// DeleteSender sends the Delete request. The method will close the
-// http.Response Body if it receives an error.
-func (client *VaultClient) DeleteSender(req *http.Request) (*http.Response, error) {
-	return autorest.SendWithSender(client,
-		req,
-		azure.DoRetryWithRegistration(client.Client))
-}
-
-// DeleteResponder handles the response to the Delete request. The method always
-// closes the http.Response Body.
-func (client *VaultClient) DeleteResponder(resp *http.Response) (result autorest.Response, err error) {
-	err = autorest.Respond(
-		resp,
-		client.ByInspecting(),
-		azure.WithErrorUnlessStatusCode(http.StatusOK),
-		autorest.ByClosing())
-	result.Response = resp
-	return
-}
-
 func (client *VaultClient) getVaultUrl(vaultName string) string {
 	return fmt.Sprintf("%s://%s.%s/", client.keyVaultEndpoint.Scheme, vaultName, client.keyVaultEndpoint.Host)
 }

vendor/github.com/Azure/azure-sdk-for-go/services/keyvault/mgmt/2018-02-14/keyvault/client.go

@@ -0,0 +1,51 @@
+// Package keyvault implements the Azure ARM Keyvault service API version 2018-02-14.
+//
+// The Azure management API provides a RESTful set of web services that interact with Azure Key Vault.
+package keyvault
+
+// Copyright (c) Microsoft and contributors.  All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// Code generated by Microsoft (R) AutoRest Code Generator.
+// Changes may cause incorrect behavior and will be lost if the code is regenerated.
+
+import (
+	"github.com/Azure/go-autorest/autorest"
+)
+
+const (
+	// DefaultBaseURI is the default URI used for the service Keyvault
+	DefaultBaseURI = "https://management.azure.com"
+)
+
+// BaseClient is the base client for Keyvault.
+type BaseClient struct {
+	autorest.Client
+	BaseURI        string
+	SubscriptionID string
+}
+
+// New creates an instance of the BaseClient client.
+func New(subscriptionID string) BaseClient {
+	return NewWithBaseURI(DefaultBaseURI, subscriptionID)
+}
+
+// NewWithBaseURI creates an instance of the BaseClient client.
+func NewWithBaseURI(baseURI string, subscriptionID string) BaseClient {
+	return BaseClient{
+		Client:         autorest.NewClientWithUserAgent(UserAgent()),
+		BaseURI:        baseURI,
+		SubscriptionID: subscriptionID,
+	}
+}

vendor/github.com/Azure/azure-sdk-for-go/services/keyvault/mgmt/2018-02-14/keyvault/operations.go

@@ -0,0 +1,148 @@
+package keyvault
+
+// Copyright (c) Microsoft and contributors.  All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// Code generated by Microsoft (R) AutoRest Code Generator.
+// Changes may cause incorrect behavior and will be lost if the code is regenerated.
+
+import (
+	"context"
+	"github.com/Azure/go-autorest/autorest"
+	"github.com/Azure/go-autorest/autorest/azure"
+	"github.com/Azure/go-autorest/tracing"
+	"net/http"
+)
+
+// OperationsClient is the the Azure management API provides a RESTful set of web services that interact with Azure Key
+// Vault.
+type OperationsClient struct {
+	BaseClient
+}
+
+// NewOperationsClient creates an instance of the OperationsClient client.
+func NewOperationsClient(subscriptionID string) OperationsClient {
+	return NewOperationsClientWithBaseURI(DefaultBaseURI, subscriptionID)
+}
+
+// NewOperationsClientWithBaseURI creates an instance of the OperationsClient client.
+func NewOperationsClientWithBaseURI(baseURI string, subscriptionID string) OperationsClient {
+	return OperationsClient{NewWithBaseURI(baseURI, subscriptionID)}
+}
+
+// List lists all of the available Key Vault Rest API operations.
+func (client OperationsClient) List(ctx context.Context) (result OperationListResultPage, err error) {
+	if tracing.IsEnabled() {
+		ctx = tracing.StartSpan(ctx, fqdn+"/OperationsClient.List")
+		defer func() {
+			sc := -1
+			if result.olr.Response.Response != nil {
+				sc = result.olr.Response.Response.StatusCode
+			}
+			tracing.EndSpan(ctx, sc, err)
+		}()
+	}
+	result.fn = client.listNextResults
+	req, err := client.ListPreparer(ctx)
+	if err != nil {
+		err = autorest.NewErrorWithError(err, "keyvault.OperationsClient", "List", nil, "Failure preparing request")
+		return
+	}
+
+	resp, err := client.ListSender(req)
+	if err != nil {
+		result.olr.Response = autorest.Response{Response: resp}
+		err = autorest.NewErrorWithError(err, "keyvault.OperationsClient", "List", resp, "Failure sending request")
+		return
+	}
+
+	result.olr, err = client.ListResponder(resp)
+	if err != nil {
+		err = autorest.NewErrorWithError(err, "keyvault.OperationsClient", "List", resp, "Failure responding to request")
+	}
+
+	return
+}
+
+// ListPreparer prepares the List request.
+func (client OperationsClient) ListPreparer(ctx context.Context) (*http.Request, error) {
+	const APIVersion = "2018-02-14"
+	queryParameters := map[string]interface{}{
+		"api-version": APIVersion,
+	}
+
+	preparer := autorest.CreatePreparer(
+		autorest.AsGet(),
+		autorest.WithBaseURL(client.BaseURI),
+		autorest.WithPath("/providers/Microsoft.KeyVault/operations"),
+		autorest.WithQueryParameters(queryParameters))
+	return preparer.Prepare((&http.Request{}).WithContext(ctx))
+}
+
+// ListSender sends the List request. The method will close the
+// http.Response Body if it receives an error.
+func (client OperationsClient) ListSender(req *http.Request) (*http.Response, error) {
+	return autorest.SendWithSender(client, req,
+		autorest.DoRetryForStatusCodes(client.RetryAttempts, client.RetryDuration, autorest.StatusCodesForRetry...))
+}
+
+// ListResponder handles the response to the List request. The method always
+// closes the http.Response Body.
+func (client OperationsClient) ListResponder(resp *http.Response) (result OperationListResult, err error) {
+	err = autorest.Respond(
+		resp,
+		client.ByInspecting(),
+		azure.WithErrorUnlessStatusCode(http.StatusOK),
+		autorest.ByUnmarshallingJSON(&result),
+		autorest.ByClosing())
+	result.Response = autorest.Response{Response: resp}
+	return
+}
+
+// listNextResults retrieves the next set of results, if any.
+func (client OperationsClient) listNextResults(ctx context.Context, lastResults OperationListResult) (result OperationListResult, err error) {
+	req, err := lastResults.operationListResultPreparer(ctx)
+	if err != nil {
+		return result, autorest.NewErrorWithError(err, "keyvault.OperationsClient", "listNextResults", nil, "Failure preparing next results request")
+	}
+	if req == nil {
+		return
+	}
+	resp, err := client.ListSender(req)
+	if err != nil {
+		result.Response = autorest.Response{Response: resp}
+		return result, autorest.NewErrorWithError(err, "keyvault.OperationsClient", "listNextResults", resp, "Failure sending next results request")
+	}
+	result, err = client.ListResponder(resp)
+	if err != nil {
+		err = autorest.NewErrorWithError(err, "keyvault.OperationsClient", "listNextResults", resp, "Failure responding to next results request")
+	}
+	return
+}
+
+// ListComplete enumerates all values, automatically crossing page boundaries as required.
+func (client OperationsClient) ListComplete(ctx context.Context) (result OperationListResultIterator, err error) {
+	if tracing.IsEnabled() {
+		ctx = tracing.StartSpan(ctx, fqdn+"/OperationsClient.List")
+		defer func() {
+			sc := -1
+			if result.Response().Response.Response != nil {
+				sc = result.page.Response().Response.Response.StatusCode
+			}
+			tracing.EndSpan(ctx, sc, err)
+		}()
+	}
+	result.page, err = client.List(ctx)
+	return
+}

vendor/github.com/Azure/azure-sdk-for-go/services/keyvault/mgmt/2018-02-14/keyvault/version.go

@@ -0,0 +1,30 @@
+package keyvault
+
+import "github.com/Azure/azure-sdk-for-go/version"
+
+// Copyright (c) Microsoft and contributors.  All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// Code generated by Microsoft (R) AutoRest Code Generator.
+// Changes may cause incorrect behavior and will be lost if the code is regenerated.
+
+// UserAgent returns the UserAgent string to use when sending http.Requests.
+func UserAgent() string {
+	return "Azure-SDK-For-Go/" + version.Number + " keyvault/2018-02-14"
+}
+
+// Version returns the semantic version (see http://semver.org) of the client.
+func Version() string {
+	return version.Number
+}

vendor/modules.txt

@@ -18,6 +18,7 @@ github.com/Azure/azure-sdk-for-go/profiles/latest/compute/mgmt/compute/computeap
 github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute
 github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-03-01/compute
 github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-03-01/compute/computeapi
+github.com/Azure/azure-sdk-for-go/services/keyvault/mgmt/2018-02-14/keyvault
 github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-01-01/network
 github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2016-06-01/subscriptions
 github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources