Fail flags deployment if archive contains unknown files

4 files changed, 35 insertions, 8 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployResult.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployResult.java
index bc00090dc88..c55d9d9a5e9 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployResult.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployResult.java
@@ -218,6 +218,10 @@ class SystemFlagsDeployResult {
             return new OperationError(message, Set.of(target), OperationType.DELETE, id, null);
         }
 
+        static OperationError archiveValidationFailed(String message) {
+            return new OperationError(message, Set.of(), OperationType.VALIDATE_ARCHIVE, null, null);
+        }
+
         String message() { return message; }
         Set<FlagsTarget> targets() { return targets; }
         OperationType operation() { return operation; }
@@ -251,7 +255,7 @@ class SystemFlagsDeployResult {
     }
 
     enum OperationType {
-        CREATE("create"), DELETE("delete"), UPDATE("update"), LIST("list");
+        CREATE("create"), DELETE("delete"), UPDATE("update"), LIST("list"), VALIDATE_ARCHIVE("validate-archive");
 
         private final String stringValue;
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployer.java
index b89a9dd7f5a..c75a6d7b413 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployer.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.controller.restapi.systemflags;
 
 import com.yahoo.concurrent.DaemonThreadFactory;
+import com.yahoo.config.provision.SystemName;
 import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.athenz.identity.ServiceIdentityProvider;
 import com.yahoo.vespa.flags.FlagId;
@@ -38,16 +39,18 @@ class SystemFlagsDeployer  {
     private static final Logger log = Logger.getLogger(SystemFlagsDeployer.class.getName());
 
     private final FlagsClient client;
+    private final SystemName system;
     private final Set<FlagsTarget> targets;
     private final ExecutorService executor = Executors.newCachedThreadPool(new DaemonThreadFactory("system-flags-deployer-"));
 
 
-    SystemFlagsDeployer(ServiceIdentityProvider identityProvider, Set<FlagsTarget> targets) {
-        this(new FlagsClient(identityProvider, targets), targets);
+    SystemFlagsDeployer(ServiceIdentityProvider identityProvider, SystemName system, Set<FlagsTarget> targets) {
+        this(new FlagsClient(identityProvider, targets), system, targets);
     }
 
-    SystemFlagsDeployer(FlagsClient client, Set<FlagsTarget> targets) {
+    SystemFlagsDeployer(FlagsClient client, SystemName system, Set<FlagsTarget> targets) {
         this.client = client;
+        this.system = system;
         this.targets = targets;
     }
 
@@ -65,6 +68,11 @@ class SystemFlagsDeployer  {
                 throw new RuntimeException(e);
             }
         });
+        try {
+            archive.validateAllFilesAreForTargets(system, targets);
+        } catch (IllegalArgumentException e) {
+            results.add(new SystemFlagsDeployResult(List.of(OperationError.archiveValidationFailed(e.getMessage()))));
+        }
         return SystemFlagsDeployResult.merge(results);
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java
index fd594cd57bc..2f7716aaff0 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java
@@ -35,7 +35,7 @@ public class SystemFlagsHandler extends LoggingRequestHandler {
                               Executor executor,
                               AccessLog accessLog) {
         super(executor, accessLog);
-        this.deployer = new SystemFlagsDeployer(identityProvider, FlagsTarget.getAllTargetsInSystem(zoneRegistry));
+        this.deployer = new SystemFlagsDeployer(identityProvider, zoneRegistry.system(), FlagsTarget.getAllTargetsInSystem(zoneRegistry));
     }
 
     @Override
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployerTest.java
index 1eb4523c344..4c41b54585f 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployerTest.java
@@ -58,7 +58,7 @@ public class SystemFlagsDeployerTest {
                 .build();
 
         SystemFlagsDeployer deployer =
-                new SystemFlagsDeployer(flagsClient, Set.of(controllerTarget, prodUsWest1Target, prodUsEast3Target));
+                new SystemFlagsDeployer(flagsClient, SYSTEM, Set.of(controllerTarget, prodUsWest1Target, prodUsEast3Target));
 
 
         SystemFlagsDeployResult result = deployer.deployFlags(archive, false);
@@ -83,7 +83,7 @@ public class SystemFlagsDeployerTest {
                 .addFile("main.json", defaultData)
                 .build();
 
-        SystemFlagsDeployer deployer = new SystemFlagsDeployer(flagsClient, Set.of(controllerTarget));
+        SystemFlagsDeployer deployer = new SystemFlagsDeployer(flagsClient, SYSTEM, Set.of(controllerTarget));
         SystemFlagsDeployResult result = deployer.deployFlags(archive, true);
 
         verify(flagsClient, times(1)).listFlagData(controllerTarget);
@@ -107,7 +107,7 @@ public class SystemFlagsDeployerTest {
                 .addFile("main.json", defaultData)
                 .build();
 
-        SystemFlagsDeployer deployer = new SystemFlagsDeployer(flagsClient, Set.of(prodUsWest1Target, prodUsEast3Target));
+        SystemFlagsDeployer deployer = new SystemFlagsDeployer(flagsClient, SYSTEM, Set.of(prodUsWest1Target, prodUsEast3Target));
 
         SystemFlagsDeployResult result = deployer.deployFlags(archive, false);
 
@@ -117,6 +117,21 @@ public class SystemFlagsDeployerTest {
                 FlagDataChange.created(FLAG_ID, prodUsEast3Target, defaultData));
     }
 
+    @Test
+    public void creates_error_entry_for_invalid_flag_archive() throws IOException {
+        FlagsClient flagsClient = mock(FlagsClient.class);
+        FlagData defaultData = flagData("flags/my-flag/main.json");
+        SystemFlagsDataArchive archive = new SystemFlagsDataArchive.Builder()
+                .addFile("main.prod.unknown-region.json", defaultData)
+                .build();
+        SystemFlagsDeployer deployer = new SystemFlagsDeployer(flagsClient, SYSTEM, Set.of(controllerTarget));
+        SystemFlagsDeployResult result = deployer.deployFlags(archive, false);
+        assertThat(result.flagChanges())
+                .isEmpty();
+        assertThat(result.errors())
+                .containsOnly(OperationError.archiveValidationFailed("Unknown flag file: flags/my-flag/main.prod.unknown-region.json"));
+    }
+
     private static FlagData flagData(String filename) throws IOException {
         return FlagData.deserializeUtf8Json(Files.readAllBytes(Paths.get("src/test/resources/system-flags/" + filename)));
     }