From d043c37ad41dd1df465e434a94d4c23c617d811d Mon Sep 17 00:00:00 2001 From: Jamie Lennox Date: Fri, 1 Dec 2017 11:43:35 +1100 Subject: [PATCH] Azure: Don't provide location for build_resource_group_name Location is required by default because you must specify where to create the resource group containing the packer resources. When using build_resource_group_name you are specifying that packer should use an existing resource group and so the location that resources are in can be determined by fetching the information from the existing group. It is forbidden to pass both variables as it is easier and more intuitive that the location comes from the group rather than ignore a parameter. Closes: #5655 --- builder/azure/arm/builder.go | 11 +++++- builder/azure/arm/builder_test.go | 1 - builder/azure/arm/config.go | 8 ++--- builder/azure/arm/config_test.go | 1 + website/source/docs/builders/azure.html.md | 40 ++++++++++++++++------ 5 files changed, 45 insertions(+), 16 deletions(-) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index c6b8bcbe1..d1f98bbd2 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -103,6 +103,15 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe } } + if b.config.BuildResourceGroupName != "" { + group, err := azureClient.GroupsClient.Get(b.config.BuildResourceGroupName) + if err != nil { + return nil, fmt.Errorf("Cannot locate the existing build resource resource group %s.", b.config.BuildResourceGroupName) + } + + b.config.Location = *group.Location + } + if b.config.StorageAccount != "" { account, err := b.getBlobAccount(azureClient, b.config.ResourceGroupName, b.config.StorageAccount) if err != nil { @@ -290,7 +299,6 @@ func (b *Builder) configureStateBag(stateBag multistep.StateBag) { stateBag.Put(constants.ArmKeyVaultDeploymentName, fmt.Sprintf("kv%s", b.config.tmpDeploymentName)) } stateBag.Put(constants.ArmKeyVaultName, b.config.tmpKeyVaultName) - stateBag.Put(constants.ArmLocation, b.config.Location) stateBag.Put(constants.ArmNicName, DefaultNicName) stateBag.Put(constants.ArmPublicIPAddressName, DefaultPublicIPAddressName) if b.config.TempResourceGroupName != "" && b.config.BuildResourceGroupName != "" { @@ -312,6 +320,7 @@ func (b *Builder) configureStateBag(stateBag multistep.StateBag) { // Parameters that are only known at runtime after querying Azure. func (b *Builder) setRuntimeParameters(stateBag multistep.StateBag) { + stateBag.Put(constants.ArmLocation, b.config.Location) stateBag.Put(constants.ArmManagedImageLocation, b.config.manageImageLocation) } diff --git a/builder/azure/arm/builder_test.go b/builder/azure/arm/builder_test.go index a44d363d0..2db475a02 100644 --- a/builder/azure/arm/builder_test.go +++ b/builder/azure/arm/builder_test.go @@ -20,7 +20,6 @@ func TestStateBagShouldBePopulatedExpectedValues(t *testing.T) { constants.ArmTags, constants.ArmComputeName, constants.ArmDeploymentName, - constants.ArmLocation, constants.ArmNicName, constants.ArmResourceGroupName, constants.ArmStorageAccountName, diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go index b2301db6c..f384c88bc 100644 --- a/builder/azure/arm/config.go +++ b/builder/azure/arm/config.go @@ -590,10 +590,6 @@ func assertRequiredParametersSet(c *Config, errs *packer.MultiError) { } } - if c.Location == "" { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("A location must be specified")) - } - ///////////////////////////////////////////// // Deployment xor := func(a, b bool) bool { @@ -604,6 +600,10 @@ func assertRequiredParametersSet(c *Config, errs *packer.MultiError) { errs = packer.MultiErrorAppend(errs, fmt.Errorf("Specify either a VHD (storage_account and resource_group_name) or Managed Image (managed_image_resource_group_name and managed_image_name) output")) } + if !xor(c.Location != "", c.BuildResourceGroupName != "") { + errs = packer.MultiErrorAppend(errs, fmt.Errorf("Must specify either a location to create the resource group in or an existing build_resource_group_name.")) + } + if c.ManagedImageName == "" && c.ManagedImageResourceGroupName == "" { if c.StorageAccount == "" { errs = packer.MultiErrorAppend(errs, fmt.Errorf("A storage_account must be specified")) diff --git a/builder/azure/arm/config_test.go b/builder/azure/arm/config_test.go index 75d397831..4cefd6730 100644 --- a/builder/azure/arm/config_test.go +++ b/builder/azure/arm/config_test.go @@ -972,6 +972,7 @@ func TestConfigShouldRejectInvalidResourceGroupNames(t *testing.T) { } } + delete(config, "location") // not valid for build_resource_group_name delete(config, x) } } diff --git a/website/source/docs/builders/azure.html.md b/website/source/docs/builders/azure.html.md index 779a374b8..8fa59f4e8 100644 --- a/website/source/docs/builders/azure.html.md +++ b/website/source/docs/builders/azure.html.md @@ -44,10 +44,6 @@ builder. CLI example `azure vm image list-skus -l westus -p Canonical -o UbuntuServer` -- `location` (string) Azure datacenter in which your VM will build. - - CLI example `azure location list` - #### VHD or Managed Image The Azure builder can create either a VHD, or a managed image. If you @@ -77,14 +73,42 @@ When creating a managed image the following two options are required. set. See [documentation](https://docs.microsoft.com/en-us/azure/storage/storage-managed-disks-overview#images) to learn more about managed images. +#### Resource Group Usage + +The Azure builder can either provision resources into a new resource group that +it controls (default) or an existing one. The advantage of using a packer +defined resource group is that failed resource cleanup is easier because you +can simply remove the entire resource group, however this means that the +provided credentials must have permission to create and remove resource groups. +By using an existing resource group you can scope the provided credentials to +just this group, however failed builds are more likely to leave unused +artifacts. + +To have packer create a resource group you **must** provide: + +- `location` (string) Azure datacenter in which your VM will build. + + CLI example `azure location list` + +and optionally: + +- `temp_resource_group_name` (string) name assigned to the temporary resource + group created during the build. If this value is not set, a random value will + be assigned. This resource group is deleted at the end of the build. + +To use an existing resource group you **must** provide: + +- `build_resource_group_name` (string) - Specify an existing resource group + to run the build in. + +Providing `temp_resource_group_name` or `location` in combination with `build_resource_group_name` is not allowed. + ### Optional: - `azure_tags` (object of name/value strings) - the user can define up to 15 tags. Tag names cannot exceed 512 characters, and tag values cannot exceed 256 characters. Tags are applied to every resource deployed by a Packer build, i.e. Resource Group, VM, NIC, VNET, Public IP, KeyVault, etc. -- `build_resource_group_name` (string) - Specify an existing resource group to run the build in. Cannot be used together with `temp_resource_group_name` and requires less permissions due to not creating or destroying a resource group. - - `cloud_environment_name` (string) One of `Public`, `China`, `Germany`, or `USGovernment`. Defaults to `Public`. Long forms such as `USGovernmentCloud` and `AzureUSGovernmentCloud` are also supported. @@ -134,10 +158,6 @@ When creating a managed image the following two options are required. assigned. Knowing the resource group and VM name allows one to execute commands to update the VM during a Packer build, e.g. attach a resource disk to the VM. -- `temp_resource_group_name` (string) name assigned to the temporary resource group created during the build. If this - value is not set, a random value will be assigned. This resource group is deleted at the end of the build. Cannot be - used together with `build_resource_group_name`. - - `tenant_id` (string) The account identifier with which your `client_id` and `subscription_id` are associated. If not specified, `tenant_id` will be looked up using `subscription_id`.