Fix omjob pod/label naming length constraints#27143
Fix omjob pod/label naming length constraints#27143pmbrull merged 8 commits intoopen-metadata:mainfrom
Conversation
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Fixes Kubernetes 63-character label-value constraint violations in the OMJob operator by truncating/sanitizing OMJob-derived label values and ensuring scheduled OMJob names/pod names stay within label-safe limits.
Changes:
- Sanitize + truncate
omjob.pipelines.openmetadata.org/namelabel value to ≤63 chars (and trim trailing separators after truncation). - Constrain operator-generated pod names for
-main/-exitto be label-safe (≤63 chars including suffix). - Constrain scheduled OMJob names (CronOMJob → OMJob) to ≤63 chars while preserving the timestamp suffix; add/adjust unit tests for long-name scenarios.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| openmetadata-k8s-operator/src/main/java/org/openmetadata/operator/util/LabelBuilder.java | Sanitizes/truncates OMJob name label value to comply with K8s label limits. |
| openmetadata-k8s-operator/src/main/java/org/openmetadata/operator/service/PodManager.java | Generates label-safe pod names for main/exit pods; adds constants and warning log on truncation. |
| openmetadata-k8s-operator/src/main/java/org/openmetadata/operator/controller/CronOMJobReconciler.java | Builds scheduled OMJob names capped to 63 chars while keeping epoch suffix; ensures labels map is mutable before adding run-id. |
| openmetadata-k8s-operator/src/test/java/org/openmetadata/operator/unit/LabelBuilderTest.java | Adds coverage for long OMJob name sanitization/truncation and trailing-separator trimming. |
| openmetadata-k8s-operator/src/test/java/org/openmetadata/operator/unit/CronOMJobReconcilerTest.java | Adds coverage for scheduled OMJob name trimming while keeping timestamp suffix. |
| openmetadata-k8s-operator/src/test/java/org/openmetadata/operator/service/PodManagerTest.java | Adds coverage for long OMJob name producing safe pod names and label values. |
| String baseName = omJob.getMetadata().getName(); | ||
|
|
||
| // Account for suffix length when calculating max base name length | ||
| // We use MAX_LABEL_LENGTH instead of MAX_POD_NAME_LENGTH to ensure | ||
| // the pod name can be used as a label value | ||
| int maxBaseLength = MAX_LABEL_LENGTH - suffix.length(); | ||
|
|
||
| if (baseName.length() > maxBaseLength) { | ||
| // Truncate to fit within the limit and remove trailing hyphens | ||
| baseName = baseName.substring(0, maxBaseLength).replaceAll("-+$", ""); | ||
| } |
There was a problem hiding this comment.
buildPodName only strips trailing hyphens after truncation. If the original OMJob name contains '.' and the truncation boundary lands on '.', the resulting pod name can end with a non-alphanumeric character, which violates Kubernetes DNS label/subdomain rules and can still fail pod creation. After truncation, ensure the baseName ends with [a-z0-9] (e.g., strip trailing "-" and "."/"_" or reuse a shared sanitizer that enforces the full name regex).
| String baseName = omJob.getMetadata().getName(); | ||
|
|
||
| // Account for suffix length when calculating max base name length |
There was a problem hiding this comment.
There are blank lines here that contain trailing whitespace. This can cause formatting checks (e.g., Spotless) to fail; please remove trailing spaces (or run the module's formatter) before merging.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
|
|
||
| private static final Logger LOG = LoggerFactory.getLogger(PodManager.class); | ||
| private static final String PIPELINE_STATUS = "pipelineStatus"; | ||
| private static final int MAX_POD_NAME_LENGTH = 253; |
There was a problem hiding this comment.
MAX_POD_NAME_LENGTH is introduced but not used (pod names are currently capped using MAX_LABEL_LENGTH). Consider removing it to avoid dead code, or apply it where appropriate if you still intend to enforce the 253-char DNS subdomain limit elsewhere.
| private static final int MAX_POD_NAME_LENGTH = 253; |
| } | ||
|
|
||
| if (baseName.length() > maxBaseLength) { | ||
| baseName = baseName.substring(0, maxBaseLength).replaceAll("-+$", ""); |
There was a problem hiding this comment.
buildScheduledOMJobName truncates the CronOMJob baseName and only strips trailing hyphens. If the truncation boundary lands on '.', the resulting OMJob name can end with '.', which violates Kubernetes resource name validation (must end with an alphanumeric). After truncation, strip all trailing non-alphanumeric separators (at least '-' and '.') or validate the final name against the DNS subdomain rules.
| baseName = baseName.substring(0, maxBaseLength).replaceAll("-+$", ""); | |
| baseName = baseName.substring(0, maxBaseLength).replaceAll("[-.]+$", ""); |
harshach
added
the
safe to test
|
add safe to test label @harshach |
|
The Java checkstyle failed. Please run You can install the pre-commit hooks with |
🟡 Playwright Results — all passed (27 flaky)✅ 3638 passed · ❌ 0 failed · 🟡 27 flaky · ⏭️ 84 skipped
🟡 27 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
|
The Java checkstyle failed. Please run You can install the pre-commit hooks with |
|
The Java checkstyle failed. Please run You can install the pre-commit hooks with |
1 similar comment
|
The Java checkstyle failed. Please run You can install the pre-commit hooks with |
- Fix SHA-256 hash byte formatting with & 0xff mask for proper 2-hex-digit encoding per byte - Enforce Kubernetes 63-character label limit via hash-based truncation - Extract shared hash utility to HashUtils - Add comprehensive tests for truncation, uniqueness, and edge cases Fixes open-metadata#27004
…t substring - Guard ensureValidLabelValue fallback against StringIndexOutOfBounds - Add tests for separator-only inputs exercising the fallback path - Remove redundant .substring() since HashUtils.hash() already returns 6 chars - Use 253 (K8s DNS subdomain limit) instead of 260 in PodManagerTest - Fix wrong assertion in LabelBuilderTest (podSelector has 2 entries) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add HTTP 409 idempotency handling in TableClass.create() for sharded Playwright tests - Apply Java Spotless formatting to fix checkstyle violations - Apply Playwright formatting: organize-imports, eslint --fix, prettier --write - Resolve all 10 code review findings: ✅ StringIndexOutOfBoundsException in hash truncation (already fixed) ✅ Redundant substring operation (already fixed) ✅ Duplicate hash code extraction (already done) ✅ Playwright 409 conflict handling (now added) ✅ Java formatting compliance (now applied) ✅ TypeScript formatting compliance (now applied) PR is now ready for merge with all CI checks expected to pass.
Code Review ✅ Approved 10 resolved / 10 findingsAdjusted omjob pod and label naming constraints to prevent length overflows. Resolved issues concerning repository bloat, duplicate utility logic, dangling statements, and potential runtime errors in string processing. ✅ 10 resolved✅ Edge Case: Fallback name "omjob" could theoretically exceed limit
✅ Quality: Committed .m2/ Maven cache and debug.json bloat the repository
✅ Quality: Excessive documentation files for a small code change
✅ Bug: Dangling statements outside function in persona.ts
✅ Quality: Duplicate sanitizeEmailSegment with divergent behavior
...and 5 more resolved from earlier reviews OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
|
Hi @pmbrull @harshach @akash-jain-10 @PubChimps |
Describe your changes
Fixes #27004 @pmbrull @harshach
This PR addresses an issue where long pipeline/job names could generate
Kubernetes label values exceeding the 63-character limit, leading to pod
creation failures.
Summary of Changes
Core Fix: Kubernetes Label Constraint Enforcement
Fixed critical SHA-256 hash byte formatting bug in pod/label name truncation
& 0xffmask to byte formatting inKubernetesNameBuilder.hash()& 0xffmask to byte formatting inLabelBuilder.hash()Enforced Kubernetes 63-character label limit across generated resources
Code Quality Improvements
HashUtilsutility classStringIndexOutOfBoundsExceptionin hash truncationPlaywright Test Reliability
TableClass.tsRoot Cause
Kubernetes enforces a 63-character limit on label values, while previous
logic only constrained pod names to the DNS limit (253 characters).
Additionally, signed byte formatting in hash functions produced incorrect hex
encoding (e.g.,
ffffff80instead of80), reducing hash quality andincreasing collision risk for deterministic name truncation.
Solution
String.format("%02x", value & 0xff)forcorrect 2-digit hex per byte
MAX_LABEL_LENGTH = 63constantHashUtilsclass to eliminate code duplicationTesting
PodManagerTest.javaImpact
Before
After
Type of change
Checklist