fix(RELEASE-2685): treat empty cpe label as validation failure - #1001
Open
davidmogar wants to merge 1 commit into
Open
fix(RELEASE-2685): treat empty cpe label as validation failure#1001davidmogar wants to merge 1 commit into
davidmogar wants to merge 1 commit into
Conversation
check-labels treated a completely unset cpe label and a
present-but-empty cpe label identically, silently skipping
enforcement for both. this hid cases where a Containerfile sets
LABEL cpe=${CPE} but forgets to declare ARG CPE, leaving the
label present but empty.
get_label_value now returns None only when the label key is
missing, and "" when it is present but empty, so callers can
tell the two cases apart. an empty cpe label now fails
validation under --enforce (or warns otherwise), while a truly
unset label is still skipped as before.
Assisted-by: Claude
Signed-off-by: David Moreno García <damoreno@redhat.com>
PR Summary by QodoTreat empty CPE labels as validation failures
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo
1. Whitespace bypasses label validation
|
![qodo-app-for-konflux-ci[bot]](https://avatars.githubusercontent.com/u/155583288?s=60&v=4){kind=small}
Comment on lines
+210
to
+211
| def test_empty_cpe_label_enforce(tmp_path: Path) -> None: | ||
| """Fail when the CPE label is present but set to an empty string.""" |
There was a problem hiding this comment.
1. Unit tests not co-located 📘 Rule violation ▣ Testability
The new check_labels unit tests are placed in a nested tests/ directory rather than alongside check_labels.py. This violates the required test placement for source files outside utils/.
Agent Prompt
## Issue description
The newly added unit tests are under a nested `tests/` directory instead of being co-located with `check_labels.py`.
## Issue Context
PR Compliance ID 909 requires unit tests for source files outside `utils/` to reside in the same directory as their source file. Move or consolidate the tests into a co-located test module while preserving pytest discovery and imports.
## Fix Focus Areas
- src/tasks/managed/check_labels/tests/test_check_labels.py[111-260]
- src/tasks/managed/check_labels/check_labels.py[46-58]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| val = label.get("value") | ||
| if val is not None and str(val).strip(): | ||
| return str(val) | ||
| return str(val).strip() if val is not None else "" |
There was a problem hiding this comment.
2. Whitespace bypasses label validation 🐞 Bug ≡ Correctness
get_label_value strips surrounding whitespace from every non-empty label before validation, so a stored name or cpe value such as " expected " incorrectly matches "expected". This allows malformed image metadata to pass enforce-mode validation.
Agent Prompt
## Issue description
`get_label_value` must distinguish missing and empty labels without altering non-empty values. Its new unconditional `strip()` causes labels containing surrounding whitespace to pass exact-equality validation.
## Issue Context
Return `""` when a value is empty or whitespace-only, but return the original string representation when it contains non-whitespace characters. Add tests proving whitespace-padded `name` and `cpe` labels fail or warn rather than matching normalized expected values.
## Fix Focus Areas
- src/tasks/managed/check_labels/check_labels.py[46-59]
- src/tasks/managed/check_labels/tests/test_check_labels.py[90-120]
- src/tasks/managed/check_labels/tests/test_check_labels.py[144-260]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1001 +/- ##
=======================================
Coverage 97.24% 97.24%
=======================================
Files 205 205
Lines 12808 12813 +5
=======================================
+ Hits 12455 12460 +5
Misses 353 353
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
swickersh
approved these changes
Sep 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
check-labels treated a completely unset cpe label and a present-but-empty cpe label identically, silently skipping enforcement for both. this hid cases where a Containerfile sets LABEL cpe=${CPE} but forgets to declare ARG CPE, leaving the label present but empty.
get_label_value now returns None only when the label key is missing, and "" when it is present but empty, so callers can tell the two cases apart. an empty cpe label now fails validation under --enforce (or warns otherwise), while a truly unset label is still skipped as before.
Assisted-by: Claude