Hosted feature flags validation

2 files changed, 24 insertions, 7 deletions

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 355f06fc753..2c38066eddd 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
@@ -8,6 +8,7 @@ import com.yahoo.vespa.flags.FlagId;
 import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.flags.json.FlagData;
 import com.yahoo.vespa.hosted.controller.api.integration.ControllerIdentityProvider;
+import com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagValidationException;
 import com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget;
 import com.yahoo.vespa.hosted.controller.api.systemflags.v1.SystemFlagsDataArchive;
 import com.yahoo.vespa.hosted.controller.restapi.systemflags.SystemFlagsDeployResult.OperationError;
@@ -57,6 +58,12 @@ class SystemFlagsDeployer  {
     }
 
     SystemFlagsDeployResult deployFlags(SystemFlagsDataArchive archive, boolean dryRun) {
+        try {
+            archive.validateAllFilesAreForTargets(targets);
+        } catch (FlagValidationException e) {
+            return new SystemFlagsDeployResult(List.of(OperationError.archiveValidationFailed(e.getMessage())));
+        }
+
         Map<FlagsTarget, Future<SystemFlagsDeployResult>> futures = new HashMap<>();
         for (FlagsTarget target : targets) {
             futures.put(target, executor.submit(() -> deployFlags(target, archive.flagData(target), dryRun)));
@@ -70,11 +77,6 @@ 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 e9b087690ff..bb285b8b742 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
@@ -10,10 +10,13 @@ import com.yahoo.restapi.JacksonJsonResponse;
 import com.yahoo.restapi.Path;
 import com.yahoo.vespa.hosted.controller.api.integration.ControllerIdentityProvider;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
+import com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagValidationException;
 import com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget;
 import com.yahoo.vespa.hosted.controller.api.systemflags.v1.SystemFlagsDataArchive;
 import com.yahoo.vespa.hosted.controller.restapi.ErrorResponses;
 
+import java.io.InputStream;
+import java.util.List;
 import java.util.concurrent.Executor;
 
 /**
@@ -27,12 +30,14 @@ public class SystemFlagsHandler extends ThreadedHttpRequestHandler {
     private static final String API_PREFIX = "/system-flags/v1";
 
     private final SystemFlagsDeployer deployer;
+    private final ZoneRegistry zoneRegistry;
 
     @Inject
     public SystemFlagsHandler(ZoneRegistry zoneRegistry,
                               ControllerIdentityProvider identityProvider,
                               Executor executor) {
         super(executor);
+        this.zoneRegistry = zoneRegistry;
         this.deployer = new SystemFlagsDeployer(identityProvider, zoneRegistry.system(), FlagsTarget.getAllTargetsInSystem(zoneRegistry, true));
     }
 
@@ -57,12 +62,22 @@ public class SystemFlagsHandler extends ThreadedHttpRequestHandler {
             if (!contentType.equalsIgnoreCase("application/zip")) {
                 return ErrorResponse.badRequest("Invalid content type: " + contentType);
             }
-            SystemFlagsDataArchive archive = SystemFlagsDataArchive.fromZip(request.getData());
-            SystemFlagsDeployResult result = deployer.deployFlags(archive, dryRun);
+            SystemFlagsDeployResult result = deploy(request.getData(), dryRun);
             return new JacksonJsonResponse<>(200, result.toWire());
         } catch (Exception e) {
             return ErrorResponses.logThrowing(request, log, e);
         }
     }
 
+    private SystemFlagsDeployResult deploy(InputStream zipStream, boolean dryRun) {
+        SystemFlagsDataArchive archive;
+        try {
+            archive = SystemFlagsDataArchive.fromZip(zipStream, zoneRegistry);
+        } catch (FlagValidationException e) {
+            return new SystemFlagsDeployResult(List.of(SystemFlagsDeployResult.OperationError.archiveValidationFailed(e.getMessage())));
+        }
+
+        return deployer.deployFlags(archive, dryRun);
+    }
+
 }