Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle KongIngress deprecation in admission webhook and in KongIngress validation rules #5022

Merged
merged 14 commits into from
Oct 31, 2023

Conversation

rainest
Copy link
Contributor

@rainest rainest commented Oct 30, 2023

What this PR does / why we need it:

Alternative to #5015. Deprecates KongIngress in the sense that it's still available for the time being, but results in logs instructing users to migrate rather than removing it immediately.

Which issue this PR fixes:

For #4720 but not using auto-resolve since this isn't targeted to main.

Would need a follow-up "Remove KongIngress support" issue.

Notes:

I did leave in the checks for route and service even though CEL now blocks them. I figure it doesn't really hurt anything to have them there under normal circumstances, and we often had abnormal circumstances because users forget to update CRDs.

Added a Service webhook handler also. I don't really love having that in place for such a common resource, but it is at least only temporary: we'd remove it once we actually remove KongIngress.

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@codecov
Copy link

codecov bot commented Oct 30, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (dce791d) 75.5% compared to head (8c6c952) 75.5%.
Report is 1 commits behind head on main.

❗ Current head 8c6c952 differs from pull request most recent head 057d0da. Consider uploading reports for the commit 057d0da to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #5022     +/-   ##
=======================================
- Coverage   75.5%   75.5%   -0.1%     
=======================================
  Files        167     167             
  Lines      18848   18836     -12     
=======================================
- Hits       14238   14226     -12     
- Misses      3783    3785      +2     
+ Partials     827     825      -2     
Files Coverage Δ
internal/dataplane/kongstate/kongstate.go 86.2% <100.0%> (+1.5%) ⬆️
internal/admission/handler.go 61.0% <78.2%> (+1.5%) ⬆️

... and 6 files with indirect coverage changes

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

@pull-request-size pull-request-size bot added size/L and removed size/M labels Oct 30, 2023
CHANGELOG.md Outdated Show resolved Hide resolved
Base automatically changed from feat/kongupstreampolicy-parser to main October 31, 2023 10:58
@pmalek pmalek added this to the KIC v3.0.0 milestone Oct 31, 2023
@czeslavo czeslavo changed the title Log warnings for KongIngress use chore: do not allow 'proxy' and 'route', deprecate 'upstream' in KongIngress Oct 31, 2023
@czeslavo czeslavo added area/CRD Changes in existing CRDs or introduction of new ones area/maintenance Cleanup, refactoring, and other maintenance improvements that don't change functionality. labels Oct 31, 2023
@pmalek pmalek changed the title chore: do not allow 'proxy' and 'route', deprecate 'upstream' in KongIngress Handle KongIngress deprecation in admission webhook and in KongIngress validation rules Oct 31, 2023
@czeslavo czeslavo requested a review from a team October 31, 2023 12:26
@czeslavo
Copy link
Contributor

I've rebased the PR onto main and added CEL validations.

pmalek
pmalek previously approved these changes Oct 31, 2023
CHANGELOG.md Outdated Show resolved Hide resolved
pmalek
pmalek previously approved these changes Oct 31, 2023
CHANGELOG.md Outdated Show resolved Hide resolved
pmalek
pmalek previously approved these changes Oct 31, 2023
rainest and others added 5 commits October 31, 2023 16:23
@czeslavo
Copy link
Contributor

I had to rebase (conflict in go.sum), sorry @pmalek 😞

@czeslavo czeslavo enabled auto-merge (squash) October 31, 2023 15:50
@czeslavo czeslavo merged commit 2fccacb into main Oct 31, 2023
33 checks passed
@czeslavo czeslavo deleted the feat/kongingress-log-warn branch October 31, 2023 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CRD Changes in existing CRDs or introduction of new ones area/maintenance Cleanup, refactoring, and other maintenance improvements that don't change functionality. size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants