fix: label values sanitization and normalization#2724
Conversation
|
|
|
/ok-to-test |
|
@filariow you need to sign easycla |
There was a problem hiding this comment.
Code Review
This pull request updates the CleanValueKubernetes function to include periods in the character trimming set and adds relevant test cases to verify this behavior. The reviewer identified a potential issue regarding the order of operations, noting that performing trimming before character replacement may lead to invalid Kubernetes label formats. They suggested reordering the logic to perform replacements first, followed by trimming and length validation, and provided a code snippet to implement this improvement.
Label values can't start nor finish with a `.` (dot). This contribution ensures that any dot at the beginning or end of a label value is removed. Signed-off-by: Francesco Ilario <filario@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
d2a9f42 to
ff4697a
Compare
|
/ok-to-test |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2724 +/- ##
==========================================
+ Coverage 59.25% 59.36% +0.10%
==========================================
Files 208 208
Lines 20573 20626 +53
==========================================
+ Hits 12191 12244 +53
Misses 7610 7610
Partials 772 772 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Build Service konflux-ci/build-service#561 changed Repository CR naming to use the full git URL. PAC truncates the label to 62 chars but doesn't trim leading dots (tektoncd/pipelines-as-code#2724), causing PR to be rejected. Add ensure_safe_pac_label() to detect when truncation would leave a leading "." and pad the repo name by one char to shift past it. Assisted-by: Claude Signed-off-by: Sean Conroy <sconroy@redhat.com>
Signed-off-by: Francesco Ilario <filario@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
|
/retest |
Signed-off-by: Francesco Ilario <filario@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
Signed-off-by: Francesco Ilario <filario@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
Signed-off-by: Francesco Ilario <filario@redhat.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
|
/ok-to-test |
|
/lgtm |
There was a problem hiding this comment.
Congrats @filariow your PR Has been approved 🎉
✅ Pull Request Approved
Approval Status:
- Required Approvals: 1
- Current Approvals: 1
👥 Reviewers Who Approved:
| Reviewer | Permission Level | Approval Status |
|---|---|---|
| @zakisk | write |
✅ |
📝 Next Steps
- Ensure all required checks pass
- Comply with branch protection rules
- Request a maintainer to merge using the
/mergecommand (or merge it
directly if you have repository permission).
Automated by the PAC Boussole 🧭
|
@filariow Thank you for your contribution! 🎉 |
📝 Description of the Change
Label values need to respect strict rules.
This contribution ensures that Label Values are sanitized and normalized to respect the rules above.
🔗 Linked GitHub Issue
N/A
🧪 Testing Strategy
🤖 AI Assistance
AI assistance can be used for various tasks, such as code generation,
documentation, or testing.
Please indicate whether you have used AI assistance
for this PR and provide details if applicable.
To help me navigate the codebase, as I'm mostly new to it, and to review the changes before submitting them.
The changes and tests were hand-written.
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.
Signed-off-by: Francesco Ilario filario@redhat.com