Fail hosted-feature-flags dry run if overrides for undefined flags

3 files changed, 21 insertions, 11 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 d169cd97df7..f140648c6dd 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
@@ -5,12 +5,14 @@ import com.fasterxml.jackson.databind.JsonNode;
 import com.yahoo.vespa.flags.FlagDefinition;
 import com.yahoo.vespa.flags.FlagId;
 import com.yahoo.vespa.flags.Flags;
+import com.yahoo.vespa.flags.PermanentFlags;
 import com.yahoo.vespa.flags.json.FlagData;
 import com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget;
 import com.yahoo.vespa.hosted.controller.api.systemflags.v1.wire.WireSystemFlagsDeployResult;
 import com.yahoo.vespa.hosted.controller.api.systemflags.v1.wire.WireSystemFlagsDeployResult.WireFlagDataChange;
 import com.yahoo.vespa.hosted.controller.api.systemflags.v1.wire.WireSystemFlagsDeployResult.WireOperationFailure;
 import com.yahoo.vespa.hosted.controller.api.systemflags.v1.wire.WireSystemFlagsDeployResult.WireWarning;
+import org.apache.zookeeper.Op;
 
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -251,6 +253,13 @@ class SystemFlagsDeployResult {
             return new OperationError(message, Set.of(), OperationType.VALIDATE_ARCHIVE, null, null);
         }
 
+        static OperationError dataForUndefinedFlag(FlagsTarget target, FlagId id) {
+            return new OperationError("Flag data present for undefined flag. Remove flag data files if flag's definition " +
+                                      "is already removed from Flags / PermanentFlags. Consult ModelContext.FeatureFlags " +
+                                      "for safe removal of flag used by config-model.",
+                                      Set.of(), OperationType.DATA_FOR_UNDEFINED_FLAG, id, null);
+        }
+
         String message() { return message; }
         Set<FlagsTarget> targets() { return targets; }
         OperationType operation() { return operation; }
@@ -284,7 +293,8 @@ class SystemFlagsDeployResult {
     }
 
     enum OperationType {
-        CREATE("create"), DELETE("delete"), UPDATE("update"), LIST("list"), VALIDATE_ARCHIVE("validate-archive");
+        CREATE("create"), DELETE("delete"), UPDATE("update"), LIST("list"), VALIDATE_ARCHIVE("validate-archive"),
+        DATA_FOR_UNDEFINED_FLAG("data-for-undefined-flag");
 
         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 21a429b59ad..e0b65b0834d 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
@@ -78,7 +78,7 @@ class SystemFlagsDeployer  {
         return SystemFlagsDeployResult.merge(results);
     }
 
-    private SystemFlagsDeployResult deployFlags(FlagsTarget target, Set<FlagData> flagData, boolean dryRun) {
+    private SystemFlagsDeployResult deployFlags(FlagsTarget target, List<FlagData> flagData, boolean dryRun) {
         Map<FlagId, FlagData> wantedFlagData = lookupTable(flagData);
         Map<FlagId, FlagData> currentFlagData;
         List<FlagId> definedFlags;
@@ -98,7 +98,7 @@ class SystemFlagsDeployer  {
         updateExistingFlagData(target, dryRun, wantedFlagData, currentFlagData, results, errors);
         removeOldFlagData(target, dryRun, wantedFlagData, currentFlagData, results, errors);
         failOnNewFlagDataForUndefinedFlags(target, wantedFlagData, currentFlagData, definedFlags, errors);
-        warnOnExistingFlagDataForUndefinedFlags(target, wantedFlagData, currentFlagData, definedFlags, warnings);
+        failOnFlagDataForUndefinedFlags(target, wantedFlagData, currentFlagData, definedFlags, errors);
         return new SystemFlagsDeployResult(results, errors, warnings);
     }
 
@@ -191,14 +191,14 @@ class SystemFlagsDeployer  {
         }
     }
 
-    private static void warnOnExistingFlagDataForUndefinedFlags(FlagsTarget target,
-                                                                Map<FlagId, FlagData> wantedFlagData,
-                                                                Map<FlagId, FlagData> currentFlagData,
-                                                                List<FlagId> definedFlags,
-                                                                List<Warning> warnings) {
+    private static void failOnFlagDataForUndefinedFlags(FlagsTarget target,
+                                                        Map<FlagId, FlagData> wantedFlagData,
+                                                        Map<FlagId, FlagData> currentFlagData,
+                                                        List<FlagId> definedFlags,
+                                                        List<OperationError> errors) {
         for (FlagId flagId : currentFlagData.keySet()) {
             if (wantedFlagData.containsKey(flagId) && !definedFlags.contains(flagId)) {
-                warnings.add(Warning.dataForUndefinedFlag(target, flagId));
+                errors.add(OperationError.dataForUndefinedFlag(target, flagId));
             }
         }
     }
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 35a13cdeeec..549dd1ed253 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
@@ -166,8 +166,8 @@ public class SystemFlagsDeployerTest {
                 .build();
         SystemFlagsDeployer deployer = new SystemFlagsDeployer(flagsClient, SYSTEM, Set.of(prodUsEast3Target));
         SystemFlagsDeployResult result = deployer.deployFlags(archive, true);
-        assertThat(result.warnings())
-                .containsOnly(SystemFlagsDeployResult.Warning.dataForUndefinedFlag(prodUsEast3Target, new FlagId("my-flag")));
+        assertThat(result.errors())
+                .containsOnly(OperationError.dataForUndefinedFlag(prodUsEast3Target, new FlagId("my-flag")));
     }
 
     private static FlagData flagData(String filename) throws IOException {