From 654fade0a9198fc6448307e58c071ec5dc9f0aac Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 24 Sep 2020 12:19:26 +0200 Subject: [PATCH] azure arm: make map[string]*string => map[string]string to simplify things (#9985) * azure arm: make map[string]*string => map[string]string to simplify things * go generate ./builder/azure/... * tests ! fix #9984 --- builder/azure/arm/config.go | 23 ++++---- builder/azure/arm/config.hcl2spec.go | 2 +- builder/azure/arm/config_test.go | 57 +++++++++++++------ builder/azure/common/map.go | 10 ++++ builder/azure/common/template/template.go | 20 +++---- .../azure/common/template/template_builder.go | 2 +- .../builder/azure/arm/Config-not-required.mdx | 2 +- 7 files changed, 74 insertions(+), 42 deletions(-) create mode 100644 builder/azure/common/map.go diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go index 958098347..10f3a47f7 100644 --- a/builder/azure/arm/config.go +++ b/builder/azure/arm/config.go @@ -288,7 +288,7 @@ type Config struct { // Group, VM, NIC, VNET, Public IP, KeyVault, etc. The user can define up // to 15 tags. Tag names cannot exceed 512 characters, and tag values // cannot exceed 256 characters. - AzureTags map[string]*string `mapstructure:"azure_tags" required:"false"` + AzureTags map[string]string `mapstructure:"azure_tags" required:"false"` // Same as [`azure_tags`](#azure_tags) but defined as a singular repeatable block // containing a `name` and a `value` field. In HCL2 mode the // [`dynamic_block`](/docs/configuration/from-1.5/expressions#dynamic-blocks) @@ -513,7 +513,7 @@ func (c *Config) toImageParameters() *compute.Image { }, }, Location: to.StringPtr(c.Location), - Tags: c.AzureTags, + Tags: azcommon.MapToAzureTags(c.AzureTags), } } @@ -596,10 +596,7 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, error) { } // copy singular blocks - for _, kv := range c.AzureTag { - v := kv.Value - c.AzureTags[kv.Name] = &v - } + c.AzureTag.CopyOn(&c.AzureTags) err = c.ClientConfig.SetDefaultValues() if err != nil { @@ -807,8 +804,8 @@ func assertTagProperties(c *Config, errs *packer.MultiError) { if len(k) > 512 { errs = packer.MultiErrorAppend(errs, fmt.Errorf("the tag name %q exceeds (%d) the 512 character limit", k, len(k))) } - if len(*v) > 256 { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("the tag name %q exceeds (%d) the 256 character limit", *v, len(*v))) + if len(v) > 256 { + errs = packer.MultiErrorAppend(errs, fmt.Errorf("the tag name %q exceeds (%d) the 256 character limit", v, len(v))) } } } @@ -1063,13 +1060,13 @@ func assertRequiredParametersSet(c *Config, errs *packer.MultiError) { errs = packer.MultiErrorAppend(errs, fmt.Errorf("if either plan_name, plan_product, plan_publisher, or plan_promotion_code are defined then plan_name, plan_product, and plan_publisher must be defined")) } else { if c.AzureTags == nil { - c.AzureTags = make(map[string]*string) + c.AzureTags = make(map[string]string) } - c.AzureTags["PlanInfo"] = &c.PlanInfo.PlanName - c.AzureTags["PlanProduct"] = &c.PlanInfo.PlanProduct - c.AzureTags["PlanPublisher"] = &c.PlanInfo.PlanPublisher - c.AzureTags["PlanPromotionCode"] = &c.PlanInfo.PlanPromotionCode + c.AzureTags["PlanInfo"] = c.PlanInfo.PlanName + c.AzureTags["PlanProduct"] = c.PlanInfo.PlanProduct + c.AzureTags["PlanPublisher"] = c.PlanInfo.PlanPublisher + c.AzureTags["PlanPromotionCode"] = c.PlanInfo.PlanPromotionCode } } diff --git a/builder/azure/arm/config.hcl2spec.go b/builder/azure/arm/config.hcl2spec.go index 64de1fb5e..27c798dc0 100644 --- a/builder/azure/arm/config.hcl2spec.go +++ b/builder/azure/arm/config.hcl2spec.go @@ -49,7 +49,7 @@ type FlatConfig struct { ManagedImageOSDiskSnapshotName *string `mapstructure:"managed_image_os_disk_snapshot_name" required:"false" cty:"managed_image_os_disk_snapshot_name" hcl:"managed_image_os_disk_snapshot_name"` ManagedImageDataDiskSnapshotPrefix *string `mapstructure:"managed_image_data_disk_snapshot_prefix" required:"false" cty:"managed_image_data_disk_snapshot_prefix" hcl:"managed_image_data_disk_snapshot_prefix"` ManagedImageZoneResilient *bool `mapstructure:"managed_image_zone_resilient" required:"false" cty:"managed_image_zone_resilient" hcl:"managed_image_zone_resilient"` - AzureTags map[string]*string `mapstructure:"azure_tags" required:"false" cty:"azure_tags" hcl:"azure_tags"` + AzureTags map[string]string `mapstructure:"azure_tags" required:"false" cty:"azure_tags" hcl:"azure_tags"` AzureTag []hcl2template.FlatNameValue `mapstructure:"azure_tag" required:"false" cty:"azure_tag" hcl:"azure_tag"` ResourceGroupName *string `mapstructure:"resource_group_name" cty:"resource_group_name" hcl:"resource_group_name"` StorageAccount *string `mapstructure:"storage_account" cty:"storage_account" hcl:"storage_account"` diff --git a/builder/azure/arm/config_test.go b/builder/azure/arm/config_test.go index 3f6df81ff..a6a25665a 100644 --- a/builder/azure/arm/config_test.go +++ b/builder/azure/arm/config_test.go @@ -6,7 +6,9 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute" + "github.com/google/go-cmp/cmp" "github.com/hashicorp/packer/builder/azure/common/constants" + "github.com/hashicorp/packer/hcl2template" ) // List of configuration parameters that are required by the ARM builder. @@ -910,32 +912,55 @@ func TestConfigShouldAcceptTags(t *testing.T) { }, } - var c Config + c := Config{ + AzureTag: hcl2template.NameValues{ + {Name: "tag03", Value: "value03"}, + }, + } _, err := c.Prepare(config, getPackerConfiguration()) - if err != nil { t.Fatal(err) } - if len(c.AzureTags) != 2 { - t.Fatalf("expected to find 2 tags, but got %d", len(c.AzureTags)) + if diff := cmp.Diff(c.AzureTags, map[string]string{ + "tag01": "value01", + "tag02": "value02", + "tag03": "value03", + }); diff != "" { + t.Fatalf("unexpected azure tags: %s", diff) + } +} + +func TestConfigShouldAcceptTag(t *testing.T) { + config := map[string]interface{}{ + "capture_name_prefix": "ignore", + "capture_container_name": "ignore", + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "storage_account": "ignore", + "resource_group_name": "ignore", + "subscription_id": "ignore", + "communicator": "none", + // Does not matter for this test case, just pick one. + "os_type": constants.Target_Linux, } - if _, ok := c.AzureTags["tag01"]; !ok { - t.Error("expected to find key=\"tag01\", but did not") + c := Config{ + AzureTag: hcl2template.NameValues{ + {Name: "tag03", Value: "value03"}, + }, } - if _, ok := c.AzureTags["tag02"]; !ok { - t.Error("expected to find key=\"tag02\", but did not") + _, err := c.Prepare(config, getPackerConfiguration()) + if err != nil { + t.Fatal(err) } - value := c.AzureTags["tag01"] - if *value != "value01" { - t.Errorf("expected AzureTags[\"tag01\"] to have value \"value01\", but got %q", *value) - } - - value = c.AzureTags["tag02"] - if *value != "value02" { - t.Errorf("expected AzureTags[\"tag02\"] to have value \"value02\", but got %q", *value) + if diff := cmp.Diff(c.AzureTags, map[string]string{ + "tag03": "value03", + }); diff != "" { + t.Fatalf("unexpected azure tags: %s", diff) } } diff --git a/builder/azure/common/map.go b/builder/azure/common/map.go new file mode 100644 index 000000000..318f2780b --- /dev/null +++ b/builder/azure/common/map.go @@ -0,0 +1,10 @@ +package common + +func MapToAzureTags(in map[string]string) map[string]*string { + res := map[string]*string{} + for k := range in { + v := in[k] + res[k] = &v + } + return res +} diff --git a/builder/azure/common/template/template.go b/builder/azure/common/template/template.go index dad684735..32fa39917 100644 --- a/builder/azure/common/template/template.go +++ b/builder/azure/common/template/template.go @@ -25,16 +25,16 @@ type Parameters struct { ///////////////////////////////////////////////// // Template > Resource type Resource struct { - ApiVersion *string `json:"apiVersion"` - Name *string `json:"name"` - Type *string `json:"type"` - Location *string `json:"location,omitempty"` - DependsOn *[]string `json:"dependsOn,omitempty"` - Plan *Plan `json:"plan,omitempty"` - Properties *Properties `json:"properties,omitempty"` - Tags *map[string]*string `json:"tags,omitempty"` - Resources *[]Resource `json:"resources,omitempty"` - Identity *Identity `json:"identity,omitempty"` + ApiVersion *string `json:"apiVersion"` + Name *string `json:"name"` + Type *string `json:"type"` + Location *string `json:"location,omitempty"` + DependsOn *[]string `json:"dependsOn,omitempty"` + Plan *Plan `json:"plan,omitempty"` + Properties *Properties `json:"properties,omitempty"` + Tags *map[string]string `json:"tags,omitempty"` + Resources *[]Resource `json:"resources,omitempty"` + Identity *Identity `json:"identity,omitempty"` } type Plan struct { diff --git a/builder/azure/common/template/template_builder.go b/builder/azure/common/template/template_builder.go index 754c2a91e..d4248d04b 100644 --- a/builder/azure/common/template/template_builder.go +++ b/builder/azure/common/template/template_builder.go @@ -374,7 +374,7 @@ func (s *TemplateBuilder) SetNetworkSecurityGroup(ipAddresses []string, port int return nil } -func (s *TemplateBuilder) SetTags(tags *map[string]*string) error { +func (s *TemplateBuilder) SetTags(tags *map[string]string) error { if tags == nil || len(*tags) == 0 { return nil } diff --git a/website/pages/partials/builder/azure/arm/Config-not-required.mdx b/website/pages/partials/builder/azure/arm/Config-not-required.mdx index 4f2330308..f1bfc1ec9 100644 --- a/website/pages/partials/builder/azure/arm/Config-not-required.mdx +++ b/website/pages/partials/builder/azure/arm/Config-not-required.mdx @@ -137,7 +137,7 @@ region that supports [availability zones](https://docs.microsoft.com/en-us/azure/availability-zones/az-overview). -- `azure_tags` (map[string]\*string) - Name/value pair tags to apply to every resource deployed i.e. Resource +- `azure_tags` (map[string]string) - Name/value pair tags to apply to every resource deployed i.e. Resource Group, VM, NIC, VNET, Public IP, KeyVault, etc. The user can define up to 15 tags. Tag names cannot exceed 512 characters, and tag values cannot exceed 256 characters.