Remove sentinel null valued rules independent of any conditions

3 files changed, 58 insertions, 11 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 1c547fea8ba..f6ffd711f51 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
@@ -213,12 +213,14 @@ public class SystemFlagsDataArchive {
 
             String serializedData = flagData.serializeToJson();
             if (!JSON.equals(serializedData, normalizedRawData)) {
-                throw new IllegalArgumentException(filePath + " contains unknown non-comment fields: " +
-                        "after removing any comment fields the JSON is:\n  " +
-                        normalizedRawData +
-                        "\nbut deserializing this ended up with a JSON that are missing some of the fields:\n  " +
-                        serializedData +
-                        "\nSee https://git.ouroath.com/vespa/hosted-feature-flags for more info on the JSON syntax");
+                throw new IllegalArgumentException("""
+                                                   %s contains unknown non-comment fields or rules with null values: after removing any comment fields the JSON is:
+                                                     %s
+                                                   but deserializing this ended up with:
+                                                     %s
+                                                   These fields may be spelled wrong, or remove them?
+                                                   See https://git.ouroath.com/vespa/hosted-feature-flags for more info on the JSON syntax
+                                                   """.formatted(filePath, normalizedRawData, serializedData));
             }
         }
 
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 a24bed54a8a..f1e356eead2 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
@@ -146,11 +146,31 @@ public class SystemFlagsDataArchiveTest {
         Throwable exception = assertThrows(IllegalArgumentException.class, () -> {
             SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-unknown-field-name/"));
         });
-        assertTrue(exception.getMessage().contains("flags/my-test-flag/main.prod.us-west-1.json contains unknown non-comment fields: after removing any comment fields the JSON is:\n" +
-                "  {\"id\":\"my-test-flag\",\"rules\":[{\"condition\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\",\"values\":[\"foo.com\"]}],\"value\":\"default\"}]}\n" +
-                "but deserializing this ended up with a JSON that are missing some of the fields:\n" +
-                "  {\"id\":\"my-test-flag\",\"rules\":[{\"value\":\"default\"}]}\n" +
-                "See https://git.ouroath.com/vespa/hosted-feature-flags for more info on the JSON syntax"));
+        assertEquals("""
+                     flags/my-test-flag/main.prod.us-west-1.json contains unknown non-comment fields or rules with null values: after removing any comment fields the JSON is:
+                       {"id":"my-test-flag","rules":[{"condition":[{"type":"whitelist","dimension":"hostname","values":["foo.com"]}],"value":"default"}]}
+                     but deserializing this ended up with:
+                       {"id":"my-test-flag","rules":[{"value":"default"}]}
+                     These fields may be spelled wrong, or remove them?
+                     See https://git.ouroath.com/vespa/hosted-feature-flags for more info on the JSON syntax
+                     """,
+                     exception.getMessage());
+    }
+
+    @Test
+    void throws_on_null_value() {
+        Throwable exception = assertThrows(IllegalArgumentException.class, () -> {
+            SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-null-value/"));
+        });
+        assertEquals("""
+                     flags/my-test-flag/main.prod.us-west-1.json contains unknown non-comment fields or rules with null values: after removing any comment fields the JSON is:
+                       {"id":"my-test-flag","rules":[{"conditions":[{"type":"whitelist","dimension":"application","values":["a:b:c"]}],"value":null},{"conditions":[{"type":"whitelist","dimension":"hostname","values":["foo.com"]}],"value":true}]}
+                     but deserializing this ended up with:
+                       {"id":"my-test-flag","rules":[{"conditions":[{"type":"whitelist","dimension":"application","values":["a:b:c"]}]},{"conditions":[{"type":"whitelist","dimension":"hostname","values":["foo.com"]}],"value":true}]}
+                     These fields may be spelled wrong, or remove them?
+                     See https://git.ouroath.com/vespa/hosted-feature-flags for more info on the JSON syntax
+                     """,
+                     exception.getMessage());
     }
 
     @Test
diff --git a/controller-api/src/test/resources/system-flags-with-null-value/flags/my-test-flag/main.prod.us-west-1.json b/controller-api/src/test/resources/system-flags-with-null-value/flags/my-test-flag/main.prod.us-west-1.json
new file mode 100644
index 00000000000..75cffdea009
--- /dev/null
+++ b/controller-api/src/test/resources/system-flags-with-null-value/flags/my-test-flag/main.prod.us-west-1.json
@@ -0,0 +1,25 @@
+{
+    "id" : "my-test-flag",
+    "rules" : [
+        {
+            "conditions": [
+                {
+                    "type": "whitelist",
+                    "dimension": "application",
+                    "values": ["a:b:c"]
+                }
+            ],
+            "value" : null
+        },
+        {
+            "conditions": [
+                {
+                    "type": "whitelist",
+                    "dimension": "hostname",
+                    "values": ["foo.com"]
+                }
+            ],
+            "value" : true
+        }
+    ]
+}