Validate deserialization of flag data

5 files changed, 34 insertions, 2 deletions

diff --git a/controller-api/pom.xml b/controller-api/pom.xml
index 5c744db2d6d..5d0bbff1c62 100644
--- a/controller-api/pom.xml
+++ b/controller-api/pom.xml
@@ -53,6 +53,13 @@
             <version>${project.version}</version>
         </dependency>
 
+        <dependency>
+            <groupId>com.yahoo.vespa</groupId>
+            <artifactId>vespajlib</artifactId>
+            <scope>provided</scope>
+            <version>${project.version}</version>
+        </dependency>
+
         <!-- compile -->
 
         <dependency>
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java
index 0cb4baab790..1817b3e6e50 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.controller.api.systemflags.v1;
 
 import com.yahoo.config.provision.SystemName;
+import com.yahoo.text.JSON;
 import com.yahoo.vespa.flags.FlagId;
 import com.yahoo.vespa.flags.json.FlagData;
 
@@ -152,6 +153,9 @@ public class SystemFlagsDataArchive {
                 throw new IllegalArgumentException(
                         String.format("Flag data file with flag id '%s' in directory for '%s'",
                                       flagData.id(), directoryDeducedFlagId.toString()));
+            } else if (!JSON.equals(flagData.serializeToJson(), rawData)) {
+                throw new IllegalArgumentException("Failed to reconstruct the original JSON at " +
+                        filePath + ", got: " + flagData.serializeToJson());
             }
         }
         builder.addFile(filename, flagData);
diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java
index 6e78b8da91c..51683a4a82f 100644
--- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java
+++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java
@@ -104,6 +104,13 @@ public class SystemFlagsDataArchiveTest {
         archive.validateAllFilesAreForTargets(SystemName.main, Set.of(mainControllerTarget, prodUsWestCfgTarget));
     }
 
+    @Test
+    public void throws_on_unknown_field() {
+        expectedException.expect(IllegalArgumentException.class);
+        expectedException.expectMessage("Failed to reconstruct the original JSON at flags/my-test-flag/main.prod.us-west-1.json, got: {\"id\":\"my-test-flag\",\"rules\":[{\"value\":\"default\"}]}");
+        SystemFlagsDataArchive archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-unknown-field-name/"));
+    }
+
     private static void assertArchiveReturnsCorrectTestFlagDataForTarget(SystemFlagsDataArchive archive) {
         assertFlagDataHasValue(archive, MY_TEST_FLAG, mainControllerTarget, "main.controller");
         assertFlagDataHasValue(archive, MY_TEST_FLAG, prodUsWestCfgTarget, "main.prod.us-west-1");
diff --git a/controller-api/src/test/resources/system-flags-with-unknown-field-name/flags/my-test-flag/main.prod.us-west-1.json b/controller-api/src/test/resources/system-flags-with-unknown-field-name/flags/my-test-flag/main.prod.us-west-1.json
new file mode 100644
index 00000000000..c41083fc7ab
--- /dev/null
+++ b/controller-api/src/test/resources/system-flags-with-unknown-field-name/flags/my-test-flag/main.prod.us-west-1.json
@@ -0,0 +1,15 @@
+{
+    "id" : "my-test-flag",
+    "rules" : [
+        {
+            "condition": [
+                {
+                    "type": "whitelist",
+                    "dimension": "hostname",
+                    "values": ["foo.com"]
+                }
+            ],
+            "value" : "default"
+        }
+    ]
+}
diff --git a/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.controller.json b/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.controller.json
index c9b46d68ace..7b9e785b6b4 100644
--- a/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.controller.json
+++ b/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.controller.json
@@ -1,4 +1,3 @@
 {
-  "id" : "flag-with-empty-data",
-  "comment": "empty data using only id field"
+  "id" : "flag-with-empty-data"
 }
\ No newline at end of file