Skip to content

istio: ignore defaultRevision#1178

Merged
istio-testing merged 4 commits intoistio-ecosystem:mainfrom
jewertow:ignore-default-revision
Oct 9, 2025
Merged

istio: ignore defaultRevision#1178
istio-testing merged 4 commits intoistio-ecosystem:mainfrom
jewertow:ignore-default-revision

Conversation

@jewertow
Copy link
Copy Markdown
Contributor

What type of PR is this?

  • Enhancement / New Feature
  • Bug Fix
  • Refactor
  • Optimization
  • Test
  • Documentation Update

What this PR does / why we need it:

Istio resource includes the defaultRevision field which is used for controlling creation of the default validation webhook, which belongs to the base chart. Since the base chart is intended for managing cluster-scoped objects, it shouldn't be exposed in the Istio resource, because different revisions or tenants can affect webhooks created by others.

That field was deprecated in #760, hidden from documentation, but it wasn't actually ignored as a comment for that field says.

This may be potentially a breaking change for users who rely on the default revision webhook, so we may consider adding a CRD for managing cluster-scoped objects first.

Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
@jewertow jewertow requested a review from a team as a code owner August 28, 2025 11:43
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.71%. Comparing base (c845edd) to head (afa62af).
⚠️ Report is 37 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1178      +/-   ##
==========================================
+ Coverage   77.42%   77.71%   +0.29%     
==========================================
  Files          44       44              
  Lines        2817     2827      +10     
==========================================
+ Hits         2181     2197      +16     
+ Misses        528      523       -5     
+ Partials      108      107       -1     

☔ View full report in Codecov by Sentry.
📢 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.

Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
@FilipB
Copy link
Copy Markdown
Collaborator

FilipB commented Sep 11, 2025

@jewertow Do you think this should be backported to release-1.26?

@jewertow
Copy link
Copy Markdown
Contributor Author

Hey @FilipB, I think it should not be backported to any Z-stream release. It's a fairly significant change.

@dgn
Copy link
Copy Markdown
Collaborator

dgn commented Sep 12, 2025

could you extend the tests to verify that when creating an IstioRevisionTag called default, we will create the global validating webhook?

@dgn
Copy link
Copy Markdown
Collaborator

dgn commented Sep 15, 2025

could you extend the tests to verify that when creating an IstioRevisionTag called default, we will create the global validating webhook?

Nevermind, did it myself :)

Signed-off-by: Daniel Grimm <dgrimm@redhat.com>
@dgn
Copy link
Copy Markdown
Collaborator

dgn commented Oct 9, 2025

/retest

@istio-testing istio-testing merged commit 97f104c into istio-ecosystem:main Oct 9, 2025
17 checks passed
dgn added a commit to dgn/sail-operator that referenced this pull request Mar 17, 2026
* istio: ignore `defaultRevision`

Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>

* Fix failing tests

Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>

* Fix lint error

Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>

* test: check for presence of default validator

Signed-off-by: Daniel Grimm <dgrimm@redhat.com>

---------

Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Signed-off-by: Daniel Grimm <dgrimm@redhat.com>
Co-authored-by: Daniel Grimm <dgrimm@redhat.com>
Signed-off-by: Daniel Grimm <dgrimm@redhat.com>
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.

4 participants