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: Working authClassName filter if multiple heimdall deployments are present in a cluster #742

Merged
merged 3 commits into from
Jun 28, 2023

Conversation

netthier
Copy link
Contributor

@netthier netthier commented Jun 28, 2023

Related issue(s)

closes #736

Checklist

  • I agree to follow this project's Code of Conduct.
  • I have read, and I am following this repository's Contributing Guidelines.
  • I have read the Security Policy.
  • I have referenced an issue describing the bug/feature request.
  • I have added tests that prove the correctness of my implementation.
  • I have updated the documentation.

Description

This PR fixes the issue mentioned above by moving the filtering logic into the creation, update and deletion functions.
It does make the code less DRY, but I've never written Go before so I'm unsure how to solve this more cleanly.
It furthermore also solves an issue where rewrites have to have all fields set, which is incorrect behavior.

It also contains fixes to make the DNS resolution tests pass more reliably

Changelist

fix: Fix authClassName filtering
fix: Make DNS resolution tests pass more reliably
fix: Allow rewrites with only a subset of fields set

@codecov
Copy link

codecov bot commented Jun 28, 2023

Codecov Report

Merging #742 (958c724) into main (dff3d4d) will decrease coverage by 0.15%.
The diff coverage is 44.00%.

@@            Coverage Diff             @@
##             main     #742      +/-   ##
==========================================
- Coverage   89.59%   89.45%   -0.15%     
==========================================
  Files         215      215              
  Lines        9229     9238       +9     
==========================================
- Hits         8269     8264       -5     
- Misses        767      779      +12     
- Partials      193      195       +2     
Impacted Files Coverage Δ
internal/rules/provider/kubernetes/provider.go 81.81% <36.36%> (-6.95%) ⬇️
internal/rules/rule_factory_impl.go 97.09% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

netthier added 3 commits June 28, 2023 17:20
Signed-off-by: netthier <admin@netthier.net>
Signed-off-by: netthier <admin@netthier.net>
Signed-off-by: netthier <admin@netthier.net>
@dadrus dadrus changed the title Fix authClassName filter fix: Working authClassName filter if multiple heimdall deployments are present in a cluster Jun 28, 2023
@dadrus dadrus self-requested a review June 28, 2023 17:43
@dadrus
Copy link
Owner

dadrus commented Jun 28, 2023

I'm merging it as is and going to create another PR with some test for the given fixes

@dadrus dadrus merged commit 109365f into dadrus:main Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kubernetes provider gets stuck when encountering unconfigured authClass
2 participants