Conversation
|
This change is part of the following stack: Change managed by git-spice. |
|
Skipping CI for Draft Pull Request. |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Several calls that previously asserted on errors now ignore them (e.g.,
ListComplianceScanConfigurationsandDeleteComplianceScanConfigurationin tests use_ =/_ , _ :=); consider restoring explicit error checks or adding a clear justification, since silently dropping these errors can hide real regressions in the compliance API. - The
e2e-tailored-profileidentifier is now hard-coded in Go tests, e2e shell scripts, and YAML; consider centralizing this as a shared constant or adding a clear comment in each place to reduce the risk of the name drifting between components.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Several calls that previously asserted on errors now ignore them (e.g., `ListComplianceScanConfigurations` and `DeleteComplianceScanConfiguration` in tests use `_ =`/`_ , _ :=`); consider restoring explicit error checks or adding a clear justification, since silently dropping these errors can hide real regressions in the compliance API.
- The `e2e-tailored-profile` identifier is now hard-coded in Go tests, e2e shell scripts, and YAML; consider centralizing this as a shared constant or adding a clear comment in each place to reduce the risk of the name drifting between components.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
🚀 Build Images ReadyImages are ready for commit 38c2fdc. To use with deploy scripts: export MAIN_IMAGE_TAG=4.11.x-621-g38c2fdc69b |
ed0f017 to
d8b4493
Compare
This is about an unrelated commit I pushed accidentally. I also deleted a similar coderabbit comment |
📝 WalkthroughWalkthroughDeferred and conditional startup of compliance tailored profiles informer. The handler registration is moved from an early startup block to after the preTopLevelDeploymentWaitGroup phase completes, remaining conditional on Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19938 +/- ##
==========================================
- Coverage 49.56% 49.56% -0.01%
==========================================
Files 2764 2764
Lines 208346 208348 +2
==========================================
- Hits 103271 103268 -3
- Misses 97423 97429 +6
+ Partials 7652 7651 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@guzalv: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Description
Tailored profiles usually extend a base profile (add or remove rules from it). This PR fixes a race condition where a TP's base profile may not have synced by the time it was processed, because profiles and tailored profiles informers were started concurrently:
User-facing documentation
Testing and quality
Automated testing
How I validated my change
CI, deployed in a cluster and verified that the race is gone.