ROX-25948: Generic certificate requester#13280
Conversation
|
Skipping CI for Draft Pull Request. |
|
Images are ready for the commit at 0943635. To use with deploy scripts, first |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #13280 +/- ##
==========================================
- Coverage 48.52% 48.50% -0.03%
==========================================
Files 2471 2470 -1
Lines 178336 178320 -16
==========================================
- Hits 86533 86486 -47
- Misses 84870 84900 +30
- Partials 6933 6934 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
porridge
left a comment
There was a problem hiding this comment.
I think this is actually more readable than I would expect!
If we really have to keep both APIs around for 1+ year, then I think I'd prefer this incarnation, even though it's just ~130 lines shorter than the identical-siblings version.
a31d811 to
e069aa8
Compare
2388e44 to
b1a79da
Compare
3f47760 to
c284edd
Compare
jschnath
left a comment
There was a problem hiding this comment.
Nice test coverage, looks good to me
c284edd to
2dbda44
Compare
2dbda44 to
0943635
Compare
Description
This PR uses generics instead of code duplication for
localscanner.CertificateRequesterandsecuredcluster.CertificateRequester.Even though the Local Scanner certificate refresh mechanism will be deprecated starting with ACS 4.7, it will still be supported for backward compatibility with older Centrals that do not support the Secured Cluster cert refresh API. Since they'll have to co-exist for a longer time, it might be better if they reuse code instead of duplicating it.
User-facing documentation
Testing and quality
Automated testing
How I validated my change
This PR is a refactor and should not change functionality, existing tests should verify that.
I also verified with a local deployment that a set of Secured Cluster certificates is being created on Sensor startup, and that a refresh is being scheduled.