-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 GCP PodIdentity not considered with WorkloadIdentity #5021
Fix GCP PodIdentity not considered with WorkloadIdentity #5021
Conversation
Thank you for your contribution! 🙏 We will review your PR as soon as possible. While you are waiting, make sure to:
Learn more about: |
Hello,
|
Hi @JorTurFer given this is a fix, to merge this PR, is the extra e2e required or a nice to have? @juldrixx are you still working on this? I may be able to fork and fix the failing test but wouldn't be able to add the e2e in the short term Cheers both |
I'm currently rather busy, but I might have some time next week to look at the test part. But if you want to do it before then, no worries you can. |
Any kind of test is required for preventing regressions and in this case, I'd say that e2e test is the easiest way because that code it's quite coupled and there isn't any other test already done that you can use as base for having all the test setup ready, you'd need to prepare all the unit test setup, which probably requires more time than writing an e2e test (but it's not mandatory an e2e test) |
Replaced by: #5255 |
Provide a description of what has been changed
Checklist
#5011
Relates to #