Fail flags deployment if archive contains unknown files

3 files changed, 44 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 c5533307dbf..0cb4baab790 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,7 @@
 // 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.yahoo.config.provision.SystemName;
 import com.yahoo.vespa.flags.FlagId;
 import com.yahoo.vespa.flags.json.FlagData;
 
@@ -13,11 +14,13 @@ import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipInputStream;
@@ -86,7 +89,7 @@ public class SystemFlagsDataArchive {
         files.forEach((flagId, fileMap) -> {
             fileMap.forEach((filename, flagData) -> {
                 uncheck(() -> {
-                    zipOut.putNextEntry(new ZipEntry("flags/" + flagId.toString() + "/" + filename));
+                    zipOut.putNextEntry(new ZipEntry(toFilePath(flagId, filename)));
                     zipOut.write(flagData.serializeToUtf8Json());
                     zipOut.closeEntry();
                 });
@@ -112,6 +115,25 @@ public class SystemFlagsDataArchive {
         return targetData;
     }
 
+    public void validateAllFilesAreForTargets(SystemName currentSystem, Set<FlagsTarget> targets) throws IllegalArgumentException {
+        Set<String> validFiles = targets.stream()
+                .flatMap(target -> target.flagDataFilesPrioritized().stream())
+                .collect(Collectors.toSet());
+        Set<SystemName> otherSystems = Arrays.stream(SystemName.values())
+                .filter(systemName -> systemName != currentSystem)
+                .collect(Collectors.toSet());
+        files.forEach((flagId, fileMap) -> {
+            for (String filename : fileMap.keySet()) {
+                boolean isFileForOtherSystem = otherSystems.stream()
+                        .anyMatch(system -> filename.startsWith(system.value() + "."));
+                boolean isFileForCurrentSystem = validFiles.contains(filename);
+                if (!isFileForOtherSystem && !isFileForCurrentSystem) {
+                    throw new IllegalArgumentException("Unknown flag file: " + toFilePath(flagId, filename));
+                }
+            }
+        });
+    }
+
     private static void addFile(Builder builder, String rawData, Path filePath) {
         String filename = filePath.getFileName().toString();
         if (filename.startsWith(".")) {
@@ -135,6 +157,10 @@ public class SystemFlagsDataArchive {
         builder.addFile(filename, flagData);
     }
 
+    private static String toFilePath(FlagId flagId, String filename) {
+        return "flags/" + flagId.toString() + "/" + filename;
+    }
+
     public static class Builder {
         private final Map<FlagId, Map<String, FlagData>> files = new TreeMap<>();
 
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 3476e24c570..6e78b8da91c 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
@@ -28,6 +28,7 @@ import java.net.URI;
 import java.nio.file.Paths;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import static java.util.stream.Collectors.toList;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -95,6 +96,14 @@ public class SystemFlagsDataArchiveTest {
         SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-invalid-file-name/"));
     }
 
+    @Test
+    public void throws_exception_on_unknown_file() {
+        SystemFlagsDataArchive archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-unknown-file-name/"));
+        expectedException.expect(IllegalArgumentException.class);
+        expectedException.expectMessage("Unknown flag file: flags/my-test-flag/main.prod.unknown-region.json");
+        archive.validateAllFilesAreForTargets(SystemName.main, Set.of(mainControllerTarget, prodUsWestCfgTarget));
+    }
+
     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-file-name/flags/my-test-flag/main.prod.unknown-region.json b/controller-api/src/test/resources/system-flags-with-unknown-file-name/flags/my-test-flag/main.prod.unknown-region.json
new file mode 100644
index 00000000000..5924eb860c0
--- /dev/null
+++ b/controller-api/src/test/resources/system-flags-with-unknown-file-name/flags/my-test-flag/main.prod.unknown-region.json
@@ -0,0 +1,8 @@
+{
+  "id" : "my-test-flag",
+  "rules" : [
+    {
+      "value" : "default"
+    }
+  ]
+}
\ No newline at end of file