Ignore comment fields

4 files changed, 68 insertions, 7 deletions

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 1817b3e6e50..6604da144b9 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
@@ -1,6 +1,9 @@
 // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.api.systemflags.v1;
 
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.text.JSON;
 import com.yahoo.vespa.flags.FlagId;
@@ -39,6 +42,8 @@ import static com.yahoo.yolean.Exceptions.uncheck;
  */
 public class SystemFlagsDataArchive {
 
+    private static final ObjectMapper mapper = new ObjectMapper();
+
     private final Map<FlagId, Map<String, FlagData>> files;
 
     private SystemFlagsDataArchive(Map<FlagId, Map<String, FlagData>> files) {
@@ -152,15 +157,34 @@ public class SystemFlagsDataArchive {
             if (!directoryDeducedFlagId.equals(flagData.id())) {
                 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());
+                                flagData.id(), directoryDeducedFlagId.toString()));
+            }
+
+            String serializedData = flagData.serializeToJson();
+            String normalizedRawData = removeCommentsFromJson(rawData);
+            if (!JSON.equals(serializedData, normalizedRawData)) {
+                throw new IllegalArgumentException(filePath + " contains unknown non-comment fields: " +
+                        "was deserialized to " + serializedData);
             }
         }
         builder.addFile(filename, flagData);
     }
 
+    static String removeCommentsFromJson(String json) {
+        JsonNode jsonNode = uncheck(() -> mapper.readTree(json));
+        removeComments(jsonNode);
+        return jsonNode.toString();
+    }
+
+    private static void removeComments(JsonNode node) {
+        if (node instanceof ObjectNode) {
+            ObjectNode objectNode = (ObjectNode) node;
+            objectNode.remove("comment");
+        }
+
+        node.forEach(SystemFlagsDataArchive::removeComments);
+    }
+
     private static String toFilePath(FlagId flagId, String filename) {
         return "flags/" + flagId.toString() + "/" + filename;
     }
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 51683a4a82f..5ff1cbf7530 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
@@ -6,6 +6,7 @@ import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.zone.ZoneId;
+import com.yahoo.text.JSON;
 import com.yahoo.vespa.athenz.api.AthenzService;
 import com.yahoo.vespa.flags.FetchVector;
 import com.yahoo.vespa.flags.FlagId;
@@ -32,6 +33,7 @@ import java.util.Set;
 
 import static java.util.stream.Collectors.toList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertTrue;
 
 /**
  * @author bjorncs
@@ -107,8 +109,40 @@ public class SystemFlagsDataArchiveTest {
     @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/"));
+        expectedException.expectMessage("flags/my-test-flag/main.prod.us-west-1.json contains unknown non-comment fields: was deserialized to {\"id\":\"my-test-flag\",\"rules\":[{\"value\":\"default\"}]}");
+        SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-unknown-field-name/"));
+    }
+
+    @Test
+    public void remove_comments() {
+        assertTrue(JSON.equals("{\n" +
+                "    \"a\": {\n" +
+                "        \"b\": 1\n" +
+                "    },\n" +
+                "    \"list\": [\n" +
+                "        {\n" +
+                "            \"c\": 2\n" +
+                "        },\n" +
+                "        {\n" +
+                "        }\n" +
+                "    ]\n" +
+                "}",
+                SystemFlagsDataArchive.removeCommentsFromJson("{\n" +
+                "    \"comment\": \"comment a\",\n" +
+                "    \"a\": {\n" +
+                "        \"comment\": \"comment b\",\n" +
+                "        \"b\": 1\n" +
+                "    },\n" +
+                "    \"list\": [\n" +
+                "        {\n" +
+                "            \"comment\": \"comment c\",\n" +
+                "            \"c\": 2\n" +
+                "        },\n" +
+                "        {\n" +
+                "            \"comment\": \"comment d\"\n" +
+                "        }\n" +
+                "    ]\n" +
+                "}")));
     }
 
     private static void assertArchiveReturnsCorrectTestFlagDataForTarget(SystemFlagsDataArchive archive) {
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 7b9e785b6b4..c9b46d68ace 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,3 +1,4 @@
 {
-  "id" : "flag-with-empty-data"
+  "id" : "flag-with-empty-data",
+  "comment": "empty data using only id field"
 }
\ No newline at end of file
diff --git a/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.json b/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.json
index cef75be02b7..29160dc0081 100644
--- a/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.json
+++ b/controller-api/src/test/resources/system-flags/flags/flag-with-empty-data/main.json
@@ -1,7 +1,9 @@
 {
+  "comment": "comment a",
   "id" : "flag-with-empty-data",
   "rules" : [
     {
+      "comment": "comment b",
       "value" : "main"
     }
   ]