Merge pull request #28681 from vespa-engine/mortent/application-dimension

Add APPLICATION_ID/application dimension

4 files changed, 39 insertions, 19 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 856af9f4132..8f47ac68cda 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
@@ -8,6 +8,7 @@ import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.fasterxml.jackson.databind.node.TextNode;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.ApplicationName;
 import com.yahoo.config.provision.CloudAccount;
 import com.yahoo.config.provision.CloudName;
 import com.yahoo.config.provision.ClusterSpec;
@@ -191,8 +192,8 @@ public class SystemFlagsDataArchive {
         flagData.rules().forEach(rule -> rule.conditions().forEach(condition -> {
             int force_switch_expression_dummy = switch (condition.type()) {
                 case RELATIONAL -> switch (condition.dimension()) {
-                    case INSTANCE_ID, CLOUD, CLOUD_ACCOUNT, CLUSTER_ID, CLUSTER_TYPE, CONSOLE_USER_EMAIL,
-                            ENVIRONMENT, HOSTNAME, NODE_TYPE, SYSTEM, TENANT_ID, ZONE_ID ->
+                    case APPLICATION_ID, CLOUD, CLOUD_ACCOUNT, CLUSTER_ID, CLUSTER_TYPE, CONSOLE_USER_EMAIL,
+                            ENVIRONMENT, HOSTNAME, INSTANCE_ID, NODE_TYPE, SYSTEM, TENANT_ID, ZONE_ID ->
                             throw new FlagValidationException(condition.type().toWire() + " " +
                                                               DimensionHelper.toWire(condition.dimension()) +
                                                               " condition is not supported");
@@ -206,7 +207,7 @@ public class SystemFlagsDataArchive {
                 };
 
                 case WHITELIST, BLACKLIST -> switch (condition.dimension()) {
-                    case INSTANCE_ID -> validateConditionValues(condition, ApplicationId::fromSerializedForm);
+                    case APPLICATION_ID -> validateConditionValues(condition, SystemFlagsDataArchive::validateTenantApplication);
                     case CONSOLE_USER_EMAIL -> validateConditionValues(condition, email -> {
                         if (!email.contains("@"))
                             throw new FlagValidationException("Invalid email address: " + email);
@@ -220,6 +221,7 @@ public class SystemFlagsDataArchive {
                     case CLUSTER_TYPE -> validateConditionValues(condition, ClusterSpec.Type::from);
                     case ENVIRONMENT -> validateConditionValues(condition, Environment::from);
                     case HOSTNAME -> validateConditionValues(condition, HostName::of);
+                    case INSTANCE_ID -> validateConditionValues(condition, ApplicationId::fromSerializedForm);
                     case NODE_TYPE -> validateConditionValues(condition, NodeType::valueOf);
                     case SYSTEM -> throw new IllegalStateException("Flag data contains system dimension");
                     case TENANT_ID -> validateConditionValues(condition, TenantName::from);
@@ -250,23 +252,20 @@ public class SystemFlagsDataArchive {
         return 0;  // dummy to force switch expression
     }
 
+    private static void validateTenantApplication(String application) {
+        String[] parts = application.split(":");
+        if (parts.length != 2)
+            throw new IllegalArgumentException("Applications must be on the form tenant:application, but was %s".formatted(application));
+        TenantName.from(parts[0]);
+        ApplicationName.from(parts[1]);
+    }
+
     private static FlagData parseFlagData(FlagId flagId, String fileContent, ZoneRegistry zoneRegistry, boolean inController) {
         if (fileContent.isBlank()) return new FlagData(flagId);
 
         final JsonNode root;
         try {
             root = mapper.readTree(fileContent);
-            // TODO (mortent): Remove this after completing migration of APPLICATION_ID dimension
-            // replace "application" with "instance" for all dimension fields
-            List<JsonNode> dimensionParents = root.findParents("dimension");
-            for (JsonNode parentNode : dimensionParents) {
-                JsonNode dimension = parentNode.get("dimension");
-                if (dimension.isTextual() && "application".equals(dimension.textValue())) {
-                    ObjectNode parent = (ObjectNode) parentNode;
-                    parent.remove("dimension");
-                    parent.put("dimension", "instance");
-                }
-            }
         } catch (JsonProcessingException e) {
             throw new FlagValidationException("Invalid JSON: " + e.getMessage());
         }
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 373f8ba9de2..2631f518720 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
@@ -250,6 +250,16 @@ public class SystemFlagsDataArchiveTest {
                                        }
                                      ],
                                      "value": true
+                                   },
+                                   {
+                                     "conditions": [
+                                       {
+                                         "type": "whitelist",
+                                         "dimension": "application",
+                                         "values": [ "f:o" ]
+                                       }
+                                     ],
+                                     "value": true
                                    }
                                  ]
                                }""",
@@ -292,6 +302,16 @@ public class SystemFlagsDataArchiveTest {
                                        }
                                      ],
                                      "value": true
+                                   },
+                                   {
+                                     "conditions": [
+                                       {
+                                         "type": "whitelist",
+                                         "dimension": "application",
+                                         "values": [ "f:o" ]
+                                       }
+                                     ],
+                                     "value": true
                                    }
                                  ]
                                }""")));
@@ -307,8 +327,7 @@ public class SystemFlagsDataArchiveTest {
 
     @Test
     void normalize_json_succeed_on_valid_values() {
-        addFile(Condition.Type.WHITELIST, "application", "a:b:c");
-        addFile(Condition.Type.WHITELIST, "instance", "a:b:c");
+        addFile(Condition.Type.WHITELIST, "application", "a:b");
         addFile(Condition.Type.WHITELIST, "cloud", "yahoo");
         addFile(Condition.Type.WHITELIST, "cloud", "aws");
         addFile(Condition.Type.WHITELIST, "cloud", "gcp");
@@ -321,6 +340,7 @@ public class SystemFlagsDataArchiveTest {
         addFile(Condition.Type.WHITELIST, "environment", "staging");
         addFile(Condition.Type.WHITELIST, "environment", "test");
         addFile(Condition.Type.WHITELIST, "hostname", "2080046-v6-11.ostk.bm2.prod.gq1.yahoo.com");
+        addFile(Condition.Type.WHITELIST, "instance", "a:b:c");
         addFile(Condition.Type.WHITELIST, "node-type", "tenant");
         addFile(Condition.Type.WHITELIST, "node-type", "host");
         addFile(Condition.Type.WHITELIST, "node-type", "config");
@@ -362,12 +382,13 @@ public class SystemFlagsDataArchiveTest {
 
     @Test
     void normalize_json_fail_on_invalid_values() {
-        failAddFile(Condition.Type.WHITELIST, "application", "a.b.c", "In file flags/temporary/foo/default.json: Invalid instance 'a.b.c' in whitelist condition: Application ids must be on the form tenant:application:instance, but was a.b.c");
+        failAddFile(Condition.Type.WHITELIST, "application", "a.b", "In file flags/temporary/foo/default.json: Invalid application 'a.b' in whitelist condition: Applications must be on the form tenant:application, but was a.b");
         failAddFile(Condition.Type.WHITELIST, "cloud", "foo", "In file flags/temporary/foo/default.json: Unknown cloud: foo");
         // cluster-id: any String is valid
         failAddFile(Condition.Type.WHITELIST, "cluster-type", "foo", "In file flags/temporary/foo/default.json: Invalid cluster-type 'foo' in whitelist condition: Illegal cluster type 'foo'");
         failAddFile(Condition.Type.WHITELIST, "console-user-email", "not-valid-email-address", "In file flags/temporary/foo/default.json: Invalid email address: not-valid-email-address");
         failAddFile(Condition.Type.WHITELIST, "environment", "foo", "In file flags/temporary/foo/default.json: Invalid environment 'foo' in whitelist condition: 'foo' is not a valid environment identifier");
+        failAddFile(Condition.Type.WHITELIST, "instance", "a.b.c", "In file flags/temporary/foo/default.json: Invalid instance 'a.b.c' in whitelist condition: Application ids must be on the form tenant:application:instance, but was a.b.c");
         failAddFile(Condition.Type.WHITELIST, "hostname", "not:a:hostname", "In file flags/temporary/foo/default.json: Invalid hostname 'not:a:hostname' in whitelist condition: hostname must match '(([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9-]{0,61}[A-Za-z0-9])\\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9-]{0,61}[A-Za-z0-9])\\.?', but got: 'not:a:hostname'");
         failAddFile(Condition.Type.WHITELIST, "node-type", "footype", "In file flags/temporary/foo/default.json: Invalid node-type 'footype' in whitelist condition: No enum constant com.yahoo.config.provision.NodeType.footype");
         failAddFile(Condition.Type.WHITELIST, "system", "bar", "In file flags/temporary/foo/default.json: Invalid system 'bar' in whitelist condition: 'bar' is not a valid system");
diff --git a/controller-api/src/test/resources/system-flags-with-null-value/flags/my-test-flag/main.prod.us-east-3.json b/controller-api/src/test/resources/system-flags-with-null-value/flags/my-test-flag/main.prod.us-east-3.json
index b79e0913c22..c4dca9aa2e1 100644
--- a/controller-api/src/test/resources/system-flags-with-null-value/flags/my-test-flag/main.prod.us-east-3.json
+++ b/controller-api/src/test/resources/system-flags-with-null-value/flags/my-test-flag/main.prod.us-east-3.json
@@ -5,7 +5,7 @@
             "conditions": [
                 {
                     "type": "whitelist",
-                    "dimension": "application",
+                    "dimension": "instance",
                     "values": ["a:b:c"]
                 }
             ]
diff --git a/controller-api/src/test/resources/system-flags-with-null-value/flags/my-test-flag/main.prod.us-west-1.json b/controller-api/src/test/resources/system-flags-with-null-value/flags/my-test-flag/main.prod.us-west-1.json
index 75cffdea009..283b09d5c0b 100644
--- a/controller-api/src/test/resources/system-flags-with-null-value/flags/my-test-flag/main.prod.us-west-1.json
+++ b/controller-api/src/test/resources/system-flags-with-null-value/flags/my-test-flag/main.prod.us-west-1.json
@@ -5,7 +5,7 @@
             "conditions": [
                 {
                     "type": "whitelist",
-                    "dimension": "application",
+                    "dimension": "instance",
                     "values": ["a:b:c"]
                 }
             ],