Merge pull request #19335 from vespa-engine/hakonhall/fail-hosted-feature-flags-dry-run-if-overrides-for-undefined-flags

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

3 files changed, 19 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..529e892ced9 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
@@ -251,6 +251,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 +291,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 {