From b2047bd938932319c84ab304a2016b2ddc57f9c2 Mon Sep 17 00:00:00 2001 From: Braunson <5280764+braunsonm@users.noreply.github.com> Date: Wed, 30 Sep 2020 09:55:46 -0400 Subject: [PATCH] Put the correct AzureTags type in StateBag (#10014) Azure expects the tags format to use a pointer to the string for the map value. The configuration from the builder is not a pointer so when storing in the state bag for reference in later execution we convert it when creating the StateBag. Fixes #10012 and #10013. * Use the MapToAzureTags helper and error check in resource group creation * Added test case for tag values not using a pointer * test/azure/arm: Add test to verify tags stored in state * test/azure/arm: Add azure_tags to existing acceptance test for Linux Test Before Fix ``` --- FAIL: TestBuilderAcc_ManagedDisk_Linux (1.81s) panic: interface conversion: interface {} is map[string]string, not map[string]*string [recovered] panic: interface conversion: interface {} is map[string]string, not map[string]*string FAIL github.com/hashicorp/packer/builder/azure/arm 1.822s ``` Test After Fix ``` 2020/09/29 17:23:03 ui: ==> test: Resource group has been deleted. --- PASS: TestBuilderAcc_ManagedDisk_Linux (517.41s) PASS ok github.com/hashicorp/packer/builder/azure/arm 517.426s ``` Co-authored-by: Wilken Rivera --- builder/azure/arm/builder.go | 2 +- builder/azure/arm/builder_acc_test.go | 6 +++- builder/azure/arm/builder_test.go | 32 +++++++++++++++++++ builder/azure/arm/config_test.go | 4 +-- .../azure/arm/step_create_resource_group.go | 8 ++++- .../arm/step_create_resource_group_test.go | 29 +++++++++++++++++ 6 files changed, 76 insertions(+), 5 deletions(-) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index b0eced19e..3b01a5bfa 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -390,7 +390,7 @@ func (b *Builder) getBlobAccount(ctx context.Context, client *AzureClient, resou func (b *Builder) configureStateBag(stateBag multistep.StateBag) { stateBag.Put(constants.AuthorizedKey, b.config.sshAuthorizedKey) - stateBag.Put(constants.ArmTags, b.config.AzureTags) + stateBag.Put(constants.ArmTags, packerAzureCommon.MapToAzureTags(b.config.AzureTags)) stateBag.Put(constants.ArmComputeName, b.config.tmpComputeName) stateBag.Put(constants.ArmDeploymentName, b.config.tmpDeploymentName) diff --git a/builder/azure/arm/builder_acc_test.go b/builder/azure/arm/builder_acc_test.go index 57e48a4cb..b52f7fd42 100644 --- a/builder/azure/arm/builder_acc_test.go +++ b/builder/azure/arm/builder_acc_test.go @@ -223,7 +223,11 @@ const testBuilderAccManagedDiskLinux = ` "image_sku": "16.04-LTS", "location": "South Central US", - "vm_size": "Standard_DS2_v2" + "vm_size": "Standard_DS2_v2", + "azure_tags": { + "env": "testing", + "builder": "packer" + } }] } ` diff --git a/builder/azure/arm/builder_test.go b/builder/azure/arm/builder_test.go index 547a0c18a..3f4bb77c0 100644 --- a/builder/azure/arm/builder_test.go +++ b/builder/azure/arm/builder_test.go @@ -33,3 +33,35 @@ func TestStateBagShouldBePopulatedExpectedValues(t *testing.T) { } } } + +func TestStateBagShouldPoluateExpectedTags(t *testing.T) { + var testSubject Builder + + expectedTags := map[string]string{ + "env": "test", + "builder": "packer", + } + armConfig := getArmBuilderConfiguration() + armConfig["azure_tags"] = expectedTags + + _, _, err := testSubject.Prepare(armConfig, getPackerConfiguration()) + if err != nil { + t.Fatalf("failed to prepare: %s", err) + } + + tags, ok := testSubject.stateBag.Get(constants.ArmTags).(map[string]*string) + if !ok { + t.Errorf("Expected the builder's state bag to contain tags of type %T, but didn't.", testSubject.config.AzureTags) + } + + if len(tags) != len(expectedTags) { + t.Errorf("expect tags from state to be the same length as tags from config") + } + + for k, v := range tags { + if expectedTags[k] != *v { + t.Errorf("expect tag value of %s to be %s, but got %s", k, expectedTags[k], *v) + } + } + +} diff --git a/builder/azure/arm/config_test.go b/builder/azure/arm/config_test.go index a6a25665a..dda6e9234 100644 --- a/builder/azure/arm/config_test.go +++ b/builder/azure/arm/config_test.go @@ -2072,8 +2072,8 @@ func TestConfig_PrepareProvidedWinRMPassword(t *testing.T) { } } -func getArmBuilderConfiguration() map[string]string { - m := make(map[string]string) +func getArmBuilderConfiguration() map[string]interface{} { + m := make(map[string]interface{}) for _, v := range requiredConfigValues { m[v] = "ignored00" } diff --git a/builder/azure/arm/step_create_resource_group.go b/builder/azure/arm/step_create_resource_group.go index b5be70452..f745d5054 100644 --- a/builder/azure/arm/step_create_resource_group.go +++ b/builder/azure/arm/step_create_resource_group.go @@ -61,7 +61,13 @@ func (s *StepCreateResourceGroup) Run(ctx context.Context, state multistep.State var resourceGroupName = state.Get(constants.ArmResourceGroupName).(string) var location = state.Get(constants.ArmLocation).(string) - var tags = state.Get(constants.ArmTags).(map[string]*string) + tags, ok := state.Get(constants.ArmTags).(map[string]*string) + if !ok { + err := fmt.Errorf("failed to extract tags from state bag") + state.Put(constants.Error, err) + s.error(err) + return multistep.ActionHalt + } exists, err := s.exists(ctx, resourceGroupName) if err != nil { diff --git a/builder/azure/arm/step_create_resource_group_test.go b/builder/azure/arm/step_create_resource_group_test.go index cb8707fbe..2073aa32c 100644 --- a/builder/azure/arm/step_create_resource_group_test.go +++ b/builder/azure/arm/step_create_resource_group_test.go @@ -220,3 +220,32 @@ func createTestExistingStateBagStepCreateResourceGroup() multistep.StateBag { stateBag.Put(constants.ArmTags, tags) return stateBag } + +func TestStepCreateResourceGroupShouldFailIfTagsFailCast(t *testing.T) { + stateBag := new(multistep.BasicStateBag) + + stateBag.Put(constants.ArmLocation, "Unit Test: Location") + stateBag.Put(constants.ArmResourceGroupName, "Unit Test: ResourceGroupName") + stateBag.Put(constants.ArmIsExistingResourceGroup, true) + + value := "Unit Test: Tags" + tags := map[string]string{ + "tag01": value, + } + + stateBag.Put(constants.ArmTags, tags) + var testSubject = &StepCreateResourceGroup{ + create: func(context.Context, string, string, map[string]*string) error { return nil }, + say: func(message string) {}, + error: func(e error) {}, + exists: func(context.Context, string) (bool, error) { return false, nil }, + } + var result = testSubject.Run(context.Background(), stateBag) + if result != multistep.ActionHalt { + t.Fatalf("Expected the step to return 'ActionHalt', but got '%d'.", result) + } + + if _, ok := stateBag.GetOk(constants.Error); ok == false { + t.Fatalf("Expected the step to set stateBag['%s'], but it was not.", constants.Error) + } +}