Merge pull request #27082 from vespa-engine/hakonhall/validate-deserialization-instead-of-serialization

Validate deserialization instead of serialization

2 files changed, 35 insertions, 6 deletions

diff --git a/flags/src/main/java/com/yahoo/vespa/flags/UnboundStringFlag.java b/flags/src/main/java/com/yahoo/vespa/flags/UnboundStringFlag.java
index 9c69e917fa6..a7b90885363 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/UnboundStringFlag.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/UnboundStringFlag.java
@@ -27,12 +27,13 @@ public class UnboundStringFlag extends UnboundFlagImpl<String, StringFlag, Unbou
 
     public UnboundStringFlag(FlagId id, String defaultValue, FetchVector fetchVector, Predicate<String> validator) {
         this(id, defaultValue, fetchVector,
-             new SimpleFlagSerializer<>(stringValue -> {
-                                            if (!validator.test(stringValue))
-                                                throw new IllegalArgumentException("Invalid value: '" + stringValue + "'");
-                                            return new TextNode(stringValue);
-                                        },
-                                        JsonNode::isTextual, JsonNode::asText));
+             new SimpleFlagSerializer<>(TextNode::new,
+                                        JsonNode::isTextual,
+                                        jsonNode -> {
+                                            if (!validator.test(jsonNode.asText()))
+                                                throw new IllegalArgumentException("Invalid value: '" + jsonNode.asText() + "'");
+                                            return jsonNode.asText();
+                                        }));
     }
 
     public UnboundStringFlag(FlagId id, String defaultValue, FetchVector defaultFetchVector,
diff --git a/flags/src/test/java/com/yahoo/vespa/flags/PermanentFlagsTest.java b/flags/src/test/java/com/yahoo/vespa/flags/PermanentFlagsTest.java
index 903b0d899e2..c4fa8ac7e1b 100644
--- a/flags/src/test/java/com/yahoo/vespa/flags/PermanentFlagsTest.java
+++ b/flags/src/test/java/com/yahoo/vespa/flags/PermanentFlagsTest.java
@@ -3,11 +3,15 @@ package com.yahoo.vespa.flags;
 
 import com.yahoo.vespa.flags.custom.HostResources;
 import com.yahoo.vespa.flags.custom.SharedHost;
+import com.yahoo.vespa.flags.json.FlagData;
 import org.junit.jupiter.api.Test;
 
 import java.util.List;
 
 import static com.yahoo.vespa.flags.FlagsTest.testGeneric;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * @author bjorncs
@@ -21,4 +25,28 @@ class PermanentFlagsTest {
         testGeneric(PermanentFlags.SHARED_HOST, sharedHost);
     }
 
+    @Test
+    void wantedDockerTagValidation() {
+        IllegalArgumentException illegalArgumentException = assertThrows(
+                IllegalArgumentException.class,
+                () -> wantedDockerTagFlagData("do.not.work").validate(PermanentFlags.WANTED_DOCKER_TAG.serializer()));
+        assertThat(illegalArgumentException.getMessage(), equalTo("Invalid value: 'do.not.work'"));
+
+        wantedDockerTagFlagData("a").validate(PermanentFlags.WANTED_DOCKER_TAG.serializer());
+        wantedDockerTagFlagData("8.1.2").validate(PermanentFlags.WANTED_DOCKER_TAG.serializer());
+    }
+
+    private static FlagData wantedDockerTagFlagData(String value) {
+        return FlagData.deserialize("""
+                {
+                    "id": "wanted-docker-tag",
+                    "rules": [
+                       {
+                            "value": "%s"
+                        }
+                    ]
+                }
+                """.formatted(value));
+    }
+
 }