Repository: kestra-io/kestra (java)

Issue:
Change system labels pattern to use dot instead of underscore as a separator to match existing naming conventions
### Feature description

We realized that the current pattern using underscore as a system flow separator: `system_readOnly` or `system_correlationId` can be confusing to some users given that we mix `camelCase` with `snake_case` notation.

Let's change system labels to use dot:

```yaml
id: read_only
namespace: company.team

labels:
  mynr: 42
  system.readOnly: true
  system.correlationId: i174t8

tasks:
  - id: hello
    type: io.kestra.plugin.core.log.Log
    message: Hello {{ labels.system }}
```

Once we use the dot notation, some users will likely expect that this pattern `{{ labels.system }}` can be used to retrieve all system labels, so we should enable that pebble if feasible. 

Pull request description:
feat(*): change system label prefix to use a dot
Fixes #5708

Patch:
diff --git a/cli/src/main/resources/application.yml b/cli/src/main/resources/application.yml
index 1576eb908..a8c14701b 100644
--- a/cli/src/main/resources/application.yml
+++ b/cli/src/main/resources/application.yml
@@ -191,5 +191,5 @@ kestra:
 
   hidden-labels:
     prefixes:
-      - system_
-      - internal_
+      - system.
+      - internal.
diff --git a/core/src/main/java/io/kestra/core/models/Label.java b/core/src/main/java/io/kestra/core/models/Label.java
index aa4be5153..d7664a325 100644
--- a/core/src/main/java/io/kestra/core/models/Label.java
+++ b/core/src/main/java/io/kestra/core/models/Label.java
@@ -3,7 +3,7 @@ package io.kestra.core.models;
 import jakarta.validation.constraints.NotNull;
 
 public record Label(@NotNull String key, @NotNull String value) {
-    public static final String SYSTEM_PREFIX = "system_";
+    public static final String SYSTEM_PREFIX = "system.";
 
     // system labels
     public static final String CORRELATION_ID = SYSTEM_PREFIX + "correlationId";
diff --git a/core/src/main/java/io/kestra/core/runners/RunVariables.java b/core/src/main/java/io/kestra/core/runners/RunVariables.java
index 0ac5673c1..c348146ae 100644
--- a/core/src/main/java/io/kestra/core/runners/RunVariables.java
+++ b/core/src/main/java/io/kestra/core/runners/RunVariables.java
@@ -1,6 +1,7 @@
 package io.kestra.core.runners;
 
 import com.google.common.collect.ImmutableMap;
+import io.kestra.core.models.Label;
 import io.kestra.core.models.executions.Execution;
 import io.kestra.core.models.executions.TaskRun;
 import io.kestra.core.models.flows.Flow;
@@ -312,10 +313,7 @@ public final class RunVariables {
                     builder.put("labels", execution.getLabels()
                         .stream()
                         .filter(label -> label.value() != null && label.key() != null)
-                        .map(label -> new AbstractMap.SimpleEntry<>(
-                            label.key(),
-                            label.value()
-                        ))
+                        .map(label -> mapLabel(label))
                         // using an accumulator in case labels with the same key exists: the first is kept
                         .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, (first, second) -> first))
                     );
@@ -349,5 +347,22 @@ public final class RunVariables {
         }
     }
 
+    private static Map.Entry<String, Object> mapLabel(Label label) {
+        if (label.key().startsWith(Label.SYSTEM_PREFIX)) {
+            return Map.entry(
+                label.key().substring(0, Label.SYSTEM_PREFIX.length() - 1),
+                Map.entry(
+                    label.key().substring(Label.SYSTEM_PREFIX.length()),
+                    label.value()
+                )
+            );
+        } else {
+            return Map.entry(
+                label.key(),
+                label.value()
+            );
+        }
+    }
+
     private RunVariables(){}
 }
diff --git a/core/src/main/java/io/kestra/plugin/core/execution/Labels.java b/core/src/main/java/io/kestra/plugin/core/execution/Labels.java
index c633ff47b..3700fde86 100644
--- a/core/src/main/java/io/kestra/plugin/core/execution/Labels.java
+++ b/core/src/main/java/io/kestra/plugin/core/execution/Labels.java
@@ -37,7 +37,7 @@ import static io.kestra.core.utils.Rethrow.throwFunction;
 @NoArgsConstructor
 @Schema(
     title = "Allow to add or overwrite labels for the current execution at runtime.",
-    description = "Trying to pass a system label (a label starting with `system_`) will fail the task."
+    description = "Trying to pass a system label (a label starting with `system.`) will fail the task."
 )
 @Plugin(
     examples = {
diff --git a/ui/src/components/flows/FlowRoot.vue b/ui/src/components/flows/FlowRoot.vue
index 520bd00af..f3e6e0b81 100644
--- a/ui/src/components/flows/FlowRoot.vue
+++ b/ui/src/components/flows/FlowRoot.vue
@@ -332,7 +332,7 @@
                     return false;
                 }
 
-                return this.flow.labels?.system_readOnly === "true" ?? false;
+                return this.flow.labels?.["system.readOnly"] === "true" ?? false;
             },
             routeFlowDependencies() {
                 return this.dependenciesCount > 0 ? FlowDependencies : FlowNoDependencies;
diff --git a/webserver/src/main/java/io/kestra/webserver/controllers/api/ExecutionController.java b/webserver/src/main/java/io/kestra/webserver/controllers/api/ExecutionController.java
index 3aa7ca76e..542adea7a 100644
--- a/webserver/src/main/java/io/kestra/webserver/controllers/api/ExecutionController.java
+++ b/webserver/src/main/java/io/kestra/webserver/controllers/api/ExecutionController.java
@@ -688,7 +688,7 @@ public class ExecutionController {
             .map(entry -> new Label(entry.getKey(), entry.getValue()))
             .toList();
 
-        // check for system labels: none can be passed at execution creation time except system_correlationId
+        // check for system labels: none can be passed at execution creation time except system.correlationId
         Optional<Label> first = parsedLabels.stream().filter(label -> !label.key().equals(CORRELATION_ID) && label.key().startsWith(SYSTEM_PREFIX)).findFirst();
         if (first.isPresent()) {
             throw new IllegalArgumentException("System labels can only be set by Kestra itself, offending label: " + first.get().key() + "=" + first.get().value());


Test patch:
diff --git a/core/src/test/java/io/kestra/core/serializers/YamlFlowParserTest.java b/core/src/test/java/io/kestra/core/serializers/YamlFlowParserTest.java
index 94ee3db6a..bfc678b1e 100644
--- a/core/src/test/java/io/kestra/core/serializers/YamlFlowParserTest.java
+++ b/core/src/test/java/io/kestra/core/serializers/YamlFlowParserTest.java
@@ -167,7 +167,7 @@ class YamlFlowParserTest {
         Flow flow = this.parse("flows/valids/minimal.yaml");
 
         String s = mapper.writeValueAsString(flow);
-        assertThat(s, is("{\"id\":\"minimal\",\"namespace\":\"io.kestra.tests\",\"revision\":2,\"disabled\":false,\"deleted\":false,\"labels\":[{\"key\":\"system_readOnly\",\"value\":\"true\"}],\"tasks\":[{\"id\":\"date\",\"type\":\"io.kestra.plugin.core.debug.Return\",\"format\":\"{{taskrun.startDate}}\"}]}"));
+        assertThat(s, is("{\"id\":\"minimal\",\"namespace\":\"io.kestra.tests\",\"revision\":2,\"disabled\":false,\"deleted\":false,\"labels\":[{\"key\":\"system.readOnly\",\"value\":\"true\"}],\"tasks\":[{\"id\":\"date\",\"type\":\"io.kestra.plugin.core.debug.Return\",\"format\":\"{{taskrun.startDate}}\"}]}"));
     }
 
     @Test
diff --git a/core/src/test/java/io/kestra/core/services/LabelServiceTest.java b/core/src/test/java/io/kestra/core/services/LabelServiceTest.java
index cb28162e8..42f0a78ff 100644
--- a/core/src/test/java/io/kestra/core/services/LabelServiceTest.java
+++ b/core/src/test/java/io/kestra/core/services/LabelServiceTest.java
@@ -25,7 +25,7 @@ class LabelServiceTest {
     @Test
     void shouldFilterSystemLabels() {
         Flow flow = Flow.builder()
-            .labels(List.of(new Label("key", "value"), new Label("system_Label", "systemValue")))
+            .labels(List.of(new Label("key", "value"), new Label(Label.SYSTEM_PREFIX + "label", "systemValue")))
             .build();
 
         List<Label> labels = LabelService.labelsExcludingSystem(flow);
@@ -38,7 +38,7 @@ class LabelServiceTest {
     void shouldReturnLabelsFromFlowAndTrigger() {
         RunContext runContext = runContextFactory.of(Map.of("variable", "variableValue"));
         Flow flow = Flow.builder()
-            .labels(List.of(new Label("key", "value"), new Label("system_Label", "systemValue")))
+            .labels(List.of(new Label("key", "value"), new Label(Label.SYSTEM_PREFIX + "label", "systemValue")))
             .build();
         AbstractTrigger trigger = Schedule.builder()
             .labels(List.of(new Label("scheduleLabel", "scheduleValue"), new Label("variable", "{{variable}}")))
@@ -54,7 +54,7 @@ class LabelServiceTest {
     void shouldFilterNonRenderableLabels() {
         RunContext runContext = runContextFactory.of();
         Flow flow = Flow.builder()
-            .labels(List.of(new Label("key", "value"), new Label("system_Label", "systemValue")))
+            .labels(List.of(new Label("key", "value"), new Label(Label.SYSTEM_PREFIX + "label", "systemValue")))
             .build();
         AbstractTrigger trigger = Schedule.builder()
             .labels(List.of(new Label("scheduleLabel", "scheduleValue"), new Label("variable", "{{variable}}")))
diff --git a/core/src/test/java/io/kestra/core/validations/FlowValidationTest.java b/core/src/test/java/io/kestra/core/validations/FlowValidationTest.java
index b92f343f4..3866e67ba 100644
--- a/core/src/test/java/io/kestra/core/validations/FlowValidationTest.java
+++ b/core/src/test/java/io/kestra/core/validations/FlowValidationTest.java
@@ -39,8 +39,8 @@ class FlowValidationTest {
         Optional<ConstraintViolationException> validate = modelValidator.isValid(flow);
 
         assertThat(validate.isPresent(), is(true));
-        assertThat(validate.get().getMessage(), containsString("System labels can only be set by Kestra itself, offending label: system_label=system_key"));
-        assertThat(validate.get().getMessage(), containsString("System labels can only be set by Kestra itself, offending label: system_id=id"));
+        assertThat(validate.get().getMessage(), containsString("System labels can only be set by Kestra itself, offending label: system.label=system_key"));
+        assertThat(validate.get().getMessage(), containsString("System labels can only be set by Kestra itself, offending label: system.id=id"));
     }
 
     @Test
diff --git a/core/src/test/java/io/kestra/plugin/core/flow/RuntimeLabelsTest.java b/core/src/test/java/io/kestra/plugin/core/flow/RuntimeLabelsTest.java
index 05fc8ccf0..0c7afe3f4 100644
--- a/core/src/test/java/io/kestra/plugin/core/flow/RuntimeLabelsTest.java
+++ b/core/src/test/java/io/kestra/plugin/core/flow/RuntimeLabelsTest.java
@@ -41,7 +41,7 @@ class RuntimeLabelsTest extends AbstractMemoryRunnerTest {
 
         String labelsOverriderTaskRunId = execution.findTaskRunsByTaskId("override-labels").getFirst().getId();
         assertThat(execution.getLabels(), containsInAnyOrder(
-            is(new Label("system_correlationId", execution.getId())),
+            is(new Label(Label.CORRELATION_ID, execution.getId())),
             is(new Label("flowLabelKey", "flowLabelValue")),
             is(new Label("overriddenFlowLabelKey", "io.kestra.tests.labels-update-task")),
             is(new Label("keyFromJson", "valueFromJson")),
diff --git a/core/src/test/resources/flows/invalids/system-labels.yaml b/core/src/test/resources/flows/invalids/system-labels.yaml
index 8f8ff6292..acc6b2a7b 100644
--- a/core/src/test/resources/flows/invalids/system-labels.yaml
+++ b/core/src/test/resources/flows/invalids/system-labels.yaml
@@ -2,8 +2,8 @@ id: system-labels
 namespace: io.kestra.tests
 
 labels:
-  system_label: system_key
-  system_id: id
+  system.label: system_key
+  system.id: id
   another: label
 
 tasks:
diff --git a/core/src/test/resources/flows/valids/minimal.yaml b/core/src/test/resources/flows/valids/minimal.yaml
index c8e99bfbf..51e78582d 100644
--- a/core/src/test/resources/flows/valids/minimal.yaml
+++ b/core/src/test/resources/flows/valids/minimal.yaml
@@ -3,7 +3,7 @@ namespace: io.kestra.tests
 revision: 2
 
 labels:
-  system_readOnly: "true"
+  system.readOnly: "true"
 
 tasks:
 - id: date
diff --git a/core/src/test/resources/flows/valids/subflow-grand-child.yaml b/core/src/test/resources/flows/valids/subflow-grand-child.yaml
index 09e1ca5a8..1a136efed 100644
--- a/core/src/test/resources/flows/valids/subflow-grand-child.yaml
+++ b/core/src/test/resources/flows/valids/subflow-grand-child.yaml
@@ -4,4 +4,4 @@ namespace: io.kestra.tests
 tasks:
   - id: firstLevel
     type: io.kestra.plugin.core.log.Log
-    message: "My grandparent is {{labels.system_correlationId}}"
\ No newline at end of file
+    message: "My grandparent is {{labels.system.correlationId}}"
\ No newline at end of file
diff --git a/webserver/src/test/java/io/kestra/webserver/controllers/api/ExecutionControllerTest.java b/webserver/src/test/java/io/kestra/webserver/controllers/api/ExecutionControllerTest.java
index 5ac32d003..0799798e2 100644
--- a/webserver/src/test/java/io/kestra/webserver/controllers/api/ExecutionControllerTest.java
+++ b/webserver/src/test/java/io/kestra/webserver/controllers/api/ExecutionControllerTest.java
@@ -1597,7 +1597,7 @@ class ExecutionControllerTest extends JdbcH2ControllerTest {
     void shouldRefuseSystemLabelsWhenCreatingAnExecution() {
         var error = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().retrieve(
             HttpRequest
-                .POST("/api/v1/executions/io.kestra.tests/minimal?labels=system_label:system", null)
+                .POST("/api/v1/executions/io.kestra.tests/minimal?labels=system.label:system", null)
                 .contentType(MediaType.MULTIPART_FORM_DATA_TYPE),
             Execution.class
         ));
@@ -1612,7 +1612,7 @@ class ExecutionControllerTest extends JdbcH2ControllerTest {
         assertThat(result.getState().getCurrent(), is(State.Type.SUCCESS));
 
         var error = assertThrows(HttpClientResponseException.class, () -> client.toBlocking().retrieve(
-                HttpRequest.POST("/api/v1/executions/" + result.getId() + "/labels", List.of(new Label("system_label", "value"))),
+                HttpRequest.POST("/api/v1/executions/" + result.getId() + "/labels", List.of(new Label("system.label", "value"))),
                 Execution.class
             )
         );