feature: Add configurable labels for Argo Workflows - #2781
Conversation
Capiru
changed the title
Greptile SummaryThis PR adds configurable labels for Argo Workflow resources. The main changes are:
Confidence Score: 4/5This is close, but the label key validation should be fixed before merging.
Files Needing Attention: metaflow/plugins/kubernetes/kube_utils.py
|
| Filename | Overview |
|---|---|
| metaflow/metaflow_config.py | Adds the new Argo labels configuration default. |
| metaflow/plugins/argo/argo_workflows.py | Parses configured Argo labels and applies them to workflow-level metadata while preserving Metaflow-owned labels. |
| metaflow/plugins/kubernetes/kube_utils.py | Adds optional key validation for Kubernetes labels, but prefix validation still accepts keys Kubernetes rejects. |
| test/unit/test_argo_workflows_labels.py | Adds unit coverage for configured Argo labels, protected labels, and invalid label values. |
| test/ux/core/test_argo_compilation.py | Adds compilation coverage for configured labels in Argo WorkflowTemplate output. |
Reviews (5): Last reviewed commit: "Improve Argo workflow label tests" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| self._schedule, self._timezone = self._get_schedule() | ||
|
|
||
| self._base_labels = self._base_kubernetes_labels() | ||
| self._base_labels = self._base_argo_labels() |
There was a problem hiding this comment.
just to double-check. base_labels also applies to sensors. Is it intended that the configurable ARGO_WORKFLOWS_LABELS applies to all argo resources, not only workflow/workflow templates?
There was a problem hiding this comment.
This is a nice catch, I made it explicit so that it only applies to Workflow/Templates
| def validate_label_key(key: str): | ||
| prefix, _, name = key.rpartition("/") | ||
| if prefix: | ||
| prefix_regex = r"^[A-Za-z0-9]([-A-Za-z0-9.]{0,251}[A-Za-z0-9])?$" |
There was a problem hiding this comment.
METAFLOW_ARGO_WORKFLOWS_LABELS="Team/name=ml" passes because uppercase letters are allowed here, and a single 64-character prefix component also passes because only total prefix length is checked. Those labels then reach Argo metadata and fail later when Kubernetes validates the resource. Please validate the prefix as a DNS subdomain, including lowercase-only components and the per-component length limit.
| prefix_regex = r"^[A-Za-z0-9]([-A-Za-z0-9.]{0,251}[A-Za-z0-9])?$" | ||
| if not re.search(prefix_regex, prefix): |
There was a problem hiding this comment.
This prefix regex still accepts label keys that Kubernetes rejects. For example, METAFLOW_ARGO_WORKFLOWS_LABELS="Team/name=ml", "a..b/name=ml", or a key with a 64-character prefix component before /name can pass local validation. Those labels are then emitted into Argo metadata and the resource can fail later when Kubernetes validates it. Please validate the prefix as a DNS-1123 subdomain, including lowercase labels, non-empty dot-separated components, and the per-component length limit.
| self._base_labels = self._base_kubernetes_labels() | ||
| self._workflow_labels = self._base_argo_labels() |
There was a problem hiding this comment.
This split sends configured Argo labels only through _workflow_labels, while pods, JobSets, and Sensors keep using _base_labels. With METAFLOW_ARGO_WORKFLOWS_LABELS="team=ml", the WorkflowTemplate and Workflow get team=ml, but workflow pods, JobSets, and Sensors do not. Selectors, policy, or cost attribution that target those resources can still miss the workflow. The configured labels need to be included on those resource paths as defaults while keeping internal labels protected and preserving step-level overrides.
- validate label keys (not just values) since ARGO_WORKFLOWS_LABELS keys are fully user-supplied - protect internal labels (app.kubernetes.io/part-of) from being overridden by custom env labels - scope custom labels to WorkflowTemplate/Workflow level only; pod/JobSet/ Sensor metadata keep using internal-only labels
98fc92f to
692c2c2
Compare
| def validate_label_key(key: str): | ||
| prefix, _, name = key.rpartition("/") | ||
| if prefix: | ||
| prefix_regex = r"^[A-Za-z0-9]([-A-Za-z0-9.]{0,251}[A-Za-z0-9])?$" |
There was a problem hiding this comment.
The new key-validation path still accepts label prefixes that Kubernetes rejects. When METAFLOW_ARGO_WORKFLOWS_LABELS is set to values like Team/name=ml, a..b/name=ml, a-/name=ml, or a key with a 64-character prefix component before /name, this regex can pass the key locally. The label is then emitted into WorkflowTemplate and Workflow metadata, where Kubernetes rejects it because label prefixes must be DNS subdomains with lowercase, non-empty dot-separated labels, each starting and ending alphanumeric and each at most 63 characters. Please validate the prefix component-by-component before accepting user-supplied label keys.
3 participants
Summary
Adds
METAFLOW_ARGO_WORKFLOWS_LABELSfor configuring labels on Argo WorkflowTemplates and generated Workflow resources.Custom labels are intentionally not applied to Pods, JobSets, or Sensors. Step-level Kubernetes labels remain controlled by
METAFLOW_KUBERNETES_LABELSand@kubernetes(labels=...).Metaflow-owned labels take precedence over configured labels.
Usage
Configured labels are added to:
WorkflowTemplate.metadata.labelsWorkflowTemplate.spec.workflowMetadata.labels, which supplies labels to generated Workflow resourcesTesting
Fixes #2780