chore: adding annotation to generate VAPB right away once the waiting window is over to protect against clock skews #3773
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3773 +/- ##
==========================================
- Coverage 54.49% 47.76% -6.74%
==========================================
Files 134 235 +101
Lines 12329 19865 +7536
==========================================
+ Hits 6719 9488 +2769
- Misses 5116 9488 +4372
- Partials 494 889 +395
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -65,9 +65,12 @@ import ( | |||
|
|
|||
| const ( | |||
| BlockVAPBGenerationUntilAnnotation = "gatekeeper.sh/block-vapb-generation-until" | |||
| VAPBGenerationAnnotation = "gatekeeper.sh/vapb-generation" | |||
There was a problem hiding this comment.
nit: vapb-generation-lock or similar to more clearly specify intent
There was a problem hiding this comment.
How about vapb-generation-state?
| @@ -745,6 +747,9 @@ func TestReconcile(t *testing.T) { | |||
| if vapBindingCreationTime.Before(blockTime) { | |||
| return fmt.Errorf("VAPBinding should be created after default wait") | |||
| } | |||
| if ct.GetAnnotations()[constraint.VAPBGenerationAnnotation] == constraint.VAPBGenerationUnblocked { | |||
| return fmt.Errorf("expected %s annotations on CT to be unblocked", constraint.VAPBGenerationAnnotation) | |||
There was a problem hiding this comment.
shouldnt the expected to be the opposite?
| @@ -873,6 +881,9 @@ func TestReconcile(t *testing.T) { | |||
| if vapBindingCreationTime.Before(blockTime) { | |||
| return fmt.Errorf("VAPBinding should not be created before the timestamp") | |||
| } | |||
| if ct.GetAnnotations()[constraint.VAPBGenerationAnnotation] == constraint.VAPBGenerationUnblocked { | |||
| return fmt.Errorf("expected %s annotations on CT to be unblocked", constraint.VAPBGenerationAnnotation) | |||
| } | |||
There was a problem hiding this comment.
Updated the test
There was a problem hiding this comment.
Did the tests fail before you updated this test logic?
There was a problem hiding this comment.
No, but in that case the condition was not evaluating to true (due to CT reconciler being slightly slow to update the annotation than constraint controller in creating vapb once the "time window" is crossed) so the error was not being returned.
… is over to protect against clock skews Signed-off-by: Jaydip Gabani <[email protected]>
Signed-off-by: Jaydip Gabani <[email protected]>
JaydipGabani
force-pushed
the
vapb-annotation
branch
from
6573d8c to
6a17e69
Compare
Signed-off-by: Jaydip Gabani <[email protected]>
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when the PR gets merged):Fixes #3683
Special notes for your reviewer: