Skip to content

Return API errors from IstioRevision tag conflict check - #2293

Open
sridhargaddam wants to merge 1 commit into
istio-ecosystem:mainfrom
sridhargaddam:issue-2082
Open

Return API errors from IstioRevision tag conflict check#2293
sridhargaddam wants to merge 1 commit into
istio-ecosystem:mainfrom
sridhargaddam:issue-2082

Conversation

@sridhargaddam

Copy link
Copy Markdown
Contributor

validateNoTagConflict only reacted when Client.Get returned no error, so a transient API error looked the same as "no tag with this name". That let a conflicting IstioRevision pass validation and install, fighting an identically-named IstioRevisionTag over the same webhook resources. Now we return any non-NotFound error, matching the check that already exists in the IstioRevisionTag controller.

Fixes: #2082

validateNoTagConflict only reacted when Client.Get returned no error,
so a transient API error looked the same as "no tag with this name".
That let a conflicting IstioRevision pass validation and install,
fighting an identically-named IstioRevisionTag over the same webhook
resources. Now we return any non-NotFound error, matching the check
that already exists in the IstioRevisionTag controller.

Fixes: istio-ecosystem#2082
Signed-off-by: Sridhar Gaddam <sgaddam@redhat.com>
@sridhargaddam
sridhargaddam requested a review from a team as a code owner August 31, 2026 18:06
@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.12%. Comparing base (0dd46a2) to head (0f7e2b3).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2293      +/-   ##
==========================================
+ Coverage   87.81%   88.12%   +0.30%     
==========================================
  Files          56       56              
  Lines        2658     2660       +2     
==========================================
+ Hits         2334     2344      +10     
+ Misses        324      316       -8     
Flag Coverage Δ
integration-tests 74.61% <50.00%> (+0.59%) ⬆️
unit-tests 65.03% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dgn

dgn commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

/retest

@dgn dgn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @sridhargaddam

@sridhargaddam

Copy link
Copy Markdown
Contributor Author

/test e2e-kind-dualstack

@istio-testing

Copy link
Copy Markdown
Collaborator

@sridhargaddam: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
e2e-kind-dualstack_sail-operator_main 0f7e2b3 link true /test e2e-kind-dualstack
Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] validateNoTagConflict swallows non-NotFound API errors

4 participants