Fix target region building bug

hashicorp · Feb 5, 2024 · 1e4d28b · 1e4d28b
1 parent a834ea1
commit 1e4d28b
Show file tree

Hide file tree

Showing 8 changed files with 36 additions and 41 deletions.
diff --git a/.web-docs/components/builder/arm/README.md b/.web-docs/components/builder/arm/README.md
@@ -645,7 +645,7 @@ The shared_image_gallery_destination block is available for publishing a new ima
 
 - `target_region` ([]TargetRegion) - A target region to store the image version in. The attribute supersedes `replication_regions` which is now considered deprecated.
   One or more target_region blocks can be specified for storing an imager version to various regions. In addition to specifying a region,
-  a DiskEncryptionSetId can be specified for each target region to support multi-region disk encryption, as well as a StorageAccountType to customize storage per region.
+  a DiskEncryptionSetId can be specified for each target region to support multi-region disk encryption.
   At a minimum their must be one target region entry for the primary build region where the image version will be stored.
   Target region must only contain one entry matching the build region when using shallow replication.
 

diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go
@@ -269,7 +269,6 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook)
 		}
 
 		b.stateBag.Put(constants.ArmSharedImageGalleryDestinationTargetRegions, b.config.SharedGalleryDestination.SigDestinationTargetRegions)
-
 	}
 
 	sourceImageSpecialized := false

diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go
@@ -107,7 +107,7 @@ type SharedImageGalleryDestination struct {
 	SigDestinationReplicationRegions []string `mapstructure:"replication_regions"`
 	// A target region to store the image version in. The attribute supersedes `replication_regions` which is now considered deprecated.
 	// One or more target_region blocks can be specified for storing an imager version to various regions. In addition to specifying a region,
-	// a DiskEncryptionSetId can be specified for each target region to support multi-region disk encryption, as well as a StorageAccountType to customize storage per region.
+	// a DiskEncryptionSetId can be specified for each target region to support multi-region disk encryption.
 	// At a minimum their must be one target region entry for the primary build region where the image version will be stored.
 	// Target region must only contain one entry matching the build region when using shallow replication.
 	SigDestinationTargetRegions []TargetRegion `mapstructure:"target_region"`
@@ -127,14 +127,10 @@ type SharedImageGalleryDestination struct {
 type TargetRegion struct {
 	// Name of the Azure region
 	Name string `mapstructure:"name" required:"true"`
-	// EncryptionSetID for Disk Encryption Set in Region. Needed for supporting
+	// DiskEncryptionSetId for Disk Encryption Set in Region. Needed for supporting
 	// the replication of encrypted disks across regions. CMKs must
 	// already exist within the target regions.
 	DiskEncryptionSetId string `mapstructure:"disk_encryption_set_id"`
-
-	//StorageAccountType a storage account type for the Shared Image Gallery Image Version.
-	// Defaults to `Standard_LRS`. Accepted values are `Standard_LRS`, `Standard_ZRS` and `Premium_LRS`
-	StorageAccountType string `mapstructure:"storage_account_type"`
 }
 
 type Spot struct {

diff --git a/builder/azure/arm/config.hcl2spec.go b/builder/azure/arm/config.hcl2spec.go
diff --git a/builder/azure/arm/step_publish_to_shared_image_gallery.go b/builder/azure/arm/step_publish_to_shared_image_gallery.go
@@ -95,24 +95,25 @@ func getSigDestination(state multistep.StateBag) SharedImageGalleryDestination {
 	}
 }
 
-func buildAzureImageTargetRegions(in []TargetRegion) []galleryimageversions.TargetRegion {
-	targetRegions := make([]galleryimageversions.TargetRegion, 0, len(in))
-	for _, tr := range in {
-		r := galleryimageversions.TargetRegion{Name: tr.Name}
-		if tr.DiskEncryptionSetId != "" {
-			r.Encryption = &galleryimageversions.EncryptionImages{
-				OsDiskImage: &galleryimageversions.OSDiskImageEncryption{
-					DiskEncryptionSetId: &tr.DiskEncryptionSetId,
-				},
-			}
+func buildAzureImageTargetRegions(regions []TargetRegion) []galleryimageversions.TargetRegion {
+	targetRegions := make([]galleryimageversions.TargetRegion, 0, len(regions))
+	for _, r := range regions {
+		name := r.Name
+		tr := galleryimageversions.TargetRegion{Name: name}
+
+		id := r.DiskEncryptionSetId
+		tr.Encryption = &galleryimageversions.EncryptionImages{
+			OsDiskImage: &galleryimageversions.OSDiskImageEncryption{
+				DiskEncryptionSetId: &id,
+			},
 		}
-		targetRegions = append(targetRegions, r)
+		targetRegions = append(targetRegions, tr)
 	}
 	return targetRegions
 }
+
 func (s *StepPublishToSharedImageGallery) publishToSig(ctx context.Context, args PublishArgs) (string, error) {
 	imageVersionRegions := buildAzureImageTargetRegions(args.SharedImageGallery.SigDestinationTargetRegions)
-
 	storageAccountType, err := getSigDestinationStorageAccountType(args.SharedImageGallery.SigDestinationStorageAccountType)
 	if err != nil {
 		s.error(err)

diff --git a/builder/azure/arm/step_publish_to_shared_image_gallery_test.go b/builder/azure/arm/step_publish_to_shared_image_gallery_test.go
@@ -11,7 +11,6 @@ import (
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/hashicorp/go-azure-sdk/resource-manager/compute/2022-03-01/images"
 
-	"github.com/hashicorp/go-azure-sdk/resource-manager/compute/2022-03-03/galleryimageversions"
 	"github.com/hashicorp/packer-plugin-azure/builder/azure/common"
 	"github.com/hashicorp/packer-plugin-azure/builder/azure/common/constants"
 	"github.com/hashicorp/packer-plugin-sdk/multistep"
@@ -279,25 +278,30 @@ func TestStepPublishToSharedImageGalleryShouldPublishTargetRegions(t *testing.T)
 
 func TestPublishToSharedImageGalleryBuildAzureImageTargetRegions(t *testing.T) {
 	tt := []struct {
-		name string
-		in   []TargetRegion
-		want []galleryimageversions.TargetRegion
+		name            string
+		in              []TargetRegion
+		expectedRegions int
 	}{
-		{name: "empty regions", in: nil, want: make([]galleryimageversions.TargetRegion, 0)},
-		{name: "empty regions non nil", in: make([]TargetRegion, 0), want: make([]galleryimageversions.TargetRegion, 0)},
-		{name: "one named region", in: []TargetRegion{{Name: "unit-test-location"}}, want: []galleryimageversions.TargetRegion{{Name: "unit-test-location"}}},
-		{name: "two named region", in: []TargetRegion{{Name: "unit-test-location"}, {Name: "unit-test-location-2"}}, want: []galleryimageversions.TargetRegion{{Name: "unit-test-location"}, {Name: "unit-test-location-2"}}},
+		{name: "empty regions", in: nil, expectedRegions: 0},
+		{name: "empty regions non nil", in: make([]TargetRegion, 0), expectedRegions: 0},
+		{name: "one named region", in: []TargetRegion{{Name: "unit-test-location"}}, expectedRegions: 1},
+		{name: "two named region", in: []TargetRegion{{Name: "unit-test-location"}, {Name: "unit-test-location-2"}}, expectedRegions: 2},
 		{
-			name: "named region with encryption",
-			in:   []TargetRegion{{Name: "unit-test-location", DiskEncryptionSetId: "boguskey"}},
-			want: []galleryimageversions.TargetRegion{{Name: "unit-test-location", Encryption: &galleryimageversions.EncryptionImages{OsDiskImage: &galleryimageversions.OSDiskImageEncryption{}}}},
+			name:            "named region with encryption",
+			in:              []TargetRegion{{Name: "unit-test-location", DiskEncryptionSetId: "boguskey"}},
+			expectedRegions: 1,
+		},
+		{
+			name:            "two named region with encryption",
+			in:              []TargetRegion{{Name: "unit-test-location", DiskEncryptionSetId: "boguskey"}, {Name: "unit-test-location-west", DiskEncryptionSetId: "boguskeywest"}},
+			expectedRegions: 2,
 		},
 	}
 
 	for _, tc := range tt {
 		got := buildAzureImageTargetRegions(tc.in)
-		if len(got) != len(tc.want) {
-			t.Errorf("expected configureTargetRegion() to have same region count: got %d expected %d", len(tc.in), len(tc.want))
+		if len(got) != tc.expectedRegions {
+			t.Errorf("expected configureTargetRegion() to have same region count: got %d expected %d", len(tc.in), tc.expectedRegions)
 		}
 
 		for i, tr := range got {
@@ -307,7 +311,7 @@ func TestPublishToSharedImageGalleryBuildAzureImageTargetRegions(t *testing.T) {
 			}
 
 			if (inputRegion.DiskEncryptionSetId != "") && (*tr.Encryption.OsDiskImage.DiskEncryptionSetId != inputRegion.DiskEncryptionSetId) {
-				t.Errorf("expected configured region to contain set DES Id %q but got %q", inputRegion.DiskEncryptionSetId, *tr.Encryption.OsDiskImage.DiskEncryptionSetId)
+				t.Errorf("[%q]: expected configured region to contain set DES Id %q but got %q", tc.name, inputRegion.DiskEncryptionSetId, *tr.Encryption.OsDiskImage.DiskEncryptionSetId)
 			}
 		}
 	}

diff --git a/docs-partials/builder/azure/arm/SharedImageGalleryDestination-not-required.mdx b/docs-partials/builder/azure/arm/SharedImageGalleryDestination-not-required.mdx
@@ -15,7 +15,7 @@
 
 - `target_region` ([]TargetRegion) - A target region to store the image version in. The attribute supersedes `replication_regions` which is now considered deprecated.
   One or more target_region blocks can be specified for storing an imager version to various regions. In addition to specifying a region,
-  a DiskEncryptionSetId can be specified for each target region to support multi-region disk encryption, as well as a StorageAccountType to customize storage per region.
+  a DiskEncryptionSetId can be specified for each target region to support multi-region disk encryption.
   At a minimum their must be one target region entry for the primary build region where the image version will be stored.
   Target region must only contain one entry matching the build region when using shallow replication.
 

diff --git a/docs-partials/builder/azure/arm/TargetRegion-not-required.mdx b/docs-partials/builder/azure/arm/TargetRegion-not-required.mdx
@@ -1,10 +1,7 @@
 <!-- Code generated from the comments of the TargetRegion struct in builder/azure/arm/config.go; DO NOT EDIT MANUALLY -->
 
-- `disk_encryption_set_id` (string) - EncryptionSetID for Disk Encryption Set in Region. Needed for supporting
+- `disk_encryption_set_id` (string) - DiskEncryptionSetId for Disk Encryption Set in Region. Needed for supporting
   the replication of encrypted disks across regions. CMKs must
   already exist within the target regions.
 
-- `storage_account_type` (string) - StorageAccountType a storage account type for the Shared Image Gallery Image Version.
-  Defaults to `Standard_LRS`. Accepted values are `Standard_LRS`, `Standard_ZRS` and `Premium_LRS`
-
 <!-- End of code generated from the comments of the TargetRegion struct in builder/azure/arm/config.go; -->
-Original file line number
+Diff line change
@@ Expand Up @@
     		}
     		b.stateBag.Put(constants.ArmSharedImageGalleryDestinationTargetRegions, b.config.SharedGalleryDestination.SigDestinationTargetRegions)
     	}
     	sourceImageSpecialized := false
@@ Expand Down @@