6439: Throw clearer exception when tags malformed

java/core/src/main/java/sleeper/core/properties/instance/CommonProperty.java

@@ -74,7 +74,9 @@ public interface CommonProperty {
                     "then the list should be \"user,some-user,project-name,sleeper-test\".\n" +
                     "Preferably, tags should be specified in a separate file called tags.properties.\n" +
                     "See https://github.com/gchq/sleeper/blob/develop/docs/deployment/instance-configuration.md for further details.")
-            .propertyGroup(InstancePropertyGroup.COMMON).runCdkDeployWhenChanged(true)
+            .propertyGroup(InstancePropertyGroup.COMMON)
+            .validationPredicate(SleeperPropertyValueUtils::isListInKeyValueFormat)
+            .runCdkDeployWhenChanged(true)
             .includedInTemplate(false).build();
     UserDefinedInstanceProperty STACK_TAG_NAME = Index.propertyBuilder("sleeper.stack.tag.name")
             .description("A name for a tag to identify the stack that deployed a resource. This will be set for all AWS resources, to the ID of " +

java/core/src/main/java/sleeper/core/properties/instance/InstanceProperties.java

@@ -203,8 +203,10 @@ public static Map<String, String> csvTagsToMap(String csvTags) {
         Map<String, String> tags = new HashMap<>();
         if (null != csvTags && !csvTags.isEmpty()) {
             String[] split = csvTags.split(",");
-            for (int i = 0; i < split.length; i += 2) {
-                tags.put(split[i], split[i + 1]);
+            if (split.length % 2 == 0) { //Ensure matching number of keys and values
+                for (int i = 0; i < split.length; i += 2) {
+                    tags.put(split[i], split[i + 1]);
+                }
             }
         }
         return tags;

java/core/src/main/java/sleeper/core/properties/model/SleeperPropertyValueUtils.java

@@ -341,6 +341,17 @@ public static boolean isListWithMaxSize(String input, int maxSize) {
         return values.size() <= maxSize;
     }
 
+    /**
+     * Checks if a property value is a comma-separated list with a matching number of key and value pairs.
+     *
+     * @param  input the value
+     * @return       true if the value meets the requirement
+     */
+    public static boolean isListInKeyValueFormat(String input) {
+        List<String> values = SleeperPropertyValueUtils.readList(input);
+        return values.size() % 2 == 0; //Expect even amount of items when in key-value format
+    }
+
     private static boolean parseAndCheckInteger(String string, IntPredicate check) {
         try {
             return check.test(Integer.parseInt(string));

java/core/src/test/java/sleeper/core/properties/instance/InstancePropertiesTest.java

@@ -17,6 +17,8 @@
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import sleeper.core.properties.SleeperPropertiesInvalidException;
 
@@ -58,6 +60,7 @@
 import static sleeper.core.properties.instance.CommonProperty.LOG_RETENTION_IN_DAYS;
 import static sleeper.core.properties.instance.CommonProperty.MAXIMUM_CONNECTIONS_TO_S3;
 import static sleeper.core.properties.instance.CommonProperty.SUBNETS;
+import static sleeper.core.properties.instance.CommonProperty.TAGS;
 import static sleeper.core.properties.instance.CommonProperty.TASK_RUNNER_LAMBDA_MEMORY_IN_MB;
 import static sleeper.core.properties.instance.CommonProperty.TASK_RUNNER_LAMBDA_TIMEOUT_IN_SECONDS;
 import static sleeper.core.properties.instance.CommonProperty.VPC_ID;
@@ -250,6 +253,31 @@ void shouldValidatePredicateOfTheDefaultPropertyVersusWhatIsSet() {
                 .hasMessageContaining("-62");
     }
 
+    @ParameterizedTest(name = "Create with invalid Tags: {0}")
+    @ValueSource(strings = {"key1=value1", "key1,value1,key2=value2"})
+    void shouldCreateInstancePropertiesFineWithInvalidTags(String tags) {
+        // Given / When
+        InstanceProperties instanceProperties = InstanceProperties.createWithoutValidation(
+                loadProperties("sleeper.tags=" + tags));
+
+        // Then
+        assertThat(instanceProperties.get(TAGS)).isEqualTo(tags);
+        assertThat(instanceProperties.getTags()).isEqualTo(Map.of());
+    }
+
+    @ParameterizedTest(name = "Fail validation with invalid Tags: {0}")
+    @ValueSource(strings = {"key1=value1", "key1,value1,key2=value2"})
+    void shouldFailValidationForInstancePropertiesWithInvalidTags(String tags) {
+        // Given
+        InstanceProperties properties = createTestInstanceProperties();
+        properties.set(TAGS, tags);
+
+        // When /  Then
+        assertThatThrownBy(() -> properties.validate())
+                .isInstanceOf(SleeperPropertiesInvalidException.class)
+                .hasMessage("Property sleeper.tags was invalid. It was \"" + tags + "\".");
+    }
+
     private static InstanceProperties getSleeperProperties() {
         InstanceProperties instanceProperties = new InstanceProperties();
         instanceProperties.set(ACCOUNT, "1234567890");

java/core/src/test/java/sleeper/core/properties/model/SleeperPropertyValueUtilsTest.java

@@ -166,6 +166,21 @@ void shouldValidateStringWhenListSizeMeetsMaxSize() {
         void shouldFailToValidateStringWhenListSizeExceedsMaxSize() {
             assertThat(SleeperPropertyValueUtils.isListWithMaxSize("a,b,c,d,e", 4)).isFalse();
         }
+
+        @Test
+        void shouldValidateEmptyListToBeInKeyValueFormat() {
+            assertThat(SleeperPropertyValueUtils.isListInKeyValueFormat("")).isTrue();
+        }
+
+        @Test
+        void shouldValidateListInKeyValueFormat() {
+            assertThat(SleeperPropertyValueUtils.isListInKeyValueFormat("key,value,key,value")).isTrue();
+        }
+
+        @Test
+        void shouldFailToValidateListNotInKeyValueFormat() {
+            assertThat(SleeperPropertyValueUtils.isListInKeyValueFormat("key=value")).isFalse();
+        }
     }
 
 }