Merge pull request #13977 from vespa-engine/frodelu/multi-level-flagdirs

Support multiple levels of directories for flags

6 files changed, 62 insertions, 1 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 e6310cc6432..a00992da815 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
@@ -45,6 +45,11 @@ import static com.yahoo.yolean.Exceptions.uncheck;
  * The flag files must reside in a 'flags/' root directory containing a directory for each flag name:
  * {@code ./flags/<flag-id>/*.json}
  *
+ * Optionally, there can be an arbitrary number of directories "between" 'flags/' root directory and
+ * the flag name directory:
+ * {@code ./flags/onelevel/<flag-id>/*.json}
+ * {@code ./flags/onelevel/anotherlevel/<flag-id>/*.json}
+ *
  * @author bjorncs
  */
 public class SystemFlagsDataArchive {
@@ -155,7 +160,7 @@ public class SystemFlagsDataArchive {
         if (!filename.endsWith(".json")) {
             throw new IllegalArgumentException(String.format("Only JSON files are allowed in 'flags/' directory (found '%s')", filePath.toString()));
         }
-        FlagId directoryDeducedFlagId = new FlagId(filePath.getName(1).toString());
+        FlagId directoryDeducedFlagId = new FlagId(filePath.getName(filePath.getNameCount()-2).toString());
         FlagData flagData;
         if (rawData.isBlank()) {
             flagData = new FlagData(directoryDeducedFlagId);
@@ -178,6 +183,13 @@ public class SystemFlagsDataArchive {
                         "\nSee https://git.ouroath.com/vespa/hosted-feature-flags for more info on the JSON syntax");
             }
         }
+
+        if (builder.hasFile(filename, flagData)) {
+            throw new IllegalArgumentException(
+                String.format("Flag data file in '%s' contains redundant flag data for id '%s' already set in another directory!",
+                              filePath, flagData.id()));
+        }
+
         builder.addFile(filename, flagData);
     }
 
@@ -236,6 +248,10 @@ public class SystemFlagsDataArchive {
             return this;
         }
 
+        public boolean hasFile(String filename, FlagData data) {
+            return files.containsKey(data.id()) && files.get(data.id()).containsKey(filename);
+        }
+
         public SystemFlagsDataArchive build() {
             Map<FlagId, Map<String, FlagData>> copy = new TreeMap<>();
             files.forEach((flagId, map) -> copy.put(flagId, new TreeMap<>(map)));
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 4cdbe5241bc..21e89cb5ea3 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
@@ -84,6 +84,19 @@ public class SystemFlagsDataArchiveTest {
     }
 
     @Test
+    public void supports_multi_level_flags_directory() {
+        var archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-multi-level/"));
+        assertFlagDataHasValue(archive, MY_TEST_FLAG, mainControllerTarget, "default");
+    }
+
+    @Test
+    public void duplicated_flagdata_is_detected() {
+        expectedException.expect(IllegalArgumentException.class);
+        expectedException.expectMessage("Flag data file in 'flags/group-1/my-test-flag/default.json' contains redundant flag data for id 'my-test-flag' already set in another directory!");
+        var archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-multi-level-with-duplicated-flagdata/"));
+    }
+
+    @Test
     public void empty_files_are_handled_as_no_flag_data_for_target() {
         var archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags/"));
         assertNoFlagData(archive, FLAG_WITH_EMPTY_DATA, mainControllerTarget);
diff --git a/controller-api/src/test/resources/system-flags-multi-level-with-duplicated-flagdata/flags/group-1/my-test-flag/default.json b/controller-api/src/test/resources/system-flags-multi-level-with-duplicated-flagdata/flags/group-1/my-test-flag/default.json
new file mode 100644
index 00000000000..5924eb860c0
--- /dev/null
+++ b/controller-api/src/test/resources/system-flags-multi-level-with-duplicated-flagdata/flags/group-1/my-test-flag/default.json
@@ -0,0 +1,8 @@
+{
+  "id" : "my-test-flag",
+  "rules" : [
+    {
+      "value" : "default"
+    }
+  ]
+}
\ No newline at end of file
diff --git a/controller-api/src/test/resources/system-flags-multi-level-with-duplicated-flagdata/flags/group-2/my-test-flag/default.json b/controller-api/src/test/resources/system-flags-multi-level-with-duplicated-flagdata/flags/group-2/my-test-flag/default.json
new file mode 100644
index 00000000000..5924eb860c0
--- /dev/null
+++ b/controller-api/src/test/resources/system-flags-multi-level-with-duplicated-flagdata/flags/group-2/my-test-flag/default.json
@@ -0,0 +1,8 @@
+{
+  "id" : "my-test-flag",
+  "rules" : [
+    {
+      "value" : "default"
+    }
+  ]
+}
\ No newline at end of file
diff --git a/controller-api/src/test/resources/system-flags-multi-level/flags/group-1/my-test-flag/default.json b/controller-api/src/test/resources/system-flags-multi-level/flags/group-1/my-test-flag/default.json
new file mode 100644
index 00000000000..5924eb860c0
--- /dev/null
+++ b/controller-api/src/test/resources/system-flags-multi-level/flags/group-1/my-test-flag/default.json
@@ -0,0 +1,8 @@
+{
+  "id" : "my-test-flag",
+  "rules" : [
+    {
+      "value" : "default"
+    }
+  ]
+}
\ No newline at end of file
diff --git a/controller-api/src/test/resources/system-flags-multi-level/flags/group-2/my-other-test-flag/default.json b/controller-api/src/test/resources/system-flags-multi-level/flags/group-2/my-other-test-flag/default.json
new file mode 100644
index 00000000000..e30485b755c
--- /dev/null
+++ b/controller-api/src/test/resources/system-flags-multi-level/flags/group-2/my-other-test-flag/default.json
@@ -0,0 +1,8 @@
+{
+  "id" : "my-other-test-flag",
+  "rules" : [
+    {
+      "value" : "default"
+    }
+  ]
+}
\ No newline at end of file