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

fix: fix webhook validation of KongConsumers referring Secrets with konghq.com/credential label #5067

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

pmalek
Copy link
Member

@pmalek pmalek commented Nov 2, 2023

What this PR does / why we need it:

After introducing credential type labels in #4825, it seems that webhook wasn't checked for letting the Secrets with only said label (where it should).

This PR fixes it by fixing the webhook logic in KongHTTPValidator.ValidateCredential.

Which issue this PR fixes:

Fixes: #5069

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

No CHANGELOG entry since the change regarding keyCredType wasn't released yet.

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

@pmalek pmalek added this to the KIC v3.0.0 milestone Nov 2, 2023
@pmalek pmalek self-assigned this Nov 2, 2023
@pmalek pmalek marked this pull request as ready for review November 2, 2023 18:46
@pmalek pmalek requested a review from a team as a code owner November 2, 2023 18:46
Copy link

codecov bot commented Nov 2, 2023

Codecov Report

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

Comparison is base (c39e31e) 75.5% compared to head (4631b6f) 75.5%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #5067   +/-   ##
=====================================
  Coverage   75.5%   75.5%           
=====================================
  Files        167     167           
  Lines      18913   18909    -4     
=====================================
+ Hits       14285   14290    +5     
+ Misses      3798    3793    -5     
+ Partials     830     826    -4     
Files Coverage Δ
...ion/validation/consumers/credentials/validation.go 92.2% <16.6%> (-4.1%) ⬇️

... and 5 files with indirect coverage changes

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

Copy link
Contributor

@rainest rainest left a comment

Choose a reason for hiding this comment

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

Cross-resource checks, ever a blessing.

@rainest rainest enabled auto-merge (squash) November 2, 2023 19:27
@rainest rainest disabled auto-merge November 2, 2023 19:27
@rainest rainest merged commit 6fb8469 into main Nov 2, 2023
45 checks passed
@rainest rainest deleted the fix-webhook-consumer-secret-validation-with-label branch November 2, 2023 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KongConsumers referring Secrets with konghq.com/credential label without keyCredType field are rejected
2 participants