Move shallow replication validation to config.go

builder/azure/arm/builder.go

@@ -213,20 +213,6 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook)
 			return nil, fmt.Errorf("a gallery image version for image name:version %s:%s already exists in gallery %s", b.config.SharedGalleryDestination.SigDestinationImageName, b.config.SharedGalleryDestination.SigDestinationImageVersion, b.config.SharedGalleryDestination.SigDestinationGalleryName)
 		}
 
-		// TODO It would be better if validation could be handled in a central location
-		// Currently we rely on the build Resource Group being queried if used to get the build location
-		// So we have to do this validation afterwards
-		// We should remove this logic builder and handle this logic via the `Step` pattern
-		if b.config.SharedGalleryDestination.SigDestinationUseShallowReplicationMode {
-			err := errors.New("when `use_shallow_replication` there can only be one destination region defined and it must match `location` or match the region of `build_resource_group_name`.")
-			switch {
-			case len(b.config.SharedGalleryDestination.SigDestinationTargetRegions) > 1:
-				return nil, err
-			case len(b.config.SharedGalleryDestination.SigDestinationReplicationRegions) > 1:
-				return nil, err
-			}
-		}
-
 		if len(b.config.SharedGalleryDestination.SigDestinationTargetRegions) > 0 {
 			normalizedRegions := make([]TargetRegion, 0, len(b.config.SharedGalleryDestination.SigDestinationTargetRegions))
 			for _, tr := range b.config.SharedGalleryDestination.SigDestinationTargetRegions {

builder/azure/arm/config.go

@@ -124,6 +124,18 @@ type SharedImageGalleryDestination struct {
 	SigDestinationUseShallowReplicationMode bool `mapstructure:"use_shallow_replication" required:"false"`
 }
 
+func (d SharedImageGalleryDestination) ValidateShallowReplicationRegion() error {
+
+	n := len(d.SigDestinationTargetRegions) | len(d.SigDestinationReplicationRegions)
+	if n == 0 {
+		return errors.New("when `use_shallow_replication` is set there must be one destination region must match the build location of the Shared Image Gallery.")
+	}
+	if n > 1 {
+		return errors.New("when `use_shallow_replication` there can only be one destination region must match the build location of the Shared Image Gallery.")
+	}
+	return nil
+}
+
 // TargetRegion describes a destination region for storing the image version of a Shard Image Gallery.
 type TargetRegion struct {
 	// Name of the Azure region
@@ -1288,18 +1300,24 @@ func assertRequiredParametersSet(c *Config, errs *packersdk.MultiError) {
 		if c.SharedGalleryDestination.SigDestinationSubscription == "" {
 			c.SharedGalleryDestination.SigDestinationSubscription = c.ClientConfig.SubscriptionID
 		}
+		// Validate target region settings; it can be the deprecated replicated_regions attribute or multiple target_region blocks
+		if (len(c.SharedGalleryDestination.SigDestinationReplicationRegions) > 0) && (len(c.SharedGalleryDestination.SigDestinationTargetRegions) > 0) {
+			errs = packersdk.MultiErrorAppend(errs, errors.New("`replicated_regions` can not be defined alongside `target_region`; you can defined a target_region for each destination region you wish to replicate to."))
+		}
+
 		if c.SharedGalleryDestination.SigDestinationUseShallowReplicationMode {
+			if err := c.SharedGalleryDestination.ValidateShallowReplicationRegion(); err != nil {
+				errs = packersdk.MultiErrorAppend(errs, err)
+			}
+
 			if c.SharedGalleryImageVersionReplicaCount == 0 {
 				c.SharedGalleryImageVersionReplicaCount = 1
 			}
 
 			if c.SharedGalleryImageVersionReplicaCount != 1 {
 				errs = packersdk.MultiErrorAppend(errs, fmt.Errorf("When using shallow replication the replica count can only be 1, leaving this value unset will default to 1"))
 			}
-		}
-		// Validate target region settings; it can be the deprecated replicated_regions attribute or multiple target_region blocks
-		if (len(c.SharedGalleryDestination.SigDestinationReplicationRegions) > 0) && (len(c.SharedGalleryDestination.SigDestinationTargetRegions) > 0) {
-			errs = packersdk.MultiErrorAppend(errs, errors.New("`replicated_regions` can not be defined alongside `target_region`; you can defined a target_region for each destination region you wish to replicate to."))
+
 		}
 	}
 	if c.SharedGalleryTimeout == 0 {

builder/azure/arm/config_test.go

@@ -1362,6 +1362,116 @@ func TestConfigShouldAcceptShallowReplicationWithWithUnsetReplicaCount(t *testin
 		t.Fatalf("expected config to accept shallow replication with unset replica count build: %v", err)
 	}
 }
+
+func TestConfigValidateShallowReplicationRegion(t *testing.T) {
+	tt := []struct {
+		name          string
+		config        map[string]interface{}
+		errorExpected bool
+	}{
+		{
+			name: "with replication region",
+			config: map[string]interface{}{
+				"image_offer":     "ignore",
+				"image_publisher": "ignore",
+				"image_sku":       "ignore",
+				"location":        "ignore",
+				"subscription_id": "ignore",
+				"communicator":    "none",
+				"os_type":         constants.Target_Linux,
+				"shared_image_gallery_destination": map[string]interface{}{
+					"resource_group":          "ignore",
+					"gallery_name":            "ignore",
+					"image_name":              "ignore",
+					"image_version":           "1.0.1",
+					"replication_regions":     "ignore",
+					"use_shallow_replication": "true",
+				},
+			},
+		},
+		{
+			name: "with target region",
+			config: map[string]interface{}{
+				"image_offer":     "ignore",
+				"image_publisher": "ignore",
+				"image_sku":       "ignore",
+				"location":        "ignore",
+				"subscription_id": "ignore",
+				"communicator":    "none",
+				"os_type":         constants.Target_Linux,
+				"shared_image_gallery_destination": map[string]interface{}{
+					"resource_group": "ignore",
+					"gallery_name":   "ignore",
+					"image_name":     "ignore",
+					"image_version":  "1.0.1",
+					"target_region": map[string]interface{}{
+						"name": "ignore",
+					},
+					"use_shallow_replication": "true",
+				},
+			},
+		},
+		{
+			name:          "with multiple replication regions",
+			errorExpected: true,
+			config: map[string]interface{}{
+				"image_offer":     "ignore",
+				"image_publisher": "ignore",
+				"image_sku":       "ignore",
+				"location":        "ignore",
+				"subscription_id": "ignore",
+				"communicator":    "none",
+				"os_type":         constants.Target_Linux,
+				"shared_image_gallery_destination": map[string]interface{}{
+					"resource_group":          "ignore",
+					"gallery_name":            "ignore",
+					"image_name":              "ignore",
+					"image_version":           "1.0.1",
+					"replication_regions":     []string{"one", "two"},
+					"use_shallow_replication": "true",
+				},
+			},
+		},
+		{
+			name:          "with multiple target region",
+			errorExpected: true,
+			config: map[string]interface{}{
+				"image_offer":     "ignore",
+				"image_publisher": "ignore",
+				"image_sku":       "ignore",
+				"location":        "ignore",
+				"subscription_id": "ignore",
+				"communicator":    "none",
+				"os_type":         constants.Target_Linux,
+				"shared_image_gallery_destination": map[string]interface{}{
+					"resource_group": "ignore",
+					"gallery_name":   "ignore",
+					"image_name":     "ignore",
+					"image_version":  "1.0.1",
+					"target_region": map[string]interface{}{
+						"name":  "ignore",
+						"name2": "ignore",
+					},
+					"use_shallow_replication": "true",
+				},
+			},
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			var c Config
+			_, err := c.Prepare(tc.config, getPackerConfiguration())
+			if !tc.errorExpected && err != nil {
+				t.Errorf("unexpected error returned when validating shallow replication regions: %s", err)
+			}
+			if tc.errorExpected && err == nil {
+				t.Errorf("expected an error but got none for %s", tc.name)
+			}
+		})
+	}
+}
+
 func TestConfigShouldRejectManagedImageOSDiskSnapshotNameAndManagedImageDataDiskSnapshotPrefixWithCaptureContainerName(t *testing.T) {
 	config := map[string]interface{}{
 		"image_offer":                         "ignore",