Repository: gchq/sleeper
Issue: Deployment fails if duplicate x86 or arm64 instances are defined in properties

### Description

CDK fails to configure the instance fleets if duplicate instances are defined in x86 or arm64 instance properties. 

An example error is below:
`The instance fleet: Driver contains duplicate instance types [m6g.xlarge]`

### Steps to reproduce

1. Set one of the x86 or arm64 instance properties to contain duplicates in the list
2. Observe deployment fails

### Expected behaviour

Duplicates should be disallowed in these properties. We can add a validation rule to do this.


Changed files:
java/configuration/src/test/java/sleeper/configuration/UtilsTest.java
java/configuration/src/main/java/sleeper/configuration/properties/instance/PersistentEMRProperty.java
java/configuration/src/main/java/sleeper/configuration/properties/instance/NonPersistentEMRProperty.java
java/configuration/src/main/java/sleeper/configuration/Utils.java

diff --git a/java/configuration/src/main/java/sleeper/configuration/Utils.java b/java/configuration/src/main/java/sleeper/configuration/Utils.java
index 891b89e87..7753af077 100644
--- a/java/configuration/src/main/java/sleeper/configuration/Utils.java
+++ b/java/configuration/src/main/java/sleeper/configuration/Utils.java
@@ -22,7 +22,6 @@ import sleeper.configuration.properties.SleeperProperties;
 import sleeper.configuration.properties.table.CompressionCodec;
 import sleeper.configuration.properties.validation.EmrInstanceArchitecture;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 import java.util.function.DoublePredicate;
@@ -159,10 +158,12 @@ public class Utils {
                 .allMatch(architecture -> EnumUtils.isValidEnumIgnoreCase(EmrInstanceArchitecture.class, architecture));
     }
 
-    public static <T, A extends T, B extends T> List<T> combineLists(List<A> list1, List<B> list2) {
-        List<T> combinedList = new ArrayList<>(list1);
-        combinedList.addAll(list2);
-        return combinedList;
+    public static boolean isUniqueList(String input) {
+        if (input == null) {
+            return false;
+        }
+        List<String> inputList = SleeperProperties.readList(input);
+        return inputList.stream().distinct().count() == inputList.size();
     }
 
     private static boolean parseAndCheckInteger(String string, IntPredicate check) {
diff --git a/java/configuration/src/main/java/sleeper/configuration/properties/instance/NonPersistentEMRProperty.java b/java/configuration/src/main/java/sleeper/configuration/properties/instance/NonPersistentEMRProperty.java
index 1be1ff384..70824e5e8 100644
--- a/java/configuration/src/main/java/sleeper/configuration/properties/instance/NonPersistentEMRProperty.java
+++ b/java/configuration/src/main/java/sleeper/configuration/properties/instance/NonPersistentEMRProperty.java
@@ -42,24 +42,28 @@ public interface NonPersistentEMRProperty {
                     "node of the EMR cluster. " +
                     "For more information, see the Bulk import using EMR - Instance types section in docs/05-ingest.md")
             .defaultValue("m6i.xlarge")
+            .validationPredicate(Utils::isUniqueList)
             .propertyGroup(InstancePropertyGroup.BULK_IMPORT).build();
     UserDefinedInstanceProperty DEFAULT_BULK_IMPORT_EMR_EXECUTOR_X86_INSTANCE_TYPES = Index.propertyBuilder("sleeper.default.bulk.import.emr.executor.x86.instance.types")
             .description("(Non-persistent EMR mode only) The default EC2 x86_64 instance types to be used for the executor " +
                     "nodes of the EMR cluster. " +
                     "For more information, see the Bulk import using EMR - Instance types section in docs/05-ingest.md")
             .defaultValue("m6i.4xlarge")
+            .validationPredicate(Utils::isUniqueList)
             .propertyGroup(InstancePropertyGroup.BULK_IMPORT).build();
     UserDefinedInstanceProperty DEFAULT_BULK_IMPORT_EMR_MASTER_ARM_INSTANCE_TYPES = Index.propertyBuilder("sleeper.default.bulk.import.emr.master.arm.instance.types")
             .description("(Non-persistent EMR mode only) The default EC2 ARM64 instance types to be used for the master " +
                     "node of the EMR cluster. " +
                     "For more information, see the Bulk import using EMR - Instance types section in docs/05-ingest.md")
             .defaultValue("m6g.xlarge")
+            .validationPredicate(Utils::isUniqueList)
             .propertyGroup(InstancePropertyGroup.BULK_IMPORT).build();
     UserDefinedInstanceProperty DEFAULT_BULK_IMPORT_EMR_EXECUTOR_ARM_INSTANCE_TYPES = Index.propertyBuilder("sleeper.default.bulk.import.emr.executor.arm.instance.types")
             .description("(Non-persistent EMR mode only) The default EC2 ARM64 instance types to be used for the executor " +
                     "nodes of the EMR cluster. " +
                     "For more information, see the Bulk import using EMR - Instance types section in docs/05-ingest.md")
             .defaultValue("m6g.4xlarge")
+            .validationPredicate(Utils::isUniqueList)
             .propertyGroup(InstancePropertyGroup.BULK_IMPORT).build();
     UserDefinedInstanceProperty DEFAULT_BULK_IMPORT_EMR_EXECUTOR_MARKET_TYPE = Index.propertyBuilder("sleeper.default.bulk.import.emr.executor.market.type")
             .description("(Non-persistent EMR mode only) The default purchasing option to be used for the executor " +
diff --git a/java/configuration/src/main/java/sleeper/configuration/properties/instance/PersistentEMRProperty.java b/java/configuration/src/main/java/sleeper/configuration/properties/instance/PersistentEMRProperty.java
index e76822c68..c31c79c71 100644
--- a/java/configuration/src/main/java/sleeper/configuration/properties/instance/PersistentEMRProperty.java
+++ b/java/configuration/src/main/java/sleeper/configuration/properties/instance/PersistentEMRProperty.java
@@ -46,6 +46,7 @@ public interface PersistentEMRProperty {
                     "persistent EMR cluster. " +
                     "For more information, see the Bulk import using EMR - Instance types section in docs/05-ingest.md")
             .defaultValue(DEFAULT_BULK_IMPORT_EMR_MASTER_X86_INSTANCE_TYPES.getDefaultValue())
+            .validationPredicate(Utils::isUniqueList)
             .propertyGroup(InstancePropertyGroup.BULK_IMPORT)
             .runCDKDeployWhenChanged(true).build();
     UserDefinedInstanceProperty BULK_IMPORT_PERSISTENT_EMR_EXECUTOR_X86_INSTANCE_TYPES = Index.propertyBuilder("sleeper.bulk.import.persistent.emr.executor.x86.instance.types")
@@ -53,6 +54,7 @@ public interface PersistentEMRProperty {
                     "persistent EMR cluster. " +
                     "For more information, see the Bulk import using EMR - Instance types section in docs/05-ingest.md")
             .defaultValue(DEFAULT_BULK_IMPORT_EMR_EXECUTOR_X86_INSTANCE_TYPES.getDefaultValue())
+            .validationPredicate(Utils::isUniqueList)
             .propertyGroup(InstancePropertyGroup.BULK_IMPORT)
             .runCDKDeployWhenChanged(true).build();
     UserDefinedInstanceProperty BULK_IMPORT_PERSISTENT_EMR_MASTER_ARM_INSTANCE_TYPES = Index.propertyBuilder("sleeper.bulk.import.persistent.emr.master.arm.instance.types")
@@ -60,6 +62,7 @@ public interface PersistentEMRProperty {
                     "persistent EMR cluster. " +
                     "For more information, see the Bulk import using EMR - Instance types section in docs/05-ingest.md")
             .defaultValue(DEFAULT_BULK_IMPORT_EMR_MASTER_ARM_INSTANCE_TYPES.getDefaultValue())
+            .validationPredicate(Utils::isUniqueList)
             .propertyGroup(InstancePropertyGroup.BULK_IMPORT)
             .runCDKDeployWhenChanged(true).build();
     UserDefinedInstanceProperty BULK_IMPORT_PERSISTENT_EMR_EXECUTOR_ARM_INSTANCE_TYPES = Index.propertyBuilder("sleeper.bulk.import.persistent.emr.executor.arm.instance.types")
@@ -67,6 +70,7 @@ public interface PersistentEMRProperty {
                     "persistent EMR cluster. " +
                     "For more information, see the Bulk import using EMR - Instance types section in docs/05-ingest.md")
             .defaultValue(DEFAULT_BULK_IMPORT_EMR_EXECUTOR_ARM_INSTANCE_TYPES.getDefaultValue())
+            .validationPredicate(Utils::isUniqueList)
             .propertyGroup(InstancePropertyGroup.BULK_IMPORT)
             .runCDKDeployWhenChanged(true).build();
     UserDefinedInstanceProperty BULK_IMPORT_PERSISTENT_EMR_USE_MANAGED_SCALING = Index.propertyBuilder("sleeper.bulk.import.persistent.emr.use.managed.scaling")
diff --git a/java/configuration/src/test/java/sleeper/configuration/UtilsTest.java b/java/configuration/src/test/java/sleeper/configuration/UtilsTest.java
index 5fb256685..1b2920d67 100644
--- a/java/configuration/src/test/java/sleeper/configuration/UtilsTest.java
+++ b/java/configuration/src/test/java/sleeper/configuration/UtilsTest.java
@@ -16,88 +16,97 @@
 
 package sleeper.configuration;
 
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 
-import java.util.List;
-
 import static org.assertj.core.api.Assertions.assertThat;
-import static sleeper.configuration.Utils.combineLists;
 
 class UtilsTest {
 
-    @Test
-    void shouldCombineLists() {
-        // Given
-        List<String> list1 = List.of("test1", "test2");
-        List<String> list2 = List.of("test3", "test4");
-
-        // When
-        List<String> combinedList = combineLists(list1, list2);
-
-        // Then
-        assertThat(combinedList)
-                .containsExactly("test1", "test2", "test3", "test4");
-    }
-
-    @Test
-    void shouldNotThrowExceptionDuringPositiveIntegerCheck() {
-        // When/Then
-        assertThat(Utils.isPositiveInteger("123"))
-                .isTrue();
-        assertThat(Utils.isPositiveInteger("ABC"))
-                .isFalse();
-    }
-
-    @Test
-    void shouldNotThrowExceptionDuringPositiveLongCheck() {
-        // When/Then
-        assertThat(Utils.isPositiveLong("123"))
-                .isTrue();
-        assertThat(Utils.isPositiveLong("ABC"))
-                .isFalse();
-    }
-
-    @Test
-    void shouldNotThrowExceptionDuringPositiveDoubleCheck() {
-        // When/Then
-        assertThat(Utils.isPositiveDouble("123"))
-                .isTrue();
-        assertThat(Utils.isPositiveDouble("ABC"))
-                .isFalse();
-    }
-
-    @Test
-    void shouldReadBytesSize() {
-        assertThat(Utils.readBytes("42")).isEqualTo(42L);
-    }
-
-    @Test
-    void shouldReadKilobytesSize() {
-        assertThat(Utils.readBytes("2K")).isEqualTo(2048L);
-    }
-
-    @Test
-    void shouldReadMegabytesSize() {
-        assertThat(Utils.readBytes("2M")).isEqualTo(2 * 1024 * 1024L);
-    }
-
-    @Test
-    void shouldReadGigabytesSize() {
-        assertThat(Utils.readBytes("2G")).isEqualTo(2 * 1024 * 1024 * 1024L);
-    }
-
-    @Test
-    void shouldReadTerabytesSize() {
-        assertThat(Utils.readBytes("2T")).isEqualTo(2 * 1024 * 1024 * 1024L * 1024L);
+    @Nested
+    @DisplayName("Validate lists")
+    class ValidateLists {
+        @Test
+        void shouldValidateListWithUniqueElements() {
+            assertThat(Utils.isUniqueList("test-a,test-b,test-c"))
+                    .isTrue();
+        }
+
+        @Test
+        void shouldFailToValidateListWithDuplicates() {
+            assertThat(Utils.isUniqueList("test-a,test-b,test-a"))
+                    .isFalse();
+        }
     }
 
-    @Test
-    void shouldReadPetabytesSize() {
-        assertThat(Utils.readBytes("2P")).isEqualTo(2 * 1024 * 1024 * 1024L * 1024L * 1024L);
+    @Nested
+    @DisplayName("Validate numbers")
+    class ValidateNumbers {
+        @Test
+        void shouldNotThrowExceptionDuringPositiveIntegerCheck() {
+            // When/Then
+            assertThat(Utils.isPositiveInteger("123"))
+                    .isTrue();
+            assertThat(Utils.isPositiveInteger("ABC"))
+                    .isFalse();
+        }
+
+        @Test
+        void shouldNotThrowExceptionDuringPositiveLongCheck() {
+            // When/Then
+            assertThat(Utils.isPositiveLong("123"))
+                    .isTrue();
+            assertThat(Utils.isPositiveLong("ABC"))
+                    .isFalse();
+        }
+
+        @Test
+        void shouldNotThrowExceptionDuringPositiveDoubleCheck() {
+            // When/Then
+            assertThat(Utils.isPositiveDouble("123"))
+                    .isTrue();
+            assertThat(Utils.isPositiveDouble("ABC"))
+                    .isFalse();
+        }
     }
 
-    @Test
-    void shouldReadExabytesSize() {
-        assertThat(Utils.readBytes("2E")).isEqualTo(2 * 1024 * 1024 * 1024L * 1024L * 1024L * 1024L);
+    @Nested
+    @DisplayName("Validate bytes size")
+    class ValidateBytesSize {
+        @Test
+        void shouldReadBytesSize() {
+            assertThat(Utils.readBytes("42")).isEqualTo(42L);
+        }
+
+        @Test
+        void shouldReadKilobytesSize() {
+            assertThat(Utils.readBytes("2K")).isEqualTo(2048L);
+        }
+
+        @Test
+        void shouldReadMegabytesSize() {
+            assertThat(Utils.readBytes("2M")).isEqualTo(2 * 1024 * 1024L);
+        }
+
+        @Test
+        void shouldReadGigabytesSize() {
+            assertThat(Utils.readBytes("2G")).isEqualTo(2 * 1024 * 1024 * 1024L);
+        }
+
+        @Test
+        void shouldReadTerabytesSize() {
+            assertThat(Utils.readBytes("2T")).isEqualTo(2 * 1024 * 1024 * 1024L * 1024L);
+        }
+
+        @Test
+        void shouldReadPetabytesSize() {
+            assertThat(Utils.readBytes("2P")).isEqualTo(2 * 1024 * 1024 * 1024L * 1024L * 1024L);
+        }
+
+        @Test
+        void shouldReadExabytesSize() {
+            assertThat(Utils.readBytes("2E")).isEqualTo(2 * 1024 * 1024 * 1024L * 1024L * 1024L * 1024L);
+        }
     }
 }