Add comments and tests

hashicorp · Oct 16, 2023 · ff7d7b6 · ff7d7b6
1 parent 02fd8ae
commit ff7d7b6
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 2 deletions.
diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go
@@ -213,7 +213,7 @@ 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)
 		}
 
-		// SIG requires that replication regions include the region in which the source image resides
+		// SIG requires that replication regions include the region in which the created image version resides
 		buildLocation := normalizeAzureRegion(b.stateBag.Get(constants.ArmLocation).(string))
 		foundMandatoryReplicationRegion := false
 		var normalizedReplicationRegions []string
@@ -229,6 +229,10 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook)
 		if foundMandatoryReplicationRegion == false {
 			b.config.SharedGalleryDestination.SigDestinationReplicationRegions = append(normalizedReplicationRegions, buildLocation)
 		}
+		// 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 {
 			if len(normalizedReplicationRegions) != 1 || !foundMandatoryReplicationRegion {
 				return nil, fmt.Errorf("when `use_shallow_replication` is set the build region (from `location` or `build_resource_group_name`) can only be region set in `replicated_regions`")

diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go
@@ -1266,7 +1266,7 @@ func assertRequiredParametersSet(c *Config, errs *packersdk.MultiError) {
 
 		if c.SharedGalleryDestination.SigDestinationUseShallowReplicationMode {
 			// When the user doesn't set the replica count it uses the int null value (0) so we have to check for that
-			if c.SharedGalleryImageVersionReplicaCount != 1 || c.SharedGalleryImageVersionReplicaCount == 0 {
+			if c.SharedGalleryImageVersionReplicaCount != 1 && c.SharedGalleryImageVersionReplicaCount != 0 {
 				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"))
 			}
 		}

diff --git a/builder/azure/arm/config_test.go b/builder/azure/arm/config_test.go
@@ -1283,6 +1283,85 @@ func TestConfigShouldAcceptAbsentManagedImageButPresentSharedImageGalleryDestina
 	}
 }
 
+func TestConfigShouldRejectShallowReplicationWithInvalidReplicationCount(t *testing.T) {
+	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_replica_count": "2",
+		"shared_image_gallery_destination": map[string]string{
+			"resource_group":          "ignore",
+			"gallery_name":            "ignore",
+			"image_name":              "ignore",
+			"image_version":           "1.0.1",
+			"replication_regions":     "ignore",
+			"use_shallow_replication": "true",
+		},
+	}
+	expectedErrorMessage := "When using shallow replication the replica count can only be 1, leaving this value unset will default to 1"
+	var c Config
+	_, err := c.Prepare(config, getPackerConfiguration())
+	if err == nil {
+		t.Fatalf("expected config to reject invalid replica count using shallow replication but it was accepted")
+	} else if !strings.Contains(err.Error(), expectedErrorMessage) {
+		t.Fatalf("unexpected rejection reason, expected %s to contain %s", err.Error(), expectedErrorMessage)
+	}
+}
+
+func TestConfigShouldAcceptShallowReplicationWithWithSetReplicaCount(t *testing.T) {
+	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_replica_count": "1",
+		"shared_image_gallery_destination": map[string]string{
+			"resource_group":          "ignore",
+			"gallery_name":            "ignore",
+			"image_name":              "ignore",
+			"image_version":           "1.0.1",
+			"replication_regions":     "ignore",
+			"use_shallow_replication": "true",
+		},
+	}
+	var c Config
+	_, err := c.Prepare(config, getPackerConfiguration())
+	if err != nil {
+		t.Fatalf("expected config to accept shallow replication with set replica count (1) build: %v", err)
+	}
+}
+
+func TestConfigShouldAcceptShallowReplicationWithWithUnsetReplicaCount(t *testing.T) {
+	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]string{
+			"resource_group":          "ignore",
+			"gallery_name":            "ignore",
+			"image_name":              "ignore",
+			"image_version":           "1.0.1",
+			"replication_regions":     "ignore",
+			"use_shallow_replication": "true",
+		},
+	}
+	var c Config
+	_, err := c.Prepare(config, getPackerConfiguration())
+	if err != nil {
+		t.Fatalf("expected config to accept shallow replication with unset replica count build: %v", err)
+	}
+}
 func TestConfigShouldRejectManagedImageOSDiskSnapshotNameAndManagedImageDataDiskSnapshotPrefixWithCaptureContainerName(t *testing.T) {
 	config := map[string]interface{}{
 		"image_offer":                         "ignore",